From fa1377ba55db79ac214fa6cf087ebb13e4e51264 Mon Sep 17 00:00:00 2001 From: EMRG Evolution Date: Wed, 16 Sep 2026 09:38:57 +0800 Subject: [PATCH 1/4] emrg: the daemon recovers reconstructible working-tree dirt itself The dirty-tree downgrade (#979) was re-keyed on loss by #1237, but its exit was still manual: a reconstructible tree kept its tier and was told to run scripts/recover-worktree.py. A guard whose only exit is a command a human must be present to run strands the tree -- the shape of the measured 33-cycle loss, in which the tier that blocked the cycle also blocked the repair of the thing that blocked it. Host directive 2026-09-16: dirt must be recovered from, not merely detected and reported. - `scheduler._recover_dirty_tree_sync` converges a reconstructible tree reversibly: `git stash push --include-untracked`, `HEAD` asserted unmoved, a receipt written into the git state dir. It re-asks the criterion itself when no measured verdict is supplied, so it refuses work that exists nowhere else even if a caller forgets to ask first -- the action cannot discard what the guard protects. - `_effective_sandbox` calls it on the reconstructible branch, so a cycle begins on a clean tree with nothing asked of the host, and keeps its configured tier. Unique dirt is still forced read-only and left untouched: moving a host's unsaved work aside is a decision for whoever wrote it. - `scripts/recover-worktree.py` now owns no policy -- it calls the daemon's implementation of both the criterion and the action, so a hand diagnosis and the tier decision cannot drift apart. It is the manual route for a tree no cycle is about to touch. - DEVELOPMENT.md: the self-recovery contract, and the one thing it does not do. Mutation-tested in both directions: 10 mutants, none survived (7 removing the criterion's clauses and the verdict's use, 3 removing self-recovery, the action's own re-check, and the receipt location). Suite: 2624 passed / 17 skipped against master's 2609 / 16. --- DEVELOPMENT.md | 69 +++++++- emrg/server/scheduler.py | 297 +++++++++++++++++++++++++++++++-- scripts/recover-worktree.py | 156 +++++++++++++++++ tests/test_recover_worktree.py | 287 +++++++++++++++++++++++++++++++ tests/test_scheduler.py | 133 ++++++++++++++- 5 files changed, 915 insertions(+), 27 deletions(-) create mode 100644 scripts/recover-worktree.py create mode 100644 tests/test_recover_worktree.py diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 31f6bb66..0dd94015 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -199,12 +199,59 @@ They're products. EMRG is an experiment in *closing the loop* — the AI improve ### Why is my evolution cycle running read-only? Every cycle gets one bash tier: `read-only`, `workspace-write` (the default), or -`danger-full-access`. **If the task's source repository has any uncommitted -change, the cycle is forced down to `read-only` whatever the configuration -says** — the dirty-tree guard (community issue #979). The probe is literally -"does `git status --porcelain` print anything?", so a single tracked -modification or one untracked scratch file is enough to downgrade the cycle, and -a cycle that leaves scratch files behind can lock the next one. +`danger-full-access`. The tier is decided per cycle and is keyed on **whether +discarding the working tree would lose anything**, not on whether the tree is +dirty (issue #1237). When the task's source repository has uncommitted changes: + +- **nothing exists only there** — every change is already recoverable from a commit + git holds (`HEAD`, or the upstream tip) — so the daemon converges the tree + itself and the cycle keeps its configured tier; +- **something exists only there** — an untracked file, a staged addition, a + conflict, a modification outrunning both, or a commit only on this branch — so + the cycle is forced down to `read-only` whatever the configuration says + (community issue #979), and the paths that caused it are named in the log. + +Why the distinction matters more than it looks: `read-only` refuses the very git +verbs that could clean the tree, so the state that triggered the downgrade also +disabled its only exit. Measured cost of one instance of that loop: **33 +consecutive zero-commit cycles**, whose verified output had to be parked outside +the repository. And the dirt in it was not work at all — of what git reported, the +modified files were byte-identical to upstream's own blobs, i.e. the second bullet +above had been applied where the first was true. + +#### Recovering a tree + +**A cycle recovers itself.** At the start of every cycle the daemon asks the +question above, and when the answer is "nothing exists only here" it converges the +tree itself, before the cycle's first tool call — reversibly (stash, `HEAD` +unmoved, receipt in the git state dir). The exit from a downgrade is therefore the +daemon's own action, not a command a human has to be present to run: a guard whose +only exit is manual strands the tree, which is the shape of the measured 33-cycle +loss above. Nothing is asked of the host. + +The same action is available from a shell, for a tree no cycle is about to touch: + +```bash +uv run --no-sync python3 scripts/recover-worktree.py --repo +uv run --no-sync python3 scripts/recover-worktree.py --repo --apply +``` + +The script owns no policy — it calls the daemon's implementation of both the +criterion and the action, so a diagnosis you run by hand and the tier decision the +daemon makes cannot drift apart. Without `--apply` it only reports. It asks the +same question the guard asks, and then: + +- **refuses, naming the paths, when the work exists nowhere else.** Nothing is + written, and no stash is made. That is the guard working; commit, stash or copy + that content out deliberately and re-run. +- **stashes it when it is reconstructible** (`git stash push --include-untracked`), + leaving the worktree clean and every byte one `git stash pop` away. It never + moves `HEAD`, so it cannot orphan a commit, and the stash makes the action + undoable — which is why an agent is allowed to take it. +- **writes a receipt** into the git state dir (`emrg-recovery-receipt.json`), as + every release of a safety rule requires. Deliberately not beside the tree: a + receipt at `/.emrg/…` re-dirties the tree it just cleaned in any repository + that does not ignore `.emrg/`, which would re-arm the guard on the next cycle. Read-only blocks the destructive shapes the guard recognises: redirects to anything but `/dev/null`, `rm` / `rmdir`, `mv` / `cp` destinations, `truncate` / @@ -216,8 +263,9 @@ It is a static scan, not an OS boundary (`enforcement="partial"`), so a command outside that shape list (`mkdir`, `touch`, an interpreter writing a file) still runs and a downgraded cycle can leave scratch files behind. The decision is per command string, not per statement: one blocked shape anywhere in the call refuses -the whole call. Reads are unaffected, so the problem can still be diagnosed from -inside a downgraded cycle. +the whole call. Reads are unaffected — including the read-only git verbs the +recovery tool's diagnosis uses — so the problem can still be diagnosed from inside +a downgraded cycle; only the repair needs a writable tier. #### Diagnose before you repair @@ -243,7 +291,10 @@ git merge-base --is-ancestor HEAD FETCH_HEAD \ That is the common case in a workspace whose change was merged upstream after the local checkout: the working tree holds master's content while the index still matches the older HEAD, and the "uncommitted work" is already published. -Recover with: +`scripts/recover-worktree.py --apply` clears the dirty state reversibly without +moving `HEAD`; the sequence below is for when you also want to move the branch +onto upstream, which is a decision the tool deliberately leaves to you. Recover +with: ```bash git fetch origin master diff --git a/emrg/server/scheduler.py b/emrg/server/scheduler.py index 470b29ea..08c05d5d 100644 --- a/emrg/server/scheduler.py +++ b/emrg/server/scheduler.py @@ -19,7 +19,7 @@ import os import re import time -from datetime import datetime +from datetime import datetime, timezone from pathlib import Path import yaml @@ -378,17 +378,273 @@ def _is_dirty_tree_sync(source_dir: str) -> bool: return False return bool(out.stdout.strip()) if out.returncode == 0 else False - async def _effective_sandbox(self, dirty: bool | None = None) -> str: + @staticmethod + def _dirty_tree_would_lose_work_sync(source_dir: str) -> tuple[bool, str]: + """Whether discarding the working tree could lose content found nowhere else. + + The question the guard actually needs (host directive 2026-09-16, issue + #1237): *would anything be lost?* — not *is the tree dirty?*. Those differ, + and the difference cost 33 consecutive zero-commit cycles: a tree can be + "dirty" because two files carry bytes that upstream's own commit already + holds, and forcing such a cycle read-only protects nothing while disabling + the git verbs it would need to converge the tree itself. + + Answers ``(True, why)`` when the dirt is **unique** — the caller must then + keep the cycle read-only — and ``(False, why)`` when every listed change is + already recoverable from a commit git holds, so discarding it loses nothing. + Read-only operations only, so it runs under the tier it is deciding about. + + The criterion, per ``git status --porcelain`` entry: + + * ``??`` untracked, a staged addition, a conflict, or a rename — content + that exists only in the index or the worktree ⇒ unique; + * a **deletion** loses nothing (discarding restores the blob from ``HEAD``); + * a **modification** is recoverable when its blob equals ``HEAD``'s or the + upstream tip's for that path — the two places a blob can already live; + * a commit reachable only from this branch would be orphaned by a branch + reset ⇒ unique. + + Fail-closed throughout: an unreadable status, an unreadable file, or no + upstream ref to compare against all answer *unique*, i.e. exactly the + pre-existing read-only behaviour. Only a positive, complete measurement of + "recoverable" ever releases the tier. + """ + import subprocess as _sp # noqa: PLC0415 — local import keeps the module invariant + + def git(*args: str): + return _sp.run( + ["git", "-C", source_dir, *args], + capture_output=True, text=True, timeout=10, + # Paths, not console output — same pinning as the sibling probe. + encoding="utf-8", errors="replace", + ) + + status = git("status", "--porcelain", "--untracked-files=normal") + if status.returncode != 0: + return True, "the working tree state could not be read" + if not status.stdout.strip(): + return False, "the tree is clean" + + # The second place a blob can already live. A repo with no upstream keeps + # only HEAD, which can only make *more* dirt count as unique (safe side). + upstream = None + for ref in ("origin/master", "FETCH_HEAD"): + probe = git("rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}") + if probe.returncode == 0 and probe.stdout.strip(): + upstream = probe.stdout.strip() + break + + unique: list[str] = [] + for line in status.stdout.splitlines(): + if not line.strip(): + continue + code, path = line[:2], line[3:].strip() + if code == "??" or "U" in code or code in ("AA", "DD"): + unique.append(f"{path} exists only in this checkout") + continue + if "A" in code: + unique.append(f"{path} is staged but not committed") + continue + if "R" in code or "C" in code: + unique.append(f"{path} was renamed") + continue + if "D" in code: + continue # a deletion loses nothing: HEAD holds it + blob = git("hash-object", "--", path) + if blob.returncode != 0 or not blob.stdout.strip(): + unique.append(f"{path} could not be read to compare") + continue + digest = blob.stdout.strip() + for ref in filter(None, ("HEAD", upstream)): + have = git("rev-parse", "--verify", "--quiet", f"{ref}:{path}") + if have.returncode == 0 and have.stdout.strip() == digest: + break + else: + unique.append(f"{path} differs from HEAD and from the upstream tip") + + if upstream: + ahead = git("rev-list", "--count", f"{upstream}..HEAD") + if ahead.returncode == 0 and ahead.stdout.strip() not in ("", "0"): + unique.append( + f"{ahead.stdout.strip()} commit(s) exist only on this branch" + ) + + if unique: + shown = "; ".join(unique[:3]) + if len(unique) > 3: + shown += f"; and {len(unique) - 3} more" + return True, shown + return False, "every change is already in HEAD or in the upstream tip" + + @staticmethod + def _git_state_dir(source_dir: str) -> str | None: + """The git dir, where a recovery receipt belongs (issue #1237). + + Measured while writing this recovery: a receipt beside the tree + (``/.emrg/...``) re-dirtied the tree it had just cleaned in any + repository that does not ignore ``.emrg/``, re-arming the guard on the next + cycle - the action causing the condition it exists to clear. Nothing inside + the git dir is tracked or reported by ``git status``, and it is where git + keeps its own recovery records (``ORIG_HEAD``, ``stash``). + + Returns None when it cannot be resolved: a receipt is not worth failing a + verified recovery over, but the caller reports that it is missing. + """ + import subprocess as _sp # noqa: PLC0415 — local import keeps the module invariant + + for args in (("rev-parse", "--absolute-git-dir"), ("rev-parse", "--git-dir")): + try: + out = _sp.run( + ["git", "-C", source_dir, *args], + capture_output=True, text=True, timeout=10, + encoding="utf-8", errors="replace", + ) + except (OSError, _sp.SubprocessError): + return None + if out.returncode == 0 and out.stdout.strip(): + path = out.stdout.strip() + if os.path.isabs(path): + return path + return os.path.normpath(os.path.join(source_dir, path)) + return None + + @staticmethod + def _write_recovery_receipt(source_dir: str, payload: dict) -> str | None: + """Write the receipt of a recovery into the git dir; return its path or None. + + Best-effort by design: by the time this runs the recovery has already + happened and been verified, and the stash itself is the durable record of + what moved - so a missing receipt is worth reporting, not worth undoing a + verified recovery over. + """ + state = TaskHandler._git_state_dir(source_dir) + if state is None: + return None + target = os.path.join(state, "emrg-recovery-receipt.json") + try: + with open(target, "w", encoding="utf-8") as handle: + json.dump(payload, handle, ensure_ascii=False, indent=2) + handle.write("\n") + except OSError: + return None + return target + + @staticmethod + def _recover_dirty_tree_sync( + source_dir: str, reason: str | None = None + ) -> tuple[bool, str]: + """Converge a working tree whose dirt is reconstructible, reversibly (issue #1237). + + The exit the downgrade never had, performed by the daemon **itself** (host + directive 2026-09-16: dirt must be recovered from, not merely detected and + reported). Nothing here is a decision for a human: the criterion says every + byte in the tree is already in ``HEAD`` or in the upstream tip, so moving it + aside cannot lose anything - and the move is a stash, which leaves every byte + one ``git stash pop`` away. No branch is reset and no commit is dropped: + ``HEAD`` is asserted unmoved before and after. + + Answers ``(True, detail)`` when the tree is now verified clean, and + ``(False, why)`` when it is not - refused (the tree holds work found nowhere + else), already clean, or unmeasurable. Callers must not read ``False`` as + "clean". + + ``reason`` is the caller's already-measured verdict from + ``_dirty_tree_would_lose_work_sync``; when omitted the criterion is re-checked + here, so the action can never run on unique work even if a caller forgets to + ask first. That re-check is why this is safe to expose as a command. + """ + import subprocess as _sp # noqa: PLC0415 — local import keeps the module invariant + + def git(*args: str): + return _sp.run( + ["git", "-C", source_dir, *args], + capture_output=True, text=True, timeout=30, + encoding="utf-8", errors="replace", + ) + + try: + head = git("rev-parse", "--verify", "--quiet", "HEAD") + except (OSError, _sp.SubprocessError) as exc: + return False, f"the recovery could not run: {exc}" + if head.returncode != 0 or not head.stdout.strip(): + return False, "the recovery could not run: not a git repository with a commit" + + status = git("status", "--porcelain", "--untracked-files=normal") + if status.returncode != 0: + return False, "the recovery could not run: `git status` failed" + before = [line for line in status.stdout.splitlines() if line.strip()] + if not before: + return False, "the tree was already clean; nothing to recover" + + if reason is None: + loses, reason = TaskHandler._dirty_tree_would_lose_work_sync(source_dir) + if loses: + return False, f"refused: this tree holds work that exists nowhere else ({reason})" + + message = "emrg-recovery-" + datetime.now(timezone.utc).strftime("%Y%m%dT%H%M%SZ") + stash = git("stash", "push", "--include-untracked", "-m", message) + if stash.returncode != 0: + detail = (stash.stderr or stash.stdout).strip() + return False, f"the recovery could not run: `git stash push` failed: {detail}" + + after = git("status", "--porcelain", "--untracked-files=normal") + if after.returncode != 0 or after.stdout.strip(): + return False, ( + "the tree is still not clean after stashing; recover the stash with " + f"`git stash pop` (stash {message}) and investigate" + ) + + head_after = git("rev-parse", "HEAD").stdout.strip() + TaskHandler._write_recovery_receipt(source_dir, { + "timestamp": datetime.now(timezone.utc).isoformat(), + "repo": source_dir, + "reason": reason, + "head_before": head.stdout.strip(), + "head_after": head_after, + "status_before": before, + "status_after": [], + "stash_message": message, + "action": "git stash push --include-untracked", + "reversible_with": f"git stash pop (or `git stash list` -> {message})", + }) + return True, ( + f"{len(before)} reconstructible change(s) stashed as {message}; " + f"HEAD unmoved at {head_after[:8]}; recover with `git stash pop`" + ) + + async def _effective_sandbox( + self, dirty: bool | None = None, loses_unique: bool | None = None + ) -> str: """Per-cycle effective bash sandbox tier for the task message. Structural dirty-tree guard (community issue #979 — heinrichneb's "audited override" pattern: inconvenient by default, possible on - explicit human override, every exception is a receipt). When the - source dir has uncommitted changes the cycle runs read-only - regardless of configuration — the host's live edits are out of - reach structurally, not by prompt aspiration. A human may override - with the env var ``EMRG_TASK_DIRTY_OVERRIDE`` (comma-separated task - names, or ``*`` for all); every override is logged as a receipt. + explicit human override, every exception is a receipt), **keyed on loss + rather than on dirt** (host directive 2026-09-16, issue #1237). + + The guard exists so a cycle cannot destroy work that lives only in the + host's working tree. "The tree is dirty" was a proxy for that, and a bad + one: it fired on states that hold no unique work at all — most expensively + on the 33-cycle stretch where two modified files were byte-identical to + upstream's own blobs — while making the cycle read-only *also* disabled + the git verbs it would have needed to converge the tree, so the state that + triggered the downgrade kept the cycle locked in it. + + So the question is now asked directly, by ``_dirty_tree_would_lose_work_sync``, + and answered, by ``_recover_dirty_tree_sync`` (host directive 2026-09-16: dirt + must be recovered from, not merely detected and reported - a guard whose exit + is a command a human has to run is a guard that strands the tree): + + * a dirty tree whose changes are all recoverable from ``HEAD`` or the upstream + tip is **converged by the daemon itself**, reversibly (stash, ``HEAD`` + unmoved, receipt in the git dir), and the configured tier is left intact; + * a dirty tree holding anything found nowhere else is forced ``read-only`` as + before, with the offending paths named - the daemon touches nothing, because + discarding that is a decision for whoever wrote it. + + A human may still override with the env var ``EMRG_TASK_DIRTY_OVERRIDE`` + (comma-separated task names, or ``*`` for all); every release of the guard is + logged as a receipt. """ if dirty is None: dirty = await asyncio.to_thread( @@ -405,9 +661,30 @@ async def _effective_sandbox(self, dirty: bool | None = None) -> str: self.name, self._sandbox, ) return self._sandbox + if loses_unique is None: + loses_unique, why = await asyncio.to_thread( + self._dirty_tree_would_lose_work_sync, str(self._source_dir) + ) + else: + why = "reported by the caller" + if not loses_unique: + recovered, detail = await asyncio.to_thread( + self._recover_dirty_tree_sync, str(self._source_dir), why + ) + self._logger.warning( + "TaskHandler[%s]: dirty working tree holding no unique work (%s) — " + "self-recovery %s; cycle keeps %s (structural guard, community " + "issue #979 / #1237, audited receipt)", + self.name, why, detail if recovered else "did not complete", self._sandbox, + ) + return self._sandbox self._logger.warning( - "TaskHandler[%s]: dirty working tree — cycle forced read-only " - "(structural guard, community issue #979)", self.name, + "TaskHandler[%s]: dirty working tree holding work that exists nowhere " + "else (%s) — cycle forced read-only (structural guard, community " + "issue #979); the daemon leaves that work untouched and names it here; " + "a human decides whether to commit, stash or discard it " + "(`scripts/recover-worktree.py --repo ` reports the same verdict)", + self.name, why, ) return "read-only" diff --git a/scripts/recover-worktree.py b/scripts/recover-worktree.py new file mode 100644 index 00000000..33161194 --- /dev/null +++ b/scripts/recover-worktree.py @@ -0,0 +1,156 @@ +#!/usr/bin/env python3 +"""Recover a working tree whose dirt is **reconstructible** (issue #1237). + +Why this exists +--------------- +`scheduler._effective_sandbox` forces a cycle whose source tree is dirty down to +`read-only`, and `read-only` refuses every git verb that could clean it: measured +`checkout`/`reset`/`restore`/`switch`/`merge`/`stash`/`update-ref`/`symbolic-ref`/ +`commit`/`push` = **10/10 BLOCK**. The state that triggered the downgrade therefore +disabled the only exit from it, and the cost was measured: **33 consecutive +zero-commit cycles**, with 40+ verified patches stranded outside the repository. + +The criterion that makes acting decidable is *"would anything be lost?"*, not *"is +the tree dirty?"* — the two differ, and they differed expensively: of the dirt in +that stretch, git reported 2 modified files and **both were byte-identical to +upstream's own blobs**, so discarding them lost nothing at all. +`scheduler._dirty_tree_would_lose_work_sync` answers that question, and +`scheduler._recover_dirty_tree_sync` acts on it: a reconstructible tree is now +converged by the daemon itself at the start of a cycle, so the deadlock is not +something a human has to be present to break. This command is the **manual** route +— the same action, from a shell, for a tree no cycle is about to touch. + +What it does +------------ +Diagnose, then — only with `--apply` — converge, through the one implementation the +daemon also uses (this script owns no policy; if it disagreed with the guard about +what "reconstructible" means, the deadlock would come back through the script): + +* the tree is already clean -> nothing to do; +* the dirt holds work that exists nowhere else -> **refuse**, name the paths, and + touch nothing. Committing, stashing or copying that work out is a decision for + whoever wrote it, and an agent that discards it is doing the thing the guard + exists to prevent; +* the dirt is reconstructible -> `git stash push -u` it, which leaves the worktree + clean *and* keeps every byte in the stash, so the action is undoable with + `git stash pop`. **No branch is reset and no commit is dropped** — this never + moves `HEAD`. + +Every `--apply` writes a receipt into the **git state dir** (see +`scheduler._git_state_dir` — a location that cannot dirty the tree it just cleaned), +as the structural guard's contract requires of every release of a safety rule. + +Exit codes +---------- +``0`` nothing to do, or the tree was converged and verified (also: a dry run of a + tree that *would* converge). ``1`` refused: the tree holds work found + nowhere else, nothing was written. ``2`` the question could not be + answered (not a git repo, an unreadable state) — never reported as a pass. + +Usage +----- + uv run --no-sync python scripts/recover-worktree.py --repo + uv run --no-sync python scripts/recover-worktree.py --repo --apply +""" + +from __future__ import annotations + +import argparse +import os +import subprocess +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from emrg.server.scheduler import TaskHandler # noqa: E402 (needs the path above) + + +def _git(repo: Path, *args: str, timeout: int = 30): + """Run git in `repo`. `-C` rather than chdir so the caller's cwd is untouched.""" + return subprocess.run( + ["git", "-C", str(repo), *args], + capture_output=True, text=True, timeout=timeout, + encoding="utf-8", errors="replace", + ) + + +def _status_lines(repo: Path) -> list[str] | None: + """`git status --porcelain` lines, or None when it could not be read.""" + out = _git(repo, "status", "--porcelain", "--untracked-files=normal") + if out.returncode != 0: + return None + return [line for line in out.stdout.splitlines() if line.strip()] + + +def _rev(repo: Path, ref: str) -> str | None: + out = _git(repo, "rev-parse", "--verify", "--quiet", ref) + if out.returncode != 0 or not out.stdout.strip(): + return None + return out.stdout.strip() + + +def _receipt_path(repo: Path) -> str | None: + """Where the daemon's own recovery writes its receipt, or None.""" + state = TaskHandler._git_state_dir(str(repo)) + return None if state is None else os.path.join(state, "emrg-recovery-receipt.json") + + +def recover(repo: Path, apply: bool) -> int: + """Diagnose `repo`, and converge it when `apply` and it is safe. Returns an exit code.""" + if _rev(repo, "HEAD") is None: + print(f"could not measure: {repo} is not a git repository with a commit") + return 2 + + status = _status_lines(repo) + if status is None: + print(f"could not measure: `git status` failed in {repo}") + return 2 + if not status: + print(f"clean: {repo} has no uncommitted changes; nothing to recover") + return 0 + + # One owner for the criterion: the same question the guard asks, so the tool + # and the tier decision can never disagree about what "reconstructible" means. + loses, why = TaskHandler._dirty_tree_would_lose_work_sync(str(repo)) + if loses: + print(f"refused: {repo} holds work that exists nowhere else: {why}") + print("nothing was changed. Commit, stash or copy that work out first;") + print("this tool will not discard it.") + return 1 + + if not apply: + print(f"recoverable: {why}") + print(f"{len(status)} entr(y|ies) would be stashed (reversible); re-run with --apply") + return 0 + + # One owner for the action too: the daemon runs this exact function at the start + # of a cycle, so what a human runs here and what the guard does cannot drift. + ok, detail = TaskHandler._recover_dirty_tree_sync(str(repo), reason=why) + if not ok: + print(f"could not measure: {detail}") + return 2 + + receipt = _receipt_path(repo) + print(f"recovered: {repo} converged to a clean tree; {detail}") + print("reversible: git stash pop") + print(f"receipt: {receipt}" if receipt else "receipt: could not be written") + return 0 + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + description="Converge a working tree whose uncommitted changes are " + "reconstructible, reversibly, with a receipt.", + ) + parser.add_argument("--repo", default=".", help="repository to inspect (default: .)") + parser.add_argument( + "--apply", action="store_true", + help="perform the recovery; without it the tool only reports", + ) + args = parser.parse_args(argv) + return recover(Path(args.repo).resolve(), args.apply) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/test_recover_worktree.py b/tests/test_recover_worktree.py new file mode 100644 index 00000000..98969aef --- /dev/null +++ b/tests/test_recover_worktree.py @@ -0,0 +1,287 @@ +"""The reconstructible-dirt criterion and `scripts/recover-worktree.py` (#1237). + +The guard's contract is "a cycle must not destroy work that exists nowhere else". +These pin the two halves: dirt that IS unique is refused (nothing touched), and +dirt that is NOT unique is converged **reversibly** — the scenario the measured +33-cycle deadlock was made of, built here as a real repository. +""" + +from __future__ import annotations + +import importlib.util +import json +import subprocess +import sys +from pathlib import Path + +SCRIPT = Path(__file__).resolve().parent.parent / "scripts" / "recover-worktree.py" + + +def _load(): + spec = importlib.util.spec_from_file_location("recover_worktree", SCRIPT) + module = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = module + spec.loader.exec_module(module) + return module + + +def _git(repo: Path, *args: str): + return subprocess.run( + ["git", "-C", str(repo), *args], capture_output=True, text=True, timeout=30 + ) + + +def _status(repo: Path) -> str: + return _git(repo, "status", "--porcelain").stdout + + +def _new_repo(path: Path, content: str = "v1") -> None: + """A one-commit repository. `-b master` because the criterion's upstream ref is + the default branch, and a test whose branch name drifts would measure nothing.""" + path.mkdir(parents=True, exist_ok=True) + _git(path, "init", "-q", "-b", "master") + _git(path, "config", "user.email", "t@t.t") + _git(path, "config", "user.name", "t") + (path / "f.txt").write_text(content, encoding="utf-8") + _git(path, "add", "f.txt") + _git(path, "commit", "-q", "-m", "base") + + +def _with_upstream(tmp_path: Path): + """The deadlock's exact shape: HEAD behind, worktree carrying upstream's bytes. + + Measures `(repo, head_sha)`: the tree is dirty (one modified tracked file) and + holds **no unique work at all** — the file's bytes are the upstream tip's blob. + Of the 33 lost cycles, this was the state of the working tree. + """ + origin = tmp_path / "origin.git" + subprocess.run( + ["git", "init", "-q", "--bare", "-b", "master", str(origin)], + capture_output=True, text=True, timeout=30, + ) + work = tmp_path / "work" + _new_repo(work, "v1") + _git(work, "remote", "add", "origin", str(origin)) + _git(work, "push", "-q", "-u", "origin", "master") + head = _git(work, "rev-parse", "HEAD").stdout.strip() + (work / "f.txt").write_text("v2", encoding="utf-8") + _git(work, "commit", "-q", "-am", "v2") + _git(work, "push", "-q", "origin", "master") + _git(work, "reset", "-q", "--hard", head) # HEAD back to v1 … + (work / "f.txt").write_text("v2", encoding="utf-8") # … worktree at upstream's v2 + assert _status(work).startswith(" M"), _status(work) + return work, head + + +# ── the criterion itself, against real repositories ────────────────────────── + + +def test_dirt_carrying_upstreams_bytes_is_reconstructible(tmp_path): + """False = nothing would be lost. This is the case that cost 33 cycles.""" + work, _ = _with_upstream(tmp_path) + loses, why = _load().TaskHandler._dirty_tree_would_lose_work_sync(str(work)) + assert loses is False, why + assert "upstream" in why + + +def test_a_modification_found_nowhere_is_unique(tmp_path): + """The real protection, in the other direction: nothing upstream holds these bytes.""" + _new_repo(tmp_path / "repo", "v1") + repo = tmp_path / "repo" + (repo / "f.txt").write_text("host's unreleased work", encoding="utf-8") + loses, why = _load().TaskHandler._dirty_tree_would_lose_work_sync(str(repo)) + assert loses is True + assert "f.txt" in why + + +def test_an_untracked_file_is_unique(tmp_path): + """Untracked content exists in exactly one place, so it is never reconstructible.""" + _new_repo(tmp_path / "repo") + repo = tmp_path / "repo" + (repo / "notes.md").write_text("only here", encoding="utf-8") + loses, why = _load().TaskHandler._dirty_tree_would_lose_work_sync(str(repo)) + assert loses is True + assert "notes.md" in why + + +def test_an_untracked_file_is_unique_even_when_upstream_has_those_bytes(tmp_path): + """The clause that decides this case: "untracked == 0" is a precondition. + + Found by mutation (removing the `??` fast path changed no existing answer — + the content comparison reaches the same verdict for a file git never tracked). + That is not true here: when the path is *absent from HEAD* but present in the + upstream tip, the content comparison finds a matching blob and would release + the file, while the criterion says otherwise. An untracked file is content git + has never tracked, so it is the host's by construction; releasing it because a + copy was published once would discard a file the host created. Conservative by + design, and the safe side: naming it unique only refuses. + """ + origin = tmp_path / "origin.git" + subprocess.run( + ["git", "init", "-q", "--bare", "-b", "master", str(origin)], + capture_output=True, text=True, timeout=30, + ) + repo = tmp_path / "repo" + _new_repo(repo, "v1") + _git(repo, "remote", "add", "origin", str(origin)) + _git(repo, "push", "-q", "-u", "origin", "master") + head = _git(repo, "rev-parse", "HEAD").stdout.strip() + (repo / "b.txt").write_text("published elsewhere", encoding="utf-8") + _git(repo, "add", "b.txt") + _git(repo, "commit", "-q", "-m", "b") + _git(repo, "push", "-q", "origin", "master") + _git(repo, "reset", "-q", "--hard", head) # b.txt now exists only upstream + (repo / "b.txt").write_text("published elsewhere", encoding="utf-8") # untracked again + assert _status(repo).strip() == "?? b.txt", _status(repo) + + loses, why = _load().TaskHandler._dirty_tree_would_lose_work_sync(str(repo)) + assert loses is True, "untracked content is never reconstructible" + assert "b.txt" in why + + +def test_a_deletion_loses_nothing(tmp_path): + """A deleted tracked file is restored by discarding, so it is not unique work.""" + _new_repo(tmp_path / "repo") + repo = tmp_path / "repo" + (repo / "f.txt").unlink() + assert _status(repo).startswith(" D"), _status(repo) + loses, why = _load().TaskHandler._dirty_tree_would_lose_work_sync(str(repo)) + assert loses is False, why + + +def test_a_commit_only_on_this_branch_is_unique(tmp_path): + """A branch reset would orphan it: dirty *and* ahead is never reconstructible.""" + origin = tmp_path / "origin.git" + subprocess.run( + ["git", "init", "-q", "--bare", "-b", "master", str(origin)], + capture_output=True, text=True, timeout=30, + ) + repo = tmp_path / "repo" + _new_repo(repo) + _git(repo, "remote", "add", "origin", str(origin)) + _git(repo, "push", "-q", "-u", "origin", "master") + _git(repo, "commit", "-q", "--allow-empty", "-m", "local only") + (repo / "f.txt").write_text("v2", encoding="utf-8") + loses, why = _load().TaskHandler._dirty_tree_would_lose_work_sync(str(repo)) + assert loses is True + assert "only on this branch" in why + + +def test_a_clean_tree_is_not_dirt(tmp_path): + _new_repo(tmp_path / "repo") + loses, why = _load().TaskHandler._dirty_tree_would_lose_work_sync(str(tmp_path / "repo")) + assert loses is False + assert "clean" in why + + +# ── the tool: refuse, converge, stay reversible ────────────────────────────── + + +def test_the_tool_refuses_unique_work_and_touches_nothing(tmp_path, capsys): + """The refusal is the guard working: the file must survive the attempt.""" + repo = tmp_path / "repo" + _new_repo(repo) + (repo / "notes.md").write_text("only here", encoding="utf-8") + + assert _load().recover(repo, apply=True) == 1 + out = capsys.readouterr().out + assert "refused" in out + assert (repo / "notes.md").read_text(encoding="utf-8") == "only here" + assert _git(repo, "stash", "list").stdout.strip() == "" + assert not (Path(_git(repo, "rev-parse", "--absolute-git-dir").stdout.strip()) + / "emrg-recovery-receipt.json").exists() + + +def test_the_tool_converges_reconstructible_dirt_reversibly(tmp_path, capsys): + """The deadlock's state, repaired: clean tree, HEAD unmoved, work recoverable.""" + work, head = _with_upstream(tmp_path) + + assert _load().recover(work, apply=False) == 0 + assert "recoverable" in capsys.readouterr().out + assert _status(work).startswith(" M"), "a dry run must not change the tree" + + assert _load().recover(work, apply=True) == 0 + out = capsys.readouterr().out + assert "recovered" in out and "reversible" in out + assert _status(work).strip() == "", "the tool's whole job is a clean tree" + assert _git(work, "rev-parse", "HEAD").stdout.strip() == head, "HEAD must not move" + + # Reversibility is the reason this action is allowed at all: every byte is + # still there, one command away. + _git(work, "stash", "pop") + assert (work / "f.txt").read_text(encoding="utf-8") == "v2" + + +def test_the_tool_writes_a_receipt_of_what_it_moved(tmp_path): + """The structural guard's contract: every release of a safety rule is a receipt.""" + work, head = _with_upstream(tmp_path) + + assert _load().recover(work, apply=True) == 0 + git_dir = Path(_git(work, "rev-parse", "--absolute-git-dir").stdout.strip()) + receipt = json.loads( + (git_dir / "emrg-recovery-receipt.json").read_text(encoding="utf-8") + ) + # The receipt must not re-dirty the tree it just cleaned — measured while + # writing this tool, when the receipt lived beside it. + assert _status(work).strip() == "", "the receipt itself must not be dirt" + + assert receipt["repo"] == str(work) + assert receipt["head_before"] == head + assert receipt["head_after"] == head, "the receipt must show HEAD did not move" + assert any(line.startswith(" M") for line in receipt["status_before"]) + assert receipt["status_after"] == [] + assert "stash" in receipt["action"] + assert "stash pop" in receipt["reversible_with"] + assert "upstream" in receipt["reason"] + + +def test_the_action_asks_the_criterion_itself_before_touching_anything(tmp_path): + """Defence in depth: the action refuses unique work without being told to. + + Called with no measured verdict (the way a future caller who has not read this + module would call it), it must ask the criterion itself. Otherwise the safety of + the guard would rest on every caller remembering to check first -- and the one + that forgets would discard a host's unsaved work while the receipt called it a + recovery. + """ + repo = tmp_path / "repo" + _new_repo(repo) + (repo / "notes.md").write_text("only here", encoding="utf-8") + + ok, detail = _load().TaskHandler._recover_dirty_tree_sync(str(repo)) + assert ok is False and "refused" in detail, detail + assert (repo / "notes.md").read_text(encoding="utf-8") == "only here" + assert _git(repo, "stash", "list").stdout.strip() == "", "nothing may be moved aside" + + # The other direction, same entry point: reconstructible dirt does converge. + work, head = _with_upstream(tmp_path) + ok, detail = _load().TaskHandler._recover_dirty_tree_sync(str(work)) + assert ok is True and "stash pop" in detail, detail + assert _status(work).strip() == "" + + +def test_a_clean_tree_is_a_no_op(tmp_path, capsys): + repo = tmp_path / "repo" + _new_repo(repo) + + assert _load().recover(repo, apply=True) == 0 + assert "nothing to recover" in capsys.readouterr().out + assert _git(repo, "stash", "list").stdout.strip() == "" + + +def test_a_directory_that_is_not_a_repo_cannot_be_measured(tmp_path, capsys): + """Exit 2 — a question that could not be answered is never reported as a pass.""" + plain = tmp_path / "plain" + plain.mkdir() + + assert _load().recover(plain, apply=True) == 2 + assert "could not measure" in capsys.readouterr().out + + +def test_main_parses_the_repo_and_apply_flags(tmp_path, capsys): + """The CLI wiring, so the documented invocation is the tested one.""" + work, _ = _with_upstream(tmp_path) + + assert _load().main(["--repo", str(work)]) == 0 + assert "recoverable" in capsys.readouterr().out + assert _status(work).startswith(" M"), "without --apply nothing is written" diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index 2888b9b3..c2c96781 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -3,6 +3,7 @@ from __future__ import annotations import asyncio +import json import logging import os import subprocess @@ -2653,20 +2654,128 @@ def test_is_dirty_tree_linked_worktree(): def test_dirty_tree_forces_read_only_structural_guard(): - """Community issue #979: a dirty source tree forces the cycle's effective - sandbox to read-only regardless of configuration — topology over rules.""" + """Community issue #979 + #1237: a dirty tree holding work that exists nowhere + else forces the cycle's effective sandbox to read-only regardless of + configuration — topology over rules. `loses_unique` is injected here so the + test states the *reason* rather than depending on what the tree under test + happens to contain; the end-to-end probes are the two tests below.""" handler = TaskHandler( name="emrg-task", config={"project": "emrg"}, interval=60, identity=InstanceIdentity(), ) assert handler._sandbox == "workspace-write" # configured default - assert asyncio.run(handler._effective_sandbox(dirty=True)) == "read-only" + assert asyncio.run( + handler._effective_sandbox(dirty=True, loses_unique=True) + ) == "read-only" # configured read-only stays read-only (no weakening) handler2 = TaskHandler( name="ro-task", config={}, interval=60, identity=InstanceIdentity(), sandbox="read-only", ) - assert asyncio.run(handler2._effective_sandbox(dirty=True)) == "read-only" + assert asyncio.run( + handler2._effective_sandbox(dirty=True, loses_unique=True) + ) == "read-only" + + +def _status(repo: str) -> str: + """`git status --porcelain` in `repo`, as text.""" + return subprocess.run(["git", "-C", repo, "status", "--porcelain"], + capture_output=True, text=True, timeout=30, + encoding="utf-8", errors="replace").stdout + + +def _git_out(repo: str, *args: str) -> str: + """Stdout of `git ` in `repo`, stripped.""" + return subprocess.run(["git", "-C", repo, *args], capture_output=True, text=True, + timeout=30, encoding="utf-8", errors="replace").stdout.strip() + + +def _repo_with_dirt(tmp_path, kind: str) -> str: + """A one-commit repo made dirty in a named way. Returns its path. + + `deleted` — a tracked file removed from the worktree: `git status` reports + ` D`, and discarding restores it, so **nothing exists only here**. + `untracked` — a file git has never seen: its bytes exist in exactly one place. + """ + repo = tmp_path / f"repo-{kind}" + repo.mkdir() + subprocess.run(["git", "init", "-q", "-b", "master", str(repo)], + capture_output=True, timeout=30) + for key, value in (("user.email", "t@t.t"), ("user.name", "t")): + subprocess.run(["git", "-C", str(repo), "config", key, value], + capture_output=True, timeout=30) + (repo / "f.txt").write_text("v1", encoding="utf-8") + subprocess.run(["git", "-C", str(repo), "add", "f.txt"], + capture_output=True, timeout=30) + subprocess.run(["git", "-C", str(repo), "commit", "-q", "-m", "base"], + capture_output=True, timeout=30) + if kind == "deleted": + (repo / "f.txt").unlink() + else: + (repo / "notes.md").write_text("only here", encoding="utf-8") + return str(repo) + + +def test_reconstructible_dirt_is_recovered_by_the_daemon_itself(tmp_path): + """Host directive 2026-09-16 (issue #1237): dirt must be *recovered from*. + + The downgrade is keyed on loss, and its exit is the daemon's own action -- not a + command a human has to be present to run. A guard whose exit needs a human + strands the tree, which is the shape of the loss that cost 33 consecutive + zero-commit cycles: the tier that blocked the cycle was also the tier that + blocked the repair of the thing blocking it. + + Measured end-to-end through the real probe and the real recovery: the tree is + clean afterwards without anyone asking, HEAD never moved, the moved work is one + `git stash pop` away, and a receipt in the git dir records what happened. + """ + repo = _repo_with_dirt(tmp_path, "deleted") + assert _status(repo).startswith(" D"), "precondition: deletion-only dirt" + head = _git_out(repo, "rev-parse", "HEAD") + + handler = TaskHandler(name="emrg-task", config={"path": repo}, interval=60, + identity=InstanceIdentity()) + assert asyncio.run(handler._effective_sandbox()) == "workspace-write", \ + "dirt holding no unique work must not cost the cycle its tier" + + assert _status(repo).strip() == "", \ + "the daemon must converge a reconstructible tree itself, not ask a human to" + assert _git_out(repo, "rev-parse", "HEAD") == head, "the recovery must not move HEAD" + assert "emrg-recovery-" in _git_out(repo, "stash", "list"), \ + "the action must be reversible: the work lives in a stash" + + git_dir = Path(_git_out(repo, "rev-parse", "--absolute-git-dir")) + receipt = json.loads((git_dir / "emrg-recovery-receipt.json").read_text(encoding="utf-8")) + assert receipt["repo"] == repo + assert receipt["head_before"] == receipt["head_after"] == head + assert receipt["status_before"] == [" D f.txt"], receipt["status_before"] + assert receipt["status_after"] == [] + assert "stash pop" in receipt["reversible_with"] + + # Reversibility is why the action is allowed at all: popping restores the exact + # state the daemon moved aside -- here, the deletion itself, byte for byte. + pop = subprocess.run(["git", "-C", repo, "stash", "pop"], + capture_output=True, text=True, timeout=30, + encoding="utf-8", errors="replace") + assert pop.returncode == 0, pop.stderr + assert _status(repo) == " D f.txt\n", "the stash must carry the exact dirt it moved" + + +def test_unique_dirt_still_forces_read_only(tmp_path): + """The protection, unchanged: an untracked file exists nowhere else.""" + repo = _repo_with_dirt(tmp_path, "untracked") + + handler = TaskHandler(name="emrg-task", config={"path": repo}, interval=60, + identity=InstanceIdentity()) + assert asyncio.run(handler._effective_sandbox()) == "read-only" + # The work the guard protects is still there after the decision -- and the + # daemon's self-recovery must not have run at all: a stash here would be the + # guard moving a host's unsaved work out from under it. + assert (tmp_path / "repo-untracked" / "notes.md").read_text(encoding="utf-8") == "only here" + assert _git_out(repo, "stash", "list") == "", "unique work must not be stashed" + assert not (Path(_git_out(repo, "rev-parse", "--absolute-git-dir")) + / "emrg-recovery-receipt.json").exists(), \ + "no recovery ran, so there is no recovery to receipt" def test_dirty_tree_override_env_audited_receipt(): @@ -2681,13 +2790,19 @@ def test_dirty_tree_override_env_audited_receipt(): try: # task named in the override → configured tier restored os.environ["EMRG_TASK_DIRTY_OVERRIDE"] = "other-task,emrg-task" - assert asyncio.run(handler._effective_sandbox(dirty=True)) == "workspace-write" + assert asyncio.run( + handler._effective_sandbox(dirty=True, loses_unique=True) + ) == "workspace-write" # wildcard → configured tier restored os.environ["EMRG_TASK_DIRTY_OVERRIDE"] = "*" - assert asyncio.run(handler._effective_sandbox(dirty=True)) == "workspace-write" + assert asyncio.run( + handler._effective_sandbox(dirty=True, loses_unique=True) + ) == "workspace-write" # override for a different task → guard still applies os.environ["EMRG_TASK_DIRTY_OVERRIDE"] = "other-task" - assert asyncio.run(handler._effective_sandbox(dirty=True)) == "read-only" + assert asyncio.run( + handler._effective_sandbox(dirty=True, loses_unique=True) + ) == "read-only" finally: if old is None: os.environ.pop("EMRG_TASK_DIRTY_OVERRIDE", None) @@ -2702,4 +2817,6 @@ def test_clean_tree_keeps_configured_sandbox(): name="emrg-task", config={"project": "emrg"}, interval=60, identity=InstanceIdentity(), ) - assert asyncio.run(handler._effective_sandbox(dirty=False)) == "workspace-write" + assert asyncio.run( + handler._effective_sandbox(dirty=False, loses_unique=True) + ) == "workspace-write", "a clean tree is never asked about loss" From 6509ba3d123a323f6e102835763f727603ca9d2e Mon Sep 17 00:00:00 2001 From: EMRG Evolution Date: Wed, 16 Sep 2026 09:43:55 +0800 Subject: [PATCH 2/4] emrg: pin the encoding on the new tests' git calls (CI guard, not the local run) CI red on the ubuntu leg while the local suite was green, and the difference is worth recording: `test_script_decode_is_locale_independent.py` scans **tracked** files. Before the first commit this test file was untracked, so the guard never saw it and the local run proved less than it looked -- a green local suite over an untracked file is not evidence about that file. Four text-mode `subprocess.run(["git", ...])` calls in `tests/test_recover_worktree.py` now pin `encoding="utf-8", errors="replace"`, for the reason the guard states: a text-mode subprocess without it decodes with the host locale codec and raises or mojibakes on cp936/cp1252 for data that is valid UTF-8 (issue #1132). Re-ran the full suite *after* `git add`, i.e. in the tree CI sees: 2624 passed / 17 skipped. --- tests/test_recover_worktree.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/test_recover_worktree.py b/tests/test_recover_worktree.py index 98969aef..1b4f98e6 100644 --- a/tests/test_recover_worktree.py +++ b/tests/test_recover_worktree.py @@ -27,7 +27,11 @@ def _load(): def _git(repo: Path, *args: str): return subprocess.run( - ["git", "-C", str(repo), *args], capture_output=True, text=True, timeout=30 + ["git", "-C", str(repo), *args], capture_output=True, text=True, timeout=30, + # The guard's rule, and the reason for it: a text-mode subprocess without + # an explicit encoding decodes with the *host* locale codec, so it raises or + # mojibakes on cp936/cp1252 for data that is valid UTF-8 (issue #1132). + encoding="utf-8", errors="replace", ) @@ -58,6 +62,7 @@ def _with_upstream(tmp_path: Path): subprocess.run( ["git", "init", "-q", "--bare", "-b", "master", str(origin)], capture_output=True, text=True, timeout=30, + encoding="utf-8", errors="replace", ) work = tmp_path / "work" _new_repo(work, "v1") @@ -120,6 +125,7 @@ def test_an_untracked_file_is_unique_even_when_upstream_has_those_bytes(tmp_path subprocess.run( ["git", "init", "-q", "--bare", "-b", "master", str(origin)], capture_output=True, text=True, timeout=30, + encoding="utf-8", errors="replace", ) repo = tmp_path / "repo" _new_repo(repo, "v1") @@ -155,6 +161,7 @@ def test_a_commit_only_on_this_branch_is_unique(tmp_path): subprocess.run( ["git", "init", "-q", "--bare", "-b", "master", str(origin)], capture_output=True, text=True, timeout=30, + encoding="utf-8", errors="replace", ) repo = tmp_path / "repo" _new_repo(repo) From f15895c1b6e72e93662adf47a72c21a8975e8fdb Mon Sep 17 00:00:00 2001 From: EMRG Evolution Date: Wed, 16 Sep 2026 10:20:31 +0800 Subject: [PATCH 3/4] emrg: the recovery asks the criterion itself, and its answer governs the tier An independent review of #1274 measured two things the PR stated and one it did not hold. The hole: _recover_dirty_tree_sync took the caller's already-measured verdict as an optional argument, so a caller that forgot to measure -- or lied -- disarmed the net. Reproduced end-to-end: with loses_unique=False on a tree holding an untracked file, the cycle was granted workspace-write and the unique work was stashed, with the receipt calling it a recovery. A guarantee that holds only while every caller passes the truth is not a guarantee, so the parameter is gone rather than documented, and the action's own refusal now forces read-only regardless of the verdict above. That also closes a window the old shape left open: verdict and stash were two separate git status runs, so a file that became unique in between was described by the earlier answer while being moved by the later one. The stale clause: the local-commit rationale named a branch reset, an operation this self-recovery path does not perform -- it is a stash, which is indifferent to commits. The clause now says why ahead-of-upstream still forces read-only (the cycle's own write access could reset the branch away), and notes the measured consequence: a tree merely ahead keeps read-only even when every byte of its dirt is reconstructible. The action now answers four states instead of a bool, because False meant three different things and a caller reading any of them as 'nothing happened' would report a refused tree as though the question were settled. Verification: pytest 2641 -> 2643 collected (+2), 2626 passed / 17 skipped, no failures; import and CLI checks green; check-doc-count.py green. Both new tests were mutation-tested against 6509ba3d and fail there (the tier pin reproduces the workspace-write grant and the stash), so they pin the fix rather than the code. --- emrg/server/scheduler.py | 126 +++++++++++++++++++++++---------- scripts/recover-worktree.py | 28 +++++--- tests/test_recover_worktree.py | 64 +++++++++++++---- tests/test_scheduler.py | 29 +++++++- 4 files changed, 189 insertions(+), 58 deletions(-) diff --git a/emrg/server/scheduler.py b/emrg/server/scheduler.py index 08c05d5d..80c73e4f 100644 --- a/emrg/server/scheduler.py +++ b/emrg/server/scheduler.py @@ -401,8 +401,13 @@ def _dirty_tree_would_lose_work_sync(source_dir: str) -> tuple[bool, str]: * a **deletion** loses nothing (discarding restores the blob from ``HEAD``); * a **modification** is recoverable when its blob equals ``HEAD``'s or the upstream tip's for that path — the two places a blob can already live; - * a commit reachable only from this branch would be orphaned by a branch - reset ⇒ unique. + * a commit reachable only from this branch is unique: it is work that exists + in no other checkout, so it forces ``read-only``. The reason is *not* the + recovery - that is a stash, which is indifferent to commits (measured: a + recovery leaves ``HEAD`` at the same sha) - but the cycle's own write access, + which could reset the branch away. A reviewer measured the consequence and + it is the fail-closed direction: a tree that is merely *ahead* of upstream + keeps ``read-only`` even when every byte of its dirt is reconstructible. Fail-closed throughout: an unreadable status, an unreadable file, or no upstream ref to compare against all answer *unique*, i.e. exactly the @@ -530,9 +535,7 @@ def _write_recovery_receipt(source_dir: str, payload: dict) -> str | None: return target @staticmethod - def _recover_dirty_tree_sync( - source_dir: str, reason: str | None = None - ) -> tuple[bool, str]: + def _recover_dirty_tree_sync(source_dir: str) -> tuple[str, str]: """Converge a working tree whose dirt is reconstructible, reversibly (issue #1237). The exit the downgrade never had, performed by the daemon **itself** (host @@ -541,17 +544,30 @@ def _recover_dirty_tree_sync( byte in the tree is already in ``HEAD`` or in the upstream tip, so moving it aside cannot lose anything - and the move is a stash, which leaves every byte one ``git stash pop`` away. No branch is reset and no commit is dropped: - ``HEAD`` is asserted unmoved before and after. - - Answers ``(True, detail)`` when the tree is now verified clean, and - ``(False, why)`` when it is not - refused (the tree holds work found nowhere - else), already clean, or unmeasurable. Callers must not read ``False`` as - "clean". - - ``reason`` is the caller's already-measured verdict from - ``_dirty_tree_would_lose_work_sync``; when omitted the criterion is re-checked - here, so the action can never run on unique work even if a caller forgets to - ask first. That re-check is why this is safe to expose as a command. + ``HEAD`` is compared before and after, and a moved ``HEAD`` is reported. + + **The criterion is measured here, always, and no caller can supply an + answer.** An earlier revision accepted the caller's already-measured verdict + so a second ``git status`` could be avoided - and that turned the net into a + conditional one: an independent review (#1274) measured + ``_recover_dirty_tree_sync(repo, "reported by the caller")`` stashing a tree + holding an untracked file, i.e. *unique* work, with the receipt calling it a + recovery. The parameter is gone rather than documented, because a guarantee + that holds only while every caller passes the truth is not a guarantee. It + also closes a window the old shape left open: the verdict and the stash were + two separate ``git status`` runs, so a file that became unique in between was + described by the earlier answer while being moved by the later one. + + Answers ``(status, detail)``, and callers must **branch on the status** rather + than on truthiness - the four are not two: + + ``"recovered"`` the tree was converged and verified clean; ``detail`` names + the stash to reverse it; + ``"clean"`` there was nothing to recover, no stash was made; + ``"refused"`` the tree holds work that exists nowhere else: **nothing was + touched**, and ``detail`` names it; + ``"error"`` the question could not be answered, or the convergence + failed; ``detail`` says which. """ import subprocess as _sp # noqa: PLC0415 — local import keeps the module invariant @@ -565,33 +581,32 @@ def git(*args: str): try: head = git("rev-parse", "--verify", "--quiet", "HEAD") except (OSError, _sp.SubprocessError) as exc: - return False, f"the recovery could not run: {exc}" + return "error", f"the recovery could not run: {exc}" if head.returncode != 0 or not head.stdout.strip(): - return False, "the recovery could not run: not a git repository with a commit" + return "error", "the recovery could not run: not a git repository with a commit" - status = git("status", "--porcelain", "--untracked-files=normal") - if status.returncode != 0: - return False, "the recovery could not run: `git status` failed" - before = [line for line in status.stdout.splitlines() if line.strip()] + porcelain = git("status", "--porcelain", "--untracked-files=normal") + if porcelain.returncode != 0: + return "error", "the recovery could not run: `git status` failed" + before = [line for line in porcelain.stdout.splitlines() if line.strip()] if not before: - return False, "the tree was already clean; nothing to recover" + return "clean", "the tree was already clean; nothing to recover" - if reason is None: - loses, reason = TaskHandler._dirty_tree_would_lose_work_sync(source_dir) - if loses: - return False, f"refused: this tree holds work that exists nowhere else ({reason})" + loses, reason = TaskHandler._dirty_tree_would_lose_work_sync(source_dir) + if loses: + return "refused", f"this tree holds work that exists nowhere else ({reason})" message = "emrg-recovery-" + datetime.now(timezone.utc).strftime("%Y%m%dT%H%M%SZ") stash = git("stash", "push", "--include-untracked", "-m", message) if stash.returncode != 0: detail = (stash.stderr or stash.stdout).strip() - return False, f"the recovery could not run: `git stash push` failed: {detail}" + return "error", f"the recovery could not run: `git stash push` failed: {detail}" after = git("status", "--porcelain", "--untracked-files=normal") if after.returncode != 0 or after.stdout.strip(): - return False, ( + return "error", ( "the tree is still not clean after stashing; recover the stash with " - f"`git stash pop` (stash {message}) and investigate" + f"`git stash list` -> {message} and investigate" ) head_after = git("rev-parse", "HEAD").stdout.strip() @@ -605,11 +620,27 @@ def git(*args: str): "status_after": [], "stash_message": message, "action": "git stash push --include-untracked", - "reversible_with": f"git stash pop (or `git stash list` -> {message})", + # The named form is the primary one: a host may already have stashes, and + # a bare `git stash pop` is only correct while this stash is the newest + # (measured on the authoring workspace, which had an unrelated + # `stash@{0}` when this was written). + "reversible_with": ( + f"`git stash list` -> {message}, then `git stash apply stash^{{/{message}}}`" + f" (a bare `git stash pop` takes the newest, which is this one" + f" only until the next stash is made)" + ), }) - return True, ( + if head_after != head.stdout.strip(): + # Not expected - a stash does not move HEAD - so this is asserted rather + # than assumed, and the receipt above is the evidence either way. + return "error", ( + f"the recovery moved HEAD ({head.stdout.strip()[:8]} -> {head_after[:8]}); " + f"the stash is intact, inspect it before continuing " + f"(`git stash list` -> {message})" + ) + return "recovered", ( f"{len(before)} reconstructible change(s) stashed as {message}; " - f"HEAD unmoved at {head_after[:8]}; recover with `git stash pop`" + f"HEAD unmoved at {head_after[:8]}" ) async def _effective_sandbox( @@ -637,11 +668,18 @@ async def _effective_sandbox( * a dirty tree whose changes are all recoverable from ``HEAD`` or the upstream tip is **converged by the daemon itself**, reversibly (stash, ``HEAD`` - unmoved, receipt in the git dir), and the configured tier is left intact; + compared, receipt in the git dir), and the configured tier is left intact; * a dirty tree holding anything found nowhere else is forced ``read-only`` as before, with the offending paths named - the daemon touches nothing, because discarding that is a decision for whoever wrote it. + The action re-measures the criterion and **its answer governs the tier**: a + refusal (unique work) or a failed convergence forces ``read-only`` regardless + of the verdict reached above, so the safety of the tree does not depend on how + this method's own parameters were filled in. ``loses_unique`` remains as a + test seam and an optimisation for the *log line*; it cannot unlock a tree + holding unique work, because the action would find it. + A human may still override with the env var ``EMRG_TASK_DIRTY_OVERRIDE`` (comma-separated task names, or ``*`` for all); every release of the guard is logged as a receipt. @@ -668,14 +706,28 @@ async def _effective_sandbox( else: why = "reported by the caller" if not loses_unique: - recovered, detail = await asyncio.to_thread( - self._recover_dirty_tree_sync, str(self._source_dir), why + status, detail = await asyncio.to_thread( + self._recover_dirty_tree_sync, str(self._source_dir) ) + if status in ("refused", "error"): + # The action measures the criterion itself, so this is the freshest + # answer there is, and it governs: when it finds work that exists + # nowhere else - or cannot clear the tree - the cycle gets read-only + # even if the verdict above said otherwise. Without this, supplying a + # "no loss" verdict would buy a write-capable cycle on a tree holding + # unique work, which is the state the guard exists to prevent. + self._logger.warning( + "TaskHandler[%s]: dirty working tree — self-recovery did not clear " + "it (%s) — cycle forced read-only (structural guard, community " + "issue #979 / #1237, audited receipt)", + self.name, detail, + ) + return "read-only" self._logger.warning( "TaskHandler[%s]: dirty working tree holding no unique work (%s) — " "self-recovery %s; cycle keeps %s (structural guard, community " "issue #979 / #1237, audited receipt)", - self.name, why, detail if recovered else "did not complete", self._sandbox, + self.name, why, detail, self._sandbox, ) return self._sandbox self._logger.warning( diff --git a/scripts/recover-worktree.py b/scripts/recover-worktree.py index 33161194..1b90bb68 100644 --- a/scripts/recover-worktree.py +++ b/scripts/recover-worktree.py @@ -102,11 +102,11 @@ def recover(repo: Path, apply: bool) -> int: print(f"could not measure: {repo} is not a git repository with a commit") return 2 - status = _status_lines(repo) - if status is None: + entries = _status_lines(repo) + if entries is None: print(f"could not measure: `git status` failed in {repo}") return 2 - if not status: + if not entries: print(f"clean: {repo} has no uncommitted changes; nothing to recover") return 0 @@ -121,19 +121,31 @@ def recover(repo: Path, apply: bool) -> int: if not apply: print(f"recoverable: {why}") - print(f"{len(status)} entr(y|ies) would be stashed (reversible); re-run with --apply") + print(f"{len(entries)} entr(y|ies) would be stashed (reversible); re-run with --apply") return 0 # One owner for the action too: the daemon runs this exact function at the start - # of a cycle, so what a human runs here and what the guard does cannot drift. - ok, detail = TaskHandler._recover_dirty_tree_sync(str(repo), reason=why) - if not ok: + # of a cycle, so what a human runs here and what the guard does cannot drift. It + # accepts no verdict from a caller and re-measures the criterion itself, so a tree + # that changed between the diagnosis above and this line is judged on its current + # state rather than on the earlier answer. + status, detail = TaskHandler._recover_dirty_tree_sync(str(repo)) + if status == "refused": + print(f"refused: {repo} holds work that exists nowhere else: {detail}") + print("nothing was changed. Commit, stash or copy that work out first;") + print("this tool will not discard it.") + return 1 + if status == "error": print(f"could not measure: {detail}") return 2 + if status == "clean": + print(f"clean: {repo} has no uncommitted changes; nothing to recover") + return 0 receipt = _receipt_path(repo) print(f"recovered: {repo} converged to a clean tree; {detail}") - print("reversible: git stash pop") + print("reversible: `git stash list` -> the named stash (a bare `git stash pop`") + print("takes the newest, which is this one only until the next stash is made)") print(f"receipt: {receipt}" if receipt else "receipt: could not be written") return 0 diff --git a/tests/test_recover_worktree.py b/tests/test_recover_worktree.py index 1b4f98e6..53e5860f 100644 --- a/tests/test_recover_worktree.py +++ b/tests/test_recover_worktree.py @@ -14,6 +14,8 @@ import sys from pathlib import Path +import pytest + SCRIPT = Path(__file__).resolve().parent.parent / "scripts" / "recover-worktree.py" @@ -238,33 +240,71 @@ def test_the_tool_writes_a_receipt_of_what_it_moved(tmp_path): assert any(line.startswith(" M") for line in receipt["status_before"]) assert receipt["status_after"] == [] assert "stash" in receipt["action"] - assert "stash pop" in receipt["reversible_with"] + # The route must name *this* stash: a host may already have stashes, so a bare + # `git stash pop` is only correct until the next one is made (measured on the + # authoring workspace, which held an unrelated `stash@{0}` when this was written). + assert receipt["stash_message"] in receipt["reversible_with"] assert "upstream" in receipt["reason"] -def test_the_action_asks_the_criterion_itself_before_touching_anything(tmp_path): - """Defence in depth: the action refuses unique work without being told to. +def test_the_action_asks_the_criterion_itself_and_cannot_be_told_the_answer(tmp_path): + """Defence in depth: no caller's verdict can disarm the net (found in review, #1274). - Called with no measured verdict (the way a future caller who has not read this - module would call it), it must ask the criterion itself. Otherwise the safety of - the guard would rest on every caller remembering to check first -- and the one - that forgets would discard a host's unsaved work while the receipt called it a - recovery. + An independent review measured the earlier shape — the action accepted the + caller's already-measured verdict as an optional argument — stashing a tree that + held an untracked file, i.e. work that exists nowhere else, with the receipt + calling it a recovery. A guarantee that holds only while every caller passes the + truth is not a guarantee, so the parameter is gone: a caller that has not measured + now gets a ``TypeError`` instead of a silently wrong answer. """ repo = tmp_path / "repo" _new_repo(repo) (repo / "notes.md").write_text("only here", encoding="utf-8") - ok, detail = _load().TaskHandler._recover_dirty_tree_sync(str(repo)) - assert ok is False and "refused" in detail, detail + status, detail = _load().TaskHandler._recover_dirty_tree_sync(str(repo)) + assert status == "refused" and "notes.md" in detail, detail assert (repo / "notes.md").read_text(encoding="utf-8") == "only here" assert _git(repo, "stash", "list").stdout.strip() == "", "nothing may be moved aside" + # The reviewed exploit, verbatim: `_recover_dirty_tree_sync(repo, )`. + # It must not be expressible rather than merely discouraged. + with pytest.raises(TypeError): + _load().TaskHandler._recover_dirty_tree_sync(str(repo), "reported by the caller") + # The other direction, same entry point: reconstructible dirt does converge. work, head = _with_upstream(tmp_path) - ok, detail = _load().TaskHandler._recover_dirty_tree_sync(str(work)) - assert ok is True and "stash pop" in detail, detail + status, detail = _load().TaskHandler._recover_dirty_tree_sync(str(work)) + assert status == "recovered" and "stash" in detail, detail assert _status(work).strip() == "" + assert _git(work, "rev-parse", "HEAD").stdout.strip() == head, "HEAD must not move" + + +def test_the_four_outcomes_are_not_two(tmp_path): + """`(bool, detail)` collapsed three different answers into `False`; #1274 split them. + + A caller reading any non-recovered answer as "nothing happened" would report a + tree it just *refused* to touch as though the question had been settled, and one + reading `clean` as `recovered` would claim a convergence that never ran. So the + states are pinned apart, including the one that means "I could not answer". + """ + seen = {} + + clean = tmp_path / "clean" + _new_repo(clean) + seen["clean"], _ = _load().TaskHandler._recover_dirty_tree_sync(str(clean)) + assert _git(clean, "stash", "list").stdout.strip() == "", "a no-op makes no stash" + + unique = tmp_path / "unique" + _new_repo(unique) + (unique / "notes.md").write_text("only here", encoding="utf-8") + seen["refused"], _ = _load().TaskHandler._recover_dirty_tree_sync(str(unique)) + + plain = tmp_path / "plain" + plain.mkdir() + seen["error"], detail = _load().TaskHandler._recover_dirty_tree_sync(str(plain)) + assert "could not run" in detail, detail + + assert sorted(seen.values()) == ["clean", "error", "refused"], seen def test_a_clean_tree_is_a_no_op(tmp_path, capsys): diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index c2c96781..d8037f7a 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -2750,7 +2750,8 @@ def test_reconstructible_dirt_is_recovered_by_the_daemon_itself(tmp_path): assert receipt["head_before"] == receipt["head_after"] == head assert receipt["status_before"] == [" D f.txt"], receipt["status_before"] assert receipt["status_after"] == [] - assert "stash pop" in receipt["reversible_with"] + assert receipt["stash_message"] in receipt["reversible_with"], \ + "the reversal route must name the stash that was made, not the newest one" # Reversibility is why the action is allowed at all: popping restores the exact # state the daemon moved aside -- here, the deletion itself, byte for byte. @@ -2778,6 +2779,32 @@ def test_unique_dirt_still_forces_read_only(tmp_path): "no recovery ran, so there is no recovery to receipt" +def test_a_stale_verdict_cannot_unlock_a_tree_holding_unique_work(tmp_path): + """Found in review of #1274: the action's own measurement governs the tier. + + `loses_unique` is a test seam and an optimisation for the log line, so a caller + can fill it in -- and the earlier shape *believed* it: passing `loses_unique=False` + on a tree that does hold unique work bought the cycle `workspace-write` **and** + stashed the work, with the receipt calling it a recovery. The net held only while + every caller passed the truth, which is the same as not holding. + + Now the action re-measures the criterion itself and its refusal governs, so a + wrong or stale verdict is contradicted instead of honoured. + """ + repo = _repo_with_dirt(tmp_path, "untracked") + + handler = TaskHandler(name="emrg-task", config={"path": repo}, interval=60, + identity=InstanceIdentity()) + assert asyncio.run( + handler._effective_sandbox(dirty=True, loses_unique=False) + ) == "read-only", "a verdict supplied by the caller must not decide this" + + assert (tmp_path / "repo-untracked" / "notes.md").read_text(encoding="utf-8") == "only here" + assert _git_out(repo, "stash", "list") == "", "a refused action moves nothing" + assert not (Path(_git_out(repo, "rev-parse", "--absolute-git-dir")) + / "emrg-recovery-receipt.json").exists() + + def test_dirty_tree_override_env_audited_receipt(): """Community issue #979: EMRG_TASK_DIRTY_OVERRIDE (comma-separated task names, or *) lets a human lift the guard — every exception is logged as a From db972430b46f5f190edf9086c0626224aa99dbbc Mon Sep 17 00:00:00 2001 From: EMRG Evolution Date: Wed, 16 Sep 2026 11:50:30 +0800 Subject: [PATCH 4/4] emrg: the criterion measures the index side too, where the worktree cannot evidence it --- emrg/server/scheduler.py | 72 +++++++++++++++++++--- tests/test_recover_worktree.py | 107 +++++++++++++++++++++++++++++++++ tests/test_scheduler.py | 2 + 3 files changed, 172 insertions(+), 9 deletions(-) diff --git a/emrg/server/scheduler.py b/emrg/server/scheduler.py index 80c73e4f..ef92298b 100644 --- a/emrg/server/scheduler.py +++ b/emrg/server/scheduler.py @@ -401,6 +401,15 @@ def _dirty_tree_would_lose_work_sync(source_dir: str) -> tuple[bool, str]: * a **deletion** loses nothing (discarding restores the blob from ``HEAD``); * a **modification** is recoverable when its blob equals ``HEAD``'s or the upstream tip's for that path — the two places a blob can already live; + * a **staged** change is measured on the index side as well, because the + worktree cannot evidence it: in ``MM`` (staged, then the worktree copy + reverted to ``HEAD``) the worktree blob *is* ``HEAD``'s, and in ``MD`` + (staged, then the worktree copy removed) there is no worktree blob at all. + Measured before this clause existed: both answered "recoverable" while + `git log --all --find-object` found the staged blob in **no commit**, so + the tier was released over content a plain `git stash pop` then dropped as + an unreferenced object. Content must now be in ``HEAD`` or the upstream tip + on *both* sides before the tier is released; * a commit reachable only from this branch is unique: it is work that exists in no other checkout, so it forces ``read-only``. The reason is *not* the recovery - that is a stash, which is indifferent to commits (measured: a @@ -444,6 +453,10 @@ def git(*args: str): if not line.strip(): continue code, path = line[:2], line[3:].strip() + # Porcelain v1 is two columns: the index status and the worktree status. + # Naming them is what makes "which side is this fact about?" askable — + # several defects here came from reading the pair as one flag. + index_side, worktree_side = code[0], code[1] if code == "??" or "U" in code or code in ("AA", "DD"): unique.append(f"{path} exists only in this checkout") continue @@ -453,8 +466,47 @@ def git(*args: str): if "R" in code or "C" in code: unique.append(f"{path} was renamed") continue - if "D" in code: - continue # a deletion loses nothing: HEAD holds it + if index_side not in (" ", "D"): + # The index is the third place content can live, and the *worktree + # cannot evidence it*: `MM` is a staged change whose worktree copy was + # reverted to HEAD (so `hash-object` returns HEAD's own blob and the + # comparison below passes), and `MD` is a staged change whose worktree + # copy was removed (so there is no worktree blob to read at all). Both + # were measured answering "recoverable" while `git log --all + # --find-object` found the staged blob in **no commit** — the tier was + # released over content that a plain `git stash pop` then dropped as an + # unreferenced object (measured: index back to HEAD's blob, 4 + # unreachable objects). ' ' is HEAD's own blob and 'D' a staged + # deletion, so neither can hide content the index alone holds. + staged = git("ls-files", "-s", "--", path) + fields = staged.stdout.split() + if staged.returncode != 0 or len(fields) < 2: + unique.append(f"{path} is staged but its index content could not be read") + continue + index_digest = fields[1] + for ref in filter(None, ("HEAD", upstream)): + have = git("rev-parse", "--verify", "--quiet", f"{ref}:{path}") + if have.returncode == 0 and have.stdout.strip() == index_digest: + break + else: + unique.append( + f"{path} is staged with content that is in neither HEAD nor " + f"the upstream tip" + ) + continue + if index_side == "D" or worktree_side == "D": + # No worktree copy of this path exists, so there is nothing to read and + # nothing to lose — `HEAD` holds the blob, and whatever the index side + # held has just been measured above. + # + # Both halves of this are load-bearing, and each was measured by + # removing it: ` D` needs the worktree half. `D ` (a staged deletion, + # so the index has no entry *and* no file is on disk) needs the index + # half — without it the walk asks `hash-object` about a path with no + # file, reads the failure as "could not be read to compare", and + # refuses a tree that loses nothing. This is what a blanket + # `"D" in code` got right by accident and everything else wrong. + continue blob = git("hash-object", "--", path) if blob.returncode != 0 or not blob.stdout.strip(): unique.append(f"{path} could not be read to compare") @@ -620,14 +672,16 @@ def git(*args: str): "status_after": [], "stash_message": message, "action": "git stash push --include-untracked", - # The named form is the primary one: a host may already have stashes, and - # a bare `git stash pop` is only correct while this stash is the newest - # (measured on the authoring workspace, which had an unrelated - # `stash@{0}` when this was written). + # `--index` is not decoration: a stash carries the index side as well, and + # without it a stash whose change is index-only is popped as "Already up + # to date." and **dropped**, leaving that blob unreferenced (measured: + # plain `pop` left the index at HEAD's blob and 4 unreachable objects + # behind, while `pop --index` restored the staged content byte for byte). "reversible_with": ( - f"`git stash list` -> {message}, then `git stash apply stash^{{/{message}}}`" - f" (a bare `git stash pop` takes the newest, which is this one" - f" only until the next stash is made)" + f"`git stash list` -> {message}, then " + f"`git stash apply --index stash^{{/{message}}}` (`--index` restores " + f"the staged side too, and a bare `git stash pop` takes the newest, " + f"which is this one only until the next stash is made)" ), }) if head_after != head.stdout.strip(): diff --git a/tests/test_recover_worktree.py b/tests/test_recover_worktree.py index 53e5860f..e5352a6f 100644 --- a/tests/test_recover_worktree.py +++ b/tests/test_recover_worktree.py @@ -157,6 +157,108 @@ def test_a_deletion_loses_nothing(tmp_path): assert loses is False, why +def test_a_staged_deletion_loses_nothing(tmp_path): + """The same verdict on the *index* side of a deletion (`D `, not ` D`). + + Pinned because the clause that skips deletions was narrowed from `"D" in code` + to the two sides that actually lose nothing. `D ` is the case that a careless + narrowing drops: the worktree copy is gone as well, so a check that looked for a + worktree blob would answer "could not be read to compare" and refuse a tree that + loses nothing. Both spellings are deletions; both must stay recoverable. + """ + _new_repo(tmp_path / "repo") + repo = tmp_path / "repo" + _git(repo, "rm", "-q", "--cached", "f.txt") + (repo / "f.txt").unlink() + assert _status(repo) == "D f.txt\n", _status(repo) + loses, why = _load().TaskHandler._dirty_tree_would_lose_work_sync(str(repo)) + assert loses is False, why + + +def test_staged_content_the_worktree_cannot_evidence_is_unique(tmp_path): + """`MM`: staged, then the worktree copy reverted to HEAD (review of #1274). + + The worktree blob *is* HEAD's, so a measurement that reads only the worktree + answers "recoverable" — while the staged blob is in no commit at all, and a plain + `git stash pop` then drops it as an unreferenced object (measured: the index went + back to HEAD's blob and the unique bytes became unreachable). The criterion must + measure the index side too. + """ + repo = tmp_path / "repo" + _new_repo(repo) + (repo / "f.txt").write_text("STAGED-ONLY-UNIQUE", encoding="utf-8") + _git(repo, "add", "f.txt") + (repo / "f.txt").write_text("v1", encoding="utf-8") # back to HEAD's own bytes + assert _status(repo) == "MM f.txt\n", _status(repo) + + staged = _git(repo, "rev-parse", ":f.txt").stdout.strip() + reachable = _git(repo, "log", "--all", "--oneline", f"--find-object={staged}") + assert reachable.stdout.strip() == "", "precondition: the staged blob is in no commit" + + loses, why = _load().TaskHandler._dirty_tree_would_lose_work_sync(str(repo)) + assert loses is True, why + assert "staged" in why, why + + # And the action obeys that verdict: nothing may be moved aside. + status, detail = _load().TaskHandler._recover_dirty_tree_sync(str(repo)) + assert status == "refused", f"{status}: {detail}" + assert _git(repo, "stash", "list").stdout.strip() == "" + assert _git(repo, "rev-parse", ":f.txt").stdout.strip() == staged, \ + "the staged blob must still be in the index" + + +def test_staged_content_with_no_worktree_copy_is_unique(tmp_path): + """`MD`: staged, then the worktree copy removed — the same hole, other spelling. + + The worktree cannot evidence the index here for a different reason (there is no + worktree blob at all), which is why the deletion clause had to be narrowed rather + than left to skip anything containing a `D`. + """ + repo = tmp_path / "repo" + _new_repo(repo) + (repo / "f.txt").write_text("STAGED-ONLY-UNIQUE", encoding="utf-8") + _git(repo, "add", "f.txt") + (repo / "f.txt").unlink() + assert _status(repo) == "MD f.txt\n", _status(repo) + + staged = _git(repo, "rev-parse", ":f.txt").stdout.strip() + assert _git(repo, "log", "--all", "--oneline", + f"--find-object={staged}").stdout.strip() == "" + + loses, why = _load().TaskHandler._dirty_tree_would_lose_work_sync(str(repo)) + assert loses is True, why + assert "staged" in why, why + + +def test_staged_content_upstream_already_holds_is_recoverable(tmp_path): + """The other direction: the index clause must not refuse published content. + + Both states carry an index blob that equals the **upstream tip's** blob for that + path, so discarding loses nothing (the bytes are in a commit anyone can reach). + A clause that refused every staged change would trade the #1274 hole for the + over-block class #1273 tracks — the two are not interchangeable. + """ + for kind in ("mm", "md"): + work, _head = _with_upstream(tmp_path / kind) + published = _git(work, "rev-parse", "origin/master:f.txt").stdout.strip() + # Stage upstream's own blob for that path (HEAD still holds v1's). + staged = _git(work, "update-index", "--cacheinfo", f"100644,{published},f.txt") + assert staged.returncode == 0, staged.stderr + if kind == "md": + (work / "f.txt").unlink() + else: + # Back to HEAD's bytes, so the worktree cannot evidence the index either — + # the same `MM` geometry as the unique case above, differing only in + # whether a commit already holds the staged blob. + (work / "f.txt").write_text( + _git(work, "show", "HEAD:f.txt").stdout, encoding="utf-8" + ) + assert _status(work) == f"{'MM' if kind == 'mm' else 'MD'} f.txt\n", _status(work) + + loses, why = _load().TaskHandler._dirty_tree_would_lose_work_sync(str(work)) + assert loses is False, f"{kind}: {why}" + + def test_a_commit_only_on_this_branch_is_unique(tmp_path): """A branch reset would orphan it: dirty *and* ahead is never reconstructible.""" origin = tmp_path / "origin.git" @@ -244,6 +346,11 @@ def test_the_tool_writes_a_receipt_of_what_it_moved(tmp_path): # `git stash pop` is only correct until the next one is made (measured on the # authoring workspace, which held an unrelated `stash@{0}` when this was written). assert receipt["stash_message"] in receipt["reversible_with"] + # A stash carries the index side as well, and a plain pop does not restore it: + # measured on an index-only change, `stash pop` printed "Already up to date.", + # dropped the stash and left the index at HEAD's blob (#1274 review). The + # documented inverse must therefore be the `--index` spelling. + assert "--index" in receipt["reversible_with"] assert "upstream" in receipt["reason"] diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index d8037f7a..d3acf295 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -2752,6 +2752,8 @@ def test_reconstructible_dirt_is_recovered_by_the_daemon_itself(tmp_path): assert receipt["status_after"] == [] assert receipt["stash_message"] in receipt["reversible_with"], \ "the reversal route must name the stash that was made, not the newest one" + assert "--index" in receipt["reversible_with"], \ + "a stash carries the index side; the inverse must restore it" # Reversibility is why the action is allowed at all: popping restores the exact # state the daemon moved aside -- here, the deletion itself, byte for byte.