fix(code-review, fd3): close the defects the console-* runs kept repeating

.claude-plugin/marketplace.json

@@ -117,7 +117,7 @@
     {
       "name": "code-review",
       "source": "./plugins/code-review",
-      "description": "Unified code review that fans out parallel scanners over a change — one per active lens, six to eight per run — and merges them into one per-file report. Eight lenses: comment quality, four quality/craft lenses (readability & tests, naming & module, objects & patterns, simplicity & types), an always-on narrow security lens (secrets, injection, access checks, boundary validation, insecure settings), a performance lens for executable source (N+1, unbounded fetch, blocking-in-async, wasted renders), and a spec lens via --spec <path>. Explicit rules in a root CODING_STANDARDS.md (plus a gitignored .local.md overlay) generate standards findings. The standalone comment-review and quality-review skills stay invocable for a single-lens pass. Reviews the current branch diff by default (or explicit paths / --base). Quality findings are tagged family · rule · severity; comment verdicts are R1–R12 · KEEP/REMOVE/REWRITE/MOVE/ADD, shown side by side and applied through a single risk-cut menu. Successor to the comment-review and quality-review plugins.",
+      "description": "Unified code review that fans out parallel scanners over a change — one per active lens, six to eight per run — and merges them into one per-file report. Eight lenses: comment quality, four quality/craft lenses (readability & tests, naming & module, objects & patterns, simplicity & types), an always-on narrow security lens (secrets, injection, access checks, boundary validation, insecure settings, IaC exposure, access widening), a performance lens for executable source (N+1, unbounded fetch, blocking-in-async, wasted renders), and a spec lens via --spec <path> or a spec file the diff carries. Explicit rules in a root CODING_STANDARDS.md (plus a gitignored .local.md overlay) generate standards findings. The standalone comment-review and quality-review skills stay invocable for a single-lens pass. Reviews the current branch diff by default (or explicit paths / --base). Quality findings are tagged family · rule · severity; comment verdicts are R1–R12 · KEEP/REMOVE/REWRITE/MOVE/ADD, shown side by side and applied through a single risk-cut menu. Successor to the comment-review and quality-review plugins.",
       "version": "0.3.0",
       "author": {
         "name": "Mateusz Gostański",

.github/workflows/code-review-scripts.yml

@@ -0,0 +1,21 @@
+name: code-review scripts
+
+on:
+  pull_request:
+    paths:
+      - 'plugins/code-review/scripts/**'
+      - '.github/workflows/code-review-scripts.yml'
+
+jobs:
+  pytest:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-python@v5
+        with:
+          python-version: '3.12'
+      - name: Install pytest
+        run: python -m pip install pytest
+      # The suite builds real git repositories and sets their identity itself, so no git config is needed here.
+      - name: Run tests
+        run: python -m pytest plugins/code-review/scripts/tests

.github/workflows/fd3-evals.yml

@@ -17,7 +17,8 @@ jobs:
     runs-on: ubuntu-latest
     timeout-minutes: 60
     env:
-      ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }}
+      # Runs on a subscription OAuth token (`claude setup-token`), not a metered API key.
+      CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
     steps:
       - uses: actions/checkout@v4
       - uses: pnpm/action-setup@v4

.gitignore

