Review

Overview

Unified review skill covering code quality, QA analysis, and visual design audit.

When to Use

• The user says "review this PR", "review my code", "check this diff", "QA analysis", "test coverage gaps", "design audit", "check accessibility", "check performance", or "check SEO".
• A pull request, merge request, or local branch needs findings before merge.
• A feature needs its behavior paths mapped against its tests.
• A page or component needs a contrast, responsive, accessibility, or performance audit.

For a full architecture assessment, use /assessment; for project-wide security scanning, use /audit; for comments on your own PR, use /respond; for shipping, use /ship.

Process

If no subcommand is given, default to code.

code

Review a pull request, merge request, or local branch changes with rigorous, detail-oriented analysis. Every line of the diff is scrutinized for correctness, security, performance, maintainability, and adherence to best practices.

Use two references:

1. ../../checklists/checklist.md for all 71 quality categories.
2. reviewer-prompt.md in this directory for comment format and examples.

Arguments

• No arguments: review the PR for the current branch. If no PR, fall back to local mode.
• PR number(s) or URL(s): review those PRs sequentially, then correlate them before presenting.
• --local: review local branch diff against base.
• --post: post review as inline comments without asking, someone else's PR only.
• --backend: review only backend/infra files.
• --frontend: review only frontend files.

Scope Filtering

When --backend or --frontend is passed, classify each file:

Frontend: paths containing frontend/, web/, client/, src/app/, src/pages/, src/components/, src/hooks/, src/styles/, public/. Extensions: .tsx, .jsx, .vue, .svelte, .css, .scss.

Backend: paths containing backend/, server/, api/, services/, workers/, jobs/. Extensions: .go, .py, .rb, .rs, .java, .kt. Infra included with backend.

Shared: packages/, libs/, shared/, root config, prisma/, migrations/. Included in both scopes.

The filter bounds which files get a line-by-line pass. It never bounds which files a finding may name. Whenever a changed file declares a type, a constant, a validation rule, a format, or a helper that its counterpart on the other side of the wire also declares, read that counterpart and report the divergence, even when the filter excluded it.

The duplication itself is the finding, and it is invisible from one side. A response type written out in both the service and the API client, a fingerprint helper copied across the boundary with a comment asking the two to stay identical, a date parsed in one language on the server and another on the client: none of these live in a shared folder, because the absence of a shared folder is the defect. A scoped review that reports only its own half hands back a diagnosis that reads as complete and is not.

Say in the terminal report which side got the line-by-line pass and which side was only traced for contracts.

Steps

1. Gather context, parallel: remote URL, branch, CLI tool, account resolution. Parse flags.

2. Determine mode: PR mode if PR exists, local mode otherwise. Check PR state, must be OPEN.

3. Get diff and context: PR mode gets metadata and diff via GH_TOKEN=$(gh auth token --user <account>) gh pr diff/glab mr diff. Local mode detects base, fetches, diffs. Warn about uncommitted changes in local mode. Fail fast here, before any subagent is spawned: confirm the base ref resolves with git rev-parse --verify <base> and the diff is non-empty. A bad ref or an empty diff ends the review with one line naming the ref or the empty range.

   • Independent reviewer. When this session authored the diff, meaning it made the commits under review or edited those files earlier in the conversation, run steps 5 through 10 and the Spec Axis in a fresh subagent. Brief it with the diff command or PR number, the commit list, and the checklist path, never with this session's reasoning about the change. The authoring context holds every assumption that shaped the code, so reviewing from inside it confirms those assumptions instead of testing them.

4. Apply scope filter if --backend or --frontend passed.

5. Read context: PR description, commit messages, every changed file in full, imported modules, verify PR description matches diff. For existing feedback, sweep every channel in ../../standards/pr-comment-channels.md: inline threads, review bodies, PR-level conversation, and commit comments. Reading only inline threads means re-raising a point another reviewer already made in the conversation box, or contradicting a decision the author already explained there. Start the Spec Axis subagent now, in parallel with the remaining steps.

