fix(code-review): key signal cache on the diff

- ISS-8961: the signals/ namespace key was (diff_tip, taxonomy_hash,
  prompt_hash), and diff_tip is a ref name -- literally "HEAD" for a
  local branch review -- so no component varied with the diff. The
  cache dir is ~/.claude/cr-cache-global-repo-<worktree basename>, so
  two reviews from one pooled directory shared directory and key, and
  the second was served the first's extracted signals.
- Add signal_input_hash: SHA-256 over the canonical serialization of
  the agent input bundle extract-signals-prepare builds, which is
  exactly what the extraction agent reads. Key is now (diff_tip,
  input_hash, taxonomy_hash, prompt_hash).
- Build the bundle before computing the key rather than only on a miss.
- A bundle with no changed files has no diff identity: emit an empty
  cache_key plus cache_bypass_reason "no-changed-files", which
  suppresses the read and (via consolidate's empty-key guard) the write.
- Correct the --diff-tip help, which claimed the flag was a SHA.

Testing: pytest plugins/ (3 pre-existing failures unrelated to this
change: TestOverrideCache/TestPR114ReviewFixes fail on clean main
because their override fixture is dated 2026-05-29 and the overrides
TTL is 90 days). Counterfactual: reverting code_review_helpers.py turns
TestPooledWorktreeSignalCacheIsolation red with lane 2's manifest
reading status "cache_hit". ruff check; pyright.

Risks: cached extractions written under the old key scheme are misses
after this lands, so the first review in each cache directory re-runs
one Haiku extraction.

Author: mikeangstadt

CHANGELOG.md

@@ -4,6 +4,11 @@ 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.7.1
+
+#### Fixed
+- **Signal extraction no longer serves one review's signals to a different review that shares a cache directory (ISS-8961).** The `signals/` namespace key was `(diff_tip, taxonomy_hash, prompt_hash)`, and `diff_tip` is a ref *name* — `resolve-scope` leaves it as the literal `"HEAD"` for every local branch review and sets it to `origin/<head_ref>` for a PR — so no component of the key varied with the diff. The cache directory is `~/.claude/cr-cache-global-repo-<basename of the git toplevel>`, which is the worktree directory name, so any two reviews run from the same directory shared both the directory and the key: the second review was served the first review's extracted signals and `extract_signals.json` described a diff that was never under review. The key now includes `signal_input_hash` — a SHA-256 over the canonical serialization of the agent input bundle `extract-signals-prepare` builds (file list, per-file added/removed excerpts, intent hint), which is exactly what the extraction agent reads — so a different diff produces a different key even at an identical path, branch, and tip. `extract-signals-prepare` builds that bundle before computing the key rather than only on a miss. When the bundle carries no changed files there is no diff identity to key on, so the run bypasses the cache in both directions: it emits an empty `cache_key` plus `cache_bypass_reason: "no-changed-files"` in the manifest, which suppresses the read and, through `extract-signals-consolidate`'s existing empty-key guard, the write. A miss re-runs a single Haiku extraction; a wrong hit silently misdescribes the change under review.
+
 ### code v1.14.10
 
 #### Added

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

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

plugins/code-review/SCHEMA.md

@@ -585,7 +585,7 @@ A MAJOR `schema_version` bump invalidates every cache namespace at once.
 | Namespace           | Path                                              | Key inputs                                                    | TTL    |
 | ------------------- | ------------------------------------------------- | ------------------------------------------------------------- | ------ |
 | BHA findings        | `<CACHE_DIR>/bha/<file_hash>.json`                | file_content_hash + prompt_hash + model_id + schema_version   | 30 d   |
-| Signal extraction   | `<CACHE_DIR>/signals/<diff_tip>.json`             | diff_tip + taxonomy_hash + signal_prompt_hash                 | 7 d    |
+| Signal extraction   | `<CACHE_DIR>/signals/<key>.json`                  | diff_tip + agent_input_hash + taxonomy_hash + signal_prompt_hash | 7 d |
 | Coverage critic     | `<CACHE_DIR>/coverage_critic/<diff_tip>.json`     | coverage_plan_initial_hash + signals_hash + critic_prompt_hash | 7 d   |
 | Verification        | `<CACHE_DIR>/verifications/<finding_id>.json`     | finding_id + file_content_hash + verifier_model + verifier_prompt_hash | 30 d |
 | Overrides           | `<CACHE_DIR>/overrides/<finding_id>.json`         | finding_id (file content change invalidates)                  | 90 d   |

plugins/code-review/tools/python/code_review_helpers.py

@@ -6223,7 +6223,8 @@ def cmd_detect_injection(args: argparse.Namespace) -> int:
 # ---------------------------------------------------------------------------
 # Two-step LLM stage modelled on PLN-722's verifier:
 #   1. ``extract-signals-prepare`` — read diff_data.json + intent + taxonomy,
-#      compute the cache key, check the cache. On hit: write the final
+#      build the agent input bundle, compute the cache key from that
+#      bundle's content hash, check the cache. On hit: write the final
 #      ``extract_signals.json`` immediately. On miss: write the agent input
 #      bundle (diff summary + taxonomy reference) and the manifest the
 #      orchestrator uses to spawn a single Haiku agent.
@@ -6240,6 +6241,10 @@ def cmd_detect_injection(args: argparse.Namespace) -> int:
 SIGNAL_EXTRACTION_MARKER = "signal-extraction-failed"
 SIGNAL_TAXONOMY_FILENAME = "signal_taxonomy.json"
 SIGNAL_EXTRACTION_PROMPT_FILENAME = "signal_extraction_prompt.txt"
+# Recorded in the manifest when the agent input carries no changed files:
+# there is no diff identity to key on, so the run neither reads nor writes
+# the signals cache (ISS-8961 — a miss is always cheaper than a wrong hit).
+SIGNAL_CACHE_BYPASS_NO_FILES = "no-changed-files"
 
 # Cap on per-file excerpt size injected into the agent input. The taxonomy
 # is the agent's reference — the diff context is the evidence. We need
@@ -6331,21 +6336,47 @@ def _signal_extraction_prompt_hash(path: Path) -> str:
     return hashlib.sha256(path.read_bytes()).hexdigest()
 
 
+def signal_input_hash(agent_input: dict[str, Any]) -> str:
+    """Content fingerprint of the exact bundle the extraction agent reads.
+
+    ``_build_signal_input`` is what the agent consumes — file list, per-file
+    excerpts, and the intent hint — so hashing its canonical serialization
+    fingerprints every input the extraction is a function of. Sorted keys
+    make the serialization order-independent; the builder already emits
+    files and excerpts in a deterministic order.
+
+    ISS-8961: this is the component that makes the key diff-derived. The
+    ref-name component (``diff_tip``) is ``"HEAD"`` for every local branch
+    review, so before this hash existed the key was constant across every
+    review that shared a cache directory — and a pooled worktree makes the
+    cache directory shared by construction.
+    """
+    canonical = json.dumps(
+        agent_input, sort_keys=True, separators=(",", ":"), ensure_ascii=True,
+    )
+    return hashlib.sha256(canonical.encode("utf-8", "replace")).hexdigest()
+
+
 def signal_extraction_cache_key(
-    diff_tip: str, taxonomy_hash: str, prompt_hash: str,
+    diff_tip: str, input_hash: str, taxonomy_hash: str, prompt_hash: str,
 ) -> str:
     """Cache key for the ``signals`` namespace (PLN-725).
 
-    Tuple ``(diff_tip, taxonomy_hash, prompt_hash)`` is the complete set
-    of inputs the extraction is a pure function of. Both
-    ``taxonomy_hash`` and ``prompt_hash`` are content-addressed hashes of
-    the on-disk asset bytes (``_taxonomy_hash`` and
-    ``_signal_extraction_prompt_hash``), computed inside
+    Tuple ``(diff_tip, input_hash, taxonomy_hash, prompt_hash)`` is the
+    complete set of inputs the extraction is a pure function of.
+    ``input_hash`` (``signal_input_hash``), ``taxonomy_hash`` and
+    ``prompt_hash`` are content-addressed hashes of, respectively, the
+    agent input bundle and the on-disk asset bytes, all computed inside
     ``cmd_extract_signals_prepare`` rather than taken on faith from
-    caller-supplied flags. Editing either asset flips the key for real.
+    caller-supplied flags. Editing any of them flips the key for real.
+
+    ``diff_tip`` is only a ref *name* (``"HEAD"``, ``origin/<branch>``),
+    never a commit id, so it discriminates nothing on its own — keep it as
+    a coarse extra component, never as the diff identity.
     """
     payload = (
         (diff_tip or "") + "\0"
+        + (input_hash or "") + "\0"
         + (taxonomy_hash or "") + "\0"
         + (prompt_hash or "")
     )
@@ -6535,8 +6566,10 @@ def fail_closed_signal_set(taxonomy: dict[str, Any]) -> list[dict[str, Any]]:
 def cmd_extract_signals_prepare(args: argparse.Namespace) -> int:
     """PLN-725 Stage 1a: prep the signal-extraction agent input + check cache.
 
-    Reads ``diff_data.json`` and (optionally) an intent summary, computes
-    the ``(diff_tip, taxonomy_hash, prompt_hash)`` cache key, and either:
+    Reads ``diff_data.json`` and (optionally) an intent summary, builds the
+    agent input bundle, computes the
+    ``(diff_tip, input_hash, taxonomy_hash, prompt_hash)`` cache key, and
+    either:
 
       - **Cache hit** — writes the cached extraction directly to
         ``<cr_dir>/extract_signals.json`` and emits a manifest with
@@ -6548,6 +6581,13 @@ def cmd_extract_signals_prepare(args: argparse.Namespace) -> int:
         ``taxonomy_path``, ``prompt_path`` so the orchestrator can spawn
         a single Haiku agent.
 
+    ISS-8961: the bundle is built *before* the key so the key is derived
+    from the diff the agent will actually read. A degenerate bundle (no
+    changed files) has no diff identity to key on, so the cache is bypassed
+    in both directions — miss now, no cache write later — rather than
+    keyed on the components that remain, which are identical across every
+    review sharing a cache directory.
+
     Always exits 0; structural failures (no diff_data, malformed
     taxonomy) print to stderr and return 1.
     """
@@ -6599,12 +6639,31 @@ def cmd_extract_signals_prepare(args: argparse.Namespace) -> int:
         return 1
 
     taxonomy_hash = _taxonomy_hash(taxonomy_bytes)
-    key = signal_extraction_cache_key(diff_tip, taxonomy_hash, prompt_hash)
+
+    intent_summary: dict[str, Any] | None = None
+    if intent_path is not None:
+        try:
+            with open(intent_path) as f:
+                intent_summary = json.load(f)
+        except (OSError, json.JSONDecodeError):
+            intent_summary = None
+        if not isinstance(intent_summary, dict):
+            intent_summary = None
+
+    agent_input = _build_signal_input(diff_data, intent_summary)
+    keyable = bool(agent_input.get("files"))
+    key = (
+        signal_extraction_cache_key(
+            diff_tip, signal_input_hash(agent_input), taxonomy_hash, prompt_hash,
+        )
+        if keyable
+        else ""
+    )
 
     output_path = cr_dir / "extract_signals.json"
 
     manifest_path = cr_dir / "extract_signals_manifest.json"
-    cached = _read_cached_signals(cache_dir, key)
+    cached = _read_cached_signals(cache_dir, key) if key else None
     if cached is not None:
         # Strip cache-only metadata before writing the canonical output.
         canonical = {k: v for k, v in cached.items() if k != "written_at"}
@@ -6623,17 +6682,6 @@ def cmd_extract_signals_prepare(args: argparse.Namespace) -> int:
             "model": model,
         })
 
