fix(agent): strip whole text-encoded tool-call blocks, not just tags (#1260)

stripGarbledToolXML removed only the tags of a model-emitted
<function_calls> block, leaking the inner <parameter> argument values
into the user-facing reply, and the dropped call was completely silent.
Detect a complete tool-call block, remove it whole, and log the
attempted tool name(s) at WARN so the no-op is diagnosable. Partial
artifacts still fall through to the existing tag-level strip.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

internal/agent/sanitize.go

@@ -82,6 +82,20 @@ var garbledToolXMLPattern = regexp.MustCompile(
 	`(?s)</?(?:function_calls?|functioninvoke|invoke|invfunction_calls|tool_call|tool_use|parameter|minimax:tool_call)[^>]*>`,
 )
 
+// fullToolCallBlockPattern matches a COMPLETE Anthropic-style tool-call block
+// (`<function_calls>...</function_calls>`) 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 <parameter> 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)<function_calls?>.*?</function_calls?>`,
+)
+
+// invokeNamePattern extracts the tool name from an `<invoke name="...">` tag so a
+// dropped text-encoded tool call can be logged with the tool it tried to call.
+var invokeNamePattern = regexp.MustCompile(`(?i)<invoke\s+name="([^"]+)"`)
+
 var garbledToolXMLIndicators = []string{
 	"invfunction_calls",
 	"functioninvoke",
@@ -106,18 +120,44 @@ func stripGarbledToolXML(content string) string {
 		return content
 	}
 
-	cleaned := garbledToolXMLPattern.ReplaceAllString(content, "")
-	cleaned = strings.TrimSpace(cleaned)
+	original := content
+
+	// A COMPLETE <function_calls>...</function_calls> 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 <parameter> 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
 }
 

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" +
+				"<function_calls>\n<invoke name=\"send_message\">\n" +
+				"<parameter name=\"channel\">argus</parameter>\n" +
+				"<parameter name=\"text\">PR #1217 approved</parameter>\n" +
+				"</invoke>\n</function_calls>",
+			want: "PR #1217 looks good - approving.",
+		},
+		{
+			name: "block-only response collapses to empty",
+			input: "<function_calls><invoke name=\"send_message\">" +
+				"<parameter name=\"text\">hi</parameter></invoke></function_calls>",
+			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 <tool_call>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 <parameter> 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<function_calls>\n<invoke name=\"send_message\">\n" +
+		"<parameter name=\"channel\">argus</parameter>\n" +
+		"<parameter name=\"text\">SECRET_ARG_VALUE</parameter>\n" +
+		"</invoke>\n</function_calls>"
+
+	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.")
+	}
+}