6. Read the checklist, then discover applicable standards and rules. Open ../../checklists/checklist.md and read it in this session. Not "recall the 71 category names", not "apply the summary in the system prompt": read the file. The category headings are an index; the findings live in the individual bullets under them, and a bullet you have not read is a bullet you will not check. A review that never opened the file is a review that silently downgraded itself to whatever the model happened to remember.

   Then read ~/.claude/rules/index.yml. Scan the project for technology signals: file extensions, framework markers, package.json, go.mod, Cargo.toml, Gemfile, requirements.txt, pyproject.toml, import statements in changed files, directory names, and config files. Match signals against trigger keywords in the on_demand section. Load every matched standard file plus all always_loaded rules.

   This makes the review aware of domain-specific best practices. A PR that adds a database migration is reviewed against standards/database.md. A PR that adds a GraphQL resolver is reviewed against standards/graphql-api-design.md. A PR that adds a queue consumer is reviewed against standards/message-queues.md.

   Also load these rules for review context:

   • rules/verification.md: when reviewing claims about test coverage or build success
   • rules/pre-flight.md: when reviewing whether the author checked for existing solutions
   • rules/security.md: security criteria, OAuth 2.1, passkeys, NIST 800-63B, secrets management, supply chain
   • rules/writing-precision.md: quality gate for review comments themselves
   • rules/code-style.md: completeness, immutability, error classification, type conventions, LLM trust boundary, TypeScript 5.x
   • rules/testing.md: mock policy, test structure with the three bare AAA markers as the only comments, faker, deterministic tests, contract testing, performance regression
   • standards/performance.md: Core Web Vitals budgets, API latency targets, bundle size limits
   • rules/privacy-defaults.md: data minimization, retention, erasure when the diff touches personal data
   • rules/ai-guardrails.md: when the diff processes, stores, or acts on LLM-generated output

   Before suggesting fixes that call library APIs, check standards/llm-docs.md for the library's documentation URL. Verify the API exists.

   Record which standards were loaded for inclusion in the review verdict.

7. Scope detection. Categorize the diff into scope signals to determine which specialist checks to prioritize:

   Signal	Detection	Specialist focus
   SCOPE_FRONTEND	.tsx, .jsx, .vue, .css files, components/, pages/	Performance budget (cat 54), design quality (cat 52), accessibility
   SCOPE_BACKEND	.go, .py, .rs, service files, handlers	API design, error handling, concurrency
   SCOPE_API	Route definitions, controllers, OpenAPI changes	API versioning, rate limiting, backward compatibility
   SCOPE_AUTH	Auth middleware, token handling, login/signup	OAuth 2.1, passkeys, rate limits, NIST 800-63B
   SCOPE_MIGRATIONS	Migration files, schema changes	Schema-migration sync (cat 69), expand-contract (cat 55), backward compatibility
   SCOPE_EVENTS	Event handlers, queue consumers, publishers	Event-driven patterns (cat 57), idempotency, DLQ
   SCOPE_DEPS	package.json, go.mod, requirements.txt changes	Supply chain (cat 56), SBOM, typosquatting
   SCOPE_INFRA	Dockerfile, terraform, k8s manifests, CI config	Container security, zero-downtime (cat 55)
   SCOPE_LLM	LLM client calls, prompt templates, AI output processing	LLM trust boundary (cat 53), output validation
   SCOPE_WEBHOOK	Webhook handlers, signature verification, external system callbacks	Idempotency key stability, replay protection, concurrent delivery, input canonicalization, startup secret validation
   SCOPE_TESTS	Test files	Mock policy, coverage, three marked AAA blocks with no other comment, faker
   SCOPE_DOCS	README, docs/, CHANGELOG	Documentation accuracy, stale references
   SCOPE_CONFIG	.env, config files, settings	Secret exposure, env var completeness

   Use scope signals to prioritize review depth: apply the most thorough analysis to categories matching the detected scope. Categories outside the scope still receive baseline checks.

