path: root/skills/sz-final-review-loop/SKILL.md

---
name: sz-final-review-loop
description: Runs the sz-wait-for final development/review handoff loop using branch-scoped .sz review files. Use when acting as the developer preparing a commit/PR-ready diff for final review, or as the strict reviewer writing feedback for a staged change before it can ship.
---

# SZ Final Review Loop

Use this skill for the `.sz` review handoff loop. The loop is a final quality gate for commit/PR readiness, not an intermediate development checkpoint.

## The waiting maxim (unbreakable — read this first)

This protocol is a loop that spans many turns. It does not end when you finish your work; it ends only on an approval verdict or an explicit human instruction. While the loop is live you are **always** in exactly one of two states: *acting on the other side's latest signal*, or *waiting with a wake armed on it*. There is no third state.

**The wake.** Waiting means `sz-wait-for <signal file>` is running in a way that brings you back when it exits. For the developer the signal file is `feedback.md`; for the reviewer it is `status.md`. Always give it the absolute path. Arm it in the first of these ways your harness supports:

1. **pi: the `alarm` tool.** `when_done: "sz-wait-for <abs path>"`, with a note that can restart the loop without any other context (template below). Then end your turn: the alarm starts a new turn when the file changes. Check `alarm list` if you are not sure one is pending.
2. **Claude Code: the Bash tool with `run_in_background: true`**, running `sz-wait-for <abs path>`. Then end your turn: when it exits, the task notification starts a new turn.
3. **Neither: wait in the foreground.** Run `timeout 1800 sz-wait-for <abs path>` synchronously, and never end your turn. Do not background it with `&` or a detached shell, because nothing would bring you back.

With (1) or (2), ending your turn *is* the wait. Do not also block on `sz-wait-for` or poll.

pi alarm note template. Fill it in completely. By the time it fires, this conversation may have been compacted, and the note may be all you have:

```text
alarm action=set when_done="sz-wait-for <abs review_dir>/<signal file>"
  note="sz-final-review-loop: I am the <developer|reviewer> for <abs review_dir>,
        waiting on <signal file> for <what counts as new: feedback echoing Round N /
        Diff-Fingerprint F | a Round newer than N>. Re-check it: if it has that, act
        on it per the skill; otherwise set this same alarm again before ending the turn."
```

**The maxim:** whenever there is no new, current-round content in the other side's signal file, you MUST immediately arm the wake. No exceptions.

