diff --git a/internal/core/edit.go b/internal/core/edit.go index 558b1279..ec3f854b 100644 --- a/internal/core/edit.go +++ b/internal/core/edit.go @@ -21,12 +21,9 @@ type Edit struct { } // editArgs holds the arguments for the edit tool. -// Supports both single-edit mode (old_text/new_text) and multi-edit mode (edits array). type editArgs struct { - Path string `json:"path"` - OldText string `json:"old_text"` // Single-edit mode - NewText string `json:"new_text"` // Single-edit mode - Edits []Edit `json:"edits"` // Multi-edit mode + Path string `json:"path"` + Edits []Edit `json:"edits"` } // replacement represents a normalized edit ready for processing. @@ -52,20 +49,12 @@ func NewEditTool(opts ...ToolOption) fantasy.AgentTool { return &coreTool{ info: fantasy.ToolInfo{ Name: "edit", - Description: "Edit a file by replacing exact text. Supports single edit via old_text/new_text, or multiple edits via the edits array. All edits in the array are matched against the original file content (non-incremental) and must be non-overlapping.", + Description: "Edit a file by replacing exact text. All edits in the array are matched against the original file content (non-incremental) and must be non-overlapping.", Parameters: map[string]any{ "path": map[string]any{ "type": "string", "description": "Path to the file to edit (relative or absolute)", }, - "old_text": map[string]any{ - "type": "string", - "description": "Exact text to find and replace (single-edit mode). Must not be used with 'edits' array.", - }, - "new_text": map[string]any{ - "type": "string", - "description": "New text to replace the old text with (single-edit mode). Must not be used with 'edits' array.", - }, "edits": map[string]any{ "type": "array", "description": "Array of edits for multi-region replacement. Each edit must have unique, non-overlapping old_text. All matches are against the original file content.", @@ -85,7 +74,7 @@ func NewEditTool(opts ...ToolOption) fantasy.AgentTool { }, }, }, - Required: []string{"path"}, + Required: []string{"path", "edits"}, }, handler: func(ctx context.Context, call fantasy.ToolCall) (fantasy.ToolResponse, error) { return executeEdit(ctx, call, cfg.WorkDir) @@ -163,36 +152,11 @@ func executeEdit(ctx context.Context, call fantasy.ToolCall, workDir string) (fa } // normalizeEditInput validates and normalizes the edit input. -// Returns error if both single-edit and multi-edit modes are used. func normalizeEditInput(args editArgs) ([]replacement, error) { - singleMode := args.OldText != "" || args.NewText != "" - multiMode := len(args.Edits) > 0 - - if singleMode && multiMode { - return nil, fmt.Errorf("cannot use old_text/new_text together with edits array") + if len(args.Edits) == 0 { + return nil, fmt.Errorf("edits array is required and must not be empty") } - if !singleMode && !multiMode { - return nil, fmt.Errorf("must provide either old_text/new_text or edits array") - } - - if singleMode { - if args.OldText == "" { - return nil, fmt.Errorf("old_text is required when using single-edit mode") - } - if args.NewText == "" { - return nil, fmt.Errorf("new_text is required when using single-edit mode") - } - return []replacement{{ - oldText: strings.ReplaceAll(args.OldText, "\r\n", "\n"), - newText: strings.ReplaceAll(args.NewText, "\r\n", "\n"), - originalOld: args.OldText, - originalNew: args.NewText, - index: 0, - }}, nil - } - - // Multi-edit mode var reps []replacement for i, edit := range args.Edits { if edit.OldText == "" { diff --git a/internal/core/edit_test.go b/internal/core/edit_test.go index 1a0233f7..4e1268eb 100644 --- a/internal/core/edit_test.go +++ b/internal/core/edit_test.go @@ -389,9 +389,11 @@ func TestExecuteEdit_ExactMatch(t *testing.T) { writeFileOrFail(t, path, original) input, _ := json.Marshal(editArgs{ - Path: path, - OldText: "fmt.Println(\"hello\")", - NewText: "fmt.Println(\"world\")", + Path: path, + Edits: []Edit{{ + OldText: "fmt.Println(\"hello\")", + NewText: "fmt.Println(\"world\")", + }}, }) resp, err := executeEdit(t.Context(), fantasy.ToolCall{Input: string(input)}, dir) @@ -426,9 +428,11 @@ func TestExecuteEdit_ExactMatch_DoesNotCorruptRest(t *testing.T) { target := lines[49] replacement := "REPLACED_LINE_50" input, _ := json.Marshal(editArgs{ - Path: path, - OldText: target, - NewText: replacement, + Path: path, + Edits: []Edit{{ + OldText: target, + NewText: replacement, + }}, }) resp, err := executeEdit(t.Context(), fantasy.ToolCall{Input: string(input)}, dir) @@ -470,9 +474,11 @@ func TestExecuteEdit_FuzzyMatch_TrailingWhitespace(t *testing.T) { // Search without trailing whitespace (common LLM behavior) input, _ := json.Marshal(editArgs{ - Path: path, - OldText: "func foo() {\n\treturn 1\n}", - NewText: "func foo() {\n\treturn 2\n}", + Path: path, + Edits: []Edit{{ + OldText: "func foo() {\n\treturn 1\n}", + NewText: "func foo() {\n\treturn 2\n}", + }}, }) resp, err := executeEdit(t.Context(), fantasy.ToolCall{Input: string(input)}, dir) @@ -519,9 +525,11 @@ func TestExecuteEdit_FuzzyMatch_DoesNotCorruptRest(t *testing.T) { search := strings.Repeat("x", 10) + "\n" + strings.Repeat("x", 10) // But this matches lines 1-2, 2-3, etc. — should fail due to ambiguity. input, _ := json.Marshal(editArgs{ - Path: path, - OldText: search, - NewText: "REPLACED", + Path: path, + Edits: []Edit{{ + OldText: search, + NewText: "REPLACED", + }}, }) resp, err := executeEdit(t.Context(), fantasy.ToolCall{Input: string(input)}, dir) @@ -546,9 +554,11 @@ func TestExecuteEdit_MultipleMatches_Fails(t *testing.T) { writeFileOrFail(t, path, "hello\nworld\nhello\n") input, _ := json.Marshal(editArgs{ - Path: path, - OldText: "hello", - NewText: "goodbye", + Path: path, + Edits: []Edit{{ + OldText: "hello", + NewText: "goodbye", + }}, }) resp, err := executeEdit(t.Context(), fantasy.ToolCall{Input: string(input)}, dir) @@ -575,9 +585,11 @@ func TestExecuteEdit_NoMatch_Fails(t *testing.T) { writeFileOrFail(t, path, "hello world\n") input, _ := json.Marshal(editArgs{ - Path: path, - OldText: "nonexistent text", - NewText: "replacement", + Path: path, + Edits: []Edit{{ + OldText: "nonexistent text", + NewText: "replacement", + }}, }) resp, err := executeEdit(t.Context(), fantasy.ToolCall{Input: string(input)}, dir) @@ -601,9 +613,11 @@ func TestExecuteEdit_CRLFNormalization(t *testing.T) { writeFileOrFail(t, path, "line1\r\nline2\r\nline3\r\n") input, _ := json.Marshal(editArgs{ - Path: path, - OldText: "line2", - NewText: "LINE2", + Path: path, + Edits: []Edit{{ + OldText: "line2", + NewText: "LINE2", + }}, }) resp, err := executeEdit(t.Context(), fantasy.ToolCall{Input: string(input)}, dir) @@ -622,8 +636,10 @@ func TestExecuteEdit_CRLFNormalization(t *testing.T) { func TestExecuteEdit_MissingPath(t *testing.T) { input, _ := json.Marshal(editArgs{ - OldText: "x", - NewText: "y", + Edits: []Edit{{ + OldText: "x", + NewText: "y", + }}, }) resp, err := executeEdit(t.Context(), fantasy.ToolCall{Input: string(input)}, "") if err != nil { @@ -636,9 +652,11 @@ func TestExecuteEdit_MissingPath(t *testing.T) { func TestExecuteEdit_NonexistentFile(t *testing.T) { input, _ := json.Marshal(editArgs{ - Path: "/tmp/nonexistent_edit_test_file_12345.go", - OldText: "x", - NewText: "y", + Path: "/tmp/nonexistent_edit_test_file_12345.go", + Edits: []Edit{{ + OldText: "x", + NewText: "y", + }}, }) resp, err := executeEdit(t.Context(), fantasy.ToolCall{Input: string(input)}, "") if err != nil { @@ -661,9 +679,11 @@ func TestExecuteEdit_DiffContainsHunkHeader(t *testing.T) { writeFileOrFail(t, path, strings.Join(lines, "\n")+"\n") input, _ := json.Marshal(editArgs{ - Path: path, - OldText: "line_10_content", - NewText: "REPLACED", + Path: path, + Edits: []Edit{{ + OldText: "line_10_content", + NewText: "REPLACED", + }}, }) resp, err := executeEdit(t.Context(), fantasy.ToolCall{Input: string(input)}, dir) @@ -684,9 +704,11 @@ func TestExecuteEdit_MetadataContainsFileDiffs(t *testing.T) { writeFileOrFail(t, path, "old content\n") input, _ := json.Marshal(editArgs{ - Path: path, - OldText: "old content", - NewText: "new content", + Path: path, + Edits: []Edit{{ + OldText: "old content", + NewText: "new content", + }}, }) resp, err := executeEdit(t.Context(), fantasy.ToolCall{Input: string(input)}, dir) @@ -905,18 +927,14 @@ func TestExecuteEdit_MultiEdit_EmptyArray(t *testing.T) { } } -func TestExecuteEdit_MultiEdit_MixedWithSingleMode(t *testing.T) { +func TestExecuteEdit_EmptyEditsArray_Fails(t *testing.T) { dir := t.TempDir() - path := filepath.Join(dir, "mixed.txt") + path := filepath.Join(dir, "empty.txt") writeFileOrFail(t, path, "hello\n") - input, _ := json.Marshal(map[string]any{ - "path": path, - "old_text": "hello", - "new_text": "HELLO", - "edits": []Edit{ - {OldText: "hello", NewText: "HI"}, - }, + input, _ := json.Marshal(editArgs{ + Path: path, + Edits: []Edit{}, }) resp, err := executeEdit(t.Context(), fantasy.ToolCall{Input: string(input)}, dir) @@ -924,10 +942,10 @@ func TestExecuteEdit_MultiEdit_MixedWithSingleMode(t *testing.T) { t.Fatalf("executeEdit error: %v", err) } if !resp.IsError { - t.Error("expected error when mixing single and multi-edit modes") + t.Error("expected error for empty edits array") } - if !strings.Contains(resp.Content, "cannot use") { - t.Errorf("expected 'cannot use' in error, got: %s", resp.Content) + if !strings.Contains(resp.Content, "required") { + t.Errorf("expected 'required' in error, got: %s", resp.Content) } } diff --git a/internal/ui/messages.go b/internal/ui/messages.go index 1b6a6da4..cfe39efc 100644 --- a/internal/ui/messages.go +++ b/internal/ui/messages.go @@ -88,13 +88,9 @@ func formatToolParams(toolArgs string, maxWidth int) string { } bodyKeys := map[string]bool{ - "content": true, - "old_text": true, - "new_text": true, - "oldText": true, - "newText": true, - "edits": true, - "todos": true, + "content": true, + "edits": true, + "todos": true, } var remaining []string for key, val := range params { diff --git a/internal/ui/tool_renderers.go b/internal/ui/tool_renderers.go index 4500cbac..caf6b7d3 100644 --- a/internal/ui/tool_renderers.go +++ b/internal/ui/tool_renderers.go @@ -79,8 +79,7 @@ func renderToolBody(toolName, toolArgs, toolResult string, width int) string { // Edit tool — side-by-side diff // --------------------------------------------------------------------------- -// renderEditBody renders a side-by-side diff from old_text/new_text in toolArgs. -// Supports both single-edit mode and multi-edit mode (edits array). +// renderEditBody renders a side-by-side diff from the edits array in toolArgs. func renderEditBody(toolArgs, toolResult string, width int) string { var args map[string]any if err := json.Unmarshal([]byte(toolArgs), &args); err != nil { @@ -90,35 +89,28 @@ func renderEditBody(toolArgs, toolResult string, width int) string { // Try to extract the starting line number from the unified diff in the result startLine := extractDiffStartLine(toolResult) - // Check for multi-edit mode (edits array) - if editsArr, ok := args["edits"].([]any); ok && len(editsArr) > 0 { - var results []string - for _, edit := range editsArr { - if e, ok := edit.(map[string]any); ok { - oldText, _ := e["old_text"].(string) - newText, _ := e["new_text"].(string) - if oldText != "" || newText != "" { - diff := renderDiffBlock(oldText, newText, startLine, width) - if diff != "" { - results = append(results, diff) - } + editsArr, ok := args["edits"].([]any) + if !ok || len(editsArr) == 0 { + return "" + } + + var results []string + for _, edit := range editsArr { + if e, ok := edit.(map[string]any); ok { + oldText, _ := e["old_text"].(string) + newText, _ := e["new_text"].(string) + if oldText != "" || newText != "" { + diff := renderDiffBlock(oldText, newText, startLine, width) + if diff != "" { + results = append(results, diff) } } } - if len(results) > 0 { - return strings.Join(results, "\n") - } - return "" } - - // Single-edit mode (legacy) - oldText, _ := args["old_text"].(string) - newText, _ := args["new_text"].(string) - if oldText == "" && newText == "" { - return "" + if len(results) > 0 { + return strings.Join(results, "\n") } - - return renderDiffBlock(oldText, newText, startLine, width) + return "" } // extractDiffStartLine parses the first @@ hunk header from a unified diff diff --git a/www/public/session/index.html b/www/public/session/index.html index e6e77446..f2e88c84 100644 --- a/www/public/session/index.html +++ b/www/public/session/index.html @@ -1968,35 +1968,42 @@ a:hover { text-decoration: underline; } function renderEditBody(input, result) { let html = ''; - const oldText = input.old_text || ''; - const newText = input.new_text || ''; + const edits = input.edits || []; html += '
'; html += '
Changes
'; - html += '
'; - // Render old text lines as removed - if (oldText) { - const oldLines = oldText.split('\n'); - for (const line of oldLines) { - html += `
- ${escapeHtml(line)}
`; + for (const edit of edits) { + const oldText = edit.old_text || ''; + const newText = edit.new_text || ''; + + html += '
'; + + // Render old text lines as removed + if (oldText) { + const oldLines = oldText.split('\n'); + for (const line of oldLines) { + html += `
- ${escapeHtml(line)}
`; + } } - } - // Separator - if (oldText && newText) { - html += '
───
'; - } - - // Render new text lines as added - if (newText) { - const newLines = newText.split('\n'); - for (const line of newLines) { - html += `
+ ${escapeHtml(line)}
`; + // Separator + if (oldText && newText) { + html += '
───
'; } + + // Render new text lines as added + if (newText) { + const newLines = newText.split('\n'); + for (const line of newLines) { + html += `
+ ${escapeHtml(line)}
`; + } + } + + html += '
'; } - html += '
'; + html += ''; if (result && result.is_error) { html += renderToolOutput(result.content, true);