feat: Crash recovery salvage: persist verified candidate + resume (not rebuild) + orphan sweep .gN fix (#91)

coder_seam.py

@@ -873,6 +873,11 @@ async def _run_acceptance_tests(self, candidate_wt: str):
 
 
 RecordGens = Callable[[int], None]
+# The verify-boundary salvage record (#91): called with (branch, sha, worktree) once a
+# candidate has PASSED its acceptance tests and been promoted — the loop persists it on
+# the bead (store.record_verified_candidate) so a crash between here and open_pr can
+# resume the verified build instead of rebuilding fresh.
+RecordVerified = Callable[[str, str, str], None]
 
 
 def _record_gens_best_effort(record_gens: RecordGens, fid: str, n: int) -> None:
@@ -912,6 +917,8 @@ async def dispatch(
     fusion_max_file_chars: int = FUSION_MAX_FILE_CHARS_DEFAULT,
     env_passthrough: Iterable[str] = (),
     tier: str = "",
+    record_verified: RecordVerified | None = None,
+    commit_message: str = "",
     _solve=None,
     _budget_cls=None,
     _verdict_cls=None,
@@ -930,6 +937,12 @@ async def dispatch(
 
     ``record_gens`` (if given) is called with ``result.gens_spent`` exactly once,
     win or lose — the cost accounting (ADR 0064) must survive a failed search too.
+    ``record_verified`` (if given) is called once with the promoted winner's
+    ``(branch, sha, worktree)`` at the verify boundary — the crash-salvage record
+    (#91), persisted by the loop so a crash between here and ``open_pr`` resumes the
+    verified build instead of rebuilding fresh. ``commit_message`` names the commit
+    that gives the verified tree its sha (the loop passes the PR title, so the
+    shipped commit message is unchanged from what ``open_pr`` would have written).
     ``_solve``/``_budget_cls``/``_verdict_cls`` are test-injection seams for
     ``solve()``/``Budget``/``Verdict``; production callers never pass them (the real
     import happens here, deferred so this module carries no hard dependency on the
@@ -1041,6 +1054,22 @@ async def dispatch(
     win_wt = result.solution
     win_branch = next(b for wt, b in adapter.candidates if wt == win_wt)
     canon_wt, canon_branch = await worktree.promote_worktree(repo, win_wt, win_branch, fid, root)
+    # The verify boundary's crash-salvage record (#91): the candidate PASSED its
+    # acceptance tests but open_pr is still ahead (fixups + the pre-PR gate can take
+    # minutes) — a crash in that window used to throw the whole verified build away.
+    # Commit the verified tree so its content has a real sha, then persist
+    # {branch, sha, worktree} via `record_verified` (the loop writes it on the bead)
+    # so recovery can resume at promote→fixups→gate→open_pr instead of re-solving.
+    # Best-effort: a bookkeeping failure must never fail a build that already passed.
+    if record_verified is not None:
+        try:
+            await worktree.commit_worktree(canon_wt, commit_message or f"feat: {fid} (verified candidate)")
+            rc, head, _err = await worktree._git(canon_wt, "rev-parse", "HEAD")
+            sha = (head or "").strip()
+            if rc == 0 and sha:
+                record_verified(canon_branch, sha, canon_wt)
+        except Exception:  # noqa: BLE001 — fire-and-forget salvage bookkeeping
+            log.warning("[project_board] %s could not record the verified candidate (ignored)", fid, exc_info=True)
     for wt, branch in adapter.candidates:
         if wt != win_wt:
             await worktree.remove_worktree(repo, wt, branch)

loop.py

@@ -518,18 +518,94 @@ async def stop(self):
     # ── crash recovery (runs once, before the puller claims new work) ──────────
     async def _reconcile_orphan(self, fid: str):
         """A claimed feature with no live drive: if its PR actually got opened (a crash
-        between ``open_pr`` and ``open_review``) adopt it → ``in_review``; otherwise
-        reset it to ``ready`` for a clean rebuild (a stale worktree is cleaned when the
+        between ``open_pr`` and ``open_review``) adopt it → ``in_review``; else, if a
+        VERIFIED candidate was recorded at coder_seam's verify boundary and still checks
+        out on disk (a crash between verify and ``open_pr``), salvage it — resume at
+        promote → fixups → gate → open_pr instead of re-solving (#91); otherwise reset
+        it to ``ready`` for a clean rebuild (a stale worktree is cleaned when the
         puller re-claims it). Shared by boot recovery and the health sweep."""
         store = self._store()
         pr_url = await worktree.pr_url_for_branch(f"feat/{fid}", cwd=self._store_kw["repo"])
         if pr_url:
             store.open_review(fid, pr_url=pr_url)
             log.info("[project_board] %s already had a PR → in_review (%s)", fid, pr_url)
+        elif await self._salvage_verified_candidate(store, fid):
+            pass  # resumed + PR opened → in_review (logged inside)
         else:
             store.requeue(fid)
             log.info("[project_board] %s reset to ready (no PR — rebuild fresh)", fid)
 
+    @staticmethod
+    def _clear_verified(store, fid: str) -> None:
+        """Best-effort drop of the salvage record — bookkeeping only, never raises."""
+        try:
+            store.clear_verified_candidate(fid)
+        except Exception:  # noqa: BLE001 — a failed clear must not fail recovery
+            log.warning("[project_board] %s clear_verified_candidate failed (ignored)", fid, exc_info=True)
+
+    async def _salvage_verified_candidate(self, store, fid: str) -> bool:
+        """Crash salvage (#91): resume a build whose candidate already PASSED its
+        acceptance tests but crashed before ``open_pr``.
+
+        ``coder_seam.dispatch`` records the verified candidate at its verify boundary
+        (a ``verified:<sha>`` label + a bead comment with {branch, sha, worktree}). If
+        that record still checks out EXACTLY — the canonical worktree dir exists, it
+        has the recorded branch checked out at the recorded sha, and the pre-PR gate
+        passes on it NOW — resume the tail of the drive (promote → fixups → gate →
+        open_pr → in_review) instead of throwing a verified build away to re-solve.
+        ANY doubt (no record, worktree gone, branch/sha drift, gate red now, any error
+        anywhere) → False, and the caller falls through to today's rebuild-fresh
+        unchanged — a wrong salvage ships unverified code; a skipped one only costs a
+        rebuild."""
+        try:
+            f = store.get_feature(fid) or {}
+            sha = str(f.get("verified_sha") or "").strip()
+            if not sha:
+                return False
+            repo = f.get("repo") or self._store_kw["repo"]
+            base = f.get("base_branch") or self._store_kw.get("base_branch") or "main"
+            branch = f"feat/{fid}"
+            wt = os.path.join(repo, self.root, f"feat-{fid}")
+            if not os.path.isdir(wt):
+                log.info("[project_board] %s salvage: verified worktree gone — rebuild fresh", fid)
+                self._clear_verified(store, fid)
+                return False
+            rc, head, _err = await worktree._git(wt, "rev-parse", "HEAD")
+            if rc != 0 or head.strip() != sha:
+                log.info(
+                    "[project_board] %s salvage: sha drift (%s ≠ recorded %s) — rebuild fresh",
+                    fid,
+                    head.strip()[:12],
+                    sha[:12],
+                )
+                self._clear_verified(store, fid)
+                return False
+            rc, cur, _err = await worktree._git(wt, "rev-parse", "--abbrev-ref", "HEAD")
+            if rc != 0 or cur.strip() != branch:
+                log.info("[project_board] %s salvage: branch drift (%s ≠ %s) — rebuild fresh", fid, cur.strip(), branch)
+                self._clear_verified(store, fid)
+                return False
+            # Resume the drive's tail in its normal order: promote (a no-op — the
+            # record is written post-promote, so the candidate already holds the
+            # canonical name) → fixups → gate → open_pr. The gate re-runs NOW: a
+            # candidate that verified before the crash but fails today (base moved,
+            # env changed) is a doubt, not a ship.
+            wt, branch = await worktree.promote_worktree(repo, wt, branch, fid, self.root)
+            await self._run_fixups(wt)
+            if await self._run_local_gate(wt) is not None:
+                log.info("[project_board] %s salvage: gate fails on the candidate now — rebuild fresh", fid)
+                self._clear_verified(store, fid)
+                return False
+            title = f"feat: {f.get('title') or fid}"
+            pr_url = await worktree.open_pr(wt, branch, base=base, title=title, body=_pr_body("", f))
+            store.open_review(fid, pr_url=pr_url)
+            self._clear_verified(store, fid)
+            log.info("[project_board] %s salvaged the verified candidate → %s (no re-solve)", fid, pr_url)
+            return True
+        except Exception:  # noqa: BLE001 — ANY doubt/error → today's rebuild-fresh path
+            log.warning("[project_board] %s salvage attempt failed — rebuild fresh", fid, exc_info=True)
+            return False
+
     async def _recover(self):
         """On boot, reconcile every ``in_progress`` feature the previous run left
         mid-drive (a drive doesn't survive a restart). ``in_review`` features are NOT
@@ -567,16 +643,22 @@ async def _sweep(self):
                 await self._reconcile_orphan(fid)
             except Exception:  # noqa: BLE001
                 log.warning("[project_board] sweep reconcile for %s failed", fid, exc_info=True)
-        for fid in worktree.list_feature_worktrees(repo, self.root):
+        for wtid in worktree.list_feature_worktrees(repo, self.root):
+            # A `.gN`/`.cN` candidate worktree is not a feature id (bd-1cp.g1) — its
+            # board state lives on the PARENT feature, so resolve through that (#91):
+            # skip while the parent's drive is live, reap when the parent is gone or
+            # done. The old raw-id `get_feature` lookup failed every sweep and just
+            # warned forever without ever reaping the candidate.
+            fid = worktree.parent_feature_id(wtid)
             if fid in self._inflight_files:
-                continue  # a live drive owns this worktree
+                continue  # a live drive owns this worktree (or its candidates)
             try:
                 f = store.get_feature(fid)
                 if f is None or f["board_state"] == "done":
-                    await worktree.reap_feature_worktree(repo, self.root, fid)
-                    log.info("[project_board] sweep: reaped orphaned worktree feat-%s", fid)
+                    await worktree.reap_feature_worktree(repo, self.root, wtid)
+                    log.info("[project_board] sweep: reaped orphaned worktree feat-%s", wtid)
             except Exception:  # noqa: BLE001
-                log.warning("[project_board] sweep reap for %s failed", fid, exc_info=True)
+                log.warning("[project_board] sweep reap for %s failed", wtid, exc_info=True)
 
     # ── the puller ────────────────────────────────────────────────────────────
     async def _run(self):
@@ -963,6 +1045,12 @@ async def _drive(self, feature: dict):
                             # get — keep the whitelist consistent across every subprocess.
                             env_passthrough=self.env_passthrough,
                             tier=tier,  # #84: label each solve gen with the current tier
+                            # #91: persist the verified candidate on the bead at the
+                            # verify boundary, so a crash before open_pr is salvageable.
+                            record_verified=lambda br_name, sha, wt_path: store.record_verified_candidate(
+                                fid, branch=br_name, sha=sha, worktree=wt_path
+                            ),
+                            commit_message=title,
                         )
                         self._inflight[fid] = (repo, wt, branch)
                     elif self.max_mode_n > 1 and not self._ci_feedback.get(fid):

store.py

@@ -90,6 +90,14 @@
 # seam) — `gens:<total>`, replaced (not accumulated as separate labels) each time so a
 # single label always carries the running total for `portfolio_rollup` to read.
 LABEL_GENS_PREFIX = "gens:"
+# Crash-salvage record (#91) — `verified:<sha>`, replaced (never accumulated) each time
+# coder.solve()'s verify boundary promotes a test-PASSING candidate. Written on the bead
+# (not loop memory) so it survives a crash between verify and open_pr; recovery's no-PR
+# path checks it and resumes at promote→fixups→gate→open_pr instead of rebuilding fresh.
+# The branch/worktree are the CANONICAL `feat/<id>` / `feat-<id>` names (the record is
+# written post-promote), so the sha is the only piece that must ride the label; the full
+# {branch, sha, worktree} triple lands in a comment for the audit trail.
+LABEL_VERIFIED_PREFIX = "verified:"
 
 # difficulty → initial model tier (the escalation ladder's first rung, D10).
 DIFFICULTY_TIER = {"small": "smart", "medium": "reasoning", "large": "reasoning", "architectural": "opus"}
@@ -685,6 +693,37 @@ def record_gens_spent(self, fid: str, n: int) -> dict:
         self._run(*args)
         return self.get_feature(fid)
 
+    # ── verified-candidate salvage record (#91) ───────────────────────────────
+    def record_verified_candidate(self, fid: str, *, branch: str, sha: str, worktree: str) -> dict:
+        """Persist the verified candidate's identity — a single, replaced
+        `verified:<sha>` label (the `gens:` pattern) plus a comment carrying the full
+        {branch, sha, worktree} — written at coder_seam's verify boundary so a crash
+        between verify and open_pr can salvage the already-test-passing build instead
+        of rebuilding fresh. Fire-and-forget like record_gens_spent: a `br` hiccup
+        here must never fail a build whose tests already passed."""
+        f = self._require(fid)
+        args = ["update", fid]
+        for stale in [l for l in f.get("labels") or [] if l.startswith(LABEL_VERIFIED_PREFIX)]:
+            args += ["--remove-label", stale]
+        args += ["--add-label", f"{LABEL_VERIFIED_PREFIX}{sha}"]
+        self._run(*args)
+        self._comment(fid, f"verified candidate: branch={branch} sha={sha} worktree={worktree}")
+        return self.get_feature(fid)
+
+    def clear_verified_candidate(self, fid: str) -> dict:
+        """Drop the `verified:` salvage record — the crash window it covers has closed
+        (the PR opened) or the record failed its recovery checks (worktree/sha drift),
+        so it must not linger to confuse a later recovery. No-op without the label."""
+        f = self._require(fid)
+        stale = [l for l in f.get("labels") or [] if l.startswith(LABEL_VERIFIED_PREFIX)]
+        if not stale:
+            return f
+        args = ["update", fid]
+        for label in stale:
+            args += ["--remove-label", label]
+        self._run(*args)
+        return self.get_feature(fid)
+
     # ── reads (the projection) ────────────────────────────────────────────────
     def get_feature(self, fid: str) -> dict | None:
         rows = self._run("show", fid, want_json=True)
@@ -835,6 +874,12 @@ def _project(self, bead: dict) -> dict:
             ),
             0,
         )
+        # The crash-salvage record (#91): the sha of the last test-verified candidate,
+        # from the single replaced `verified:<sha>` label — "" when none was recorded.
+        verified_sha = next(
+            (l[len(LABEL_VERIFIED_PREFIX) :] for l in labels if l.startswith(LABEL_VERIFIED_PREFIX)),
+            "",
+        )
         # `dag_blocked`: marked `ready` but a `blocks` dependency is still open, so
         # the puller won't claim it. Only `br show` carries dependencies (`br list`
         # doesn't); list_features patches this by cross-referencing the puller.
@@ -864,6 +909,7 @@ def _project(self, bead: dict) -> dict:
             "difficulty": diff,
             "attempts": attempts,
             "gens_spent": gens_spent,
+            "verified_sha": verified_sha,
             "labels": labels,
             "repo": self.repo,
             "base_branch": self.base_branch,

tests/test_coder_seam.py

@@ -224,6 +224,98 @@ async def _fake_solve(task, *, generate, verify, budget, k, tree_depth, fusion_g
     assert gens == [1]
 
 
+async def test_dispatch_records_the_verified_candidate_at_the_verify_boundary(monkeypatch):
+    """#91: once a candidate PASSES its tests and is promoted, dispatch() commits the
+    verified tree (so its content has a real sha — the loop's PR title keeps the
+    shipped commit message unchanged) and hands {branch, sha, worktree} to
+    ``record_verified`` — the crash-salvage record recovery resumes from if the
+    process dies before open_pr."""
+    _stub_worktree(monkeypatch)
+    committed = []
+
+    async def _commit(wt, message):
+        committed.append((wt, message))
+
+    monkeypatch.setattr(worktree, "commit_worktree", _commit)
+
+    async def _git(wt, *args, timeout=60):
+        assert args == ("rev-parse", "HEAD") and wt == "/wt/feat-bd-1"
+        return (0, "abc123\n", "")
+
+    monkeypatch.setattr(worktree, "_git", _git)
+
+    async def _fake_solve(task, *, generate, verify, budget, k, tree_depth, fusion_generate=None, fusion_k=2):
+        c0 = await generate(task, feedback=None)
+        return _FakeResult(solution=c0, passed=True, rung="greedy", gens_spent=1, candidates_tried=1)
+
+    recorded = []
+    wt, branch, _result = await dispatch(
+        task="t",
+        coder=object(),
+        repo="/repo",
+        base="main",
+        root=".worktrees",
+        fid="bd-1",
+        dispatch_timeout=None,
+        test_cmd="pytest -q",
+        test_timeout=30,
+        budget=6,
+        k=3,
+        tree_depth=2,
+        record_verified=lambda b, s, w: recorded.append((b, s, w)),
+        commit_message="feat: the title",
+        _solve=_fake_solve,
+        _budget_cls=_FakeBudget,
+        _verdict_cls=_FakeVerdict,
+    )
+    assert committed == [("/wt/feat-bd-1", "feat: the title")]  # the CANONICAL (promoted) tree
+    assert recorded == [("feat/bd-1", "abc123", "/wt/feat-bd-1")]
+    assert (wt, branch) == ("/wt/feat-bd-1", "feat/bd-1")
+
+
+async def test_dispatch_returns_the_winner_even_if_record_verified_raises(monkeypatch):
+    """The salvage record is fire-and-forget bookkeeping (like record_gens): a `br`
+    hiccup persisting it must never discard a build whose tests already passed."""
+    _stub_worktree(monkeypatch)
+
+    async def _commit(wt, message):
+        pass
+
+    monkeypatch.setattr(worktree, "commit_worktree", _commit)
+
+    async def _git(wt, *args, timeout=60):
+        return (0, "abc123\n", "")
+
+    monkeypatch.setattr(worktree, "_git", _git)
+
+    async def _fake_solve(task, *, generate, verify, budget, k, tree_depth, fusion_generate=None, fusion_k=2):
+        c0 = await generate(task, feedback=None)
+        return _FakeResult(solution=c0, passed=True, rung="greedy", gens_spent=1, candidates_tried=1)
+
+    def _boom(b, s, w):
+        raise RuntimeError("br hiccup: lock contention")
+
+    wt, branch, _result = await dispatch(
+        task="t",
+        coder=object(),
+        repo="/repo",
+        base="main",
+        root=".worktrees",
+        fid="bd-1",
+        dispatch_timeout=None,
+        test_cmd="pytest -q",
+        test_timeout=30,
+        budget=6,
+        k=3,
+        tree_depth=2,
+        record_verified=_boom,
+        _solve=_fake_solve,
+        _budget_cls=_FakeBudget,
+        _verdict_cls=_FakeVerdict,
+    )
+    assert (wt, branch) == ("/wt/feat-bd-1", "feat/bd-1")  # dispatch() itself never raised
+
+
 async def test_dispatch_promotes_the_winner_even_if_record_gens_raises(monkeypatch):
     """`store.record_gens_spent` documents itself as fire-and-forget ("a br hiccup
     here must never fail the build the way a missing PR would") — a `BoardError`