8. Blast radius analysis: the diff is not the review boundary, the project is. For every changed file, trace outward to find code that depends on the change. Read every impacted file, not just the diff.

   7a. Identify what changed at the interface level. Extract every modified export, function signature, type, interface, enum, route, database column, env var, config key, event name, and public API contract from the diff.

   7b. Find all consumers. For each changed interface, grep the entire project for:

   What changed	Search for
   Exported function or class	All import { name } and call sites
   Type or interface	All files that reference the type name
   Enum or constant	All files that use the enum or constant
   API route or endpoint	All fetch, axios, trpc, href, action references to that path
   Database model or column	All services, repositories, seed files, and migrations referencing it
   Env var	All process.env reads and .env.example
   Event name or message type	All publishers and subscribers
   Config key	All consumers of the config module
   CSS class or design token	All className references and Tailwind config

   7c. Read every impacted file. Read the full content of every consumer found in 7b, not just the import line. Verify the consumer still works correctly with the new interface. A function that changes its return type from string to string | undefined might have 40 callers that do not handle undefined.

   7d. Flag impact findings. For each consumer that would break or behave differently after the change, record: the consumer file and line, what it expects, and how the change violates that expectation. These findings have the same severity as bugs found in the diff itself.

   7e. Cross-PR correlation, when more than one PR is under review. An interface change in one PR and a consumer of it in another are invisible from either side. Each branch typechecks against the base, both report green, and the break appears only when the second one merges. Reviewing the PRs one after another and presenting two independent verdicts hands back a diagnosis that reads as complete and is not.

   For every symbol one PR adds, narrows, or makes required, grep the other PRs' branches for constructors and callers that would no longer satisfy it: a new required field on a shared interface, a narrowed return type, a changed function signature, a bumped wire-format version, a renamed env var, a new enum member that widens an exhaustive switch. Check the base branch too, since a symbol absent there is what lets both PRs pass in isolation.

   Report the collision on both PRs, not one. Name which PR is the natural home for the fix, and say plainly that merge order decides whether the base branch breaks. When one PR's infrastructure already provisions what the other's code expects, say so: that turns a design argument into a missing line.