@@ -15,3 +15,7 @@ plugins/*/evals/.results/
 
 # the target app's own runtime store, written on every boot
 plugins/tester/evals/fixtures/target-app/state.json
+
+# Python bytecode
+__pycache__/
+*.pyc

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

@@ -1,7 +1,7 @@
 {
   "name": "code-review",
   "version": "0.3.0",
-  "description": "Unified code review that fans out parallel scanners over a change — one per active lens, six to eight per run — and merges them into one per-file report. Eight lenses: comment quality, four quality/craft lenses (readability & tests, naming & module, objects & patterns, simplicity & types), an always-on narrow security lens (secrets, injection, access checks, boundary validation, insecure settings), a performance lens for executable source (N+1, unbounded fetch, blocking-in-async, wasted renders), and a spec lens via --spec <path>. Explicit rules in a root CODING_STANDARDS.md (plus a gitignored .local.md overlay) generate standards findings. The standalone comment-review and quality-review skills stay invocable for a single-lens pass. Reviews the current branch diff by default (or explicit paths / --base). Quality findings are tagged family · rule · severity; comment verdicts are R1–R12 · KEEP/REMOVE/REWRITE/MOVE/ADD, shown side by side and applied through a single risk-cut menu. Successor to the comment-review and quality-review plugins.",
+  "description": "Unified code review that fans out parallel scanners over a change — one per active lens, six to eight per run — and merges them into one per-file report. Eight lenses: comment quality, four quality/craft lenses (readability & tests, naming & module, objects & patterns, simplicity & types), an always-on narrow security lens (secrets, injection, access checks, boundary validation, insecure settings, IaC exposure, access widening), a performance lens for executable source (N+1, unbounded fetch, blocking-in-async, wasted renders), and a spec lens via --spec <path> or a spec file the diff carries. Explicit rules in a root CODING_STANDARDS.md (plus a gitignored .local.md overlay) generate standards findings. The standalone comment-review and quality-review skills stay invocable for a single-lens pass. Reviews the current branch diff by default (or explicit paths / --base). Quality findings are tagged family · rule · severity; comment verdicts are R1–R12 · KEEP/REMOVE/REWRITE/MOVE/ADD, shown side by side and applied through a single risk-cut menu. Successor to the comment-review and quality-review plugins.",
   "author": {
     "name": "Mateusz Gostański",
     "email": "mg@grixu.dev"

plugins/code-review/CHANGELOG.md

@@ -10,6 +10,76 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - Author metadata now reads `Mateusz Gostański <mg@grixu.dev>` in `plugin.json` and the marketplace entry.
+- Step 3 spells out that waiting for the Scanners means ending the turn: no `sleep`, `ListAgents`
+  polling, transcript `stat`s, placeholder calls or `Monitor`/`until` loops, and never `TaskStop`
+  on a Scanner — elapsed time is not a state the Orchestrator can observe
+- The conventions note is one byte-identical, suppress-only text in every brief, with each rule
+  quoted verbatim from its file: no per-Lens threat hypotheses, no "do not raise" lists, no
+  paraphrases
+- A Scanner dispatches no agent of its own, waits in no background, and its final message is its
+  whole output; a `<result>` presented as an amendment or a partial list counts as truncated and
+  the Lens is re-dispatched
+- The unavailable-lens question offers exactly proceed-without-it or abort — reviewing that lens
+  inline is not an option on it
+- Step 3 states what to do where the `Agent` tool is absent (inside another agent or a workflow
+  step): say so up front and hand the caller the choice, never discover it halfway and report a
+  single pass as an eight-lens review
+- Scanner search is described tool-neutrally (`Grep`, or `git grep` where sub-agents have no
+  `Grep` tool)
+- Step 4 now judges the fix as well as the finding — behaviour preserved, no contradiction with
+  another fix, no new smell — and re-routes a fix that fails any of the three to the structural
+  walk (with the behaviour change named) or to report-only
+- Boy-scout extras are sorted by risk: a structural or `security` boy-scout fix walks one at a
+  time instead of riding the batch; a `Not flagged` item reaches the menu only as its own named
+  option; and the slot order puts `security`, then verified rule-less correctness problems, ahead
+  of boy-scout extras
+- The `Reconciliation` line gained a `P primary dropped` term and each of its counts now names
+  the rendered block it is checked against (`C` against `Boy-scout`, `D + P` against `Not
+  flagged`); `Not flagged` entries stay countable so the check can be verified from the report
+- Apply-phase discipline: edits go through the `Edit` tool (no `sed`/heredoc rewrites), a
+  formatter runs only on the files the review edited, an approved fix that cannot be applied as
+  approved goes back to the user instead of being substituted, and the wrap-up lists every fix
+  skipped, substituted or extended
+- The rendered report and the user's selection are written to the session scratchpad before the
+  apply walk, so a compaction mid-walk no longer costs the approved list
+- A confirmed exposure that no rule names still leads the report from its own `Not flagged`
+  bullet
+
+- `get_changes.py` reports an `alternate` base when the resolved one saw no committed
+  change and another (`origin/main`, …) holds commits — a branch pushed to its own remote
+  counterpart no longer reads as "nothing to review"; `start-cr`, `comment-review` and
+  `quality-review` offer the re-run instead of stopping
+- A `security` scanner's `CANDIDATES` block is a heading with bullets, never a fenced code block
+- Scope: `.mjs`/`.cjs`/`.mts`/`.cts` are reviewed as source, a directory whose name
+  contains `e2e` (or ends in `-tests`) classifies as `test`, `.txt` is skipped, and a CI
+  workflow file is skipped with a sentence naming its triggers, permissions and secret
+  handling as `/security-review` territory
+- The "substance of the change" sentence now covers a docs, spec/ADR or release-notes
+  branch, not only a dependency manifest
+- When no `--spec` was passed and the diff carries a spec-shaped file (`specs/`,
+  `docs/adr/`, `tasks/`, `*SPEC*.md`, …), `start-cr` offers to review the change against
+  it instead of silently leaving the `spec` lens off
+- `comments` · R4 resolves a spec-id-shaped token against the code before stripping it — a
+  token bound to an identifier or a string literal is a code value, not a document pointer,
+  and the verdict line says which of the two it was
+- `Not flagged` is required whenever a look-alike was cleared, and a clean file is the case
+  that needs it most: without the line a reader cannot tell a review that cleared candidates
+  from one that never looked
+
+### Added
+
+- `security` · **`iac-exposure`** (high) — infrastructure code that materializes a secret into
+  state or an unmarked output, or grants trust wider than the identity it names (an OIDC
+  condition matching beyond the intended workflow, a wildcard principal, anonymous access)
+- `security` · **`access-widening`** (high) — a change that relaxes an authorization boundary
+  that existed: a weaker permission, a removed guard, a dropped owner predicate, a widened
+  allowlist
+- `missing-access-check` calibration now routes a test that would stay green if the guard
+  regressed to `tests` · test-fidelity, instead of grading a test gap as a security high
+- Evals — a unified-diff scanner track plus four scenarios covering the defects this round
+  fixed: `iac-exposure` recall, `access-widening` on a diff, a guard whose weak tests must route
+  to `tests` · test-fidelity rather than a security high, and scope classification over a mixed
+  tree (`.mjs`/`.cjs`, an `e2e` directory, `.txt`, a CI workflow)
 
 ## [0.3.0] - 2026-09-02
 

plugins/code-review/CONTEXT.md

@@ -15,7 +15,7 @@ One parallel review subagent running exactly one Lens.
 _Avoid_: role, reviewer, worker
 
 **Lens**:
-One of the eight rule clusters: comments (`R1`–`R12`), readability & tests, naming & module, objects & patterns, simplicity & types, security, performance, spec. Three sit beyond the craft five: security (always on), and two gated ones — performance (executable source files in scope), spec (`--spec <path>` given). Comments is a Lens like any other, not a special case; the three added Lenses have no standalone skill.
+One of the eight rule clusters: comments (`R1`–`R12`), readability & tests, naming & module, objects & patterns, simplicity & types, security, performance, spec. Three sit beyond the craft five: security (always on), and two gated ones — performance (executable source files in scope), spec (a spec file named — `--spec <path>`, or the one the diff carries, accepted by the user). Comments is a Lens like any other, not a special case; the three added Lenses have no standalone skill.
 _Avoid_: theme, dimension
 
 **Rules file**:
@@ -25,7 +25,7 @@ The single source of truth for one Lens's rule text: `references/rules/<lens>.md
 One of the eleven stable top-level labels in the quality vocabulary: `readability`, `tests`, `naming`, `module`, `objects`, `patterns`, `simplicity`, `security`, `performance`, `spec`, and `standards`. Ten are fixed by the plugin; `standards` is repo-defined (its rules come from the Standards file).
 
 **Rule**:
-A specific sub-tag under a Family — one of the 42 fixed rules across the ten plugin-defined Families, or a repo-defined `standards` rule whose slug derives from the quoted rule — or one of the comment rules `R1`–`R12`.
+A specific sub-tag under a Family — one of the 44 fixed rules across the ten plugin-defined Families, or a repo-defined `standards` rule whose slug derives from the quoted rule — or one of the comment rules `R1`–`R12`.
 
 **Finding**:
 The quality-side unit of output: `family` · rule · severity · lines → fix.
@@ -42,7 +42,7 @@ The root pair `CODING_STANDARDS.md` + `CODING_STANDARDS.local.md`, read together
 _Avoid_: style guide, conventions file (a conventions file — `CLAUDE.md`, `AGENTS.md`, `CONTRIBUTING.md`, `.claude/rules`, `.cursor/rules` — only suppresses)
 
 **Active lens set**:
-The N Lenses (6 to 8) that Step 2b of **start-cr** resolves for one run: the five craft Lenses and security always, performance when the source-kind subset of the resolved files (minus `.sh`) is non-empty, spec when `--spec` was given. The report's `Lenses: L of 8` line records it, naming every inactive Lens with its reason.
+The N Lenses (6 to 8) that Step 2b of **start-cr** resolves for one run: the five craft Lenses and security always, performance when the source-kind subset of the resolved files (minus `.sh`) is non-empty, spec when a spec file was named. The report's `Lenses: L of 8` line records it, naming every inactive Lens with its reason.
 _Avoid_: lens selection (the user never picks), enabled lenses
 
 ## Relationships
@@ -60,7 +60,7 @@ _Avoid_: lens selection (the user never picks), enabled lenses
 ## Example dialogue
 
 > **Dev:** "Can I run just the comment **Scanner**?"
-> **Domain expert:** "Invoke the `comment-review` skill directly — **start-cr** always runs its whole **Active lens set**; a **Scanner** is its internal unit of fan-out, not a user-facing switch. The set is decided by the change and the `--spec` flag, never by picking lenses."
+> **Domain expert:** "Invoke the `comment-review` skill directly — **start-cr** always runs its whole **Active lens set**; a **Scanner** is its internal unit of fan-out, not a user-facing switch. The set is decided by the change and by whether a spec file is named, never by picking lenses."
 
 ## Flagged ambiguities
 

plugins/code-review/README.md

@@ -28,7 +28,8 @@ From the `grixu/cc-toolkit` marketplace:
 
 `/start-cr` has no lens switch; the change decides which lenses run. The five
 craft lenses and `security` run every time; `performance` runs when the change
-touches executable source; `spec` runs only when you pass `--spec <path>`. The
+touches executable source; `spec` runs when a spec file is named — by `--spec <path>`,
+or by accepting the one the review offers when the diff itself carries a spec. The
 report's `Lenses: L of 8` line names every lens that sat out and why. For a
 partial review, invoke `/comment-review` or `/quality-review` directly; both stay
 independently available and share the same rule text as the command. The three
@@ -79,14 +80,17 @@ Three more sit beyond the craft five — one always on, two gated — and the re
 says which ran:
 
 - **security** — always on. Secrets in source, injection sinks, missing access
-  checks, unvalidated boundaries, and insecure settings in source files. A
-  finding names both the source and the sink; a pattern alone is never a finding.
+  checks, unvalidated boundaries, insecure settings, infrastructure code that
+  exposes a secret or trusts too widely, and a change that relaxes an existing
+  authorization boundary. A finding names both the source and the sink; a pattern
+  alone is never a finding.
 - **performance** — only when the change touches executable source (not tests,
   not infrastructure-as-code, not `.sh`). N+1 calls, unbounded fetches, blocking calls on an
   async path, wasted React renders. Every finding names the multiplier, the call
   inside it, the missing bound, and the batch/limit API that exists; "could be
   slow" is not a finding.
-- **spec** — only with `--spec <path>` (a local file). A spec line nothing
+- **spec** — only with a named spec file (`--spec <path>`, or the one the review
+  offers from the diff). A spec line nothing
   implements, one implemented against its wording, one only partly met, and scope
   creep the spec never asked for. Every finding quotes the spec line.
 
@@ -123,7 +127,7 @@ Tests, infrastructure-as-code, and `.sh` files are reviewed by the craft lenses
 
 **This is a craft review plus a narrow security lens, not a security audit.** The
 security lens looks for secrets, injection, access checks, boundary validation,
-and insecure settings in source files; the performance lens raises diff-level
+insecure settings, infrastructure exposure, and widened access in source files; the performance lens raises diff-level
 hypotheses it can point at a line. Neither is a dependency, config, or data-flow
 audit: `.env` files, manifests, and lockfiles are not scanned, and a
 vulnerability outside those shapes will surface only by accident. Do not read a