Wrap-up: Phase 3a — split the watch predicate from the merge gate

config/dev-model.yaml

@@ -103,19 +103,20 @@ review:
     - "actionable comments posted: 0"
     - "<!-- linear-linkback -->"
   # Bodies signalling the reviewer could not run. Deliberately NOT noise: these
-  # surface and block `done`, because a blocked bot is an action signal (run the
-  # configured fallback), never a silent review waiver.
+  # surface and block `converged`, because a blocked bot is an action signal (run
+  # the configured fallback), never a silent review waiver.
   unavailable_markers:
     - "bugbot needs on-demand usage enabled"
     - "review limit reached"
     - "rate limited by coderabbit"
     - "couldn't start this review"
     - "review skipped"
     - "no review credits"
-  # Status checks that are advisory only and must never block `done`. A review
-  # bot's check can sit PENDING forever after a trivial follow-up commit; its real
-  # findings arrive as comments (which DO block). Matched case-insensitively
-  # against the check name.
+  # Status checks that are advisory only and must never block `converged` — this
+  # is the anti-wedge property: a review bot's check can sit PENDING forever after
+  # a trivial follow-up commit, and the watch loop must still be able to finish.
+  # Its real findings arrive as comments (which DO block). Matched
+  # case-insensitively against the check name.
   informational_checks: [coderabbit]
   # Whether a PR must have at least one real (non-informational) CI check before
   # pr_watch will report green. True is the safe default: it stops an autonomous

docs/kit-friction-log.md

@@ -13,6 +13,40 @@
 
 ## 2026-07-25 — inbox
 
