From c3923f2ccd0852fd90fa75a1ab2fb1d3054c2982 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Thu, 12 Mar 2026 06:49:53 -0700 Subject: [PATCH] fix(chatd): keep provider-executed tool results in assistant content (#22991) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem Anthropic's API returns a 400 error when `web_search` tool results are missing: ``` web_search tool use with id srvtoolu_... was found without a corresponding web_search_tool_result block ``` **Root cause:** `persistStep` in `chatd.go` splits ALL `ToolResultContent` blocks into separate tool-role DB rows. Provider-executed (PE) tool results like `web_search` must stay in the assistant message — Anthropic expects `server_tool_use` and `web_search_tool_result` in the same turn. The previous fix (#22976) added repair passes to drop PE results during reconstruction, which fixed cross-step orphans but broke the normal case (PE result correctly in the same step). ## Fix Three changes that address the root cause: 1. **`persistStep` (chatd.go):** Check `ProviderExecuted` before splitting `ToolResultContent` into tool rows. PE results stay in `assistantBlocks` and are stored in the assistant content column. 2. **`ToMessageParts` (chatprompt.go):** Propagate the `ProviderExecuted` field to `ToolResultPart` so the fantasy Anthropic provider can identify PE results and reconstruct the `web_search_tool_result` block. 3. **Keep existing repair passes** for backward compatibility with legacy DB data where PE results were incorrectly persisted as separate tool messages. ## Tests - `TestProviderExecutedResultInAssistantContent` — PE result stored inline in assistant content round-trips correctly with `ProviderExecuted` preserved. - `TestProviderExecutedResult_LegacyToolRow` — legacy PE results in tool-role rows are still dropped correctly. - All existing tests pass (including the 3 PE tests from #22976). --- coderd/chatd/chatd.go | 17 ++- coderd/chatd/chatprompt/chatprompt.go | 14 ++- coderd/chatd/chatprompt/chatprompt_test.go | 124 +++++++++++++++++++++ 3 files changed, 144 insertions(+), 11 deletions(-) diff --git a/coderd/chatd/chatd.go b/coderd/chatd/chatd.go index de3c2676b9..566a23af4b 100644 --- a/coderd/chatd/chatd.go +++ b/coderd/chatd/chatd.go @@ -2298,17 +2298,24 @@ func (p *Server) runChat( // Split the step content into assistant blocks and tool // result blocks so they can be stored as separate messages - // with the appropriate roles. + // with the appropriate roles. Provider-executed tool results + // (e.g. web_search) stay in the assistant content because + // the LLM provider expects them inline in the assistant + // turn, not as separate tool messages. var assistantBlocks []fantasy.Content var toolResults []fantasy.ToolResultContent for _, block := range step.Content { if tr, ok := fantasy.AsContentType[fantasy.ToolResultContent](block); ok { - toolResults = append(toolResults, tr) - continue + if !tr.ProviderExecuted { + toolResults = append(toolResults, tr) + continue + } } if trPtr, ok := fantasy.AsContentType[*fantasy.ToolResultContent](block); ok && trPtr != nil { - toolResults = append(toolResults, *trPtr) - continue + if !trPtr.ProviderExecuted { + toolResults = append(toolResults, *trPtr) + continue + } } assistantBlocks = append(assistantBlocks, block) } diff --git a/coderd/chatd/chatprompt/chatprompt.go b/coderd/chatd/chatprompt/chatprompt.go index 9a0a7825cf..72a35b3a10 100644 --- a/coderd/chatd/chatprompt/chatprompt.go +++ b/coderd/chatd/chatprompt/chatprompt.go @@ -516,15 +516,17 @@ func ToMessageParts(content []fantasy.Content) []fantasy.MessagePart { }) case fantasy.ToolResultContent: parts = append(parts, fantasy.ToolResultPart{ - ToolCallID: sanitizeToolCallID(value.ToolCallID), - Output: value.Result, - ProviderOptions: fantasy.ProviderOptions(value.ProviderMetadata), + ToolCallID: sanitizeToolCallID(value.ToolCallID), + ProviderExecuted: value.ProviderExecuted, + Output: value.Result, + ProviderOptions: fantasy.ProviderOptions(value.ProviderMetadata), }) case *fantasy.ToolResultContent: parts = append(parts, fantasy.ToolResultPart{ - ToolCallID: sanitizeToolCallID(value.ToolCallID), - Output: value.Result, - ProviderOptions: fantasy.ProviderOptions(value.ProviderMetadata), + ToolCallID: sanitizeToolCallID(value.ToolCallID), + ProviderExecuted: value.ProviderExecuted, + Output: value.Result, + ProviderOptions: fantasy.ProviderOptions(value.ProviderMetadata), }) } } diff --git a/coderd/chatd/chatprompt/chatprompt_test.go b/coderd/chatd/chatprompt/chatprompt_test.go index 201ea0eb11..ab593e3926 100644 --- a/coderd/chatd/chatprompt/chatprompt_test.go +++ b/coderd/chatd/chatprompt/chatprompt_test.go @@ -463,6 +463,130 @@ func TestInjectMissingToolUses_DropsOnlyProviderExecutedMessage(t *testing.T) { require.Equal(t, fantasy.MessageRoleAssistant, prompt[2].Role) } +// TestProviderExecutedResultInAssistantContent verifies the +// round-trip for the new persistence model: provider-executed tool +// results (e.g. web_search) are stored inline in the assistant +// content row (not as separate tool-role messages). After marshal → +// parse → ToMessageParts, the ToolResultPart must carry +// ProviderExecuted = true so the fantasy Anthropic provider can +// reconstruct the web_search_tool_result block. +func TestProviderExecutedResultInAssistantContent(t *testing.T) { + t.Parallel() + + // The assistant message contains a PE tool call, a PE tool result, + // and a text block — mimicking a web_search step where persistStep + // keeps the PE result inline. + assistantContent := mustMarshalContent(t, []fantasy.Content{ + fantasy.ToolCallContent{ + ToolCallID: "srvtoolu_WS", + ToolName: "web_search", + Input: `{"query":"golang testing"}`, + ProviderExecuted: true, + }, + fantasy.ToolResultContent{ + ToolCallID: "srvtoolu_WS", + ToolName: "web_search", + Result: fantasy.ToolResultOutputContentText{Text: `{"results":"some search results"}`}, + ProviderExecuted: true, + }, + fantasy.TextContent{Text: "Here is what I found."}, + }) + + prompt, err := chatprompt.ConvertMessages([]database.ChatMessage{ + {Role: "assistant", Visibility: database.ChatMessageVisibilityBoth, Content: assistantContent}, + {Role: "user", Visibility: database.ChatMessageVisibilityBoth, Content: mustMarshalContent(t, []fantasy.Content{ + fantasy.TextContent{Text: "Thanks!"}, + })}, + }) + require.NoError(t, err) + + // Should be 2 messages: assistant + user. + require.Len(t, prompt, 2) + require.Equal(t, fantasy.MessageRoleAssistant, prompt[0].Role) + require.Equal(t, fantasy.MessageRoleUser, prompt[1].Role) + + // The assistant message must contain 3 parts: tool_call, tool_result, text. + var foundToolCall, foundToolResult, foundText bool + for _, part := range prompt[0].Content { + if tc, ok := fantasy.AsMessagePart[fantasy.ToolCallPart](part); ok { + require.Equal(t, "srvtoolu_WS", tc.ToolCallID) + require.True(t, tc.ProviderExecuted, "ToolCallPart.ProviderExecuted must be true") + foundToolCall = true + } + if tr, ok := fantasy.AsMessagePart[fantasy.ToolResultPart](part); ok { + require.Equal(t, "srvtoolu_WS", tr.ToolCallID) + require.True(t, tr.ProviderExecuted, "ToolResultPart.ProviderExecuted must be true") + foundToolResult = true + } + if tp, ok := fantasy.AsMessagePart[fantasy.TextPart](part); ok { + require.Equal(t, "Here is what I found.", tp.Text) + foundText = true + } + } + require.True(t, foundToolCall, "expected PE tool call in assistant message") + require.True(t, foundToolResult, "expected PE tool result in assistant message") + require.True(t, foundText, "expected text part in assistant message") +} + +// TestProviderExecutedResult_LegacyToolRow verifies backward +// compatibility: PE tool results that were stored as separate +// tool-role rows (legacy persistence) are still handled correctly +// by the repair passes — orphaned PE results are dropped, and +// matching PE results in the same step work via the existing +// injectMissingToolUses logic. +func TestProviderExecutedResult_LegacyToolRow(t *testing.T) { + t.Parallel() + + // Assistant with PE web_search + regular tool call. + assistantContent := mustMarshalContent(t, []fantasy.Content{ + fantasy.ToolCallContent{ + ToolCallID: "srvtoolu_WS", + ToolName: "web_search", + Input: `{"query":"test"}`, + ProviderExecuted: true, + }, + fantasy.ToolCallContent{ + ToolCallID: "toolu_exec", + ToolName: "execute", + Input: `{"command":"ls"}`, + }, + fantasy.TextContent{Text: "Results."}, + }) + + // Legacy: PE result stored as separate tool-role message. + peResult := mustMarshalToolResult(t, + "srvtoolu_WS", "web_search", + json.RawMessage(`{"results":"cached"}`), + false, true, // providerExecuted = true + ) + execResult := mustMarshalToolResult(t, + "toolu_exec", "execute", + json.RawMessage(`{"output":"file.txt"}`), + false, false, + ) + + prompt, err := chatprompt.ConvertMessages([]database.ChatMessage{ + {Role: "assistant", Visibility: database.ChatMessageVisibilityBoth, Content: assistantContent}, + {Role: "tool", Visibility: database.ChatMessageVisibilityBoth, Content: peResult}, + {Role: "tool", Visibility: database.ChatMessageVisibilityBoth, Content: execResult}, + {Role: "user", Visibility: database.ChatMessageVisibilityBoth, Content: mustMarshalContent(t, []fantasy.Content{ + fantasy.TextContent{Text: "next"}, + })}, + }) + require.NoError(t, err) + + // The PE tool result should be dropped by injectMissingToolUses, + // leaving: assistant, tool(exec), user. + require.Len(t, prompt, 3, "expected 3 messages after PE result is dropped") + require.Equal(t, fantasy.MessageRoleAssistant, prompt[0].Role) + require.Equal(t, fantasy.MessageRoleTool, prompt[1].Role) + require.Equal(t, fantasy.MessageRoleUser, prompt[2].Role) + + // Tool message should only contain the exec result, not the PE one. + toolIDs := extractToolResultIDs(t, prompt[1]) + require.Equal(t, []string{"toolu_exec"}, toolIDs) +} + func mustJSON(t *testing.T, v any) json.RawMessage { t.Helper() data, err := json.Marshal(v)