Repository navigation
Fix/broken tool call arguments - #700
Merged
Merged
Conversation
A tool call whose arguments are not valid JSON was handled differently by each API, and never well. openai-chat dropped the call and sent a normal finish, so the turn ended as a clean stop: clients kept showing the call as being prepared, and a subagent returned an old assistant text as its result. This happened when a model closed a `compact_chat` call with `</summary>` and no closing quote. Anthropic, OpenAI Responses and Bedrock did not guard the parse: the Jackson exception is an IOException, so it was reported as "Connection error", classified as overloaded and auto-continued with a generic nudge, and the valid calls of the same response did not run. While compacting, that recovery is blocked, so the compaction failed on every API. Parse the arguments in one place, `llm-util/parse-tool-call-arguments`, which never throws and marks an invalid call with `:arguments-error` (the parser message and the end of the text). The chat rejects such a call through the existing rejection path, without approval or hooks, with the error as its result, and the tool loop goes on as after a hook rejection. The LLM can send the call again, so a broken `compact_chat` recovers and the task resumes. Valid calls in the same response run, and a rejection by the user or the config still decides how the turn goes on. openai-chat stopped continuing on such calls in 5c18fe0, as resending the same request looped. Now the history gains the error, and after 3 responses in a row with invalid arguments the turn ends with a prompt error, so a subagent reports a failure instead of a result. The result gives no advice to shorten or split the call: the model sees how its text ended, and of a split `compact_chat` call only the last part would count.
The protocol lists the `reason` of `toolCallRejected` as `userChoiceDeny`, `userConfigDeny`, `hookRejected`, `userPromptStop`, `userStop`, `user` or `interrupted`. The server sent `user` for every call denied by the user, the config or a hook, and for every stopped running call: since 58fe983 the action reads `(:code (:reason event-data) :user)`, but `:send-reject` and `:stop-attempted` passed a bare code, not a decision reason map. The codes that did get through went out in kebab-case, e.g. `user-prompt-stop`, as only map keys are converted on the way out. Pass a decision reason map from those two as well, and send its code in the protocol's camelCase. This also sends two codes the protocol did not list, so document them: `hookStopped`, for a preToolCall hook stopping the turn, and `invalidArguments`, for the calls with invalid JSON arguments rejected since the previous commit. No client compares the value: of the clients in the organization only eca-omarchy reads it, to show it as text.
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.
A model sometimes sends a tool call with broken JSON arguments. ECA then either ended the turn silently or reported a fake connection error, which made a subagent's failed compaction look like a finished result.
This branch returns the parse error to the model as the call's result on every provider, so it can send the call again. It also makes the server send clients the real reason a tool call was rejected.