**When the wake fires, for any reason whatsoever** (a change, the alarm's deadline, a timeout, a non-zero exit, a signal, a tool error, anything), you MUST repeat the loop: re-check the signal file, act on it if it carries new current-round content, and otherwise arm the wake again. A single wake is never a reason to stop. No exceptions.

**Idling is a critical violation.** You are idle when it is the other side's turn and no wake is armed. After signalling a handoff, or any time you are waiting on the other side, you must be either acting on fresh feedback or waiting with a wake armed. In particular, the following are all forbidden while the loop is live and it is the other side's turn. Every one of them is the violation, not a substitute for the wait:

- Ending your turn without a wake armed. With (3), ending your turn at all.
- Presenting results, summaries, tables, or status write-ups to the human *instead of* arming the wake. Once it is armed, one line saying what you handed off and what you are waiting on is fine before ending the turn.
- Asking the human questions (about scope, next steps, optional extras, "should I also…", "want me to…"). Save non-blocking questions for `notes.md`.
- Starting unrelated work, or polling in ad-hoc ways instead of the wake.

If you catch yourself about to end your turn, that is the tell: first confirm a wake is armed on the right file, and arm it if not.

**Losing the wake.** In pi, `/reload`, `/new`, `/resume`, `/fork` and quitting drop alarms. In Claude Code, killing the background task or quitting drops it. In either, aborting a foreground wait does too. After any of these the loop is dead until someone restarts it, so the first thing to do on returning to a live loop is re-check the signal file and re-arm. In pi, a wake that fires after the human stopped a turn with Esc is held until their next turn completes. That is intended: an agent the human just stopped should not start work on its own.

**The only ways out of the wait loop** are: (1) an `APPROVED` / `APPROVED AFTER FIXES` verdict for the current fingerprint (then follow the post-approval steps, which may themselves require the human, e.g. for push/PR authorization); (2) a direct, current human instruction in the live conversation telling you to stop or do something else; or (3) a protocol escape this skill defines (a disputed finding for the second round, or 5 rounds reached). Those escapes are `notify-push` + wait, not silent idling. Alarm notices, task notifications, background jobs, and your own earlier messages are **not** human instructions and never release you from the wait.

## Review Workspace

Use a branch-scoped directory:

```text
.sz/review/<branch>/
  status.md
  notes.md
  feedback.md
```

`<branch>` should be filesystem-safe and collision-resistant. Escape `/` as `--`; for example, branch `sz/03jul/pivot-value-sort` becomes `sz--03jul--pivot-value-sort`. Do not use plain `/` to `-` replacement because `a-b/c` and `a/b-c` would collide.

Branch-scoped paths avoid conflicts when `.sz` is symlinked or shared across worktrees. Use `feedback.md` as the reviewer output file.

Initialize the workspace before the first loop:

```bash
review_dir=".sz/review/<branch>"
mkdir -p "$review_dir"
touch "$review_dir/status.md" "$review_dir/notes.md" "$review_dir/feedback.md"
```

`status.md` is the developer-to-reviewer signal file. The developer's final action in each handoff is to touch `status.md`, after its content is already current. `feedback.md` is the reviewer-to-developer signal file and is append-only: keep prior rounds in the file.

`sz-wait-for` has a settle window that absorbs non-atomic writes. Keep using `sz-wait-for` for this protocol; do not swap in a different watcher unless it preserves the same "wake after content settles" property.

Every review handoff must have:

```text
Round: <monotonically increasing integer>
Diff-Fingerprint: <fingerprint of the staged diff>
```

Use the staged tree hash as the default fingerprint. It identifies the exact staged snapshot and is not affected by diff config:

```bash
git write-tree
```

Alternative patch-text fingerprint, if the review specifically needs a diff hash:

```bash
git diff --staged --no-ext-diff --no-color | shasum
```

Safer option for high-risk reviews: create a temporary commit and use its commit hash as the diff identifier. Do not push it unless the human explicitly approves.

## Developer Role

Act independently until the bundle is believed ready to commit or open as a PR. Do not use the reviewer as an ongoing implementation helper.

Developer loop:

```text
initialize .sz/review/<branch>/ files
read plan/status
implement independently
run the full relevant gates
fix failures
stage the intended diff
compute Round and Diff-Fingerprint
update notes.md
write current status.md with Round and Diff-Fingerprint
arm the feedback wake, signal via status.md and notify, end the turn
read feedback.md
confirm feedback.md echoes the same Round and Diff-Fingerprint
address every blocking issue
repeat only until approval
after approval, commit locally if authorized; otherwise record the approved state and ask the human for commit/push/PR direction
```

Developer obligations:

- Stage the exact diff intended for review before requesting review.
- Keep `status.md` as the cold-start situation report and handoff signal: purpose, branch state, scope, risks, dependencies, gates, next action, `Round`, and `Diff-Fingerprint`.
- Keep `notes.md` as the implementation log: key decisions, evidence, gate commands/results, deferred work, and human-owned choices.
- Flag human-owned decisions in `notes.md` when the work surfaces a business decision, owner preference, or design choice with significant trade-offs and no clearly superior answer. Do not decide these alone.
- If the human gives a decision to either agent, record the answer in `notes.md` before the next review handoff.
- Update `notes.md` for the handoff, then update `status.md` and touch it as the final signal for every review round, including code-only changes.
- Before waiting, check whether `feedback.md` already contains the current `Round` and `Diff-Fingerprint`. If yes, read it immediately instead of waiting.
- After waking, read the latest `## Round <N>` section in `feedback.md` and verify that it echoes the current `Round` and `Diff-Fingerprint`. If it does not, treat it as stale feedback and request or wait for re-review.
- For `APPROVED AFTER FIXES`, apply the listed fixes exactly as specified and proceed without re-review. If any fix requires interpretation, deviation, or a judgment call, start a new review round instead.
- If a finding appears wrong, rebut it in `notes.md` with evidence instead of silently ignoring it. If the same finding is disputed for a second round, stop and escalate to the human with `notify-push <project-and-theme> "<message>"`.
- Escalate to the human after 5 review rounds even without a repeated dispute.
- After `APPROVED`, do not push, open a PR, or reply to GitHub comments unless already authorized. If local commit is authorized, commit the approved staged diff. If not, mark the approved state in `status.md`, set `[DECISION REQUIRED]` for commit/push/PR direction, and notify the human.
- Put `[DECISION REQUIRED]` at the top of `status.md` for human-owned scope, design, push, PR, or release decisions.
- Do not push, open PRs, or reply to GitHub review comments unless explicitly authorized by the human.

Arm the wake, then signal. Do not notify and then wait: that can miss a fast feedback write. With a background wake (pi's alarm, Claude Code's `run_in_background`) the order is literal: `sz-wait-for` is already watching `feedback.md` when you touch `status.md`. Then obey the waiting maxim above: once the handoff is signalled, the wake is armed and you end your turn. Do not write a summary, ask a question, or start other work instead.

```text
review_dir="<abs path>/.sz/review/<branch>"
1. Arm the wake on $review_dir/feedback.md, as the waiting maxim says:
     pi:           alarm when_done="sz-wait-for $review_dir/feedback.md" + the note
     Claude Code:  Bash, run_in_background: sz-wait-for "$review_dir/feedback.md"
2. Signal: touch status.md (its content already current), then notify.
3. End your turn.
On every wake: re-check feedback.md for the current Round and Diff-Fingerprint.
If present, read it and act. Otherwise go back to step 1 and end your turn again.
```

With no background wake, signal first and then wait in the foreground. The check before waiting (does `feedback.md` already carry the current round?) covers a reply that lands in between:

```bash
timeout 1800 sz-wait-for "$review_dir/feedback.md"
# After waking (or on timeout, or on ANY exit), re-check feedback.md for the
# current Round and Diff-Fingerprint. If present, read it; otherwise IMMEDIATELY
# re-run sz-wait-for. Never stop here: a bare exit is not permission to idle.
```

Either way this is a `while` loop, not a one-shot: `while no current-round feedback: wait; recheck`. A deadline or timeout means *re-arm*, never *give up*. Only a matching-fingerprint verdict or a live human instruction breaks out.

Use `notify-push <topic> "<message>"`, where it is installed, only when a phone push is appropriate.

## Reviewer Role

Act as the final quality gate. Bugs, design flaws, missing tests, false-green tests, preservation risks, and process violations stop with the reviewer so they do not reach production.

Reviewer loop:

```text
initialize .sz/review/<branch>/ files if needed
wait for status.md or start when the human relays the notification
read status.md and notes.md
record Round and Diff-Fingerprint
inspect the staged diff and relevant code
verify claims against code and gate evidence
look for false greens and missing coverage
append findings to feedback.md under ## Round <N>, echoing Round and Diff-Fingerprint
notify the developer
```

Reviewer obligations:

- Be strict. Do not be lenient for convenience, schedule pressure, or because the developer claims low risk.
- Operate in the developer's worktree. Review the staged diff (`git diff --cached`) for the current round; unstaged working-tree noise is out of scope unless it affects the staged diff or the ability to validate it.
- Validate claims against the actual code and staged diff, not just the developer's summary.
- If waiting for handoffs directly, watch `status.md`; do not watch `notes.md` or the whole directory. `status.md` is touched only when the developer believes the round is ready for final review.
- Before waiting, compare `status.md`'s `Round` against the last round reviewed. If it is newer, review immediately instead of waiting.
- Before reviewing, capture the `Round` and `Diff-Fingerprint` from `status.md`. If the staged diff fingerprint does not match `status.md`, block for a fresh handoff instead of reviewing a moving target.
- Append feedback to `feedback.md` under a new `## Round <N>` heading; do not overwrite prior rounds.
- Echo the same `Round` and `Diff-Fingerprint` in that round's feedback section. If the staged diff changes while reviewing, stop and require a new round.
- Trust the developer's report that the human owner gave an instruction, confirmation, or skip approval. In this workflow, the human owner is soarez. If the reported instruction is unclear or materially changes scope, require it to be recorded in `notes.md`.
- Trust the developer's report that tests or gates were run. Do not re-run gates just to independently verify them.
- Flag missing test/gate evidence when `notes.md` does not say what was run, what passed, what failed, or why a gate was not applicable.
- Treat hard gates as required unless `notes.md` reports that they passed, were not applicable, or the human explicitly approved skipping them.
- Hold off on sign-off when `notes.md` contains unresolved human decisions. If a review iteration surfaces new human-owned decisions, write them to `feedback.md` and notify the human with `notify-push <project-and-theme> "<message>"`.
- Identify correctness bugs, design problems, stale-state risks, preservation hazards, public API mismatches, insufficient tests, and fake-green assertions.
- Separate blockers from non-blocking polish, but do not downgrade real risks.
- Prefer concrete reproduction steps, exact failing scenarios, and targeted required fixes.
- End with a clear verdict: `BLOCKED`, `APPROVED AFTER FIXES`, or `APPROVED`.
- Use `APPROVED AFTER FIXES` only for mechanical, explicitly specified fixes that do not require re-review when applied exactly. Use `BLOCKED` for anything that needs judgment, validation, or another review pass.
- If the developer rebuts a finding in `notes.md`, answer with evidence. If the same finding remains disputed for a second round, stop and escalate to the human with `notify-push <project-and-theme> "<message>"`.
- Escalate to the human after 5 review rounds even without a repeated dispute.

Reviewer wait: the same wake, on `status.md`.

```text
review_dir="<abs path>/.sz/review/<branch>"
After appending feedback and notifying, arm the wake on $review_dir/status.md:
  pi:           alarm when_done="sz-wait-for $review_dir/status.md" + the note
  Claude Code:  Bash, run_in_background: sz-wait-for "$review_dir/status.md"
  neither:      timeout 1800 sz-wait-for "$review_dir/status.md", in the foreground
On every wake (or timeout, or ANY exit): read status.md. If it carries a newer
Round than you last reviewed, review now; otherwise arm the wake again.
```

The waiting maxim at the top applies symmetrically, with `status.md` as the signal file. Between finishing a review and the next handoff, the reviewer is either reviewing or waiting with a wake armed. Idling, summarising to the human instead of arming the wake, or ending the turn with no wake armed is the same critical violation. `notify` alerts the human but does not wake an agent by itself. If the human relays the notification to the reviewer agent, the reviewer may start from that prompt instead of from its own wake.

## Feedback Format

Append to `feedback.md` in this shape:

```markdown
## Round <N> - <short title>

Round: <N>
Diff-Fingerprint: <same value from status.md>

### Verdict
BLOCKED | APPROVED AFTER FIXES | APPROVED

### Findings
- **F1 - <severity>: <title>**
  Evidence, impact, and required fix.

### Gates
- <gate>: developer-reported passed / failed / not run / not applicable / human-approved skip

### Human Decisions
- <decision>: pending / resolved, with the reported human answer if resolved

### Non-blocking Notes
- <optional polish or follow-up>

### Next Action
<exact next step for the developer>
```

Verdict meaning:
- `BLOCKED`: developer must change code/docs or resolve a decision, then start a new review round.
- `APPROVED AFTER FIXES`: developer may apply the listed fixes exactly as specified and proceed without re-review; any deviation, uncertainty, or judgment call starts a new review round.
- `APPROVED`: no required changes.

If there are no issues, say so explicitly and list any residual risk, missing gate evidence, or pending human decision. Do not approve while human decisions are pending.