diff --git a/agent/agentfiles/files.go b/agent/agentfiles/files.go index df6e1cb957e..6532ee4959d 100644 --- a/agent/agentfiles/files.go +++ b/agent/agentfiles/files.go @@ -1081,19 +1081,43 @@ func buildReplacementLines(matched, searchLines []string, replace, forcedEnding return b.String() } -// 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: +// errOldTextAmbiguous is returned when the old_text matches more +// than one location and replace_all is false. Shared by the +// byte-level pass and the line-level passes so both spell the +// remediation identically. +const errOldTextAmbiguous = "old_text matches %d occurrences " + + "(expected exactly 1). Include more surrounding " + + "context to make the match unique, or set " + + "replace_all to true" + +// errOldTextNotFoundPrefix is the shared lead of every not-found +// message; the per-pass tails below complete it. Rewording the lead +// touches this one constant. +const errOldTextNotFoundPrefix = "old_text not found in file. Verify that old_text " + + "matches the file content exactly, including whitespace" + +// errOldTextNotFound is the leading sentence returned when all match +// passes miss; diagnostic hints are appended after it. +const errOldTextNotFound = errOldTextNotFoundPrefix + " and indentation" + +// errOldTextNotFoundLineEndings is the ending-rewrite variant, used +// when the caller signaled intent to change line endings and the +// exact pass missed. +const errOldTextNotFoundLineEndings = errOldTextNotFoundPrefix + ", indentation, and line endings" + +// fuzzyReplace attempts to find the edit's old_text inside content +// and replace it with new_text. 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). // -// 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. +// When edit.ReplaceAll is false (the default), the old_text 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. // // When a fuzzy match is found (passes 2 or 3), buildReplacementLines // emits the spliced output by per-position substitution at @@ -1112,7 +1136,7 @@ func fuzzyReplace(content string, edit workspacesdk.FileEdit) (string, error) { // replace_all=true can't silently inject the replacement between // every byte. if search == "" { - return "", xerrors.New("search string must not be empty; include the " + + return "", xerrors.New("old_text must not be empty; include the " + "text you want to match") } @@ -1159,10 +1183,7 @@ func fuzzyReplace(content string, edit workspacesdk.FileEdit) (string, error) { } 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) + return "", xerrors.Errorf(errOldTextAmbiguous, count) } // Exactly one match. return strings.Replace(content, search, pass1Replace, 1), nil @@ -1171,9 +1192,7 @@ func fuzzyReplace(content string, edit workspacesdk.FileEdit) (string, error) { if callerEndingIntent { // Intent signaled but pass 1 missed; reject rather than let // pass 2's CRLF/LF interchange bridge a mismatched search. - return "", xerrors.New("search string not found in file. Verify the search " + - "string matches the file content exactly, including whitespace, " + - "indentation, and line endings") + return "", xerrors.New(errOldTextNotFoundLineEndings) } trimRight := func(a, b string) bool { @@ -1201,9 +1220,7 @@ func fuzzyReplace(content string, edit workspacesdk.FileEdit) (string, error) { return result, err } - msg := "search string not found in file. Verify the search " + - "string matches the file content exactly, including whitespace " + - "and indentation" + msg := errOldTextNotFound // miscount takes precedence: a near-match means the search is the // model's typo'd new text, not a swapped field. Emitting both can // trick an agent into following the inversion hint and corrupting @@ -1222,8 +1239,8 @@ func fuzzyReplace(content string, edit workspacesdk.FileEdit) (string, error) { // truncation with " and N more". const maxHintLines = 5 -// inversionHint detects the case where the caller swapped `search` -// and `replace`: search did not match but replace appears in the file. +// inversionHint detects the case where the caller swapped `old_text` +// and `new_text`: old_text did not match but new_text appears in the file. func inversionHint( content string, contentLines []string, @@ -1246,8 +1263,8 @@ func inversionHint( return "" } return fmt.Sprintf( - "Did you swap %q and %q? Your replace string appears at line %s", - "search", "replace", formatLineList(lines), + "Did you swap %q and %q? Your new_text string appears at line %s", + "old_text", "new_text", formatLineList(lines), ) } @@ -1367,7 +1384,7 @@ func miscountHint(contentLines, searchLines []string) string { // formatMiscount renders one miscount candidate group. func formatMiscount(sCount int, r rune, cands []candidate) string { var b strings.Builder - _, _ = fmt.Fprintf(&b, "Your search has %d %q (U+%04X); the file has ", sCount, string(r), r) + _, _ = fmt.Fprintf(&b, "Your old_text has %d %q (U+%04X); the file has ", sCount, string(r), r) shown := min(len(cands), maxHintLines) for i := 0; i < shown; i++ { if i > 0 { @@ -1505,10 +1522,7 @@ func fuzzyReplaceLines( if !replaceAll { if count := countLineMatches(contentLines, searchLines, eq); count > 1 { - return "", true, 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 "", true, xerrors.Errorf(errOldTextAmbiguous, count) } var b strings.Builder for _, l := range contentLines[:start] { diff --git a/agent/agentfiles/files_test.go b/agent/agentfiles/files_test.go index 3ec6fba9142..1c62b5052b9 100644 --- a/agent/agentfiles/files_test.go +++ b/agent/agentfiles/files_test.go @@ -851,7 +851,7 @@ func TestEditFiles(t *testing.T) { }, }, errCode: http.StatusBadRequest, - errors: []string{"search string not found in file"}, + errors: []string{"old_text not found in file"}, // File should remain unchanged. expected: map[string]string{filepath.Join(tmpdir, "no-match"): "original content"}, }, @@ -1025,7 +1025,7 @@ func TestEditFiles(t *testing.T) { }, }, errCode: http.StatusBadRequest, - errors: []string{"search string not found"}, + errors: []string{"old_text not found"}, expected: map[string]string{ filepath.Join(tmpdir, "atomic-a"): "aaa", filepath.Join(tmpdir, "atomic-b"): "bbb", @@ -2376,7 +2376,7 @@ func TestEditFiles_WhitespaceAndLineEndings(t *testing.T) { content: "above\n\n\nbelow\n", search: "above\nbelow", replace: "above\nbelow", - errSub: "search string not found", + errSub: "old_text not found", }, // Search matches blank lines exactly; replacement @@ -2522,7 +2522,7 @@ func TestFuzzyReplace_Rejects(t *testing.T) { name: "EmptySearch_Rejects", content: "hello\n", edits: []edit{{search: "", replace: "X"}}, - errSub: "search string must not be empty", + errSub: "old_text must not be empty", }, // Empty search with replace_all=true: historically // injected the replacement between every byte, silently @@ -2531,7 +2531,7 @@ func TestFuzzyReplace_Rejects(t *testing.T) { name: "EmptySearch_ReplaceAll_Rejects", content: "hello\n", edits: []edit{{search: "", replace: "X", replaceAll: true}}, - errSub: "search string must not be empty", + errSub: "old_text must not be empty", }, // Ambiguous single-replace: 3 distinct matches, caller // did not ask for replace_all. @@ -2547,7 +2547,7 @@ func TestFuzzyReplace_Rejects(t *testing.T) { name: "NotFound_Rejects", content: "hello\nworld\n", edits: []edit{{search: "nonexistent\n", replace: "X\n"}}, - errSub: "search string not found", + errSub: "old_text not found", }, // Content mismatch that trimming cannot recover: search // has different letters, not just different whitespace. @@ -2555,7 +2555,7 @@ func TestFuzzyReplace_Rejects(t *testing.T) { name: "ContentMismatch_Rejects", content: "hello\n", edits: []edit{{search: "Hello\n", replace: "HELLO\n"}}, - errSub: "search string not found", + errSub: "old_text not found", }, // Blank lines in the file that the search omits: the // fuzzy window cannot align against the blank lines, so @@ -2564,7 +2564,7 @@ func TestFuzzyReplace_Rejects(t *testing.T) { name: "BlankLineMismatch_Rejects", content: "above\n\n\nbelow\n", edits: []edit{{search: "above\nbelow\n", replace: "above\nbelow\n"}}, - errSub: "search string not found", + errSub: "old_text not found", }, // Search/replace disagreement signals intent to rewrite // endings; search must byte-match the file's. LF search @@ -2574,7 +2574,7 @@ func TestFuzzyReplace_Rejects(t *testing.T) { name: "CallerIntent_SearchDoesNotMatchFileEnding_Rejects", content: "x\r\ny\r\nz\r\n", edits: []edit{{search: "x\ny\n", replace: "X\r\nY\r\n"}}, - errSub: "search string not found", + errSub: "old_text not found", }, } @@ -3223,8 +3223,8 @@ func TestFuzzyReplace_Expansion_PreservesFileIndent(t *testing.T) { // returns when all three passes miss. It must remain the leading // sentence even when diagnostic hints are appended, so existing log // scrapers continue to match. -const baseFuzzyNotFoundMessage = "search string not found in file. " + - "Verify the search string matches the file content exactly, " + +const baseFuzzyNotFoundMessage = "old_text not found in file. " + + "Verify that old_text matches the file content exactly, " + "including whitespace and indentation" // TestFuzzyReplace_Hints exercises the post-fail diagnostic hints: @@ -3260,9 +3260,25 @@ func TestFuzzyReplace_Hints(t *testing.T) { }, wantSubs: []string{ baseFuzzyNotFoundMessage, - `Did you swap "search" and "replace"? Your replace string appears at line 3`, + `Did you swap "old_text" and "new_text"? Your new_text string appears at line 3`, }, }, + { + // CODAGT-523 falsifier: the hint must name the current + // schema fields, not the deprecated search/replace. + name: "Inversion_HintUsesNewSchemaNames", + content: "package main\n" + + "\n" + + "func adder(a int, b int) int { return a + b }\n" + + "\n" + + "// trailing comment\n", + edit: edit{ + search: "func adder(a, b int) int {\n\treturn a + b\n}\n", + replace: "func adder(a int, b int) int { return a + b }\n", + }, + wantSubs: []string{`"old_text"`, `"new_text"`}, + notWantSubs: []string{`"search"`, `"replace"`}, + }, { name: "Inversion_ThreeAnchors_AllListed", content: "a\n" + @@ -3278,7 +3294,7 @@ func TestFuzzyReplace_Hints(t *testing.T) { }, wantSubs: []string{ baseFuzzyNotFoundMessage, - `Did you swap "search" and "replace"? Your replace string appears at line 2, 4, 6`, + `Did you swap "old_text" and "new_text"? Your new_text string appears at line 2, 4, 6`, }, notWantSubs: []string{"more"}, }, @@ -3297,7 +3313,7 @@ func TestFuzzyReplace_Hints(t *testing.T) { }, wantSubs: []string{ baseFuzzyNotFoundMessage, - `Did you swap "search" and "replace"? Your replace string appears at line 1, 2, 3, 4, 5 and 2 more`, + `Did you swap "old_text" and "new_text"? Your new_text string appears at line 1, 2, 3, 4, 5 and 2 more`, }, }, { @@ -3313,7 +3329,7 @@ func TestFuzzyReplace_Hints(t *testing.T) { }, wantSubs: []string{ baseFuzzyNotFoundMessage, - `Did you swap "search" and "replace"? Your replace string appears at line 2, 3, 4, 5, 6 and 2 more`, + `Did you swap "old_text" and "new_text"? Your new_text string appears at line 2, 3, 4, 5, 6 and 2 more`, }, }, { @@ -3327,7 +3343,7 @@ func TestFuzzyReplace_Hints(t *testing.T) { }, wantSubs: []string{ baseFuzzyNotFoundMessage, - "Your search has 32 \"\u2500\" (U+2500); the file has 37 at line 2", + "Your old_text has 32 \"\u2500\" (U+2500); the file has 37 at line 2", }, }, { @@ -3341,7 +3357,7 @@ func TestFuzzyReplace_Hints(t *testing.T) { }, wantSubs: []string{ baseFuzzyNotFoundMessage, - `Your search has 5 "=" (U+003D); the file has 7 at line 2`, + `Your old_text has 5 "=" (U+003D); the file has 7 at line 2`, }, }, { @@ -3354,7 +3370,7 @@ func TestFuzzyReplace_Hints(t *testing.T) { }, wantSubs: []string{ baseFuzzyNotFoundMessage, - `Your search has 5 "=" (U+003D); the file has 7 at line 1, 3 at line 2`, + `Your old_text has 5 "=" (U+003D); the file has 7 at line 1, 3 at line 2`, }, notWantSubs: []string{"more"}, }, @@ -3372,7 +3388,7 @@ func TestFuzzyReplace_Hints(t *testing.T) { }, wantSubs: []string{ baseFuzzyNotFoundMessage, - `Your search has 5 "=" (U+003D); the file has 2 at line 1, 3 at line 2, 6 at line 3, 7 at line 4, 8 at line 5 and 1 more`, + `Your old_text has 5 "=" (U+003D); the file has 2 at line 1, 3 at line 2, 6 at line 3, 7 at line 4, 8 at line 5 and 1 more`, }, }, { @@ -3385,7 +3401,7 @@ func TestFuzzyReplace_Hints(t *testing.T) { replace: "unused\n", }, wantSubs: []string{baseFuzzyNotFoundMessage}, - notWantSubs: []string{"Your search has", "the file has"}, + notWantSubs: []string{"Your old_text has", "the file has"}, }, { name: "Miscount_Unrelated_NoHint", @@ -3395,7 +3411,7 @@ func TestFuzzyReplace_Hints(t *testing.T) { replace: "unused\n", }, wantSubs: []string{baseFuzzyNotFoundMessage}, - notWantSubs: []string{"Your search has", "the file has"}, + notWantSubs: []string{"Your old_text has", "the file has"}, }, { name: "Miscount_SuppressesInversion_WhenBothCouldFire", @@ -3415,7 +3431,7 @@ func TestFuzzyReplace_Hints(t *testing.T) { }, wantSubs: []string{ baseFuzzyNotFoundMessage, - "Your search has 6 \"\u2500\" (U+2500); the file has 8 at line 2", + "Your old_text has 6 \"\u2500\" (U+2500); the file has 8 at line 2", }, notWantSubs: []string{"swap", "appears at line"}, }, @@ -3430,7 +3446,7 @@ func TestFuzzyReplace_Hints(t *testing.T) { }, wantSubs: []string{ baseFuzzyNotFoundMessage, - `Did you swap "search" and "replace"? Your replace string appears at line 2`, + `Did you swap "old_text" and "new_text"? Your new_text string appears at line 2`, }, // Line 2 must appear once, not 2, 2, 2. notWantSubs: []string{"line 2, 2", "more"}, @@ -3449,7 +3465,7 @@ func TestFuzzyReplace_Hints(t *testing.T) { }, wantSubs: []string{ baseFuzzyNotFoundMessage, - `Did you swap "search" and "replace"? Your replace string appears at line 2`, + `Did you swap "old_text" and "new_text"? Your new_text string appears at line 2`, }, }, { @@ -3467,7 +3483,7 @@ func TestFuzzyReplace_Hints(t *testing.T) { }, wantSubs: []string{ baseFuzzyNotFoundMessage, - `Did you swap "search" and "replace"? Your replace string appears at line 2`, + `Did you swap "old_text" and "new_text"? Your new_text string appears at line 2`, }, }, { @@ -3480,7 +3496,7 @@ func TestFuzzyReplace_Hints(t *testing.T) { replace: "unused\n", }, wantSubs: []string{baseFuzzyNotFoundMessage}, - notWantSubs: []string{"Your search has", "the file has"}, + notWantSubs: []string{"Your old_text has", "the file has"}, }, { name: "Miscount_TotalHintsCapped", @@ -3500,9 +3516,9 @@ func TestFuzzyReplace_Hints(t *testing.T) { }, wantSubs: []string{ baseFuzzyNotFoundMessage, - `Your search has 4 "=" (U+003D)`, - `Your search has 4 "+" (U+002B)`, - `Your search has 4 "#" (U+0023)`, + `Your old_text has 4 "=" (U+003D)`, + `Your old_text has 4 "+" (U+002B)`, + `Your old_text has 4 "#" (U+0023)`, "and 1 more", }, // The fourth hint (`@`) is suppressed by the cap. @@ -3526,7 +3542,7 @@ func TestFuzzyReplace_Hints(t *testing.T) { }, wantSubs: []string{ baseFuzzyNotFoundMessage, - `Did you swap "search" and "replace"? Your replace string appears at line 1, 3`, + `Did you swap "old_text" and "new_text"? Your new_text string appears at line 1, 3`, }, notWantSubs: []string{"more"}, }, @@ -3542,7 +3558,7 @@ func TestFuzzyReplace_Hints(t *testing.T) { }, wantSubs: []string{ baseFuzzyNotFoundMessage, - `Your search has 0 "b" (U+0062); the file has 2 at line 1`, + `Your old_text has 0 "b" (U+0062); the file has 2 at line 1`, }, }, { @@ -3555,7 +3571,7 @@ func TestFuzzyReplace_Hints(t *testing.T) { replace: "new\n", }, wantSubs: []string{baseFuzzyNotFoundMessage}, - notWantSubs: []string{"swap", "Your search has", "appears at line"}, + notWantSubs: []string{"swap", "Your old_text has", "appears at line"}, }, } diff --git a/docs/ai-coder/agents/architecture.md b/docs/ai-coder/agents/architecture.md index 10a01c13ca2..e35dfbbf080 100644 --- a/docs/ai-coder/agents/architecture.md +++ b/docs/ai-coder/agents/architecture.md @@ -141,7 +141,7 @@ They traverse the same Tailnet tunnel used by web terminals and IDE connections. |------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | `read_file` | Reads file contents with line-number pagination. | | `write_file` | Writes content to a file. | -| `edit_files` | Performs atomic search-and-replace edits across one or more files. | +| `edit_files` | Performs atomic, per-file edits across one or more files, replacing `old_text` with `new_text`. | | `execute` | Runs a shell command, waiting for completion up to a timeout. | | `process_output` | Retrieves output from a tracked process. | | `process_list` | Lists all tracked processes in the workspace. | diff --git a/docs/ai-coder/agents/index.md b/docs/ai-coder/agents/index.md index 466ef003c08..7feb5bfd274 100644 --- a/docs/ai-coder/agents/index.md +++ b/docs/ai-coder/agents/index.md @@ -244,7 +244,7 @@ tasks: | `ask_user_question` | Ask the user structured clarification questions during plan mode | | `read_file` | Read file contents from the workspace | | `write_file` | Write a file to the workspace | -| `edit_files` | Perform search-and-replace edits across files | +| `edit_files` | Replace `old_text` with `new_text` across files | | `execute` | Run shell commands in the workspace | | `process_output` | Retrieve output from a background process | | `process_list` | List all tracked processes in the workspace | diff --git a/site/src/pages/AgentsPage/components/ChatElements/tools/utils.ts b/site/src/pages/AgentsPage/components/ChatElements/tools/utils.ts index 68d9d1f75f9..e557500b748 100644 --- a/site/src/pages/AgentsPage/components/ChatElements/tools/utils.ts +++ b/site/src/pages/AgentsPage/components/ChatElements/tools/utils.ts @@ -13,7 +13,9 @@ export interface EditFilesFileEntry { // Validates that the edit has at least the shape of an object with // string-typed text fields. Accepts both current field names -// (old_text/new_text) and deprecated names (search/replace). +// (old_text/new_text) and old names (search/replace): tool-call args +// persisted from older chats use the old names and are immutable, so +// this branch must stay to keep their diffs rendering. const normalizeEdit = ( e: unknown, ): { search: string; replace: string } | null => {