fix(tools): patch idempotency guard prevents duplicate-content loops on re-edit (#18426)

[Tranquil-Flow]

What

Re-applying the same old_string/new_string after the first patch already landed left old_string absent from the file. The context_aware / block_anchor fuzzy strategies then matched the already-modified region and substituted new_string over a partial slice, corrupting the file by duplicating trailing lines. Repeated re-application could multiply a section 2–4× and the agent would loop on apparent success until interrupted manually.

Fix

Add an idempotency guard at the top of fuzzy_find_and_replace() (tools/fuzzy_match.py): when old_string is no longer present in the file but new_string already is, fail cleanly with a re-read hint instead of falling through to the fuzzy strategy chain.

if new_string and old_string not in content and new_string in content:
    return content, 0, None, (
        "old_string was not found in the file, but new_string is already "
        "present — this patch appears to have been applied already. "
        "Re-read the file to see its current state before editing again."
    )

Why this is safe (no false positives on legitimate edits)

• Exact first patch: old_string is present → guard skipped, exact match proceeds.
• Legitimate fuzzy first patch (indent/whitespace drift): old_string is absent but new_string is also absent (the change has not landed yet) → guard skipped, fuzzy proceeds normally.
• Deletion (new_string=""): the new_string and short-circuit prevents the vacuous "" in content test from firing.
• Fuzzy-first re-patch: new_string is re-indented on write, so the raw new_string in content check is False → guard skipped; the existing "Found 2 matches" uniqueness guard catches it.

Verification

• Reproduced the bug on main: apply old→new once (exact, succeeds), re-apply the same old→new → context_aware (50% line-similarity) matched the modified region and duplicated the trailing return result line (count 1 → 2).
• RED: 2 of the 6 new tests fail without the fix (headline corruption repro + error-message test).
• GREEN: all 6 new tests pass with the fix; full focused suite 259 passed, 0 regressions (test_fuzzy_match.py + test_file_operations.py + test_skill_manager_tool.py).
• Rebases cleanly onto current main (0 behind / 1 ahead).

Tests

New TestIdempotencyGuard class in tests/tools/test_fuzzy_match.py (6 tests, all call the real fuzzy_find_and_replace production path):

1. test_repatch_after_apply_does_not_duplicate — the headline repro
2. test_already_applied_error_guides_reread — message guides a re-read
3. test_legitimate_fuzzy_match_with_new_absent_still_works — regression guard
4. test_replace_all_already_applied_errors_cleanly — replace_all path
5. test_old_absent_new_absent_still_fuzzy_matches — fuzzy still works when neither present
6. test_deletion_with_empty_new_does_not_trip_guard — empty new_string edge case

Closes #18426.

---

*Auto-published by Moonsong via Path B automated pipeline.

[alt-glitch]

This was generated by AI during triage.

Competing fix for #18426. #18614 (open, earlier) adds an idempotency guard by checking whether the matched region already equals new_string inside the match loop (and handles replace_all); this PR guards at the top of fuzzy_find_and_replace() on whole-content presence (old_string not in content and new_string in content) before the fuzzy strategy chain runs. Same goal, different mechanism/site — flagging for a maintainer to pick one. Related: #18426 (issue), #54572 (block_anchor wrong-region edit in the same fuzzy matcher).

[teknium1]

Thanks for isolating a real retry-corruption path. Current main still reproduces it: context_aware accepts the changed retry candidate at tools/fuzzy_match.py:696-728, and fuzzy_find_and_replace applies that span at tools/fuzzy_match.py:85-153.

Problems

• The new whole-content condition at tools/fuzzy_match.py:85 in this PR is too broad. A valid first fuzzy edit is rejected when new_string appears in a separate section of the file. On current main, a config with production status = enabled and whitespace-drifted staging status = disabled successfully updates staging through whitespace_normalized; this PR's predicate is true before matching.
• Coverage is helper-only. The mutation boundary is ShellFileOperations.patch_replace at tools/file_operations.py:1593-1606, so the no-write-on-retry guarantee is not exercised through file I/O.

Suggested changes

• Scope the guard to the selected candidate rather than whole-file new_string presence, and add the unrelated-occurrence regression.
• Add a live patch_replace retry test asserting both an error and unchanged on-disk content.

Automated hermes-sweeper review.

[teknium1]

This whole-file predicate rejects a valid first fuzzy edit when new_string already occurs elsewhere. For example, production may already contain status = enabled while a whitespace-drifted staging status = disabled should be changed to the same value; current main matches staging through whitespace_normalized, but this condition returns early. Please scope the check to the selected candidate rather than global content presence.