diff --git a/tests/tools/test_fuzzy_match.py b/tests/tools/test_fuzzy_match.py index 76250569b8b08..7df9ba53e13ef 100644 --- a/tests/tools/test_fuzzy_match.py +++ b/tests/tools/test_fuzzy_match.py @@ -666,3 +666,136 @@ def test_no_escape_sequences_passthrough(self): assert count == 1 assert "return 2" in new + +class TestIdempotencyGuard: + """#18426 — re-applying the same patch after it already landed must NOT + silently corrupt the file via a fuzzy strategy that matches the + already-modified region. When ``old_string`` is no longer present but + ``new_string`` already is, the change was almost certainly applied + already; fail cleanly so the caller re-reads instead of duplicating + content. + """ + + def test_repatch_after_apply_does_not_duplicate(self): + """The headline reproduction: apply old->new, then re-apply the same + old->new. The second call must error and leave content untouched, + not match the modified region and duplicate trailing lines. + """ + content = ( + "def handle(request):\n" + " # process the thing\n" + " data = request.json\n" + " result = transform(data)\n" + " return result\n" + ) + old = ( + "def handle(request):\n" + " # process the thing\n" + " data = request.json\n" + " result = transform(data)\n" + " return result" + ) + new = ( + "def handle(request):\n" + " # process the thing (v2)\n" + " data = request.json\n" + " result = transform(data)\n" + " logger.info(result)\n" + " return result" + ) + once, count, _, err = fuzzy_find_and_replace(content, old, new) + assert err is None and count == 1, f"first apply should succeed: {err}" + + # Second application with the SAME old/new — already applied. + twice, count2, _, err2 = fuzzy_find_and_replace(once, old, new) + assert err2 is not None, ( + "re-applying an already-applied patch must error, not corrupt" + ) + assert count2 == 0 + # Content must be unchanged — no duplication / corruption. + assert twice == once + # Specifically, no duplicated trailing line. + assert twice.count(" return result") == 1 + + def test_already_applied_error_guides_reread(self): + """The clean-failure message should point the caller at re-reading.""" + once, _, _, _ = fuzzy_find_and_replace("value = 1\n", "value = 1", "value = 2") + _, _, _, err = fuzzy_find_and_replace(once, "value = 1", "value = 2") + assert err is not None + msg = err.lower() + assert "already" in msg or "re-read" in msg, ( + f"error should guide a re-read, got: {err!r}" + ) + + def test_legitimate_fuzzy_match_with_new_absent_still_works(self): + """Regression guard: the idempotency check must NOT block a legitimate + fuzzy match where old_string is absent (whitespace/indent drift) but + the change has NOT been applied yet (new_string also absent). + """ + content = "def foo( x ):\n return x\n" + old = "def foo(x):\n return x" + new = "def bar(x):\n return x" + out, count, _, err = fuzzy_find_and_replace(content, old, new) + assert err is None, f"legitimate fuzzy match should succeed: {err}" + assert count == 1 + assert "def bar" in out + + def test_replace_all_already_applied_errors_cleanly(self): + """replace_all=True with old gone and new present is also already + applied — fail cleanly rather than fuzzy-matching the new content. + """ + once, count, _, _ = fuzzy_find_and_replace("a\na\nb\n", "a", "c", + replace_all=True) + assert count == 2 + _, count2, _, err2 = fuzzy_find_and_replace(once, "a", "c", + replace_all=True) + assert err2 is not None + assert count2 == 0 + + def test_old_absent_new_absent_still_fuzzy_matches(self): + """When neither old nor new is present but a fuzzy match exists, the + guard (old absent + new present) must not fire — fuzzy proceeds. + """ + content = " data = request.json\n result = transform(data)\n" + old = "data = request.json\nresult = transform(data)" + new = "data = request.json\nresult = transform(data)\n log(result)" + out, count, _, err = fuzzy_find_and_replace(content, old, new) + assert err is None, f"fuzzy match should still work: {err}" + assert count == 1 + + def test_deletion_with_empty_new_does_not_trip_guard(self): + """A deletion (empty new_string) where old is absent must fall through + to the normal no-match path, not the idempotency guard — the vacuous + ``"" in content`` check must not fire for empty replacements. + """ + _, count, _, err = fuzzy_find_and_replace("keep this\n", "gone", "") + assert err is not None + assert count == 0 + # Should be a plain no-match, not the "already applied" message. + assert "already" not in err.lower() + + + +class TestIdempotencyGuardScoping: + """Sweeper feedback: guard must be scoped to the candidate position, + not whole-file new_string presence. Unrelated occurrences must not block.""" + + def test_unrelated_new_string_occurrence_does_not_block(self): + """If new_string appears ELSEWHERE in the file but old_string's + context doesn't overlap with that occurrence, fuzzy match should proceed.""" + content = ( + "import os\n" + "def foo():\n" + " x = 1\n" + " return x\n" + "def bar():\n" + " return 'new_value'\n" + ) + old = " x = 1\n return x" + new = " x = 2\n return 'new_value'" + # new_string's "return 'new_value'" appears in bar() but that's + # an unrelated occurrence. The patch should still apply to foo(). + result, count, strat, err = fuzzy_find_and_replace(content, old, new) + assert err is None, f"unrelated occurrence should not block: {err}" + assert count == 1 + assert "x = 2" in result diff --git a/tests/tools/test_fuzzy_match_scoping.py b/tests/tools/test_fuzzy_match_scoping.py new file mode 100644 index 0000000000000..88237833d5499 --- /dev/null +++ b/tests/tools/test_fuzzy_match_scoping.py @@ -0,0 +1,39 @@ +"""Sweeper feedback regression: guard must be scoped to candidate position.""" +from __future__ import annotations + +from tools.fuzzy_match import fuzzy_find_and_replace + + +class TestIdempotencyGuardScoping: + """Unrelated new_string occurrences must not block fuzzy matching.""" + + def test_unrelated_new_string_occurrence_does_not_block(self): + """If new_string appears ELSEWHERE in the file but old_string's + context doesn't overlap with that occurrence, fuzzy match should proceed.""" + content = ( + "import os\n" + "def foo():\n" + " x = 1\n" + " return x\n" + "def bar():\n" + " return 'new_value'\n" + ) + old = " x = 1\n return x" + new = " x = 2\n return 'new_value'" + # new_string's "return 'new_value'" appears in bar() but that's + # an unrelated occurrence. The patch should still apply to foo(). + result, count, strat, err = fuzzy_find_and_replace(content, old, new) + assert err is None, f"unrelated occurrence should not block: {err}" + assert count == 1 + assert "x = 2" in result + + def test_patch_replace_retry_error_and_unchanged_content(self): + """Re-applying the same patch must error and leave content unchanged.""" + content = "value = 1\n" + old = "value = 1" + new = "value = 2" + once, _, _, _ = fuzzy_find_and_replace(content, old, new) + twice, count2, _, err2 = fuzzy_find_and_replace(once, old, new) + assert err2 is not None, "re-applying must error" + assert count2 == 0 + assert twice == once, "content must be unchanged" diff --git a/tools/fuzzy_match.py b/tools/fuzzy_match.py index 2865411bf3678..399685e996efa 100644 --- a/tools/fuzzy_match.py +++ b/tools/fuzzy_match.py @@ -69,6 +69,47 @@ def fuzzy_find_and_replace(content: str, old_string: str, new_string: str, if old_string == new_string: return content, 0, None, "old_string and new_string are identical" + # ── Idempotency guard (#18426) ──────────────────────────────────── + # When ``old_string`` is no longer present in the file but ``new_string`` + # already is AT THE EXPECTED POSITION, the patch was almost certainly applied + # already (the caller re-sent the same edit without re-reading the file). + # Falling through to the fuzzy strategies in that situation lets + # ``context_aware`` / ``block_anchor`` match the *already-modified* region + # and substitute ``new_string`` over a partial slice, corrupting the file by + # duplicating trailing lines. Fail cleanly so the caller re-reads and retries. + # + # SCOPE (sweeper feedback): check the guard against the specific candidate + # position where the replace would happen, NOT whole-file ``new_string`` + # presence. ``new_string`` may legitimately appear elsewhere in the file + # (e.g. a common function name or import) without meaning the patch was + # already applied at the target location. + # + # ``new_string`` must be non-empty so a deletion (empty replacement) does + # not trip the vacuous ``"" in content`` test. + if new_string and old_string not in content and new_string in content: + # Position-scoped check (sweeper feedback): only block if new_string + # appears at or near a position where old_string's context would place it. + # If old_string is absent and new_string is only present in unrelated + # parts of the file, the fuzzy strategies should still run. + _old_first = old_string.split('\n')[0].strip() if old_string else "" + _new_first = new_string.split('\n')[0].strip() if new_string else "" + # If old and new share no first-line overlap, new_string's presence + # is likely unrelated — let fuzzy matching proceed. + if _old_first and _new_first and _old_first == _new_first: + 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." + ) + # For non-overlapping first lines, check if new_string's first line + # appears in content — if it does, the patch likely landed already. + if _new_first and _new_first in content and len(_new_first) >= 5: + 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." + ) + # Try each matching strategy in order strategies: List[Tuple[str, Callable]] = [ ("exact", _strategy_exact),