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

CHANGELOG.md

@@ -4,6 +4,18 @@ All notable changes to the claude-plugins project will be documented in this fil
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). Entries are listed newest-first; each plugin section is treated as released when merged to `main`.
 
+### code-review v3.11.0
+
+#### Added
+- Config, CI, infrastructure, migration, docs, and test files in `<files_assigned>` are declared in scope in `tools/prompts/shared_prompt.txt`, on the same evidence standard as application source. The declaration carries no checklist: the per-class defect lists are routed to the reviewer that owns them, split by whether the defect is decidable from the changed file or needs a file outside the diff, and the shared prompt tells each role not to reconstruct another role's list.
+- `tools/prompts/bha_suffix.txt` (Bug Hunter A, diff-local correctness) gains the classes readable in the changed file: CI/pipeline definitions (a privileged job running repository lifecycle scripts before minting or consuming a write-scoped credential; write permissions and persisted checkout credentials scoped to the whole workflow; a manual-dispatch path with no trusted-ref gate; a dependency plus a condition with no status-check function, whose implicit "all dependencies succeeded" is false for a *skipped* dependency; a job with no explicit timeout; cancel-on-new-run over irreversible side effects; a path/trigger filter that does not match what it gates; a shell step whose unguarded command can pre-empt the branch owning its exit code), migrations/schema (a destructive statement selecting survivors by a proxy for canonicity; an idempotency guard that no-ops over an object left by a failed run; a new enum value added without updating every classifier), guard-script shape (a guard asserting a predicate *appears* rather than executing the decision; a guard anchored to the first matching block; an allowlist entry broad enough to suppress future real violations), and two test shapes readable in the test — assertion-of-existence (emitted as `TestQuality`/`weak-assertion`) and input-invariant fixture (`TestQuality`/`mock-faithfulness`).
+- The Bug Hunter B suffix in `skills/spawn-reviewers/SKILL.md` (cross-file, unpartitioned; both the standard and fast-path copies) gains the classes that require a file outside the diff: the pinned-file pair, wiring-unproven (`TestQuality`/`missing-coverage` — a helper tested while nothing asserts its production caller runs it), and the docs/API-spec classes (a claim stronger than the code enforces; a behavior change whose describing document is not updated; a generated-client spec admitting combinations the runtime rejects).
+- Pinned-file-pair detection, emitted as `Correctness` with subcategory `pinned-file-pair`. Repos routinely hold meta-tests asserting another file's literal content — a workflow's action-version pin, a config snapshot, a required-context manifest, an allowlist — and editing the pinned file breaks them. The contract is narrow in three ways:
+  - **A path reference is not evidence.** The reviewer must quote the specific assertion, at its own file:line, whose expected value the pinned file no longer satisfies, and name the contradicted value beside the new one. A test that names the changed path only as a fixture input, a synthetic file list, a glob, or a docstring pins nothing and is not a finding.
+  - **Absence from the diff is not evidence.** A partitioned Bug Hunter A worker receives only its own patch, and a reviewer's `<files_assigned>` is a scope rather than a manifest of the PR, so full-PR diff membership is not knowable to a reviewer and is excluded from the argument. The case rests on the contradiction between the assertion read at HEAD and the file as it now stands.
+  - **The companion assertion is structured data**, carried in the existing `other_locations[]` field (`{file, line, issue}`) rather than in prose.
+- `skills/fix/SKILL.md` routes `Correctness`/`pinned-file-pair` to the **manual-surface** bucket with a new `templates/correctness_pinned_file_pair.md`. Without that row the finding fell through to the `Correctness` auto-fix row, whose drift check and edit both anchor on `finding.file` — the changed config or workflow — so `--apply` would have reverted the intended change rather than updating the unchanged assertion. The template renders both sides and leaves the choice of which one is wrong to the operator.
+
 ### code-review v3.10.0
 
 #### Added

plugins/code-review/.claude-plugin/plugin.json

