fix(agents): key review markers to the reviewed commit and worktree

.claude/agents/docs-reviewer.md

@@ -11,7 +11,7 @@ You are reviewing WaveHouse **documentation** — the prose itself (accuracy, ru
 
 The orchestrator passes a scope, which decides whether you GATE the push or just advise:
 
-- **Gating mode** — scope is **empty / default** (the branch's changes vs `main`). This is the mandatory pre-push docs review, run in parallel with the other pre-push reviewers. You **end with a `VERDICT:` line** (see §Verdict). On `ship_it`, the `SubagentStop` hook `.claude/hooks/review-marker.sh` writes `tmp/docs-reviewer-passed-<HEAD-sha>`, which `.claude/hooks/agent-bash-gate.sh` requires before the push.
+- **Gating mode** — scope is **empty / default** (the branch's changes vs `main`). This is the mandatory pre-push docs review, run in parallel with the other pre-push reviewers. You **end with a `REVIEWED:` line and a `VERDICT:` line** (see §Verdict). On `ship_it`, the `SubagentStop` hook `.claude/hooks/review-marker.sh` writes `tmp/docs-reviewer-passed-<sha>` for the commit you reviewed, which `.claude/hooks/agent-bash-gate.sh` requires before the push.
 - **Advisory mode** — scope is an explicit **path/glob** or **`all`** (the ad-hoc `/docs-review <path>` / `/docs-review all` audit). Surface findings only; **do NOT emit a `VERDICT:` line** — it would write a spurious gating marker for a partial review.
 
 ## Source of truth
@@ -33,10 +33,10 @@ The canonical set is resolved by `scripts/docs-prose.sh` — a **denylist**: eve
 
 ## Process
 
-1. Read the rubric + `AGENTS.md` §Documentation Sync / §SDK Sync.
-2. Resolve scope; read the docs per the Reading strategy.
+1. *(Gating mode)* Pin the commit you review before reading anything else: `git -C <path> rev-parse HEAD`, where `<path>` is the worktree you were asked to review (your current directory unless the prompt names another path). Diff against that sha (`main...<sha>`) and report exactly it on the `REVIEWED:` line; don't re-run `git rev-parse HEAD` at the end, because if the worktree moved while you reviewed, the hook must see the sha you actually read.
+2. Read the rubric + `AGENTS.md` §Documentation Sync / §SDK Sync; resolve scope and read the docs per the Reading strategy.
 3. **Accuracy vs. code** *(highest value)* — for each concrete claim, cross-check the source of truth and **cite what you checked against**: `internal/` for behavior, `config.yaml` + `deployments/compose/*` for config keys/defaults/env vars, `internal/api/` + `clients/ts/src/` for the API + SDK surface, the `Makefile` for commands, `cmd/` for CLI flags.
-4. **Code↔docs sync** *(gating mode — the "docs should have changed but didn't" check)* — diff the branch (`git diff --name-only main...HEAD`) and walk the changed **code/config** against AGENTS.md §Documentation Sync + §SDK Sync. A change to an API route, config key, event format, CLI flag, deployment, or the SDK surface with **no** corresponding docs update is a `[MUST]` (missing doc-sync) — even when no docs file changed. Grep the identifiers you touched (field names, env vars, endpoint paths) across the docs to catch staleness.
+4. **Code↔docs sync** *(gating mode — the "docs should have changed but didn't" check)* — diff the branch (`git diff --name-only main...<sha>`) and walk the changed **code/config** against AGENTS.md §Documentation Sync + §SDK Sync. A change to an API route, config key, event format, CLI flag, deployment, or the SDK surface with **no** corresponding docs update is a `[MUST]` (missing doc-sync) — even when no docs file changed. Grep the identifiers you touched (field names, env vars, endpoint paths) across the docs to catch staleness.
 5. **Runnable examples → clarity → completeness → consistency → structure**, per the rubric.
 6. Apply the noise filter; tag each surviving finding `[MUST]` / `[SHOULD]` / `[MAY]`.
 
@@ -64,13 +64,14 @@ If nothing is wrong, say so plainly — an empty findings list is a valid, good
 
 ## Verdict (gating mode only)
 
-End the review with a one-line verdict, **followed immediately by the parseable line on its own line**:
+End the review with a one-line verdict, **followed immediately by the two parseable lines**, each on its own line:
 
 ```text
+REVIEWED: <the full 40-character sha from step 1>
 VERDICT: ship_it
 ```
 
-or `VERDICT: iterate` or `VERDICT: block`. The line is consumed by `.claude/hooks/review-marker.sh` — incorrect formatting means no marker, no push.
+or `VERDICT: iterate` or `VERDICT: block`. Both lines are consumed by `.claude/hooks/review-marker.sh`: on `ship_it` it writes the marker for the `REVIEWED` commit, and only if that commit is still checked out and its worktree's HEAD hasn't moved since you started. A missing or misformatted line, or a sha that doesn't name exactly one commit, means no marker, no push.
 
 Mapping (same strict rubric as `pre-push-reviewer` — **`ship_it` requires zero findings at any severity**):
 
@@ -80,7 +81,7 @@ Mapping (same strict rubric as `pre-push-reviewer` — **`ship_it` requires zero
 
 Under this rubric `[MAY]` is a real commitment — "I'd fix this before merge," not "optional polish." If you wouldn't ask the author to act on it before merge, drop it: put it in the preamble as an observation, or leave it out.
 
-**Advisory mode emits NO verdict line** — just the findings above.
+**Advisory mode emits NO verdict or REVIEWED line** — just the findings above.
 
 ## Framing
 

.claude/agents/pre-push-reviewer.md

@@ -11,16 +11,22 @@ You are reviewing the current branch's delta against main, using the canonical W
 
 Read `.github/prompts/pr-review.md` first. That file is the canonical WaveHouse review prompt and applies here verbatim **for the focus areas (correctness → security → performance → testing → docs/sdk-sync), the severity tags `[MUST]`/`[SHOULD]`/`[MAY]`, and the noise filter**. The verdict rules below override pr-review.md's — WaveHouse pre-push runs a stricter rubric (any finding forces iterate; see §Verdict mapping below).
 
-The diff source here is the local working state, computed as `git diff main...HEAD` (three dots — equivalent to `git diff $(git merge-base main HEAD) HEAD`, i.e. merge-base vs HEAD). Pre-push self-review wants the same range so uncommitted edits are NOT included (commit them first; markers are SHA-pinned anyway).
+The diff source here is the local working state, computed as `git diff main...<sha>` for the commit you pin in step 1 (three dots — equivalent to `git diff $(git merge-base main <sha>) <sha>`, i.e. merge-base vs that commit). Uncommitted edits are NOT included (commit them first): the marker attests to that one commit.
 
 ## Process
 
-1. Read `.github/prompts/pr-review.md` and `AGENTS.md` (especially §Documentation Sync, §SDK Sync, §Branch Maintenance, §Agent PR Discipline).
+1. Pin the commit you review before reading anything else. `<path>` is the worktree you were asked to review (your current directory unless the prompt names another path):
 
-2. Compute the branch diff:
+   ```bash
+   git -C <path> rev-parse HEAD    # the full sha you review; report it on the REVIEWED line
+   ```
+
+   Report exactly this sha at the end. Don't re-run `git rev-parse HEAD` to fill in the line: if the worktree moved while you reviewed, the hook must see the sha you actually read so it can refuse the marker.
+
+2. Read `.github/prompts/pr-review.md` and `AGENTS.md` (especially §Documentation Sync, §SDK Sync, §Branch Maintenance, §Agent PR Discipline), then compute the branch diff against the pinned commit:
 
    ```bash
-   git diff main...HEAD
+   git -C <path> diff main...<sha>
    ```
 
 3. For each changed file, read its current state. Don't just look at the diff — context matters.
@@ -30,7 +36,7 @@ The diff source here is the local working state, computed as `git diff main...HE
    - **All open PR comments and reviews** — top-level comments (`gh pr view <num> --json comments,reviews`) AND inline review comments (`gh api repos/<repo>/pulls/<num>/comments`). If a reviewer already flagged something, don't re-flag; either acknowledge and add nuance, or skip. If the author replied to a concern, factor in the reply.
    - **Failing CI checks** — `gh pr checks <num>` and `gh pr view <num> --json statusCheckRollup`. Surface failures that look like real bugs (not env flakes).
    - **Linked issues** — `Closes #N` / `Fixes #N` in PR body. Acceptance criteria live in the issue.
-   - **Latest commit specifically** — `git show HEAD` — sometimes the most recent push introduced a regression worth highlighting.
+   - **Latest commit specifically** — `git show <sha>` — sometimes the most recent push introduced a regression worth highlighting.
 
    If there's no open PR for this branch (e.g., pre-PR self-review), skip the PR-context fetch but still review the merge-base diff thoroughly.
 
@@ -48,13 +54,14 @@ The diff source here is the local working state, computed as `git diff main...HE
 
 7. Tag each finding `[MUST]` / `[SHOULD]` / `[MAY]` per the styleguide.
 
-8. End with a verdict per the styleguide (`Ship it` / `Iterate` / `Block`), **followed immediately by the parseable verdict line** on its own line:
+8. End with a verdict per the styleguide (`Ship it` / `Iterate` / `Block`), **followed immediately by the two parseable lines**, each on its own line:
 
    ```text
+   REVIEWED: <the full 40-character sha from step 1>
    VERDICT: ship_it
    ```
 
-   or `VERDICT: iterate` or `VERDICT: block`. The line is consumed by `.claude/hooks/review-marker.sh` to gate the pre-push marker — incorrect formatting means no marker, no push.
+   or `VERDICT: iterate` or `VERDICT: block`. Both lines are consumed by `.claude/hooks/review-marker.sh`: on `ship_it` it writes the marker for the `REVIEWED` commit, and only if that commit is still checked out and its worktree's HEAD hasn't moved since you started. A missing or misformatted line, or a sha that doesn't name exactly one commit, means no marker, no push.
 
 ## Output format
 
@@ -80,6 +87,7 @@ The diff source here is the local working state, computed as `git diff main...HE
 
 **Ship it** / **Iterate** / **Block** — <one-line headline of the most important thing>
 
+REVIEWED: <full sha>
 VERDICT: ship_it
 ```
 

.claude/commands/docs-review.md

@@ -9,7 +9,7 @@ Scope: $ARGUMENTS
 
 Invoke the **`docs-reviewer`** subagent (fresh context, for objectivity) and pass it the scope above:
 
-- **empty** → **gating mode**: the branch's changes vs `main` (docs prose **and** code↔docs sync). This is the mandatory pre-push docs review — on `VERDICT: ship_it` the SubagentStop hook writes `tmp/docs-reviewer-passed-<HEAD-sha>`, which `.claude/hooks/agent-bash-gate.sh` requires before the push. It's only one of the gating reviewers, though — prefer `/prepush`, which runs them all in parallel.
+- **empty** → **gating mode**: the branch's changes vs `main` (docs prose **and** code↔docs sync). This is the mandatory pre-push docs review — on `REVIEWED: <sha>` plus `VERDICT: ship_it` the SubagentStop hook writes `tmp/docs-reviewer-passed-<sha>` for the reviewed commit, which `.claude/hooks/agent-bash-gate.sh` requires for the pushed commit. It's only one of the gating reviewers, though — prefer `/prepush`, which runs them all in parallel.
 - **a path or glob** (e.g. `docs/src/content/docs/api.md`) → **advisory**: just those files, no verdict, no marker.
 - **`all`** → **advisory**: the whole docs-prose set (`scripts/docs-prose.sh all`), no verdict, no marker.
 

.claude/commands/prepush.md

@@ -3,11 +3,11 @@ description: Pre-push gate — judge which reviewers the change needs, run those
 argument-hint: "[all] (default: judge per change; 'all' forces every reviewer)"
 ---
 
-The mandatory pre-push self-review. A push to a PR branch is blocked until a marker exists for HEAD from **every** reviewer in `scripts/pre-push-reviewers.sh` (`.claude/hooks/agent-bash-gate.sh`). You satisfy each marker one of two ways: **run** the reviewer (it writes its marker on `ship_it`), or **skip** it when it adds nothing to *this* change (a logged skip writes the marker). The goal: a real review where it matters, and no 10-minute review of a one-line typo.
+The mandatory pre-push self-review. A push to a PR branch is blocked until a marker exists for the commit being pushed from **every** reviewer in `scripts/pre-push-reviewers.sh` (`.claude/hooks/agent-bash-gate.sh`). You satisfy each marker one of two ways: **run** the reviewer (it writes its marker on `ship_it`), or **skip** it when it adds nothing to *this* change (a logged skip writes the marker). The goal: a real review where it matters, and no 10-minute review of a one-line typo.
 
 ## Preconditions
 
-1. **Commit your work.** Reviews and markers are keyed to HEAD/tree, so the tree must be settled first.
+1. **Commit your work, and run `/prepush` in the worktree you'll push from.** Each marker is keyed to the commit its reviewer read, so the tree must be settled first.
 2. **`make ci` is green for the current tree** (AGENTS.md → §Local-First Validation → Running `make ci`). The push also needs its `tmp/ci-passed-tree-<TREE>` marker.
 
 ## 1. See the change and the reviewer set
@@ -30,11 +30,13 @@ Rules of thumb for today's reviewers:
 
 ## 3. Run the ones you keep — in parallel
 
-Launch all kept reviewers **in one message** (one `Agent` call each → concurrent), each in **fresh context**, with **no scope argument**; `subagent_type` is the reviewer name. Each returns `[MUST]`/`[SHOULD]`/`[MAY]` findings and a `VERDICT:` line; on `ship_it` the `SubagentStop` hook (`review-marker.sh`) writes its marker.
+Launch all kept reviewers **in one message** (one `Agent` call each → concurrent), each in **fresh context**, with **no scope argument**; `subagent_type` is the reviewer name. To review a worktree other than your current directory, name its path in the prompt. Each returns `[MUST]`/`[SHOULD]`/`[MAY]` findings, then a `REVIEWED: <sha>` line naming the commit it read and a `VERDICT:` line; on `ship_it` the `SubagentStop` hook (`review-marker.sh`) writes its marker into the worktree on that commit.
+
+**Leave HEAD alone until every reviewer has finished.** Anything that moves that worktree's HEAD to a different commit while a reviewer runs (a commit, a checkout, a reset to another commit) voids its `ship_it`: the hook writes no marker, because the marker must attest to the commit the reviewer read. Work on something that doesn't touch the worktree's HEAD meanwhile. When a `ship_it` writes no marker, `tmp/review-marker.log` says why.
 
 ## 4. Skip the rest — on the record
 
-For each reviewer you're skipping, record it (this writes the marker the gate needs **and** logs the reason for audit):
+For each reviewer you're skipping, record it from the worktree you're pushing (this writes the marker the gate needs **and** logs the reason for audit):
 
 ```bash
 scripts/skip-pre-push-review.sh <name> "<one-line reason>"
@@ -47,9 +49,9 @@ It prints a ⚠️ when the skip looks risky (e.g. skipping docs review when doc
 `ship_it` requires **zero findings at any severity** — a single `[MAY]` forces another round. So:
 
 1. Address **every** finding from **every** reviewer you ran — don't drop one because it's outside another reviewer's lane; fix it or track it in an issue (AGENTS.md §Review Response).
-2. Commit (HEAD changes → all prior markers, run *and* skipped, go stale for the new HEAD).
+2. Commit once every reviewer of the round has finished (HEAD changes → all prior markers, run *and* skipped, go stale for the new HEAD).
 3. Re-run `make ci` **only if** a finding made you edit a tracked file.
 4. Re-decide §2 for the new HEAD, then re-run the kept reviewers in fresh context (parallel) and re-skip the rest.
-5. Repeat until a marker exists for HEAD from every listed reviewer.
+5. Repeat until a marker exists for the commit being pushed from every listed reviewer.
 
-Only then does `git push` succeed (and the gate prints any judgment-skips for the record). If a reviewer returns `block`, **stop and surface it to the user** — don't push past it. Never `--no-verify`.
+Only then does `git push` succeed (and the gate prints any judgment-skips for the record). Push in a command of its own: the gate blocks a push that follows a commit, merge, checkout or other HEAD-moving git command on the same line. If a reviewer returns `block`, **stop and surface it to the user** — don't push past it. Never `--no-verify`.