fix: error on ambiguous matches in edit_files tool (#23125)

## Problem

The `edit_files` tool used `strings.ReplaceAll` for exact substring
matches, silently replacing **every** occurrence. When an LLM's search
string wasn't unique in the file, this caused unintended edits. Fuzzy
matches (passes 2 and 3) only replaced the first occurrence, creating
inconsistent behavior. Zero matches were also silently ignored.

## Investigation

Investigated how **coder/mux** and **openai/codex** handle this:

| Tool | Multiple matches | No match | Flag |
|---|---|---|---|
| **coder/mux** `file_edit_replace_string` | Error (default
`replace_count=1`) | Error | `replace_count` (int, default 1, -1=all) |
| **openai/codex** `apply_patch` | Uses first match after cursor
(structural disambiguation via context lines + `@@` markers) | Error |
None (different paradigm) |
| **coder/coder** `edit_files` (before) | Exact: replaces all. Fuzzy:
replaces first. | Silent success | None |

## Solution

Adopted the mux approach (error on ambiguity) with a simpler
`replace_all: bool` instead of `replace_count: int`:

- **Default (`replace_all: false`)**: search string must match exactly
once. Multiple matches → error with guidance: *"search string matches N
occurrences. Include more surrounding context to make the match unique,
or set replace_all to true"*
- **`replace_all: true`**: replaces all occurrences (opt-in for
intentional bulk operations like variable renames)
- **Zero matches**: now returns an error instead of silently succeeding

Chose `bool` over `int` count because:
1. LLMs are bad at counting occurrences
2. The real intent is binary (one specific spot vs. all occurrences)
3. Simpler error recovery loop for the LLM

## Changes

| File | Change |
|---|---|
| `codersdk/workspacesdk/agentconn.go` | Add `ReplaceAll bool` to
`FileEdit` struct |
| `agent/agentfiles/files.go` | Count matches before replacing; error if
>1 and not opted in; error on zero matches; add `countLineMatches`
helper |
| `codersdk/toolsdk/toolsdk.go` | Expose `replace_all` in tool schema
with description |
| `agent/agentfiles/files_test.go` | Update existing tests, add
`EditEditAmbiguous`, `EditEditReplaceAll`, `NoMatchErrors`,
`AmbiguousExactMatch`, `ReplaceAllExact` |

agent/agentfiles/files.go

@@ -447,13 +447,10 @@ func (api *API) editFile(ctx context.Context, path string, edits []workspacesdk.
 	content := string(data)
 
 	for _, edit := range edits {
-		var ok bool
-		content, ok = fuzzyReplace(content, edit.Search, edit.Replace)
-		if !ok {
-			api.logger.Warn(ctx, "edit search string not found, skipping",
-				slog.F("path", path),
-				slog.F("search_preview", truncate(edit.Search, 64)),
-			)
+		var err error
+		content, err = fuzzyReplace(content, edit)
+		if err != nil {
+			return http.StatusBadRequest, xerrors.Errorf("edit %s: %w", path, err)
 		}
 	}
 
@@ -480,51 +477,92 @@ func (api *API) editFile(ctx context.Context, path string, edits []workspacesdk.
 	return 0, nil
 }
 
-// fuzzyReplace attempts to find `search` inside `content` and replace its first
-// occurrence with `replace`. It uses a cascading match strategy inspired by
+// fuzzyReplace attempts to find `search` inside `content` and replace it
+// with `replace`. It uses a cascading match strategy inspired by
 // openai/codex's apply_patch:
 //
 //  1. Exact substring match (byte-for-byte).
 //  2. Line-by-line match ignoring trailing whitespace on each line.
-//  3. Line-by-line match ignoring all leading/trailing whitespace (indentation-tolerant).
+//  3. Line-by-line match ignoring all leading/trailing whitespace
+//     (indentation-tolerant).
 //
-// When a fuzzy match is found (passes 2 or 3), the replacement is still applied
-// at the byte offsets of the original content so that surrounding text (including
-// indentation of untouched lines) is preserved.
+// When edit.ReplaceAll is false (the default), the search string must
+// match exactly one location. If multiple matches are found, an error
+// is returned asking the caller to include more context or set
+// replace_all.
 //
-// Returns the (possibly modified) content and a bool indicating whether a match
-// was found.
-func fuzzyReplace(content, search, replace string) (string, bool) {
-	// Pass 1 – exact substring (replace all occurrences).
+// When a fuzzy match is found (passes 2 or 3), the replacement is still
+// applied at the byte offsets of the original content so that surrounding
+// text (including indentation of untouched lines) is preserved.
+func fuzzyReplace(content string, edit workspacesdk.FileEdit) (string, error) {
+	search := edit.Search
+	replace := edit.Replace
+
+	// Pass 1 – exact substring match.
 	if strings.Contains(content, search) {
-		return strings.ReplaceAll(content, search, replace), true
+		if edit.ReplaceAll {
+			return strings.ReplaceAll(content, search, replace), nil
+		}
+		count := strings.Count(content, search)
+		if count > 1 {
+			return "", xerrors.Errorf("search string matches %d occurrences "+
+				"(expected exactly 1). Include more surrounding "+
+				"context to make the match unique, or set "+
+				"replace_all to true", count)
+		}
+		// Exactly one match.
+		return strings.Replace(content, search, replace, 1), nil
 	}
 
-	// For line-level fuzzy matching we split both content and search into lines.
+	// For line-level fuzzy matching we split both content and search
+	// into lines.
 	contentLines := strings.SplitAfter(content, "\n")
 	searchLines := strings.SplitAfter(search, "\n")
 
-	// A trailing newline in the search produces an empty final element from
-	// SplitAfter.  Drop it so it doesn't interfere with line matching.
+	// A trailing newline in the search produces an empty final element
+	// from SplitAfter. Drop it so it doesn't interfere with line
+	// matching.
 	if len(searchLines) > 0 && searchLines[len(searchLines)-1] == "" {
 		searchLines = searchLines[:len(searchLines)-1]
 	}
 
-	// Pass 2 – trim trailing whitespace on each line.
-	if start, end, ok := seekLines(contentLines, searchLines, func(a, b string) bool {
+	trimRight := func(a, b string) bool {
 		return strings.TrimRight(a, " \t\r\n") == strings.TrimRight(b, " \t\r\n")
-	}); ok {
-		return spliceLines(contentLines, start, end, replace), true
 	}
-
-	// Pass 3 – trim all leading and trailing whitespace (indentation-tolerant).
-	if start, end, ok := seekLines(contentLines, searchLines, func(a, b string) bool {
+	trimAll := func(a, b string) bool {
 		return strings.TrimSpace(a) == strings.TrimSpace(b)
-	}); ok {
-		return spliceLines(contentLines, start, end, replace), true
 	}
 
-	return content, false
+	// Pass 2 – trim trailing whitespace on each line.
+	if start, end, ok := seekLines(contentLines, searchLines, trimRight); ok {
+		if !edit.ReplaceAll {
+			if count := countLineMatches(contentLines, searchLines, trimRight); count > 1 {
+				return "", xerrors.Errorf("search string matches %d occurrences "+
+					"(expected exactly 1). Include more surrounding "+
+					"context to make the match unique, or set "+
+					"replace_all to true", count)
+			}
+		}
+		return spliceLines(contentLines, start, end, replace), nil
+	}
+
+	// Pass 3 – trim all leading and trailing whitespace
+	// (indentation-tolerant).
+	if start, end, ok := seekLines(contentLines, searchLines, trimAll); ok {
+		if !edit.ReplaceAll {
+			if count := countLineMatches(contentLines, searchLines, trimAll); count > 1 {
+				return "", xerrors.Errorf("search string matches %d occurrences "+
+					"(expected exactly 1). Include more surrounding "+
+					"context to make the match unique, or set "+
+					"replace_all to true", count)
+			}
+		}
+		return spliceLines(contentLines, start, end, replace), nil
+	}
+
+	return "", xerrors.New("search string not found in file. Verify the search " +
+		"string matches the file content exactly, including whitespace " +
+		"and indentation")
 }
 
 // seekLines scans contentLines looking for a contiguous subsequence that matches
@@ -549,6 +587,26 @@ outer:
 	return 0, 0, false
 }
 
+// countLineMatches counts how many non-overlapping contiguous
+// subsequences of contentLines match searchLines according to eq.
+func countLineMatches(contentLines, searchLines []string, eq func(a, b string) bool) int {
+	count := 0
+	if len(searchLines) == 0 || len(searchLines) > len(contentLines) {
+		return count
+	}
+outer:
+	for i := 0; i <= len(contentLines)-len(searchLines); i++ {
+		for j, sLine := range searchLines {
+			if !eq(contentLines[i+j], sLine) {
+				continue outer
+			}
+		}
+		count++
+		i += len(searchLines) - 1 // skip past this match
+	}
+	return count
+}
+
 // spliceLines replaces contentLines[start:end] with replacement text, returning
 // the full content as a single string.
 func spliceLines(contentLines []string, start, end int, replacement string) string {
@@ -562,10 +620,3 @@ func spliceLines(contentLines []string, start, end int, replacement string) stri
 	}
 	return b.String()
 }
-
-func truncate(s string, n int) string {
-	if len(s) <= n {
-		return s
-	}
-	return s[:n] + "..."
-}

agent/agentfiles/files_test.go

@@ -576,7 +576,9 @@ func TestEditFiles(t *testing.T) {
 			expected: map[string]string{filepath.Join(tmpdir, "edit1"): "bar bar"},
 		},
 		{
-			name:     "EditEdit", // Edits affect previous edits.
+			// When the second edit creates ambiguity (two "bar"
+			// occurrences), it should fail.
+			name:     "EditEditAmbiguous",
 			contents: map[string]string{filepath.Join(tmpdir, "edit-edit"): "foo bar"},
 			edits: []workspacesdk.FileEdits{
 				{
@@ -593,7 +595,33 @@ func TestEditFiles(t *testing.T) {
 					},
 				},
 			},
-			expected: map[string]string{filepath.Join(tmpdir, "edit-edit"): "qux qux"},
+			errCode: http.StatusBadRequest,
+			errors:  []string{"matches 2 occurrences"},
+			// File should not be modified on error.
+			expected: map[string]string{filepath.Join(tmpdir, "edit-edit"): "foo bar"},
+		},
+		{
+			// With replace_all the cascading edit replaces
+			// both occurrences.
+			name:     "EditEditReplaceAll",
+			contents: map[string]string{filepath.Join(tmpdir, "edit-edit-ra"): "foo bar"},
+			edits: []workspacesdk.FileEdits{
+				{
+					Path: filepath.Join(tmpdir, "edit-edit-ra"),
+					Edits: []workspacesdk.FileEdit{
+						{
+							Search:  "foo",
+							Replace: "bar",
+						},
+						{
+							Search:     "bar",
+							Replace:    "qux",
+							ReplaceAll: true,
+						},
+					},
+				},
+			},
+			expected: map[string]string{filepath.Join(tmpdir, "edit-edit-ra"): "qux qux"},
 		},
 		{
 			name:     "Multiline",
@@ -720,7 +748,7 @@ func TestEditFiles(t *testing.T) {
 			expected: map[string]string{filepath.Join(tmpdir, "exact-preferred"): "goodbye world"},
 		},
 		{
-			name:     "NoMatchStillSucceeds",
+			name:     "NoMatchErrors",
 			contents: map[string]string{filepath.Join(tmpdir, "no-match"): "original content"},
 			edits: []workspacesdk.FileEdits{
 				{
@@ -733,9 +761,46 @@ func TestEditFiles(t *testing.T) {
 					},
 				},
 			},
+			errCode: http.StatusBadRequest,
+			errors:  []string{"search string not found in file"},
 			// File should remain unchanged.
 			expected: map[string]string{filepath.Join(tmpdir, "no-match"): "original content"},
 		},
+		{
+			name:     "AmbiguousExactMatch",
+			contents: map[string]string{filepath.Join(tmpdir, "ambig-exact"): "foo bar foo baz foo"},
+			edits: []workspacesdk.FileEdits{
+				{
+					Path: filepath.Join(tmpdir, "ambig-exact"),
+					Edits: []workspacesdk.FileEdit{
+						{
+							Search:  "foo",
+							Replace: "qux",
+						},
+					},
+				},
+			},
+			errCode:  http.StatusBadRequest,
+			errors:   []string{"matches 3 occurrences"},
+			expected: map[string]string{filepath.Join(tmpdir, "ambig-exact"): "foo bar foo baz foo"},
+		},
+		{
+			name:     "ReplaceAllExact",
+			contents: map[string]string{filepath.Join(tmpdir, "ra-exact"): "foo bar foo baz foo"},
+			edits: []workspacesdk.FileEdits{
+				{
+					Path: filepath.Join(tmpdir, "ra-exact"),
+					Edits: []workspacesdk.FileEdit{
+						{
+							Search:     "foo",
+							Replace:    "qux",
+							ReplaceAll: true,
+						},
+					},
+				},
+			},
+			expected: map[string]string{filepath.Join(tmpdir, "ra-exact"): "qux bar qux baz qux"},
+		},
 		{
 			name:     "MixedWhitespaceMultiline",
 			contents: map[string]string{filepath.Join(tmpdir, "mixed-ws"): "func main() {\n\tresult := compute()\n\tfmt.Println(result)\n}"},

codersdk/toolsdk/toolsdk.go

@@ -1702,12 +1702,16 @@ var WorkspaceEditFiles = Tool[WorkspaceEditFilesArgs, codersdk.Response]{
 									"properties": map[string]any{
 										"search": map[string]any{
 											"type":        "string",
-											"description": "The old string to replace.",
+											"description": "The old string to replace. Must uniquely match exactly one location in the file unless replace_all is true. Include enough surrounding context to make the match unique.",
 										},
 										"replace": map[string]any{
 											"type":        "string",
 											"description": "The new string that replaces the old string.",
 										},
+										"replace_all": map[string]any{
+											"type":        "boolean",
+											"description": "When true, replaces all occurrences of the search string. Defaults to false, which requires the search string to match exactly once.",
+										},
 									},
 									"required": []string{"search", "replace"},
 								},

codersdk/workspacesdk/agentconn.go

@@ -899,8 +899,9 @@ func DefaultReadFileLinesLimits() ReadFileLinesLimits {
 }
 
 type FileEdit struct {
-	Search  string `json:"search"`
-	Replace string `json:"replace"`
+	Search     string `json:"search"`
+	Replace    string `json:"replace"`
+	ReplaceAll bool   `json:"replace_all,omitempty"`
 }
 
 type FileEdits struct {