feat: enforce review-loop legality and derive fleet state from GitHub

.claude/hooks/check-worktree-path.sh

@@ -53,4 +53,45 @@ if [ "$target_root" != "$session_root" ]; then
   exit 0
 fi
 
+# ---------------------------------------------------------------------------
+# Second check: is this session in a worktree at all?
+#
+# CLAUDE.md's rule is unconditional -- "Never edit any file on main, even a
+# one-line doc fix" -- and it is prose, so it has to be REMEMBERED at the moment
+# of the first edit. That is precisely when it isn't: a session that opens as a
+# question ("why does X happen?") and drifts into implementing never re-evaluates
+# a rule it had no reason to consider at the start. Six live sessions currently
+# sit in the main checkout for exactly that legitimate read-only reason.
+#
+# The cost is not hypothetical. PR #619's branch carries a merge of itself --
+# `Merge remote-tracking branch 'origin/fix/...' into fix/...` -- because two
+# writers worked the same branch from different bases and diverged for fifteen
+# hours. The reviewer reviewed one line three times while the other line, based
+# on a commit from 08:09, never saw a single verdict.
+#
+# So the check is mechanical and fires at the transition itself: the first
+# Edit/Write IS the moment a question becomes an implementation. A linked
+# worktree has its own git dir under the main one, so `--git-dir` and
+# `--git-common-dir` differ there and are identical in the main checkout. That
+# is a path comparison, the only shape docs/agents/rules.md sanctions for this
+# hook -- it never has to guess what a command string will touch.
+#
+# Sibling checkouts (`../bess-manager-feature/`) are linked worktrees too, so
+# they pass: this enforces "work in a worktree", not "work under .claude/".
+git_dir=$(git rev-parse --absolute-git-dir 2>/dev/null || true)
+common_dir=$(cd "$(git rev-parse --git-common-dir 2>/dev/null || echo .)" 2>/dev/null && pwd || true)
+
+if [ -n "$git_dir" ] && [ -n "$common_dir" ] && [ "$git_dir" = "$common_dir" ]; then
+  branch=$(git rev-parse --abbrev-ref HEAD 2>/dev/null || echo "?")
+  reason="BLOCKED: this session is in the MAIN checkout ('${session_root}', branch '${branch}'), not a worktree, so editing '${target_path}' would write straight into the shared checkout. CLAUDE.md: work in a worktree before ANY edit -- unconditional, including a one-line doc fix. If this session started as a question and has become implementation work, that is the common path here and this is the moment to switch: call EnterWorktree (never \`git worktree add\`, which the sandbox denies), then redo the edit there. The main checkout stays read-only: questions, \`gh\`, the backlog, and dispatch all work fine from it."
+  jq -n --arg reason "$reason" '{
+    hookSpecificOutput: {
+      hookEventName: "PreToolUse",
+      permissionDecision: "deny",
+      permissionDecisionReason: $reason
+    }
+  }'
+  exit 0
+fi
+
 echo '{"continue": true}'

.claude/skills/implement-issue/SKILL.md

@@ -121,6 +121,30 @@ Re-enter at the **earliest incomplete** step and run forward normally. A PR
 carrying `CHANGES_REQUESTED` re-enters at Step 11's `CHANGES_REQUESTED` branch;
 one carrying `APPROVED` needs only `gh pr ready`.
 
+**A verdict alone does not say how far Step 11 got — compare it against the
+last push.** The loop is request → verdict → fix → push → request, so the same
+`CHANGES_REQUESTED` means two different things depending on which side of it
+HEAD sits:
+
+| Newest verdict vs newest commit | The dead session had |
+|---|---|
+| verdict **newer** than last commit | received the findings and not yet acted on them — resume by fixing them |
+| last commit **newer** than verdict | already fixed and pushed — resume by requesting the next round |
+
+```bash
+gh pr view <n> --json reviews,commits --jq \
+  '{verdict: ([.reviews[] | select(.state=="APPROVED" or .state=="CHANGES_REQUESTED")]
+              | sort_by(.submittedAt) | last | .submittedAt),
+    pushed: ([.commits[].committedDate] | sort | last)}'
+```
+
+Getting this backwards is what happened on #619: four `@claude-bot review`
+comments, two paid verdicts, and one diff that never changed between them,
+because "have I acted on this yet" was held in a session that had died.
+`request-pr-review.sh` now refuses the illegal round rather than trusting the
+caller to have checked, but read it here too — the answer also tells you *what
+to do*, which the script cannot.
+
 **Rehydrate the diagnosis before touching code.** Step 2's analysis died with
 the session, and Step 11 depends on holding it. It is recoverable only because
 this skill already forces it to be written down:
@@ -609,6 +633,30 @@ verdict lands, printing `VERDICT <APPROVED|CHANGES_REQUESTED|COMMENTED>
 <submittedAt> <author>` (exit 2 on a 15-minute timeout, after dumping recent
 `PR Review` runs).
 
+**Exit 1 means the round was illegal and no review was requested — read the
+message, do not retry.** The script checks, before posting anything, whether
+asking can possibly help:
+
+| Refusal | What it means | The actual next move |
+|---|---|---|
+| unconsumed `CHANGES_REQUESTED` | the newest verdict is newer than the last push, so it describes the diff as it stands | address the findings and push; then the next round is legal |
+| already `APPROVED` | approved on the current diff, nothing pushed since | re-check mergeability, then `gh pr ready` |
+| 3 decisive rounds | the cap below, enforced rather than remembered | hand the outstanding findings to the user verbatim |
+| gate unreadable | `gh` failed, so legality was never established | re-run; it costs nothing, a needless round costs a paid review |
+
+This exists because **the cap and "have I acted on the last verdict" are the
+two pieces of loop state a dead session takes with it**, and asking again is
+the one move a confused loop can always make. On #619 that produced four
+requests, zero consumed verdicts, and two paid reviews of byte-identical code.
+Both facts are already on the PR — decisive review count, and whether a commit
+follows the newest verdict — so the script reads them instead of trusting the
+caller to remember.
+
+`--allow-unconsumed` overrides the first row, for the one case this step
+sanctions: the finding was wrong, and you have replied on the PR saying why
+rather than pushing. It is a flag so that "the reviewer is mistaken" is a
+decision you take deliberately, not the path a stalled loop slides into.
+
 **`COMMENTED` is ambiguous, and the script resolves it for you — don't
 second-guess it.** `pr-review.yml` allows three final verdicts (`APPROVE`,
 `REQUEST_CHANGES`, `COMMENT`), so a real `COMMENT` verdict is possible and