Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions pkg/acp/filesystem.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,11 @@ func (t *FilesystemToolset) handleEditFile(ctx context.Context, toolCall tools.T
modifiedContent := resp.Content

for i, edit := range args.Edits {
if !strings.Contains(modifiedContent, edit.OldText) {
return tools.ResultError(fmt.Sprintf("Edit %d failed: old text not found", i+1)), nil
match, err := builtin.FindMatch(modifiedContent, edit.OldText)
if err != nil {
return tools.ResultError(fmt.Sprintf("Edit %d failed: %s", i+1, err)), nil
}
modifiedContent = strings.Replace(modifiedContent, edit.OldText, edit.NewText, 1)
modifiedContent = strings.Replace(modifiedContent, match.SearchText, edit.NewText, 1)
}

_, err = t.agent.conn.WriteTextFile(ctx, acp.WriteTextFileRequest{
Expand Down
161 changes: 161 additions & 0 deletions pkg/tools/builtin/edit_match.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
package builtin

import (
"errors"
"strings"
"unicode/utf8"
)

// Inspired by opencode's edit tool (github.com/anomalyco/opencode):
//
// - Cascading replacer chain: try strategies in order, first unique match
// wins (opencode: SimpleReplacer → … → EscapeNormalizedReplacer → …).
// - Uniqueness gate: a candidate is accepted only when it appears exactly
// once in content (opencode: indexOf !== lastIndexOf check in replace()).
// - EscapeNormalizedReplacer: un-escape both sides to handle the common
// LLM failure of confusing JSON-level escaping with file content
// (opencode: EscapeNormalizedReplacer with the same unescapeString
// logic covering \", \', \n, \t, \r, \\, \`, \$).

// MatchResult describes which text in the file content should be replaced.
type MatchResult struct {
// SearchText is the exact substring of content to replace (may differ
// from the caller's oldText when a fuzzy replacer found the match).
SearchText string
// Strategy is the name of the replacer that produced the match.
Strategy string
}

// replacer yields candidate search strings from content for a given find
// string. Each candidate must be an exact substring of content.
type replacer struct {
name string
fn func(content, find string) []string
}

// replacers is the cascade tried by FindMatch, in order.
// The first replacer to produce exactly one unique match wins.
var replacers = []replacer{
{"exact", exactReplacer},
{"escape-normalized", escapeNormalizedReplacer},
}

// FindMatch searches content for oldText using a cascade of increasingly
// fuzzy strategies. It returns the exact substring of content that should
// be replaced, or an error explaining why no unique match was found.
func FindMatch(content, oldText string) (MatchResult, error) {
sawMultiple := false

for _, r := range replacers {
for _, candidate := range r.fn(content, oldText) {
before, after, found := strings.Cut(content, candidate)
if !found {
continue
}
// Ensure the candidate appears exactly once.
_ = before
if strings.Contains(after, candidate) {
sawMultiple = true
continue
}
return MatchResult{SearchText: candidate, Strategy: r.name}, nil
}
}

if sawMultiple {
return MatchResult{}, errors.New(
"old text matched multiple locations — provide more surrounding context to make the match unique")
}
return MatchResult{}, errors.New("old text not found")
}

// ---------------------------------------------------------------------------
// Replacers
// ---------------------------------------------------------------------------

// exactReplacer returns oldText itself if it appears in content.
func exactReplacer(content, find string) []string {
if strings.Contains(content, find) {
return []string{find}
}
return nil
}

// escapeNormalizedReplacer handles the common LLM failure mode where
// escape sequences like \" in file content are emitted as plain " in
// the tool call arguments, or vice versa.
//
// It un-escapes both the find string and candidate blocks in content,
// then compares. The returned match is the original (escaped) text from
// content so the replacement targets the correct bytes on disk.
func escapeNormalizedReplacer(content, find string) []string {
unescapedFind := unescapeString(find)

// Fast path: the un-escaped find appears verbatim in content.
if unescapedFind != find && strings.Contains(content, unescapedFind) {
return []string{unescapedFind}
}

// Slow path: un-escape same-sized blocks of content and compare.
contentLines := strings.Split(content, "\n")
findLines := strings.Split(unescapedFind, "\n")
if len(findLines) == 0 {
return nil
}

var matches []string
for i := 0; i <= len(contentLines)-len(findLines); i++ {
block := strings.Join(contentLines[i:i+len(findLines)], "\n")
if unescapeString(block) == unescapedFind {
matches = append(matches, block)
}
}
return matches
}

// ---------------------------------------------------------------------------
// Helpers
// ---------------------------------------------------------------------------

// unescapeString resolves common escape sequences that LLMs produce when
// they confuse JSON-level escaping with content-level escaping.
func unescapeString(s string) string {
var b strings.Builder
b.Grow(len(s))
i := 0
for i < len(s) {
r, size := utf8.DecodeRuneInString(s[i:])
if r == '\\' && i+size < len(s) {
next, nextSize := utf8.DecodeRuneInString(s[i+size:])
var replacement byte
switch next {
case 'n':
replacement = '\n'
case 't':
replacement = '\t'
case 'r':
replacement = '\r'
case '"':
replacement = '"'
case '\'':
replacement = '\''
case '`':
replacement = '`'
case '\\':
replacement = '\\'
case '$':
replacement = '$'
case '\n':
replacement = '\n'
}
if replacement != 0 {
b.WriteByte(replacement)
i += size + nextSize
continue
}
}
b.WriteRune(r)
i += size
}
return b.String()
}
161 changes: 161 additions & 0 deletions pkg/tools/builtin/edit_match_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
package builtin

import (
"strings"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestFindMatch(t *testing.T) {
t.Parallel()

tests := []struct {
name string
content string
find string
wantText string // expected SearchText (empty → use find)
wantStrategy string
wantErr string // substring of error; empty → expect success
}{
// Exact
{
name: "exact",
content: "hello world\nfoo bar\nbaz",
find: "foo bar",
wantStrategy: "exact",
},
{
name: "exact when both sides escaped",
content: `echo \"hello\"`,
find: `echo \"hello\"`,
wantStrategy: "exact",
},

// Escape-normalized
{
name: `\" in file vs " in find`,
content: "echo \\\"brew install failed\\\"",
find: `echo "brew install failed"`,
wantText: "echo \\\"brew install failed\\\"",
wantStrategy: "escape-normalized",
},
{
name: `literal \n in find vs real newline`,
content: "line1\nline2",
find: `line1\nline2`,
wantText: "line1\nline2",
wantStrategy: "escape-normalized",
},
{
name: `literal \t in find vs real tab`,
content: "hello\tworld",
find: `hello\tworld`,
wantText: "hello\tworld",
wantStrategy: "escape-normalized",
},
{
name: `\$ in find vs $ in file`,
content: "echo $HOME",
find: `echo \$HOME`,
wantText: "echo $HOME",
wantStrategy: "escape-normalized",
},

// Ambiguity
{
name: "ambiguous exact duplicate",
content: "foo\nbar\nfoo\nbaz",
find: "foo",
wantErr: "multiple locations",
},
{
name: "ambiguous escaped duplicate",
content: "echo \\\"hi\\\"\necho \\\"hi\\\"",
find: `echo "hi"`,
wantErr: "multiple locations",
},

// Not found
{
name: "not found",
content: "hello world",
find: "goodbye universe",
wantErr: "old text not found",
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
m, err := FindMatch(tc.content, tc.find)
if tc.wantErr != "" {
require.Error(t, err)
assert.Contains(t, err.Error(), tc.wantErr)
return
}
require.NoError(t, err)
if tc.wantText != "" {
assert.Equal(t, tc.wantText, m.SearchText)
}
assert.Equal(t, tc.wantStrategy, m.Strategy)
})
}
}

func TestUnescapeString(t *testing.T) {
t.Parallel()
for _, tc := range []struct{ in, want string }{
{`hello`, "hello"},
{`hello\nworld`, "hello\nworld"},
{`hello\tworld`, "hello\tworld"},
{`echo \"hi\"`, `echo "hi"`},
{`echo \'hi\'`, `echo 'hi'`},
{`path\\to\\file`, `path\to\file`},
{`\$HOME`, "$HOME"},
{`hello\xworld`, `hello\xworld`}, // unknown escape preserved
} {
t.Run(tc.in, func(t *testing.T) {
t.Parallel()
assert.Equal(t, tc.want, unescapeString(tc.in))
})
}
}

// Reproduce the exact Dockerfile scenario that motivated this change.
func TestFindMatch_DockerfileBrewScenario(t *testing.T) {
t.Parallel()

// File on disk has literal \" around "brew install failed".
fileContent := strings.Join([]string{
`RUN if [ "${INSTALL_BREW}" = "1" ]; then \`,
` if ! id -u linuxbrew >/dev/null 2>&1; then useradd -m -s /bin/bash linuxbrew; fi; \`,
` mkdir -p "${BREW_INSTALL_DIR}"; \`,
` chown -R linuxbrew:linuxbrew "$(dirname "${BREW_INSTALL_DIR}")"; \`,
` su - linuxbrew -c "NONINTERACTIVE=1 CI=1 /bin/bash -c '$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)'"; \`,
` if [ ! -e "${BREW_INSTALL_DIR}/Library" ]; then ln -s "${BREW_INSTALL_DIR}/Homebrew/Library" "${BREW_INSTALL_DIR}/Library"; fi; \`,
` if [ ! -x "${BREW_INSTALL_DIR}/bin/brew" ]; then echo \"brew install failed\"; exit 1; fi; \`,
` ln -sf "${BREW_INSTALL_DIR}/bin/brew" /usr/local/bin/brew; \`,
`fi`,
}, "\n")

// LLM dropped the backslashes: \" → "
llmOldText := strings.Join([]string{
`RUN if [ "${INSTALL_BREW}" = "1" ]; then \`,
` if ! id -u linuxbrew >/dev/null 2>&1; then useradd -m -s /bin/bash linuxbrew; fi; \`,
` mkdir -p "${BREW_INSTALL_DIR}"; \`,
` chown -R linuxbrew:linuxbrew "$(dirname "${BREW_INSTALL_DIR}")"; \`,
` su - linuxbrew -c "NONINTERACTIVE=1 CI=1 /bin/bash -c '$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)'"; \`,
` if [ ! -e "${BREW_INSTALL_DIR}/Library" ]; then ln -s "${BREW_INSTALL_DIR}/Homebrew/Library" "${BREW_INSTALL_DIR}/Library"; fi; \`,
` if [ ! -x "${BREW_INSTALL_DIR}/bin/brew" ]; then echo "brew install failed"; exit 1; fi; \`,
` ln -sf "${BREW_INSTALL_DIR}/bin/brew" /usr/local/bin/brew; \`,
`fi`,
}, "\n")

m, err := FindMatch(fileContent, llmOldText)
require.NoError(t, err)
assert.Equal(t, fileContent, m.SearchText)
assert.Equal(t, "escape-normalized", m.Strategy)
assert.Equal(t, "REPLACED", strings.Replace(fileContent, m.SearchText, "REPLACED", 1))
}
13 changes: 9 additions & 4 deletions pkg/tools/builtin/filesystem.go
Original file line number Diff line number Diff line change
Expand Up @@ -477,11 +477,16 @@ func (t *FilesystemTool) handleEditFile(ctx context.Context, args EditFileArgs)

var changes []string
for i, edit := range args.Edits {
if !strings.Contains(modifiedContent, edit.OldText) {
return tools.ResultError(fmt.Sprintf("Edit %d failed: old text not found", i+1)), nil
match, err := FindMatch(modifiedContent, edit.OldText)
if err != nil {
return tools.ResultError(fmt.Sprintf("Edit %d failed: %s", i+1, err)), nil
}
modifiedContent = strings.Replace(modifiedContent, match.SearchText, edit.NewText, 1)
detail := fmt.Sprintf("Edit %d: Replaced %d characters", i+1, len(match.SearchText))
if match.Strategy != "exact" {
detail += fmt.Sprintf(" (matched via %s)", match.Strategy)
}
modifiedContent = strings.Replace(modifiedContent, edit.OldText, edit.NewText, 1)
changes = append(changes, fmt.Sprintf("Edit %d: Replaced %d characters", i+1, len(edit.OldText)))
changes = append(changes, detail)
}

if err := os.WriteFile(resolvedPath, []byte(modifiedContent), 0o644); err != nil {
Expand Down
Loading