+- **The fallback review pass has no independence when the cockpit authored the PR (severity: H).**
+  On #22 CodeRabbit was rate-limited, so `review.fallback_commands` ran — but the agent
+  running it was the same one that wrote the diff. It found three real issues, yet
+  `safety-critical-changes.md` rule 2 is explicit that this is not a pass: "A single-lens
+  'converged' verdict is an incomplete review, not a green light." Rule 3 also went unmet —
+  the fallback's own approve was written in the same pass that produced the final commit, so
+  no *independent* pass ever covered `32f3e4f` — the fallback did see that code (it posted
+  seconds after the commit, naming it), but a self-review re-reading its own fixes is not a
+  rule-3 re-review. **Proposed fix:** make `review.fallback_commands` a *panel*
+  spec rather than an inline command — one fresh-context subagent per lens
+  (adversarial/bypass-focused + general-correctness, the two the doctrine says find
+  **disjoint** holes), each handed the raw diff with no framing from the author, and a
+  distinct receipt source (`fallback:panel`) so the audit trail does not read as a primary
+  review. **Evidence, with the conflict of interest stated:** the panel was trialled on the
+  wrap-up PR that carries this entry (#24), so this is self-reported. What it produced is
+  checkable in that PR's diff — a correctness lens found a merge-gate comment in
+  `dev_session.sh` describing the *rejected* design, shipped to `main` in #22 and missed by
+  the author's three passes over it; a second adversarial lens then found nine further
+  issues in the wrap-up, overlapping the first on none of them. Filed at **H** rather than
+  the M this would have got before that trial: a documented rule is being violated, not a
+  soft gap.
+
+- **A safety-critical PR merged without any review of its final design (severity: H).**
+  Also #22: CodeRabbit's only completed review was bound to the first commit; the design then
+  changed materially (the fail-open rework), and the rate limit never lifted. The receipt
+  mechanism records *that a review happened at this head* — satisfied by the fallback — with
+  no notion of "the primary reviewer saw an earlier, materially different design."
+  **Proposed fix:** invalidate a receipt when the diff changes *shape* rather than only when
+  the head moves — e.g. require a fresh receipt when a later push touches a file the recorded
+  review never saw. Cheaper interim: have `pr_watch` surface the primary bot's last-reviewed
+  SHA next to the current head, so the gap is visible at merge time instead of reconstructible
+  only from the PR thread. Related to #23 but distinct: #23 is about detecting the outage,
+  this is about what a receipt should mean when the outage outlasts a redesign.
+
 - **The `cp -r` quickstart can't distinguish kit-owned from adopter-owned files (severity: M).**
   Any file the kit tracks lands in an adopter's repo, which is why this repo's own narrative
   docs had to be renamed `kit-*.md` rather than simply filled in. `kit-manifest.json` now

docs/kit-handoff.md

@@ -14,11 +14,102 @@
 > Older session blocks graduate to [`kit-handoff-history.md`](kit-handoff-history.md) once
 > this file crosses its line budget (`scripts/check_doc_budget.py`).
 
-Last updated: 2026-07-25 — first full assessment-and-remediation pass; the kit now fixes
-its own adoption path, and can diagnose an installed copy.
+Last updated: 2026-07-25 — Phase 3a shipped: the watch-loop predicate and the merge gate
+are now separate, and the schema change that separates them is purely additive.
 
 ## Latest session — 2026-07-25
 
+**Theme —** Made the Phase 3 sequencing decision, and it changed under scrutiny — twice.
+Both times the correction came from asking what a *stale reader* of the mechanism would do.
+
+> **What "Phase 3" and "the cs-toolkit back-port" mean.** cs-toolkit
+> (`/Users/topi/Coding/in-parallel/cs-toolkit`, a separate private repo) is where this kit's
+> mechanisms originated; the kit generalized them, and the back-port is returning the
+> improved versions. Phase 3 is the review-receipt + merge-gate slice of that. The vocabulary
+> has never been written down outside this handoff, which made the claim below unverifiable
+> from inside this repo — recorded here so the next session doesn't have to reconstruct it.
+
+- **The blocking problem was not the porting order.** `decide_done` conflated "is there
+  more for me to fix?" with "is this authorized to merge?", because `cmd_merge` had no
+  other hook — it re-polled `pr_watch --json` and gated on `done`. That conflation, not the
+  sequence of ports, is what would have wedged cs-toolkit's nightly fixer — its per-lane
+  review step (`.claude/commands/nightly-fixer.md` Step 6.2 in that repo) watches to
+  green-and-clean and records no receipt. Fixing it removed a whole phase from the plan.
+- **#22 merged.** `converged` (watch loop) and `mergeable` (merge gate) are now distinct;
+  `dev_session.sh merge` gates on `mergeable`. Tests 196 → 202.
+- **The first cut of #22 failed open, and my own adversarial re-read caught it — not
+  CodeRabbit, whose pass on that commit raised only a `local`-declaration nit and a test
+  nitpick.** It redefined `done` to
+  mean watch-convergence. Because `/upgrade` refreshes engines **per file** (`missing` is a
+  supported state — "a sized-down adoption omits engines deliberately (one surveyed repo
+  installs 2 of 6 on purpose)"), a new
+  `pr_watch.py` can run against an older `dev_session.sh` whose gate reads `done` — which
+  would then have authorized merges on PRs with no review receipt at all.
+- **So the schema only grows.** `done` stays an unchanged alias of `mergeable`, and both
+  skew directions fail closed. Note what is pinned where: the *function* `decide_done` is
+  held to the pre-split expression across all 32 boolean inputs, but the thing that actually
+  protects an older `dev_session.sh` is the report **key**, and that is pinned by a matrix of
+  report shapes rather than exhaustively. Worth keeping straight — the same function-vs-key
+  confusion is the next bullet's finding.
+- **CodeRabbit was rate-limited**, so the configured fallback pass ran instead. It found
+  three further issues, including a docstring that claimed a compatibility guarantee the
+  *function* doesn't provide — the report **key** does.
+
+**Decided**
+
+- Enforce at the merge point, never at `converged`. A watch loop asking "anything left to
+  fix?" should never be answered "no" only once a review receipt exists.
+- A field that a safety gate reads may be added to, never redefined.
+- #19 and #23 get designed together — they are the same ambiguity on two surfaces, and
+  both run into the informational-check exclusion being load-bearing against wedging.
+
+**Learned**
+
+- **Documentation does not reach a stale reader.** Redefining `done` was safe by every
+  local reading of the new code and unsafe in fact, because the component that would have
+  been wrong is the one that never sees the new docs. Version skew is not hypothetical
+  here: per-file engine upgrades are a supported, documented workflow.
+- **An unavailable reviewer can be indistinguishable from a clean one.** CodeRabbit's
+  rate-limit arrived as a status-check *description* on a check classified informational,
+  so nothing surfaced it (#23). The doctrine's "a blocked bot is an action signal" rule can
+  only fire if the outage is detected.
+- **#22 merged without satisfying review rules 2 or 3, and that should be recorded as a
+  violation rather than a compromise.** The doctrine has no "floor" the author's own pass
+  can meet: rule 2 says a single-lens verdict is "not a green light", and rule 3 wants
+  re-review until a pass finds nothing new — but the fallback's approve was written in the
+  same pass that produced `32f3e4f`, so no *independent* review ever covered the final
+  commit — the fallback saw that code, but only as the author re-reading their own fixes.
+  CodeRabbit's only review was bound to the first commit, before the redesign.
+- **A cold-context subagent reviewer found what three self-review passes missed** — a stale
+  comment on the merge gate itself (`dev_session.sh` `cmd_merge`), describing the design that
+  was rejected. It shipped to `main` in #22 and was fixed in the wrap-up PR (#24), so the
+  artifact is visible only in that diff. Authorship anchoring, not capability, is what
+  self-review cannot escape.
+- A second, adversarially-prompted subagent pass then found nine further issues in the
+  wrap-up itself — including this handoff misattributing a test-coverage claim, and the PR
+  description still carrying the "floor" framing the diff had already retracted. The two
+  lenses overlapped on nothing, which is the doctrine's disjointness claim holding up.
+
+**Open, and owned by nothing yet**
+
+- **#22's merged design has still never had an independent review.** CodeRabbit's only pass
+  covered the first commit; the request for a pass over the final design (posted on #22)
+  was refused for rate limits again. Re-request it — this is a safety-critical merge gate
+  running on a two-lens subagent panel and the author's own reads.
+- The two H-severity entries in [`kit-friction-log.md`](kit-friction-log.md) (fallback
+  independence; a receipt outliving the design it reviewed) are unfiled. Both now carry a
+  proposed fix, so they are issue-shaped — `triage-friction-log` should graduate them rather
+  than leaving them in the inbox.
+
+▶ Next: fix **#19 + #23 together** (Phase 3b) — the queued-vs-unavailable ambiguity, on
+both the comment and status-check surfaces. Design constraint: the informational-check
+exclusion must keep preventing a wedge on a bot that never reports, so neither can be fixed
+by simply letting that check block. Wants CodeRabbit actually available for review.
+
+______________________________________________________________________
+
+## Earlier session — 2026-07-25
+
 **Theme —** Assessed the kit against its own ten principles, then fixed what the
 assessment found. Six PRs merged; tests 83 → 188. The recurring shape: **the kit had
 written down rules it was itself violating.**
@@ -73,7 +164,7 @@ written down rules it was itself violating.**
   required. Corrected in #21: marking ready is the lane's, landing it is the cockpit's.
   **The lesson isn't "check for drift" — it's that finding two sources doesn't tell you
   which one is right, and I picked by proximity rather than by testing either against the
-  baseline."
+  baseline.**
 - **A guard that fails open must be loud.** Three separate silent-no-op bugs this session
   (`origin/main`, the uninstalled hook, `paths.engines`). Silence is indistinguishable from
   "checked and clean".
@@ -83,11 +174,10 @@ written down rules it was itself violating.**
   queued, and its four valid findings landed after the merge. `decide_done` can't tell the
   two apart.
 
-▶ Next: decide the **Phase 3 sequencing** for the cs-toolkit back-port — porting the review
-receipts as-is would break the nightly fixer (nothing there records a receipt, so
-`decide_done` would report `done=False` forever on every PR it opens). Proposed order:
-additive merge gate first → receipts behind a flag → wire the fixer's own `/code-review`
-step to record → flip the flag.
+✔ Superseded by the Phase 3a session (PR #22). The proposed order (merge
+gate → receipts behind a flag → wire the fixer → flip) was replaced: the flag existed to
+defer a breakage caused by `done` conflating two predicates, so splitting them removed the
+need for it.
 
 ______________________________________________________________________
 

kit-manifest.json

@@ -59,7 +59,7 @@
     },
     "scripts/dev_session.sh": {
       "role": "engine",
-      "sha256": "52d962af8088e70fd7b4682d67ec7d6d4354c70f3d8db49b724d4c42c04d98f6"
+      "sha256": "50dd24f5506fc40c55a7007d3e78395beea69b184194faa2fa3c47e883b5d486"
     },
     "scripts/hooks/pre-push": {
       "role": "hook",

scripts/dev_session.sh

@@ -746,10 +746,15 @@ cmd_merge() {
     report="$(GH_REPO="$repo_nwo" DEVKIT_STATE_ROOT="$session_dir/state" \
         uv run "$SCRIPT_DIR/pr_watch.py" "$pr" --json)" \
         || _die "pr-watch failed for PR #$pr"
-    # Gate on `mergeable`, NOT `done`. `done` is the watch-loop predicate
-    # ("anything left to fix?") and is deliberately true on a green, comment-clean
-    # PR that carries no review receipt. `mergeable` is the merge-gate predicate:
-    # done AND no deterministic merge blocker AND a receipt bound to this head.
+    # Gate on `mergeable`: converged AND no deterministic merge blocker AND a
+    # review receipt bound to this head. The report's `done` is an unchanged
+    # alias of `mergeable`, and every pre-split pr_watch emitted `done` with this
+    # same merge-authorization meaning — so either key is safe against a kit
+    # version skew, and `mergeable` additionally fails CLOSED when absent. Read
+    # `mergeable` because it is the precise name and is not the one retained for
+    # backward compatibility. The key that must NEVER gate a merge is `converged` —
+    # the watch-loop predicate ("anything left to fix?"), which is deliberately
+    # true on a green, comment-clean PR carrying no receipt.
     # Read the flag pr_watch computes — never re-derive it here, or this gate
     # becomes a second copy of the contract that can drift from the engine's.
     IFS=$'\t' read -r mergeable validated_pr validated_base validated_head <<< "$(printf '%s' "$report" | python3 -c '

scripts/tests/test_pr_watch.py

@@ -97,17 +97,32 @@ def original_done(checks, new_items, merge_blockers, review_evidence, settling):
 
 
 def test_report_done_is_always_identical_to_mergeable() -> None:
-    """The alias must never drift from the field it aliases."""
-    pr_watch = _load_pr_watch()
-    receipt = {"head": "abc123", "source": "fallback:codex"}
+    """The alias must never drift from the field it aliases.
 
-    for report in (
-        pr_watch.build_report(_green_view(), [], set()),
-        pr_watch.build_report(_green_view(), [], set(), review_receipt=receipt),
-        pr_watch.build_report(_green_view(isDraft=True), [], set()),
-        pr_watch.build_report(_green_view(state="MERGED"), [], set()),
-    ):
-        assert report["done"] is report["mergeable"]
+    The report **key** — not :func:`decide_done`, which has no in-engine caller —
+    is what keeps an older `dev_session.sh` gating on merge authorization. So it
+    is the key that needs the coverage: walk a matrix of report shapes spanning
+    converged/not, blocked/clean, and receipt/none, rather than a few examples.
+    """
+    pr_watch = _load_pr_watch()
+    current = {"head": "abc123", "source": "fallback:codex"}
+    stale = {"head": "older", "source": "fallback:codex"}
+    comment = {"id": "c1", "author": {"login": "someone"}, "body": "please fix"}
+
+    views = (
+        _green_view(),
+        _green_view(isDraft=True),
+        _green_view(state="MERGED"),
+        _green_view(mergeStateStatus="BLOCKED"),
+        _green_view(reviewDecision="CHANGES_REQUESTED"),
+        _green_view(statusCheckRollup=[{"name": "t", "conclusion": "FAILURE"}]),
+        _green_view(statusCheckRollup=[]),
+        _green_view(comments=[comment]),
+    )
+    for view in views:
+        for receipt in (None, current, stale):
+            report = pr_watch.build_report(view, [], set(), review_receipt=receipt)
+            assert report["done"] is report["mergeable"], (view, receipt)
 
 
 def test_predicates_are_strictly_bool_typed() -> None: