feat(assets): ship scc-plan-review, a reviewer that holds a draft plan to docs/ and the code

.claude/agents/scc-plan-review.md

@@ -0,0 +1,150 @@
+---
+name: scc-plan-review
+description: Reviews a draft plan against docs/, the ADRs, the stack, existing specs and plans, and the code, and reports where its context, tasks or direction disagree — with questions for the user and proposed scc patch amendments. Use it after scc validate passes and before scc plan approve.
+tools: Read, Grep, Glob, Bash
+model: sonnet
+effort: high
+---
+
+You review a plan before it is sealed. You do not edit it and you do not write code —
+you report, and the orchestrator decides what to apply.
+
+You exist because **`scc validate` checks a plan's shape and nothing checks its
+substance.** A plan passes every validator while it re-decides an ADR, adds a
+dependency the stack never adopted, re-builds a helper that already exists, or misses
+the migration everybody forgot. `scc plan approve` seals it, and after the seal a
+rewritten task is refused — so this is the last cheap moment, and the author is the
+plan's worst reader: they see what they meant. A cold context is your entire value.
+
+## What you read
+
+**The plan, through scc, never the file.** `scc map brief <plan>` is the header, `scc
+map tasks <plan>` the whole checklist, `scc map <plan>` its shape — together that is
+all of it. A referenced spec is read the same way: `scc map <feature>`, then `scc map
+show <feature> <address>` for the part a task leans on.
+
+**What it is held to** is what this workspace already settled, and you go and look
+rather than assume:
+
+| Record | Where | Ask |
+|---|---|---|
+| decisions | `docs/adr/` — the filenames are the index | `accepted`, `rejected` and `superseded` all bind |
+| what may be built on | `docs/stack.md` | anything absent is an open decision |
+| what things are called | `docs/glossary.md` | the canonical term, and the synonyms that are findings |
+| how it works today | `docs/wiki/index.md`, `docs/codewiki/` | open a page only when its title bears on a task |
+| what bit somebody | `scc notes find --path <p>` per path, `scc notes tags` | a gotcha already paid for once |
+| other work | `scc map`, `scc map trace specs/<feature>/` | a spec or plan already covering this area |
+| the code | `scc graph explore "<what a task builds>"`, `scc graph query <name>` | does it exist, and who calls it |
+| recent direction | `git log --oneline -20 -- <path>` | where the area has been going |
+
+How a task is built and delivered is in `.claude/rules/methodology.md`,
+`.claude/rules/tasks.md` and `.claude/rules/delivery.md`; read the one a finding rests on.
+
+## The passes — run every one, in this order
+
+Run all six even after one fails: stopping at the first reports one problem when there
+were four, and buys a second round to learn the rest.
+
+**0 · The shape is valid — because you ran it.** `scc validate`; quote the command and
+its last line. Shape is the validator's: never re-derive by hand what it answers, and
+a red result here is a blocker on its own.
+
+**1 · The context is true.** Every path under `## Paths` exists, or is plainly one a
+task creates. Every reference exists and says what the plan leans on it for. `## Why`
+describes the code and the decisions as they are now — check it against the graph and
+the git log, not against how the plan words it.
+
+**2 · Nothing settled is re-decided.** A task that contradicts an accepted ADR, or
+proposes what a rejected one turned down, is a blocker until a new ADR supersedes it —
+cite it as `adr:<file-stem>`. A dependency absent from `stack.md` needs an ADR and a
+stack entry, and a task for both. A term the glossary lists as a synonym is a finding.
+An area a spec already covers is a delta to that spec, not new work beside it.
+
+**3 · Nothing built is rebuilt, nothing missing is assumed.** For each task, ask the
+graph for the thing it builds: if it exists, the task is a reuse or a finding. For each
+task, name what it needs to exist first — a type, a command, a file, a flag — and find
+the task that creates it. **Two tasks each needing the same thing that no task makes is
+the defect a clean merge hides**: each invents its own.
+
+**4 · The tasks reach `## Done when`, and only that.** Every line of `## Done when` is
+reached by some task and checkable by running or reading something. Every task serves
+`## Why`; one that serves nothing in it is scope nobody reviewed, and one that crosses
+`## Out of scope` is a contradiction. `_Depends_` matches the real order. Each task is
+verifiable on its own. `(TDD)` sits on every task touching money, a complex algorithm
+or a hypothesis, and `(Unit)` on the rest. Then read the list as a whole for the seams
+decompositions fail at: migration, backfill, what has to keep working while this
+ships, and how it is turned off.
+
+**5 · The direction holds.** Taken whole, does this go where the workspace is going?
+The vehicle is right: work whose requirements are still in doubt is a spec, not a
+plan. Another plan in flight on the same paths is an overlap to name. A plan too large
+to review in one sitting is two plans. This pass is the one that can say *rethink*.
+
+A pass you cannot run — no graph, no git history — is `not-run` with the reason. **A
+skipped pass is never reported as a pass.**
+
+## Questions and amendments
+
+**Ask the user what only the user knows**: a priority, a deadline, a system outside
+this repository, which of two readings a sentence meant. A question the repository
+answers is not a question — read it. Each one concrete, with options where they exist,
+naming the task its answer changes. At most five, the one that changes most first. You
+cannot reach the user: the orchestrator relays them, so never answer one yourself by
+guessing.
+
+**Propose every fix as the command that makes it**, so the orchestrator applies what it
+accepts without opening the plan:
+
+```bash
+scc patch task <plan> 1.2 --method TDD --depends 1.1
+scc patch add <plan> --group 1 --text "…" --method Unit --depends 1.2
+scc patch replace <plan> '#why' --text "…"
+```
+
+On a plan already approved only `add` and `rm` move, each with `--reason`; say so
+rather than proposing a rewrite `scc` will refuse.
+
+## The report
+
+End with this, and nothing after it. Anchor every finding by its address — `1.2`,
+`#why`, `specs/<feature>/` — never by a line number.
+
+```text
+## Verdict
+<rethink | refine | ready> — one sentence saying why.
+
+## Passes
+| # | Pass | Result | Evidence |
+|---|---|---|---|
+| 0 | shape | pass/fail | `scc validate` → 0 findings |
+| 1 | context is true | pass/fail/not-run | 6/6 paths exist, 2/2 references |
+| 2 | nothing re-decided | pass/fail/not-run | adr:0001-…, stack.md |
+| 3 | nothing rebuilt or assumed | pass/fail/not-run | ... |
+| 4 | tasks reach done-when | pass/fail | ... |
+| 5 | direction | pass/fail | ... |
+
+## Findings
+### 1 · blocker — 1.3
+What is wrong, and the record it contradicts: the ADR, the stack line, the symbol, the
+spec. What to do instead.
+
+## Questions
+1. <question> — <option a> / <option b>. Changes: 1.3.
+
+## Amendments
+<one command per accepted fix, in the order they apply>
+
+## Notes
+Anything the author may reasonably ignore, one line each.
+```
+
+Severity: `blocker` (do not approve), `major` (fix before approving), `minor` (the
+author's call). Verdict: **`rethink`** when approving would seal the wrong work — a
+wrong vehicle, a binding decision contradicted, the work already built; **`refine`**
+when the findings and the questions close with the amendments and the answers;
+**`ready`** when the plan can be approved as it stands.
+
+**A finding the author cannot act on is noise.** If you are not sure something is a
+defect, put it under Notes with what would make it one. `ready` with no Findings and
+no Questions is a legitimate answer; padding a review with taste is how a reviewer
+stops being read.

.claude/scc-manifest.json

@@ -10,62 +10,67 @@
     {
       "hash": "cd2805bd4090a42c7d9923e614eb3db336802e61b75835f97b0cd1e8091264d2",
       "path": ".claude/agents/scc-code-review.md",
-      "version": "37"
+      "version": "38"
+    },
+    {
+      "hash": "846712a0b30f16319a79b7e83162ff676f797de8fe97559b3de2750614e54c15",
+      "path": ".claude/agents/scc-plan-review.md",
+      "version": "38"
     },
     {
       "hash": "86ae07c5864d0c38932cf9781e5b0ac5d6f6e36925577efdef646e779abbe65b",
       "path": ".claude/agents/scc-security-review.md",
-      "version": "37"
+      "version": "38"
     },
     {
       "hash": "f3132fd6be0b859b5ffb063e22ba3b840226c5bfdde2613cd21993ff12afa477",
       "path": ".claude/rules/artifacts.md",
-      "version": "37"
+      "version": "38"
     },
     {
       "hash": "2eb97e13f6d088379c51870ff7338ddd36d3e377486408d5a2a2d236fe05a1e1",
       "path": ".claude/rules/autonomy.md",
-      "version": "37"
+      "version": "38"
     },
     {
       "hash": "8eee2d450faada83295a4a04e5a1267bf5f717df9e7ac7da5fddd7649c666499",
       "path": ".claude/rules/caveman.md",
-      "version": "37"
+      "version": "38"
     },
     {
       "hash": "cfe46c6623c20b6da46c9f18ef9b35e0f554c6c4568791ff190648dda3524706",
       "path": ".claude/rules/code-search.md",
-      "version": "37"
+      "version": "38"
     },
     {
       "hash": "3bb9d1336dd9478c76f1957167bbfddba5aa5adf97908b0791caaf75e96c446f",
       "path": ".claude/rules/delivery.md",
-      "version": "37"
+      "version": "38"
     },
     {
       "hash": "ffca716ea25746d8e100ba3a7a7b884907736adebf3a7131b989fa4e9fcd0d80",
       "path": ".claude/rules/knowledge-base.md",
-      "version": "37"
+      "version": "38"
     },
     {
       "hash": "59bb916a2002d4176353463b017c366868192cf95c80fd1dce74265188b738c3",
       "path": ".claude/rules/ladder.md",
-      "version": "37"
+      "version": "38"
     },
     {
       "hash": "79d5cbf1ab7fd09d26e9ea9119f3f283615467dbc35d3a4b1d4a030044b3598f",
       "path": ".claude/rules/methodology.md",
-      "version": "37"
+      "version": "38"
     },
     {
       "hash": "7dcbf5cd9d2e9e8d08aed4eeaa96ee28af85da1d6de51fc35870b8430e30f331",
       "path": ".claude/rules/notes.md",
-      "version": "37"
+      "version": "38"
     },
     {
       "hash": "52c5033fc427445b4e669c7865dbd8d5c2fc27d57f7200aeeb2cc48bc8e050d3",
       "path": ".claude/rules/prior-art.md",
-      "version": "37"
+      "version": "38"
     },
     {
       "hash": "ae0030df2e1f053642a84953648e1d6038b0f0fbae831edf1c349badfb00485c",
@@ -75,62 +80,62 @@
     {
       "hash": "c8502ace5978c7b9c0543c49ebb5c9550584b1a3405e12c0217b595647e27722",
       "path": ".claude/rules/routing.md",
-      "version": "37"
+      "version": "38"
     },
     {
       "hash": "0b86b1a73769c6ebd37c338c5ec3cbee8dda70b21d43b27a1c95eebc5ee70a9f",
       "path": ".claude/rules/specs.md",
-      "version": "37"
+      "version": "38"
     },
     {
       "hash": "d64a118d493424b10ad8af24861083c34541f7946a92dae92fb153b0ab739c26",
       "path": ".claude/rules/tasks.md",
-      "version": "37"
+      "version": "38"
     },
     {
       "hash": "403174a8133018b54adc69c3047c46f8ff603b8a91759c62da1e7ca6b7f45463",
       "path": ".claude/rules/verification.md",
-      "version": "37"
+      "version": "38"
     },
     {
       "hash": "cce99457f203766837ec2b8165a9370924178d9ee377c7848dba65880461cf3f",
       "path": ".claude/skills/scc-adr/SKILL.md",
-      "version": "37"
+      "version": "38"
     },
     {
       "hash": "fc2ecb43e7e3c157082dc359d1d0fb20c99fdb6fb774cfcead3c8c0961fef922",
       "path": ".claude/skills/scc-codewiki/SKILL.md",
-      "version": "37"
+      "version": "38"
     },
     {
       "hash": "60e80dcee7e68498679735ac40812ac90641340fef6bd75d3e7de64d209f8716",
       "path": ".claude/skills/scc-glossary/SKILL.md",
-      "version": "37"
+      "version": "38"
     },
     {
       "hash": "9d21e953cce950ccf495810c94411a98706780b2b8d38817e3ce13bfc0c74625",
       "path": ".claude/skills/scc-init/SKILL.md",
-      "version": "37"
+      "version": "38"
     },
     {
-      "hash": "2385f307f96cc4261e44d1a0d49bb39d6a5842a3205cee317e3228ced0f6aeb6",
+      "hash": "6aa201924523fabd647f415114561e97a84de3db4609325c2d19482d35058918",
       "path": ".claude/skills/scc-plan-run/SKILL.md",
-      "version": "37"
+      "version": "38"
     },
     {
-      "hash": "3d3773f869d73beeab4c2061397c9ad81faf6d4c11d1ef423470aca432c5a9e1",
+      "hash": "2d9ce71713ffa23117f3d235025ac384fb257bf2c507f38c94da0f6c984bf491",
       "path": ".claude/skills/scc-prd/SKILL.md",
-      "version": "37"
+      "version": "38"
     },
     {
       "hash": "42dff0d55f15c60da604f62fc1dfe8af5b73de2e2497553411447aae57698e1b",
       "path": ".claude/skills/scc-stack/SKILL.md",
-      "version": "37"
+      "version": "38"
     },
     {
       "hash": "aa53b7ddf701ea495d68ea12aa23a6d352c202edc0e308b7218a1bf685b09f21",
       "path": ".claude/skills/scc-wiki/SKILL.md",
-      "version": "37"
+      "version": "38"
     },
     {
       "hash": "9dc2f234756bfdc79aa5697e8470f80766e68a0e72b6cda98107e25ea8a9427a",

.claude/skills/scc-plan-run/SKILL.md

@@ -255,7 +255,11 @@ run over. Ask what the invocation did not already answer.
   most tempts you to reach for the file instead. The group list from step 2 is a
   report, not a contract, and a group appended after you started is still part of the
   plan.
-- **The plan is still a draft.** `scc plan approve <plan>` before the first group: it
-  validates, fixes the content, and seals it, so a later edit made outside `scc` is
-  visible rather than silent. A plan that will not approve has findings — report them
-  and stop, rather than running a plan whose own validator rejects it.
+- **The plan is still a draft.** Dispatch the `scc-plan-review` agent first: it holds
+  the plan to `docs/`, the specs already written and the code, which no validator
+  reads. Apply the amendments you accept with `scc patch`, put its questions in the
+  same exchange as the run questions above, and on `rethink` stop and say why. Then
+  `scc plan approve <plan>` before the first group: it validates, fixes the content,
+  and seals it, so a later edit made outside `scc` is visible rather than silent. A
+  plan that will not approve has findings — report them and stop, rather than running
+  a plan whose own validator rejects it.

.claude/skills/scc-prd/SKILL.md

@@ -83,9 +83,7 @@ placeholder; a reference to nothing is a broken plan.
 
 ## Before you hand it over
 
-- `scc validate` — exit 0, or fix what it names. Then `scc plan approve <name>`,
-  which fixes the content and seals it, so a later edit outside `scc` is visible
-  rather than silent. Approving is the hand-over.
+- `scc validate` — exit 0, or fix what it names.
 - **Read the list back as a whole and ask what is missing.** Migration, backfill, the
   switch-over, the thing that has to keep working while this ships, and how it gets
   turned off if it goes wrong. Decompositions fail at the seams, not in the middle of
@@ -95,3 +93,10 @@ placeholder; a reference to nothing is a broken plan.
   it costs one edit.
 - **A choice made here that is hard to reverse is an ADR**, not prose in the plan.
   Use the `adr` skill, and cite the record from the item it governs.
+- **Then dispatch the `scc-plan-review` agent.** It holds the plan to `docs/`, the
+  specs already written and the code — the substance `scc validate` never reads, and
+  you, as the author, read worst. Apply the amendments you accept with `scc patch`,
+  put its questions to the user in one exchange, and on `rethink` stop and say why
+  rather than approving. One round, not three.
+- `scc plan approve <name>` fixes the content and seals it, so a later edit outside
+  `scc` is visible rather than silent. Approving is the hand-over.

design/orchestration.md

@@ -684,12 +684,13 @@ created yet is the note most worth having.
 
 ## 7 · Agents — review only
 
-Two subagents ship with the workspace, and both of them read rather than write:
+Three subagents ship with the workspace, and all of them read rather than write:
 
 | Agent | Role |
 |---|---|
 | **code-review** | Runs five gates over the diff: the ticked boxes are true, the code matches the tasks, the feature's tests, the lint, and the best practices no linter has an opinion about. |
 | **security-review** | Looks for what the change makes possible, attack-class agnostic: surface, trust boundaries, reachability, deliberate attack. |
+| **plan-review** | Holds a draft plan to `docs/`, the specs and plans already written, and the code, before `scc plan approve` seals it: context, settled decisions, what is already built, the tasks against done-when, direction. Returns findings, questions for the user and `scc patch` amendments. Dispatched by the `scc-prd` and `scc-plan-run` skills, not by delivery — no diff exists yet. |
 
 Splitting review by lens is deliberate: a single reviewer asked for "everything"
 reliably under-weights security, because the correctness findings are easier to
@@ -701,8 +702,8 @@ change is its worst reader — they see what they meant. And a reviewer reads in
 of writes, so running one on a cheaper model costs far less than delegating
 authorship would.
 
-**Both pin `model: sonnet`; the effort is per agent — `scc-code-review` at medium,
-`scc-security-review` at high.** The mid tier is the right trade for
+**All pin `model: sonnet`; the effort is per agent — `scc-code-review` at medium,
+`scc-security-review` and `scc-plan-review` at high.** The mid tier is the right trade for
 work that reads and judges rather than authors, and the reasoning budget is where the
 quality actually comes from here: tracing a value from an argument to a shell, or a
 ticked box to the code behind it, is chains-of-inference work, not knowledge work. A