From ea1d6c96aeda535cbd6bd258a6c146f1e7fb51bd Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 19 Jun 2026 19:27:34 +0000 Subject: [PATCH 1/6] fix(coderd/x/chatd): drop foreign provider-executed tool history on provider switch --- coderd/x/chatd/generation_preparer.go | 7 + coderd/x/chatd/provider_switch_sanitize.go | 150 ++++++++++++++++ .../x/chatd/provider_switch_sanitize_test.go | 169 ++++++++++++++++++ 3 files changed, 326 insertions(+) create mode 100644 coderd/x/chatd/provider_switch_sanitize.go create mode 100644 coderd/x/chatd/provider_switch_sanitize_test.go diff --git a/coderd/x/chatd/generation_preparer.go b/coderd/x/chatd/generation_preparer.go index a0aec403ba279..e5671b8d3ed81 100644 --- a/coderd/x/chatd/generation_preparer.go +++ b/coderd/x/chatd/generation_preparer.go @@ -215,6 +215,13 @@ func (server *Server) prepareGeneration( resolvedUserPrompt string ) + // Drop provider-executed tool history produced by a different provider + // before building the prompt. A provider that shares another's wire format + // (e.g. Bedrock and Anthropic) can still reject the other's + // provider-executed blocks, so a mid-chat provider switch must not replay + // them. + promptRows = server.sanitizeForeignProviderToolRows(ctx, logger, promptRows, modelConfig.ID) + persistedSkills := skillsFromParts(promptRows) hasContextFiles := false if chat.WorkspaceID.Valid { diff --git a/coderd/x/chatd/provider_switch_sanitize.go b/coderd/x/chatd/provider_switch_sanitize.go new file mode 100644 index 0000000000000..50933de42c5a6 --- /dev/null +++ b/coderd/x/chatd/provider_switch_sanitize.go @@ -0,0 +1,150 @@ +package chatd + +import ( + "context" + + "github.com/google/uuid" + + "cdr.dev/slog/v3" + + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/x/chatd/chatprompt" + "github.com/coder/coder/v2/codersdk" +) + +// providerSwitchStripStats counts the provider-executed tool history removed +// when sanitizing a prompt for a model-provider switch. +type providerSwitchStripStats struct { + RemovedToolCalls int + RemovedToolResults int + DroppedMessages int +} + +// stripForeignProviderExecutedToolRows removes provider-executed tool blocks +// (both calls and results) from assistant history rows whose producing provider +// differs from targetProvider. Provider-executed tool blocks are only valid for +// the provider that produced them: a provider sharing another's wire format can +// still reject them (e.g. Bedrock rejects Anthropic web_search_tool_result with +// HTTP 400), so switching providers mid-chat must drop the foreign blocks. +// +// originProvider resolves a row's ModelConfigID to a normalized provider name; +// ok is false when the origin cannot be determined, in which case the row is +// treated as foreign (fail closed). Rows from the target provider, non-assistant +// rows, rows with no provider-executed parts, and rows that fail to parse or +// re-marshal are returned unchanged. Rows emptied by stripping are dropped. +// +// Provenance is the model config provider (derived from the AI provider type), +// not anything fantasy reports, so it stays correct when requests route through +// aibridged, which serializes both Anthropic and Bedrock as the Anthropic wire +// format. +func stripForeignProviderExecutedToolRows( + rows []database.ChatMessage, + targetProvider string, + originProvider func(uuid.NullUUID) (string, bool), +) ([]database.ChatMessage, providerSwitchStripStats) { + var stats providerSwitchStripStats + if targetProvider == "" || len(rows) == 0 { + return rows, stats + } + + out := make([]database.ChatMessage, 0, len(rows)) + for _, row := range rows { + // Provider-executed tool blocks only appear on assistant rows. + if row.Role != database.ChatMessageRoleAssistant { + out = append(out, row) + continue + } + if origin, ok := originProvider(row.ModelConfigID); ok && origin == targetProvider { + out = append(out, row) + continue + } + + parts, err := chatprompt.ParseContent(row) + if err != nil { + // Leave unparseable rows untouched; the converter handles them. + out = append(out, row) + continue + } + + kept := make([]codersdk.ChatMessagePart, 0, len(parts)) + var removedCalls, removedResults int + for _, part := range parts { + switch { + case part.Type == codersdk.ChatMessagePartTypeToolCall && part.ProviderExecuted: + removedCalls++ + case part.Type == codersdk.ChatMessagePartTypeToolResult && part.ProviderExecuted: + removedResults++ + default: + kept = append(kept, part) + } + } + if removedCalls == 0 && removedResults == 0 { + out = append(out, row) + continue + } + if len(kept) == 0 { + stats.RemovedToolCalls += removedCalls + stats.RemovedToolResults += removedResults + stats.DroppedMessages++ + continue + } + + content, err := chatprompt.MarshalParts(kept) + if err != nil { + // Keep the original row rather than corrupting history. + out = append(out, row) + continue + } + row.Content = content + row.ContentVersion = chatprompt.ContentVersionV1 + stats.RemovedToolCalls += removedCalls + stats.RemovedToolResults += removedResults + out = append(out, row) + } + return out, stats +} + +// sanitizeForeignProviderToolRows strips provider-executed tool history produced +// by a provider other than the one targeted by modelConfigID. It resolves each +// row's provider via the model config cache and logs a single summary when +// anything is removed. +func (server *Server) sanitizeForeignProviderToolRows( + ctx context.Context, + logger slog.Logger, + rows []database.ChatMessage, + modelConfigID uuid.UUID, +) []database.ChatMessage { + _, target, err := server.resolveModelConfigAndNormalizedProvider(ctx, modelConfigID) + if err != nil || target == "" { + // Without a known target provider we cannot classify history; leave it. + return rows + } + + cache := make(map[uuid.UUID]string) + originProvider := func(id uuid.NullUUID) (string, bool) { + if !id.Valid { + return "", false + } + if provider, seen := cache[id.UUID]; seen { + return provider, provider != "" + } + _, provider, rErr := server.resolveModelConfigAndNormalizedProvider(ctx, id.UUID) + if rErr != nil { + provider = "" + } + cache[id.UUID] = provider + return provider, provider != "" + } + + sanitized, stats := stripForeignProviderExecutedToolRows(rows, target, originProvider) + if stats != (providerSwitchStripStats{}) { + logger.Warn(ctx, "stripped foreign provider-executed tool history", + slog.F("phase", "provider_switch"), + slog.F("target_provider", target), + slog.F("removed_tool_calls", stats.RemovedToolCalls), + slog.F("removed_tool_results", stats.RemovedToolResults), + slog.F("dropped_messages", stats.DroppedMessages), + ) + } + return sanitized +} diff --git a/coderd/x/chatd/provider_switch_sanitize_test.go b/coderd/x/chatd/provider_switch_sanitize_test.go new file mode 100644 index 0000000000000..d4624ba220c41 --- /dev/null +++ b/coderd/x/chatd/provider_switch_sanitize_test.go @@ -0,0 +1,169 @@ +package chatd + +import ( + "encoding/json" + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/x/chatd/chatprompt" + "github.com/coder/coder/v2/codersdk" +) + +func TestStripForeignProviderExecutedToolRows(t *testing.T) { + t.Parallel() + + const ( + anthropic = "anthropic" + bedrock = "bedrock" + openai = "openai" + ) + + anthropicCfg := uuid.New() + openAICfg := uuid.New() + unknownCfg := uuid.New() + + peCall := func(id string) codersdk.ChatMessagePart { + p := codersdk.ChatMessageToolCall(id, "web_search", json.RawMessage(`{"query":"x"}`)) + p.ProviderExecuted = true + return p + } + peResult := func(id string) codersdk.ChatMessagePart { + p := codersdk.ChatMessageToolResult(id, "web_search", json.RawMessage(`{"ok":true}`), false, false) + p.ProviderExecuted = true + return p + } + localCall := func(id string) codersdk.ChatMessagePart { + return codersdk.ChatMessageToolCall(id, "read_file", json.RawMessage(`{}`)) + } + text := func(s string) codersdk.ChatMessagePart { return codersdk.ChatMessageText(s) } + + assistantRow := func(t *testing.T, cfg uuid.UUID, parts ...codersdk.ChatMessagePart) database.ChatMessage { + t.Helper() + content, err := chatprompt.MarshalParts(parts) + require.NoError(t, err) + return database.ChatMessage{ + Role: database.ChatMessageRoleAssistant, + ModelConfigID: uuid.NullUUID{UUID: cfg, Valid: cfg != uuid.Nil}, + Content: content, + ContentVersion: chatprompt.ContentVersionV1, + } + } + userRow := func(t *testing.T, s string) database.ChatMessage { + t.Helper() + content, err := chatprompt.MarshalParts([]codersdk.ChatMessagePart{text(s)}) + require.NoError(t, err) + return database.ChatMessage{ + Role: database.ChatMessageRoleUser, + Content: content, + ContentVersion: chatprompt.ContentVersionV1, + } + } + + // origin maps a model config ID to its normalized provider. unknownCfg is + // intentionally absent so the resolver reports an unknown origin. + origin := func(cfgByProvider map[uuid.UUID]string) func(uuid.NullUUID) (string, bool) { + return func(id uuid.NullUUID) (string, bool) { + if !id.Valid { + return "", false + } + provider, ok := cfgByProvider[id.UUID] + return provider, ok + } + } + resolver := origin(map[uuid.UUID]string{ + anthropicCfg: anthropic, + openAICfg: openai, + }) + + // partsOf parses a row's content back into SDK parts for comparison. + partsOf := func(t *testing.T, row database.ChatMessage) []codersdk.ChatMessagePart { + t.Helper() + parts, err := chatprompt.ParseContent(row) + require.NoError(t, err) + return parts + } + + t.Run("same provider kept", func(t *testing.T) { + t.Parallel() + rows := []database.ChatMessage{ + userRow(t, "hi"), + assistantRow(t, anthropicCfg, peCall("ws"), peResult("ws"), text("done")), + } + got, stats := stripForeignProviderExecutedToolRows(rows, anthropic, resolver) + require.Equal(t, rows, got) + require.Zero(t, stats) + }) + + t.Run("anthropic to bedrock drops provider blocks", func(t *testing.T) { + t.Parallel() + rows := []database.ChatMessage{ + userRow(t, "hi"), + assistantRow(t, anthropicCfg, peCall("ws"), peResult("ws"), text("done")), + } + got, stats := stripForeignProviderExecutedToolRows(rows, bedrock, resolver) + require.Len(t, got, 2) + require.Equal(t, []codersdk.ChatMessagePart{text("done")}, partsOf(t, got[1])) + require.Equal(t, providerSwitchStripStats{RemovedToolCalls: 1, RemovedToolResults: 1}, stats) + }) + + t.Run("foreign-only row dropped", func(t *testing.T) { + t.Parallel() + rows := []database.ChatMessage{ + userRow(t, "hi"), + assistantRow(t, anthropicCfg, peCall("ws")), + userRow(t, "again"), + } + got, stats := stripForeignProviderExecutedToolRows(rows, bedrock, resolver) + require.Len(t, got, 2) + require.Equal(t, database.ChatMessageRoleUser, got[0].Role) + require.Equal(t, database.ChatMessageRoleUser, got[1].Role) + require.Equal(t, providerSwitchStripStats{RemovedToolCalls: 1, DroppedMessages: 1}, stats) + }) + + t.Run("multi-provider keeps native strips foreign", func(t *testing.T) { + t.Parallel() + rows := []database.ChatMessage{ + assistantRow(t, openAICfg, peCall("os"), peResult("os"), text("openai")), + assistantRow(t, anthropicCfg, peCall("as"), peResult("as"), text("anthropic")), + } + got, stats := stripForeignProviderExecutedToolRows(rows, anthropic, resolver) + require.Len(t, got, 2) + require.Equal(t, []codersdk.ChatMessagePart{text("openai")}, partsOf(t, got[0])) + require.Equal(t, rows[1], got[1]) + require.Equal(t, providerSwitchStripStats{RemovedToolCalls: 1, RemovedToolResults: 1}, stats) + }) + + t.Run("non-provider-executed parts untouched", func(t *testing.T) { + t.Parallel() + rows := []database.ChatMessage{ + assistantRow(t, anthropicCfg, text("hello"), localCall("local")), + } + got, stats := stripForeignProviderExecutedToolRows(rows, bedrock, resolver) + require.Equal(t, rows, got) + require.Zero(t, stats) + }) + + t.Run("empty target is a no-op", func(t *testing.T) { + t.Parallel() + rows := []database.ChatMessage{ + assistantRow(t, anthropicCfg, peCall("ws"), peResult("ws")), + } + got, stats := stripForeignProviderExecutedToolRows(rows, "", resolver) + require.Equal(t, rows, got) + require.Zero(t, stats) + }) + + t.Run("unknown origin fails closed", func(t *testing.T) { + t.Parallel() + rows := []database.ChatMessage{ + assistantRow(t, unknownCfg, peResult("ws"), text("done")), + } + got, stats := stripForeignProviderExecutedToolRows(rows, bedrock, resolver) + require.Len(t, got, 1) + require.Equal(t, []codersdk.ChatMessagePart{text("done")}, partsOf(t, got[0])) + require.Equal(t, providerSwitchStripStats{RemovedToolResults: 1}, stats) + }) +} From 1f581a29999d5b1a9b58f0109636de32c88066fb Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 19 Jun 2026 19:43:24 +0000 Subject: [PATCH 2/6] fixup! fix(coderd/x/chatd): drop foreign provider-executed tool history on provider switch --- ...sanitize_test.go => provider_switch_sanitize_internal_test.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename coderd/x/chatd/{provider_switch_sanitize_test.go => provider_switch_sanitize_internal_test.go} (100%) diff --git a/coderd/x/chatd/provider_switch_sanitize_test.go b/coderd/x/chatd/provider_switch_sanitize_internal_test.go similarity index 100% rename from coderd/x/chatd/provider_switch_sanitize_test.go rename to coderd/x/chatd/provider_switch_sanitize_internal_test.go From 6bca249e70b93f4d595c488451ad67d2c65eff9b Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 19 Jun 2026 19:44:13 +0000 Subject: [PATCH 3/6] fixup! fix(coderd/x/chatd): drop foreign provider-executed tool history on provider switch --- coderd/x/chatd/provider_switch_sanitize.go | 1 - coderd/x/chatd/provider_switch_sanitize_internal_test.go | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/coderd/x/chatd/provider_switch_sanitize.go b/coderd/x/chatd/provider_switch_sanitize.go index 50933de42c5a6..3268f4249d56f 100644 --- a/coderd/x/chatd/provider_switch_sanitize.go +++ b/coderd/x/chatd/provider_switch_sanitize.go @@ -6,7 +6,6 @@ import ( "github.com/google/uuid" "cdr.dev/slog/v3" - "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/x/chatd/chatprompt" "github.com/coder/coder/v2/codersdk" diff --git a/coderd/x/chatd/provider_switch_sanitize_internal_test.go b/coderd/x/chatd/provider_switch_sanitize_internal_test.go index d4624ba220c41..1782798018c80 100644 --- a/coderd/x/chatd/provider_switch_sanitize_internal_test.go +++ b/coderd/x/chatd/provider_switch_sanitize_internal_test.go @@ -38,7 +38,7 @@ func TestStripForeignProviderExecutedToolRows(t *testing.T) { localCall := func(id string) codersdk.ChatMessagePart { return codersdk.ChatMessageToolCall(id, "read_file", json.RawMessage(`{}`)) } - text := func(s string) codersdk.ChatMessagePart { return codersdk.ChatMessageText(s) } + text := codersdk.ChatMessageText assistantRow := func(t *testing.T, cfg uuid.UUID, parts ...codersdk.ChatMessagePart) database.ChatMessage { t.Helper() From 80a6a939bf8defcc8aeb291c20be3e5de28122c0 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 19 Jun 2026 19:53:11 +0000 Subject: [PATCH 4/6] fixup! fix(coderd/x/chatd): drop foreign provider-executed tool history on provider switch --- coderd/x/chatd/provider_switch_sanitize.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/x/chatd/provider_switch_sanitize.go b/coderd/x/chatd/provider_switch_sanitize.go index 3268f4249d56f..5845425ecd496 100644 --- a/coderd/x/chatd/provider_switch_sanitize.go +++ b/coderd/x/chatd/provider_switch_sanitize.go @@ -60,7 +60,7 @@ func stripForeignProviderExecutedToolRows( parts, err := chatprompt.ParseContent(row) if err != nil { - // Leave unparseable rows untouched; the converter handles them. + // Leave unparsable rows untouched; the converter handles them. out = append(out, row) continue } From e339f1182afdcbbeec2cf6b9c4a486f3b581f29b Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 19 Jun 2026 20:12:58 +0000 Subject: [PATCH 5/6] fixup! fix(coderd/x/chatd): drop foreign provider-executed tool history on provider switch --- coderd/x/chatd/generation_preparer.go | 2 +- coderd/x/chatd/provider_switch_sanitize.go | 18 ++++++++++++------ .../provider_switch_sanitize_internal_test.go | 4 ++-- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/coderd/x/chatd/generation_preparer.go b/coderd/x/chatd/generation_preparer.go index e5671b8d3ed81..b206486b97669 100644 --- a/coderd/x/chatd/generation_preparer.go +++ b/coderd/x/chatd/generation_preparer.go @@ -220,7 +220,7 @@ func (server *Server) prepareGeneration( // (e.g. Bedrock and Anthropic) can still reject the other's // provider-executed blocks, so a mid-chat provider switch must not replay // them. - promptRows = server.sanitizeForeignProviderToolRows(ctx, logger, promptRows, modelConfig.ID) + promptRows = server.sanitizeForeignProviderExecutedToolRows(ctx, logger, promptRows, modelConfig.ID) persistedSkills := skillsFromParts(promptRows) hasContextFiles := false diff --git a/coderd/x/chatd/provider_switch_sanitize.go b/coderd/x/chatd/provider_switch_sanitize.go index 5845425ecd496..fbc087db61ccd 100644 --- a/coderd/x/chatd/provider_switch_sanitize.go +++ b/coderd/x/chatd/provider_switch_sanitize.go @@ -48,7 +48,9 @@ func stripForeignProviderExecutedToolRows( out := make([]database.ChatMessage, 0, len(rows)) for _, row := range rows { - // Provider-executed tool blocks only appear on assistant rows. + // Provider-executed blocks that must be replayed live on assistant rows. + // Provider-executed results orphaned onto tool rows are dropped during + // prompt conversion, so they never reach the provider. if row.Role != database.ChatMessageRoleAssistant { out = append(out, row) continue @@ -103,11 +105,11 @@ func stripForeignProviderExecutedToolRows( return out, stats } -// sanitizeForeignProviderToolRows strips provider-executed tool history produced -// by a provider other than the one targeted by modelConfigID. It resolves each -// row's provider via the model config cache and logs a single summary when -// anything is removed. -func (server *Server) sanitizeForeignProviderToolRows( +// sanitizeForeignProviderExecutedToolRows strips provider-executed tool history +// produced by a provider other than the one targeted by modelConfigID. It +// resolves each row's provider via the model config cache and logs a single +// summary when anything is removed. +func (server *Server) sanitizeForeignProviderExecutedToolRows( ctx context.Context, logger slog.Logger, rows []database.ChatMessage, @@ -116,6 +118,10 @@ func (server *Server) sanitizeForeignProviderToolRows( _, target, err := server.resolveModelConfigAndNormalizedProvider(ctx, modelConfigID) if err != nil || target == "" { // Without a known target provider we cannot classify history; leave it. + logger.Debug(ctx, "skipping provider-switch sanitization: target provider unresolved", + slog.F("model_config_id", modelConfigID), + slog.Error(err), + ) return rows } diff --git a/coderd/x/chatd/provider_switch_sanitize_internal_test.go b/coderd/x/chatd/provider_switch_sanitize_internal_test.go index 1782798018c80..72ed02e289f0d 100644 --- a/coderd/x/chatd/provider_switch_sanitize_internal_test.go +++ b/coderd/x/chatd/provider_switch_sanitize_internal_test.go @@ -64,12 +64,12 @@ func TestStripForeignProviderExecutedToolRows(t *testing.T) { // origin maps a model config ID to its normalized provider. unknownCfg is // intentionally absent so the resolver reports an unknown origin. - origin := func(cfgByProvider map[uuid.UUID]string) func(uuid.NullUUID) (string, bool) { + origin := func(providerByConfig map[uuid.UUID]string) func(uuid.NullUUID) (string, bool) { return func(id uuid.NullUUID) (string, bool) { if !id.Valid { return "", false } - provider, ok := cfgByProvider[id.UUID] + provider, ok := providerByConfig[id.UUID] return provider, ok } } From c46d4adf319a21c2bad10b9ed12c98d611cf5f65 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 19 Jun 2026 20:45:16 +0000 Subject: [PATCH 6/6] fixup! fix(coderd/x/chatd): drop foreign provider-executed tool history on provider switch --- coderd/x/chatd/provider_switch_sanitize.go | 7 +++++++ .../provider_switch_sanitize_internal_test.go | 14 ++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/coderd/x/chatd/provider_switch_sanitize.go b/coderd/x/chatd/provider_switch_sanitize.go index fbc087db61ccd..3a178847ed664 100644 --- a/coderd/x/chatd/provider_switch_sanitize.go +++ b/coderd/x/chatd/provider_switch_sanitize.go @@ -135,6 +135,13 @@ func (server *Server) sanitizeForeignProviderExecutedToolRows( } _, provider, rErr := server.resolveModelConfigAndNormalizedProvider(ctx, id.UUID) if rErr != nil { + // Unresolvable origin (e.g. a since-disabled or deleted config) is + // treated as foreign so we fail closed rather than replay blocks the + // target may reject. + logger.Debug(ctx, "provider-switch sanitization: origin provider unresolved, treating as foreign", + slog.F("model_config_id", id.UUID), + slog.Error(rErr), + ) provider = "" } cache[id.UUID] = provider diff --git a/coderd/x/chatd/provider_switch_sanitize_internal_test.go b/coderd/x/chatd/provider_switch_sanitize_internal_test.go index 72ed02e289f0d..7302a2c43414e 100644 --- a/coderd/x/chatd/provider_switch_sanitize_internal_test.go +++ b/coderd/x/chatd/provider_switch_sanitize_internal_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/google/uuid" + "github.com/sqlc-dev/pqtype" "github.com/stretchr/testify/require" "github.com/coder/coder/v2/coderd/database" @@ -166,4 +167,17 @@ func TestStripForeignProviderExecutedToolRows(t *testing.T) { require.Equal(t, []codersdk.ChatMessagePart{text("done")}, partsOf(t, got[0])) require.Equal(t, providerSwitchStripStats{RemovedToolResults: 1}, stats) }) + + t.Run("unparsable foreign row kept unchanged", func(t *testing.T) { + t.Parallel() + rows := []database.ChatMessage{{ + Role: database.ChatMessageRoleAssistant, + ModelConfigID: uuid.NullUUID{UUID: anthropicCfg, Valid: true}, + Content: pqtype.NullRawMessage{RawMessage: []byte("{not json"), Valid: true}, + ContentVersion: chatprompt.ContentVersionV1, + }} + got, stats := stripForeignProviderExecutedToolRows(rows, bedrock, resolver) + require.Equal(t, rows, got) + require.Zero(t, stats) + }) }