9. Behavioral Flow Analysis, MANDATORY, do not skip. Per-file analysis catches syntax-level bugs. This step catches design-level bugs: broken idempotency, race conditions, data corruption under concurrency, and attack vectors. Skipping this step is the failure mode that sends a pull request back for a second review pass.

   9a. Trace every request lifecycle end-to-end. For each new handler, endpoint, consumer, or webhook in the diff, trace the complete request from HTTP entry through every service call to every database write and external side effect. Do not review files in isolation. Follow the data.

   What to trace	Verify
   Input parsing and validation	Every field validated. Canonicalization applied (lowercase emails, trimmed strings). No implicit trust of external input shape
   Authentication and authorization	Secrets validated at startup, not just at request time. Signature verification uses constant-time comparison. Timestamp windows reject future values, not just old ones
   Idempotency key derivation	What input produces the key? Does that input change on retry by the external system? If derived from request metadata (timestamp, nonce), verify stability across application-level retries vs network-level replays
   Idempotency key lifecycle	If the key is consumed (stored) before the protected operation completes, what happens when the downstream step fails? Can the operation be retried, or is it permanently blocked?
   Multi-step mutations	Are all steps in a single transaction? If not, what state does the system reach if it fails between steps? Can a retry recover, or is the data permanently corrupted?
   Return values to external systems	Do response codes prevent unnecessary retries (200 for known non-errors)? Do they cause silent failures when misconfigured (200 for promotion-not-found hides config errors)?

   9b. Model concurrent actors. For every write path, identify every actor that can trigger it concurrently: retries from the same external system, parallel requests from different systems, user actions that touch the same data, cron jobs that overlap. Draw a timeline with at least two concurrent actors and verify no interleaving produces incorrect state.

   Run the concurrency-auditor agent unconditionally when the diff touches a write path, meaning any service, handler, route, consumer, job, or repository that persists, mutates shared state, charges, sends, or enqueues. It returns one actor timeline per write path plus file:line findings, and its output feeds the Behavioral Flow Analysis summary in step 14. Reference standards: ../../standards/concurrency.md for the race taxonomy and the correctness ladder, ../../standards/idempotency.md for key stability and replay.

   Concurrency scenario	What to check
   Two webhook deliveries for the same logical event	Do they produce the same idempotency key? If not, do they race through shared mutations?
   Webhook + user action	Can a user action (place bet, spend token, change email) interleave with a webhook mutation (void bonus, activate promotion)? Is the shared state protected by a lock?
   Retry after partial success	If step 1 of 3 succeeded but step 2 failed, does a retry re-execute step 1? Is step 1 idempotent or protected?
   Concurrent requests with different idempotency keys	If the dedup key is derived from mutable request metadata (timestamp), two requests for the same logical operation get different keys and both proceed

   9c. Read the database schema. For every application-level uniqueness check, findFirst then create, SELECT then INSERT, read the actual schema file and verify a corresponding UNIQUE constraint, @@unique, or UNIQUE INDEX exists. An application check without a DB constraint is a TOCTOU race that produces duplicates under concurrency. Never assume a constraint exists because the code checks for it.

   9c-bis. Schema-migration parity. MANDATORY when SCOPE_MIGRATIONS detected. This gate exists because a pull request in a production repository shipped a migration with 7 raw-SQL CREATE INDEX statements and zero matching @@index declarations in schema.prisma, and the drift went undetected for 17 days. Reference: the ## Prisma section of standards/orm-migrations.md. Reference checklist: category 69.

   For every changed migration file, perform these checks in order. Skipping any of them is a review failure.

   1. Enumerate every DDL statement in the migration. List CREATE INDEX, CREATE UNIQUE INDEX, DROP INDEX, ALTER TABLE ADD COLUMN, ALTER TABLE DROP COLUMN, CREATE TABLE, DROP TABLE. Extract the exact identifier name from each statement.

   2. For each enumerated DDL, grep schema.prisma for the matching declaration. Report the result per item using this exact format:

      [PARITY OK]    CREATE INDEX "Game_homeTeam_sportsbook_search_idx" -> @@index([homeTeam], map: "Game_homeTeam_sportsbook_search_idx")
      [PARITY MISS]  CREATE INDEX "Line_name_sportsbook_search_idx" -> NOT FOUND in schema.prisma (P0 BLOCKING)
      

   3. Run prisma migrate diff and paste the full output. Command:

      pnpm exec prisma migrate diff \
        --from-schema-datamodel <schema.prisma path> \
        --to-migrations <migrations dir> \
        --exit-code
      

      Non-zero exit code is a P0 blocking finding. The exact diff output must appear in the review.

   4. If a fresh DB is available, run end-to-end verification. prisma migrate deploy followed by prisma migrate dev. If the second command generates a follow-up migration, drift exists. Paste the generated migration in the review and flag as P0.

   5. Verify every index name uses an explicit map:. Auto-generated Prisma names collide with raw-SQL names and cause the exact drift described above. Names must follow <Model>_<col>(_<col>)*_<purpose>_idx.

   6. Verify any DDL without a schema counterpart is documented. Extensions, materialized views, GIN/GiST trigram indexes, custom triggers, and RLS policies cannot be modeled in Prisma. The PR description or an ADR must name the unmanaged object class and why no schema entry exists; a comment in the migration file does not count, because comments are banned in every language. Missing documentation is a P1 finding.

   None of these steps can be skipped on the grounds that "the hook would have caught it." The hook is layer 2; the review skill is layer 4. Defense-in-depth means each layer runs every check independently.

   9d. Model attacker capabilities, when SCOPE_AUTH, SCOPE_WEBHOOK, SCOPE_API detected. Assume the attacker has: the signing secret, compromised dependency, leaked env var, the ability to send arbitrary requests at high volume, knowledge of the endpoint contract. For each assumption, trace what damage is possible.

   Attacker capability	What to verify
   Compromised signing secret	Can they pre-sign requests with future timestamps? Can they generate requests that bypass replay protection?
   High-volume valid requests	Is there rate limiting? Can they exhaust Redis memory (long TTL keys), DB connection pool, or external API quotas?
   Payload manipulation	Is email canonicalized? Can mixed-case or Unicode variants bypass dedup? Can oversized arrays or deeply nested objects cause resource exhaustion?
   Replay with modification	If the attacker replays a valid request with a different timestamp (new signature), does idempotency catch it?

   9e. Verify startup and configuration safety. For every secret, API key, or credential read from environment variables at request time, verify there is a startup check that fails loudly if the value is missing or empty. A missing webhook signing secret that silently rejects all deliveries is a silent outage.