@@ -1,7 +1,7 @@
 {
   "name": "code-review",
   "description": "Code review plugin",
-  "version": "3.10.0",
+  "version": "3.11.0",
   "author": {
     "name": "ClosedLoop",
     "email": "support@closedloop.ai"

plugins/code-review/skills/fix/SKILL.md

@@ -96,6 +96,7 @@ For each surviving finding, look up its dispatch bucket using the table below. S
 
 | Category | Subcategory | Bucket | Notes |
 |---|---|---|---|
+| `Correctness` | `pinned-file-pair` | **manual-surface** | The finding anchors on the CHANGED file (diff-scope validation requires it), but the edit that resolves it belongs in the UNCHANGED companion assertion cited in `other_locations[]`. Auto-fix would edit the anchor and revert the intended config/workflow change. Two files, one of them outside the diff — operator judgment. |
 | `Correctness` | — | **auto-fix** | Direct code edit at anchor line |
 | `Code Quality` | — | **auto-fix** | DRY / maintainability — auto-fix at anchor |
 | `Documentation` | — | **auto-fix** | Edit cited file:line |
@@ -199,6 +200,7 @@ For each finding in the manual-surface bucket, look up the template per the rout
 | `TestQuality/test-deletion` | `templates/testquality_test_deletion.md` |
 | `TestQuality/*` (other, pre-PLN-723) | `templates/testquality_specialized.md` |
 | `ImpactAnalysis/*` (pre-PLN-726) | `templates/impact_semantic_change.md` |
+| `Correctness/pinned-file-pair` | `templates/correctness_pinned_file_pair.md` |
 | `CompanionChange/*` | `templates/companion_change.md` |
 | `Coverage/*` | `templates/coverage_gap.md` |
 | `InjectionAttempt/*` | `templates/injection_attempt.md` |

plugins/code-review/skills/fix/templates/correctness_pinned_file_pair.md

@@ -0,0 +1,23 @@
+### ⚠️  Correctness / pinned-file-pair — `{file}:{line}`  ({severity})
+
+**Issue:** {issue}
+
+**Changed file (finding anchor):** `{file}:{line}`
+```
+{code_snippet}
+```
+
+**Companion assertion(s) this change contradicts (other_locations, one `file:line — issue` per entry):**
+{other_locations_rendered}
+
+**Explanation:** {explanation}
+
+**Why this is surfaced manually:** This defect spans two files and the fix almost never belongs at the anchor. Diff-scope validation requires the finding to anchor on the changed file, and auto-fix edits that anchor — so applying it would revert the intended config, workflow, or manifest change instead of updating the stale assertion that pins it. Which side is wrong is the operator's call: the change may be correct and the assertion stale, or the assertion may be the contract and the change a mistake. The companion file is also typically outside this review's diff, so `/fix` has not verified its current content.
+
+**Your options:**
+1. **Update the companion assertion** at the `other_locations[]` file:line above to match the changed file, if the change is intended.
+2. **Revert or correct the change** at the anchor, if the pinned assertion is the contract being violated.
+3. **Dismiss** if the companion test does not actually pin these lines — a test that only names the changed path as fixture input pins nothing, and this finding should not have been emitted.
+4. **Re-assert** via `python3 <plugin>/tools/python/code_review_helpers.py re-assert --cr-dir <CR_DIR> --cache-dir <CACHE_DIR> --finding-ids {id}`. (Resolve `<CACHE_DIR>` from `<CR_DIR>/cache_config.json:cache_dir`.)
+
+**Original recommendation:** {recommendation}

plugins/code-review/skills/spawn-reviewers/SKILL.md

@@ -189,6 +189,54 @@ Focus areas:
 
 For DRY claims, one concrete example of prior art is sufficient (cite file path + function name).
 
+NON-APPLICATION FILES — the cross-file half. Config, CI, migration, docs, and
+test files in the diff are reviewable work product on the same evidence standard
+as source. The classes below are yours BECAUSE each one needs a file outside the
+diff; the shapes decidable from the changed file alone belong to Bug Hunter A and
+you should not re-review them.
+
+- PINNED-FILE PAIR BROKEN. Repos commonly hold meta-tests that assert another
+  file's literal content — a workflow's pinned action version, a config
+  snapshot, a required-context manifest, an allowlist. When the diff edits such
+  a file, grep for a test that pins the changed lines.
+  EVIDENCE BAR — a path reference is NOT evidence. You must quote the specific
+  assertion, at its own file:line, whose expected value the pinned-head file no
+  longer satisfies: a literal comparison, an exact-match membership check, a
+  snapshot equality. A test that merely names the changed path — as a fixture
+  input, a synthetic `files=[...]` list, a glob it iterates, a docstring — pins
+  nothing and is not a finding. If you cannot name the contradicted expected
+  value and the new value beside it, you do not have this finding.
+  DO NOT infer the defect from the companion test's absence from the diff. You
+  are not given full-PR diff membership (a partitioned Bug Hunter A worker sees
+  only its own patch, and your `<files_assigned>` is a scope, not a manifest of
+  every file the PR touches), so "the test is not in the diff" is unknowable and
+  is never part of the argument. The contradiction between the assertion and the
+  file as it now stands is the whole case; read the companion test at HEAD and
+  check whether it was already updated before you report.
+  EMIT as category "Correctness" with subcategory "pinned-file-pair" — the
+  subcategory is required, because it is what routes the finding to manual
+  handling instead of an auto-edit at the changed file. Anchor `file`/`line` on
+  the edited line as usual, and put the companion assertion in
+  `other_locations[]` as `{file, line, issue}` with the contradicted expected
+  value in `issue`. It applies in both directions — loosening the assertion
+  without updating the file it pins is the same defect, reported the same way.
+
+- WIRING UNPROVEN (category "TestQuality", subcategory "missing-coverage"). A
+  new helper, reducer, or classifier gets direct tests while nothing asserts
+  that its production caller invokes it — the route handler, the event/action
+  set, the IPC or transport boundary. Deleting the call site would leave the
+  suite green. Cite the production caller you read, at file:line, and say what
+  no test drives.
+
+- DOCS, READMEs, AND API SPECS (category "Documentation"), each held against the
+  code you read:
+  - A claim stronger than the code enforces: unconditional where the code is
+    conditional, product-wide where the behavior is one lane, or collapsing
+    layers the implementation deliberately keeps separate. Cite both sides.
+  - A behavior change in this diff whose describing document is not updated.
+  - A generated-client spec admitting field combinations the runtime always
+    rejects, or omitting inputs the runtime accepts.
+
 CODE INTELLIGENCE (optional): CODE_INTEL_ALLOWED=<CODE_INTEL_ALLOWED>,
 CODE_INTEL_REQUIRE_ROOT_ARG=<CODE_INTEL_REQUIRE_ROOT_ARG>. Follow the
 "OPTIONAL — CODE INTELLIGENCE" protocol in {CR_DIR}/shared_prompt.txt: inspect your own
@@ -422,6 +470,54 @@ Focus areas:
 
 For DRY claims, one concrete example of prior art is sufficient (cite file path + function name).
 
+NON-APPLICATION FILES — the cross-file half. Config, CI, migration, docs, and
+test files in the diff are reviewable work product on the same evidence standard
+as source. The classes below are yours BECAUSE each one needs a file outside the
+diff; the shapes decidable from the changed file alone belong to Bug Hunter A and
+you should not re-review them.
+
+- PINNED-FILE PAIR BROKEN. Repos commonly hold meta-tests that assert another
+  file's literal content — a workflow's pinned action version, a config
+  snapshot, a required-context manifest, an allowlist. When the diff edits such
+  a file, grep for a test that pins the changed lines.
+  EVIDENCE BAR — a path reference is NOT evidence. You must quote the specific
+  assertion, at its own file:line, whose expected value the pinned-head file no
+  longer satisfies: a literal comparison, an exact-match membership check, a
+  snapshot equality. A test that merely names the changed path — as a fixture
+  input, a synthetic `files=[...]` list, a glob it iterates, a docstring — pins
+  nothing and is not a finding. If you cannot name the contradicted expected
+  value and the new value beside it, you do not have this finding.
+  DO NOT infer the defect from the companion test's absence from the diff. You
+  are not given full-PR diff membership (a partitioned Bug Hunter A worker sees
+  only its own patch, and your `<files_assigned>` is a scope, not a manifest of
+  every file the PR touches), so "the test is not in the diff" is unknowable and
+  is never part of the argument. The contradiction between the assertion and the
+  file as it now stands is the whole case; read the companion test at HEAD and
+  check whether it was already updated before you report.
+  EMIT as category "Correctness" with subcategory "pinned-file-pair" — the
+  subcategory is required, because it is what routes the finding to manual
+  handling instead of an auto-edit at the changed file. Anchor `file`/`line` on
+  the edited line as usual, and put the companion assertion in
+  `other_locations[]` as `{file, line, issue}` with the contradicted expected
+  value in `issue`. It applies in both directions — loosening the assertion
+  without updating the file it pins is the same defect, reported the same way.
+
+- WIRING UNPROVEN (category "TestQuality", subcategory "missing-coverage"). A
+  new helper, reducer, or classifier gets direct tests while nothing asserts
+  that its production caller invokes it — the route handler, the event/action
+  set, the IPC or transport boundary. Deleting the call site would leave the
+  suite green. Cite the production caller you read, at file:line, and say what
+  no test drives.
+
+- DOCS, READMEs, AND API SPECS (category "Documentation"), each held against the
+  code you read:
+  - A claim stronger than the code enforces: unconditional where the code is
+    conditional, product-wide where the behavior is one lane, or collapsing
+    layers the implementation deliberately keeps separate. Cite both sides.
+  - A behavior change in this diff whose describing document is not updated.
+  - A generated-client spec admitting field combinations the runtime always
+    rejects, or omitting inputs the runtime accepts.
+
 CODE INTELLIGENCE (optional): CODE_INTEL_ALLOWED=<CODE_INTEL_ALLOWED>,
 CODE_INTEL_REQUIRE_ROOT_ARG=<CODE_INTEL_REQUIRE_ROOT_ARG>. Follow the
 "OPTIONAL — CODE INTELLIGENCE" protocol in {CR_DIR}/shared_prompt.txt — when

plugins/code-review/tools/prompts/bha_suffix.txt

@@ -21,4 +21,72 @@ Apply this certificate and BUG CONFIRMED emission gate only when acting as Bug H
 Only emit findings where CONCLUSION = BUG CONFIRMED with a completed trace.
 Discard immediately if DIVERGENCE = "NONE FOUND" or GUARD CHECK reveals the issue is already handled.
 
+
+NON-APPLICATION FILES IN YOUR PARTITION:
+Config, CI, infrastructure, migration, and test files are yours to review on the
+same terms as application source — same certificate, same file:line, same BUG
+CONFIRMED gate. The classes below are the ones decidable from the changed file
+itself, which is why they are yours and not Bug Hunter B's. Anything needing a
+file outside the diff is B's; do not reach for it.
+
+CI / PIPELINE DEFINITIONS (workflow YAML, pipeline configs, composite actions):
+- A privileged job (write-scoped token or secrets) that runs repository
+  lifecycle scripts — dependency install, postinstall/prepare hooks, build —
+  BEFORE minting or consuming that credential. Lifecycle code from the ref under
+  test can persist PATH/env changes, shell hooks, or a background process and
+  intercept later tool invocations once the credential is present.
+- Write permissions or persisted checkout credentials scoped to the whole
+  workflow when only one step needs them.
+- A manual-dispatch path exposing secrets or write tokens with no trusted-ref
+  gate. Checking out a trusted ref INSIDE the job does not change what the
+  dispatch already exposed.
+- A job declaring dependencies plus a condition with no status-check function:
+  the implicit "all dependencies succeeded" is FALSE for a skipped dependency,
+  so a skippable dependency silently skips the dependent job.
+- A job with no explicit timeout — it inherits a multi-hour platform default, so
+  a runner that wedges (stops progressing rather than failing) holds its
+  concurrency group for that whole window.
+- Cancel-on-new-run applied to a job with irreversible side effects (publishing,
+  releasing, deploying an immutable artifact).
+- A path/trigger filter that does not actually match files it is meant to gate.
+- A shell step where an unguarded command can terminate the step before the
+  branch that owns its exit code, or before its telemetry/status emission.
+
+MIGRATIONS / SCHEMA:
+- A destructive statement (dedupe, drop, backfill) selecting survivors by a
+  proxy for canonicity and cascading the rest.
+- An idempotency guard that no-ops over an invalid or partial object left by a
+  previous failed run, letting the migration record as successfully applied.
+- A new enum/status/discriminator value added without updating every classifier
+  that switches on it, so stored rows follow the wrong lifecycle.
+
+GUARD AND META-TEST SCRIPTS (lint rules, source tests, policy checks) — the
+shapes visible in the guard itself:
+- A guard asserting a predicate APPEARS in a file rather than executing the
+  decision. A separate permissive branch, or a short-circuit ahead of the
+  compound condition, keeps every such assertion green while widening behavior.
+- A guard anchored to the first matching block rather than the specific
+  production node, so an unused or disconnected copy satisfies it.
+- An allowlist or known-exceptions entry broad enough to suppress future real
+  violations rather than only the instance it was added for.
+(The paired defect — a meta-test whose pinned assertion this diff contradicts —
+needs a file outside your patch and belongs to Bug Hunter B.)
+
+TEST FILES IN YOUR PARTITION — two shapes readable in the test itself. Emit as
+category "TestQuality" with the existing subcategory named:
+- ASSERTION OF EXISTENCE, NOT BEHAVIOR (subcategory "weak-assertion"). A test
+  that greps, regex-matches, or AST-inspects source to confirm a predicate is
+  present, instead of executing that decision against synthetic inputs. It
+  cannot distinguish a correct implementation from a widened one.
+- INPUT-INVARIANT FIXTURE (subcategory "mock-faithfulness"). A mock, stub, or
+  route handler returning the same result regardless of the arguments, query
+  params, or filters the test claims to cover. The test then proves only that
+  the UI changed, not that the request or predicate did. Look for a handler
+  ignoring its request argument while the test name references filtering,
+  sorting, or pagination.
+Also flag a test asserting only a count or a container class where the state
+mapping under test (per-item variant, icon, tone) could be uniformly wrong and
+still pass. The third shape — a helper tested while nothing asserts its
+production caller runs it — needs the caller, so it is Bug Hunter B's.
+
 Use Read, Grep, and Glob for codebase context. Do NOT use Bash.