diff --git a/internal/agent/sanitize.go b/internal/agent/sanitize.go index a5e77841..fcb68a11 100644 --- a/internal/agent/sanitize.go +++ b/internal/agent/sanitize.go @@ -82,6 +82,20 @@ var garbledToolXMLPattern = regexp.MustCompile( `(?s)]*>`, ) +// fullToolCallBlockPattern matches a COMPLETE Anthropic-style tool-call block +// (`...`) that a model emitted as text instead +// of invoking it natively. Unlike garbledToolXMLPattern — which removes only the +// tags — this captures the whole block, tags plus the inner text, so +// a stray block is removed cleanly rather than leaving the argument values +// orphaned in the user-facing reply. +var fullToolCallBlockPattern = regexp.MustCompile( + `(?is).*?`, +) + +// invokeNamePattern extracts the tool name from an `` tag so a +// dropped text-encoded tool call can be logged with the tool it tried to call. +var invokeNamePattern = regexp.MustCompile(`(?i)... block is not "garble" — it + // is a tool call the model wrote as TEXT instead of invoking it natively. This + // shows up with the claude-cli thin-proxy provider, where tool execution lives + // inside the CLI: when the model emits the call as text the tool never runs, + // and the tag-only strip below would leave the inner values + // mangled into the reply. Remove whole blocks and log the attempted tool + // name(s) at WARN so this otherwise-silent no-op is diagnosable. + if blocks := fullToolCallBlockPattern.FindAllString(content, -1); len(blocks) > 0 { + tools := make([]string, 0, len(blocks)) + for _, b := range blocks { + for _, m := range invokeNamePattern.FindAllStringSubmatch(b, -1) { + tools = append(tools, m[1]) + } + } + slog.Warn("dropped text-encoded tool call from response", + "tools", tools, + "blocks", len(blocks), + "hint", "model wrote a tool call as text instead of invoking it; the tool did not run", + ) + content = fullToolCallBlockPattern.ReplaceAllString(content, "") + } + + // Strip any remaining stray tags (partial DeepSeek/GLM/Minimax artifacts). + cleaned := strings.TrimSpace(garbledToolXMLPattern.ReplaceAllString(content, "")) if cleaned == "" { - slog.Warn("stripped entire response as garbled tool XML", "original_len", len(content)) + slog.Warn("stripped entire response as garbled tool XML", "original_len", len(original)) return "" } - slog.Warn("stripped garbled tool call XML from response", - "original_len", len(content), - "remaining_len", len(cleaned), - ) + if cleaned != original { + slog.Warn("stripped garbled tool call XML from response", + "original_len", len(original), + "remaining_len", len(cleaned), + ) + } return cleaned } diff --git a/internal/agent/sanitize_toolcall_xml_test.go b/internal/agent/sanitize_toolcall_xml_test.go new file mode 100644 index 00000000..e3513617 --- /dev/null +++ b/internal/agent/sanitize_toolcall_xml_test.go @@ -0,0 +1,68 @@ +package agent + +import ( + "strings" + "testing" +) + +func TestStripGarbledToolXML_FullToolCallBlock(t *testing.T) { + tests := []struct { + name string + input string + want string + }{ + { + name: "removes whole block, keeps surrounding prose", + input: "PR #1217 looks good - approving.\n\n" + + "\n\n" + + "argus\n" + + "PR #1217 approved\n" + + "\n", + want: "PR #1217 looks good - approving.", + }, + { + name: "block-only response collapses to empty", + input: "" + + "hi", + want: "", + }, + { + name: "no tool xml — unchanged", + input: "Just a normal reply with no tool calls.", + want: "Just a normal reply with no tool calls.", + }, + { + name: "partial stray tag still stripped (backward compat)", + input: "Result here oops", + want: "Result here oops", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := stripGarbledToolXML(tt.input) + if got != tt.want { + t.Errorf("stripGarbledToolXML() = %q, want %q", got, tt.want) + } + }) + } +} + +// Regression: the tag-only strip used to leave argument values +// orphaned in the reply when a model emitted a tool call as text instead of +// invoking it natively (claude-cli thin-proxy). The whole block must be removed. +func TestSanitizeAssistantContent_NoToolCallArgumentLeak(t *testing.T) { + input := "Done.\n\n\n" + + "argus\n" + + "SECRET_ARG_VALUE\n" + + "\n" + + got := SanitizeAssistantContent(input) + + if strings.Contains(got, "SECRET_ARG_VALUE") || strings.Contains(got, "argus") { + t.Errorf("tool-call argument values leaked into reply: %q", got) + } + if got != "Done." { + t.Errorf("SanitizeAssistantContent() = %q, want %q", got, "Done.") + } +}