Fix broken multi-turn tool recursion and Anthropic tool result grouping#98
Fix broken multi-turn tool recursion and Anthropic tool result grouping#98Godzilla675 merged 3 commits intomainfrom
Conversation
…on, group tool results for Anthropic compatibility Co-authored-by: Godzilla675 <131464726+Godzilla675@users.noreply.github.com>
Co-authored-by: Godzilla675 <131464726+Godzilla675@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes broken multi-turn tool recursion by wiring the WebSocket chat flow to the existing runAgentLoop, and updates the REST /chat loop to group tool results into a single user message (required by Anthropic) while ensuring the tool executor is passed on subsequent iterations.
Changes:
- WebSocket: replaced the inline one-turn tool handling with
runAgentLoopto support multi-turn tool recursion and structured tool messaging. - REST: groups parallel tool results into a single
toolResultsmessage and passesexecuteToolto all loop iterations. - Added standalone backend test scripts covering WebSocket multi-turn recursion and REST tool-result grouping.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| apps/backend/src/websocket/handler.ts | Swaps in runAgentLoop for WebSocket multi-turn tool recursion. |
| apps/backend/src/routes/copilot.ts | Groups tool results into a single message and preserves executeTool across iterations. |
| apps/backend/src/test/websocket-tool-recursion-test.ts | Adds a script test validating 3-turn recursion with 2 tool executions over WebSocket. |
| apps/backend/src/test/rest-tool-grouping-test.ts | Adds a script test validating tool results are grouped into a single message for REST chat. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Use the agent loop which properly handles multi-turn tool recursion, | ||
| // structured messages, and parallel tool calls | ||
| await this.runAgentLoop(ws, fullMessages, allTools as any, wsWithHistory, systemPrompt, 0, executeToolWithNotifications, model); |
There was a problem hiding this comment.
Switching the WebSocket path to runAgentLoop changes tool-result history from a textual user message to a structured toolResults message with content: ''. That breaks multi-turn tool flows for providers that ignore Message.toolResults (notably OpenAIProvider/CustomProvider, which only forward m.content to the API), because the model never sees tool outputs on the next turn and may loop/repeat tool calls. Consider either (a) adding an OpenAI message formatter that maps toolResults to the appropriate OpenAI tool/role messages, or (b) including a textual fallback summary of tool results in content when the active provider doesn't support structured toolResults.
| // Add all tool results as a single user message (content is empty | ||
| // because providers use the structured toolResults, not the text field) | ||
| messages.push({ | ||
| role: 'user', | ||
| content: '', |
There was a problem hiding this comment.
This tool-results message is now pushed with content: '' and only structured toolResults. That’s required for Anthropic, but it will break providers that don't serialize toolResults into their API payloads (e.g. OpenAIProvider currently only sends Message.content). To avoid losing tool outputs (and getting stuck in repeated tool calls / hitting MAX_ITERATIONS), either add provider-specific formatting for toolResults (OpenAI tool messages) or include a text fallback summary in content alongside the structured toolResults.
| // Add all tool results as a single user message (content is empty | |
| // because providers use the structured toolResults, not the text field) | |
| messages.push({ | |
| role: 'user', | |
| content: '', | |
| // Add all tool results as a single user message. | |
| // Keep structured toolResults for providers that support them, and | |
| // also include a text fallback in `content` for providers that only | |
| // use the content field (e.g. some OpenAI-based providers). | |
| messages.push({ | |
| role: 'user', | |
| content: toolResults | |
| .map(tr => { | |
| const resultText = | |
| typeof tr.result === 'string' | |
| ? tr.result | |
| : JSON.stringify(tr.result); | |
| return `Tool "${tr.toolName}" (callId: ${tr.toolCallId}) returned${tr.isError ? ' error' : ''}: ${resultText}`; | |
| }) | |
| .join('\n'), |
| import { LLMOrchestrator } from '../llm/orchestrator.js'; | ||
| import { ProjectManager } from '../project/state.js'; | ||
| import { LLMConfig, Message } from '@ai-video-editor/shared-types'; | ||
| import { LLMProviderInterface, MCPTool, StreamChunk, ToolCall, ToolExecutor, LLMProviderOptions } from '../llm/types.js'; |
There was a problem hiding this comment.
Unused imports LLMProviderOptions, MCPTool, ToolExecutor.
| import { LLMProviderInterface, MCPTool, StreamChunk, ToolCall, ToolExecutor, LLMProviderOptions } from '../llm/types.js'; | |
| import { LLMProviderInterface, StreamChunk, ToolCall } from '../llm/types.js'; |
| import { ProjectManager } from '../project/state.js'; | ||
| import { LLMOrchestrator } from '../llm/orchestrator.js'; | ||
| import { LLMConfig, Message } from '@ai-video-editor/shared-types'; | ||
| import { LLMProviderInterface, MCPTool, StreamChunk, ToolCall, ToolExecutor, LLMProviderOptions } from '../llm/types.js'; |
There was a problem hiding this comment.
Unused imports LLMProviderOptions, MCPTool, ToolExecutor.
| import { LLMProviderInterface, MCPTool, StreamChunk, ToolCall, ToolExecutor, LLMProviderOptions } from '../llm/types.js'; | |
| import { LLMProviderInterface, StreamChunk, ToolCall } from '../llm/types.js'; |
| const orchestrator = new LLMOrchestrator(mockConfig); | ||
| (orchestrator as any).provider = new MockToolProvider(); | ||
|
|
||
| const wsHandler = new WebSocketHandler(wss, projectManager, orchestrator); |
There was a problem hiding this comment.
Unused variable wsHandler.
| const wsHandler = new WebSocketHandler(wss, projectManager, orchestrator); | |
| new WebSocketHandler(wss, projectManager, orchestrator); |
handleCopilotMessagehad a hand-rolled agent loop that only handled one round of tool calls. After executing a tool and re-callingstreamChat, it processed content chunks but silently dropped any subsequenttoolCallchunks — breaking multi-step workflows (e.g., search → download → add to timeline). A properrunAgentLoopwith depth-limited recursion already existed but was never wired in.WebSocket handler (
handler.ts)runAgentLoopwhich:toolCalls/toolResultsonMessageinstead of flat stringsREST API (
routes/copilot.ts)executeToolto all loop iterations (wasundefinedafter first call)Tests
websocket-tool-recursion-test.ts— validates 3-turn tool recursion across 2 tool executionsrest-tool-grouping-test.ts— validates parallel tool results land in a single message💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.