diff --git a/CHANGELOG.md b/CHANGELOG.md index ea8e4c5..06981de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,17 @@ breaking changes may land in a minor release. ### Fixed +- **An abandoned patch-restore no longer smuggles its files into the corrected story's commit.** + Re-arming a story whose previous re-drive had already applied a restore patch snapshotted that + patch's new (untracked) files as _pre-existing_, so every later rollback preserved them and + `finalize_commit`'s `add -A` swept the abandoned attempt into the corrected commit. The re-arm now + parses the old latch (`verify.patch_new_files`) and subtracts its creations from the refreshed + baseline snapshot — the re-drive's own reset then removes them. Best-effort: a missing or + unreadable patch degrades to the old behavior instead of failing the resolve. Commits the + escalated attempt left below the advanced baseline can't be reverted mechanically (the resolve + session's own commits share that range), so they are journaled and echoed to stderr for the human + to classify. New journal events: `stale-restore-excluded` / `-unparseable` / `-commits`. + (closes #90) - **Baseline-era untracked residue no longer vacuously satisfies the proof-of-work gate.** `has_changes_since` counted every untracked file. After an intent-gap halt the saved patch is untracked residue under the artifact dirs every reset deliberately protects, so a from-scratch diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index abda40a..3683d02 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -897,6 +897,36 @@ def _resolve_restore_patch( return str(patch), None +def _echo_stale_restore(run_dir: Path, seen_entries: int) -> None: + """Surface the `stale-restore-*` events a just-completed re-arm journaled about + the restore attempt it abandoned (runs._stale_restore_residue). The commits + variant is the one the human must act on — nothing else will.""" + for entry in Journal(run_dir).entries()[seen_entries:]: + kind = entry.get("kind", "") + if kind == "stale-restore-excluded": + files = ", ".join(entry.get("files", [])) + print( + f"note: excluded the abandoned restore's new files from the " + f"re-drive baseline: {files}", + file=sys.stderr, + ) + elif kind == "stale-restore-unparseable": + print( + f"warning: could not read the abandoned restore patch " + f"({entry.get('patch', '?')}) — its new files may be swept into the " + "next commit; check `git status` before resuming", + file=sys.stderr, + ) + elif kind == "stale-restore-commits": + n = len(entry.get("commits", [])) + print( + f"warning: {n} commit(s) sit below the re-drive's new baseline " + f"({entry.get('old_baseline', '?')[:12]}..) — if any came from the " + "abandoned attempt rather than your resolve, revert them now", + file=sys.stderr, + ) + + def cmd_resolve(args: argparse.Namespace) -> int: from .model import PAUSE_ESCALATION, Phase @@ -993,11 +1023,13 @@ def cmd_resolve(args: argparse.Namespace) -> int: if args.resume is None and not _confirm(f"re-arm {story_key} and resume run {args.run_id}?"): print("cancelled — run is still paused at the escalation") return 0 + seen_entries = len(Journal(run_dir).entries()) try: runs.rearm_escalation(run_dir, story_key, restore_patch=restore_patch) except runs.RearmError as e: print(f"error: {e}", file=sys.stderr) return 1 + _echo_stale_restore(run_dir, seen_entries) print( f"re-armed {story_key}" + (" (restoring the attempted change for review)" if restore_patch else "") diff --git a/src/bmad_loop/runs.py b/src/bmad_loop/runs.py index 0000140..65ab45e 100644 --- a/src/bmad_loop/runs.py +++ b/src/bmad_loop/runs.py @@ -593,6 +593,10 @@ def rearm_escalation( ) journal = Journal(run_dir) + # Read before the unconditional overwrite below: they describe the restore + # attempt this re-arm is abandoning, and the residue block needs both. + old_latch = task.restore_patch + old_baseline = task.baseline_commit # deliberate reset, not a normal state-machine transition (mirrors # engine._finish_inflight): a clean re-attempt against the corrected spec. task.phase = Phase.PENDING @@ -650,6 +654,21 @@ def rearm_escalation( f"(it must be readable UTF-8), then re-run resolve" ) from e + # A previous restore latch is being replaced (or re-latched onto the same + # patch): the abandoned attempt applied that patch, so its NEW files sit + # untracked in the tree right now. The refresh below would capture them as + # "pre-existing" — after which every rollback preserves them and + # finalize_commit's `add -A` sweeps the abandoned attempt into the corrected + # story's commit. Subtract them instead (issue #90). + # + # Runs after the spec block for the same reason the refresh does (a cleared + # sentinel must not be snapshotted), and before it because it feeds it. + # Nothing is deleted here: the re-drive's reset (verify.safe_rollback) removes + # whatever the refreshed snapshot no longer blesses, at the right moment. + stale_residue = _stale_restore_residue( + Path(state.project), journal, key, old_latch, old_baseline + ) + # Advance the attempt baseline to the project's current HEAD and refresh the # untracked snapshot: whatever the human-driven resolve session left on the # branch (a committed fixture, a corrected ledger, ...) is authorized input @@ -668,7 +687,7 @@ def rearm_escalation( try: repo = Path(state.project) head = verify.rev_parse_head(repo) - untracked = sorted(verify.untracked_files(repo)) + untracked = sorted(verify.untracked_files(repo) - stale_residue) task.baseline_commit = head task.baseline_untracked = untracked except Exception: # noqa: BLE001 # nosec B110 - best-effort git read, must not fail re-arm @@ -703,6 +722,77 @@ def rearm_escalation( return key +def _stale_restore_residue( + repo: Path, + journal: Journal, + story_key: str, + old_latch: str | None, + old_baseline: str | None, +) -> set[str]: + """The untracked files an abandoned patch-restore attempt left in the tree — + to be subtracted from the re-arm's refreshed `baseline_untracked` (issue #90). + + Empty when no restore was latched. Deliberately *not* a `git apply -R`: the + re-drive's own reset already reverts the patch's tracked hunks, an `apply -R` + fails outright on any drift the resolve session introduced, and it misbehaves + on the committed variant below. Only the patch's new files are durable + contamination, and naming them is enough — `verify.safe_rollback` deletes + whatever the refreshed snapshot stops blessing. + + Also journals (warn-only) the commits sitting between the OLD baseline and the + new one: a commit the escalated re-drive session made now becomes the next + re-drive's permanent starting point, and no reset revisits it. It is not + mechanically reversible — the resolve session's own blessed commits live in the + same range and reverting those would claw back the human's resolution — so the + human is the classifier. `bmad-loop resolve` echoes these to stderr. + + Best-effort throughout: a deleted or unreadable patch, a non-repo project, a + bad old baseline — none may wedge a resolve. Every failure degrades to the + pre-#90 behavior and says so in the journal. + """ + if not old_latch: + return set() + patch_path = Path(old_latch) + if not patch_path.is_absolute(): + patch_path = repo / patch_path + + residue: set[str] = set() + try: + residue = verify.patch_new_files(patch_path) + except (OSError, UnicodeDecodeError) as e: + # degrade to the pre-#90 snapshot rather than wedge the resolve + journal.append( + "stale-restore-unparseable", + story_key=story_key, + patch=str(patch_path), + error=f"{e.__class__.__name__}: {e}", + ) + else: + if residue: + journal.append( + "stale-restore-excluded", + story_key=story_key, + patch=str(patch_path), + files=sorted(residue), + ) + + # Independent of the parse above — an unreadable patch must not also cost the + # human the only notice they get about the committed variant. + if old_baseline: + try: + shas = verify.commits_above(repo, old_baseline) + except Exception: # noqa: BLE001 # nosec B110 - warn-only, must not fail re-arm + shas = [] + if shas: + journal.append( + "stale-restore-commits", + story_key=story_key, + old_baseline=old_baseline, + commits=shas, + ) + return residue + + def _sentinel_condition(spec_path: Path, story_key: str) -> str | None: """The blocking condition (``unresolved`` / ``ambiguous``) iff ``spec_path`` is a fixed-slug pre-planning-halt sentinel for ``story_key``, else None.""" diff --git a/src/bmad_loop/verify.py b/src/bmad_loop/verify.py index ecd2605..a89c1b1 100644 --- a/src/bmad_loop/verify.py +++ b/src/bmad_loop/verify.py @@ -26,6 +26,11 @@ GIT_TIMEOUT_S = 120 COMMAND_TIMEOUT_S = 30 * 60 +# How git's own diff format names the absent side of a creation/deletion. A +# protocol token git emits verbatim on every platform, Windows included — never +# opened, never joined onto. Only `patch_new_files` reads it. +_DIFF_ABSENT = "/dev/null" # portability: git diff-format token, not a real path + # result.json `workflow` value for the dev pass. A machine contract: the # orchestrator forges this value in `devcontract` when synthesizing the dev # result from the spec the bmad-dev-auto session leaves on disk; a mismatch @@ -1550,6 +1555,53 @@ def apply_patch(repo: Path, patch_path: Path) -> None: raise GitError(f"git apply {patch_path} failed: {out}") +def patch_new_files(patch_path: Path) -> set[str]: + """Repo-relative posix paths the saved patch *creates* — the untracked residue + an `apply_patch` leaves behind (see `runs.rearm_escalation`). + + Text-parse, not `git apply --numstat`: the caller runs after the tree has moved + on, so the patch may no longer apply, and a creation list must still come back. + Within each `diff --git` block, an old-side `---` header naming `_DIFF_ABSENT` + marks a creation, and the `+++ /` after it names the file. The + prefix is stripped by mirroring what `apply_patch`'s plain `git apply` (default + -p1) did when it laid the residue down: drop the first path component whatever + it is — `b/` standard, `w/`/`i/`/`c/` under diff.mnemonicPrefix, `2/` from + --no-index. A target -p1 cannot strip (no `/`, e.g. --no-prefix output) is + skipped: that apply failed outright, so no residue exists. Deletions (the + absent token on the *new* side) are never returned — the caller feeds this to an + *exclusion* set, and excluding a path the human later re-created would make the + next rollback delete their file. For the same reason every ambiguous entry is + skipped rather than guessed: quoted paths (`+++ "b/wéird"`, core.quotePath), + renames, and non-`git diff` unified diffs with no `diff --git` header yield fewer + results, never wrong ones. Under-reporting degrades to the pre-#90 behavior; + over-reporting deletes user data. + + Raises OSError / UnicodeDecodeError when the patch cannot be read; the caller + decides (rearm treats it as best-effort and journals `stale-restore-unparseable`). + """ + new_files: set[str] = set() + in_hunk = False # past the first `@@`, a `--- x` line is content, not a header + creating = False + for line in patch_path.read_text(encoding="utf-8").splitlines(): + if line.startswith("diff --git "): + in_hunk = creating = False + elif line.startswith("@@"): + in_hunk = True + elif in_hunk: + continue + elif line.startswith("--- "): + creating = line[4:].strip() == _DIFF_ABSENT + elif line.startswith("+++ ") and creating: + creating = False + target = line[4:].split("\t", 1)[0].strip() + if target == _DIFF_ABSENT or target.startswith('"') or "/" not in target: + continue # delete-then-create pair, quoted path, or un-strippable target + rel = target.split("/", 1)[1] # mirror `git apply`'s default -p1 + if rel: + new_files.add(rel) + return new_files + + def commit_paths(repo: Path, message: str, paths: list[Path]) -> str | None: """Commit exactly `paths` (and nothing else), leaving any unrelated working or staged changes untouched. Unlike commit_story's `add -A`, this is safe to diff --git a/tests/test_cli.py b/tests/test_cli.py index 1a4c3a2..0ca5667 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -929,6 +929,36 @@ def test_resolve_no_interactive_rearms_and_resumes(tmp_path, monkeypatch, capsys assert "ready-for-dev" in spec.read_text() +def test_resolve_echoes_this_rearms_stale_restore_events(tmp_path, monkeypatch, capsys): + """#90's journal entries reach the operator. The commits variant is warn-only — + stderr is the only place it ever surfaces. Entries from *earlier* re-arms are + already-acted-on history and must not be replayed.""" + from bmad_loop import runs + from bmad_loop.journal import Journal + + run_dir = _escalated_run(tmp_path, "r1") + Journal(run_dir).append("stale-restore-excluded", story_key="s1", files=["FROM-LAST-TIME.txt"]) + + def fake_rearm(rd, key, *, restore_patch=None): + journal = Journal(rd) + journal.append("stale-restore-excluded", story_key=key, patch="a.patch", files=["new.txt"]) + journal.append("stale-restore-unparseable", story_key=key, patch="b.patch", error="OSErr") + journal.append("stale-restore-commits", story_key=key, old_baseline="f" * 40, commits=["c"]) + return key + + monkeypatch.setattr(runs, "rearm_escalation", fake_rearm) + monkeypatch.setattr(cli, "_resume_paused_run", lambda proj, rd: 0) + assert ( + cli.main(["resolve", "--project", str(tmp_path), "r1", "--no-interactive", "--resume"]) == 0 + ) + + err = capsys.readouterr().err + assert "excluded the abandoned restore's new files from the re-drive baseline: new.txt" in err + assert "could not read the abandoned restore patch (b.patch)" in err + assert "1 commit(s) sit below the re-drive's new baseline (ffffffffffff..)" in err + assert "FROM-LAST-TIME.txt" not in err + + def test_resolve_interactive_runs_session_then_rearms(tmp_path, monkeypatch): from bmad_loop import resolve from bmad_loop.journal import load_state diff --git a/tests/test_portability_guard.py b/tests/test_portability_guard.py index 3c6268c..6025eac 100644 --- a/tests/test_portability_guard.py +++ b/tests/test_portability_guard.py @@ -31,13 +31,16 @@ # *are* the sanctioned spot (their module docstrings say so). TMUX_BACKENDS = {"adapters/tmux_base.py", "adapters/tmux_backend.py"} -# Platform-guarded files that may name a bare POSIX path, each on a line carrying -# a `# portability:` ack (and guarded by a sys.platform branch). process_host.py's -# Linux identity reader walks `/proc//stat`. +# Files that may name a bare POSIX path, each on a line carrying a `# portability:` +# ack. process_host.py's Linux identity reader walks `/proc//stat` behind a +# sys.platform branch; the Unity teardown scripts are POSIX-only. verify.py is the +# one non-platform case: git's *diff format* spells an absent file `/dev/null` on +# every platform, so `patch_new_files` compares against it as a protocol token. PATH_ALLOW = { "data/plugins/unity/unity_cleanup.py", "data/plugins/unity/unity_teardown.py", "process_host.py", + "verify.py", } # The two detach helpers that legitimately request POSIX `start_new_session`. diff --git a/tests/test_runs.py b/tests/test_runs.py index 8ce2351..cef6346 100644 --- a/tests/test_runs.py +++ b/tests/test_runs.py @@ -7,6 +7,7 @@ import tarfile import pytest +from conftest import git from bmad_loop import runs from bmad_loop.adapters import tmux_base @@ -530,11 +531,23 @@ def test_delete_run(tmp_path): assert not run_dir.exists() -def _escalated_run(tmp_path, spec_text, *, restore_patch_stale=None): +def _escalated_run(tmp_path, spec_text, *, restore_patch_stale=None, git_project=False): + """`git_project=True` makes `state.project` a real repo with the spec committed, + so rearm's baseline snapshot refresh actually runs — in a bare tmp_path its + best-effort `except` swallows every git call and the refresh silently no-ops.""" from bmad_loop.model import PAUSE_ESCALATION, Phase, StoryTask spec = tmp_path / "spec.md" spec.write_text(spec_text, encoding="utf-8") + baseline = None + if git_project: + (tmp_path / ".gitignore").write_text(".bmad-loop/\n") # keep run state out of the snapshot + git(tmp_path, "init", "-q", "-b", "main") + git(tmp_path, "config", "user.email", "test@test") + git(tmp_path, "config", "user.name", "test") + git(tmp_path, "add", "-A") + git(tmp_path, "commit", "-q", "-m", "initial") + baseline = git(tmp_path, "rev-parse", "HEAD") task = StoryTask( story_key="1-1-a", epic=1, @@ -542,6 +555,7 @@ def _escalated_run(tmp_path, spec_text, *, restore_patch_stale=None): attempt=2, spec_file=str(spec), restore_patch=restore_patch_stale, + baseline_commit=baseline, ) run_dir = _make_state_run( tmp_path, @@ -593,6 +607,121 @@ def test_rearm_plain_mode_sets_ready_for_dev_and_clears_stale_latch(tmp_path): assert entry["restore"] is False +# --------------------------------------------- #90: abandoned restore-latch residue + + +def _stale_restore_tree(tmp_path, *, latch="artifacts/attempt.patch"): + """An escalation whose latched restore already applied: `newfile.txt` is the + patch's untracked creation, `human.txt` is the resolve session's own file.""" + run_dir, spec = _escalated_run( + tmp_path, _SPEC_WITH_ARR, restore_patch_stale=latch, git_project=True + ) + patch = tmp_path / "artifacts" / "attempt.patch" + patch.parent.mkdir(parents=True, exist_ok=True) + patch.write_text( + "diff --git a/newfile.txt b/newfile.txt\n" + "new file mode 100644\n" + "index 0000000..1111111\n" + "--- /dev/null\n" + "+++ b/newfile.txt\n" + "@@ -0,0 +1 @@\n" + "+from the abandoned attempt\n", + encoding="utf-8", + ) + (tmp_path / "newfile.txt").write_text("from the abandoned attempt\n") # the applied residue + (tmp_path / "human.txt").write_text("from the resolve session\n") + return run_dir, spec, patch + + +def _kinds(run_dir, prefix="stale-restore-"): + from bmad_loop.journal import Journal + + return [e for e in Journal(run_dir).entries() if e["kind"].startswith(prefix)] + + +def test_rearm_excludes_stale_restore_residue_from_baseline_snapshot(tmp_path): + """The abandoned attempt's applied new files must NOT be blessed as + pre-existing, or finalize_commit's `add -A` sweeps them into the corrected + story's commit. The resolve session's own untracked file still is.""" + run_dir, _spec, patch = _stale_restore_tree(tmp_path) + + runs.rearm_escalation(run_dir) # from-scratch re-arm replaces the latch + + task = load_state(run_dir).tasks["1-1-a"] + assert "human.txt" in task.baseline_untracked + assert "newfile.txt" not in task.baseline_untracked + assert (tmp_path / "newfile.txt").exists() # rearm deletes nothing; the re-drive's reset does + excluded = _kinds(run_dir, "stale-restore-excluded") + assert len(excluded) == 1 + assert excluded[0]["files"] == ["newfile.txt"] + assert excluded[0]["patch"] == str(patch) + + +def test_rearm_re_latching_the_same_patch_still_excludes_its_residue(tmp_path): + """Re-arming a restore onto the same patch: the first application's files are + still residue (and `git apply` would otherwise fail with 'already exists').""" + run_dir, _spec, _patch = _stale_restore_tree(tmp_path) + + runs.rearm_escalation(run_dir, restore_patch="artifacts/attempt.patch") + + task = load_state(run_dir).tasks["1-1-a"] + assert task.restore_patch == "artifacts/attempt.patch" + assert "human.txt" in task.baseline_untracked + assert "newfile.txt" not in task.baseline_untracked + assert _kinds(run_dir, "stale-restore-excluded") + + +def test_rearm_missing_stale_patch_degrades_loudly_without_raising(tmp_path): + """A deleted patch file must never wedge resolve: journal the degrade and fall + back to the pre-#90 snapshot (everything untracked counts as pre-existing).""" + run_dir, _spec, patch = _stale_restore_tree(tmp_path) + patch.unlink() + (tmp_path / "committed.txt").write_text("from the escalated attempt\n") + git(tmp_path, "add", "committed.txt") + git(tmp_path, "commit", "-q", "-m", "attempt commit") + + runs.rearm_escalation(run_dir) # must not raise RearmError + + task = load_state(run_dir).tasks["1-1-a"] + assert {"human.txt", "newfile.txt"} <= set(task.baseline_untracked) # full snapshot + unparseable = _kinds(run_dir, "stale-restore-unparseable") + assert len(unparseable) == 1 + assert "FileNotFoundError" in unparseable[0]["error"] + assert not _kinds(run_dir, "stale-restore-excluded") + # the unreadable patch must not also cost the human the commits warning + assert _kinds(run_dir, "stale-restore-commits") + + +def test_rearm_without_a_stale_latch_journals_no_stale_restore_events(tmp_path): + run_dir, _spec = _escalated_run(tmp_path, _SPEC_WITH_ARR, git_project=True) + (tmp_path / "human.txt").write_text("from the resolve session\n") + + runs.rearm_escalation(run_dir, restore_patch="artifacts/attempt.patch") + + assert "human.txt" in load_state(run_dir).tasks["1-1-a"].baseline_untracked + assert _kinds(run_dir) == [] + + +def test_rearm_warns_about_commits_below_the_refreshed_baseline(tmp_path): + """The worse variant: commits made above the OLD baseline become the re-drive's + permanent starting point. Warn-only — a mechanical revert would claw back the + resolve session's own blessed commits, which live in the same range.""" + run_dir, _spec, _patch = _stale_restore_tree(tmp_path) + (tmp_path / "committed.txt").write_text("from the escalated attempt\n") + git(tmp_path, "add", "committed.txt") + git(tmp_path, "commit", "-q", "-m", "attempt commit") + old_baseline = load_state(run_dir).tasks["1-1-a"].baseline_commit + + runs.rearm_escalation(run_dir) + + task = load_state(run_dir).tasks["1-1-a"] + assert task.baseline_commit != old_baseline # baseline advanced past the commit + warned = _kinds(run_dir, "stale-restore-commits") + assert len(warned) == 1 + assert warned[0]["old_baseline"] == old_baseline + assert warned[0]["commits"] == [git(tmp_path, "rev-parse", "HEAD")] + + def test_archive_run(tmp_path): run_dir = _make_state_run(tmp_path, "20260611-100000-aaaa") (run_dir / "journal.jsonl").write_text('{"kind":"x"}\n') diff --git a/tests/test_verify.py b/tests/test_verify.py index 6afeeb1..92571dd 100644 --- a/tests/test_verify.py +++ b/tests/test_verify.py @@ -1031,6 +1031,131 @@ def test_apply_patch_conflict_raises(project): verify.apply_patch(repo, patch) +def _saved_patch(project, name="attempt.patch"): + """Capture the working tree as a restore patch, the way the skill saves one.""" + repo = project.project + git(repo, "add", "-AN") # intent-to-add so new files appear in `git diff HEAD` + patch = project.implementation_artifacts / name + patch.write_text(git(repo, "diff", "HEAD") + "\n", encoding="utf-8") + return patch + + +def test_patch_new_files_names_created_files(project): + repo = project.project + (repo / "new_module.py").write_text("print('hi')\n") + (repo / "pkg").mkdir() + (repo / "pkg" / "deep.txt").write_text("nested\n") + assert verify.patch_new_files(_saved_patch(project)) == {"new_module.py", "pkg/deep.txt"} + + +def test_patch_new_files_ignores_modifications(project): + repo = project.project + (repo / "src.txt").write_text("original\nedited\n") + assert verify.patch_new_files(_saved_patch(project)) == set() + + +def test_patch_new_files_never_returns_deletions(project): + """A deleted file must not land in the exclusion set: the human may re-create + that path, and excluding it would make the next rollback delete their copy.""" + repo = project.project + git(repo, "rm", "-q", "src.txt") + assert verify.patch_new_files(_saved_patch(project)) == set() + + +def test_patch_new_files_multi_file_patch(project): + """One patch carrying a creation, a modification and a deletion — only the + creation comes back.""" + repo = project.project + (repo / "created.txt").write_text("new\n") + (repo / ".gitignore").write_text("*.log\n") + git(repo, "rm", "-q", "src.txt") + assert verify.patch_new_files(_saved_patch(project)) == {"created.txt"} + + +def test_patch_new_files_ignores_header_lookalikes_in_hunk_bodies(project): + """Hunk *content* that reads like a file header (a removed `-- /dev/null` line + renders as `--- /dev/null`) must not be mistaken for a creation.""" + patch = project.implementation_artifacts / "tricky.patch" + patch.write_text( + "diff --git a/real.txt b/real.txt\n" + "new file mode 100644\n" + "index 0000000..1111111\n" + "--- /dev/null\n" + "+++ b/real.txt\n" + "@@ -0,0 +1 @@\n" + "+hello\n" + "diff --git a/mod.txt b/mod.txt\n" + "index 1111111..2222222 100644\n" + "--- a/mod.txt\n" + "+++ b/mod.txt\n" + "@@ -1 +1 @@\n" + "--- /dev/null\n" # a removed line whose content is `-- /dev/null` + "+++ b/evil.txt\n" # an added line whose content is `++ b/evil.txt` + "\\ No newline at end of file\n", + encoding="utf-8", + ) + assert verify.patch_new_files(patch) == {"real.txt"} + + +def test_patch_new_files_skips_quoted_paths(project): + """core.quotePath output is skipped rather than guessed — under-reporting is + safe, a wrong path would get a user file deleted.""" + patch = project.implementation_artifacts / "quoted.patch" + patch.write_text( + 'diff --git "a/w\\303\\251ird.txt" "b/w\\303\\251ird.txt"\n' + "new file mode 100644\n" + "--- /dev/null\n" + '+++ "b/w\\303\\251ird.txt"\n' + "@@ -0,0 +1 @@\n" + "+x\n", + encoding="utf-8", + ) + assert verify.patch_new_files(patch) == set() + + +def test_patch_new_files_strips_mnemonic_prefixes(project): + """`diff.mnemonicPrefix=true` in the user's config makes `git diff HEAD` emit + `c/`/`w/` instead of `a/`/`b/`. `apply_patch`'s plain `git apply` (-p1) strips + the first component whatever it is, so the parser must mirror that or the + recorded residue is `w/` and the exclusion silently no-ops.""" + repo = project.project + (repo / "new_module.py").write_text("print('hi')\n") + (repo / "pkg").mkdir() + (repo / "pkg" / "deep.txt").write_text("nested\n") + git(repo, "add", "-AN") + patch = project.implementation_artifacts / "mnemonic.patch" + patch.write_text( + git(repo, "-c", "diff.mnemonicPrefix=true", "diff", "HEAD") + "\n", encoding="utf-8" + ) + assert "+++ w/new_module.py" in patch.read_text(encoding="utf-8") # fixture sanity + assert verify.patch_new_files(patch) == {"new_module.py", "pkg/deep.txt"} + + +def test_patch_new_files_skips_unstrippable_targets(project): + """A prefixless single-component target (`git diff --no-prefix`) cannot survive + `git apply`'s -p1 strip — the apply would have failed, so no residue can exist. + Recording it verbatim could exclude (and later delete) a same-named file the + human created; skip it instead.""" + patch = project.implementation_artifacts / "noprefix.patch" + patch.write_text( + "diff --git newfile.txt newfile.txt\n" + "new file mode 100644\n" + "index 0000000..1111111\n" + "--- /dev/null\n" + "+++ newfile.txt\n" + "@@ -0,0 +1 @@\n" + "+x\n", + encoding="utf-8", + ) + assert verify.patch_new_files(patch) == set() + + +def test_patch_new_files_missing_patch_raises_oserror(project): + """The caller (rearm) turns this into a journaled best-effort degrade.""" + with pytest.raises(OSError): + verify.patch_new_files(project.implementation_artifacts / "gone.patch") + + def test_read_frontmatter_tolerates_garbage(project): p = project.project / "x.md" p.write_text("no frontmatter here")