review: the reviewer set is prose-enforced while the gate is seam-enforced #123

New issue

Closed

opened 2026-08-13 19:32:25 +00:00 by john · 1 comment

john commented

2026-08-13 19:32:25 +00:00

Owner

Summary

The review gate level is enforced by a seam; the review reviewer identity is enforced by prose. A model can silently substitute a same-model claude-subagent for the configured cross-model provider on a blocking trigger, and nothing refuses it or records the substitution.

Observed

2026-08-13, during /cad-verify 1 route_failures, applying an approved fix:

review.reviewers is ["openai"] (set globally).
route.mjs resolve --role cad-reviewer returned review.risk_surface: "blocking" — correct.
The risk_surface fire was then dispatched to cadence:cad-reviewer (a Claude subagent). references/review-triggers.md step 3, "Resolve the reviewer set", was never executed.
Nothing failed. The user caught it.

Why it happens

route.mjs resolve returns four knobs — model, effort, review (the trigger→gate map), verify. It does not return review.reviewers, or any resolved reviewer set. Reviewer identity lives in a separate config key that only a prose instruction (references/review-triggers.md step 3) tells the model to read.

So the two halves of one decision have different enforcement classes:

Decision	Source	Enforcement
Does this trigger gate, and how hard?	`route.mjs resolve` → `review{}`	seam, structured
Which reviewer runs?	`review.reviewers[]`	prose, honor system

Why it matters

The trace bracket for a claude-subagent fire (trace append --family lifecycle --event dispatch --plan cad-reviewer --role cad-reviewer) is identical whether or not that reviewer was the configured one. The cross-model arm deliberately writes no bracket at all. So after the fact:

/cad-report and trace render cannot distinguish "fired at the configured cross-model reviewer" from "fired at a same-model subagent instead".
A blocking gate reports as fired and passed in both cases.
The entire value of configuring a second model — an independent voice that does not share the author's blind spots — is silently lost, on the one trigger that is blocking at every stakes level.

This is the same defect class as CFG-01/CFG-02, one layer up: the config states one thing and the enforcement is prose, so inspection and enforcement can diverge. Phase 1 of v3.2.0 closed exactly this for config.json; the review seam still has it.

Proposed fix

route.mjs resolve returns the resolved reviewer set. Do step 3's availability resolution (review.providers.<name>.tiers[<trigger.tier>] non-null, claude-subagent always available, empty→["claude-subagent"] fallback) inside the seam, and return it as a reviewers field alongside review. Prose then reads a resolved value instead of re-deriving one.
Refuse a dispatch to a reviewer outside the resolved set wherever that is checkable — at minimum have the seam return the set so the fire site has no excuse, and state the fallback explicitly in the return (fallback: "openai tier flagship unassigned") so a fallback is visible rather than inferred.
Record the actual reviewer on the trace event. Add the reviewer identity (openai/gpt-5.6-sol, claude-subagent/cad-reviewer) to the lifecycle event, and give the cross-model arm some event so a fire that ran only cross-model is not invisible. Today that arm has no bracket by design — which also means a cross-model review that never happened leaves no trace either.

(3) is what makes the failure detectable after the fact; (1) is what makes it unlikely. Both are worth having — the current state has neither.

Notes

references/review-triggers.md already says "never silently skip a blocking trigger" for the empty-set fallback case. This is the adjacent hole: not skipping it, but running it at the wrong reviewer.
Also captured in .planning/CAPTURE.md under Todos.

## Summary The review **gate level** is enforced by a seam; the review **reviewer identity** is enforced by prose. A model can silently substitute a same-model `claude-subagent` for the configured cross-model provider on a `blocking` trigger, and nothing refuses it or records the substitution. ## Observed 2026-08-13, during `/cad-verify 1` `route_failures`, applying an approved fix: - `review.reviewers` is `["openai"]` (set globally). - `route.mjs resolve --role cad-reviewer` returned `review.risk_surface: "blocking"` — correct. - The `risk_surface` fire was then dispatched to `cadence:cad-reviewer` (a Claude subagent). `references/review-triggers.md` step 3, "Resolve the reviewer set", was never executed. - Nothing failed. The user caught it. ## Why it happens `route.mjs resolve` returns four knobs — `model`, `effort`, `review` (the trigger→gate map), `verify`. It does **not** return `review.reviewers`, or any resolved reviewer set. Reviewer identity lives in a separate config key that only a prose instruction (`references/review-triggers.md` step 3) tells the model to read. So the two halves of one decision have different enforcement classes: | Decision | Source | Enforcement | |---|---|---| | Does this trigger gate, and how hard? | `route.mjs resolve` → `review{}` | seam, structured | | Which reviewer runs? | `review.reviewers[]` | prose, honor system | ## Why it matters The trace bracket for a `claude-subagent` fire (`trace append --family lifecycle --event dispatch --plan cad-reviewer --role cad-reviewer`) is identical whether or not that reviewer was the configured one. The cross-model arm deliberately writes **no** bracket at all. So after the fact: - `/cad-report` and `trace render` cannot distinguish "fired at the configured cross-model reviewer" from "fired at a same-model subagent instead". - A blocking gate reports as fired and passed in both cases. - The entire value of configuring a second model — an independent voice that does not share the author's blind spots — is silently lost, on the one trigger that is `blocking` at every stakes level. This is the same defect class as CFG-01/CFG-02, one layer up: **the config states one thing and the enforcement is prose, so inspection and enforcement can diverge.** Phase 1 of v3.2.0 closed exactly this for `config.json`; the review seam still has it. ## Proposed fix 1. **`route.mjs resolve` returns the resolved reviewer set.** Do step 3's availability resolution (`review.providers.<name>.tiers[<trigger.tier>]` non-null, `claude-subagent` always available, empty→`["claude-subagent"]` fallback) inside the seam, and return it as a `reviewers` field alongside `review`. Prose then reads a resolved value instead of re-deriving one. 2. **Refuse a dispatch to a reviewer outside the resolved set** wherever that is checkable — at minimum have the seam return the set so the fire site has no excuse, and state the fallback explicitly in the return (`fallback: "openai tier flagship unassigned"`) so a fallback is visible rather than inferred. 3. **Record the actual reviewer on the trace event.** Add the reviewer identity (`openai/gpt-5.6-sol`, `claude-subagent/cad-reviewer`) to the lifecycle event, and give the cross-model arm *some* event so a fire that ran only cross-model is not invisible. Today that arm has no bracket by design — which also means a cross-model review that never happened leaves no trace either. (3) is what makes the failure detectable after the fact; (1) is what makes it unlikely. Both are worth having — the current state has neither. ## Notes - `references/review-triggers.md` already says "never silently skip a `blocking` trigger" for the empty-set fallback case. This is the adjacent hole: not skipping it, but running it at the wrong reviewer. - Also captured in `.planning/CAPTURE.md` under Todos.

john added this to the v3.2.0 milestone

2026-08-13 19:32:25 +00:00

john

2026-08-13 19:32:25 +00:00

closed this issue
added the
bug
label

john commented

2026-08-14 17:02:01 +00:00

Author

Owner

Fixed in v3.2.0 (PR #124). RVW-02, phase 3. The reviewer set is resolved in the seam beside the gate, with its fallback and cause stated in the return, and reviewer identity rides the lifecycle event. A cross-model fire leaves an event of its own, so a substituted subagent is visible afterwards instead of being indistinguishable from the review that was configured.

Verified through the phase UAT walk. The milestone audit traced 12/12 requirements with 0 broken and 36/36 acceptance criteria covered.