10. Three explicit passes, applied to the diff AND to impacted files from step 8, informed by behavioral flow analysis from step 9:

• Pass 1: Per-file analysis. Every applicable category from checklist.md, 1 through 71. This includes the extended categories: 53, LLM Trust Boundary when code processes AI output, 54, Performance Budget for frontend changes, 55, Zero-Downtime Deployment for migration and deploy changes, 56, Supply Chain for dependency changes, 57, Event-Driven for queue and event handler changes, and 58, Licensing for new or modified source files. Additionally, for each standard loaded in step 6, verify that changed code follows the patterns in that standard. When a finding originates from a loaded standard, note the standard internally for your own tracking, but never reference it in externally-posted comments. The posted comment must state the engineering reason directly. Apply to changed files first, then to impacted consumer files where the change alters behavior. Use scope signals from step 7 to prioritize depth. Findings from step 9, behavioral flow analysis feed directly into this pass: race conditions become category 4 findings, idempotency gaps become category 18 findings, missing constraints become category 5 findings.

  Security pattern analysis, when SCOPE_AUTH, SCOPE_API, SCOPE_WEBHOOK, or SCOPE_BACKEND is detected. Read ~/.claude/skills/security-patterns.md and apply it as an additional security lens. For each changed file that handles user input, perform source-to-sink tracing: map entry points to dangerous sinks, SQL, command, template, file, SSRF, redirect, XSS, deserialization, and verify sanitization at each transition. Check for vulnerability patterns matching the diff: race conditions on financial operations, IDOR, mass assignment, JWT weakness, CORS misconfiguration, missing idempotency. When a security finding is identified, run git blame -L <start>,<end> <file> on the vulnerable lines to determine when it was introduced and how long it has been exposed.

