fix(session): sanitize malformed tool names in pending parts#29156
Open
divitkashyap wants to merge 1 commit into
Open
fix(session): sanitize malformed tool names in pending parts#29156divitkashyap wants to merge 1 commit into
divitkashyap wants to merge 1 commit into
Conversation
Closes anomalyco#28989 `tool-input-start` writes a tool part to SQLite with the raw stream name before `experimental_repairToolCall` runs, so when the model emits a malformed name (e.g. `functions.bash<|tool_call|>`) it lands in the DB with status `pending`. On every subsequent turn `toModelMessages` re-serializes that part as `tool-${part.tool}`; Bedrock rejects the name with a non-retryable ValidationException and the session dies permanently. Sanitize at the serialization boundary: if a pending/running part has a name that fails `/^[a-zA-Z0-9_-]+$/`, emit a text hint instead of a dangling `tool_use`. For completed/error parts (where the bad name shouldn't normally occur) fall back to the `invalid` sentinel. Fixing on the serialization side also self-heals existing poisoned sessions in users' DBs on the next launch.
Author
|
@thdxr if you have a sec — this self-heals existing broken sessions on the next turn (no manual SQLite surgery needed for users already hit by it). |
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
1 participant
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.
Issue for this PR
Closes #28989
Type of change
What does this PR do?
There's a race in tool-call ingestion:
tool-input-start(processor.tsensureToolCall) writes the part to SQLite withtool: input.nameandstatus: "pending"beforeexperimental_repairToolCallruns. When the model emits a malformed name likefunctions.bash<|tool_call|>, the bad name persists. On every following turn,toModelMessagesre-serializes it astool-${part.tool}, Bedrock rejects the name with a non-retryableValidationException, and the session is dead forever.Fix is at the serialization boundary in
message-v2.ts: if a pending/running part's name fails/^[a-zA-Z0-9_-]+$/, emit atexthint instead of a danglingtool_useblock. For completed/error parts (where this shouldn't normally happen) fall back to theinvalidsentinel so the call still pairs with a result.I went with the serialization-side fix over plumbing the repair callback into the processor or deferring the DB write, because the bad name is already sitting in many users' DBs — sanitizing on read means those sessions self-heal on the next turn instead of staying bricked until the user manually edits SQLite.
How did you verify your code works?
test("sanitizes malformed pending tool names to avoid Bedrock ValidationException")inmessage-v2.test.tsusing the exact malformed name from the issue. Confirmed it fails ondevand passes with the fix.bunx tsc --noEmitinpackages/opencodeis clean.message-v2.test.tssuite: 37/37 pass.Checklist