mirror of
https://github.com/coder/coder.git
synced 2026-09-22 05:05:20 +08:00
fix(coderd/x/chatd): drop orphan provider tool calls on replay (#25491)
Anthropic replay can fail when stored history contains a
provider-executed tool call like `web_search` without the matching
provider-executed result. That orphaned call is incomplete
provider-internal state, so replaying it can make an otherwise usable
chat unreplayable even though there is no search result to preserve.
This fixes replay by dropping orphan provider-executed tool calls from
the model-visible prompt, preserving signed reasoning and the rest of
the assistant content, then revalidating before the request. We do not
synthesize tool results or drop reasoning. The database can retain the
historical artifact for inspection, while Anthropic only sees replayable
content.
This matches permissively licensed prior art. Vercel AI SDK
(Apache-2.0), used by mux, keeps incomplete tool state in UI/history but
omits it from model requests with `convertToModelMessages(..., {
ignoreIncompleteToolCalls: true })`. LangChain, LiteLLM, and OpenAI
Agents (MIT for the relevant open-source code) also preserve Anthropic
signed reasoning as opaque replay data. Coder applies that model-visible
replay boundary explicitly because our persisted history is already in
provider-message form.
This matches mux, is cleaner than the older idea around not persisting
the search query tool, and the model handles the repaired prompt fine.
Closes CODAGT-448
## Before
<img width="963" height="491" alt="image"
src="https://github.com/user-attachments/assets/a7788ebf-2728-4420-90cf-5e4f6905bdf7"
/>
## After
<img width="842" height="513" alt="image"
src="https://github.com/user-attachments/assets/ae39c262-7586-4e2d-b7db-1b639a7e8e15"
/>
This commit is contained in:
@@ -13,7 +13,6 @@ import (
|
||||
"golang.org/x/xerrors"
|
||||
|
||||
"cdr.dev/slog/v3/sloggers/slogtest"
|
||||
"github.com/coder/coder/v2/coderd/x/chatd/chaterror"
|
||||
"github.com/coder/coder/v2/coderd/x/chatd/chatopenai"
|
||||
"github.com/coder/coder/v2/coderd/x/chatd/chattest"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
@@ -421,16 +420,27 @@ func TestRun_ChainBrokenReloadFailureStillClearsChain(t *testing.T) {
|
||||
requireTextPrompt(t, secondPrompt, "prepared")
|
||||
}
|
||||
|
||||
func TestRun_ChainBrokenRecoveryPrepareFailureReturnsPreparePhaseError(t *testing.T) {
|
||||
func TestRun_ChainBrokenRecoveryDropsOrphanProviderToolCall(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var streamCalls int
|
||||
var (
|
||||
streamCalls int
|
||||
secondCallOpt fantasy.ProviderOptions
|
||||
secondPrompt []fantasy.Message
|
||||
)
|
||||
model := &chattest.FakeModel{
|
||||
ProviderName: fantasyanthropic.Name,
|
||||
ModelName: "claude-test",
|
||||
StreamFn: func(_ context.Context, _ fantasy.Call) (fantasy.StreamResponse, error) {
|
||||
StreamFn: func(_ context.Context, call fantasy.Call) (fantasy.StreamResponse, error) {
|
||||
streamCalls++
|
||||
return nil, xerrors.New(chainBrokenErrorMessage)
|
||||
switch streamCalls {
|
||||
case 1:
|
||||
return nil, xerrors.New(chainBrokenErrorMessage)
|
||||
default:
|
||||
secondCallOpt = call.ProviderOptions
|
||||
secondPrompt = call.Prompt
|
||||
return finishingStream(), nil
|
||||
}
|
||||
},
|
||||
}
|
||||
|
||||
@@ -465,18 +475,19 @@ func TestRun_ChainBrokenRecoveryPrepareFailureReturnsPreparePhaseError(t *testin
|
||||
},
|
||||
})
|
||||
|
||||
require.Error(t, err)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, 1, reloadCalls)
|
||||
require.Equal(t, 1, streamCalls, "retry must fail before issuing another provider call")
|
||||
require.ErrorContains(t, err, "prepare prompt:")
|
||||
require.NotContains(t, err.Error(), "stream response:")
|
||||
require.Equal(t, chaterror.ClassifiedError{
|
||||
Message: "The chat continuation failed due to an internal state mismatch. This is not a configuration or billing issue. Start a new chat to continue.",
|
||||
Detail: "Anthropic replay diagnostic: match=provider_tool_guard_postcondition_failed.",
|
||||
Kind: codersdk.ChatErrorKindGeneric,
|
||||
Provider: fantasyanthropic.Name,
|
||||
Retryable: false,
|
||||
}, chaterror.Classify(err))
|
||||
require.Equal(t, 2, streamCalls)
|
||||
require.False(t, chatopenai.HasPreviousResponseID(secondCallOpt))
|
||||
requireNoProviderExecutedToolCallPrompt(t, secondPrompt)
|
||||
requireAnthropicProviderToolPromptSafe(t, secondPrompt)
|
||||
requireTextPrompt(t, secondPrompt, "search")
|
||||
requireTextPrompt(t, secondPrompt, "partial")
|
||||
requireTextPrompt(t, secondPrompt, "continue")
|
||||
reasoningPart := requireReasoningPrompt(t, secondPrompt)
|
||||
reasoningMetadata := fantasyanthropic.GetReasoningMetadata(reasoningPart.ProviderOptions)
|
||||
require.NotNil(t, reasoningMetadata)
|
||||
require.Equal(t, "redacted-payload", reasoningMetadata.RedactedData)
|
||||
}
|
||||
|
||||
func TestRun_ChainBrokenWithoutChainModeIsSafe(t *testing.T) {
|
||||
|
||||
@@ -1275,6 +1275,21 @@ func requireNoProviderExecutedToolResultContent(t *testing.T, content []fantasy.
|
||||
}
|
||||
}
|
||||
|
||||
func requireReasoningPrompt(t *testing.T, prompt []fantasy.Message) fantasy.ReasoningPart {
|
||||
t.Helper()
|
||||
|
||||
for _, message := range prompt {
|
||||
for _, part := range message.Content {
|
||||
reasoningPart, ok := fantasy.AsMessagePart[fantasy.ReasoningPart](part)
|
||||
if ok {
|
||||
return reasoningPart
|
||||
}
|
||||
}
|
||||
}
|
||||
t.Fatal("missing prompt reasoning")
|
||||
return fantasy.ReasoningPart{}
|
||||
}
|
||||
|
||||
func requireTextPrompt(t *testing.T, prompt []fantasy.Message, text string) fantasy.TextPart {
|
||||
t.Helper()
|
||||
|
||||
@@ -3744,15 +3759,17 @@ func TestRun_AnthropicProviderToolPreRequestGuard(t *testing.T) {
|
||||
require.Equal(t, 1, requireLogField(t, entries[0], "removed_tool_calls"))
|
||||
require.Equal(t, 1, requireLogField(t, entries[0], "removed_tool_results"))
|
||||
})
|
||||
t.Run("run fails before provider call when latest signed assistant is unreplayable", func(t *testing.T) {
|
||||
t.Run("run drops orphan provider call before provider request", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
streamCalls := 0
|
||||
var capturedPrompt fantasy.Prompt
|
||||
model := &chattest.FakeModel{
|
||||
ProviderName: fantasyanthropic.Name,
|
||||
ModelName: "claude-test",
|
||||
StreamFn: func(_ context.Context, _ fantasy.Call) (fantasy.StreamResponse, error) {
|
||||
StreamFn: func(_ context.Context, call fantasy.Call) (fantasy.StreamResponse, error) {
|
||||
streamCalls++
|
||||
capturedPrompt = call.Prompt
|
||||
return finishingStream(), nil
|
||||
},
|
||||
}
|
||||
@@ -3788,15 +3805,15 @@ func TestRun_AnthropicProviderToolPreRequestGuard(t *testing.T) {
|
||||
return nil
|
||||
},
|
||||
})
|
||||
require.Error(t, err)
|
||||
require.Zero(t, streamCalls)
|
||||
require.Equal(t, chaterror.ClassifiedError{
|
||||
Message: "The chat continuation failed due to an internal state mismatch. This is not a configuration or billing issue. Start a new chat to continue.",
|
||||
Detail: "Anthropic replay diagnostic: match=provider_tool_guard_postcondition_failed.",
|
||||
Kind: codersdk.ChatErrorKindGeneric,
|
||||
Provider: "anthropic",
|
||||
Retryable: false,
|
||||
}, chaterror.Classify(err))
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, 1, streamCalls)
|
||||
requireNoProviderExecutedToolCallPrompt(t, capturedPrompt)
|
||||
requireAnthropicProviderToolPromptSafe(t, capturedPrompt)
|
||||
requireTextPrompt(t, capturedPrompt, "partial")
|
||||
reasoningPart := requireReasoningPrompt(t, capturedPrompt)
|
||||
reasoningMetadata := fantasyanthropic.GetReasoningMetadata(reasoningPart.ProviderOptions)
|
||||
require.NotNil(t, reasoningMetadata)
|
||||
require.Equal(t, "redacted-payload", reasoningMetadata.RedactedData)
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -14,10 +14,9 @@ import (
|
||||
|
||||
const maxAnthropicProviderToolViolationLogDetails = 32
|
||||
|
||||
// Anthropic immutability contract. The latest assistant message containing
|
||||
// signed or redacted reasoning is an immutable transcript boundary. Helpers
|
||||
// in this file enforce it via HasAnthropicSignedReasoningOptions,
|
||||
// latestAssistantMessageIndexWithSignedReasoning, and appendSanitizedMessage.
|
||||
// Anthropic replay contract. Signed or redacted reasoning parts are preserved.
|
||||
// Provider-executed tool calls without matching results are incomplete
|
||||
// provider-internal state and are removed from model-visible replay.
|
||||
|
||||
// supportedAnthropicProviderToolNames is the allowlist of provider-executed
|
||||
// tool names the Anthropic provider in fantasy can currently serialize.
|
||||
@@ -495,16 +494,35 @@ func ApplyAnthropicProviderToolGuard(
|
||||
if len(violations) == 0 {
|
||||
return messages, nil
|
||||
}
|
||||
|
||||
guarded, orphanCallStats := dropOrphanAnthropicProviderToolCallsFromMessages(
|
||||
messages,
|
||||
violations,
|
||||
)
|
||||
LogAnthropicProviderToolSanitization(
|
||||
ctx,
|
||||
logger,
|
||||
"pre_request_guard_orphan_call_drop",
|
||||
provider,
|
||||
modelName,
|
||||
orphanCallStats,
|
||||
slog.F("validation_violations", len(violations)),
|
||||
)
|
||||
violations = ValidateAnthropicProviderToolHistory(guarded)
|
||||
if len(violations) == 0 {
|
||||
return guarded, nil
|
||||
}
|
||||
|
||||
affectedMessages := messageIndexesFromAnthropicProviderToolViolations(
|
||||
violations,
|
||||
len(messages),
|
||||
len(guarded),
|
||||
)
|
||||
guarded := sanitizeAnthropicProviderToolGuardMessages(
|
||||
guarded = sanitizeAnthropicProviderToolGuardMessages(
|
||||
ctx,
|
||||
logger,
|
||||
provider,
|
||||
modelName,
|
||||
messages,
|
||||
guarded,
|
||||
affectedMessages,
|
||||
len(violations),
|
||||
)
|
||||
@@ -992,6 +1010,58 @@ func isSafeAnthropicProviderToolPrompt(messages []fantasy.Message) bool {
|
||||
return len(ValidateAnthropicProviderToolHistory(messages)) == 0
|
||||
}
|
||||
|
||||
func dropOrphanAnthropicProviderToolCallsFromMessages(
|
||||
messages []fantasy.Message,
|
||||
violations []AnthropicProviderToolHistoryViolation,
|
||||
) ([]fantasy.Message, AnthropicProviderToolSanitizationStats) {
|
||||
var stats AnthropicProviderToolSanitizationStats
|
||||
remove := make(map[anthropicProviderToolPartKey]struct{})
|
||||
for _, violation := range violations {
|
||||
if violation.Reason != anthropicProviderToolViolationOrphanCall {
|
||||
continue
|
||||
}
|
||||
if violation.MessageIndex < 0 || violation.MessageIndex >= len(messages) {
|
||||
continue
|
||||
}
|
||||
remove[anthropicProviderToolPartKey{
|
||||
messageIndex: violation.MessageIndex,
|
||||
partIndex: violation.PartIndex,
|
||||
}] = struct{}{}
|
||||
}
|
||||
if len(remove) == 0 {
|
||||
return messages, stats
|
||||
}
|
||||
|
||||
out := make([]fantasy.Message, 0, len(messages))
|
||||
for messageIndex, message := range messages {
|
||||
parts := make([]fantasy.MessagePart, 0, len(message.Content))
|
||||
removedFromMessage := 0
|
||||
for partIndex, part := range message.Content {
|
||||
key := anthropicProviderToolPartKey{
|
||||
messageIndex: messageIndex,
|
||||
partIndex: partIndex,
|
||||
}
|
||||
if _, ok := remove[key]; ok {
|
||||
if toolCall, ok := safeMessageToolCallPart(part); ok && toolCall.ProviderExecuted {
|
||||
stats.RemovedToolCalls++
|
||||
removedFromMessage++
|
||||
continue
|
||||
}
|
||||
}
|
||||
parts = append(parts, part)
|
||||
}
|
||||
if removedFromMessage > 0 {
|
||||
if len(parts) == 0 {
|
||||
stats.DroppedMessages++
|
||||
continue
|
||||
}
|
||||
message.Content = parts
|
||||
}
|
||||
out = appendSanitizedMessage(out, message)
|
||||
}
|
||||
return out, stats
|
||||
}
|
||||
|
||||
func messageIndexesFromAnthropicProviderToolViolations(
|
||||
violations []AnthropicProviderToolHistoryViolation,
|
||||
messageCount int,
|
||||
|
||||
@@ -190,7 +190,7 @@ func TestApplyAnthropicProviderToolGuardRepairsOlderSignedAssistantWhenLatestAss
|
||||
}
|
||||
}
|
||||
|
||||
func TestApplyAnthropicProviderToolGuardDoesNotMergeAcrossLatestReasoningAssistant(t *testing.T) {
|
||||
func TestApplyAnthropicProviderToolGuardDropsOrphanProviderCallsAcrossLatestReasoningAssistant(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
reasoningVariants := []struct {
|
||||
@@ -216,8 +216,18 @@ func TestApplyAnthropicProviderToolGuardDoesNotMergeAcrossLatestReasoningAssista
|
||||
},
|
||||
)
|
||||
|
||||
require.ErrorIs(t, err, chatsanitize.ErrAnthropicProviderToolPromptUnsafe)
|
||||
require.Nil(t, guarded)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, []fantasy.Message{
|
||||
sanitizedPriorAssistantForTest(),
|
||||
{
|
||||
Role: fantasy.MessageRoleAssistant,
|
||||
Content: []fantasy.MessagePart{
|
||||
reasoningVariant.part,
|
||||
fantasy.TextPart{Text: "answer"},
|
||||
},
|
||||
},
|
||||
}, guarded)
|
||||
require.Empty(t, chatsanitize.ValidateAnthropicProviderToolHistory(guarded))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user