Repository navigation
feat(review): add /review:quality-gate code-design mode - #8388
Conversation
Every quality-gate mode takes --worktree, --base, --quality-bar and --report-only, so an orchestrator can review a phase diff in a worker's worktree. With a base SHA no pull-request lookup or fetch runs. Adds the qg-phase-boundary-worktree eval case. Refs #8327 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The phase-boundary inputs are passed explicitly by an orchestrator, so the description needs no new trigger phrase. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-gate-phase-boundary-inputs
…d grants Restore the argument-hint to the mode set so it fits the 100-character budget and matches the lead of the Arguments line; the phase-boundary flags stay documented in the Arguments line and context/phase-boundary.md. Remove the allowed-tools entries that put a wildcard before the git subcommand (git -C * ...). A wildcard there also approves options such as -c core.fsmonitor=<cmd>, which runs a program, in a skill that reviews untrusted diffs. Worktree-targeted commands go through normal permission handling. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Under --base or --worktree, code mode skips the pr-review-toolkit path for its code-reviewer fallback, and downstream mode skips the remote downstream_probe layer and treats the floor as report. Under --report-only the size-crossing loop reads git diff -z instead of a written path file. Name the modes --quality-bar applies to, pass spec text to the spec reviewer, and fail the eval grader only on a finding in admin.js, not on a note that it predates the base. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Name pr and close-out in the --quality-bar row; brief reviewers with each mode's own required inputs; restatement dispatches under --base. - Under --report-only the standards ladder takes its non-interactive rung. - code skips size crossings under --worktree; define the report-only base. - Step 0 skips the PR-exists signal with --base; drop restated upstream. - Eval: fail on any Write, add a no-Edit grader. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
code-reviewer, security-reviewer and architecture-guardian run only the change-set commands their brief supplies (worktree path and base SHA), with no gh lookup, fetch or ls-remote; without one, step 2 is unchanged. phase-boundary.md says the brief replaces that step. criteria mode under --base or --worktree dispatches a fresh-context review:brief-reviewer instead of reviewing inline. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Restatement mode now dispatches its batch reviewer under --worktree as well as --base, so a worktree-only run no longer reviews inline from the invoking checkout. The phase-boundary reviewer brief names the worktree's absolute path as the file-read root, and code-reviewer, security-reviewer and architecture-guardian resolve every Read, Grep, Glob and listed untracked path under a supplied worktree. The advisory line no longer reads as if per-slice passes the plan's rationale. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replace the per-command worktree anchoring with one rule: when the brief supplies a worktree, every command and tool call in the review targets it (git via git -C, Read/Grep/Glob, untracked paths, and any other command such as a dependency audit), never the invoking checkout. Stated identically in code-reviewer, security-reviewer and architecture-guardian, and in the quality-gate phase-boundary brief for every dispatched reviewer, review:brief-reviewer included. The downstream probe's repository root is the worktree path under --worktree. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…alternation typos read the [Ss]earch character class as the misspelling "earch" and failed lint-repo. (?:search|Search) matches the same strings. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The mode loads the consumer's review criteria through the standards resolution ladder, falls back to small bundled criteria (Duplicated Code, Shotgun Surgery, Speculative Generality), and dispatches a fresh-context brief-reviewer. Repetition is CRITICAL only when the diff introduces an unsynced copy in code, naming the fact and the diverging change; Markdown, cascades and the prototype bar stay SUGGESTION. A contradicting consumer standard wins and the report names the overridden default; with no quality bar the review assumes production and says so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Five plugin-eval cases cover the mode's severity rules: an introduced, unsynced code copy is CRITICAL with the production assumption named; a declared-registry entry and a file move are not CRITICAL; Markdown repetition and the prototype bar stay SUGGESTION; a contradicting docs/standards file produces the override line. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The no-critical and repetition-is-suggestion graders now fail only on a rated finding (a severity table cell, a line that starts with the severity, a (CRITICAL) or [CRITICAL] label, or Severity: CRITICAL) and pass a negated mention such as "Not CRITICAL: setup.md is Markdown". The cascade grader requires the CRITICAL finding to name FREE_SHIPPING_THRESHOLD or 50, not the word threshold. code-design.md states that its repetition rule overrides severity.md's "code duplication" IMPORTANT example. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e-code-design-mode
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
claude-security-review reviewed Security review: no findings. The change since c6b5d4c is one line in
Skip gate 2 applies: the diff has no security-relevant surface. I posted nothing. |
|
claude-review reviewed Incremental review of 2cfb05f, which changes one file: The change replaces
I found no correctness or maintainability problems. All other files in the PR are unchanged since the last review at c6b5d4c. |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Closes #8329
Summary
Adds a
code-designmode to/review:quality-gate. It loads the consumer's review criteria through the standards contract and falls back to small bundled criteria when the consumer has none. It rates Duplicated Code that the diff introduces with no shared source or sync check as CRITICAL, naming the repeated fact and the later change that would make the copies diverge; every other repetition is SUGGESTION, as is Markdown repetition and anything underQuality bar: prototype. A consumer standard that contradicts a bundled default wins, and the report saysOverrides bundled default <principle> per <standards file>. A missingQuality bar:line is treated asproduction, and the report says so.Behavior change: a code-design CRITICAL is a merge blocker in unattended lanes, like any other CRITICAL the gate reports (approved as plan item DA-A).
Fix
plugins/review/skills/quality-gate/context/code-design.md(new): bundled criteria, the cascade rule, the CRITICAL exemptions (contract-restatement markers, declared registries, generated files, fixtures, rename diffs; no file-count threshold), consumer-criteria loading and the reviewer brief. The mode always dispatches a fresh-contextreview:brief-reviewer.SKILL.md:code-designin the description, argument hint and router table.context/phase-boundary.md:code-designreads--baseand--quality-bar, and its brief carries the consumer and bundled criteria paths. Built on refactor(review): let /review:quality-gate take phase-boundary inputs (worktree, base SHA, quality bar, report-only) #8327's merged phase-boundary inputs, including its cause-based out-of-phase rule.plugins/review/evals/qg-code-design-*and a review changelog fragment.Verification
bash scripts/check-changed-skills.sh origin/main: PASS (0 errors, 7 advisory warnings).bash scripts/validate-plugins.sh --only review: all manifests and the catalog validated, 0 failing tests.bash plugins/review/tests/change-set-block.test.sh: all cases passed.python3 -I -B plugins/evals/skills/validate/scripts/validate-cases.py plugins/review/evals: no FAIL (WARN only).Scaffolded-standards probe: the
qg-code-design-overridescaffold writesdocs/standards/service-boundaries.md, and itsemits-override-linegrader, which matches that path in the output, passed in the with-plugin runs, so adocs/standards/file written byscaffold_scriptis readable in an eval run.Paid evals:
claude plugin eval, plugin vs no-plugin, 3 runs each,--scaffold, judge opus, threshold 0.8.qg-code-design-cascadeqg-code-design-sanctionedqg-code-design-markdownqg-code-design-prototypeqg-code-design-overrideno-blocking-copy-finding)Total cost about $9.8.
bash scripts/check-affected.sh --tests --base origin/mainon the merged head: exit 0, 8 passed, 1 skipped (SKIP ruff (no changed .py)).Left out: the one
check-affectedskip is ruff, which has no Python file in this diff to lint.Acceptance criteria
grep -c "code-design"on the quality-gate SKILL.md is ≥ 2 (it is 4), andcontext/code-design.mdexists.docs/standards/file written by ascaffold_scriptis readable in an eval run; result recorded under Verification.qg-code-design-cascade,-sanctioned,-markdown,-prototype,-override, run with and without the plugin; deltas under Verification.check-changed-skills.sh origin/mainexits 0 andcheck-affected.sh --testsexits 0; its onlySKIPline is ruff, since the diff has no.pyfile.Related
🤖 Generated with Claude Code