mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(chatloop): keep provider-executed tool results in assistant message (#23012)
## Problem When a step contains both provider-executed tool calls (e.g. Anthropic web search) and local tool calls in parallel, the next loop iteration fails with the Anthropic API claiming the regular tool call has no result. However, sending a new user message (which reloads messages from the DB) works fine. ## Root cause `toResponseMessages` was placing **all** tool results into the tool-role message, regardless of `ProviderExecuted`. When Fantasy's Anthropic provider later converted these messages for the API, it moved the provider tool result from the tool message to the **end** of the previous assistant message (`prevMsg.Content = append(...)`). This placed `web_search_tool_result` **after** the regular `tool_use` block: ``` assistant: [server_tool_use(A), tool_use(B), web_search_tool_result(A)] ← wrong order user: [tool_result(B)] ``` The persistence layer in `chatd.go` already handles this correctly — provider-executed tool results stay in the assistant message, producing the expected ordering: ``` assistant: [server_tool_use(A), web_search_tool_result(A), tool_use(B)] ← correct order user: [tool_result(B)] ``` This is why reloading from the DB fixed it. ## Fix In the `ContentTypeToolResult` case of `toResponseMessages`, route provider-executed results to `assistantParts` instead of `toolParts`, matching the persistence layer's behavior. ## Testing Added `TestToResponseMessages_ProviderExecutedToolResultInAssistantMessage` which verifies that mixed provider+local tool results are split correctly between the assistant and tool messages.
This commit is contained in:
@@ -158,12 +158,23 @@ func (r stepResult) toResponseMessages() []fantasy.Message {
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
toolParts = append(toolParts, fantasy.ToolResultPart{
|
||||
part := fantasy.ToolResultPart{
|
||||
ToolCallID: result.ToolCallID,
|
||||
Output: result.Result,
|
||||
ProviderExecuted: result.ProviderExecuted,
|
||||
ProviderOptions: fantasy.ProviderOptions(result.ProviderMetadata),
|
||||
})
|
||||
}
|
||||
// Provider-executed tool results (e.g. web_search)
|
||||
// must stay in the assistant message so the result
|
||||
// block appears inline after the corresponding
|
||||
// server_tool_use block. This matches the persistence
|
||||
// layer in chatd.go which keeps them in
|
||||
// assistantBlocks.
|
||||
if result.ProviderExecuted {
|
||||
assistantParts = append(assistantParts, part)
|
||||
} else {
|
||||
toolParts = append(toolParts, part)
|
||||
}
|
||||
default:
|
||||
continue
|
||||
}
|
||||
|
||||
@@ -499,6 +499,82 @@ func TestRun_ShutdownDuringToolExecutionReturnsContextCanceled(t *testing.T) {
|
||||
assert.ErrorIs(t, err, context.Canceled, "shutdown should propagate as context.Canceled")
|
||||
}
|
||||
|
||||
func TestToResponseMessages_ProviderExecutedToolResultInAssistantMessage(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
sr := stepResult{
|
||||
content: []fantasy.Content{
|
||||
// Provider-executed tool call (e.g. web_search).
|
||||
fantasy.ToolCallContent{
|
||||
ToolCallID: "provider-tc-1",
|
||||
ToolName: "web_search",
|
||||
Input: `{"query":"coder"}`,
|
||||
ProviderExecuted: true,
|
||||
},
|
||||
// Provider-executed tool result — must stay in
|
||||
// assistant message.
|
||||
fantasy.ToolResultContent{
|
||||
ToolCallID: "provider-tc-1",
|
||||
ToolName: "web_search",
|
||||
ProviderExecuted: true,
|
||||
ProviderMetadata: fantasy.ProviderMetadata{"anthropic": nil},
|
||||
},
|
||||
// Local tool call (e.g. read_file).
|
||||
fantasy.ToolCallContent{
|
||||
ToolCallID: "local-tc-1",
|
||||
ToolName: "read_file",
|
||||
Input: `{"path":"main.go"}`,
|
||||
ProviderExecuted: false,
|
||||
},
|
||||
// Local tool result — should go into tool message.
|
||||
fantasy.ToolResultContent{
|
||||
ToolCallID: "local-tc-1",
|
||||
ToolName: "read_file",
|
||||
Result: fantasy.ToolResultOutputContentText{Text: "some result"},
|
||||
ProviderExecuted: false,
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
msgs := sr.toResponseMessages()
|
||||
require.Len(t, msgs, 2, "expected assistant + tool messages")
|
||||
|
||||
// First message: assistant role.
|
||||
assistantMsg := msgs[0]
|
||||
assert.Equal(t, fantasy.MessageRoleAssistant, assistantMsg.Role)
|
||||
require.Len(t, assistantMsg.Content, 3,
|
||||
"assistant message should have provider ToolCallPart, provider ToolResultPart, and local ToolCallPart")
|
||||
|
||||
// Part 0: provider tool call.
|
||||
providerTC, ok := fantasy.AsMessagePart[fantasy.ToolCallPart](assistantMsg.Content[0])
|
||||
require.True(t, ok, "part 0 should be ToolCallPart")
|
||||
assert.Equal(t, "provider-tc-1", providerTC.ToolCallID)
|
||||
assert.True(t, providerTC.ProviderExecuted)
|
||||
|
||||
// Part 1: provider tool result (inline in assistant turn).
|
||||
providerTR, ok := fantasy.AsMessagePart[fantasy.ToolResultPart](assistantMsg.Content[1])
|
||||
require.True(t, ok, "part 1 should be ToolResultPart")
|
||||
assert.Equal(t, "provider-tc-1", providerTR.ToolCallID)
|
||||
assert.True(t, providerTR.ProviderExecuted)
|
||||
|
||||
// Part 2: local tool call.
|
||||
localTC, ok := fantasy.AsMessagePart[fantasy.ToolCallPart](assistantMsg.Content[2])
|
||||
require.True(t, ok, "part 2 should be ToolCallPart")
|
||||
assert.Equal(t, "local-tc-1", localTC.ToolCallID)
|
||||
assert.False(t, localTC.ProviderExecuted)
|
||||
|
||||
// Second message: tool role.
|
||||
toolMsg := msgs[1]
|
||||
assert.Equal(t, fantasy.MessageRoleTool, toolMsg.Role)
|
||||
require.Len(t, toolMsg.Content, 1,
|
||||
"tool message should have only the local ToolResultPart")
|
||||
|
||||
localTR, ok := fantasy.AsMessagePart[fantasy.ToolResultPart](toolMsg.Content[0])
|
||||
require.True(t, ok, "tool part should be ToolResultPart")
|
||||
assert.Equal(t, "local-tc-1", localTR.ToolCallID)
|
||||
assert.False(t, localTR.ProviderExecuted)
|
||||
}
|
||||
|
||||
func hasAnthropicEphemeralCacheControl(message fantasy.Message) bool {
|
||||
if len(message.ProviderOptions) == 0 {
|
||||
return false
|
||||
|
||||
Reference in New Issue
Block a user