• Pass 2: Cross-file and project-wide consistency. Category 15. Contradictions, import chain side effects, config completeness, contract alignment, error path consistency. Verify that every consumer identified in step 8 still compiles, passes type checks, and behaves correctly. Check for: stale type assertions, missing null checks on new optional returns, tests that assert old behavior, documentation that describes old behavior, and mocks that replicate old signatures.

  Placement. Correct code in the wrong module is still a defect, and it is the one the other passes structurally cannot see, because every file reads fine on its own. For each new symbol ask: which layer owns this fact, and does the project already have a home for it?

  Signal	What it usually means
  A provider's wire format assembled outside the only module that talks to that provider	The adapter is making its caller speak the provider's dialect. Push the serialisation inward
  A hand-rolled guard where the project already has a middleware, base class, or helper for that job	Nobody looked. The inconsistent error shape it produces is a second, downstream finding
  The same type, constant, or helper declared on both sides of a boundary	It belongs in the shared package the project already has
  A workaround for an upstream quirk applied in every client	Normalize once at the layer that owns the response
  A docstring that tells the caller what it must do first	Read it as a confession. "The controller handles that conversion" names the leak out loud

  Grep before asserting a home is missing. The strongest version of this finding names a facility the project already has and the diff did not use: an existing validation middleware the new route bypassed, an existing shared package that already holds the sibling type. That is checkable and cheap to act on. A placement finding that instead proposes a new abstraction is speculation, and it should be dropped.

  Smell baseline. Match the diff against these twelve code smells from Fowler's Refactoring, chapter 3. Report each as a judgement call, "possible Data Clumps", at confidence 5 to 6 per Confidence Scoring, never as a blocking violation. A documented project convention that endorses the shape suppresses the smell, unless it contradicts a rule in ../../CLAUDE.md. Skip any smell a linter in the project already enforces.

  Smell	What it is	Fix
  Mysterious Name	A name that does not reveal what the symbol does or holds	Rename; when no honest name comes, the design is unclear
  Duplicated Code	The same logic shape in more than one hunk or file	Extract the shared shape and call it from both
  Feature Envy	A function that reads another object's data more than its own	Move it onto the data it envies
  Data Clumps	The same few fields or parameters travel together	Bundle them into one type and pass that
  Primitive Obsession	A primitive standing in for a domain concept	Give the concept its own branded or value type
  Repeated Switches	The same branch on the same type recurs across the change	One shared map or polymorphism for every site
  Shotgun Surgery	One logical change forces scattered edits across many files	Gather what changes together into one module
  Divergent Change	One module edited for several unrelated reasons	Split so each module changes for one reason
  Speculative Generality	Parameters, hooks, or abstractions no requirement asks for	Delete and inline until a real need appears
  Message Chains	Navigation such as a.b().c().d() the caller depends on	Hide the walk behind one method on the first object
  Middle Man	A unit that mostly delegates onward	Remove it and call the real target
  Refused Bequest	A subclass or implementer that overrides most of what it inherits	Replace inheritance with composition

• Pass 3: Cascading fix analysis. Category 16. For every issue: if the author fixes it exactly as suggested, what new problems could that introduce?

11. Run local verification: test, with coverage, lint, build. After tests pass, verify that coverage on changed files and their direct dependents is 100%, with no exclusion directive added. Apply ../../checklists/checklist.md category 8. If coverage is below threshold, flag it as a blocking finding.

12. Check external sources. If the PR description, commit messages, or code comments reference external projects, articles, or third-party codebases as inspiration, apply ../../checklists/checklist.md category 50, Clean Room. If no references are found, ask the author: "Were any external projects or codebases used as reference during implementation?" If yes, run the clean room checks against the diff. If no, skip category 50.

13. Check branch freshness, CI, test evidence, PR size, parallel. PR > 400 lines = warning, > 1000 = blocking.

    A branch behind its base is blocking only when the drift touches files the PR changes. Compute the intersection of the two changed-file lists rather than reading the commit count: comm -12 over git diff --name-only <head>...<base> and git diff --name-only <base>...<head>, both sorted. An empty intersection is drift, and it belongs in the terminal report as an observation, never in the verdict. A non-empty one is a real rebase requirement, and naming the overlapping files is what makes it actionable.

14. Present review in the terminal with verdict: APPROVE, REQUEST_CHANGES, or COMMENT. Include operational risk assessment for non-trivial changes. Include a blast radius summary listing every file outside the diff that is affected by the change. Include a Behavioral Flow Analysis summary covering the lifecycle traces, concurrent actor timelines, and attacker models from step 9, plus a Standards Applied line. Present the Spec Axis report under its own ## Spec heading, after the standards findings and apart from them. All of it is for the person running the skill. None of it is published: the pull request receives inline findings and a review event, nothing else.

    External output is a separate channel. The in-terminal presentation includes internal scaffolding such as Standards Applied, the Behavioral Flow Analysis section heading, and severity tiers like P0, P1, and P2. What goes to GitHub does not. Before building the JSON payload, do a forward pass that strips:

    • Conventional Comments label prefixes, issue, blocking:, issue, non-blocking:, nitpick:, suggestion:, question:, thought:, praise:, chore:, todo:. Use no label.
    • Internal severity tiers, P0, P1, P2, and section headings, Standards Applied, Behavioral Flow Analysis, Blast Radius, Spec.
    • The skill's own invocation flags, --backend, --frontend, --local, --post.
    • Internal file references such as ~/.claude/, rules/<name>.md, standards/<name>.md, checklist.md, or CLAUDE.md.
    • "Category N" or "checklist item N" phrasings; reach for the engineering reason directly.

    This is a forward filter, not a post-hoc rewrite. If you find yourself rewriting after generation to scrub these, the generation prompt was wrong. Cleanliness from the first draft is the bar. See standards/code-review.md "No Internal Config Leakage" and the internal-config-leakage.py hook, which will block the post if you miss something.

