fix: prevent duplicate tool results on provider disconnect/fallback#42
Open
alfep wants to merge 1 commit into
Open
fix: prevent duplicate tool results on provider disconnect/fallback#42alfep wants to merge 1 commit into
alfep wants to merge 1 commit into
Conversation
- Add deduplication check in StreamingToolExecutor.addTool() to prevent duplicate tool additions when provider reconnects and resends tool_use blocks - Add deduplication in yieldMissingToolResultBlocks() to prevent duplicate tool_result blocks during model fallback recovery Fixes freecodexyz#37 and freecodexyz#38
There was a problem hiding this comment.
Pull request overview
This PR targets a reliability issue in the tool execution/recovery pipeline: when a provider disconnects/reconnects (or fallback/retry paths run), the same tool_use_id can be observed more than once, leading to duplicate tool executions and/or duplicate tool_result emission.
Changes:
- Add an ID-based dedupe guard in
StreamingToolExecutor.addTool()to avoid re-queueing a tool when the sametool_useblock is replayed. - Add deduplication in
yieldMissingToolResultBlocks()to reduce duplicate “missing tool_result” emission during recovery paths.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/services/tools/StreamingToolExecutor.ts | Skips adding a tool if a tracked tool with the same tool_use_id already exists. |
| src/query.ts | Attempts to dedupe tool-use blocks before emitting synthetic/missing tool_result blocks during fallback/error recovery. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+139
to
143
| for (const assistantMessage of uniqueAssistantMessages) { | ||
| // Extract all tool use blocks from this assistant message | ||
| const toolUseBlocks = assistantMessage.message.content.filter( | ||
| content => content.type === 'tool_use', | ||
| ) as ToolUseBlock[] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes duplicate tool results when provider disconnects mid-response and reconnects, addressing issues #37 and #38.
Changes
StreamingToolExecutor.addTool()to prevent duplicate tool additions when provider reconnects and resends tool_use blocksyieldMissingToolResultBlocks()to prevent duplicate tool_result blocks during model fallback recoveryTest plan
Fixes #37, Fixes #38