diff --git a/pkg/agent/agent_utils.go b/pkg/agent/agent_utils.go index 85228b5869..65061ed0bb 100644 --- a/pkg/agent/agent_utils.go +++ b/pkg/agent/agent_utils.go @@ -69,8 +69,124 @@ func outboundTurnMetadata( return agentID, sessionKey, outboundScopeFromSessionScope(scope) } +var toolProtocolMarkers = [...]string{ + "[tool_use", + "[tool_user", + "[tool_result", +} + +// findToolProtocolMarker returns the next internal tool-protocol marker. +// Providers occasionally duplicate a native tool call in assistant content as +// text. Searching explicitly keeps ordinary square-bracketed text untouched. +func findToolProtocolMarker(content string) int { + lower := strings.ToLower(content) + next := -1 + for _, marker := range toolProtocolMarkers { + if idx := strings.Index(lower, marker); idx >= 0 && (next < 0 || idx < next) { + next = idx + } + } + return next +} + +// toolProtocolBlockEnd finds the closing bracket for an internal tool block. +// It understands quoted strings and nested arrays, so JSON arguments containing +// braces, brackets, or escaped quotes cannot prematurely terminate the scan. +func toolProtocolBlockEnd(content string) int { + if content == "" || content[0] != '[' { + return -1 + } + + depth := 0 + inString := false + escaped := false + for i := 0; i < len(content); i++ { + ch := content[i] + if inString { + if escaped { + escaped = false + continue + } + switch ch { + case '\\': + escaped = true + case '"': + inString = false + } + continue + } + + switch ch { + case '"': + inString = true + case '[': + depth++ + case ']': + depth-- + if depth == 0 { + return i + 1 + } + } + } + return -1 +} + +// stripToolUseText removes internal tool protocol syntax that must never reach +// users or conversation history. It accepts complete blocks, nested JSON, old +// tool_user variants, tool results, and truncated blocks. +func stripToolUseText(content string) string { + var cleaned strings.Builder + remaining := content + + for { + start := findToolProtocolMarker(remaining) + if start < 0 { + cleaned.WriteString(remaining) + break + } + cleaned.WriteString(remaining[:start]) + + end := toolProtocolBlockEnd(remaining[start:]) + if end >= 0 { + remaining = remaining[start+end:] + continue + } + + // A truncated provider response has no closing bracket. Drop the rest + // of that line, but preserve any subsequent human-readable answer. + if newline := strings.IndexByte(remaining[start:], '\n'); newline >= 0 { + remaining = remaining[start+newline+1:] + continue + } + break + } + + return strings.TrimSpace(cleaned.String()) +} + +// sanitizeLLMResponseToolProtocol removes textual copies of native tool calls +// at the provider boundary, before they can be published, persisted, compacted, +// or reused as tool feedback. +func sanitizeLLMResponseToolProtocol(response *providers.LLMResponse) { + if response == nil { + return + } + response.Content = stripToolUseText(response.Content) + response.Reasoning = stripToolUseText(response.Reasoning) + response.ReasoningContent = stripToolUseText(response.ReasoningContent) + for i := range response.ToolCalls { + if response.ToolCalls[i].ExtraContent == nil { + continue + } + response.ToolCalls[i].ExtraContent.ToolFeedbackExplanation = stripToolUseText( + response.ToolCalls[i].ExtraContent.ToolFeedbackExplanation, + ) + } +} + func outboundMessageForTurn(ts *turnState, content string) bus.OutboundMessage { agentID, sessionKey, scope := outboundTurnMetadata(ts.agent.ID, ts.sessionKey, ts.opts.Dispatch.SessionScope) + content = stripToolUseText(content) return bus.OutboundMessage{ Channel: ts.channel, ChatID: ts.chatID, diff --git a/pkg/agent/pipeline_llm.go b/pkg/agent/pipeline_llm.go index b4c87634ed..06d6074caa 100644 --- a/pkg/agent/pipeline_llm.go +++ b/pkg/agent/pipeline_llm.go @@ -495,6 +495,11 @@ func (p *Pipeline) CallLLM( } } + // Some OpenAI-compatible providers return a native tool_calls payload and + // duplicate the same call in assistant content as "[tool_use: ...]". + // Normalize it here, after hooks and before any publishing or persistence. + sanitizeLLMResponseToolProtocol(exec.response) + // Save finishReason and usage on the turn state. Use ts directly (the // authoritative turn state for this call) rather than a context lookup: // the raw ctx passed to CallLLM is not seeded with turnState (only turnCtx diff --git a/pkg/agent/tool_protocol_sanitize_test.go b/pkg/agent/tool_protocol_sanitize_test.go new file mode 100644 index 0000000000..7e42dea2cf --- /dev/null +++ b/pkg/agent/tool_protocol_sanitize_test.go @@ -0,0 +1,105 @@ +package agent + +import ( + "testing" + + "github.com/sipeed/picoclaw/pkg/providers" +) + +func TestStripToolUseText(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + content string + want string + }{ + { + name: "native tool call duplicate", + content: `[tool_use: exec, args: {"action":"run","command":"ls -la","timeout":5}]`, + want: "", + }, + { + name: "nested JSON and brackets in quoted command", + content: `[tool_use: exec, args: {"action":"run","options":{"env":{"MODE":"scan"}},"command":"printf '[%s]' \"ok\"","items":[1,{"nested":true}]}}]`, + want: "", + }, + { + name: "tool user variant without args label", + content: `[tool_user: exec, {"command":"id"}]`, + want: "", + }, + { + name: "tool result", + content: `Before [tool_result: {"ok":true,"data":[1,2]}] after`, + want: "Before after", + }, + { + name: "mixed explanation and tool call", + content: "I will inspect it.\n[tool_use: exec, args: {\"command\":\"find /var -name '*.log'\"}]\nPlease wait.", + want: "I will inspect it.\n\nPlease wait.", + }, + { + name: "truncated block only", + content: `[tool_use: exec, args: {"command":"id"}`, + want: "", + }, + { + name: "truncated block preserves next line", + content: "Checking now.\n[tool_use exec args={\"command\":\"id\"}\nA readable answer follows.", + want: "Checking now.\nA readable answer follows.", + }, + { + name: "case insensitive", + content: `[TOOL_USE: exec, args: {"command":"id"}]`, + want: "", + }, + { + name: "ordinary brackets untouched", + content: "Use [tool_usage] and JSON {\"result\":\"ok\"}.", + want: "Use [tool_usage] and JSON {\"result\":\"ok\"}.", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if got := stripToolUseText(tt.content); got != tt.want { + t.Fatalf("stripToolUseText() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestSanitizeLLMResponseToolProtocol(t *testing.T) { + t.Parallel() + + response := &providers.LLMResponse{ + Content: `[tool_use: exec, args: {"action":"run","command":"ls"}]`, + Reasoning: `Inspecting. [tool_user: exec, {"command":"ls"}]`, + ReasoningContent: `[tool_result: {"files":["a","b"]}] Done.`, + ToolCalls: []providers.ToolCall{{ + ID: "call_1", + Name: "exec", + ExtraContent: &providers.ExtraContent{ + ToolFeedbackExplanation: `[tool_use: exec, args: {"action":"run","command":"ls"}]`, + }, + }}, + } + + sanitizeLLMResponseToolProtocol(response) + + if response.Content != "" { + t.Fatalf("Content = %q, want empty", response.Content) + } + if response.Reasoning != "Inspecting." { + t.Fatalf("Reasoning = %q, want %q", response.Reasoning, "Inspecting.") + } + if response.ReasoningContent != "Done." { + t.Fatalf("ReasoningContent = %q, want %q", response.ReasoningContent, "Done.") + } + if got := response.ToolCalls[0].ExtraContent.ToolFeedbackExplanation; got != "" { + t.Fatalf("ToolFeedbackExplanation = %q, want empty", got) + } +} diff --git a/pkg/seahorse/parts_roundtrip_test.go b/pkg/seahorse/parts_roundtrip_test.go index 02df8a9ead..e69ad92e73 100644 --- a/pkg/seahorse/parts_roundtrip_test.go +++ b/pkg/seahorse/parts_roundtrip_test.go @@ -2,6 +2,7 @@ package seahorse import ( "context" + "strings" "testing" "time" ) @@ -142,3 +143,28 @@ func TestSearchMessagesFindsPartBasedMessages(t *testing.T) { t.Error("SearchMessages: 'TODO fix' not found — tool_result messages are invisible to search") } } + +func TestPartsToReadableContentAvoidsInternalToolProtocol(t *testing.T) { + content := partsToReadableContent([]MessagePart{ + { + Type: "tool_use", + Name: "exec", + Arguments: `{"command":"find /var -name '*.log'"}`, + }, + { + Type: "tool_result", + Text: "scan complete", + }, + }) + + for _, marker := range []string{"[tool_use", "[tool_user", "[tool_result"} { + if strings.Contains(strings.ToLower(content), marker) { + t.Fatalf("partsToReadableContent() leaked internal marker %q in %q", marker, content) + } + } + for _, want := range []string{"exec", "find /var", "scan complete"} { + if !strings.Contains(content, want) { + t.Fatalf("partsToReadableContent() = %q, want searchable text %q", content, want) + } + } +} diff --git a/pkg/seahorse/store.go b/pkg/seahorse/store.go index 342fd855b3..17a09d7762 100644 --- a/pkg/seahorse/store.go +++ b/pkg/seahorse/store.go @@ -224,9 +224,9 @@ func partsToReadableContent(parts []MessagePart) string { case "text": b.WriteString(p.Text) case "tool_use": - fmt.Fprintf(&b, "[tool_use: %s, args: %s]", p.Name, p.Arguments) + fmt.Fprintf(&b, "Tool call: %s\nArguments: %s", p.Name, p.Arguments) case "tool_result": - fmt.Fprintf(&b, "[tool_result for %s: %s]", p.ToolCallID, p.Text) + fmt.Fprintf(&b, "Tool result: %s", p.Text) case "media": fmt.Fprintf(&b, "[media: %s (%s)]", p.MediaURI, p.MimeType) default: