feat(code-review): code-intelligence capabilities C5–C7 and broker discovery

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.10.1
+
+#### Added
+- The optional code-intelligence protocol in `shared_prompt.txt` names three more capabilities a reviewer may use when its session holds a tool that answers them: C5 change impact (maps a diff to the symbols it changed and the call sites they reach), C6 index coverage (whether the index is current, whether given paths are in it, and whether an answer was cut short or degraded), and C7 duplication (near-duplicate symbols). C5 rows and C7 hits are candidates, never evidence: a C5 call is kept to one caller hop, a row limit of 25 or less, and only the assigned files whose exported surface changed, since the response grows with every file named whatever the limit, and it uses the review's diff base; both sides of a C7 hit are read before reporting. A C7 error or unbuilt duplicate data means the capability is unavailable, not "no duplicates".
+- `spawn-reviewers`, `impact_analyzer_prompt.txt`, and `design_critic_suffix.txt` say which role reaches for which capability. The Impact Analyzer seeds its candidate-symbol list with one C5 call that cross-checks, and never replaces, its own walk of the diff; C5 call sites enter the existing `discovery: "grep"` / `discovery: "graph"` flow and still need a `callsite_snippet` read from the file. Every role reads the C5 diff base from `base_ref` in `scope.json` and skips C5 when that value is absent, empty, or begins with `-`. The Design Critic consults C7 for each new module or class.
+- Reviewers may discover further read-only tools through a broker tool that searches for and calls tools by name: one search per unanswered capability, only tools whose purpose is to query, and never one that scans, executes, writes, indexes, or records. `code-review-worker-graph.md` holds a brokered tool to the same read-only rule as one called directly, and the Design Critic makes that one search for C4 before falling back to grepping imports.
+
+#### Changed
+- An empty answer from a code-intelligence tool no longer counts as evidence of absence. `shared_prompt.txt` drops "an empty result" from the conditions that degrade silently to Grep/Glob and adds a rule for any finding or dismissal that rests on absence ("no callers", "unused", "dead code", "no other implementation"): first establish through C6 that the paths the claim spans are indexed and current and that the response was not cut short; otherwise confirm with Grep across `<review_root>` or do not assert it. A clean coverage result means "no recorded gap", not proof of completeness. The Impact Analyzer's zero-usages conclusion and the Design Critic's absence-based design findings point at this rule.
+- No finding may claim a caller list is complete ("all callers", "only N call sites"); reviewers report what they found.
+- Editing `shared_prompt.txt` changes the canonical prompt hash, so the Bug Hunter A cache and the `verifications/` cache are invalidated once on upgrade. The change is prompt and agent wording only: no helper, schema, hook, or orchestrator change.
+
 ### 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.10.1",
   "author": {
     "name": "ClosedLoop",
     "email": "support@closedloop.ai"

plugins/code-review/agents/code-review-worker-graph.md

@@ -58,5 +58,6 @@ argument shape.
 
 Do NOT use Bash — everything you need is reachable with Read, Grep, and Glob.
 That applies equally to any inherited MCP tool that runs shell commands or edits
-files: a reviewer reads and reports, it never executes or mutates. All findings
+files: a reviewer reads and reports, it never executes or mutates. A tool reached
+through a broker is held to the same rule as one called directly. All findings
 are written with Write exactly as the generic worker does.

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

@@ -51,7 +51,7 @@ Context-heavy operations that cause "Prompt is too long" failures:
 **Agent type (CRITICAL — prevents context overflow AND permission issues):** every agent spawned by this command MUST use one of the two code-review worker types in the Task tool call — never `general-purpose` (background agents with that type inherit only the session's `permissions.allow` list, which often lacks bare Read/Write/Grep/Glob, causing silent permission denials) and never an omitted `subagent_type` (Claude Code then auto-selects an unrelated agent whose larger system prompt bloats context). The two types:
 
 - **`code-review:code-review-worker`** (default; `tools: Read, Write, Grep, Glob`) — use for EVERY reviewer EXCEPT the four code-intelligence-aware roles below. This includes Bug Hunter A, Unified Auditor, Domain Critics, the **verifier fleet** (stage_23), and the **PLN-725 singletons** (stage_11 / stage_15). Its explicit allowlist is what keeps these roles at exactly four tools — they inherit NOTHING from the session, keeping the trust boundary tight for the adversarial verifier and the singleton prompts that never load the code-intelligence protocol.
-- **`code-review:code-review-worker-graph`** (no `tools:` allowlist — inherits the session's tools, minus a `disallowedTools` denylist for Bash/Edit/NotebookEdit) — use ONLY for the code-intelligence-aware roles: **Bug Hunter B**, the **Impact Analyzer**, the **Design Critic**, and the **Fast Path** reviewer (which runs a BHB pass). These are the only roles whose prompts load the "OPTIONAL — CODE INTELLIGENCE" protocol. (BHB / Impact / fast-path use the cross-file capabilities C1–C3; the Design Critic also uses the structural capability C4.)
+- **`code-review:code-review-worker-graph`** (no `tools:` allowlist — inherits the session's tools, minus a `disallowedTools` denylist for Bash/Edit/NotebookEdit) — use ONLY for the code-intelligence-aware roles: **Bug Hunter B**, the **Impact Analyzer**, the **Design Critic**, and the **Fast Path** reviewer (which runs a BHB pass). These are the only roles whose prompts load the "OPTIONAL — CODE INTELLIGENCE" protocol. (Each role's suffix below names the capabilities it reaches for; the shared protocol defines them.)
 
 The two differ in what they can rely on, and the prompts account for it. `code-review-worker`'s allowlist *guarantees* the core four regardless of what the spawning session holds. The inheriting worker gets whatever that session has — which is usually the core four plus the session's MCP servers, but is NOT guaranteed: a session that supplies its own search tooling instead of `Grep`/`Glob` yields a reviewer without them. That is why the shared prompt states text search as a capability rather than a tool name and tells the reviewer to fall back to targeted `Read` calls, and why `grep_query_used` must describe a query actually executed. `Write` is inherited in practice, and the write-denied fallback in `shared_prompt.txt` (emit `<findings_json>` inline, report `file=WRITE_DENIED`) still covers the case where it is refused.
 
@@ -195,8 +195,12 @@ CODE_INTEL_REQUIRE_ROOT_ARG=<CODE_INTEL_REQUIRE_ROOT_ARG>. Follow the
 tool roster for an MCP server that indexes this repo, loading deferred schemas with
 ToolSearch first. When one is available, prefer it for your cross-file work — capability
 C3 (snippet read) to read the exact service/API implementation instead of Glob-guessing
-its file, C1/C2 (symbol lookup, usage enumeration) for DRY/duplicate lookups and import
-validation. Pass <review_root> as the root argument whenever a tool accepts one; when
+its file, C7 (duplication) first and then C1/C2 (symbol lookup, usage enumeration) for
+DRY/duplicate lookups, C1/C2 for import validation, and C5 (change impact), scoped to your
+assigned files, for what else this change reaches — pass `base_ref` Read from
+{CR_DIR}/scope.json, and skip C5 when that value is absent, empty, or begins with `-`.
+Any claim resting on absence ("unused",
+"no callers", "no existing helper") follows that protocol's empty-result rule. Pass <review_root> as the root argument whenever a tool accepts one; when
 CODE_INTEL_REQUIRE_ROOT_ARG is true, call only tools you can scope that way. Discard any
 answer for a different symbol than you asked about, and validate returned paths resolve
 under <review_root>. When CODE_INTEL_ALLOWED is false or nothing you may call answers the
@@ -292,7 +296,11 @@ deferred schemas with ToolSearch first) and ALSO use its capability C2 (usage/ca
 enumeration) to reach callers grep cannot (aliases,
 re-exports, dynamic dispatch); tag those entries `discovery: "graph"` and put
 them in the certificate's `graph_discovered_usages` per the Inputs/Step 2
-sections of impact_analyzer_prompt.txt. Run your text-search tool too whenever you
+sections of impact_analyzer_prompt.txt. If you also hold C5 (change impact), seed
+with ONE call scoped to your assigned files, passing `base_ref` Read from
+{CR_DIR}/scope.json — skip C5 when that value is absent, empty, or begins with `-`.
+A zero-usages conclusion is an absence claim: the protocol's empty-result rule
+governs it. Run your text-search tool too whenever you
 hold one, and record the real query you ran in
 `grep_query_used` for the `discovery: "grep"` entries (the verifier replays it
 against `external_usages_found`). If you hold NO text-search tool at all, leave
@@ -335,10 +343,12 @@ When CODE_INTEL_ALLOWED is true, inspect your own tool roster for an MCP server
 indexing this repo (ToolSearch for deferred schemas) and prefer it for structure and
 dependency-direction analysis — capability C4 (module layout, dependency edges,
 cycles, implementors; some servers expose this as a query language over the
-dependency graph) and C2 (call / data-flow chains). Pass <review_root> as the root
+dependency graph) and C2 (call / data-flow chains), plus C7 (duplication) for each new
+module/class; absence-based design claims follow the protocol's empty-result rule. Pass <review_root> as the root
 argument whenever a tool accepts one; when CODE_INTEL_REQUIRE_ROOT_ARG is true, call
 only tools you can scope that way. Discard any answer for a different symbol than you
 asked about, and validate returned paths resolve under <review_root>.
+If nothing visible answers C4, make the protocol's one broker search for it before falling back.
 When CODE_INTEL_ALLOWED is false or nothing you may call answers C4, grep imports instead.
 ```
 
@@ -426,8 +436,11 @@ 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
 CODE_INTEL_ALLOWED is true, inspect your own tool roster for an MCP server indexing this
-repo (ToolSearch for deferred schemas) and prefer its C1/C2/C3 capabilities for the
-cross-file lookups above; pass <review_root> as the root argument whenever a tool accepts
+repo (ToolSearch for deferred schemas) and prefer its C1/C2/C3 capabilities, plus C5
+(change impact — pass `base_ref` Read from {CR_DIR}/scope.json, and skip C5 when that
+value is absent, empty, or begins with `-`) and C7 (duplication), for the cross-file
+lookups above; any claim resting
+on absence follows that protocol's empty-result rule; pass <review_root> as the root argument whenever a tool accepts
 one (when CODE_INTEL_REQUIRE_ROOT_ARG is true, call only tools you can scope that way),
 discard any answer for a different symbol than you asked about, and validate returned
 paths resolve under <review_root>; otherwise use Grep/Glob silently.

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

@@ -17,21 +17,21 @@ Ousterhout defines complexity as anything about a system's structure that makes
 What new files/directories were created? What interfaces/APIs were added or changed? What new dependencies between modules does it introduce? What abstractions are added or modified?
 
 ### Step 2 — Audit project structure (when the diff adds files/dirs)
-- **Tooling:** Follow the OPTIONAL — CODE INTELLIGENCE protocol in `shared_prompt.txt`. If your roster holds a tool answering capability C4 (structure: module/layer layout), use it to read the actual layout instead of inferring it from Glob — it is the precise substrate for this step. Fall back to Glob when it is unavailable.
+- **Tooling:** Follow the OPTIONAL — CODE INTELLIGENCE protocol in `shared_prompt.txt`. If your roster holds a tool answering capability C4 (structure: module/layer layout), use it to read the actual layout instead of inferring it from Glob — it is the precise substrate for this step. If nothing visible answers C4, make the protocol's one broker search for it first. Fall back to Glob when it is unavailable.
 - **Screaming Architecture:** Do top-level folders reveal the business domain (`orders/`, `billing/`, `patients/`) or the framework (`controllers/`, `services/`, `models/`, `repositories/`)? Flag new structure that organizes by technical layer rather than domain.
 - **Package cohesion (CRP):** Flag new files added to dumping-ground packages — `utils`, `common`, `helpers`, `misc`, `shared`, `core` — that force callers to depend on unrelated things.
 - **Co-change (CCP):** If one logical change is smeared across 4+ directories, files that change together don't live together.
 
 ### Step 3 — Audit the dependency graph
-A code-intelligence substrate is the precise substrate for this step when your session has one: capability C2 (usage/caller enumeration) surfaces real import/call edges including the aliases and re-exports grep misses, and capability C4 (structure) answers dependency-direction and import-cycle questions directly — some servers expose C4 as a query language over the dependency graph, which is the sharpest form of it. Discover what you hold per the shared protocol; grep imports of the changed files when nothing does. Either way, every finding still cites a concrete file:line.
+A code-intelligence substrate is the precise substrate for this step when your session has one: capability C2 (usage/caller enumeration) surfaces real import/call edges including the aliases and re-exports grep misses, and capability C4 (structure) answers dependency-direction and import-cycle questions directly — some servers expose C4 as a query language over the dependency graph, which is the sharpest form of it. Discover what you hold per the shared protocol — including its one broker search when nothing visible answers C4; grep imports of the changed files when nothing does. Either way, every finding still cites a concrete file:line.
 - **Dependency Rule:** Inner layers (domain, entities, use-cases) must not import outer layers (frameworks, infra, HTTP, DB). Framework/infra import signals: `express`, `flask`, `django`, `fastapi`, `spring`, `prisma`, `sqlalchemy`, `typeorm`, `mongoose`, `boto3`, HTTP clients, DB drivers, cloud SDKs, ORM annotations (`@Entity`, `@Column`, `@Table`, `__tablename__`), HTTP types (`Request`, `Response`). Flag any such import added to a domain/use-case file.
 - **Boundary data-format leak:** Flag an inner-layer function that accepts/returns an ORM model instance, DB row/`QuerySet`, HTTP `Request`/`Response`, or framework DTO — across a boundary the payload should be a plain struct shaped for the inner layer.
 - **Abstraction ownership (DIP):** The interface should be defined by the CONSUMER (inner layer), not the provider. If an interface lives beside its single implementation in `infrastructure/` and is imported inward, ownership is inverted — it belongs in `domain/` or `application/ports/`.
 - **Concrete dependency in policy:** A `new ConcreteClass()` or direct import of a concrete infra class inside business logic is a DIP violation.
 - **Cycles (ADP):** A↔B import cycle (directly or via a chain). Import-cycle workarounds in the diff are a tell — a function-local/deferred import or `TYPE_CHECKING`-only import added to dodge a circular import.
 
 ### Step 4 — Evaluate module depth (for each new module/class/interface)
-A **deep** module has a simple interface over a powerful implementation; a **shallow** one has an interface nearly as complex as its implementation. Flag:
+A **deep** module has a simple interface over a powerful implementation; a **shallow** one has an interface nearly as complex as its implementation. For each new module/class, consult capability C7 (duplication) when you hold it, per the shared protocol: a hit is a candidate for "wrapper adds no abstraction" or information leakage — read both sides before reporting. Flag:
 - A new class/wrapper that adds no abstraction over what it wraps
 - A constructor taking 6+ params assigned straight to fields with no logic
 - A pass-through method (body is a single delegating call with a similar signature)
@@ -68,6 +68,8 @@ CAUSATION: Which part of THIS diff introduced or worsened it? (If it's pre-exist
 COST: Which symptom does it impose — change amplification, cognitive load, or unknown unknowns?
 CONCLUSION: DESIGN FLAW CONFIRMED (with the corrective refactor named) or DISCARDED (with why it's acceptable).
 
+A design finding that rests on absence — an unused interface, a single-implementation abstraction, a dead module — is governed by the shared protocol's AN EMPTY RESULT IS NOT A NEGATIVE RESULT rule: an empty substrate answer with unverified index coverage proves nothing, so confirm with Grep across `<review_root>` or do not assert it.
+
 Emit only findings where CONCLUSION = DESIGN FLAW CONFIRMED. Do not report style preferences, hypothetical future issues, or design debt outside the diff's blast radius.
 
 ## Severity (use the shared tiers; design findings are mostly MEDIUM/HIGH)
@@ -86,4 +88,4 @@ SOLID (Martin): SRP (one actor), OCP (extension without modification), LSP (beha
 
 Clean Architecture (Martin): Dependency Rule (source dependencies point only inward), Screaming Architecture (structure reveals domain not framework), Humble Object (split testable behavior from the hard-to-test shell), REP/CCP/CRP (component cohesion), ADP/SDP/SAP (component coupling — no cycles; depend toward stability; stable components are abstract).
 
-Use Read, Grep, and Glob for codebase context — plus whatever code-intelligence MCP tools your session provides, per the OPTIONAL — CODE INTELLIGENCE protocol in `shared_prompt.txt` (capabilities C1–C4; load deferred tool schemas with `ToolSearch` first). Scope every call to `<review_root>` and validate returned paths resolve under it, as that protocol requires. Do NOT use Bash.
+Use Read, Grep, and Glob for codebase context — plus whatever code-intelligence MCP tools your session provides, per the OPTIONAL — CODE INTELLIGENCE protocol in `shared_prompt.txt` (capabilities C1–C7; load deferred tool schemas with `ToolSearch` first). Scope every call to `<review_root>` and validate returned paths resolve under it, as that protocol requires. Do NOT use Bash.