feat(code-review): review non-application files + three TestQuality shapes

[mikeangstadt]

Summary

Extends the shared reviewer prompt to cover the file classes the fleet currently does not review, plus three TestQuality shapes that stay green while proving nothing.

Prompt-only. All behavior lives in tools/prompts/shared_prompt.txt, which every reviewer role loads. No dispatch, roster, category, or schema change; no new agent; no new stage.

Why: the fleet's coverage is categorically absent outside application source

Measured against five days of one consuming repo — 2,617 original review comments (replies excluded) across 326 PRs, 2026-07-30 → 2026-08-04, attributed per reviewer agent:

file class	total findings	from this fleet	fleet share
CI guard/deploy scripts	101	0	0%
workflow definitions (.github/)	69	0	0%
docs / MDX	94	0	0%
migrations / schema	28	0	0%
test files	219	1	0.5%
app code .tsx	684	465	68%
prototypes	193	139	72%
app code .ts	1,099	301	27%

The fleet is strong where it is pointed and produces zero findings on four whole file classes. Those classes are 22% of the corpus and are carried almost entirely by other reviewer agents — so the capability gap is real, not a question of whether those files contain reviewable defects.

Note the limit of this measurement: it shows output, not cause. It does not by itself prove the reviewers would have found these defects if prompted — only that they are not reporting them today. If the zeros turn out to come from file-scope filtering in partitioning rather than from prompt silence, this PR is necessary but not sufficient, and the follow-up is in derive-spawn-spec/partitioning rather than here.

What changed

<non_application_files> — declares those files in scope and enumerates the generic defect classes per class, all repo-agnostic:

• CI/pipeline: privileged jobs running repository lifecycle scripts before minting a write token; workflow-scope write permissions and persisted checkout credentials; the implicit "all dependencies succeeded" condition that is false for a skipped dependency; missing job timeouts; cancel-on-new-run over irreversible side effects; trigger filters that do not match what they gate; a shell step whose unguarded command pre-empts the branch owning its exit code.
• Guard/meta-test scripts: guards that assert a predicate appears rather than executing the decision; guards anchored to the first matching block instead of the production node; allowlist entries broad enough to suppress future real violations.
• Migrations: destructive statements selecting survivors by a proxy for canonicity; idempotency guards that no-op over an invalid object left by a failed run; an enum value added without updating every classifier that switches on it.
• Docs/API specs: claims stronger than the code enforces (unconditional where the code is conditional, product-wide where the behavior is one lane); a behavior change whose describing doc is not updated; generated-client specs admitting combinations the runtime always rejects.

Pinned-file pair detection — repos routinely hold meta-tests asserting another file's literal content (an action-version pin, a config snapshot, a required-context manifest, an allowlist). Reviewers now grep for a test referencing the changed path or a distinctive changed literal and report when that test exists but is not in the diff. Emitted as Correctness, not CompanionChange, deliberately: it is a provable CI failure with a concrete assertion to cite, and it carries no rule-driven reasoning_certificate — which templates/companion_change.md interpolates and would render empty for an LLM-emitted finding.

<test_quality> — three shapes, all generic:

1. Wiring unproven — helper tested directly, production caller never asserted; deleting the call site leaves the suite green.
2. Assertion of existence — source grepped/AST-inspected for a predicate instead of executing that decision against synthetic inputs, so it cannot distinguish a correct implementation from a widened one.
3. Input-invariant fixture — a mock or route handler returning identical results regardless of the params the test claims to cover.

New TestQuality subcategories route to the existing templates/testquality_specialized.md via the TestQuality/* fallback row, so /code-review fix needs no new template.

Test plan

• python3 -m pytest test_code_review_helpers.py test_code_review_schema.py — 1,286 passed, 0 failed.
• Checked that no test pins shared_prompt.txt content before editing it (the same rule this PR teaches): every test writes a synthetic prompt file to tmp_path; the real file is only ever copied verbatim into the CR dir by code_review_helpers.py:13719. No pinned-file pair broken.
• Verified only .claude-plugin/plugin.json is bumped per release (.codex-plugin/plugin.json tracks separately and was last touched by an unrelated commit), matching the v3.7.0 release commit's file set. Bumped 3.7.0 → 3.8.0 with a CHANGELOG entry.
• git diff --check clean; plugin.json parses.

Cost. shared_prompt.txt grows 317 → 409 lines (~17.5 KB → ~24 KB), about +1.6k input tokens per reviewer agent. It is identical across agents and stable across runs so it caches, but on a large fleet the uncached first-touch cost is real — /code-review cost will show the fleet-side delta.

What could still break. Prompt additions can shift reviewer attention: a fleet told to review workflow YAML may spend budget there on PRs where the application diff mattered more, and the new heuristics could produce false positives on repos whose CI conventions differ (a deliberate cancel-in-progress, an intentionally broad allowlist). The existing FLAG criteria and the verifier pass both still apply, and every new heuristic requires a concrete file:line, so these should surface as verifier-refuted rather than shipped — but the first few runs on a CI-heavy PR are worth watching.

[wongk]

this adds explicit things to review to the shared reviewer prompt, rather than assigning these things to specific reviewers. this means every review agent is going to duplicate these reviews, and it will dilute the agent-specific review areas.

[shafty023]

Beginning a code-review. Please allow time for me to finish before merging the PR

[mikeangstadt]

@wongk — your CHANGES_REQUESTED is addressed in 01e357f. You and @shafty023 (thread on shared_prompt.txt:106) raised the same objection: the checklists were in the shared reviewer prompt, so every agent duplicated them and the role-specific review areas were diluted.

The checklists are out of shared_prompt.txt and assigned to specific reviewers. What stays shared is a scope declaration only — those file classes are in scope and carry the same evidence standard — plus an explicit instruction not to reconstruct another role's checklist from it. 92 shared lines became 17.

The assignment is split by whether the defect is decidable from the changed file:

• Bug Hunter A (tools/prompts/bha_suffix.txt, diff-local correctness): CI/pipeline definitions, migrations/schema, guard-script shape, and the two test shapes readable in the test file.
• Bug Hunter B (suffix in skills/spawn-reviewers/SKILL.md, cross-file, unpartitioned, standard + fast-path copies): the pinned-file pair, wiring-unproven, and docs/API-spec. Each needs a file outside the diff by construction.

No defect class was dropped. The auditor, design critic, impact analyzer, domain critics and verifier now carry the 17-line declaration and nothing outside their role.

Also in that commit, from @shafty023's three other threads: the pinned-file-pair class now requires citing the contradicting assertion rather than a path reference (his test_code_review_helpers.py:21401-21412 counterexample reproduced — it names plugin.json as fixture input and asserts nothing about it); the "not in the diff" inference is removed, since a partitioned BHA worker cannot establish full-PR diff membership; the class emits Correctness/pinned-file-pair routed to manual-surface in /fix, because generic Correctness auto-fixes at the anchor and --apply would have reverted the intended config change; and the CHANGELOG entry is rewritten to /update-documentation-derivable sources only.

Merged main in — the branch was conflicting, and main had already shipped a v3.8.0, so the version moved to 3.11.0. uv run pytest plugins/ 2175 passed, 3 skipped; ruff check . and pyright clean.

I have not dismissed your review — only you can. Re-review when you have a moment.