From 0211448d098b6f902ee111e944895436dccb7b2a Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Fri, 24 Apr 2026 23:57:30 +0200 Subject: [PATCH] fix(coderd): sanitize Anthropic provider tool history (#24706) Anthropic can reject replayed chat histories when a provider-executed tool call, such as `web_search`, is present without its matching provider result block. This sanitizes unpaired Anthropic provider-executed tool calls during prompt reconstruction, before Anthropic requests, and before persistence so existing poisoned histories can continue and new malformed turns are not stored. Resolves: CODAGT-259 > Mux is acting on Mike's behalf. --- coderd/exp_chats_test.go | 2 +- coderd/x/chatd/chatd.go | 8 + coderd/x/chatd/chatd_test.go | 16 +- coderd/x/chatd/chatloop/chatloop.go | 143 +++++- coderd/x/chatd/chatloop/chatloop_test.go | 501 ++++++++++++++++++- coderd/x/chatd/chatprompt/chatprompt.go | 215 +++++++- coderd/x/chatd/chatprompt/chatprompt_test.go | 268 +++++++++- 7 files changed, 1090 insertions(+), 63 deletions(-) diff --git a/coderd/exp_chats_test.go b/coderd/exp_chats_test.go index 5a7dcfe619..b6c53eaa9f 100644 --- a/coderd/exp_chats_test.go +++ b/coderd/exp_chats_test.go @@ -1066,7 +1066,7 @@ func TestListChats(t *testing.T) { } } return false - }, testutil.WaitShort, testutil.IntervalFast) + }, testutil.WaitLong, testutil.IntervalFast) } // Fetch first page with limit=2. diff --git a/coderd/x/chatd/chatd.go b/coderd/x/chatd/chatd.go index fe9c206279..21d8a28014 100644 --- a/coderd/x/chatd/chatd.go +++ b/coderd/x/chatd/chatd.go @@ -6162,6 +6162,10 @@ func (p *Server) runChat( if err := g2.Wait(); err != nil { return result, err } + prompt, sanitizeStats := chatprompt.SanitizeAnthropicProviderToolCalls(model.Provider(), prompt) + chatprompt.LogAnthropicProviderToolSanitization( + ctx, logger, "persisted_history_replay", model.Provider(), model.Model(), sanitizeStats, + ) subagentInstruction := "" if !isRootChat { subagentInstruction = defaultSubagentInstruction @@ -6785,6 +6789,10 @@ func (p *Server) runChat( if err != nil { return nil, xerrors.Errorf("convert reloaded messages: %w", err) } + reloadedPrompt, sanitizeStats := chatprompt.SanitizeAnthropicProviderToolCalls(model.Provider(), reloadedPrompt) + chatprompt.LogAnthropicProviderToolSanitization( + reloadCtx, logger, "reload_messages", model.Provider(), model.Model(), sanitizeStats, + ) // Re-derive instruction and skills from the reloaded // messages so that any context added during the // chatloop (e.g. via persistInstructionFiles when diff --git a/coderd/x/chatd/chatd_test.go b/coderd/x/chatd/chatd_test.go index beb1216315..36dfd3b498 100644 --- a/coderd/x/chatd/chatd_test.go +++ b/coderd/x/chatd/chatd_test.go @@ -2586,7 +2586,13 @@ func TestPromoteQueuedMessageUsesQueuedModelConfigID(t *testing.T) { storedChat, err := db.GetChatByID(ctx, chat.ID) require.NoError(t, err) require.Equal(t, modelConfigB.ID, storedChat.LastModelConfigID) - require.Equal(t, database.ChatStatusPending, storedChat.Status) + // The processor can pick up the pending chat immediately after + // promotion, so this test only requires that promotion moved it out of + // waiting and preserved the queued model configuration. + require.Contains(t, []database.ChatStatus{ + database.ChatStatusPending, + database.ChatStatusRunning, + }, storedChat.Status) } func TestPromoteQueuedMessageReloadsChatWhenModelConfigChangesDuringPending(t *testing.T) { @@ -2678,7 +2684,7 @@ func TestAutoPromoteQueuedMessagesPreservesPerTurnModelOrder(t *testing.T) { t.Parallel() db, ps := dbtestutil.NewDB(t) - ctx := testutil.Context(t, testutil.WaitLong) + ctx := testutil.Context(t, testutil.WaitSuperLong) firstRunStarted := make(chan struct{}) allowFirstRunFinish := make(chan struct{}) @@ -2765,7 +2771,7 @@ func TestAutoPromoteQueuedMessagesPreservesPerTurnModelOrder(t *testing.T) { require.Eventually(t, func() bool { return requestCount.Load() >= 3 - }, testutil.WaitLong, testutil.IntervalFast) + }, testutil.WaitSuperLong, testutil.IntervalFast) chatd.WaitUntilIdleForTest(server) queuedMessages, err := db.GetChatQueuedMessages(ctx, chat.ID) @@ -2816,7 +2822,7 @@ func TestAutoPromoteQueuedMessageFallsBackForInvalidQueuedModelConfigID(t *testi func testAutoPromoteQueuedMessageFallback(t *testing.T, queuedModelConfigID uuid.NullUUID) { db, ps := dbtestutil.NewDB(t) - ctx := testutil.Context(t, testutil.WaitLong) + ctx := testutil.Context(t, testutil.WaitSuperLong) firstRunStarted := make(chan struct{}) allowFirstRunFinish := make(chan struct{}) @@ -2871,7 +2877,7 @@ func testAutoPromoteQueuedMessageFallback(t *testing.T, queuedModelConfigID uuid require.Eventually(t, func() bool { return requestCount.Load() >= 2 - }, testutil.WaitLong, testutil.IntervalFast) + }, testutil.WaitSuperLong, testutil.IntervalFast) chatd.WaitUntilIdleForTest(server) queuedMessages, err := db.GetChatQueuedMessages(ctx, chat.ID) diff --git a/coderd/x/chatd/chatloop/chatloop.go b/coderd/x/chatd/chatloop/chatloop.go index 9113297f16..915e79dd0d 100644 --- a/coderd/x/chatd/chatloop/chatloop.go +++ b/coderd/x/chatd/chatloop/chatloop.go @@ -193,7 +193,7 @@ type ProviderTool struct { // stepResult holds the accumulated output of a single streaming // step. Since we own the stream consumer, all content is tracked -// directly here — no shadow draft state needed. +// directly here, no shadow draft state needed. type stepResult struct { content []fantasy.Content usage fantasy.Usage @@ -391,6 +391,12 @@ func Run(ctx context.Context, opts RunOptions) error { } prepared := make([]fantasy.Message, len(messages)) copy(prepared, messages) + prepared, sanitizeStats := chatprompt.SanitizeAnthropicProviderToolCalls(provider, prepared) + chatprompt.LogAnthropicProviderToolSanitization( + ctx, opts.Logger, "pre_request", provider, modelName, sanitizeStats, + slog.F("step_index", step), + slog.F("total_steps", totalSteps), + ) if applyAnthropicCaching { addAnthropicPromptCaching(prepared) } @@ -518,12 +524,18 @@ func Run(ctx context.Context, opts RunOptions) error { }) } - contextLimit := extractContextLimit(result.providerMetadata) - if !contextLimit.Valid && opts.ContextLimitFallback > 0 { - contextLimit = sql.NullInt64{ - Int64: opts.ContextLimitFallback, - Valid: true, - } + contextLimit := extractContextLimitWithFallback( + result.providerMetadata, + opts.ContextLimitFallback, + ) + + result.content = sanitizeAnthropicProviderToolStepContent( + ctx, opts.Logger, provider, modelName, + "dynamic_tool_persist", step, result.finishReason, result.content, + ) + if len(result.content) == 0 && len(pending) == 0 { + tryCompactOnExit(ctx, opts, result.usage, result.providerMetadata) + return ErrDynamicToolCall } if err := opts.PersistStep(ctx, PersistedStep{ @@ -560,13 +572,21 @@ func Run(ctx context.Context, opts RunOptions) error { } } // Extract context limit from provider metadata. - contextLimit := extractContextLimit(result.providerMetadata) - if !contextLimit.Valid && opts.ContextLimitFallback > 0 { - contextLimit = sql.NullInt64{ - Int64: opts.ContextLimitFallback, - Valid: true, - } + contextLimit := extractContextLimitWithFallback( + result.providerMetadata, + opts.ContextLimitFallback, + ) + result.content = sanitizeAnthropicProviderToolStepContent( + ctx, opts.Logger, provider, modelName, + "normal_persist", step, result.finishReason, result.content, + ) + if len(result.content) == 0 { + lastUsage = result.usage + lastProviderMetadata = result.providerMetadata + stoppedByModel = true + break } + // Persist the step. If persistence fails because // the chat was interrupted between the previous // check and here, fall back to the interrupt-safe @@ -714,6 +734,67 @@ func Run(ctx context.Context, opts RunOptions) error { return nil } +func sanitizeAnthropicProviderToolStepContent( + ctx context.Context, + logger slog.Logger, + provider string, + modelName string, + phase string, + step int, + finishReason fantasy.FinishReason, + content []fantasy.Content, +) []fantasy.Content { + sanitized, stats := sanitizeAnthropicProviderToolContent(provider, content) + chatprompt.LogAnthropicProviderToolSanitization( + ctx, logger, phase, provider, modelName, stats, + slog.F("step_index", step), + slog.F("finish_reason", finishReason), + ) + return sanitized +} + +func sanitizeAnthropicProviderToolContent( + provider string, + content []fantasy.Content, +) ([]fantasy.Content, chatprompt.AnthropicProviderToolSanitizationStats) { + var stats chatprompt.AnthropicProviderToolSanitizationStats + if provider != fantasyanthropic.Name || len(content) == 0 { + return content, stats + } + + matchedResultIDs := make(map[string]struct{}) + for _, block := range content { + result, ok := fantasy.AsContentType[fantasy.ToolResultContent](block) + if !ok || !result.ProviderExecuted || result.ToolCallID == "" { + continue + } + matchedResultIDs[result.ToolCallID] = struct{}{} + } + + out := make([]fantasy.Content, 0, len(content)) + for _, block := range content { + toolCall, ok := fantasy.AsContentType[fantasy.ToolCallContent](block) + if ok && isAnthropicProviderExecutedToolCall(provider, toolCall) { + if _, hasResult := matchedResultIDs[toolCall.ToolCallID]; !hasResult { + stats.RemovedToolCalls++ + continue + } + } + out = append(out, block) + } + if stats.RemovedToolCalls == 0 { + return content, stats + } + return out, stats +} + +func isAnthropicProviderExecutedToolCall( + provider string, + toolCall fantasy.ToolCallContent, +) bool { + return provider == fantasyanthropic.Name && toolCall.ProviderExecuted +} + // guardedAttempt owns an attempt-scoped context and startup guard // around a provider stream. release is idempotent and frees the // attempt-scoped timer/context. finish canonicalizes startup timeout @@ -1281,9 +1362,9 @@ func flushActiveState( } } -// persistInterruptedStep saves all accumulated content from a -// partial stream. Since we own the stepResult directly, no shadow -// state is needed. +// persistInterruptedStep saves durable content from a partial stream. +// Provider-executed calls without results are removed because their +// result metadata cannot be synthesized safely. func persistInterruptedStep( ctx context.Context, opts RunOptions, @@ -1293,6 +1374,18 @@ func persistInterruptedStep( return } + provider := "" + modelName := "" + if opts.Model != nil { + provider = opts.Model.Provider() + modelName = opts.Model.Model() + } + var sanitizeStats chatprompt.AnthropicProviderToolSanitizationStats + result.content, sanitizeStats = sanitizeAnthropicProviderToolContent(provider, result.content) + chatprompt.LogAnthropicProviderToolSanitization( + ctx, opts.Logger, "interrupted_persist", provider, modelName, sanitizeStats, + ) + // Track which tool calls already have results in the content. answeredToolCalls := make(map[string]struct{}) for _, c := range result.content { @@ -1327,6 +1420,9 @@ func persistInterruptedStep( if _, exists := answeredToolCalls[tc.ToolCallID]; exists { continue } + if isAnthropicProviderExecutedToolCall(provider, tc) { + continue + } content = append(content, fantasy.ToolResultContent{ ToolCallID: tc.ToolCallID, ToolName: tc.ToolName, @@ -1344,6 +1440,10 @@ func persistInterruptedStep( answeredToolCalls[tc.ToolCallID] = struct{}{} } + if len(content) == 0 { + return + } + persistCtx := context.WithoutCancel(ctx) if err := opts.PersistStep(persistCtx, PersistedStep{ Content: content, @@ -1625,6 +1725,17 @@ func extractContextLimit(metadata fantasy.ProviderMetadata) sql.NullInt64 { } } +func extractContextLimitWithFallback(metadata fantasy.ProviderMetadata, fallback int64) sql.NullInt64 { + contextLimit := extractContextLimit(metadata) + if contextLimit.Valid || fallback <= 0 { + return contextLimit + } + return sql.NullInt64{ + Int64: fallback, + Valid: true, + } +} + func findContextLimitValue(value any) (int64, bool) { var ( limit int64 diff --git a/coderd/x/chatd/chatloop/chatloop_test.go b/coderd/x/chatd/chatloop/chatloop_test.go index 57ca1174d9..c2f63b3097 100644 --- a/coderd/x/chatd/chatloop/chatloop_test.go +++ b/coderd/x/chatd/chatloop/chatloop_test.go @@ -1019,6 +1019,95 @@ func textMessage(role fantasy.MessageRole, text string) fantasy.Message { } } +func requireNoProviderExecutedToolCallContent(t *testing.T, content []fantasy.Content) { + t.Helper() + + for i, block := range content { + toolCall, ok := fantasy.AsContentType[fantasy.ToolCallContent](block) + if ok && toolCall.ProviderExecuted { + t.Fatalf("content[%d]: unexpected provider-executed call", i) + } + } +} + +func requireNoProviderExecutedToolResultContent(t *testing.T, content []fantasy.Content) { + t.Helper() + + for i, block := range content { + toolResult, ok := fantasy.AsContentType[fantasy.ToolResultContent](block) + if ok && toolResult.ProviderExecuted { + t.Fatalf("content[%d]: unexpected provider-executed result", i) + } + } +} + +func requireNoProviderExecutedToolCallPrompt(t *testing.T, prompt []fantasy.Message) { + t.Helper() + + for i, message := range prompt { + for j, part := range message.Content { + toolCall, ok := fantasy.AsMessagePart[fantasy.ToolCallPart](part) + if ok && toolCall.ProviderExecuted { + t.Fatalf("prompt[%d].content[%d]: unexpected provider-executed call", i, j) + } + } + } +} + +func requireTextContent(t *testing.T, content []fantasy.Content, text string) fantasy.TextContent { + t.Helper() + + for _, block := range content { + textContent, ok := fantasy.AsContentType[fantasy.TextContent](block) + if ok && textContent.Text == text { + return textContent + } + } + t.Fatalf("missing text content %q", text) + return fantasy.TextContent{} +} + +func requireToolCallContent(t *testing.T, content []fantasy.Content, id, name string) fantasy.ToolCallContent { + t.Helper() + + for _, block := range content { + toolCall, ok := fantasy.AsContentType[fantasy.ToolCallContent](block) + if ok && toolCall.ToolCallID == id && toolCall.ToolName == name { + return toolCall + } + } + t.Fatalf("missing tool call %q", id) + return fantasy.ToolCallContent{} +} + +func requireToolResultContent(t *testing.T, content []fantasy.Content, id, name string) fantasy.ToolResultContent { + t.Helper() + + for _, block := range content { + toolResult, ok := fantasy.AsContentType[fantasy.ToolResultContent](block) + if ok && toolResult.ToolCallID == id && toolResult.ToolName == name { + return toolResult + } + } + t.Fatalf("missing tool result %q", id) + return fantasy.ToolResultContent{} +} + +func requireToolResultPrompt(t *testing.T, prompt []fantasy.Message, id string) fantasy.ToolResultPart { + t.Helper() + + for _, message := range prompt { + for _, part := range message.Content { + toolResult, ok := fantasy.AsMessagePart[fantasy.ToolResultPart](part) + if ok && toolResult.ToolCallID == id { + return toolResult + } + } + } + t.Fatalf("missing prompt tool result %q", id) + return fantasy.ToolResultPart{} +} + func containsPromptSentinel(prompt []fantasy.Message) bool { for _, message := range prompt { if message.Role != fantasy.MessageRoleUser || len(message.Content) != 1 { @@ -1413,7 +1502,7 @@ func TestRun_PersistStepErrorPropagates(t *testing.T) { // TestRun_ShutdownDuringToolExecutionReturnsContextCanceled verifies that // when the parent context is canceled (simulating server shutdown) while -// a tool is blocked, Run returns context.Canceled — not ErrInterrupted. +// a tool is blocked, Run returns context.Canceled, not ErrInterrupted. // This matters because the caller uses the error type to decide whether // to set chat status to "pending" (retryable on another worker) vs // "waiting" (stuck forever). @@ -1495,7 +1584,7 @@ func TestRun_ShutdownDuringToolExecutionReturnsContextCanceled(t *testing.T) { <-serverCancelDone require.Error(t, err) - // The error must NOT be ErrInterrupted — it should propagate + // The error must NOT be ErrInterrupted, it should propagate // as context.Canceled so the caller can distinguish shutdown // from user interruption. Use assert (not require) so both // checks are evaluated even if the first fails. @@ -1515,7 +1604,7 @@ func TestToResponseMessages_ProviderExecutedToolResultInAssistantMessage(t *test Input: `{"query":"coder"}`, ProviderExecuted: true, }, - // Provider-executed tool result — must stay in + // Provider-executed tool result, must stay in // assistant message. fantasy.ToolResultContent{ ToolCallID: "provider-tc-1", @@ -1530,7 +1619,7 @@ func TestToResponseMessages_ProviderExecutedToolResultInAssistantMessage(t *test Input: `{"path":"main.go"}`, ProviderExecuted: false, }, - // Local tool result — should go into tool message. + // Local tool result, should go into tool message. fantasy.ToolResultContent{ ToolCallID: "local-tc-1", ToolName: "read_file", @@ -1584,28 +1673,28 @@ func TestToResponseMessages_FiltersEmptyTextAndReasoningParts(t *testing.T) { sr := stepResult{ content: []fantasy.Content{ - // Empty text — should be filtered. + // Empty text, should be filtered. fantasy.TextContent{Text: ""}, - // Whitespace-only text — should be filtered. + // Whitespace-only text, should be filtered. fantasy.TextContent{Text: " \t\n"}, - // Empty reasoning — should be filtered. + // Empty reasoning, should be filtered. fantasy.ReasoningContent{Text: ""}, - // Whitespace-only reasoning — should be filtered. + // Whitespace-only reasoning, should be filtered. fantasy.ReasoningContent{Text: " \n"}, - // Non-empty text — should pass through. + // Non-empty text, should pass through. fantasy.TextContent{Text: "hello world"}, - // Leading/trailing whitespace with content — kept + // Leading/trailing whitespace with content, kept // with the original value (not trimmed). fantasy.TextContent{Text: " hello "}, - // Non-empty reasoning — should pass through. + // Non-empty reasoning, should pass through. fantasy.ReasoningContent{Text: "let me think"}, - // Tool call — should be unaffected by filtering. + // Tool call, should be unaffected by filtering. fantasy.ToolCallContent{ ToolCallID: "tc-1", ToolName: "read_file", Input: `{"path":"main.go"}`, }, - // Local tool result — should be unaffected by filtering. + // Local tool result, should be unaffected by filtering. fantasy.ToolResultContent{ ToolCallID: "tc-1", ToolName: "read_file", @@ -1630,7 +1719,7 @@ func TestToResponseMessages_FiltersEmptyTextAndReasoningParts(t *testing.T) { require.True(t, ok, "part 0 should be TextPart") assert.Equal(t, "hello world", textPart.Text) - // Part 1: padded text — original whitespace preserved. + // Part 1: padded text, original whitespace preserved. paddedPart, ok := fantasy.AsMessagePart[fantasy.TextPart](assistantMsg.Content[1]) require.True(t, ok, "part 1 should be TextPart") assert.Equal(t, " hello ", paddedPart.Text) @@ -1796,7 +1885,7 @@ func TestRun_ProviderExecutedToolResultTimestamps(t *testing.T) { StreamFn: func(_ context.Context, _ fantasy.Call) (fantasy.StreamResponse, error) { // Simulate a provider-executed tool call and result // (e.g. Anthropic web search) followed by a text - // response — all in a single stream. + // response, all in a single stream. return streamFromParts([]fantasy.StreamPart{ {Type: fantasy.StreamPartTypeToolInputStart, ID: "ws-1", ToolCallName: "web_search", ProviderExecuted: true}, {Type: fantasy.StreamPartTypeToolInputDelta, ID: "ws-1", Delta: `{"query":"coder"}`, ProviderExecuted: true}, @@ -1808,7 +1897,7 @@ func TestRun_ProviderExecutedToolResultTimestamps(t *testing.T) { ToolCallInput: `{"query":"coder"}`, ProviderExecuted: true, }, - // Provider-executed tool result — emitted by + // Provider-executed tool result, emitted by // the provider, not our tool runner. { Type: fantasy.StreamPartTypeToolResult, @@ -1855,6 +1944,384 @@ func TestRun_ProviderExecutedToolResultTimestamps(t *testing.T) { "tool-result timestamp must be >= tool-call timestamp") } +func TestRun_AnthropicDropsUnpairedProviderToolBeforePersist(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + toolName string + toolInput string + }{ + { + name: "web_search", + toolName: "web_search", + toolInput: `{"query":"coder"}`, + }, + { + name: "code_execution", + toolName: "code_execution", + toolInput: `{"code":"print(1)"}`, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + model := &chattest.FakeModel{ + ProviderName: fantasyanthropic.Name, + StreamFn: func(_ context.Context, _ fantasy.Call) (fantasy.StreamResponse, error) { + return streamFromParts([]fantasy.StreamPart{ + {Type: fantasy.StreamPartTypeToolInputStart, ID: "pt-1", ToolCallName: tc.toolName, ProviderExecuted: true}, + {Type: fantasy.StreamPartTypeToolInputDelta, ID: "pt-1", Delta: tc.toolInput, ProviderExecuted: true}, + {Type: fantasy.StreamPartTypeToolInputEnd, ID: "pt-1"}, + { + Type: fantasy.StreamPartTypeToolCall, + ID: "pt-1", + ToolCallName: tc.toolName, + ToolCallInput: tc.toolInput, + ProviderExecuted: true, + }, + {Type: fantasy.StreamPartTypeFinish, FinishReason: fantasy.FinishReasonStop}, + }), nil + }, + } + + persistCalls := 0 + err := Run(context.Background(), RunOptions{ + Model: model, + Messages: []fantasy.Message{ + textMessage(fantasy.MessageRoleUser, "run provider tool"), + }, + MaxSteps: 1, + PersistStep: func(_ context.Context, _ PersistedStep) error { + persistCalls++ + return nil + }, + }) + require.NoError(t, err) + require.Equal(t, 0, persistCalls) + }) + } +} + +func TestRun_AnthropicKeepsPairedWebSearchBeforePersist(t *testing.T) { + t.Parallel() + + model := &chattest.FakeModel{ + ProviderName: fantasyanthropic.Name, + StreamFn: func(_ context.Context, _ fantasy.Call) (fantasy.StreamResponse, error) { + return streamFromParts([]fantasy.StreamPart{ + {Type: fantasy.StreamPartTypeToolInputStart, ID: "ws-1", ToolCallName: "web_search", ProviderExecuted: true}, + {Type: fantasy.StreamPartTypeToolInputDelta, ID: "ws-1", Delta: `{"query":"coder"}`, ProviderExecuted: true}, + {Type: fantasy.StreamPartTypeToolInputEnd, ID: "ws-1"}, + { + Type: fantasy.StreamPartTypeToolCall, + ID: "ws-1", + ToolCallName: "web_search", + ToolCallInput: `{"query":"coder"}`, + ProviderExecuted: true, + }, + { + Type: fantasy.StreamPartTypeToolResult, + ID: "ws-1", + ToolCallName: "web_search", + ProviderExecuted: true, + }, + {Type: fantasy.StreamPartTypeTextStart, ID: "text-1"}, + {Type: fantasy.StreamPartTypeTextDelta, ID: "text-1", Delta: "search done"}, + {Type: fantasy.StreamPartTypeTextEnd, ID: "text-1"}, + {Type: fantasy.StreamPartTypeFinish, FinishReason: fantasy.FinishReasonStop}, + }), nil + }, + } + + var persistedSteps []PersistedStep + err := Run(context.Background(), RunOptions{ + Model: model, + Messages: []fantasy.Message{ + textMessage(fantasy.MessageRoleUser, "search for coder"), + }, + MaxSteps: 1, + PersistStep: func(_ context.Context, step PersistedStep) error { + persistedSteps = append(persistedSteps, step) + return nil + }, + }) + require.NoError(t, err) + require.Len(t, persistedSteps, 1) + + toolCall := requireToolCallContent(t, persistedSteps[0].Content, "ws-1", "web_search") + require.True(t, toolCall.ProviderExecuted) + toolResult := requireToolResultContent(t, persistedSteps[0].Content, "ws-1", "web_search") + require.True(t, toolResult.ProviderExecuted) + requireTextContent(t, persistedSteps[0].Content, "search done") +} + +func TestRun_AnthropicInterruptedWebSearchDoesNotPersistSyntheticResult(t *testing.T) { + t.Parallel() + + started := make(chan struct{}) + model := &chattest.FakeModel{ + ProviderName: fantasyanthropic.Name, + StreamFn: func(ctx context.Context, _ fantasy.Call) (fantasy.StreamResponse, error) { + return iter.Seq[fantasy.StreamPart](func(yield func(fantasy.StreamPart) bool) { + if !yield(fantasy.StreamPart{ + Type: fantasy.StreamPartTypeToolInputStart, + ID: "ws-1", + ToolCallName: "web_search", + ProviderExecuted: true, + }) { + return + } + if !yield(fantasy.StreamPart{ + Type: fantasy.StreamPartTypeToolInputDelta, + ID: "ws-1", + Delta: `{"query":"coder"}`, + ProviderExecuted: true, + }) { + return + } + close(started) + <-ctx.Done() + _ = yield(fantasy.StreamPart{ + Type: fantasy.StreamPartTypeError, + Error: ctx.Err(), + }) + }), nil + }, + } + + ctx, cancel := context.WithCancelCause(context.Background()) + defer cancel(nil) + go func() { + <-started + cancel(ErrInterrupted) + }() + + persistCalls := 0 + err := Run(ctx, RunOptions{ + Model: model, + Messages: []fantasy.Message{ + textMessage(fantasy.MessageRoleUser, "search for coder"), + }, + MaxSteps: 1, + PersistStep: func(_ context.Context, _ PersistedStep) error { + persistCalls++ + return nil + }, + }) + require.ErrorIs(t, err, ErrInterrupted) + require.Equal(t, 0, persistCalls) +} + +func TestRun_AnthropicInterruptedProviderToolKeepsLocalSyntheticResult(t *testing.T) { + t.Parallel() + + started := make(chan struct{}) + model := &chattest.FakeModel{ + ProviderName: fantasyanthropic.Name, + StreamFn: func(ctx context.Context, _ fantasy.Call) (fantasy.StreamResponse, error) { + return iter.Seq[fantasy.StreamPart](func(yield func(fantasy.StreamPart) bool) { + if !yield(fantasy.StreamPart{ + Type: fantasy.StreamPartTypeToolInputStart, + ID: "ws-1", + ToolCallName: "web_search", + ProviderExecuted: true, + }) { + return + } + if !yield(fantasy.StreamPart{ + Type: fantasy.StreamPartTypeToolInputDelta, + ID: "ws-1", + Delta: `{"query":"coder"}`, + ProviderExecuted: true, + }) { + return + } + if !yield(fantasy.StreamPart{ + Type: fantasy.StreamPartTypeToolInputStart, + ID: "tc-1", + ToolCallName: "read_file", + }) { + return + } + if !yield(fantasy.StreamPart{ + Type: fantasy.StreamPartTypeToolInputDelta, + ID: "tc-1", + Delta: `{"path":"main.go"}`, + }) { + return + } + close(started) + <-ctx.Done() + _ = yield(fantasy.StreamPart{ + Type: fantasy.StreamPartTypeError, + Error: ctx.Err(), + }) + }), nil + }, + } + + ctx, cancel := context.WithCancelCause(context.Background()) + defer cancel(nil) + go func() { + <-started + cancel(ErrInterrupted) + }() + + var persistedSteps []PersistedStep + err := Run(ctx, RunOptions{ + Model: model, + Messages: []fantasy.Message{ + textMessage(fantasy.MessageRoleUser, "search and read"), + }, + MaxSteps: 1, + PersistStep: func(_ context.Context, step PersistedStep) error { + persistedSteps = append(persistedSteps, step) + return nil + }, + }) + require.ErrorIs(t, err, ErrInterrupted) + require.Len(t, persistedSteps, 1) + requireNoProviderExecutedToolCallContent(t, persistedSteps[0].Content) + requireNoProviderExecutedToolResultContent(t, persistedSteps[0].Content) + + toolCall := requireToolCallContent(t, persistedSteps[0].Content, "tc-1", "read_file") + require.False(t, toolCall.ProviderExecuted) + toolResult := requireToolResultContent(t, persistedSteps[0].Content, "tc-1", "read_file") + require.False(t, toolResult.ProviderExecuted) + _, isErr := toolResult.Result.(fantasy.ToolResultOutputContentError) + require.True(t, isErr) +} + +func TestRun_AnthropicSanitizesProviderToolBeforeRequest(t *testing.T) { + t.Parallel() + + var capturedPrompt []fantasy.Message + model := &chattest.FakeModel{ + ProviderName: fantasyanthropic.Name, + StreamFn: func(_ context.Context, call fantasy.Call) (fantasy.StreamResponse, error) { + capturedPrompt = append([]fantasy.Message(nil), call.Prompt...) + return streamFromParts([]fantasy.StreamPart{ + {Type: fantasy.StreamPartTypeTextStart, ID: "text-1"}, + {Type: fantasy.StreamPartTypeTextDelta, ID: "text-1", Delta: "done"}, + {Type: fantasy.StreamPartTypeTextEnd, ID: "text-1"}, + {Type: fantasy.StreamPartTypeFinish, FinishReason: fantasy.FinishReasonStop}, + }), nil + }, + } + + err := Run(context.Background(), RunOptions{ + Model: model, + Messages: []fantasy.Message{ + textMessage(fantasy.MessageRoleUser, "search for coder"), + { + Role: fantasy.MessageRoleAssistant, + Content: []fantasy.MessagePart{ + fantasy.ToolCallPart{ + ToolCallID: "ws-1", + ToolName: "web_search", + Input: `{"query":"coder"}`, + ProviderExecuted: true, + }, + }, + }, + textMessage(fantasy.MessageRoleUser, "continue"), + }, + MaxSteps: 1, + PersistStep: func(_ context.Context, _ PersistedStep) error { + return nil + }, + }) + require.NoError(t, err) + require.Len(t, capturedPrompt, 1) + require.Equal(t, fantasy.MessageRoleUser, capturedPrompt[0].Role) + require.Len(t, capturedPrompt[0].Content, 2) + requireNoProviderExecutedToolCallPrompt(t, capturedPrompt) +} + +func TestRun_AnthropicSanitizesWebSearchBeforeContinuation(t *testing.T) { + t.Parallel() + + var mu sync.Mutex + var streamCalls int + var secondCallPrompt []fantasy.Message + model := &chattest.FakeModel{ + ProviderName: fantasyanthropic.Name, + StreamFn: func(_ context.Context, call fantasy.Call) (fantasy.StreamResponse, error) { + mu.Lock() + step := streamCalls + streamCalls++ + mu.Unlock() + + switch step { + case 0: + return streamFromParts([]fantasy.StreamPart{ + {Type: fantasy.StreamPartTypeToolInputStart, ID: "ws-1", ToolCallName: "web_search", ProviderExecuted: true}, + {Type: fantasy.StreamPartTypeToolInputDelta, ID: "ws-1", Delta: `{"query":"coder"}`, ProviderExecuted: true}, + {Type: fantasy.StreamPartTypeToolInputEnd, ID: "ws-1"}, + { + Type: fantasy.StreamPartTypeToolCall, + ID: "ws-1", + ToolCallName: "web_search", + ToolCallInput: `{"query":"coder"}`, + ProviderExecuted: true, + }, + {Type: fantasy.StreamPartTypeToolInputStart, ID: "tc-1", ToolCallName: "read_file"}, + {Type: fantasy.StreamPartTypeToolInputDelta, ID: "tc-1", Delta: `{"path":"main.go"}`}, + {Type: fantasy.StreamPartTypeToolInputEnd, ID: "tc-1"}, + { + Type: fantasy.StreamPartTypeToolCall, + ID: "tc-1", + ToolCallName: "read_file", + ToolCallInput: `{"path":"main.go"}`, + }, + {Type: fantasy.StreamPartTypeFinish, FinishReason: fantasy.FinishReasonToolCalls}, + }), nil + default: + mu.Lock() + secondCallPrompt = append([]fantasy.Message(nil), call.Prompt...) + mu.Unlock() + return streamFromParts([]fantasy.StreamPart{ + {Type: fantasy.StreamPartTypeTextStart, ID: "text-1"}, + {Type: fantasy.StreamPartTypeTextDelta, ID: "text-1", Delta: "done"}, + {Type: fantasy.StreamPartTypeTextEnd, ID: "text-1"}, + {Type: fantasy.StreamPartTypeFinish, FinishReason: fantasy.FinishReasonStop}, + }), nil + } + }, + } + + var persistedSteps []PersistedStep + err := Run(context.Background(), RunOptions{ + Model: model, + Messages: []fantasy.Message{ + textMessage(fantasy.MessageRoleUser, "search and read"), + }, + Tools: []fantasy.AgentTool{ + newNoopTool("read_file"), + }, + MaxSteps: 2, + PersistStep: func(_ context.Context, step PersistedStep) error { + persistedSteps = append(persistedSteps, step) + return nil + }, + }) + require.NoError(t, err) + require.Equal(t, 2, streamCalls) + require.Len(t, persistedSteps, 2) + requireNoProviderExecutedToolCallContent(t, persistedSteps[0].Content) + requireNoProviderExecutedToolCallPrompt(t, secondCallPrompt) + + toolCall := requireToolCallContent(t, persistedSteps[0].Content, "tc-1", "read_file") + require.False(t, toolCall.ProviderExecuted) + toolResult := requireToolResultContent(t, persistedSteps[0].Content, "tc-1", "read_file") + require.False(t, toolResult.ProviderExecuted) + promptResult := requireToolResultPrompt(t, secondCallPrompt, "tc-1") + require.False(t, promptResult.ProviderExecuted) +} + // TestRun_PersistStepInterruptedFallback verifies that when the normal // PersistStep call returns ErrInterrupted (e.g., context canceled in a // race), the step is retried via the interrupt-safe path. @@ -2018,7 +2485,7 @@ func TestRun_PrepareMessagesInjectsSystemContextMidLoop(t *testing.T) { } } if !inserted { - // No system messages — prepend. + // No system messages, prepend. result = append([]fantasy.Message{{ Role: fantasy.MessageRoleSystem, Content: []fantasy.MessagePart{ diff --git a/coderd/x/chatd/chatprompt/chatprompt.go b/coderd/x/chatd/chatprompt/chatprompt.go index e6e855e8f8..9b2ccdccfa 100644 --- a/coderd/x/chatd/chatprompt/chatprompt.go +++ b/coderd/x/chatd/chatprompt/chatprompt.go @@ -11,6 +11,7 @@ import ( "strings" "charm.land/fantasy" + fantasyanthropic "charm.land/fantasy/providers/anthropic" "github.com/google/uuid" "github.com/sqlc-dev/pqtype" "golang.org/x/xerrors" @@ -34,6 +35,194 @@ var toolCallIDSanitizer = regexp.MustCompile(`[^a-zA-Z0-9_-]`) var syntheticPasteFileNamePattern = regexp.MustCompile(`^pasted-text-\d{4}-\d{2}-\d{2}-\d{2}-\d{2}-\d{2}\.txt$`) +// AnthropicProviderToolSanitizationStats describes prompt changes made +// while removing unpaired Anthropic provider-executed tool calls. +type AnthropicProviderToolSanitizationStats struct { + RemovedToolCalls int + DroppedMessages int +} + +// LogAnthropicProviderToolSanitization logs prompt changes made while removing +// unpaired Anthropic provider-executed tool calls. +func LogAnthropicProviderToolSanitization( + ctx context.Context, + logger slog.Logger, + phase string, + provider string, + modelName string, + stats AnthropicProviderToolSanitizationStats, + extra ...slog.Field, +) { + if stats.RemovedToolCalls == 0 { + return + } + fields := []slog.Field{ + slog.F("phase", phase), + slog.F("tool_type", "provider_executed"), + slog.F("provider", provider), + slog.F("model", modelName), + slog.F("removed_tool_calls", stats.RemovedToolCalls), + slog.F("dropped_messages", stats.DroppedMessages), + } + fields = append(fields, extra...) + logger.Warn(ctx, "removed unpaired provider-executed tool calls", fields...) +} + +// SanitizeAnthropicProviderToolCalls removes Anthropic provider-executed +// calls that do not have a same-message provider result. +func SanitizeAnthropicProviderToolCalls( + provider string, + messages []fantasy.Message, +) ([]fantasy.Message, AnthropicProviderToolSanitizationStats) { + var stats AnthropicProviderToolSanitizationStats + if provider != fantasyanthropic.Name || len(messages) == 0 { + return messages, stats + } + + out := make([]fantasy.Message, 0, len(messages)) + changed := false + for _, msg := range messages { + if msg.Role != fantasy.MessageRoleAssistant { + out = appendSanitizedMessage(out, msg) + continue + } + + matchedResultIDs := make(map[string]struct{}) + for _, part := range msg.Content { + result, ok := fantasy.AsMessagePart[fantasy.ToolResultPart](part) + if !ok || !result.ProviderExecuted || result.ToolCallID == "" { + continue + } + matchedResultIDs[result.ToolCallID] = struct{}{} + } + + parts := make([]fantasy.MessagePart, 0, len(msg.Content)) + removedFromMessage := 0 + for _, part := range msg.Content { + toolCall, ok := fantasy.AsMessagePart[fantasy.ToolCallPart](part) + if ok && toolCall.ProviderExecuted { + if _, hasResult := matchedResultIDs[toolCall.ToolCallID]; !hasResult { + stats.RemovedToolCalls++ + removedFromMessage++ + changed = true + continue + } + } + parts = append(parts, part) + } + + if removedFromMessage > 0 { + if len(parts) == 0 { + stats.DroppedMessages++ + continue + } + msg.Content = parts + } + out = appendSanitizedMessage(out, msg) + } + if !changed { + return messages, stats + } + return out, stats +} + +func appendSanitizedMessage(out []fantasy.Message, msg fantasy.Message) []fantasy.Message { + if len(out) == 0 || out[len(out)-1].Role != msg.Role { + return append(out, msg) + } + + last := &out[len(out)-1] + lastContent := applyMessageProviderOptionsToLastPart(last.Content, last.ProviderOptions) + msgContent := applyMessageProviderOptionsToLastPart(msg.Content, msg.ProviderOptions) + content := make([]fantasy.MessagePart, 0, len(lastContent)+len(msgContent)) + content = append(content, lastContent...) + content = append(content, msgContent...) + last.Content = content + last.ProviderOptions = nil + return out +} + +func applyMessageProviderOptionsToLastPart( + parts []fantasy.MessagePart, + options fantasy.ProviderOptions, +) []fantasy.MessagePart { + if len(options) == 0 || len(parts) == 0 { + return parts + } + + out := make([]fantasy.MessagePart, len(parts)) + copy(out, parts) + lastIndex := len(out) - 1 + switch part := out[lastIndex].(type) { + case fantasy.TextPart: + part.ProviderOptions = mergeProviderOptions(part.ProviderOptions, options) + out[lastIndex] = part + case *fantasy.TextPart: + if part != nil { + clone := *part + clone.ProviderOptions = mergeProviderOptions(clone.ProviderOptions, options) + out[lastIndex] = &clone + } + case fantasy.ReasoningPart: + part.ProviderOptions = mergeProviderOptions(part.ProviderOptions, options) + out[lastIndex] = part + case *fantasy.ReasoningPart: + if part != nil { + clone := *part + clone.ProviderOptions = mergeProviderOptions(clone.ProviderOptions, options) + out[lastIndex] = &clone + } + case fantasy.FilePart: + part.ProviderOptions = mergeProviderOptions(part.ProviderOptions, options) + out[lastIndex] = part + case *fantasy.FilePart: + if part != nil { + clone := *part + clone.ProviderOptions = mergeProviderOptions(clone.ProviderOptions, options) + out[lastIndex] = &clone + } + case fantasy.ToolCallPart: + part.ProviderOptions = mergeProviderOptions(part.ProviderOptions, options) + out[lastIndex] = part + case *fantasy.ToolCallPart: + if part != nil { + clone := *part + clone.ProviderOptions = mergeProviderOptions(clone.ProviderOptions, options) + out[lastIndex] = &clone + } + case fantasy.ToolResultPart: + part.ProviderOptions = mergeProviderOptions(part.ProviderOptions, options) + out[lastIndex] = part + case *fantasy.ToolResultPart: + if part != nil { + clone := *part + clone.ProviderOptions = mergeProviderOptions(clone.ProviderOptions, options) + out[lastIndex] = &clone + } + } + return out +} + +func mergeProviderOptions(first, second fantasy.ProviderOptions) fantasy.ProviderOptions { + if len(first) == 0 { + return second + } + if len(second) == 0 { + return first + } + + merged := make(fantasy.ProviderOptions, len(first)+len(second)) + for provider, options := range first { + merged[provider] = options + } + for provider, options := range second { + if options != nil { + merged[provider] = options + } + } + return merged +} + // FileData holds resolved file content for LLM prompt building. type FileData struct { Name string @@ -974,12 +1163,11 @@ func injectMissingToolResults(prompt []fantasy.Message) []fantasy.Message { } // Build synthetic results for any unanswered tool calls. - // Provider-executed tool calls (e.g. web_search) are - // handled server-side by the LLM provider. Their results - // may arrive in a later step and end up stored out of - // position, so we must not inject synthetic error results - // for them. The provider will re-execute the tool when it - // sees the server_tool_use without a matching result. + // Provider-executed tool calls are handled server-side by + // the LLM provider, and their result blocks contain + // provider-owned metadata. We cannot synthesize a valid + // provider result if one is missing, so provider-specific + // sanitization removes unpaired calls before replay. var missing []fantasy.MessagePart for _, tc := range toolCalls { if tc.ProviderExecuted { @@ -1028,13 +1216,12 @@ func injectMissingToolUses( continue } - // Provider-executed tool results (e.g. web_search) may be - // persisted in a later step than the assistant message that - // initiated the tool call. When that happens they appear as - // orphans after the wrong assistant message. Filter them - // out before matching — the provider will re-execute the - // tool, and the search results are already captured in the - // subsequent assistant message's sources/text. + // Provider-executed tool results may be persisted in a + // later step than the assistant message that initiated the + // tool call. When that happens they appear as orphans after + // the wrong assistant message. Filter them out before + // matching because they cannot be converted into local + // tool-use pairs safely. toolResults := make([]fantasy.ToolResultPart, 0, len(allToolResults)) for _, tr := range allToolResults { if !tr.ProviderExecuted { @@ -1632,7 +1819,7 @@ func decodeNulInString(s string) string { _, _ = b.WriteRune(0) i++ default: - // Unpaired sentinel — preserve as-is. + // Unpaired sentinel, preserve as-is. _, _ = b.WriteRune(runes[i]) } } else { diff --git a/coderd/x/chatd/chatprompt/chatprompt_test.go b/coderd/x/chatd/chatprompt/chatprompt_test.go index 0db9c5d5b2..e1465a14b8 100644 --- a/coderd/x/chatd/chatprompt/chatprompt_test.go +++ b/coderd/x/chatd/chatprompt/chatprompt_test.go @@ -59,6 +59,245 @@ func convertMessagesWithoutFiles(t *testing.T, messages []database.ChatMessage) return prompt } +func TestSanitizeAnthropicProviderToolCalls(t *testing.T) { + t.Parallel() + + textPart := fantasy.TextPart{Text: "Here is a summary."} + webSearchCall := fantasy.ToolCallPart{ + ToolCallID: "srvtoolu_search", + ToolName: "web_search", + Input: `{"query":"coder"}`, + ProviderExecuted: true, + } + matchedResult := fantasy.ToolResultPart{ + ToolCallID: "srvtoolu_search", + Output: fantasy.ToolResultOutputContentText{Text: `{"ok":true}`}, + ProviderExecuted: true, + } + codeExecutionCall := fantasy.ToolCallPart{ + ToolCallID: "srvtoolu_code", + ToolName: "code_execution", + Input: `{"code":"print(1)"}`, + ProviderExecuted: true, + } + localCall := fantasy.ToolCallPart{ + ToolCallID: "toolu_local", + ToolName: "read_file", + Input: `{"path":"main.go"}`, + } + unpairedWebSearchCall := webSearchCall + unpairedWebSearchCall.ToolCallID = "srvtoolu_unpaired" + disableParallelToolUse := true + providerOptions := fantasy.ProviderOptions{ + fantasyanthropic.Name: &fantasyanthropic.ProviderOptions{ + DisableParallelToolUse: &disableParallelToolUse, + }, + } + enableParallelToolUse := false + providerOptionsAllowParallel := fantasy.ProviderOptions{ + fantasyanthropic.Name: &fantasyanthropic.ProviderOptions{ + DisableParallelToolUse: &enableParallelToolUse, + }, + } + + testCases := []struct { + name string + provider string + messages []fantasy.Message + want []fantasy.Message + wantRemoved int + wantDropped int + }{ + { + name: "removes unpaired call and keeps text", + provider: fantasyanthropic.Name, + messages: []fantasy.Message{{ + Role: fantasy.MessageRoleAssistant, + Content: []fantasy.MessagePart{ + textPart, + webSearchCall, + }, + }}, + want: []fantasy.Message{{ + Role: fantasy.MessageRoleAssistant, + Content: []fantasy.MessagePart{textPart}, + }}, + wantRemoved: 1, + }, + { + name: "drops assistant message when only part is removed", + provider: fantasyanthropic.Name, + messages: []fantasy.Message{{ + Role: fantasy.MessageRoleAssistant, + Content: []fantasy.MessagePart{webSearchCall}, + }}, + want: []fantasy.Message{}, + wantRemoved: 1, + wantDropped: 1, + }, + { + name: "coalesces adjacent roles after dropping empty message", + provider: fantasyanthropic.Name, + messages: []fantasy.Message{ + { + Role: fantasy.MessageRoleUser, + Content: []fantasy.MessagePart{ + fantasy.TextPart{Text: "search for coder"}, + }, + }, + { + Role: fantasy.MessageRoleAssistant, + Content: []fantasy.MessagePart{webSearchCall}, + }, + { + Role: fantasy.MessageRoleUser, + Content: []fantasy.MessagePart{ + fantasy.TextPart{Text: "now summarize"}, + }, + ProviderOptions: providerOptions, + }, + }, + want: []fantasy.Message{{ + Role: fantasy.MessageRoleUser, + Content: []fantasy.MessagePart{ + fantasy.TextPart{Text: "search for coder"}, + fantasy.TextPart{ + Text: "now summarize", + ProviderOptions: providerOptions, + }, + }, + }}, + wantRemoved: 1, + wantDropped: 1, + }, + { + name: "coalesces adjacent provider options without flattening boundaries", + provider: fantasyanthropic.Name, + messages: []fantasy.Message{ + { + Role: fantasy.MessageRoleUser, + Content: []fantasy.MessagePart{ + fantasy.TextPart{Text: "search for coder"}, + }, + ProviderOptions: providerOptionsAllowParallel, + }, + { + Role: fantasy.MessageRoleAssistant, + Content: []fantasy.MessagePart{webSearchCall}, + }, + { + Role: fantasy.MessageRoleUser, + Content: []fantasy.MessagePart{ + fantasy.TextPart{Text: "now summarize"}, + }, + ProviderOptions: providerOptions, + }, + }, + want: []fantasy.Message{{ + Role: fantasy.MessageRoleUser, + Content: []fantasy.MessagePart{ + fantasy.TextPart{ + Text: "search for coder", + ProviderOptions: providerOptionsAllowParallel, + }, + fantasy.TextPart{ + Text: "now summarize", + ProviderOptions: providerOptions, + }, + }, + }}, + wantRemoved: 1, + wantDropped: 1, + }, + { + name: "keeps matched call and result", + provider: fantasyanthropic.Name, + messages: []fantasy.Message{{ + Role: fantasy.MessageRoleAssistant, + Content: []fantasy.MessagePart{ + webSearchCall, + matchedResult, + }, + }}, + want: []fantasy.Message{{ + Role: fantasy.MessageRoleAssistant, + Content: []fantasy.MessagePart{ + webSearchCall, + matchedResult, + }, + }}, + }, + { + name: "removes only unpaired call from mixed message", + provider: fantasyanthropic.Name, + messages: []fantasy.Message{{ + Role: fantasy.MessageRoleAssistant, + Content: []fantasy.MessagePart{ + textPart, + webSearchCall, + matchedResult, + unpairedWebSearchCall, + }, + }}, + want: []fantasy.Message{{ + Role: fantasy.MessageRoleAssistant, + Content: []fantasy.MessagePart{ + textPart, + webSearchCall, + matchedResult, + }, + }}, + wantRemoved: 1, + }, + { + name: "removes unpaired provider call and keeps local call", + provider: fantasyanthropic.Name, + messages: []fantasy.Message{{ + Role: fantasy.MessageRoleAssistant, + Content: []fantasy.MessagePart{ + textPart, + codeExecutionCall, + localCall, + }, + }}, + want: []fantasy.Message{{ + Role: fantasy.MessageRoleAssistant, + Content: []fantasy.MessagePart{ + textPart, + localCall, + }, + }}, + wantRemoved: 1, + }, + { + name: "leaves other providers unchanged", + provider: "fake", + messages: []fantasy.Message{{ + Role: fantasy.MessageRoleAssistant, + Content: []fantasy.MessagePart{webSearchCall}, + }}, + want: []fantasy.Message{{ + Role: fantasy.MessageRoleAssistant, + Content: []fantasy.MessagePart{webSearchCall}, + }}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + sanitized, stats := chatprompt.SanitizeAnthropicProviderToolCalls( + tc.provider, + tc.messages, + ) + require.Equal(t, tc.wantRemoved, stats.RemovedToolCalls) + require.Equal(t, tc.wantDropped, stats.DroppedMessages) + require.Equal(t, tc.want, sanitized) + }) + } +} + func TestConvertMessagesWithFiles_NormalizesAssistantToolCallInput(t *testing.T) { t.Parallel() @@ -509,6 +748,15 @@ func TestInjectMissingToolResults_SkipsProviderExecuted(t *testing.T) { } } require.Equal(t, []string{"toolu_local"}, resultIDs) + sanitized, sanitizeStats := chatprompt.SanitizeAnthropicProviderToolCalls( + fantasyanthropic.Name, + prompt, + ) + require.Equal(t, 1, sanitizeStats.RemovedToolCalls) + require.Len(t, sanitized, 2) + remainingToolCalls := chatprompt.ExtractToolCalls(sanitized[0].Content) + require.Len(t, remainingToolCalls, 1) + require.Equal(t, "toolu_local", remainingToolCalls[0].ToolCallID) } // TestInjectMissingToolUses_DropsProviderExecutedOrphans verifies that @@ -695,7 +943,7 @@ 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 + // and a text block, mimicking a web_search step where persistStep // keeps the PE result inline. assistantContent := mustMarshalContent(t, []fantasy.Content{ fantasy.ToolCallContent{ @@ -751,7 +999,7 @@ func TestProviderExecutedResultInAssistantContent(t *testing.T) { // 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 +// 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) { @@ -1903,10 +2151,10 @@ func TestConvertMessagesWithFiles_FiltersEmptyTextAndReasoningParts(t *testing.T t.Parallel() parts := []codersdk.ChatMessagePart{ - codersdk.ChatMessageText(""), // empty — filtered - codersdk.ChatMessageText(" \t\n "), // whitespace — filtered - codersdk.ChatMessageReasoning(""), // empty — filtered - codersdk.ChatMessageReasoning(" \n"), // whitespace — filtered + codersdk.ChatMessageText(""), // empty, filtered + codersdk.ChatMessageText(" \t\n "), // whitespace, filtered + codersdk.ChatMessageReasoning(""), // empty, filtered + codersdk.ChatMessageReasoning(" \n"), // whitespace, filtered codersdk.ChatMessageText("hello"), // kept codersdk.ChatMessageText(" hello "), // kept with original whitespace codersdk.ChatMessageReasoning("thinking deeply"), // kept @@ -1930,7 +2178,7 @@ func TestConvertMessagesWithFiles_FiltersEmptyTextAndReasoningParts(t *testing.T require.True(t, ok, "expected TextPart at index 0") require.Equal(t, "hello", textPart.Text) - // Leading/trailing whitespace is preserved — only + // Leading/trailing whitespace is preserved, only // all-whitespace parts are dropped. paddedPart, ok := fantasy.AsMessagePart[fantasy.TextPart](resultParts[1]) require.True(t, ok, "expected TextPart at index 1") @@ -1953,9 +2201,9 @@ func TestConvertMessagesWithFiles_FiltersEmptyTextAndReasoningParts(t *testing.T t.Parallel() parts := []codersdk.ChatMessagePart{ - codersdk.ChatMessageText(""), // empty — filtered - codersdk.ChatMessageText(" "), // whitespace — filtered - codersdk.ChatMessageReasoning(""), // empty — filtered + codersdk.ChatMessageText(""), // empty, filtered + codersdk.ChatMessageText(" "), // whitespace, filtered + codersdk.ChatMessageReasoning(""), // empty, filtered codersdk.ChatMessageText(" reply "), // kept with whitespace codersdk.ChatMessageToolCall("tc-1", "read_file", json.RawMessage(`{"path":"x"}`)), }