mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(coderd/x/chatd): block chain mode when provider missing tool results (#24782)
When `StopAfterTool` fires (e.g., `propose_plan`), the LLM response containing a `function_call` is stored at OpenAI via `store=true`, but the tool result is only persisted locally. On the next user message, `resolveChainMode` sees the tool result in the local DB and concludes all calls are resolved. Chain mode activates with `previous_response_id`, but OpenAI rejects because its stored chain has an unresolved `function_call`. This adds a `providerMissingToolResults` check to `resolveChainMode` that detects the `assistant(tool-call) → tool(result) → user` pattern with no follow-up assistant message. The absence of a follow-up assistant proves the tool results were never round-tripped to the provider. When detected, chain mode is blocked and the system falls back to full history replay, which includes both the tool call and its result. Deploying this fix un-bricks existing affected chats with no DB migration needed. > Generated by Coder Agents.
This commit is contained in:
+70
-3
@@ -3264,6 +3264,12 @@ type chainModeInfo struct {
|
||||
// hasUnresolvedLocalToolCalls is true when previousResponseID
|
||||
// points at an assistant message with pending local tool calls.
|
||||
hasUnresolvedLocalToolCalls bool
|
||||
// providerMissingToolResults is true when the assistant message
|
||||
// has local tool calls with local results, but no follow-up
|
||||
// assistant message exists to confirm the results were sent
|
||||
// back to the provider. This happens when StopAfterTool
|
||||
// terminates a turn before the results are round-tripped.
|
||||
providerMissingToolResults bool
|
||||
}
|
||||
|
||||
func userMessageContributesToChainMode(msg database.ChatMessage) bool {
|
||||
@@ -3343,10 +3349,57 @@ func assistantHasUnresolvedLocalToolCalls(
|
||||
return len(resolvedCallIDs) != len(localCallIDs)
|
||||
}
|
||||
|
||||
// providerHasMissingToolResults reports whether the assistant message
|
||||
// at assistantIdx has local tool calls whose results exist in the DB
|
||||
// but were never sent back to the provider. This is detected by the
|
||||
// absence of a follow-up assistant message after the tool results:
|
||||
// in normal flow the LLM processes tool results and produces a
|
||||
// follow-up response, but StopAfterTool skips that round-trip.
|
||||
func providerHasMissingToolResults(
|
||||
messages []database.ChatMessage,
|
||||
assistantIdx int,
|
||||
) bool {
|
||||
if assistantIdx < 0 || assistantIdx >= len(messages) {
|
||||
return false
|
||||
}
|
||||
|
||||
parts, err := chatprompt.ParseContent(messages[assistantIdx])
|
||||
if err != nil {
|
||||
// Parsing errors are already handled by
|
||||
// assistantHasUnresolvedLocalToolCalls.
|
||||
return false
|
||||
}
|
||||
|
||||
if !slices.ContainsFunc(parts, func(p codersdk.ChatMessagePart) bool {
|
||||
return p.Type == codersdk.ChatMessagePartTypeToolCall && !p.ProviderExecuted
|
||||
}) {
|
||||
return false
|
||||
}
|
||||
|
||||
// Scan forward past tool messages. If the first non-tool message
|
||||
// is not an assistant, the tool results were never round-tripped
|
||||
// to the provider.
|
||||
for i := assistantIdx + 1; i < len(messages); i++ {
|
||||
switch messages[i].Role {
|
||||
case database.ChatMessageRoleTool:
|
||||
continue
|
||||
case database.ChatMessageRoleAssistant:
|
||||
// A follow-up assistant exists; results were sent.
|
||||
return false
|
||||
default:
|
||||
// User or system message with no follow-up assistant.
|
||||
return true
|
||||
}
|
||||
}
|
||||
// Reached end of messages without a follow-up assistant.
|
||||
return true
|
||||
}
|
||||
|
||||
// shouldActivateChainMode reports whether a follow-up turn can use
|
||||
// previous_response_id instead of replaying history. It requires store=true,
|
||||
// a matching model config, meaningful trailing user input, non-plan mode, and
|
||||
// complete local tool state so the provider has all required outputs.
|
||||
// a matching model config, meaningful trailing user input, non-plan mode,
|
||||
// complete local tool state, and confirmation that tool results were
|
||||
// actually sent to the provider (not just persisted locally).
|
||||
func shouldActivateChainMode(
|
||||
providerOptions fantasy.ProviderOptions,
|
||||
info chainModeInfo,
|
||||
@@ -3358,7 +3411,8 @@ func shouldActivateChainMode(
|
||||
info.contributingTrailingUserCount > 0 &&
|
||||
info.modelConfigID == modelConfigID &&
|
||||
!isPlanModeTurn &&
|
||||
!info.hasUnresolvedLocalToolCalls
|
||||
!info.hasUnresolvedLocalToolCalls &&
|
||||
!info.providerMissingToolResults
|
||||
}
|
||||
|
||||
// resolveChainMode scans DB messages from the end to count trailing user
|
||||
@@ -3386,6 +3440,9 @@ func resolveChainMode(messages []database.ChatMessage) chainModeInfo {
|
||||
info.modelConfigID = messages[i].ModelConfigID.UUID
|
||||
}
|
||||
info.hasUnresolvedLocalToolCalls = assistantHasUnresolvedLocalToolCalls(messages, i)
|
||||
if !info.hasUnresolvedLocalToolCalls {
|
||||
info.providerMissingToolResults = providerHasMissingToolResults(messages, i)
|
||||
}
|
||||
return info
|
||||
}
|
||||
return info
|
||||
@@ -6762,6 +6819,16 @@ func (p *Server) runChat(
|
||||
modelConfig.ID,
|
||||
isPlanModeTurn,
|
||||
)
|
||||
if !chainModeActive && chainInfo.previousResponseID != "" {
|
||||
logger.Debug(ctx, "chain mode disabled",
|
||||
slog.F("has_unresolved_local_tool_calls", chainInfo.hasUnresolvedLocalToolCalls),
|
||||
slog.F("provider_missing_tool_results", chainInfo.providerMissingToolResults),
|
||||
slog.F("is_plan_mode_turn", isPlanModeTurn),
|
||||
slog.F("model_config_match", chainInfo.modelConfigID == modelConfig.ID),
|
||||
slog.F("store_enabled", chatprovider.IsResponsesStoreEnabled(providerOptions)),
|
||||
slog.F("contributing_trailing_user_count", chainInfo.contributingTrailingUserCount),
|
||||
)
|
||||
}
|
||||
if chainModeActive {
|
||||
providerOptions = chatprovider.CloneWithPreviousResponseID(
|
||||
providerOptions,
|
||||
|
||||
@@ -3011,6 +3011,7 @@ func TestResolveChainMode_AllowsProviderExecutedOnly(t *testing.T) {
|
||||
|
||||
require.Equal(t, "resp-123", chainInfo.previousResponseID)
|
||||
require.False(t, chainInfo.hasUnresolvedLocalToolCalls)
|
||||
require.False(t, chainInfo.providerMissingToolResults)
|
||||
require.True(t, shouldActivateChainMode(
|
||||
chainModeProviderOptions(),
|
||||
chainInfo,
|
||||
@@ -3072,16 +3073,27 @@ func TestResolveChainMode_AllowsResolvedLocalCall(t *testing.T) {
|
||||
false,
|
||||
)
|
||||
|
||||
// A follow-up assistant after the tool result confirms the
|
||||
// result was sent back to the provider. Chain mode should
|
||||
// activate from the follow-up assistant's response ID.
|
||||
// Use a distinct response ID on the follow-up assistant
|
||||
// so the assertion verifies resolveChainMode selects the
|
||||
// follow-up (last assistant), not the original tool-caller.
|
||||
followUp := chainModeAssistantMessage(modelConfigID, nil)
|
||||
followUp.ProviderResponseID = sql.NullString{String: "resp-follow-up", Valid: true}
|
||||
|
||||
chainInfo := resolveChainMode([]database.ChatMessage{
|
||||
chainModeSystemMessage(),
|
||||
chainModeUserMessage("prior user message"),
|
||||
chainModeAssistantMessage(modelConfigID, []codersdk.ChatMessagePart{toolCall}),
|
||||
chainModeToolMessage([]codersdk.ChatMessagePart{toolResult}),
|
||||
followUp,
|
||||
chainModeUserMessage("latest user message"),
|
||||
})
|
||||
|
||||
require.Equal(t, "resp-123", chainInfo.previousResponseID)
|
||||
require.Equal(t, "resp-follow-up", chainInfo.previousResponseID)
|
||||
require.False(t, chainInfo.hasUnresolvedLocalToolCalls)
|
||||
require.False(t, chainInfo.providerMissingToolResults)
|
||||
require.True(t, shouldActivateChainMode(
|
||||
chainModeProviderOptions(),
|
||||
chainInfo,
|
||||
@@ -3133,6 +3145,108 @@ func TestResolveChainMode_BlocksOnMixedResolvedAndUnresolved(t *testing.T) {
|
||||
))
|
||||
}
|
||||
|
||||
// Tests for providerMissingToolResults detection.
|
||||
// These cover the StopAfterTool + chain mode desync bug where local
|
||||
// tool results exist in the DB but were never sent back to the
|
||||
// provider, leaving an unresolved function_call in the stored chain.
|
||||
|
||||
func TestResolveChainMode_BlocksWhenToolResultNeverSentToProvider(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
modelConfigID := uuid.New()
|
||||
toolCall := codersdk.ChatMessageToolCall(
|
||||
"call-local",
|
||||
"propose_plan",
|
||||
json.RawMessage(`{"path":"plan.md"}`),
|
||||
)
|
||||
toolResult := codersdk.ChatMessageToolResult(
|
||||
"call-local",
|
||||
"propose_plan",
|
||||
json.RawMessage(`{"ok":true}`),
|
||||
false,
|
||||
false,
|
||||
)
|
||||
|
||||
chainInfo := resolveChainMode([]database.ChatMessage{
|
||||
chainModeSystemMessage(),
|
||||
chainModeUserMessage("make a plan"),
|
||||
chainModeAssistantMessage(modelConfigID, []codersdk.ChatMessagePart{toolCall}),
|
||||
chainModeToolMessage([]codersdk.ChatMessagePart{toolResult}),
|
||||
// No follow-up assistant: StopAfterTool fired, tool result
|
||||
// was persisted locally but never sent back to the provider.
|
||||
chainModeUserMessage("implement the plan"),
|
||||
})
|
||||
|
||||
require.Equal(t, "resp-123", chainInfo.previousResponseID)
|
||||
// Local tool calls are resolved (result exists in DB).
|
||||
require.False(t, chainInfo.hasUnresolvedLocalToolCalls)
|
||||
// But the provider never received the result.
|
||||
require.True(t, chainInfo.providerMissingToolResults)
|
||||
// Chain mode must NOT activate.
|
||||
require.False(t, shouldActivateChainMode(
|
||||
chainModeProviderOptions(),
|
||||
chainInfo,
|
||||
modelConfigID,
|
||||
false,
|
||||
))
|
||||
}
|
||||
|
||||
func TestResolveChainMode_BlocksProviderMissingWithMultipleToolCalls(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
modelConfigID := uuid.New()
|
||||
call1 := codersdk.ChatMessageToolCall(
|
||||
"call-1", "propose_plan",
|
||||
json.RawMessage(`{"path":"plan.md"}`),
|
||||
)
|
||||
call2 := codersdk.ChatMessageToolCall(
|
||||
"call-2", "write_file",
|
||||
json.RawMessage(`{"path":"foo.go"}`),
|
||||
)
|
||||
result1 := codersdk.ChatMessageToolResult(
|
||||
"call-1", "propose_plan",
|
||||
json.RawMessage(`{"ok":true}`), false, false,
|
||||
)
|
||||
result2 := codersdk.ChatMessageToolResult(
|
||||
"call-2", "write_file",
|
||||
json.RawMessage(`{"ok":true}`), false, false,
|
||||
)
|
||||
|
||||
chainInfo := resolveChainMode([]database.ChatMessage{
|
||||
chainModeSystemMessage(),
|
||||
chainModeUserMessage("do it"),
|
||||
chainModeAssistantMessage(modelConfigID, []codersdk.ChatMessagePart{call1, call2}),
|
||||
chainModeToolMessage([]codersdk.ChatMessagePart{result1, result2}),
|
||||
chainModeUserMessage("next"),
|
||||
})
|
||||
|
||||
require.False(t, chainInfo.hasUnresolvedLocalToolCalls)
|
||||
require.True(t, chainInfo.providerMissingToolResults)
|
||||
require.False(t, shouldActivateChainMode(
|
||||
chainModeProviderOptions(), chainInfo, modelConfigID, false,
|
||||
))
|
||||
}
|
||||
|
||||
func TestResolveChainMode_AllowsWhenNoToolCalls(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
modelConfigID := uuid.New()
|
||||
|
||||
chainInfo := resolveChainMode([]database.ChatMessage{
|
||||
chainModeSystemMessage(),
|
||||
chainModeUserMessage("hello"),
|
||||
chainModeAssistantMessage(modelConfigID, nil),
|
||||
chainModeUserMessage("thanks"),
|
||||
})
|
||||
|
||||
require.Equal(t, "resp-123", chainInfo.previousResponseID)
|
||||
require.False(t, chainInfo.hasUnresolvedLocalToolCalls)
|
||||
require.False(t, chainInfo.providerMissingToolResults)
|
||||
require.True(t, shouldActivateChainMode(
|
||||
chainModeProviderOptions(), chainInfo, modelConfigID, false,
|
||||
))
|
||||
}
|
||||
|
||||
func chainModeProviderOptions() fantasy.ProviderOptions {
|
||||
store := true
|
||||
return fantasy.ProviderOptions{
|
||||
|
||||
Reference in New Issue
Block a user