/review-record

Review Record and Close-Out

Every `/craft:review` run that reached a reviewer leaves one record in `.craft/reviews/`, clean or not. A clean record is the denominator: without it, "reviewed and fine" and "never reviewed" look the same on disk. The close-out then graduates each finding to the craft artifact that fits it, and the record points at what each finding became.

Read this file at Step 5b (to write the record) and Step 6 (to close out).

Who writes what

• The reviewer cannot write files. The orchestrator writes the record, from the reviewer's report, in one call to `review-capture.sh`.
• `review-capture.sh` owns the grammar. It derives the file name, craft version, counts and outcome itself, stamps a `graduated_to:` line under every finding (`docs` for a doc-drift finding, blank for the rest), and refuses a body with no Verified list.
• `review-graduate.sh` is the only thing that edits a record afterward.
• The record is immutable after capture. Every finding stays in full, exactly as the reviewer wrote it, forever. The only lines that ever change are a finding's `graduated_to:` line and, on a dismissal, the one `dismissed:` line under it. The record carries `outcome:` (`clean` or `findings`) and no status.

The record

The frontmatter, in order, as capture writes it:

type: review
created, captured_at
mode (branch | story | project), strategy (standard | maze)
reviewed (branch name, story name, or audited path)
ref (<base>..<head> short SHAs, or the head SHA alone in project mode)
focus, files, lines (omitted in project mode)
craft_version, reviewer, gates
issues, suggestions, patterns
outcome (clean | findings)

The body is `## Verified`, one line per check the reviewer made and found clean, then `## Findings`: `None.` when clean, otherwise each finding under `### N. <Issue|Suggestion|Pattern> (<category>): <title>`, numbered 1..N in the order Step 5 presented them. The category is one of security, logic, performance, consistency, doc-drift. A finding keeps the reviewer's seven fields (Symptom, Expected, Actual, Consequences, Reproduce, Evidence, Fix) verbatim, plus `**Withdrawn.** <why>` when the reviewer raised it and then disproved it.

The record name is the file name without `.md`. It is the value every `review:` back-link and every graduate call uses.

The capture call

One call per `/craft:review` invocation, with the body on stdin:

bash "${CLAUDE_PLUGIN_ROOT}/hooks/scripts/review-capture.sh" --mode=<branch|story|project> --strategy=<standard|maze> \
  --reviewed="<branch name | story name | audited path>" --ref="<base7>..<head7> | <head7>" \
  --focus="<csv>" --files=<N> [--lines=<N>] --reviewer="<agent name[, agent name]>" \
  --gates="<what ran - result | none | not reported>" --stdin <<'BODY'
<the body>
BODY

• `--lines` is required for branch and story runs and refused for project runs.
• `--gates` is `not reported` when the reviewer's report has no Gates run line. `none` means the reviewer said it ran nothing; an unknown is never written as `none`.
• When the report has no Verified list, the body carries the single line `- None reported - the reviewer's report listed no checks it found clean.` under `## Verified`. Never invent a check.
• A finding the reviewer withdrew stays in the body in full.
• The body must not already hold a `graduated_to:` line outside a fenced block: capture stamps its own.

The script prints the record's absolute path as its last line. It exits 2 and writes nothing when a flag is missing or the body breaks the grammar; fix the named problem and run it again.

A worked example body that capture accepts (finding 1 is one the reviewer withdrew, so the close-out would propose dismissing it; finding 2 is the issue that matters):

~~~review-body

Verified

• Every caller of `parseToken` handles the new null return
• The migration is idempotent when run twice against the same database

Findings

1. Suggestion (consistency): Error message wording differs from the sibling handler

**Symptom:** The new handler says "Invalid token" where its sibling says "Token rejected". **Expected:** Handlers in `api/` share one wording - "No requirement found"; looked in `locked.md` and the sibling handlers. **Actual:** Read in the code, not run: `api/refresh.ts:31` and `api/login.ts:27` differ. **Consequences:** A client matching on the message text would handle one route and miss the other. **Reproduce:** 1. Check out the branch at the head commit. 2. Compare the two lines. **Evidence:**

api/refresh.ts:31: throw new HttpError(401, "Invalid token")
api/login.ts:27:   throw new HttpError(401, "Token rejected")

**Fix:** Align the wording. **Withdrawn.** Clients branch on the status code, and the two routes already document different messages on purpose.

2. Issue (logic): Refresh path drops the session when the token is null

**Symptom:** A user whose token expired is signed out instead of refreshed. **Expected:** "An expired token is refreshed once before the session is dropped" (story acceptance line 4). **Actual:** Observed - ran `npm test auth`, got `1 failed: refresh keeps session`. **Consequences:** Every user hitting an expired token loses their session mid-work. **Reproduce:** 1. Check out the branch at the head commit. 2. Run `npm test auth`. **Evidence:**

FAIL auth/refresh.test.ts > refresh keeps session
  expected session to exist, got null

**Fix:**

- if (!token) return endSession()
+ if (!token) return refreshOnce()

~~~

Immutability and the one allowed edit

Graduate through the script, never by editing the file:

bash "${CLAUDE_PLUGIN_ROOT}/hooks/scripts/review-graduate.sh" <record name> <N[,N...]> <value> [--by=user|reviewer] [--reason="<text>"]

• `value` is the name of the artifact the finding became (a fix, tweak, bug, story or notebook todo), or `docs`, or `dismissed`.
• A comma list fills several findings in one write, all or nothing.
• `dismissed` requires `--by` and