fix: allow partial args through parseToolCall finalization to enable proper retry behavior#12080
Draft
roomote-v0[bot] wants to merge 1 commit intomainfrom
Draft
fix: allow partial args through parseToolCall finalization to enable proper retry behavior#12080roomote-v0[bot] wants to merge 1 commit intomainfrom
roomote-v0[bot] wants to merge 1 commit intomainfrom
Conversation
…to_file and other tools When the model response gets truncated (e.g., due to output token limits), required parameters like `content` for write_to_file arrive as `undefined`. The finalization path in parseToolCall() required ALL params via AND conditions, causing nativeArgs to stay undefined and the tool call to be silently dropped or throw a generic error -- bypassing the tool's own user-friendly retry logic. The streaming/partial path already uses OR conditions to allow partial args through. This change aligns the finalization path with the same pattern, so that partially-constructed args flow through to the tool's execute() method, which can produce specific "missing parameter X, retrying..." messages that get sent back to the model. Fixes #12079
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.
Related GitHub Issue
Closes: #12079
Description
This PR attempts to address Issue #12079 where
write_to_fileintermittently fails because the model's response gets truncated (e.g., due to output token limits), causing the requiredcontentparameter to arrive asundefined.Root cause: In
NativeToolCallParser.parseToolCall(), the finalization path forwrite_to_file(and other multi-required-param tools) used AND conditions requiring ALL parameters to be present before settingnativeArgs. When any required param was missing,nativeArgsstayedundefined, causing either a silent drop (null return) or a generic error -- bypassing the tool's own user-friendly retry logic inWriteToFileTool.execute().Fix: Changed the AND conditions to OR conditions in the finalization path, matching the pattern already used in the streaming/partial path. This allows partially-constructed args to flow through to the tool's
execute()method, which can produce specific "Roo tried to use write_to_file without value for required parameter 'content'. Retrying..." messages that get sent back to the model for self-correction.Affected tools:
write_to_file,apply_diff,edit/search_and_replace,search_replace,edit_file,ask_followup_question,generate_image,search_files,switch_mode,use_mcp_tool,access_mcp_resource,new_task.Feedback and guidance are welcome.
Test Procedure
NativeToolCallParser.spec.tsforwrite_to_filewith missingcontent, missingpath, and both presentapply_diffwith missingdiffcd src && npx vitest run core/assistant-message/__tests__/NativeToolCallParser.spec.tsPre-Submission Checklist
Documentation Updates
Additional Notes
The streaming/partial path in the same file already uses OR conditions (e.g., line 468:
if (partialArgs.path || partialArgs.content)). This change aligns the finalization path with that existing pattern. The fix is defensive -- each tool'sexecute()method already has per-parameter validation that produces actionable retry messages, but that validation was unreachable whennativeArgswas never constructed.Interactively review PR in Roo Code Cloud