code-review-loop

Code Review Loop

Overview

End-to-end code review etiquette. Covers the requesting side (preparing a PR that's reviewable) and the receiving side (acting on feedback). The skill exists because most code review failures aren't disagreement — they're noise. Reviewers get PRs they can't reasonably review (1500 lines, mixed concerns, no description) and authors get feedback they don't engage with seriously (silent dismissals, "fixed" without explanation, defensive replies). The skill enforces structure on both ends and dispatches claudekit:code-reviewer / claudekit:security-auditor agents on the diff. Used after verification-gate, before merge.

When to Use

• Opening a PR for review
• Responding to review comments on a PR you authored
• Reviewing a PR another engineer authored (the skill applies symmetrically)
• Re-requesting review after addressing feedback

When NOT to Use

• Quick fixes via direct push to a branch nobody else uses (no review needed)
• A PR is already merged and you have post-merge feedback (file a follow-up issue, don't re-litigate)
• Reviewing infra/config that the project's policy explicitly auto-approves

Process

Step 1: Prepare the PR (requesting side)

Goal: A reviewable PR.

Inputs: A branch with verified changes (you've run verification-gate).

Actions:

1. The PR title is one line, describing what changed. Not "Updates" or "Fix stuff." Verb-led: "Add idempotency key to charge endpoint."
2. The PR description has these sections:
   • What: 1-3 sentences naming the change.
   • Why: the spec link, the ticket, the bug being fixed.
   • How: the design choice, especially if non-obvious. Cite the plan if one exists.
   • Verification: the output from verification-gate (paste or link).
   • Risk + rollback: if the change has any risk, name it and the rollback procedure.
3. Check the diff size. If >400 lines (excluding tests, generated files, lockfiles), consider splitting the PR. Reviewers won't read the whole thing carefully; they'll skim, miss issues, and approve.
4. Tag the right reviewers. For sensitive paths (auth, payments, data), tag the security-savvy reviewer too.

Output: A PR open for review with the description filled out.

Step 2: Dispatch the reviewer agents

Goal: A first pass before human reviewers spend their time.

Inputs: The open PR.

Actions:

1. Dispatch claudekit:code-reviewer on the diff. Returns: structural findings (data flow, error handling, edge cases), style findings, complexity findings.
2. If the diff touches auth/, payments/, crypto/, users/, sessions/, tokens/, or any path with sensitive-data semantics, also dispatch claudekit:security-auditor. Returns: input-validation findings, OWASP-aligned findings, secret-handling findings.
3. Read both findings lists. Address obvious issues (typos, missing error handling, easily-fixed structural notes) yourself before human reviewers see the PR.
4. Push the changes. Note in the PR description that automated reviewer agents ran, plus any findings you intentionally deferred.

Output: A PR that has been pre-reviewed by agents; obvious findings already addressed.

Step 3: Receive feedback (receiving side)

Goal: Engage with every comment.

Inputs: Reviewer comments on the PR.

Actions:

1. Read every comment before responding to any. Get the full picture; don't start replying piecemeal.
2. For each comment, choose one of three responses:
   • Agree + apply: make the change. Reply with the commit hash that applied it. Don't reply "fixed" without the hash.
   • Disagree + explain: explain why you disagree. Cite evidence (a test, a constraint, a decision in the spec). Ask the reviewer if your reasoning resolves their concern.
   • Need more context: ask the reviewer for clarification. Don't guess at what they meant.
3. Never silently dismiss a comment. If you didn't apply it and didn't reply, the reviewer assumes you missed it.

Output: Every comment has a response thread.

Step 4: Apply changes in coherent commits

Goal: Make the diff history easy to re-review.

Inputs: The agreed changes from Step 3.

Actions:

1. Group changes by topic. One commit per topic, even if multiple comments contributed to it.
2. Each commit message names what changed and references the comment thread ("Address review: extract validation to <module>; thread #N").
3. Don't squash before re-review unless the project's policy demands it. Reviewers want to see what changed since their last pass.

Output: New commits on the branch addressing the agreed feedback.

Step 5: Re-request review

Goal: Hand back to the reviewer with a clear next step.

Inputs: The branch with applied changes.

Actions:

1. Add a single comment on the PR summarizing what you addressed and what you pushed back on:
   • "Addressed: comments #1, #3, #5 (commits a1b2c, c3d4e)"
   • "Pushed back: comments #2, #4 — see threads"
2. Re-request review through the platform's mechanism (re-assign, request re-review, etc.).
3. Don't ping by Slack/IM unless the PR is blocking and reviewers are unaware.

Output: Reviewers re-engaged with a summary of what changed.

Step 6: Close the loop

Goal: Merge cleanly.

Inputs: Approval from required reviewers.

Actions:

1. Confirm CI is green at the most recent commit (not the branch tip from when review was requested).
2. Resolve all comment threads. If a thread has unresolved disagreement, the PR shouldn't merge yet — escalate or compromise.
3. Merge using the project's standard method (squash, merge commit, rebase).
4. If the PR introduced anything not yet rolled out (feature flag off, config not flipped), the PR is merged but not delivered — track delivery separately.

Output: PR merged. Any pending delivery steps tracked.

Rationalizations

Content truncated.