feat(implementation): dispatch tagged phase-boundary reviews to quality-gate

[kyle-sexton]

Closes #8331

Summary

A plan phase tagged Review: architecture, security, code-design or concurrency now gets that review at its phase boundary, run by /implementation:implement and /implementation:implement-dispatch through /review:quality-gate. An untagged phase gets no review.

Fix

• New plugins/implementation/skills/implement/context/phase-review.md holds the sequence: invoke /review:quality-gate architecture, security, code-design or slice concurrency with the previous phase-boundary commit as --base, the plan's Quality bar: line when present, --report-only, and --worktree for a worker's worktree, passing no rationale. For a worker branch's first phase, --base is the commit the branch was created from. Then fix inside the phase fence, re-review once with the same --base as the first call (the whole phase diff stays in scope, so a fix that missed its finding still rates it and a regression the fix added rates too), and give the acceptance verdict last.
• Any CRITICAL on the re-review stops the loop (the original finding, a regression the fix introduced, or a defect the first call missed): it goes to DEVIATIONS.md as a blocking human-decision and is escalated once, never looped.
• implement/SKILL.md and implement-dispatch/SKILL.md load that file at a tagged boundary, and implement-dispatch carries the full review call.
• Permission decision: this adds no new grants, and no git -C * allowed-tools rule anywhere. When the session's permission mode denies the review's worktree reads, the run logs the review to DEVIATIONS.md as not run (unverified), the phase-verifier still runs, and Step 5's report lists that phase and tag as not reviewed (decision on #8331).
• Four eval cases: pb-tagged-phase-reviews-before-commit (review before commit, the review invocation, verifier after review), pb-untagged-phase-no-review, pb-repeated-critical-escalates-once (same finding CRITICAL again), and pb-regression-critical-stops (the fix clears the first CRITICAL but the re-review rates a different one the fix introduced). In both re-review cases, an llm judge checks that the run plans no further fix brief or review; steps addressed to a person do not count.
• Changelog fragment under .changes/implementation/.

Verification

• phase-verifier: 9/9 acceptance criteria pass.

• Paid evals at aa2efee (3 runs per arm, regex graders, all three runs VALID), with plugin vs without: tagged phase 1.0 vs 0.0; untagged phase 1.0 vs 0.5; repeated critical 0.889 vs 0.667. One case each, so the noise report calls n too small for an interval. The repeated-critical miss: one with-arm run planned another fix brief and re-review instead of a blocking human-decision.

• Paid evals at f84cffe9 (r4, 3 runs per arm, no run errors), with plugin vs without:

  Case	With	Without	Delta
  pb-regression-critical-stops	1.0	0.667	+0.333
  pb-repeated-critical-escalates-once	1.0	0.667	+0.333

  blocking-human-decision (and phase-not-done) stay regex graders. not-a-repeat, no-further-round and no-third-round are now llm judges with explicit pass/fail criteria, because three rounds of regex could not settle these semantic checks. Every with-plugin run passes every grader. Every without run fails only blocking-human-decision. The judges pass the without runs' further-round check because their extra fix and review steps are addressed to a person. That is why the delta is lower than r3's regex grade, which counted those lines against them.

• bash scripts/check-affected.sh --tests --base origin/main from the worktree after merging current main: 7 passed, 2 skipped (no shell or Python changed) over 29 changed files, rerun at f84cffe.

• typos over the changed files: clean.

Related

• feat(planning): carry universal, overridable design defaults through planning output #8326 (parent)
• feat(review): add /review:quality-gate code-design mode #8388 (/review:quality-gate code-design mode, which this branch was stacked on; merged)

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-19T05:13:55.056664Z	ee39af4	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ee39af47ca

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[github-actions]

claude-security-review reviewed f84cffe94a08349455c8dec98c09d8096c18805e: 0 finding(s), 0 as inline comments.

No security issues found in the incremental change (f84cffe, since 7f373fb).

The change touches only eval fixtures under plugins/implementation/evals/:

• It moves three inline regex graders per case into graders/*.md files. The regex patterns are the same as before.
• It replaces two of the regex graders with LLM-judge rubrics: no-further-round and not-a-repeat in pb-regression-critical-stops, and no-third-round in pb-repeated-critical-escalates-once.
• It adds sample answers to the samples/*.json files.

None of these files contain executable code, credentials, workflow or permission changes, or network access. The rubrics are static text.

I did not read the instruction exception register. The change touches no CLAUDE.md, AGENTS.md, rules file, skill or agent body, or hook. The removed regexes were test graders, so no guardrail is deleted.

Moving the regexes to files adds no new trust boundary. The no-further-round regex is dropped in favor of an LLM rubric, and the other two keep theirs. Those patterns only ever run on eval outputs, not on untrusted PR input. A ReDoS in an eval-only grader would not cross a trust boundary.

[github-actions]

claude-review reviewed f84cffe94a08349455c8dec98c09d8096c18805e: 0 finding(s), 0 as inline comments.

Incremental review of f84cffe (changes since 7f373fb): no findings.

The change moves the grader definitions for pb-regression-critical-stops and pb-repeated-critical-escalates-once out of case.yaml and into graders/*.md. It turns no-further-round, no-third-round and not-a-repeat from regex graders into LLM-judged ones, and extends the sample files.

What I checked by reading, not by running:

• Grader layout: collect_graders in plugins/evals/skills/validate/scripts/validate-cases.py takes the union of the case.yaml grader list and graders/*.md, so a case.yaml with no graders: key is valid. plugins/architecture/evals/ already uses the graders/*.md layout.
• Frontmatter: the keys type, arm: both and focus: last_message are inside the validator's allowed sets (GRADER_COMMON, and focus for llm).
• Regex graders: the moved blocking-human-decision and phase-not-done patterns, flags and match are unchanged from the removed case.yaml entries (checked against the diff).
• Samples: the validator only warns that LLM-judged samples are not checked, since that needs a paid calibration run. The new samples are consistent with the new judge criteria:
  • "Next step:", "Next,", "should run the review again", "need to re-run", and "Plan:" are in the fail lists.
  • The person-addressed options and remediation lines are in the pass lists.
  • The "not a repeat" and "does not repeat" lines are in the pass list for not-a-repeat.

I did not run validate-cases.py because the sandbox needed approval for it. Nothing here has been executed.

Evidence note (not a code defect): the PR body's table (1.0 vs 0.333 and 1.0 vs 0.444) says the scores were regraded offline with the graders at 7f373fb, which were regex. The head now uses LLM judges for no-further-round, no-third-round and not-a-repeat, and I saw no paid run against them. Those scores are author-claimed and describe a different grader set than the head carries. If the "with vs without" numbers matter for #8331's "difference in the PR body" criterion, refresh them with a run at this head.