Skip to content

chore: add a local code review workflow for claude code sessions - #5088

Draft
mbani01 wants to merge 1 commit into
mainfrom
chore/local-review-workflow
Draft

mbani01 wants to merge 1 commit into
mainfrom
chore/local-review-workflow

Conversation

@mbani01

@mbani01 mbani01 commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

What

A repo-local code review workflow for Claude Code sessions. Draft: shared for discussion, not planned for an early merge.

  • REVIEW.md is the review contract: what counts as a failure in CDP, the tags (Important, Nit, Pre-existing), review depth by path, human-approval tiers, skip list, convergence rules and the summary format. Humans and agents follow the same file.
  • /review (.claude/skills/review/SKILL.md) runs one bug-hunter agent per lens in a fresh context, then independent finding-verifier agents that vote on every finding. The report starts with a count. A review of the whole current change records a content-keyed marker.
  • .claude/hooks/review-state.sh gate is a PreToolUse hook on Bash that refuses git push from Claude until the current change has a review with zero open Important findings and the working tree matches HEAD. CDP_SKIP_REVIEW=1 is a one-push bypass the user types.
  • /review-pr (someone else's PR) calls /review <N> for the bug hunt and posts only after the user approves the draft. /preflight gains a local review check.

Scope

The gate only intercepts pushes that Claude runs through its Bash tool. Pushes from a terminal, an IDE or another agent are not gated, and the human review tiers on the PR still apply. Nothing is posted to GitHub automatically.

Prerequisites

Claude Code 2.1.295 or newer, jq on PATH, git 2.31 or newer, and origin/main (or CDP_REVIEW_BASE for a fork). Listed in REVIEW.md.

Verification

  • The gate's 111-case command matrix (push forms, wrappers, aliases, heredocs, refspecs, unresolvable and command-substituted refspecs, redirections, invocations of the state script itself, bypass, fail-closed on bad base or missing jq, multi-repo pushes) passes on macOS and on Alpine Linux 3.20 (busybox awk, no shasum).
  • The change was reviewed with /review itself: round 1 found two Important defects in the gate (it refused its own mark and decline commands when a finding mentioned a push, and an unresolvable refspec such as $BRANCH bypassed the HEAD-only check); both are fixed and covered by the matrix. Round 2 found two more in the skill text (the $STATE shorthand defeated the hook's exemption for its own commands, and Phase 1 ran vitest on the working tree for PR-number targets); both fixed. Round 3 found two more tokenizer defects (a refspec built by $(...) was split off the push and escaped the HEAD-only check; 2>&1 and > file operands were read as push arguments and refused a clean push); both fixed and in the matrix. Two Nits remain open: quoted text that merely mentions a push trips the non-git branch, and a push in a foreign repository is refused with advice to run /review, which cannot mark that repository.
  • The hook and agent frontmatter fields were checked against the current Claude Code docs.
  • pnpm exec oxfmt --check passes on every changed file.

REVIEW.md is the review contract. The /review skill runs parallel bug-hunter agents per lens and independent finding-verifier agents that vote on every finding, reports a count first, and records a content-keyed marker. A PreToolUse hook refuses git push from Claude until the current change has a review with no open Important findings. /review-pr and /preflight route to it. Nothing is posted to GitHub automatically.

Signed-off-by: Mouad BANI <mouad-mb@outlook.com>
Copilot AI balanced review requested due to automatic review settings October 22, 2026 14:35
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The gate can permit unreviewed commits, and review-state and classification defects can incorrectly produce a clean result.

8 open issues
What changed in this PR

Review: 8 Important, 0 Nit, 0 Pre-existing.

This draft adds a repository-local review workflow for Claude Code, connecting automated defect review to a content-keyed push gate while keeping human approval and GitHub posting separate.

Changes:

  • Defines review standards, specialist review agents, and independent finding verification.
  • Adds review-state markers and a Bash push gate.
  • Integrates local review into preflight, skill routing, and approval-controlled PR reviews.
File Description
REVIEW.md Defines review expectations, risk tiers, and convergence rules.
.claude/​skills/​review/​SKILL.md Orchestrates review, verification, reporting, and markers.
.claude/​skills/​review-pr/​SKILL.md Integrates defect review and approval-controlled GitHub posting.
.claude/​skills/​preflight/​SKILL.md Adds a required local-review check.
.claude/​settings.json Registers the Bash push-gate hook.
.claude/​rules/​skill-guidance.md Routes local and external PR review requests.
.claude/​rules/​review-workflow.md Establishes local review and push-gate workflow rules.
.claude/​hooks/​review-state.sh Implements review markers and push validation.
.claude/​agents/​finding-verifier.md Defines independent verification of candidate findings.
.claude/​agents/​bug-hunter.md Defines specialized defect discovery and structured output.

Review effort: Balanced


1. Open the file at the stated line in the working tree. If the line does not show the code the finding describes, look for it nearby; if it is nowhere in the file, the verdict is REJECTED.
2. Rebuild the failure scenario yourself from the code, following the actual call path. Read callers and callees as needed. Do not take the hunter's `evidence` on trust.
3. Check whether the defective line exists at `BASE`. If it does, set `pre_existing: true` whatever the hunter said.
*) i=$((i + 1)) ;;
esac
done
[[ $i -lt ${#t[@]} && "${t[$i]}" == "push" ]] || return 0
Comment on lines +230 to +235
--all | --mirror | --branches) echo "$arg"; return 1 ;;
-*) continue ;;
esac
if [[ $first -eq 1 ]]; then first=0; continue; fi
src="${arg%%:*}"; src="${src#+}"
[[ -z "$src" ]] && continue
refuse "the push names '$other', which is not the checked-out HEAD or does not resolve locally, so its review state cannot be checked." \
"Check out the branch you want to push, run /review there, then push it as HEAD."
fi
diff_is_empty && return 0
;;
mark)
load_state || fail_base 1
key=$(diff_key)
Comment on lines +43 to +45
- `status` is `clean` and the user did not ask for a re-review: say so in one line and stop. The gate will let the push through.
- `status` is `empty`: nothing to review, stop.
- New files you created this session that are untracked belong to the change: `git add -N <those files>` so the diff and the key include them. Do not add unrelated untracked files.
- `status` is `clean` and the user did not ask for a re-review: say so in one line and stop. The gate will let the push through.
- `status` is `empty`: nothing to review, stop.
- New files you created this session that are untracked belong to the change: `git add -N <those files>` so the diff and the key include them. Do not add unrelated untracked files.
- Round = `last.round + 1` when `last` is for this branch and either its key differs from the current one or it still has open Important findings; otherwise 1. Round 4 is not allowed: stop, show the still-open Important findings, and tell the author the approach needs reconsidering (REVIEW.md, Convergence). After that reconsideration the author starts a new cycle with `--fresh`, which treats `last` as round 0; `--fresh` is typed by the user, never added by you.

**PR number:** `gh pr view <N> --json title,body,headRefName,baseRefName`, `git fetch origin pull/<N>/head:refs/remotes/origin/pr/<N>`, set `BASE=$(git merge-base origin/<baseRefName> origin/pr/<N>)`, and produce the diff with `git diff $BASE origin/pr/<N>`. Tell the hunters and verifiers to read head code with `git show origin/pr/<N>:<path>` instead of the working tree. `ALREADY_REPORTED` is `[]`.

**Range or paths:** `BASE` is the range start (or the merge base for paths); produce the diff with `git diff <start> <end>` or `git diff $BASE -- <paths>`. `ALREADY_REPORTED` is `[]`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants