Skip to content

edit_file silently edits the first match when oldText occurs more than once #3929

Description

@dwin-gharibi

Description

handleEditFile checks only whether oldText is present, then replaces a single occurrence:

// pkg/tools/builtin/filesystem/filesystem.go:1019-1025
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
    }
    modifiedContent = strings.Replace(modifiedContent, edit.OldText, edit.NewText, 1)
    changes = append(changes, fmt.Sprintf("Edit %d: Replaced %d characters", i+1, len(edit.OldText)))
}

There is a guard for zero matches but none for multiple. With N > 1 matches the first is
rewritten, the rest are left alone, and the reported message —
Replaced %d characters — mentions neither the count nor which occurrence was touched. It is
indistinguishable from an unambiguous edit.

Expected Behavior

An oldText that matches more than once is refused, so the caller disambiguates by supplying
more surrounding context. The file is left unmodified.

Actual Behavior

The first occurrence is rewritten and the tool reports plain success.

Steps to Reproduce

conf.py:
  def dev():
      debug = True

  def prod():
      debug = True
{"path":"conf.py","edits":[{"oldText":"    debug = True","newText":"    debug = False"}]}
occurrences of oldText in file = 2
tool reported: "File edited successfully. Replaced 16 characters"

file after:
  def dev():
      debug = False      <- changed
  def prod():
      debug = True       <- silently left alone

Docker Agent version

No response

OS & terminal

No response

Model used

No response

Error output

Screenshots

No response

Additional context

Impact

The model cannot tell whether it edited the site it intended. When it meant the second
occurrence, the wrong code is now modified and the agent has been told it succeeded — so it
does not re-read the file to verify, and the mistake propagates.

The shape in the reproduction (the same assignment in a dev branch and a prod branch) is common
in configuration and environment-branching code, which makes the wrong-site edit both easy to
trigger and hard to trace afterwards.

Refusing an ambiguous match is the established behaviour for this class of tool: the caller is
expected to include enough context to identify a unique site.

Additional context

  • Occurrences must be counted against the running content rather than the original file: a
    multi-edit call can legitimately have an earlier edit remove one of the duplicates, leaving a
    later edit unambiguous. Counting against the original would break that case.
  • strings.Count folds in the existing zero-match check, so the strings.Contains call is not
    needed alongside it — one scan instead of two.
  • This is a behaviour change for any caller relying on first-match-wins. Nothing in the Go tests,
    e2e/ or the recorded cassettes does: the cassettes contain only the edit_file JSON schema
    sent to the model, not multi-occurrence edit calls.
  • Related, already fixed separately: an empty oldText also passed the presence check and
    silently prepended to the file.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/toolsFor features/issues/fixes related to the usage of built-in and MCP tools

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions