diff --git a/cmd/olliesrv/internal/agent/history.go b/cmd/olliesrv/internal/agent/history.go index 57c76e2..f057fa9 100644 --- a/cmd/olliesrv/internal/agent/history.go +++ b/cmd/olliesrv/internal/agent/history.go @@ -343,7 +343,9 @@ func buildCompactedHistory(summary string, allMessages []backend.Message) []back // Hot zone: verbatim recent messages. out = append(out, allMessages[hotStart:]...) - return out + + // Sanitize to ensure no dangling tool calls (defensive). + return backend.SanitizeMessages(out) } // summarizeMessage produces a one-line summary of a message for the warm index. diff --git a/cmd/olliesrv/internal/backend/validate.go b/cmd/olliesrv/internal/backend/validate.go new file mode 100644 index 0000000..759474c --- /dev/null +++ b/cmd/olliesrv/internal/backend/validate.go @@ -0,0 +1,77 @@ +package backend + +// SanitizeMessages filters a conversation history, removing invalid sequences. +// It ensures: +// 1. Every assistant message with ToolCalls has matching tool result messages +// 2. No orphan tool result messages (tool results without preceding tool calls) +// 3. System/user/assistant messages without tool calls are preserved +// +// The returned slice shares memory with the input — messages are not copied. +// This is primarily used when inheriting context for sub-agents. +func SanitizeMessages(msgs []Message) []Message { + if len(msgs) == 0 { + return msgs + } + + // We'll build a validated slice by scanning forward. + // Track pending tool call IDs that need results. + var result []Message + var pendingToolCalls map[string]bool // tool call IDs awaiting results + var toolCallStartIdx int // index in result where current tool sequence started + + for _, msg := range msgs { + switch msg.Role { + case "system", "user": + // System and user messages are always valid. + // But if we have pending tool calls, that's a problem — the sequence is broken. + if len(pendingToolCalls) > 0 { + // Truncate back to before the incomplete tool sequence + result = result[:toolCallStartIdx] + pendingToolCalls = nil + } + result = append(result, msg) + + case "assistant": + if len(msg.ToolCalls) > 0 { + // Assistant with tool calls — start tracking + if len(pendingToolCalls) > 0 { + // Previous tool calls weren't satisfied — truncate + result = result[:toolCallStartIdx] + } + pendingToolCalls = make(map[string]bool) + toolCallStartIdx = len(result) + for _, tc := range msg.ToolCalls { + pendingToolCalls[tc.ID] = true + } + result = append(result, msg) + } else { + // Assistant without tool calls + if len(pendingToolCalls) > 0 { + // Previous tool calls weren't satisfied — truncate + result = result[:toolCallStartIdx] + pendingToolCalls = nil + } + result = append(result, msg) + } + + case "tool": + if len(pendingToolCalls) == 0 { + // Orphan tool result — skip it + continue + } + if !pendingToolCalls[msg.ToolCallID] { + // Tool result for unknown call — skip it + continue + } + delete(pendingToolCalls, msg.ToolCallID) + result = append(result, msg) + } + } + + // Final check: if we still have pending tool calls, truncate + if len(pendingToolCalls) > 0 { + result = result[:toolCallStartIdx] + } + + return result +} diff --git a/cmd/olliesrv/internal/backend/validate_test.go b/cmd/olliesrv/internal/backend/validate_test.go new file mode 100644 index 0000000..0e3f80a --- /dev/null +++ b/cmd/olliesrv/internal/backend/validate_test.go @@ -0,0 +1,208 @@ +package backend_test + +import ( + "encoding/json" + "testing" + + "ollie/cmd/olliesrv/internal/backend" +) + +// TestSanitizeMessages defines what a valid conversation context means. +// Rules: +// 1. Every assistant message with ToolCalls must be followed by tool messages +// with matching ToolCallIDs (one tool message per tool call). +// 2. The conversation should not end with a user message expecting a response +// (this is actually fine for sub-agent inheritance — the sub-agent will respond). +// 3. System messages can appear anywhere but typically at the start. +// 4. Empty conversations are valid. + +func TestSanitizeMessages_Empty(t *testing.T) { + msgs := []backend.Message{} + got := backend.SanitizeMessages(msgs) + if len(got) != 0 { + t.Errorf("expected empty, got %d messages", len(got)) + } +} + +func TestSanitizeMessages_SimpleConversation(t *testing.T) { + msgs := []backend.Message{ + {Role: "user", Content: "Hello"}, + {Role: "assistant", Content: "Hi there!"}, + } + got := backend.SanitizeMessages(msgs) + if len(got) != 2 { + t.Errorf("expected 2 messages, got %d", len(got)) + } +} + +func TestSanitizeMessages_UserOnly(t *testing.T) { + // A conversation ending with user message is valid — assistant will respond + msgs := []backend.Message{ + {Role: "user", Content: "Hello"}, + } + got := backend.SanitizeMessages(msgs) + if len(got) != 1 { + t.Errorf("expected 1 message, got %d", len(got)) + } +} + +func TestSanitizeMessages_ToolCallWithResults(t *testing.T) { + // Valid: assistant calls tools, followed by tool results + msgs := []backend.Message{ + {Role: "user", Content: "Read the file"}, + {Role: "assistant", ToolCalls: []backend.ToolCall{ + {ID: "call_1", Name: "file_read", Arguments: json.RawMessage(`{"path":"/tmp/test"}`)}, + }}, + {Role: "tool", ToolCallID: "call_1", Content: "file contents here"}, + {Role: "assistant", Content: "The file contains..."}, + } + got := backend.SanitizeMessages(msgs) + if len(got) != 4 { + t.Errorf("expected 4 messages, got %d", len(got)) + } +} + +func TestSanitizeMessages_DanglingToolCall(t *testing.T) { + // Invalid: assistant has tool calls but no tool results follow + // Should drop the dangling assistant message + msgs := []backend.Message{ + {Role: "user", Content: "Read the file"}, + {Role: "assistant", ToolCalls: []backend.ToolCall{ + {ID: "call_1", Name: "file_read", Arguments: json.RawMessage(`{"path":"/tmp/test"}`)}, + }}, + } + got := backend.SanitizeMessages(msgs) + // Should drop the assistant message with dangling tool call + if len(got) != 1 { + t.Errorf("expected 1 message (user only), got %d", len(got)) + } + if got[0].Role != "user" { + t.Errorf("expected user message, got %s", got[0].Role) + } +} + +func TestSanitizeMessages_PartialToolResults(t *testing.T) { + // Invalid: assistant calls 2 tools but only 1 result follows + // Should drop the assistant + partial tool results + msgs := []backend.Message{ + {Role: "user", Content: "Read two files"}, + {Role: "assistant", ToolCalls: []backend.ToolCall{ + {ID: "call_1", Name: "file_read", Arguments: json.RawMessage(`{"path":"/tmp/a"}`)}, + {ID: "call_2", Name: "file_read", Arguments: json.RawMessage(`{"path":"/tmp/b"}`)}, + }}, + {Role: "tool", ToolCallID: "call_1", Content: "contents of a"}, + // Missing call_2 result + } + got := backend.SanitizeMessages(msgs) + // Should drop the incomplete tool call sequence + if len(got) != 1 { + t.Errorf("expected 1 message (user only), got %d", len(got)) + } +} + +func TestSanitizeMessages_MultipleToolCallSequences(t *testing.T) { + // Valid: multiple complete tool call sequences + msgs := []backend.Message{ + {Role: "user", Content: "Do stuff"}, + {Role: "assistant", ToolCalls: []backend.ToolCall{ + {ID: "call_1", Name: "tool_a", Arguments: json.RawMessage(`{}`)}, + }}, + {Role: "tool", ToolCallID: "call_1", Content: "result 1"}, + {Role: "assistant", ToolCalls: []backend.ToolCall{ + {ID: "call_2", Name: "tool_b", Arguments: json.RawMessage(`{}`)}, + }}, + {Role: "tool", ToolCallID: "call_2", Content: "result 2"}, + {Role: "assistant", Content: "Done!"}, + } + got := backend.SanitizeMessages(msgs) + if len(got) != 6 { + t.Errorf("expected 6 messages, got %d", len(got)) + } +} + +func TestSanitizeMessages_DanglingToolCallInMiddle(t *testing.T) { + // First tool call sequence is complete, second is dangling + msgs := []backend.Message{ + {Role: "user", Content: "Do stuff"}, + {Role: "assistant", ToolCalls: []backend.ToolCall{ + {ID: "call_1", Name: "tool_a", Arguments: json.RawMessage(`{}`)}, + }}, + {Role: "tool", ToolCallID: "call_1", Content: "result 1"}, + {Role: "assistant", Content: "First part done"}, + {Role: "user", Content: "Now do more"}, + {Role: "assistant", ToolCalls: []backend.ToolCall{ + {ID: "call_2", Name: "tool_b", Arguments: json.RawMessage(`{}`)}, + }}, + // Missing call_2 result + } + got := backend.SanitizeMessages(msgs) + // Should keep the first complete sequence, drop the dangling one + if len(got) != 5 { + t.Errorf("expected 5 messages, got %d", len(got)) + } + // Last message should be "Now do more" (user) + if got[len(got)-1].Role != "user" { + t.Errorf("expected last message to be user, got %s", got[len(got)-1].Role) + } +} + +func TestSanitizeMessages_OrphanToolResult(t *testing.T) { + // Tool result without preceding tool call — invalid, drop it + msgs := []backend.Message{ + {Role: "user", Content: "Hello"}, + {Role: "tool", ToolCallID: "orphan_call", Content: "orphan result"}, + {Role: "assistant", Content: "Hi"}, + } + got := backend.SanitizeMessages(msgs) + // Should drop the orphan tool result + if len(got) != 2 { + t.Errorf("expected 2 messages, got %d", len(got)) + } +} + +func TestSanitizeMessages_SystemMessage(t *testing.T) { + // System messages are preserved + msgs := []backend.Message{ + {Role: "system", Content: "You are helpful"}, + {Role: "user", Content: "Hello"}, + {Role: "assistant", Content: "Hi"}, + } + got := backend.SanitizeMessages(msgs) + if len(got) != 3 { + t.Errorf("expected 3 messages, got %d", len(got)) + } +} + +func TestSanitizeMessages_AssistantWithContentAndToolCalls(t *testing.T) { + // Assistant message can have both content and tool calls + msgs := []backend.Message{ + {Role: "user", Content: "Read the file and summarize"}, + {Role: "assistant", Content: "I'll read the file now.", ToolCalls: []backend.ToolCall{ + {ID: "call_1", Name: "file_read", Arguments: json.RawMessage(`{"path":"/tmp/test"}`)}, + }}, + {Role: "tool", ToolCallID: "call_1", Content: "file contents"}, + {Role: "assistant", Content: "The file contains..."}, + } + got := backend.SanitizeMessages(msgs) + if len(got) != 4 { + t.Errorf("expected 4 messages, got %d", len(got)) + } +} + +func TestSanitizeMessages_ParallelToolCalls(t *testing.T) { + // Valid: assistant calls multiple tools in parallel, all results present + msgs := []backend.Message{ + {Role: "user", Content: "Read both files"}, + {Role: "assistant", ToolCalls: []backend.ToolCall{ + {ID: "call_1", Name: "file_read", Arguments: json.RawMessage(`{"path":"/tmp/a"}`)}, + {ID: "call_2", Name: "file_read", Arguments: json.RawMessage(`{"path":"/tmp/b"}`)}, + }}, + {Role: "tool", ToolCallID: "call_1", Content: "contents of a"}, + {Role: "tool", ToolCallID: "call_2", Content: "contents of b"}, + {Role: "assistant", Content: "Both files contain..."}, + } + got := backend.SanitizeMessages(msgs) + if len(got) != 5 { + t.Errorf("expected 5 messages, got %d", len(got)) + } +} diff --git a/cmd/olliesrv/internal/fs/spec.go b/cmd/olliesrv/internal/fs/spec.go index 95260e0..1d9f995 100644 --- a/cmd/olliesrv/internal/fs/spec.go +++ b/cmd/olliesrv/internal/fs/spec.go @@ -253,7 +253,7 @@ func buildTreeSpec(cfg *Config) virtfs.FsNodeDecl { } } if cwd == "" { - cwd, _ = os.Getwd() + return nil, fmt.Errorf("cwd is required") } sess, err := session.CreateEmpty(name, remote) if err != nil { @@ -460,9 +460,6 @@ func buildSessionChildren( virtfs.Rdwr(func(ctx context.Context, data []byte) ([]byte, error) { req := parseAgentNewRequest(data) req.Params.ParentID = req.ParentID - if req.Cwd != "" { - s.SetCwd(req.Cwd) - } if req.Prompt == "" { // Persistent agent creation (no sub-agent mode). @@ -513,6 +510,8 @@ func buildSessionChildren( if req.ForkAt > 0 { msgs = messagesUpToTurn(msgs, req.ForkAt) } + // Sanitize to remove dangling tool calls + msgs = backend.SanitizeMessages(msgs) req.Params.History = agent.RestoreHistoryFromMessages(msgs) parent.IncChildren() defer parent.DecChildren() @@ -916,15 +915,7 @@ func buildAgentChildren(a *agent.Agent, s *session.Session) []virtfs.FsNodeDecl return []byte(strings.Join(args, " ") + "\n"), nil }, "cwd": func(args []string) ([]byte, error) { - if len(args) == 0 { - return []byte(a.Cwd() + "\n"), nil - } - dir := util.ExpandHome(strings.Join(args, " ")) - if _, err := os.Stat(dir); err != nil { - return nil, fmt.Errorf("cwd: %w", err) - } - a.SetCWD(dir) - return []byte(dir + "\n"), nil + return []byte(a.Cwd() + "\n"), nil }, "proc": func(args []string) ([]byte, error) { ts := a.ToolServer() diff --git a/cmd/olliesrv/internal/fs/support.go b/cmd/olliesrv/internal/fs/support.go index 0586d2f..003fb27 100644 --- a/cmd/olliesrv/internal/fs/support.go +++ b/cmd/olliesrv/internal/fs/support.go @@ -92,7 +92,6 @@ type agentNewRequest struct { Params session.AgentParams Prompt string ParentID string - Cwd string // sets session cwd if provided ForkAt int Timeout int // seconds; 0 = use default (600) MaxDepth int // max sub-agent depth; 0 = use default (1) @@ -126,8 +125,6 @@ func parseAgentNewRequest(data []byte) agentNewRequest { if k, v, ok := strings.Cut(tok, "="); ok { v = protocol.UnescapeValue(v) switch k { - case "cwd": - r.Cwd = v case "backend": r.Params.Backend = v case "model": diff --git a/cmd/olliesrv/internal/session/persist.go b/cmd/olliesrv/internal/session/persist.go index 732bd4b..70f3ea7 100644 --- a/cmd/olliesrv/internal/session/persist.go +++ b/cmd/olliesrv/internal/session/persist.go @@ -84,7 +84,7 @@ func PersistSession(name string) error { Profile: ag.Profile(), Backend: ag.BackendName(), Model: ag.ModelName(), - Messages: ag.Messages(), + Messages: backend.SanitizeMessages(ag.Messages()), } if usage := ag.Usage(); usage != nil { pa.TotalInputTokens = usage.TotalInputTokens diff --git a/cmd/olliesrv/internal/session/registry.go b/cmd/olliesrv/internal/session/registry.go index 4dfc129..b0346cb 100644 --- a/cmd/olliesrv/internal/session/registry.go +++ b/cmd/olliesrv/internal/session/registry.go @@ -458,11 +458,10 @@ func buildAgent(sess *Session, p AgentParams) (*agent.Agent, error) { } // CreateAgent creates a new agent within an existing session. -// Args: [profile] [key=value...] where keys include name, backend, model, remote, cwd. +// Args: [profile] [key=value...] where keys include name, backend, model, remote. func CreateAgent(sessName string, args []string) (*agent.Agent, error) { // Parse args into AgentParams. p := AgentParams{Profile: "default"} - var setCwd string for _, arg := range args { if k, v, ok := strings.Cut(arg, "="); ok { switch k { @@ -474,8 +473,6 @@ func CreateAgent(sessName string, args []string) (*agent.Agent, error) { p.Model = v case "remote": p.Remote = v - case "cwd": - setCwd = v } } else if p.Profile == "default" { p.Profile = arg @@ -485,9 +482,6 @@ func CreateAgent(sessName string, args []string) (*agent.Agent, error) { if sess == nil { return nil, fmt.Errorf("session not found: %s", sessName) } - if setCwd != "" { - sess.SetCwd(setCwd) - } return CreateAgentWithParams(sessName, p) } diff --git a/kde/dolphin/ollie-session-here b/kde/dolphin/ollie-session-here index faa4ce7..fb6c0ab 100755 --- a/kde/dolphin/ollie-session-here +++ b/kde/dolphin/ollie-session-here @@ -15,7 +15,7 @@ AGENT_NAME="$(basename "$TARGET")" # Create default session (idempotent) and named agent printf 'name=default\ncwd=%s\n' "$TARGET" | ollie-9p write session/new -printf 'name=%s\ncwd=%s\n' "$AGENT_NAME" "$TARGET" | ollie-9p write session/default/agent/new +printf 'name=%s\n' "$AGENT_NAME" | ollie-9p write session/default/agent/new if [ -t 1 ]; then # Terminal: print session info diff --git a/kde/gui/NewAgentDialog.qml b/kde/gui/NewAgentDialog.qml index d17f323..da5b4bb 100644 --- a/kde/gui/NewAgentDialog.qml +++ b/kde/gui/NewAgentDialog.qml @@ -79,24 +79,6 @@ Dialog { wrapMode: Text.Wrap } - RowLayout { - Layout.fillWidth: true - Label { text: "Directory:"; Layout.preferredWidth: 80 } - TextField { - id: dirField - Layout.fillWidth: true - placeholderText: "/path/to/project" - } - Button { - text: "Browse..." - onClicked: { - var folder = theme.chooseFolder("Select Project Directory", dirField.text || "") - if (folder !== "") - dirField.text = folder - } - } - } - RowLayout { Layout.fillWidth: true Label { text: "Backend:"; Layout.preferredWidth: 80 } @@ -180,17 +162,12 @@ Dialog { text: "OK" onClicked: { errorLabel.text = "" - var cwd = dirField.text.trim() - if (cwd === "") { - errorLabel.text = "Directory is required" - return - } var backend = backendCombo.currentText.trim() var model = modelCombo.currentIndex >= 0 ? modelCombo.currentText.trim() : "" var agent = agentCombo.currentText var agentAlias = aliasField.text.trim() - var newAgentId = ollie.createAgent(root.targetSessionId, cwd, backend, model, agent, "", agentAlias) + var newAgentId = ollie.createAgent(root.targetSessionId, backend, model, agent, "", agentAlias) var err = ollie.lastError() if (err === "") { // Success: refresh then switch to the newly created agent. diff --git a/kde/gui/NewSessionDialog.qml b/kde/gui/NewSessionDialog.qml index 7e57ee2..5cd34aa 100644 --- a/kde/gui/NewSessionDialog.qml +++ b/kde/gui/NewSessionDialog.qml @@ -85,6 +85,24 @@ Dialog { } } + RowLayout { + Layout.fillWidth: true + Label { text: "Directory:"; Layout.preferredWidth: 80 } + TextField { + id: cwdField + Layout.fillWidth: true + placeholderText: "/path/to/project" + } + Button { + text: "Browse..." + onClicked: { + var folder = theme.chooseFolder("Select Working Directory", cwdField.text || "") + if (folder !== "") + cwdField.text = folder + } + } + } + RowLayout { Layout.fillWidth: true Label { text: "Remote:"; Layout.preferredWidth: 80 } @@ -125,9 +143,10 @@ Dialog { errorLabel.text = "" var name = nameField.text.trim() var remote = remoteField.text.trim() + var cwd = cwdField.text.trim() var workflow = workflowCombo.currentText - var err = ollie.createSession(name, remote, workflow) + var err = ollie.createSession(name, remote, cwd, workflow) if (err === "") { ollie.refreshSessions() root.sessionCreated() @@ -149,6 +168,7 @@ Dialog { onOpened: { errorLabel.text = "" nameField.text = "" + cwdField.text = "" remoteField.text = "" var idx = ollie.availableWorkflows.indexOf("conductor") workflowCombo.currentIndex = idx >= 0 ? idx : 0 diff --git a/kde/gui/ollie9pclient.cpp b/kde/gui/ollie9pclient.cpp index 1a863cf..005814b 100644 --- a/kde/gui/ollie9pclient.cpp +++ b/kde/gui/ollie9pclient.cpp @@ -609,13 +609,14 @@ bool Ollie9pClient::createQuickSession() return !result.isEmpty() || m_9p->lastError().isEmpty(); } -QString Ollie9pClient::createSession(const QString &name, const QString &remote, const QString &workflow) +QString Ollie9pClient::createSession(const QString &name, const QString &remote, const QString &cwd, const QString &workflow) { if (!m_9p || !m_9p->isConnected()) return QStringLiteral("not connected"); QStringList args; if (!name.isEmpty()) args << "name=" + name; if (!remote.isEmpty()) args << "remote=" + remote; + if (!cwd.isEmpty()) args << "cwd=" + cwd; if (!workflow.isEmpty()) args << "workflow=" + workflow; QByteArray result = m_9p->rdwr(QStringLiteral("session/new"), (args.join(" ") + "\n").toUtf8()); @@ -635,13 +636,12 @@ void Ollie9pClient::setGoal(const QString &sessionName, const QString &goalText) m_9p->write(path, goalText.toUtf8()); } -QString Ollie9pClient::createAgent(const QString &sessionId, const QString &cwd, const QString &backend, const QString &model, const QString &agent, const QString &remote, const QString &agentAlias) +QString Ollie9pClient::createAgent(const QString &sessionId, const QString &backend, const QString &model, const QString &agent, const QString &remote, const QString &agentAlias) { - if (sessionId.isEmpty() || cwd.isEmpty()) return QStringLiteral("session ID and directory are required"); + if (sessionId.isEmpty()) return QStringLiteral("session ID is required"); if (!m_9p || !m_9p->isConnected()) return QStringLiteral("not connected"); QStringList agentArgs; - agentArgs << "cwd=" + cwd; if (!backend.isEmpty()) agentArgs << "backend=" + backend; if (!model.isEmpty()) agentArgs << "model=" + model; if (!agent.isEmpty()) agentArgs << "agent=" + agent; diff --git a/kde/gui/ollie9pclient.h b/kde/gui/ollie9pclient.h index ffe8dbd..56217b1 100644 --- a/kde/gui/ollie9pclient.h +++ b/kde/gui/ollie9pclient.h @@ -85,9 +85,9 @@ public: Q_INVOKABLE void loadWorkflows(); Q_INVOKABLE QStringList getAvailableModels(const QString &backend) const; Q_INVOKABLE bool createQuickSession(); - Q_INVOKABLE QString createSession(const QString &name, const QString &remote, const QString &workflow = QString()); + Q_INVOKABLE QString createSession(const QString &name, const QString &remote, const QString &cwd = QString(), const QString &workflow = QString()); Q_INVOKABLE void setGoal(const QString &sessionName, const QString &goalText); - Q_INVOKABLE QString createAgent(const QString &sessionId, const QString &cwd, const QString &backend, const QString &model, const QString &agent, const QString &remote, const QString &agentAlias); + Q_INVOKABLE QString createAgent(const QString &sessionId, const QString &backend, const QString &model, const QString &agent, const QString &remote, const QString &agentAlias); Q_INVOKABLE bool killAgent(const QString &sessionId, const QString &agentId); Q_INVOKABLE bool renameSession(const QString &sessionId, const QString &newName); Q_INVOKABLE bool renameAgent(const QString &sessionId, const QString &agentId, const QString &newName); diff --git a/kde/kate/ollie_kate.cpp b/kde/kate/ollie_kate.cpp index 7dbc186..67a24bf 100644 --- a/kde/kate/ollie_kate.cpp +++ b/kde/kate/ollie_kate.cpp @@ -203,7 +203,9 @@ void OllieKateView::ensureAgent() // Ensure kate session exists if (m_sessionId.isEmpty()) { - m_9p->write("session/new", "name=kate\n"); + QString cwd = projectBaseDir(); + if (cwd.isEmpty()) cwd = QDir::homePath(); + m_9p->write("session/new", ("name=kate\ncwd=" + cwd + "\n").toUtf8()); m_sessionId = "kate"; }