-    intent_summary: dict[str, Any] | None = None
-    if intent_path is not None:
-        try:
-            with open(intent_path) as f:
-                intent_summary = json.load(f)
-        except (OSError, json.JSONDecodeError):
-            intent_summary = None
-        if not isinstance(intent_summary, dict):
-            intent_summary = None
-
-    agent_input = _build_signal_input(diff_data, intent_summary)
     input_path = cr_dir / "extract_signals_input.json"
     with open(input_path, "w") as f:
         json.dump(agent_input, f, indent=2)
@@ -6643,7 +6691,7 @@ def cmd_extract_signals_prepare(args: argparse.Namespace) -> int:
     taxonomy_snapshot_path = cr_dir / "extract_signals_taxonomy.json"
     taxonomy_snapshot_path.write_bytes(taxonomy_bytes)
 
-    return _write_and_emit_manifest(manifest_path, {
+    manifest: dict[str, Any] = {
         "status": "needs_agent",
         "cache_key": key,
         "taxonomy_hash": taxonomy_hash,
@@ -6653,7 +6701,13 @@ def cmd_extract_signals_prepare(args: argparse.Namespace) -> int:
         "prompt_path": str(prompt_path),
         "output_path": str(output_path),
         "model": model,
-    })
+    }
+    if not keyable:
+        # An empty cache_key also stops consolidate writing this run into
+        # the cache, so a bundle with no diff identity is neither served
+        # from the cache nor served to a later lane.
+        manifest["cache_bypass_reason"] = SIGNAL_CACHE_BYPASS_NO_FILES
+    return _write_and_emit_manifest(manifest_path, manifest)
 
 
 def cmd_extract_signals_consolidate(args: argparse.Namespace) -> int:

plugins/code-review/tools/python/config/cli.json

@@ -284,7 +284,7 @@
             "--diff-tip"
           ],
           "required": true,
-          "help": "Diff tip SHA for cache key"
+          "help": "Diff tip ref name; a coarse cache-key component, not the diff identity."
         },
         {
           "flags": [
@@ -297,7 +297,7 @@
           "flags": [
             "--cache-dir"
           ],
-          "help": "Optional cache directory; fresh (diff_tip, taxonomy_hash, prompt_hash) tuples are served from the signals/ namespace."
+          "help": "Optional cache directory; fresh (diff_tip, input_hash, taxonomy_hash, prompt_hash) tuples are served from the signals/ namespace."
         },
         {
           "flags": [

plugins/code-review/tools/python/prefix_fixtures/golden_prefix_empty_diff/expected/extract_signals_manifest.json

@@ -1,4 +1,5 @@
 {
+  "cache_bypass_reason": "no-changed-files",
   "cache_key": "<CACHE_KEY>",
   "input_path": "<CR_DIR>/extract_signals_input.json",
   "model": "haiku",