15. Next steps:

    • Own PR / local: offer to fix issues. Convergence loop, max 5 iterations: fix, re-verify, re-audit. If 5 iterations are exhausted with findings still open, stop, list the remaining issues, and inform the author. Five iterations is enough for any reasonable convergence; remaining issues likely need a design change, not another fix pass.
    • Someone else's PR: offer to post inline comments. Show the exact payload first: each comment with file, line, body text, and suggestion blocks. Ask for confirmation before posting. --post skips the confirmation prompt but still shows the payload summary. Inline findings are the only thing that gets posted; the review carries no summary body, per ../../rules/pr-comment-discipline.md.

Spec Axis

The checklist passes ask whether the change is built right. The spec axis asks whether it is the right thing. A change can pass one and fail the other: code that meets every standard while implementing the wrong behavior, or code that does exactly what was asked while breaking conventions. Reported in one list, the passing axis hides the failing one.

1. Find the spec, stopping at the first hit:
   1. The ## Requirements section of the active plan folder under <root>/plans/, where the workspace root is <repo>/.work/ or <repo>/docs/ resolved by hooks/_lib/project_workspace.py, per ../../rules/project-workspace.md. The active folder is the newest one whose slug matches the branch or the PR title.
   2. The ADDED, MODIFIED, and REMOVED delta sections that plan folder carries against specs/current/, per ../../rules/living-specs.md.
   3. A ticket referenced from the commit messages, the branch name, or the PR body, such as Closes #45 or a Linear key, fetched with the account resolved in step 1.
   4. Nothing found: record Spec: no spec found and skip the axis. A missing spec never blocks the review, and the axis never infers requirements from the code.
2. Run it in an isolated subagent. The brief carries the diff command or PR number, the commit list, and the spec path or fetched text, and nothing from the standards passes. It ends with: "Perform this review directly. Do not invoke /review or spawn agents. Under 400 words." The subagent returns three lists, and every finding quotes the spec line it rests on:
   • Requirements missing or only partially implemented.
   • Behavior in the diff that no requirement asked for.
   • Requirements that look implemented but are implemented wrong.
3. Report it apart. The terminal report carries it under ## Spec. Never merge, rerank, or deduplicate it with the standards findings. The closing line names the worst finding within each axis, never one winner across both. A spec finding anchored to a diff line follows the same posting rules as any other finding.

Posting Comments via Pending Review

Posting applies to a pull request someone else authored. On your own pull request a finding is a commit, so nothing is posted.

../../rules/pr-comment-discipline.md bounds what a review may carry: inline findings anchored to a line, and nothing else. No summary body, no verdict prose, no praise line, no count of what was checked. The verdict reaches the author through the review event, APPROVE, REQUEST_CHANGES, or COMMENT, which is one word and cannot be padded. Everything the summary body used to hold goes to the terminal, for the person running the skill.

When posting, always use the pending review API to batch all comments into a single notification. Use a JSON file with --input to avoid shell escaping issues with markdown, tables, and code blocks in comment bodies.

Step 1: Get the latest commit SHA.

GH_TOKEN=$(gh auth token --user <account>) gh pr view <PR_NUMBER> --json commits --jq '.commits[-1].oid'

