Skip to content

Commit 092285f

Browse files
committed
fix: repair malformed JSON in edit_file tool call arguments
LLMs occasionally generate structurally invalid JSON for edit_file arguments when the text payload contains characters that confuse brace counting (YAML, Dockerfiles, shell scripts). The most common pattern is an extra closing brace or bracket near the end of the arguments. Go's json.Unmarshal scanner rejects this before any custom UnmarshalJSON method runs, so the existing double-serialization fix never fires for these cases. Add tryRepairEditFileJSON which iteratively removes the offending character at the json.SyntaxError offset (up to 3 rounds), handling extra }, extra ], stray backslash-n, and stray backslash before quotes. Replace NewHandler with a custom editFileHandler that calls ParseEditFileArgs (which does repair then unmarshal) before delegating to handleEditFile. Update all other call sites (ACP filesystem handler, TUI rendering) to use ParseEditFileArgs as well.
1 parent d6b8269 commit 092285f

File tree

5 files changed

+166
-29
lines changed

5 files changed

+166
-29
lines changed

pkg/acp/filesystem.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -188,8 +188,12 @@ func (t *FilesystemToolset) handleWriteFile(ctx context.Context, toolCall tools.
188188
}
189189

190190
func (t *FilesystemToolset) handleEditFile(ctx context.Context, toolCall tools.ToolCall) (*tools.ToolCallResult, error) {
191-
var args builtin.EditFileArgs
192-
if err := json.Unmarshal([]byte(toolCall.Function.Arguments), &args); err != nil {
191+
data := toolCall.Function.Arguments
192+
if data == "" {
193+
data = "{}"
194+
}
195+
args, err := builtin.ParseEditFileArgs([]byte(data))
196+
if err != nil {
193197
return nil, fmt.Errorf("failed to parse arguments: %w", err)
194198
}
195199

pkg/tools/builtin/filesystem.go

Lines changed: 101 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -166,40 +166,109 @@ type EditFileArgs struct {
166166
Edits []Edit `json:"edits" jsonschema:"Array of edit operations"`
167167
}
168168

169-
// UnmarshalJSON handles LLM-generated arguments where "edits" may be
170-
// a JSON string instead of a JSON array (double-serialized).
171-
func (a *EditFileArgs) UnmarshalJSON(data []byte) error {
169+
// ParseEditFileArgs parses LLM-generated edit_file arguments, handling two
170+
// common failure modes:
171+
// 1. The outer JSON itself is malformed — typically extra closing braces/brackets
172+
// or stray escape sequences caused by the model losing track of nesting depth
173+
// when the text payload contains structural characters (e.g. YAML, Dockerfiles).
174+
// 2. The "edits" field is double-serialized (a JSON string instead of an array).
175+
func ParseEditFileArgs(data []byte) (EditFileArgs, error) {
172176
var raw struct {
173177
Path string `json:"path"`
174178
Edits json.RawMessage `json:"edits"`
175179
}
180+
176181
if err := json.Unmarshal(data, &raw); err != nil {
177-
return fmt.Errorf("failed to parse edit_file arguments: %w", err)
182+
repaired, ok := tryRepairEditFileJSON(data)
183+
if !ok {
184+
return EditFileArgs{}, fmt.Errorf("failed to parse edit_file arguments: %w", err)
185+
}
186+
if err := json.Unmarshal(repaired, &raw); err != nil {
187+
return EditFileArgs{}, fmt.Errorf("failed to parse edit_file arguments after repair: %w", err)
188+
}
189+
slog.Debug("Repaired malformed edit_file JSON arguments")
178190
}
179191

180-
a.Path = raw.Path
192+
args := EditFileArgs{Path: raw.Path}
181193

182194
// When edits is missing or null (e.g. during argument streaming in
183195
// the TUI, or partial tool calls), accept the partial result.
184196
if len(raw.Edits) == 0 || string(raw.Edits) == "null" {
185-
return nil
197+
return args, nil
186198
}
187199

188200
// Try parsing edits as an array first (normal case).
189-
if err := json.Unmarshal(raw.Edits, &a.Edits); err == nil {
190-
return nil
201+
if err := json.Unmarshal(raw.Edits, &args.Edits); err == nil {
202+
return args, nil
191203
}
192204

193205
// Try unwrapping a double-serialized JSON string.
194206
var editsStr string
195207
if err := json.Unmarshal(raw.Edits, &editsStr); err != nil {
196-
return fmt.Errorf("edits field is neither an array nor a JSON string: %w", err)
197-
}
198-
if err := json.Unmarshal([]byte(editsStr), &a.Edits); err != nil {
199-
return fmt.Errorf("failed to parse double-serialized edits string: %w", err)
208+
return EditFileArgs{}, fmt.Errorf("edits field is neither an array nor a JSON string: %w", err)
209+
}
210+
if err := json.Unmarshal([]byte(editsStr), &args.Edits); err != nil {
211+
return EditFileArgs{}, fmt.Errorf("failed to parse double-serialized edits string: %w", err)
212+
}
213+
214+
return args, nil
215+
}
216+
217+
// tryRepairEditFileJSON attempts to fix common LLM JSON malformations by
218+
// iteratively removing the offending character(s) at each json.SyntaxError
219+
// offset. Observed failure modes from production sessions:
220+
//
221+
// - Extra '}' — model loses brace count (e.g. "}}]}" instead of "}]}")
222+
// - Extra ']' — model adds a spurious array wrapper
223+
// - Stray '\' — model emits an escape sequence outside of a string value
224+
// (e.g. literal \n between tokens, or \" where " is expected)
225+
func tryRepairEditFileJSON(data []byte) ([]byte, bool) {
226+
current := append([]byte(nil), data...) // defensive copy
227+
for range 3 {
228+
var synErr *json.SyntaxError
229+
if err := json.Unmarshal(current, &json.RawMessage{}); err == nil {
230+
return current, true
231+
} else if !errors.As(err, &synErr) {
232+
return nil, false
233+
}
234+
235+
// json.SyntaxError.Offset is 1-based.
236+
offset := int(synErr.Offset) - 1
237+
if offset < 0 || offset >= len(current) {
238+
return nil, false
239+
}
240+
241+
ch := current[offset]
242+
removeCount := 1
243+
244+
switch ch {
245+
case '}', ']':
246+
// Extra closing delimiter — just remove it.
247+
case '\\':
248+
// Stray escape sequence outside a string value. For \n, \t, \r
249+
// both characters are garbage so remove them. For \" the quote
250+
// is a valid structural character (string delimiter), so only
251+
// strip the backslash.
252+
if offset+1 < len(current) {
253+
switch current[offset+1] {
254+
case 'n', 't', 'r':
255+
removeCount = 2
256+
}
257+
}
258+
default:
259+
return nil, false
260+
}
261+
262+
repaired := make([]byte, 0, len(current)-removeCount)
263+
repaired = append(repaired, current[:offset]...)
264+
repaired = append(repaired, current[offset+removeCount:]...)
265+
current = repaired
200266
}
201267

202-
return nil
268+
if json.Valid(current) {
269+
return current, true
270+
}
271+
return nil, false
203272
}
204273

205274
func (t *FilesystemTool) Tools(context.Context) ([]tools.Tool, error) {
@@ -245,7 +314,7 @@ func (t *FilesystemTool) Tools(context.Context) ([]tools.Tool, error) {
245314
Description: "Make line-based edits to a text file. Each edit replaces exact line sequences with new content.",
246315
Parameters: tools.MustSchemaFor[EditFileArgs](),
247316
OutputSchema: tools.MustSchemaFor[string](),
248-
Handler: tools.NewHandler(t.handleEditFile),
317+
Handler: t.editFileHandler(),
249318
Annotations: tools.ToolAnnotations{
250319
Title: "Edit",
251320
},
@@ -466,6 +535,24 @@ func countTreeNodes(node *fsx.TreeNode) (files, dirs int) {
466535
return files, dirs
467536
}
468537

538+
// editFileHandler returns a ToolHandler that parses edit_file arguments with
539+
// repair logic for malformed JSON, then delegates to handleEditFile.
540+
// This bypasses tools.NewHandler because Go's json.Unmarshal scanner rejects
541+
// structurally invalid JSON before calling any custom UnmarshalJSON method.
542+
func (t *FilesystemTool) editFileHandler() tools.ToolHandler {
543+
return func(ctx context.Context, toolCall tools.ToolCall) (*tools.ToolCallResult, error) {
544+
data := toolCall.Function.Arguments
545+
if data == "" {
546+
data = "{}"
547+
}
548+
args, err := ParseEditFileArgs([]byte(data))
549+
if err != nil {
550+
return nil, err
551+
}
552+
return t.handleEditFile(ctx, args)
553+
}
554+
}
555+
469556
func (t *FilesystemTool) handleEditFile(ctx context.Context, args EditFileArgs) (*tools.ToolCallResult, error) {
470557
resolvedPath := t.resolvePath(args.Path)
471558

pkg/tools/builtin/filesystem_test.go

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ func TestFilesystemTool_EditFile(t *testing.T) {
260260
assert.Contains(t, result.Output, "old text not found")
261261
}
262262

263-
func TestEditFileArgs_UnmarshalJSON(t *testing.T) {
263+
func TestParseEditFileArgs(t *testing.T) {
264264
t.Parallel()
265265

266266
tests := []struct {
@@ -338,13 +338,61 @@ func TestEditFileArgs_UnmarshalJSON(t *testing.T) {
338338
{OldText: "a", NewText: "b"},
339339
},
340340
},
341+
342+
// Malformed outer JSON — LLM brace/bracket counting errors.
343+
{
344+
name: "repair: extra closing brace before array close",
345+
input: `{"path": "ci.yml", "edits": [{"oldText": "old", "newText": "new"}}]}`,
346+
wantPath: "ci.yml",
347+
wantEdits: []Edit{
348+
{OldText: "old", NewText: "new"},
349+
},
350+
},
351+
{
352+
name: "repair: extra closing brace with trailing newline",
353+
input: "{\"path\": \"ci.yml\", \"edits\": [{\"oldText\": \"old\", \"newText\": \"new\"}}]\n}",
354+
wantPath: "ci.yml",
355+
wantEdits: []Edit{
356+
{OldText: "old", NewText: "new"},
357+
},
358+
},
359+
{
360+
name: "repair: extra closing bracket (spurious array wrapper)",
361+
input: `{"path": "build.sh", "edits": [{"oldText": "a", "newText": "b"}]]}`,
362+
wantPath: "build.sh",
363+
wantEdits: []Edit{
364+
{OldText: "a", NewText: "b"},
365+
},
366+
},
367+
{
368+
name: "repair: stray backslash-n between tokens",
369+
input: "{\"path\": \"Dockerfile\", \"edits\": [{\"oldText\": \"a\", \"newText\": \"b\"}\\n]}",
370+
wantPath: "Dockerfile",
371+
wantEdits: []Edit{
372+
{OldText: "a", NewText: "b"},
373+
},
374+
},
375+
{
376+
name: "repair: stray backslash before property name",
377+
input: `{"path": "f.go", "edits": [{"oldText": "a", "newText": "b"},{\"oldText": "c", "newText": "d"}]}`,
378+
wantPath: "f.go",
379+
wantEdits: []Edit{
380+
{OldText: "a", NewText: "b"},
381+
{OldText: "c", NewText: "d"},
382+
},
383+
},
384+
{
385+
name: "unrepairable garbage",
386+
input: `{totally broken <<<>>>`,
387+
wantErr: true,
388+
wantErrMsg: "failed to parse edit_file arguments",
389+
},
341390
}
342391

343392
for _, tc := range tests {
344393
t.Run(tc.name, func(t *testing.T) {
345394
t.Parallel()
346-
var args EditFileArgs
347-
err := json.Unmarshal([]byte(tc.input), &args)
395+
args, err := ParseEditFileArgs([]byte(tc.input))
348396
if tc.wantErr {
349397
require.Error(t, err)
350398
assert.Contains(t, err.Error(), tc.wantErrMsg)

pkg/tui/components/tool/editfile/editfile.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package editfile
22

33
import (
4-
"encoding/json"
54
"fmt"
65

76
"github.com/docker/docker-agent/pkg/tools/builtin"
@@ -32,8 +31,8 @@ func render(
3231
_ int,
3332
) string {
3433
// Parse tool arguments to extract the file path for display.
35-
var args builtin.EditFileArgs
36-
if err := json.Unmarshal([]byte(msg.ToolCall.Function.Arguments), &args); err != nil {
34+
args, err := builtin.ParseEditFileArgs([]byte(msg.ToolCall.Function.Arguments))
35+
if err != nil {
3736
// If arguments cannot be parsed, fail silently to avoid breaking the TUI.
3837
return ""
3938
}
@@ -111,8 +110,8 @@ func renderCollapsed(
111110
width,
112111
_ int,
113112
) string {
114-
var args builtin.EditFileArgs
115-
if err := json.Unmarshal([]byte(msg.ToolCall.Function.Arguments), &args); err != nil {
113+
args, err := builtin.ParseEditFileArgs([]byte(msg.ToolCall.Function.Arguments))
114+
if err != nil {
116115
return ""
117116
}
118117

pkg/tui/components/tool/editfile/render.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package editfile
22

33
import (
4-
"encoding/json"
54
"fmt"
65
"os"
76
"path/filepath"
@@ -118,8 +117,8 @@ func renderEditFile(toolCall tools.ToolCall, width int, splitView bool, toolStat
118117
}
119118

120119
func renderEditFileUncached(toolCall tools.ToolCall, width int, splitView bool, toolStatus types.ToolStatus) string {
121-
var args builtin.EditFileArgs
122-
if err := json.Unmarshal([]byte(toolCall.Function.Arguments), &args); err != nil {
120+
args, err := builtin.ParseEditFileArgs([]byte(toolCall.Function.Arguments))
121+
if err != nil {
123122
return ""
124123
}
125124

@@ -169,8 +168,8 @@ func countDiffLines(toolCall tools.ToolCall, _ types.ToolStatus) (added, removed
169168
}
170169

171170
func countDiffLinesUncached(toolCall tools.ToolCall) (added, removed int) {
172-
var args builtin.EditFileArgs
173-
if err := json.Unmarshal([]byte(toolCall.Function.Arguments), &args); err != nil {
171+
args, err := builtin.ParseEditFileArgs([]byte(toolCall.Function.Arguments))
172+
if err != nil {
174173
return 0, 0
175174
}
176175

0 commit comments

Comments
 (0)