Auto-must-fix severity floor in review prompts

[sv-tmueller]

Part of batch #423

Part of #344 (package P3 of the Claude-of-Tanks adoption plan, docs/plans/344-claude-of-tanks-adoption.md on the PR #345 branch).

What to build

Review severity is pure model judgment with no floor. The survey's source repo used hard caps that override a critic's overall read. We already have rules that are law here and easy to check objectively, but none of them forces a severity.

Add a severity floor: any of these five conditions makes a finding must-fix, whatever the reviewer's overall read. Each one is already a rule in .claude/process-core.md, so this adds no new policy. It only makes existing policy binding on the severity field.

1. A test deleted, skipped or weakened, without the PR body saying why.
2. --no-verify, or any other bypassed git hook.
3. A new dependency with no justification in the PR body.
4. A CI job with no timeout-minutes, or a workflow with no concurrency group carrying cancel-in-progress: true.
5. A change touching the full stack, shipped without e2e.

The list goes into every review prompt surface:

• Both review workflows' embedded PROMPTS and their prompts JSON, kept in sync.
• The Hermes copy.
• Both reviewer.md files (.claude/agents/ and .claude/adapters/prompts/).

Floor sets severity, not truth (added by batch #423). The critic or consolidate prompt says it cannot downgrade a floor finding. The verify stage (added by #406) can still refute a floor finding on the facts, for example when the "deleted test" was never deleted. The verify prompt does not change.

Prompt text, not code: a static check would be stronger for conditions 2 and 4, and could be a separate future issue. This package delivers the severity floor with a prompt edit and no new code.

Acceptance criteria

• The same five-condition list appears in every review prompt surface named above.
• A new test asserts the list is present in each surface, so an edit to one copy cannot drift silently.
• The critic or consolidate prompt says a floor finding cannot be downgraded.
• npm test is green, including prompts-sync.test.mjs and role-contract-sync.test.mjs.
• version in .claude/.claude-plugin/plugin.json is bumped.

Non-goals

• Static checks or lints that detect these conditions.
• Any new policy beyond what process-core.md already states.
• Changing the severity enum.
• Changing the verify prompt.

[sv-tmueller]

Sub-plan (architect, batch #423)

SUB_PLAN

Delta against the plan. The plan is the #407 issue body, as amended by batch #423 on 2026-10-01; the P3 source is docs/plans/344-claude-of-tanks-adoption.md on the PR #345 branch. Size check: fits size:S. The change is prompt text in 8 files, one new test of about 70 lines, and a version bump. No code changes.

Surfaces, re-derived against main at 43cee0b

• .claude/workflows/tm-review-changes.js: PROMPTS.review (:291) and PROMPTS.consolidate (:295). PROMPTS.verify (:293) stays untouched (non-goal).
• .claude/workflows/tm-review-codebase.js: PROMPTS.area_review (:267), PROMPTS.architecture_review (:269) and PROMPTS.consolidate (:271). These are every template where a severity gets set. scout sets none.
• .claude/workflows/prompts/tm-review-changes.prompts.json and .claude/workflows/prompts/tm-review-codebase.prompts.json, the same keys, byte-identical to the JS.
• "The Hermes copy" is not a separate file on main. hermes-renderer.mjs:42-47 (loadPrompts) reads the prompts JSON directly, and the Hermes reviewer binding is .claude/adapters/prompts/reviewer.md. So the Hermes requirement is met by surfaces already listed. Do not edit hermes-renderer.mjs.
• To keep it that way, the list goes into the PROMPTS templates, never into the dimensions const (tm-review-codebase.js:248) or the DIMENSIONS briefs. Those have an unsynced duplicate, CODEBASE_DIMENSIONS in hermes-renderer.mjs:91, which would turn into a fourth copy to maintain.
• .claude/agents/reviewer.md and .claude/adapters/prompts/reviewer.md.
• Added by me: docs/architecture/role-contracts.md, reviewer section (:168-176). It is the host-neutral source both reviewer bindings derive from. Putting the floor in the bindings only would leave it stale, against the CLAUDE.md rule that a doc is corrected in the same PR. This is a companion edit, not a policy change. If the lead treats it as scope, the only alternative is a known-stale neutral doc. The Codex persona (.claude/adapters/personas/reviewer.toml) has no severity text, so it is left alone.

Decisions the plan left open (made here, overridable by the lead)

1. Workers: a severity rule, not a detection mandate. In the worker templates the wording is "if a finding you report matches one of these, its severity is must-fix". Keep "Stay strictly within your dimension." as it is.
   • Why: the issue frames the floor as binding on the severity field, and detection is a non-goal.
   • The rejected alternative ("in scope for every dimension") would have up to 7 workers report the same floor hit with different problem text. mustFixDeduped dedups on file + line + problem, so each copy would get its own verifier dispatch.
   • Missed floor conditions are covered by the critic's existing clause that it may add "a clear must-fix the reviewers missed", since the critic also carries the list.
2. "Cannot downgrade" means severity only. Each consolidate prompt (both workflows) says: a finding that matches a floor condition stays must-fix and cannot be downgraded to should-fix or nit, but it may still be dismissed if it is false on the facts, with the reason under dismissed.
   • Why: this follows from "floor sets severity, not truth". tm-review-codebase has no verify stage, and unverified findings in tm-review-changes are judged by the critic. Without the dismissal escape, a false floor finding could not be removed in either case.
   • Refuted findings stay as they are: finalizeReport strips them whatever the prompt says.
3. Scoping sentence for tm-review-codebase only. Its three templates add one sentence after the list: a whole-repo review has no diff and no PR body, so apply only the conditions the current tree can show.
   • Why: conditions 3 and 5 need a diff, and condition 1 partly does (a deleted or weakened test only shows in a diff). Without the sentence the list is incoherent in that workflow.
   • The list itself stays identical everywhere.
4. List wording: verbatim from the Auto-must-fix severity floor in review prompts #407 issue body (not the plan file's slightly different wording). Frame it as a sufficient condition ("any finding matching one of these is must-fix"), not "only these".
5. Do not attribute the list to process-core.md in prompt text. Condition 1 is not in .claude/process-core.md. Its source is the reviewer contract (role-contracts.md:175-176, "A weakened or deleted test is always a blocking finding"), so the issue's "each one is already a rule in process-core.md" is wrong for item 1.
   • Leave that existing "always blocking" sentence untouched in all three reviewer texts. It is stricter, and the floor adds to it rather than relaxing it.

File order

1. test: commit: new .claude/workflows/__tests__/severity-floor.test.mjs, red on main.
   • Hold the five conditions as a constant.
   • Normalize whitespace (/\s+/g to one space) on both sides, so markdown line wrapping does not matter.
   • Parse PROMPTS with the same brace-match plus node:vm approach as prompts-sync.test.mjs:35-51.
   • Assert every condition is in each of the 5 JS templates, the same 5 JSON keys, both reviewer.md files, and the ## reviewer section of role-contracts.md.
   • Assert a short no-downgrade sentinel phrase is in both consolidate templates, JS and JSON.
   • npm test already picks up the file through the scripts/test.mjs glob.
2. feat: commit: the JS PROMPTS and the prompts JSON in lockstep. If they land apart, prompts-sync fails in between.
3. feat: (or docs:) commit: the floor goes in as its own block in each of the three markdown files.
   • .claude/agents/reviewer.md: a ## Severity floor section after Pass 2, before Lean track.
   • .claude/adapters/prompts/reviewer.md: after Pass 2, before Report contract.
   • role-contracts.md: a paragraph plus a numbered list after the Pass 2 paragraph.
4. Bump .claude/.claude-plugin/plugin.json from 2.9.0 to 2.10.0 (minor, new review behavior). Make each tm- workflow's meta a literal first statement #424 plans to take 2.9.1 and merge first. On rebase, keep a version above whatever main has (2.11.0 if Token report: price Sonnet 5.5 and use the lead's real output tokens #425 took 2.10.0 first).
5. npm test green, then a draft PR.

Contracts and tests that constrain the change

• role-contract-sync.test.mjs:45-57,117-130: the reviewer ## Report contract must match role-contracts.md ### Report contract exactly. Keep the floor out of both report-contract sections. It also must not add a ### inside them, and no list line may start with ## .
• review-changes-verify.test.mjs:79-81: splits the consolidate prompt on "Confirmed must-fix findings (JSON):", "Refuted must-fix findings", "Unverified must-fix findings" and "Raw should-fix and nit findings (JSON):". The floor text goes in the instruction paragraph before those headers, and must not contain those phrases.
• prompts-sync.test.mjs:114-187: no new {{slot}} markers, inline text only.
• hermes-renderer-prompts.test.mjs:86-98: the opening sentence of each template stays unchanged.
• hermes-prompts.test.mjs:97-103: adapters/prompts/reviewer.md must not contain "CLAUDE.md".
• effort-policy.test.mjs raw-source guard: no "fable" in any new text.
• Writing style: no em dashes. Escape apostrophes in the single-quoted JS strings.
• Merge with Make each tm- workflow's meta a literal first statement #424: it rewrites the top-of-file meta and moves the SPEC header comment. This package edits only the PROMPTS region (:283-296 and :255-272), so a rebase merges cleanly.

Conflicts with docs/architecture/: none. adapter-interface.md "Spec format" is untouched (no spec or slot change), and so is hermes-adapter.md. The only architecture-doc edit is the role-contracts.md companion edit above.

[sv-tmueller]

Package summary (batch #423): PR #428 ready for review

• Pipeline: architect sub-plan (delta, five open-point calls, all accepted in batch Batch: follow-ons-from-417 #423 Decision 2); developer DONE, no deviations; tester PASS at 866409a with six drift mutations caught and named; reviewer APPROVE (quality stage). No fix rounds.
• Checks at 866409a: npm test 485 pass, 0 fail; node scripts/check-version-bump.mjs origin/main HEAD exit 0 (2.9.0 -> 2.10.0); CI green (unit tests, version bump guard). The new test fails on main (17 failures).
• Surfaces: PROMPTS.review and PROMPTS.consolidate in tm-review-changes; area_review, architecture_review and consolidate in tm-review-codebase; the same keys in both prompts JSON files; both reviewer.md files; and the reviewer section of docs/architecture/role-contracts.md. The Hermes copy is covered because the Hermes renderer reads the prompts JSON directly. PROMPTS.verify is unchanged.
• For the human review: the docs/architecture/role-contracts.md hunk. It is outside the issue's original surface list and was added so the source doc the reviewer bindings derive from does not go stale (batch Batch: follow-ons-from-417 #423 Decision 2).
• Should-fix (recommended before merge): nothing pins the "may still dismiss it if it is false on the facts" escape in the consolidate templates. It matters most in tm-review-codebase, which has no verify stage. The tester removed it from a scratch copy and npm test still passed. Reviewer's fix: add that phrase as a second sentinel and assert it in both consolidate templates, JS and JSON.
• Nits: (2) rename "verify prompt is unchanged" and loop over all five conditions (it checks only condition 1); (3) optionally pin the tm-review-codebase scoping sentence ("apply only the conditions the current tree can show"); (4) in both reviewer.md files and role-contracts.md, "is still dismissed, with the reason" assumes a dismissal field the reviewer's report contract does not have; say "is not reported", or similar, in all three together.
• Merge note: merge after PR fix: make each workflow meta a literal first statement #426 (2.9.1), keeping a version above main's. The tester's scratch merge with fix: make each workflow meta a literal first statement #426's branch conflicted only on the plugin.json line; with 2.10.0 kept, npm test gave 495 pass.