Step 2: Write the review payload to a JSON file.

{
  "commit_id": "<COMMIT_SHA>",
  "event": "REQUEST_CHANGES",
  "body": "",
  "comments": [
    {
      "path": "src/auth.ts",
      "line": 20,
      "body": "Plain-prose comment body, no label prefix. A ```suggestion\nblock\n``` can be embedded when proposing a code change."
    },
    {
      "path": "src/auth.ts",
      "line": 35,
      "body": "Second comment"
    }
  ]
}

Write the file with cat <<'EOF' > /tmp/review-payload.json, single-quoted delimiter to prevent shell expansion. Clean up after posting.

Step 3: Submit the review in a single API call.

GH_TOKEN=$(gh auth token --user <account>) gh api repos/:owner/:repo/pulls/<PR_NUMBER>/reviews \
  -X POST \
  --input /tmp/review-payload.json \
  --jq '{id: .id, state: .state}'

This creates and submits the review in one step. No separate "create PENDING then submit" flow needed.

JSON payload rules:

• line is the line number in the file, new version. Do not use side or position, they are not valid on this endpoint
• event in the top-level object sets the review type directly, and it is the whole verdict
• The top-level body is always the empty string. It is the review summary slot, and a summary is never published. body inside each comment is the inline comment text
• A review with an empty body cannot have its body edited later, which the GitHub API enforces. That is the intended outcome: there is nothing to edit
• For multi-line comments, add start_line alongside line
• Always clean up the temp file after posting: rm /tmp/review-payload.json

Establish which lines are anchorable before writing any comment. Fetch the per-file patch once with GH_TOKEN=$(gh auth token --user <account>) gh api repos/:owner/:repo/pulls/<n>/files and read the hunk headers from that. gh pr diff --patch, and a --paginate call piped through a jq select, both concatenate patches across commits or pages, so the same file comes back with several hunk sets whose line numbers do not describe head. A comment anchored from those lands on the wrong line or is rejected outright.

Files with status added are the safe case: every line is in the diff, so any line number is valid. For a modified file, only lines inside a hunk range are valid. A finding that falls outside every hunk cannot be anchored and therefore cannot be posted; report it in the terminal so the person running the skill can raise it directly.

Event type mapping:

Never post comments individually. Even a single comment goes through the JSON file flow. This prevents notification spam and avoids shell escaping failures with complex markdown.

Comment Voice (MANDATORY)

Every posted comment must read like a human colleague wrote it. The standards/code-review.md "Review Comments" section is the source of truth: no prefix labels, no structured templates, no checklist appearance. State the engineering reason directly.

Internal triage, severity, category, source rule, stays internal. It informs which comments you choose to write and how blocking they are in the verdict, but it never appears in the posted text. The reader has no access to the configuration and must never learn it exists.

Forbidden in posted bodies:

• Conventional Comments label prefixes like issue, blocking:, issue, non-blocking:, nitpick:, suggestion:, question:, thought:, praise:, chore:, todo:. These signal "I am working from a template" to any reader. Write the same content without the label.
• Internal severity tiers: P0, P1, P2, or any heading like ## P0 Blocking. The verdict you compute internally maps to GitHub's APPROVE / REQUEST_CHANGES / COMMENT event, full stop.
• The skill's own invocation flags. If you ran the review with --backend, the posted comment is not "Backend-only review (--backend)". The first-person voice is "I went through the backend changes" or just plain English.
• References to internal config: ~/.claude/, rules/<name>.md, standards/<name>.md, checklist.md, CLAUDE.md, "category 17", "checklists item 3".
• Structural section headings carried over from internal taxonomy: "Behavioral Flow Analysis", "Blast Radius Summary", "Standards Applied". Internal-only sections; their content reaches the reader in plain prose, not under those labels.

Comment shape:

Open differently across comments. Sometimes a description, sometimes a question, sometimes a one-line observation. Variet

…(truncated)