fix: handle double-serialized edits argument in edit_file tool#2144
Merged
dgageot merged 5 commits intodocker:mainfrom Mar 18, 2026
Merged
fix: handle double-serialized edits argument in edit_file tool#2144dgageot merged 5 commits intodocker:mainfrom
dgageot merged 5 commits intodocker:mainfrom
Conversation
added 2 commits
March 17, 2026 12:22
LLMs sometimes send the edits parameter as a JSON string instead of
a JSON array (e.g. "[{...}]" rather than [{...}]). Add a custom
UnmarshalJSON on EditFileArgs that detects this and transparently
double-deserializes the string into the expected []Edit slice.
This also fixes the ACP filesystem handler since it reuses the
builtin.EditFileArgs type.
Add a check that the path field is non-empty when the UnmarshalJSON fallback path is taken, so that a missing path produces a clear error instead of a confusing downstream failure.
rumpl
reviewed
Mar 17, 2026
![docker-agent[bot]](https://avatars.githubusercontent.com/in/2787757?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
added 2 commits
March 17, 2026 13:15
Remove the type alias approach (standard unmarshal + fallback) and always parse through RawMessage. This simplifies the code and ensures path validation happens consistently for both normal and double-serialized edits. Add test cases for missing/empty path with normal array edits.
The TUI renders tool calls while arguments are still streaming, so it may unmarshal EditFileArgs with only a path and no edits field. Accept nil/missing edits gracefully instead of returning an error. Also remove the strict path-is-required validation from UnmarshalJSON since the handler already produces a clear error when the path is empty. This keeps UnmarshalJSON focused on parsing concerns.
rumpl
reviewed
Mar 17, 2026
Check that each error case returns the expected error message, not just that an error occurred.
dgageot
approved these changes
Mar 18, 2026
trungutt
added a commit
that referenced
this pull request
May 6, 2026
Open-weights models (DeepSeek, Qwen, GLM) repeat the same small set of shape mistakes when calling tools whose schema includes an array field: sending a JSON-stringified array, a bare scalar, or a single-key object placeholder where the schema expects an array. Today every tool surface that doesn't have a custom UnmarshalJSON fails strictly on these and the model wastes a turn retrying with the same mistake. This generalises the validate-then-repair design we already use for edit_file (PRs #2452 and #2144) to every tool that goes through tools.NewHandler. Strict json.Unmarshal still runs first; only when it fails does the repair layer walk the destination type's fields and attempt four narrow fixes at the exact paths the schema disagreed at. Successful repairs emit a tool_input_repaired log entry tagged with the tool name and the repairs applied so per-(model, tool) repair rates can be tracked.
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
3 participants
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
LLMs sometimes send the
editsparameter ofedit_fileas a JSON string ("[{...}]") instead of a JSON array ([{...}]). This adds a customUnmarshalJSONonEditFileArgsthat detects this double-serialization and transparently unwraps it.Since the ACP filesystem handler reuses
builtin.EditFileArgs, this fix applies to both code paths.Also validates that the
pathfield is non-empty in the fallback branch, so a missing path produces a clear"path field is required"error instead of a confusing downstream failure.