From fa84341d160b6aba51ef98bcc7bc2e6d346a6b3d Mon Sep 17 00:00:00 2001 From: Kris Wong Date: Mon, 21 Sep 2026 13:03:16 -0500 Subject: [PATCH 1/6] feat(code-review): add change-impact, index-coverage and duplication capabilities PLN-2082 S1.1-S1.5, S2, S3.2-S3.4, S4 (non-broker clauses), S6. Co-Authored-By: Claude --- .../code-review/.claude-plugin/plugin.json | 2 +- .../skills/spawn-reviewers/SKILL.md | 20 +++++++---- .../tools/prompts/design_critic_suffix.txt | 6 ++-- .../tools/prompts/impact_analyzer_prompt.txt | 24 ++++++++++++++ .../tools/prompts/shared_prompt.txt | 33 ++++++++++++++++--- 5 files changed, 72 insertions(+), 13 deletions(-) diff --git a/plugins/code-review/.claude-plugin/plugin.json b/plugins/code-review/.claude-plugin/plugin.json index a2c3e3b..2ae5c6a 100644 --- a/plugins/code-review/.claude-plugin/plugin.json +++ b/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" diff --git a/plugins/code-review/skills/spawn-reviewers/SKILL.md b/plugins/code-review/skills/spawn-reviewers/SKILL.md index 8a6b58e..6300c23 100644 --- a/plugins/code-review/skills/spawn-reviewers/SKILL.md +++ b/plugins/code-review/skills/spawn-reviewers/SKILL.md @@ -195,8 +195,10 @@ 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 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. Any claim resting on absence ("unused", +"no callers", "no existing helper") follows that protocol's empty-result rule. Pass 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 . When CODE_INTEL_ALLOWED is false or nothing you may call answers the @@ -292,7 +294,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,7 +341,8 @@ 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 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 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 . @@ -426,8 +433,9 @@ CODE INTELLIGENCE (optional): CODE_INTEL_ALLOWED=, 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 as the root argument whenever a tool accepts +repo (ToolSearch for deferred schemas) and prefer its C1/C2/C3 capabilities, plus C5 +(change impact) and C7 (duplication), for the cross-file lookups above; any claim resting +on absence follows that protocol's empty-result rule; pass 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 ; otherwise use Grep/Glob silently. diff --git a/plugins/code-review/tools/prompts/design_critic_suffix.txt b/plugins/code-review/tools/prompts/design_critic_suffix.txt index 5b8d14e..5d0c46e 100644 --- a/plugins/code-review/tools/prompts/design_critic_suffix.txt +++ b/plugins/code-review/tools/prompts/design_critic_suffix.txt @@ -31,7 +31,7 @@ A code-intelligence substrate is the precise substrate for this step when your s - **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 `` 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 `` 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 `` and validate returned paths resolve under it, as that protocol requires. Do NOT use Bash. diff --git a/plugins/code-review/tools/prompts/impact_analyzer_prompt.txt b/plugins/code-review/tools/prompts/impact_analyzer_prompt.txt index 5aed0e3..336e2fa 100644 --- a/plugins/code-review/tools/prompts/impact_analyzer_prompt.txt +++ b/plugins/code-review/tools/prompts/impact_analyzer_prompt.txt @@ -100,6 +100,11 @@ Two discovery substrates, used together: `discovery: "graph"` usage — exactly the blast radius grep misses. Validate every returned path is inside this checkout before citing it (per the shared protocol). + When you also hold C5 (change impact), make it your first move: ONE + call, scoped to your assigned files with a row limit, to seed the + candidate-symbol list and its call sites. Diff base: Read `base_ref` + from `{CR_DIR}/scope.json` and pass it. If it is absent, empty, or + begins with `-`, skip C5. If the tool cannot take a base, skip C5. Do NOT use Bash. Respect repository `.gitignore` (Glob already does). @@ -151,6 +156,16 @@ Ignore symbols that are clearly **internal** (private/unexported, underscored, inside `__init__` blocks not in `__all__`, etc.). Ignore **internal body** changes when the signature is unchanged. +If you made a C5 call, its changed-symbol list **cross-checks** this +walk; it never replaces it. A symbol C5 reports that your walk missed +gets examined; a symbol your walk found that C5 lacks stays. The +external-contract filter above still applies: C5 reports body-only +changes too, and those are still ignored. When C5 offers them, take +deleted symbols that still have callers first, then exported symbols +with many callers. Risk tiers and test-caller counts are inputs to your +judgement, never severity; mention a zero test-caller count only after +independently confirming no test covers the symbol. + ### Step 2: Find external usages For each candidate symbol: @@ -169,6 +184,11 @@ For each candidate symbol: certificate's `graph_discovered_usages`, NOT in `external_usages_found` (keeping the grep-replay set clean). Read each one's source line into `callsite_snippet` (verbatim) like any other entry. + - **C5 call sites enter this same flow** as C2 callers, for the symbol + they belong to: grep-reachable → `discovery: "grep"`; otherwise + `discovery: "graph"` into `graph_discovered_usages`. Every entry + still carries a verbatim `callsite_snippet` Read from the file — a + substrate-supplied line number is a place to look, not a snippet. - Construct a Grep query that finds usages **outside the diff**. Prefer unambiguous identifiers (`FooBar`, `parseCacheKey`) over common words. For ambiguous names (`get`, `update`), narrow with @@ -242,6 +262,10 @@ If neither Grep nor the graph returns any external usage, **DO NOT EMIT A FINDING**. A symbol with no external callers cannot have cross-file impact. Log the symbol as analyzed (so cost-cap deferred-symbol accounting is correct) but emit nothing. +That is an absence conclusion, so the shared protocol's AN EMPTY RESULT +IS NOT A NEGATIVE RESULT rule governs it: when the substrate came back +empty and index coverage is unverified, the substrate proved nothing — +the grep query you ran is the evidence of record. If every external usage is guarded, **DO NOT EMIT A FINDING**. List the guarded usages in the certificate so the verifier can audit, but diff --git a/plugins/code-review/tools/prompts/shared_prompt.txt b/plugins/code-review/tools/prompts/shared_prompt.txt index 8f26b1b..406116c 100644 --- a/plugins/code-review/tools/prompts/shared_prompt.txt +++ b/plugins/code-review/tools/prompts/shared_prompt.txt @@ -78,6 +78,21 @@ particular one. Bind to what you actually hold: C3 SNIPPET READ — read a symbol's source without knowing its file. C4 STRUCTURE — module/layer layout, dependency direction, import cycles. For design/architecture-level review. + C5 CHANGE IMPACT — maps a diff to the symbols it changed and the call sites + they reach. A seed for cross-file work; every row is a candidate. + C6 INDEX COVERAGE — says whether the index is current, whether given paths + are in it, and whether an answer was cut short or degraded. May be a tool, + or fields on another tool's response. + C7 DUPLICATION — reports near-duplicate symbols. +- C5 CALL DISCIPLINE. Scope the call to your assigned files (split a large + assignment — a row limit may not bound the whole response) and pass a row + limit anyway. Use the review's diff base. Keep only rows tied to a symbol you + are analysing. Treat dunder and generic names (`__init__`, `get`, `open`) as + unverified until a C2 call or a Read confirms the target. +- C7 HITS ARE CANDIDATES. Read both sides before reporting. A tool error, or + duplicate data that has not been built, means C7 is unavailable, never "no + duplicates" — and do not build it (NO INDEXING CALLS). It matches + copy-with-renaming, not semantic equivalence. - KILL SWITCH. Your task prompt carries `CODE_INTEL_ALLOWED`. When it is `false`, use Grep/Glob only, no matter what tools you hold — the orchestrator has determined no external index can be trusted for this run (the source @@ -108,10 +123,20 @@ particular one. Bind to what you actually hold: Before citing any file a tool returns, confirm it is openable with Read at `/`. Discard (never cite) any path that does not resolve under or that escapes it via `..`. -- DEGRADE SILENTLY. A missing tool, an error, an empty result, or a stale index - means fall back to Grep/Glob and change nothing else. Do NOT mention the - substrate's presence or absence in findings, and do NOT change what you report - because of it. +- DEGRADE SILENTLY. A missing tool, an error, or a stale index means fall back + to Grep/Glob and change nothing else. Do NOT mention the substrate's presence + or absence in findings, and do NOT change what you report because of it. +- AN EMPTY RESULT IS NOT A NEGATIVE RESULT. Before any finding OR dismissal that + rests on absence — "no callers", "unused", "not referenced", "dead code", + "safe to delete", "no other implementation" — establish via C6 that the paths + the claim spans are indexed and current and that the response was not cut + short or degraded. Anything else — stale, missing, excluded, unparsed, + unknown, a freshness answer that does not speak to those paths, or no C6 held + — means the index proves nothing about that path: confirm with Grep across + , or do not assert it. + A clean coverage result means "no recorded gap", never proof of completeness. +- NO COMPLETENESS CLAIMS. No substrate field licenses saying a list of callers + is complete ("all callers", "only N call sites"); report what you found. - NEVER AN EVIDENCE SUBSTITUTE. Every finding still cites a concrete file:line you confirmed by reading it, and any field a role prompt requires for verifier replay (e.g. `grep_query_used`) is still mandatory. From 3e2b26dd9b139c676896851e5c685b7f4189575b Mon Sep 17 00:00:00 2001 From: Kris Wong Date: Mon, 21 Sep 2026 13:03:32 -0500 Subject: [PATCH 2/6] feat(code-review): let reviewers discover brokered read-only tools PLN-2082 S1.6, S3.1, S4 broker clause, S5. Revertable alone. Co-Authored-By: Claude --- plugins/code-review/agents/code-review-worker-graph.md | 3 ++- plugins/code-review/skills/spawn-reviewers/SKILL.md | 1 + plugins/code-review/tools/prompts/design_critic_suffix.txt | 4 ++-- plugins/code-review/tools/prompts/shared_prompt.txt | 5 +++++ 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/plugins/code-review/agents/code-review-worker-graph.md b/plugins/code-review/agents/code-review-worker-graph.md index 3fb5833..4654d71 100644 --- a/plugins/code-review/agents/code-review-worker-graph.md +++ b/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. diff --git a/plugins/code-review/skills/spawn-reviewers/SKILL.md b/plugins/code-review/skills/spawn-reviewers/SKILL.md index 6300c23..466508c 100644 --- a/plugins/code-review/skills/spawn-reviewers/SKILL.md +++ b/plugins/code-review/skills/spawn-reviewers/SKILL.md @@ -346,6 +346,7 @@ module/class; absence-based design claims follow the protocol's empty-result rul 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 . +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. ``` diff --git a/plugins/code-review/tools/prompts/design_critic_suffix.txt b/plugins/code-review/tools/prompts/design_critic_suffix.txt index 5d0c46e..682e1d8 100644 --- a/plugins/code-review/tools/prompts/design_critic_suffix.txt +++ b/plugins/code-review/tools/prompts/design_critic_suffix.txt @@ -17,13 +17,13 @@ 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/`. diff --git a/plugins/code-review/tools/prompts/shared_prompt.txt b/plugins/code-review/tools/prompts/shared_prompt.txt index 406116c..a896b97 100644 --- a/plugins/code-review/tools/prompts/shared_prompt.txt +++ b/plugins/code-review/tools/prompts/shared_prompt.txt @@ -70,6 +70,11 @@ particular one. Bind to what you actually hold: fails with an input-validation error — so use `ToolSearch` to load the schemas of the ones you intend to use. If nothing in your roster answers a capability, that capability is simply unavailable: use Grep/Glob and move on. + One step before that fallback: some servers expose further read-only tools + only through a broker tool that searches for and calls tools by name. Search + it ONCE per unanswered capability. Call through it only tools whose purpose is + to query — never one that scans, executes, writes, indexes, or records; unclear + purpose → do not call. Every invariant here binds a brokered call as a direct one. - CAPABILITIES worth reaching for, in priority order: C1 SYMBOL LOOKUP — locate a symbol's definition and qualified name. C2 USAGE / CALLER ENUMERATION — find references and callers, including the From 16757e055bfba4324709dc4d063f2b127eeb63f2 Mon Sep 17 00:00:00 2001 From: Kris Wong Date: Mon, 21 Sep 2026 13:27:53 -0500 Subject: [PATCH 3/6] fix(code-review): source the change-impact diff base for every reviewer role Co-Authored-By: Claude --- plugins/code-review/skills/spawn-reviewers/SKILL.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/plugins/code-review/skills/spawn-reviewers/SKILL.md b/plugins/code-review/skills/spawn-reviewers/SKILL.md index 466508c..23e56e6 100644 --- a/plugins/code-review/skills/spawn-reviewers/SKILL.md +++ b/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 `` inline, report `file=WRITE_DENIED`) still covers the case where it is refused. @@ -197,7 +197,9 @@ ToolSearch first. When one is available, prefer it for your cross-file work — C3 (snippet read) to read the exact service/API implementation instead of Glob-guessing 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. Any claim resting on absence ("unused", +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 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 @@ -435,7 +437,9 @@ 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, plus C5 -(change impact) and C7 (duplication), for the cross-file lookups above; any claim resting +(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 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 From 0f0aea5609c351516306364fc50efadf751a951d Mon Sep 17 00:00:00 2001 From: Kris Wong Date: Mon, 21 Sep 2026 13:40:43 -0500 Subject: [PATCH 4/6] docs(code-review): changelog for 3.10.1 --- CHANGELOG.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab79371..404bfd3 100644 --- a/CHANGELOG.md +++ b/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 scoped to the reviewer's assigned files with a row limit and the review's diff base, and 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 `` 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 From b5577f984b95ddabbdd55c3849fe2f15a53a7495 Mon Sep 17 00:00:00 2001 From: Kris Wong Date: Mon, 21 Sep 2026 15:10:33 -0500 Subject: [PATCH 5/6] fix(code-review): bound the change-impact call to one hop, 25 rows and changed-surface files --- .../tools/prompts/impact_analyzer_prompt.txt | 3 ++- plugins/code-review/tools/prompts/shared_prompt.txt | 10 +++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/plugins/code-review/tools/prompts/impact_analyzer_prompt.txt b/plugins/code-review/tools/prompts/impact_analyzer_prompt.txt index 336e2fa..514dc7b 100644 --- a/plugins/code-review/tools/prompts/impact_analyzer_prompt.txt +++ b/plugins/code-review/tools/prompts/impact_analyzer_prompt.txt @@ -101,7 +101,8 @@ Two discovery substrates, used together: Validate every returned path is inside this checkout before citing it (per the shared protocol). When you also hold C5 (change impact), make it your first move: ONE - call, scoped to your assigned files with a row limit, to seed the + call, naming only the files where the patch changes an exported + surface, one caller hop, row limit 25 or less, to seed the candidate-symbol list and its call sites. Diff base: Read `base_ref` from `{CR_DIR}/scope.json` and pass it. If it is absent, empty, or begins with `-`, skip C5. If the tool cannot take a base, skip C5. diff --git a/plugins/code-review/tools/prompts/shared_prompt.txt b/plugins/code-review/tools/prompts/shared_prompt.txt index a896b97..d25e4ae 100644 --- a/plugins/code-review/tools/prompts/shared_prompt.txt +++ b/plugins/code-review/tools/prompts/shared_prompt.txt @@ -89,11 +89,11 @@ particular one. Bind to what you actually hold: are in it, and whether an answer was cut short or degraded. May be a tool, or fields on another tool's response. C7 DUPLICATION — reports near-duplicate symbols. -- C5 CALL DISCIPLINE. Scope the call to your assigned files (split a large - assignment — a row limit may not bound the whole response) and pass a row - limit anyway. Use the review's diff base. Keep only rows tied to a symbol you - are analysing. Treat dunder and generic names (`__init__`, `get`, `open`) as - unverified until a C2 call or a Read confirms the target. +- C5 CALL DISCIPLINE. Keep it small: one caller hop, a row limit of 25 or less, + and only those assigned files whose exported surface changed — the response + grows with every file you name, whatever the limit. Use the review's diff base. + Keep only rows tied to a symbol you are analysing. Treat dunder and generic + names (`__init__`, `get`, `open`) as unverified until C2 or a Read confirms. - C7 HITS ARE CANDIDATES. Read both sides before reporting. A tool error, or duplicate data that has not been built, means C7 is unavailable, never "no duplicates" — and do not build it (NO INDEXING CALLS). It matches From 4e89bd65c06d17fc78614e81d2c5b5ebede4bad8 Mon Sep 17 00:00:00 2001 From: Kris Wong Date: Mon, 21 Sep 2026 15:39:14 -0500 Subject: [PATCH 6/6] docs(code-review): changelog notes the change-impact call bounds --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 404bfd3..b6217bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### 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 scoped to the reviewer's assigned files with a row limit and the review's diff base, and 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". +- 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.