--- 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 blocking, turn-consuming loop. 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 *blocked in `sz-wait-for` waiting for it*. There is no third state. **The maxim:** whenever there is no new, current-round content in the other side's signal file, you MUST immediately run `sz-wait-for ` synchronously in the foreground. For the developer that file is `feedback.md`; for the reviewer it is `status.md`. No exceptions. **When `sz-wait-for` exits — for any reason whatsoever** (settle/wake, timeout, non-zero exit, signal, tool error, anything) — you MUST repeat the loop: re-check the signal file, act on it if it carries new current-round content, otherwise run `sz-wait-for` again. A single exit is never a reason to stop. No exceptions. **Idling is a critical violation.** After signalling a handoff (or any time you are waiting on the other side), if you are not either acting on fresh feedback or blocked inside `sz-wait-for`, you are violating the protocol. 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 / going idle / "standing by" without `sz-wait-for` running. - Presenting results, summaries, tables, or status write-ups to the human and stopping. - Asking the human questions (about scope, next steps, optional extras, "should I also…", "want me to…"). Save non-blocking questions for `notes.md`; do not let them replace the wait. - Starting unrelated work, or polling in ad-hoc ways instead of `sz-wait-for`. If you catch yourself about to write a closing summary or a question to the human instead of running `sz-wait-for`, that is the tell: stop and run `sz-wait-for` instead. **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. 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) — and those escapes are `notify-push` + wait, not silent idling. Background jobs, notifications, 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// status.md notes.md feedback.md ``` `` 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/" 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: Diff-Fingerprint: ``` 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// 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 feedback wait, then signal via status.md and notify 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 ` 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 ""`. - 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 wait, then signal. Do not notify and then wait; that can miss a fast feedback write. Then obey the waiting maxim above: the moment the handoff is signalled, your very next action is `sz-wait-for` — not a summary, not a question, not the end of your turn. ```bash review_dir=".sz/review/" # Wait synchronously in the foreground. Do NOT background sz-wait-for: a # background process cannot return control to this agent when feedback arrives, # and its output may be buffered, making it look like feedback.md was never # updated. 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. ``` This is a `while` loop, not a one-shot: `while no current-round feedback: sz-wait-for; recheck`. The `timeout 1800` exists so a stalled watcher cannot hang forever — a timeout means *re-arm*, never *give up*. Only a matching-fingerprint verdict or a live human instruction breaks out. Use `notify-push ""`, 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// 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 , 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 ` 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 ""`. - 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 ""`. - Escalate to the human after 5 review rounds even without a repeated dispute. Reviewer wait: ```bash review_dir=".sz/review/" # Wait synchronously in the foreground. Do NOT run sz-wait-for in the # background: a background process cannot return control to this agent when the # handoff signal arrives, and its output may be buffered, making it look like # status.md was never updated. timeout 1800 sz-wait-for "$review_dir/status.md" # After waking (or on timeout, or on ANY exit), read status.md. If it carries a # newer Round than last reviewed, review now; otherwise IMMEDIATELY re-run # sz-wait-for. Never stop here. ``` The reviewer agent must block on `sz-wait-for` synchronously — 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 blocked in `sz-wait-for`; idling, summarising to the human, or ending the turn 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 running its own wait. ## Feedback Format Append to `feedback.md` in this shape: ```markdown ## Round - Round: Diff-Fingerprint: ### Verdict BLOCKED | APPROVED AFTER FIXES | APPROVED ### Findings - **F1 - : ** 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.