From c91682838865bf14aca122e08a5649f1b4d7921a Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Wed, 28 Oct 2026 14:02:38 -0500 Subject: [PATCH 01/15] docs: review rule selector design and plan Co-Authored-By: Claude Opus 5.5 --- .../plans/2026-10-28-review-rule-selector.md | 137 ++++++++++++++++++ .../2026-10-28-review-rule-selector-design.md | 111 ++++++++++++++ 2 files changed, 248 insertions(+) create mode 100644 docs/superpowers/plans/2026-10-28-review-rule-selector.md create mode 100644 docs/superpowers/specs/2026-10-28-review-rule-selector-design.md diff --git a/docs/superpowers/plans/2026-10-28-review-rule-selector.md b/docs/superpowers/plans/2026-10-28-review-rule-selector.md new file mode 100644 index 0000000000..b5146d2124 --- /dev/null +++ b/docs/superpowers/plans/2026-10-28-review-rule-selector.md @@ -0,0 +1,137 @@ +# Review rule selector Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** The `review`, `self-review` and `receive-review` engines gain an optional `rule-selector` slot (contract `rule-selector@1`) that selects rules for the diff, asks the reviewer for a verdict per rule, checks the verdicts, and turns failed rules into findings. + +**Architecture:** Each engine declares a `rule-selector` prop in a new block. Two new include bodies carry the shared prose: `review-rules-select-body` (the contract and the select step, all three engines) and `review-rules-verdicts-body` (check, repair, fails to findings, the verdict file; review and self-review). New graph nodes, each bound to its own short node block, place the steps in the review and self-review graphs and the select step in receive-review. Everything is engine prose compiled by `rt skills compile`; tests compile the real engines against a fixture pack. + +**Tech Stack:** Markdown block trees under `plugins/mattstack/attachments/`, Mermaid engine graphs, `rt skills compile`, `bun test`, `check-dot.py`, `certify.sh`. + +**Spec:** `docs/superpowers/specs/2026-10-28-review-rule-selector-design.md` (read it first; it is the authority). + +## Global Constraints + +- Tool-neutral: no engine, include, test fixture name or doc names a specific selector tool, product, team or employer. Fixture names use `acme`. +- Prop: name `rule-selector`, `merge: replace`, `default: ""`, `contract: "rule-selector@1"`; marker `{{props.rule-selector}}` alone on its line. +- Fill lines, verbatim: `selector: none`; or `selector: `, `verdicts: `, `fence: `. Placeholders use single braces (`{{` is a compiler token). +- Budget key `verdict-rounds:`, limit 2, charged on the problem event (one repair). +- Files in the review scratch dir: `selection.json`, `verdict-set.json`, `review-verdicts.json`. Next to the report json: `review-verdicts.json`. +- Tiers: a `blocking` rule's fail is Important, an `advisory` rule's fail is Minor; Critical is never assigned by a rule. +- Graph rules: short ids (`[a-z]+(_[a-z]+)*`), a plaintext node's id starts with its call word (`run_budget_...`), diamonds end in `?` with labelled out-edges, a node block has `node:` and a `# ` title and no `order:`. +- No em or en dashes, no personal names, no ticket ids (certify greps). Shell forms an MCP tool covers need `` only where the engine truly runs them; the selector and verdicts commands come from the fill and run on Bash. +- No new MCP tool or app capability: leave `plugins/mattstack/pack/requires.json` unchanged. +- One `plugins/mattstack/.claude-plugin/plugin.json` version bump for the branch, in the last task, with the recompiled mattstack-pack output in the same commit. +- Commit trailer: `Co-Authored-By: Claude Opus 5.5 `. +- Read `.claude/rules/build-and-plugin.md`, `plugins/mattstack/plugin/skills/editing-skills/SKILL.md` and `plugins/mattstack/plugin/skills/process-digraphs/SKILL.md` before editing engine text; load superpowers:writing-skills for the prose. + +## Review Focus + +1. A fill that resolves to `selector: none`, or no binding at all: the run is byte-for-byte today's behaviour apart from skipped steps. Pinned in Task 1 (unbound compile has no fill text) and in each graph task's prose ("with no selector, skip"). +2. A selector that exits non-zero or prints JSON without `rules`: quoted as a setup observation, dispatch goes ahead with no rule text and no verdict request. Pinned in Task 1's select prose and the GREEN walkthrough in Task 5. +3. A reviewer reply with no fenced block, or two: one problem, one repair, then unanswered rules in the notes; never a hold. Pinned in Task 1's verdicts prose and Task 5's walkthrough. +4. Head checkout provisioning failed: the select step still runs (it needs only the diff). Pinned in Task 2 (every edge into `dispatch_reviewer` passes `select_rules`). +5. A fail whose only evidence is `old`-side: anchored nowhere on the new side; it stays a finding with the location in the body. Pinned in Task 1's fails-to-findings prose. + +--- + +### Task 1: The slot and the shared bodies + +**Files:** +- Create: `plugins/mattstack/attachments/review-rules-select-body/index.block.md` and its section blocks +- Create: `plugins/mattstack/attachments/review-rules-verdicts-body/index.block.md` and its section blocks +- Create: `plugins/mattstack/attachments/review/review/rule-selector.block.md`, `.../self-review/rule-selector.block.md`, `.../receive-review/rule-selector.block.md` +- Create: `lib/skills/__tests__/fixtures/rule-selector/` (a pack: `pack/stubs.jsonc` rostering `review`, `self-review`, `receive-review`; `pack/skills.jsonc` binding `mattstack:/rule-selector` → `acme:acme-rule-selector` for all three; `attachments/acme-rule-selector/SKILL.md` with `metadata.provides: "rule-selector@1"`) +- Test: `lib/skills/__tests__/rule-selector.test.ts` + +**Interfaces:** +- Produces: block key `mattstack:/rule-selector`, prop `rule-selector`, contract `rule-selector@1`; include names `review-rules-select-body` and `review-rules-verdicts-body` with these section titles (later tasks' node blocks point at them by title): select body: `# Rule selector`, `## The contract`, `## Select the rules`, `## When the selector fails`; verdicts body: `# Rule verdicts`, `## Check the verdict set`, `## Repair the verdict set`, `## Failed rules become findings`, `## The verdict file`. + +- [ ] **Step 1: Write the failing tests** in `rule-selector.test.ts`, compiling the real `plugins/mattstack` against the fixture the way `browser-target.test.ts` does (`skillsCompile` with `--pack-dir`, `--mattstack-dir`, `--manifest`): + - `bound: each review engine inlines the fill under the rule-selector prop`: for each of the three verbs, the compiled SKILL.md contains the fill's `fence: acme-verdicts` line and a seam marker containing `prop:rule-selector.rule-selector`. + - `bound: review and self-review include both bodies, receive-review only the select body`: `## Select the rules` in all three; `## Failed rules become findings` in review and self-review only. + - `unbound: no fill text, bodies still present`: compile the same fixture with the bindings removed; no `acme-verdicts`, and `## Select the rules` still present. + - `a fill providing another contract is refused`: a second fixture fill with `provides: "review-criteria@1"` bound to `rule-selector` makes the compile fail with code `contract-mismatch`. +- [ ] **Step 2: Run them, expect FAIL.** `bun test lib/skills/__tests__/rule-selector.test.ts` (from the repo root). +- [ ] **Step 3: Write the prop blocks and the bodies.** Each engine's `rule-selector.block.md` copies `criteria.block.md`'s shape: the prop frontmatter from Global Constraints, `order:` 9 in review, 8 in self-review, 9 in receive-review (each a free value just before the engine's `reviewer` or `reply-rules` block; renumber nothing), title `# Rule selector`, the marker, then `{{include:review-rules-select-body}}`, and for review and self-review `{{include:review-rules-verdicts-body}}`. The bodies carry the spec's sections as engine prose: the contract (the spec's rule-selector@1 text), select (resolve the fill on every run; with `selector: none` or no binding skip every rule step; run the selector with `json` then `text` on the diff on stdin; make the review scratch dir here if none exists yet and write `selection.json` there; the text goes in the dispatch payload with the request to end the reply with the verdict set in a block tagged with the `fence` value), failure (quote as a setup observation; go on with no rules), check (extract the block to `verdict-set.json`; zero or two or more blocks, or unparseable JSON, is one problem; run the verdicts command with `{selection}` and `{verdicts}` replaced by the two paths; exit 0 none, 1 problems, other exit one problem), repair (charged per the budget; a fresh reviewer gets the selection text, the first verdict set and findings, and the problems, and returns a corrected verdict set only; the first findings stay), fails to findings (title `: `, tiers per Global Constraints, anchor the first `new` evidence, `old` locations in the body, merge with a reviewer finding at the same file and line, rules with no valid verdict listed as unanswered in the draft's notes with the problems left), the verdict file (`review-verdicts.json` in the scratch dir always, next to the report json when a report path was handed in). +- [ ] **Step 4: Run the tests, expect PASS;** also `bun test lib/skills/__tests__/browser-target.test.ts` stays green. +- [ ] **Step 5: Certify** each new attachment root: `sh plugins/mattstack/tests/certify.sh plugins/mattstack/attachments/review-rules-select-body` and the same for `review-rules-verdicts-body`; the engine roots are certified in Task 5. +- [ ] **Step 6: Commit.** `git add plugins/mattstack/attachments lib/skills/__tests__` then `git commit -m "skills: rule-selector slot and shared rule bodies for the review engines"`. + +--- + +### Task 2: The review graph + +**Files:** +- Modify: `plugins/mattstack/attachments/review/review/the-review-graph/index.block.md` +- Create in `.../the-review-graph/`: `select-rules.block.md`, `check-verdicts.block.md`, `repair-verdicts.block.md`, `fails-to-findings.block.md` +- Modify: `.../review/run.block.md` (budget table), `.../the-review-graph/write-post-extras.block.md`, `.../the-review-graph/write-report.block.md`, `plugins/mattstack/attachments/review-core-body-after/dispatch-the-review.block.md`, `plugins/mattstack/attachments/review-dispatch-body-after/reviewer-shape.block.md` +- Test: `lib/skills/__tests__/rule-selector.test.ts` + +**Interfaces:** +- Consumes: Task 1's section titles. +- Produces: node ids `select_rules`, `check_verdicts`, `verdict_problems`, `run_budget_verdict_rounds`, `verdict_rounds_spent`, `repair_verdicts`, `fails_to_findings` (Task 3 reuses the same ids in the self-review graph). + +- [ ] **Step 1: Failing test** `review graph places the rule steps`: in the bound compile's review SKILL.md, the dot fence has edges `select_rules -> dispatch_reviewer`, `dispatch_reviewer -> check_verdicts`, `assemble_draft -> fails_to_findings`, `fails_to_findings -> verify_blocking`, no edge `dispatch_reviewer -> assemble_draft`, and the text contains `run_budget {runDb, key: verdict-rounds:, limit: 2, stage}`. Run it, expect FAIL. +- [ ] **Step 2: Edit the graph.** Declare the nodes: + - `select_rules` (block title `Select the rules for the change`); + - `check_verdicts` (`Check the reviewer's verdict set`); + - `verdict_problems{Verdict problems left?}`; + - `run_budget_verdict_rounds[/"run_budget {runDb, key: verdict-rounds:, limit: 2, stage}"/]`; + - `verdict_rounds_spent{verdict-rounds: spent?}`; + - `repair_verdicts` (`Repair the verdict set`); + - `fails_to_findings` (`Turn failed rules into findings`). + + Edges: every edge that today ends at `dispatch_reviewer` (the two `checks not run` edges, `repro_browser -->|no...|`, `capture_repro -->|...|`) ends at `select_rules` instead, with the same labels, and `select_rules --> dispatch_reviewer`; replace `dispatch_reviewer --> assemble_draft` with `dispatch_reviewer --> check_verdicts`, `check_verdicts --> verdict_problems`, `verdict_problems -->|none, or no selector| assemble_draft`, `verdict_problems -->|yes| run_budget_verdict_rounds`, `run_budget_verdict_rounds --> verdict_rounds_spent`, `verdict_rounds_spent -->|no| repair_verdicts`, `repair_verdicts --> check_verdicts`, `verdict_rounds_spent -->|yes: list them in the notes| assemble_draft`; replace `assemble_draft --> verify_blocking` with `assemble_draft --> fails_to_findings` and `fails_to_findings --> verify_blocking`. `redispatch_reviewer --> assemble_draft` stays. +- [ ] **Step 3: Node blocks.** Each is a few lines that point to its Task 1 section by title and say what to skip with no selector (select: skip when unbound or `selector: none`; check: with no selector, `verdict_problems` takes `none`; fails: nothing to add with no selector, and on a pass after `redispatch_reviewer` the merge keeps one finding per rule). +- [ ] **Step 4: Edit the existing blocks.** `run.block.md`: a row `` `verdict-rounds:` | 2 | a verdict check at that depth leaves problems ``. `write-post-extras.block.md`: reuse the scratch dir `select_rules` made, if any, instead of making a new one. `write-report.block.md`: write `review-verdicts.json` next to the report json when a selector ran. `dispatch-the-review.block.md`: add the rule selection text, when a selector ran, to the payload list. `reviewer-shape.block.md`: when the payload carries a rule selection, the reply ends with the verdict set in the fenced block it names. +- [ ] **Step 5: Run the test, expect PASS.** Then compile to a scratch pack the way CI does (`editing-skills` "Edit or create the skill in its source path": copy `plugins/mattstack` and the two `tests/graph-*.jsonc` files to a scratch dir, `rt skills compile --pack-dir /pack --mattstack-dir /ms`) and run `python3 plugins/mattstack/plugin/skills/process-digraphs/check-dot.py --short-ids /pack/attachments/review/SKILL.md` (or wherever the review verb lands). Expected: no output, exit 0. Do the same against the bound fixture compile. +- [ ] **Step 6: Commit.** `git commit -m "skills: review selects rules before dispatch and turns failed rules into findings"`. + +--- + +### Task 3: The self-review graph + +**Files:** +- Modify: `plugins/mattstack/attachments/review/self-review/the-self-review-graph/index.block.md`, `.../self-review/run.block.md` +- Create in `.../the-self-review-graph/`: `select-rules.block.md`, `check-verdicts.block.md`, `repair-verdicts.block.md`, `fails-to-findings.block.md` +- Test: `lib/skills/__tests__/rule-selector.test.ts` + +**Interfaces:** +- Consumes: Task 1's section titles; Task 2's node ids and titles (same ids, same titles). + +- [ ] **Step 1: Failing test** `self-review graph places the rule steps`: edges `run_depth_checks -> select_rules`, `select_rules -> repro_browser`, `dispatch_reviewer -> check_verdicts`, `fails_to_findings -> assemble_draft`, the `verdict-rounds` budget node, and `make_recorded_move -> assemble_draft` unchanged. Run, expect FAIL. +- [ ] **Step 2: Edit the graph.** Same node declarations as Task 2. `run_depth_checks --> select_rules --> repro_browser` replaces `run_depth_checks --> repro_browser`. Replace `dispatch_reviewer --> assemble_draft` with the check loop of Task 2, whose `none, or no selector` and `yes: list them in the notes` edges go to `fails_to_findings`, then `fails_to_findings --> assemble_draft`. +- [ ] **Step 3: Node blocks** as in Task 2; the select block names the diff (`git diff ..HEAD` from `find_merge_base`, `` if the lint flags it). `run.block.md`: the budget row. +- [ ] **Step 4: Run the test, expect PASS;** check-dot on the compiled self-review, bound and unbound, as in Task 2. +- [ ] **Step 5: Commit.** `git commit -m "skills: self-review selects rules and turns failed rules into findings before the draft"`. + +--- + +### Task 4: receive-review selects rules for the adjudicator + +**Files:** +- Modify: `plugins/mattstack/attachments/review/receive-review/steps/index.block.md`, `.../steps/dispatch-adjudicator.block.md` +- Create: `.../steps/select-rules.block.md` +- Test: `lib/skills/__tests__/rule-selector.test.ts` + +- [ ] **Step 1: Failing test** `receive-review selects rules before the adjudicator`: edges `any_unresolved -> select_rules` (labelled `yes`), `select_rules -> dispatch_adjudicator`; no `check_verdicts` node in receive-review. Run, expect FAIL. +- [ ] **Step 2: Edit the graph.** `any_unresolved -->|yes| select_rules`, `select_rules --> dispatch_adjudicator`; the revise path that re-enters `dispatch_adjudicator` stays as is (selection reused, not re-run). Node block `select_rules` (title `Select the rules for the change`): the select section of the select body, run on the diff range the adjudicator is handed; no verdict request. +- [ ] **Step 3:** `dispatch-adjudicator.block.md`: the selection text, when a selector ran, goes to the adjudicator as reference for judging threads. +- [ ] **Step 4: Run the test, expect PASS;** check-dot on compiled receive-review, bound and unbound. +- [ ] **Step 5: Commit.** `git commit -m "skills: receive-review hands the adjudicator the selected rules"`. + +--- + +### Task 5: Docs, version and verification + +**Files:** +- Modify: `plugins/mattstack/plugin/skills/extending-a-pack/props.md` (prop table and the "what reviewers check" row), `plugins/mattstack/plugin/skills/extending-a-pack/SKILL.md` (fill `provides` lines), `plugins/mattstack/README.md` (contracts list), `website/docs/skills/packs.mdx` (if it lists review props), `plugins/mattstack/CERTIFICATION.md` (ledger row), `plugins/mattstack/.claude-plugin/plugin.json` (patch bump), the recompiled mattstack-pack output under `plugins/mattstack/skills/` + +- [ ] **Step 1: Docs.** A `rule-selector` row (merge replace, default empty, contract `rule-selector@1`, one line on what the fill resolves to); README and SKILL.md mention the contract beside `review-criteria@1`. +- [ ] **Step 2: Certify** every touched root: `sh plugins/mattstack/tests/certify.sh plugins/mattstack/attachments/review/review`, `.../review/self-review`, `.../review/receive-review`, `.../review-core-body-after`, `.../review-dispatch-body-after`, the two new bodies, and `plugins/mattstack/plugin/skills/extending-a-pack`. Expected: no `FAIL` lines. +- [ ] **Step 3: Bump and recompile.** Patch-bump `plugin.json`; `rt skills compile --pack mattstack --pack-dir plugins/mattstack` (from this worktree; never the `--pack`-only form); `rt skills check --pack-dir plugins/mattstack --strict`. Expected: check passes. +- [ ] **Step 4: Full checks.** `bun test lib/skills commands/__tests__/skills.test.ts commands/__tests__/skills-bind.test.ts scripts/ci/__tests__/block-graphs-compiled.test.ts`, then the CI-style compile plus `check-dot.py --short-ids` on every compiled SKILL.md in the scratch pack. Expected: all pass. +- [ ] **Step 5: Commit.** `git add plugins/mattstack website docs` then `git commit -m "mattstack: document the rule-selector slot; bump"`. + +The GREEN walkthrough (editing-skills) is the controller's: after Task 5, a fresh agent reads the bound compiled `review` SKILL.md and walks three scenarios (no binding; a selector that exits 1; a reply with no fenced block), reporting each step it would take. Review Focus 1 to 3 pass when it skips, quotes a setup observation and goes on, and makes exactly one repair then lists unanswered rules. diff --git a/docs/superpowers/specs/2026-10-28-review-rule-selector-design.md b/docs/superpowers/specs/2026-10-28-review-rule-selector-design.md new file mode 100644 index 0000000000..280aac585c --- /dev/null +++ b/docs/superpowers/specs/2026-10-28-review-rule-selector-design.md @@ -0,0 +1,111 @@ +# Review rule selector: design + +Status: approved for planning, 2026-10-28. + +## Why + +A pack can bind review criteria, but criteria are prose every reviewer reads +whatever the diff touches. Some teams keep their review rules as structured +files that a command can select for a given diff (only the rules whose scope +and conditions match, with the lines that triggered them) and can later check +a reviewer's answers against. The shared review engines gain an optional slot +for such a command, so a pack can hand the reviewer only the rules that apply, +get a verdict for each, and turn every failed rule into a finding. + +The engines stay tool-neutral: they know only the command lines a fill +resolves to and the JSON shapes below, never which tool sits behind them. + +## The slot + +A new prop `rule-selector` with contract `rule-selector@1`, `merge: replace`, +`default: ""`, declared in its own block in each of `review`, `self-review` +and `receive-review`. Unbound (the default), every step below is skipped and +the engines behave exactly as today. + +### rule-selector@1 + +A fill's text, followed at run time, resolves to one of: + +- the single line `selector: none`; or +- three lines: + - `selector: ` holding the placeholder `{format}`; + - `verdicts: ` holding `{selection}` and `{verdicts}`; + - `fence: `: the tag of the fenced code block the reviewer + returns its verdict set in. + +The fill may read settings or the environment to decide (a team switch that +turns rules off resolves to `selector: none`), so the resolution happens on +every run, not at compile time. + +**The selector.** Run twice, `{format}` replaced by `json` and then `text`, +each time with the change's unified diff on stdin; stdout is captured. The +JSON is the selection; the engine reads only `rules[]`, each with `id`, +`severity` (`blocking` or `advisory`) and `summary`. The text is Markdown for +the reviewer and goes into the dispatch as is; it asks for the verdict set. +A selector that exits non-zero, prints JSON the engine cannot read, or a fill +that resolves to neither shape is a setup observation, quoted as such, and the +review goes on without rules. It never blocks a review. + +**The verdict set** the reviewer returns, one fenced block tagged with the +`fence` value: `{ "verdicts": [{ "rule", "verdict": "pass" | "fail" | +"not-applicable", "evidence": [{ "file", "line", "side"?: "new" | "old" }], +"note" }] }`, other fields allowed. + +**The verdicts command** prints `{ "problems": [...] }` and exits 0 when there +are none, 1 when there are some; any other exit is a failure, treated as one +problem. + +## The steps + +### review and self-review + +1. **Select, before dispatch.** Resolve the fill; with a selector, run it + (json, then text). The JSON goes to `selection.json` in the review scratch + dir (made here, and reused by every later step that writes there), the text + goes into the dispatch payload, and the reviewer's shape asks it to end its + reply with the verdict set in the `fence` block. +2. **Check verdicts, after the reviewer returns.** Extract the fenced block to + `verdict-set.json` (a missing or unparseable block is one problem) and run + the verdicts command. Problems go to one verdict-only repair: a fresh + reviewer gets the selection text, the first verdict set and findings, and + the problems, and returns a corrected verdict set only. The first review's + findings stay; only the verdict set is replaced. The repair is charged to + the budget `verdict-rounds:`. Problems left after it are listed in + the draft's notes, the run goes on, and rules with no valid verdict are + reported as unanswered. +3. **Fails become findings.** Each `fail` becomes a finding titled with the + rule id and its summary, anchored at its first `new`-side evidence (an + `old`-side location goes in the body): a `blocking` rule's fail is + Important, an `advisory` rule's is Minor. Critical stays the reviewer's own + judgment. A fail the reviewer also reported as a finding at the same + location is merged into one. In review this runs after "Assemble the review + draft" and before "Verify each blocking finding", so the verify and demote + steps apply unchanged; in self-review it runs before "Assemble the + self-review draft". +4. **Save the verdict set** as `review-verdicts.json` in the review scratch + dir always, and next to the report json when a report path was handed in. + +### receive-review + +Only step 1, before the adjudicator is dispatched, with the diff of the +branch against its target; the selection text goes to the adjudicator as +reference for judging threads, with no verdict request. + +## Decisions + +- The `fence` line is part of the contract so the engine never hard-codes a + tool's block name. +- `verdict-rounds:` uses limit 2 with the engines' existing + charge-then-check pattern (charge on the problem event, repair while not + spent), which yields exactly one repair, as `verify-rounds:` limit 2 + yields one re-dispatch. +- The selector reads only the diff; it needs no checkout, so it runs on every + path into the reviewer dispatch, including the paths where provisioning the + head checkout failed. +- No new MCP tool or app capability: the engine floor (`minEngine`) stays. + +## Non-goals + +- Fan-out to one reviewer per rule. +- Changing what the generic review judges; rule verdicts come in addition. +- Shipping any selector. A pack's fill supplies the command. From 697acd7a72cd7048453f9c684553674c732690fb Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Wed, 28 Oct 2026 15:59:25 -0500 Subject: [PATCH 02/15] docs: rule selector plan, review round 1 Co-Authored-By: Claude Opus 5.5 --- .../plans/2026-10-28-review-rule-selector.md | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/docs/superpowers/plans/2026-10-28-review-rule-selector.md b/docs/superpowers/plans/2026-10-28-review-rule-selector.md index b5146d2124..fa5b7f9788 100644 --- a/docs/superpowers/plans/2026-10-28-review-rule-selector.md +++ b/docs/superpowers/plans/2026-10-28-review-rule-selector.md @@ -41,6 +41,7 @@ - Create: `plugins/mattstack/attachments/review-rules-select-body/index.block.md` and its section blocks - Create: `plugins/mattstack/attachments/review-rules-verdicts-body/index.block.md` and its section blocks - Create: `plugins/mattstack/attachments/review/review/rule-selector.block.md`, `.../self-review/rule-selector.block.md`, `.../receive-review/rule-selector.block.md` +- Create: `plugins/mattstack/attachments/review/review/rule-verdicts.block.md`, `.../self-review/rule-verdicts.block.md` (a block may end with only one headed include: `render.ts` raises `text-after-include` for anything after it) - Create: `lib/skills/__tests__/fixtures/rule-selector/` (a pack: `pack/stubs.jsonc` rostering `review`, `self-review`, `receive-review`; `pack/skills.jsonc` binding `mattstack:/rule-selector` → `acme:acme-rule-selector` for all three; `attachments/acme-rule-selector/SKILL.md` with `metadata.provides: "rule-selector@1"`) - Test: `lib/skills/__tests__/rule-selector.test.ts` @@ -51,9 +52,9 @@ - `bound: each review engine inlines the fill under the rule-selector prop`: for each of the three verbs, the compiled SKILL.md contains the fill's `fence: acme-verdicts` line and a seam marker containing `prop:rule-selector.rule-selector`. - `bound: review and self-review include both bodies, receive-review only the select body`: `## Select the rules` in all three; `## Failed rules become findings` in review and self-review only. - `unbound: no fill text, bodies still present`: compile the same fixture with the bindings removed; no `acme-verdicts`, and `## Select the rules` still present. - - `a fill providing another contract is refused`: a second fixture fill with `provides: "review-criteria@1"` bound to `rule-selector` makes the compile fail with code `contract-mismatch`. + - `a fill providing another contract is refused`: a second fixture fill with `provides: "review-criteria@1"` bound to `rule-selector` makes the compile fail; through `skillsCompile` the error is a `SkillsUsageError` whose text carries the message, so assert on `provides "review-criteria@1", but the prop's contract is "rule-selector@1"` (use `runExpectingCleanExit` from `lib/skills/__tests__/helpers.ts`, or assert the code `contract-mismatch` at the `compileBlockSkill` level as `compile-blocks.test.ts` does around line 724). - [ ] **Step 2: Run them, expect FAIL.** `bun test lib/skills/__tests__/rule-selector.test.ts` (from the repo root). -- [ ] **Step 3: Write the prop blocks and the bodies.** Each engine's `rule-selector.block.md` copies `criteria.block.md`'s shape: the prop frontmatter from Global Constraints, `order:` 9 in review, 8 in self-review, 9 in receive-review (each a free value just before the engine's `reviewer` or `reply-rules` block; renumber nothing), title `# Rule selector`, the marker, then `{{include:review-rules-select-body}}`, and for review and self-review `{{include:review-rules-verdicts-body}}`. The bodies carry the spec's sections as engine prose: the contract (the spec's rule-selector@1 text), select (resolve the fill on every run; with `selector: none` or no binding skip every rule step; run the selector with `json` then `text` on the diff on stdin; make the review scratch dir here if none exists yet and write `selection.json` there; the text goes in the dispatch payload with the request to end the reply with the verdict set in a block tagged with the `fence` value), failure (quote as a setup observation; go on with no rules), check (extract the block to `verdict-set.json`; zero or two or more blocks, or unparseable JSON, is one problem; run the verdicts command with `{selection}` and `{verdicts}` replaced by the two paths; exit 0 none, 1 problems, other exit one problem), repair (charged per the budget; a fresh reviewer gets the selection text, the first verdict set and findings, and the problems, and returns a corrected verdict set only; the first findings stay), fails to findings (title `: `, tiers per Global Constraints, anchor the first `new` evidence, `old` locations in the body, merge with a reviewer finding at the same file and line, rules with no valid verdict listed as unanswered in the draft's notes with the problems left), the verdict file (`review-verdicts.json` in the scratch dir always, next to the report json when a report path was handed in). +- [ ] **Step 3: Write the prop blocks and the bodies.** Each engine's `rule-selector.block.md` copies `criteria.block.md`'s shape: the prop frontmatter from Global Constraints, `order:` 9 in review, 8 in self-review, 9 in receive-review (each a free value just before the engine's `reviewer` or `reply-rules` block; renumber nothing), title `# Rule selector`, the marker, then `{{include:review-rules-select-body}}` as the block's last line. Review and self-review each get a second block, `rule-verdicts.block.md`, `order:` 9.5 in review and 8.5 in self-review (`order` is any number and sorts numerically), title `# Rule verdicts`, holding only `{{include:review-rules-verdicts-body}}`. The bodies carry the spec's sections as engine prose: the contract (the spec's rule-selector@1 text), select (resolve the fill on every run; with `selector: none` or no binding skip every rule step; run the selector with `json` then `text` on the diff on stdin; make the review scratch dir here if none exists yet and write `selection.json` there; the text goes in the dispatch payload with the request to end the reply with the verdict set in a block tagged with the `fence` value), failure (quote as a setup observation; go on with no rules), check (extract the block to `verdict-set.json`; zero or two or more blocks, or unparseable JSON, is one problem; run the verdicts command with `{selection}` and `{verdicts}` replaced by the two paths; exit 0 none, 1 problems, other exit one problem), repair (charged per the budget; a fresh reviewer gets the selection text, the first verdict set and findings, and the problems, and returns a corrected verdict set only; the first findings stay), fails to findings (title `: `, tiers per Global Constraints, anchor the first `new` evidence, `old` locations in the body, merge with a reviewer finding at the same file and line, rules with no valid verdict listed as unanswered in the draft's notes with the problems left), the verdict file (`review-verdicts.json` in the scratch dir always, next to the report json when a report path was handed in). - [ ] **Step 4: Run the tests, expect PASS;** also `bun test lib/skills/__tests__/browser-target.test.ts` stays green. - [ ] **Step 5: Certify** each new attachment root: `sh plugins/mattstack/tests/certify.sh plugins/mattstack/attachments/review-rules-select-body` and the same for `review-rules-verdicts-body`; the engine roots are certified in Task 5. - [ ] **Step 6: Commit.** `git add plugins/mattstack/attachments lib/skills/__tests__` then `git commit -m "skills: rule-selector slot and shared rule bodies for the review engines"`. @@ -72,7 +73,7 @@ - Consumes: Task 1's section titles. - Produces: node ids `select_rules`, `check_verdicts`, `verdict_problems`, `run_budget_verdict_rounds`, `verdict_rounds_spent`, `repair_verdicts`, `fails_to_findings` (Task 3 reuses the same ids in the self-review graph). -- [ ] **Step 1: Failing test** `review graph places the rule steps`: in the bound compile's review SKILL.md, the dot fence has edges `select_rules -> dispatch_reviewer`, `dispatch_reviewer -> check_verdicts`, `assemble_draft -> fails_to_findings`, `fails_to_findings -> verify_blocking`, no edge `dispatch_reviewer -> assemble_draft`, and the text contains `run_budget {runDb, key: verdict-rounds:, limit: 2, stage}`. Run it, expect FAIL. +- [ ] **Step 1: Failing test** `review graph places the rule steps`: in the bound compile's review SKILL.md, the dot fence has edges `select_rules -> dispatch_reviewer`, `dispatch_reviewer -> check_verdicts`, `assemble_draft -> fails_to_findings`, `fails_to_findings -> verify_blocking`, no edge `dispatch_reviewer -> assemble_draft`, the edge `redispatch_reviewer -> assemble_draft` still present (the verify re-dispatch bypasses the verdict loop), and the text contains `run_budget {runDb, key: verdict-rounds:, limit: 2, stage}`. Run it, expect FAIL. - [ ] **Step 2: Edit the graph.** Declare the nodes: - `select_rules` (block title `Select the rules for the change`); - `check_verdicts` (`Check the reviewer's verdict set`); @@ -102,7 +103,7 @@ - [ ] **Step 1: Failing test** `self-review graph places the rule steps`: edges `run_depth_checks -> select_rules`, `select_rules -> repro_browser`, `dispatch_reviewer -> check_verdicts`, `fails_to_findings -> assemble_draft`, the `verdict-rounds` budget node, and `make_recorded_move -> assemble_draft` unchanged. Run, expect FAIL. - [ ] **Step 2: Edit the graph.** Same node declarations as Task 2. `run_depth_checks --> select_rules --> repro_browser` replaces `run_depth_checks --> repro_browser`. Replace `dispatch_reviewer --> assemble_draft` with the check loop of Task 2, whose `none, or no selector` and `yes: list them in the notes` edges go to `fails_to_findings`, then `fails_to_findings --> assemble_draft`. -- [ ] **Step 3: Node blocks** as in Task 2; the select block names the diff (`git diff ..HEAD` from `find_merge_base`, `` if the lint flags it). `run.block.md`: the budget row. +- [ ] **Step 3: Node blocks** as in Task 2; the select block names the diff: the working-tree diff from the merge base, uncommitted changes included (`git diff `, the same diff `find_merge_base` and `dispatch_reviewer` describe; `` if the lint flags it), so the rules match what the reviewer judges. `run.block.md`: the budget row. - [ ] **Step 4: Run the test, expect PASS;** check-dot on the compiled self-review, bound and unbound, as in Task 2. - [ ] **Step 5: Commit.** `git commit -m "skills: self-review selects rules and turns failed rules into findings before the draft"`. @@ -126,10 +127,10 @@ ### Task 5: Docs, version and verification **Files:** -- Modify: `plugins/mattstack/plugin/skills/extending-a-pack/props.md` (prop table and the "what reviewers check" row), `plugins/mattstack/plugin/skills/extending-a-pack/SKILL.md` (fill `provides` lines), `plugins/mattstack/README.md` (contracts list), `website/docs/skills/packs.mdx` (if it lists review props), `plugins/mattstack/CERTIFICATION.md` (ledger row), `plugins/mattstack/.claude-plugin/plugin.json` (patch bump), the recompiled mattstack-pack output under `plugins/mattstack/skills/` +- Modify: `plugins/mattstack/plugin/skills/extending-a-pack/props.md` (prop table and the "what reviewers check" row), `plugins/mattstack/plugin/skills/extending-a-pack/SKILL.md` (fill `provides` lines), `plugins/mattstack/README.md` (contracts list), `website/docs/skills/packs.mdx` (if it lists review props), `plugins/mattstack/CERTIFICATION.md` (ledger row), `plugins/mattstack/.claude-plugin/plugin.json` (patch bump), the recompiled mattstack-pack output under `plugins/mattstack/skills/` (only the version in its compiled ids changes: the shipped roster compiles none of the review engines) - [ ] **Step 1: Docs.** A `rule-selector` row (merge replace, default empty, contract `rule-selector@1`, one line on what the fill resolves to); README and SKILL.md mention the contract beside `review-criteria@1`. -- [ ] **Step 2: Certify** every touched root: `sh plugins/mattstack/tests/certify.sh plugins/mattstack/attachments/review/review`, `.../review/self-review`, `.../review/receive-review`, `.../review-core-body-after`, `.../review-dispatch-body-after`, the two new bodies, and `plugins/mattstack/plugin/skills/extending-a-pack`. Expected: no `FAIL` lines. +- [ ] **Step 2: Certify** every touched root: `sh plugins/mattstack/tests/certify.sh plugins/mattstack/attachments/review/review` (which holds both new blocks), `.../review/self-review`, `.../review/receive-review`, `.../review-core-body-after`, `.../review-dispatch-body-after`, the two new bodies, and `plugins/mattstack/plugin/skills/extending-a-pack`. Expected: no `FAIL` lines. - [ ] **Step 3: Bump and recompile.** Patch-bump `plugin.json`; `rt skills compile --pack mattstack --pack-dir plugins/mattstack` (from this worktree; never the `--pack`-only form); `rt skills check --pack-dir plugins/mattstack --strict`. Expected: check passes. - [ ] **Step 4: Full checks.** `bun test lib/skills commands/__tests__/skills.test.ts commands/__tests__/skills-bind.test.ts scripts/ci/__tests__/block-graphs-compiled.test.ts`, then the CI-style compile plus `check-dot.py --short-ids` on every compiled SKILL.md in the scratch pack. Expected: all pass. - [ ] **Step 5: Commit.** `git add plugins/mattstack website docs` then `git commit -m "mattstack: document the rule-selector slot; bump"`. From 0fa34c695af173820604b25d848d94fccacfbea2 Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Wed, 28 Oct 2026 16:00:37 -0500 Subject: [PATCH 03/15] docs: rule selector plan approved by review Co-Authored-By: Claude Opus 5.5 --- docs/superpowers/plans/2026-10-28-review-rule-selector.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/superpowers/plans/2026-10-28-review-rule-selector.md b/docs/superpowers/plans/2026-10-28-review-rule-selector.md index fa5b7f9788..60ea9d0de2 100644 --- a/docs/superpowers/plans/2026-10-28-review-rule-selector.md +++ b/docs/superpowers/plans/2026-10-28-review-rule-selector.md @@ -86,7 +86,7 @@ Edges: every edge that today ends at `dispatch_reviewer` (the two `checks not run` edges, `repro_browser -->|no...|`, `capture_repro -->|...|`) ends at `select_rules` instead, with the same labels, and `select_rules --> dispatch_reviewer`; replace `dispatch_reviewer --> assemble_draft` with `dispatch_reviewer --> check_verdicts`, `check_verdicts --> verdict_problems`, `verdict_problems -->|none, or no selector| assemble_draft`, `verdict_problems -->|yes| run_budget_verdict_rounds`, `run_budget_verdict_rounds --> verdict_rounds_spent`, `verdict_rounds_spent -->|no| repair_verdicts`, `repair_verdicts --> check_verdicts`, `verdict_rounds_spent -->|yes: list them in the notes| assemble_draft`; replace `assemble_draft --> verify_blocking` with `assemble_draft --> fails_to_findings` and `fails_to_findings --> verify_blocking`. `redispatch_reviewer --> assemble_draft` stays. - [ ] **Step 3: Node blocks.** Each is a few lines that point to its Task 1 section by title and say what to skip with no selector (select: skip when unbound or `selector: none`; check: with no selector, `verdict_problems` takes `none`; fails: nothing to add with no selector, and on a pass after `redispatch_reviewer` the merge keeps one finding per rule). - [ ] **Step 4: Edit the existing blocks.** `run.block.md`: a row `` `verdict-rounds:` | 2 | a verdict check at that depth leaves problems ``. `write-post-extras.block.md`: reuse the scratch dir `select_rules` made, if any, instead of making a new one. `write-report.block.md`: write `review-verdicts.json` next to the report json when a selector ran. `dispatch-the-review.block.md`: add the rule selection text, when a selector ran, to the payload list. `reviewer-shape.block.md`: when the payload carries a rule selection, the reply ends with the verdict set in the fenced block it names. -- [ ] **Step 5: Run the test, expect PASS.** Then compile to a scratch pack the way CI does (`editing-skills` "Edit or create the skill in its source path": copy `plugins/mattstack` and the two `tests/graph-*.jsonc` files to a scratch dir, `rt skills compile --pack-dir /pack --mattstack-dir /ms`) and run `python3 plugins/mattstack/plugin/skills/process-digraphs/check-dot.py --short-ids /pack/attachments/review/SKILL.md` (or wherever the review verb lands). Expected: no output, exit 0. Do the same against the bound fixture compile. +- [ ] **Step 5: Run the test, expect PASS.** Then compile to a scratch pack the way CI does (`editing-skills` "Edit or create the skill in its source path": copy `plugins/mattstack` and the two `tests/graph-*.jsonc` files to a scratch dir, `rt skills compile --pack-dir /pack --mattstack-dir /ms`) and run `python3 plugins/mattstack/plugin/skills/process-digraphs/check-dot.py --short-ids /pack/skills/review/SKILL.md` (a rostered door lands under `skills/`; check `attachments/review/` if it is not there). Expected: no output, exit 0. Do the same against the bound fixture compile. - [ ] **Step 6: Commit.** `git commit -m "skills: review selects rules before dispatch and turns failed rules into findings"`. --- From f7f012112a03a93d415be161b1fe1caa40f599f7 Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Wed, 28 Oct 2026 16:07:00 -0500 Subject: [PATCH 04/15] skills: rule-selector slot and shared rule bodies for the review engines Co-Authored-By: Claude Opus 5.5 --- .../rule-selector/.claude-plugin/plugin.json | 1 + .../attachments/acme-rule-selector/SKILL.md | 16 +++ .../attachments/acme-wrong-contract/SKILL.md | 13 +++ .../fixtures/rule-selector/pack/skills.jsonc | 8 ++ .../fixtures/rule-selector/pack/stubs.jsonc | 7 ++ lib/skills/__tests__/rule-selector.test.ts | 107 ++++++++++++++++++ .../review-rules-select-body/index.block.md | 7 ++ .../select-the-rules.block.md | 27 +++++ .../the-contract.block.md | 35 ++++++ .../when-the-selector-fails.block.md | 15 +++ .../check-the-verdict-set.block.md | 17 +++ .../failed-rules-become-findings.block.md | 23 ++++ .../review-rules-verdicts-body/index.block.md | 7 ++ .../repair-the-verdict-set.block.md | 19 ++++ .../the-verdict-file.block.md | 10 ++ .../receive-review/rule-selector.block.md | 16 +++ .../review/review/rule-selector.block.md | 16 +++ .../review/review/rule-verdicts.block.md | 7 ++ .../review/self-review/rule-selector.block.md | 16 +++ .../review/self-review/rule-verdicts.block.md | 7 ++ 20 files changed, 374 insertions(+) create mode 100644 lib/skills/__tests__/fixtures/rule-selector/.claude-plugin/plugin.json create mode 100644 lib/skills/__tests__/fixtures/rule-selector/attachments/acme-rule-selector/SKILL.md create mode 100644 lib/skills/__tests__/fixtures/rule-selector/attachments/acme-wrong-contract/SKILL.md create mode 100644 lib/skills/__tests__/fixtures/rule-selector/pack/skills.jsonc create mode 100644 lib/skills/__tests__/fixtures/rule-selector/pack/stubs.jsonc create mode 100644 lib/skills/__tests__/rule-selector.test.ts create mode 100644 plugins/mattstack/attachments/review-rules-select-body/index.block.md create mode 100644 plugins/mattstack/attachments/review-rules-select-body/select-the-rules.block.md create mode 100644 plugins/mattstack/attachments/review-rules-select-body/the-contract.block.md create mode 100644 plugins/mattstack/attachments/review-rules-select-body/when-the-selector-fails.block.md create mode 100644 plugins/mattstack/attachments/review-rules-verdicts-body/check-the-verdict-set.block.md create mode 100644 plugins/mattstack/attachments/review-rules-verdicts-body/failed-rules-become-findings.block.md create mode 100644 plugins/mattstack/attachments/review-rules-verdicts-body/index.block.md create mode 100644 plugins/mattstack/attachments/review-rules-verdicts-body/repair-the-verdict-set.block.md create mode 100644 plugins/mattstack/attachments/review-rules-verdicts-body/the-verdict-file.block.md create mode 100644 plugins/mattstack/attachments/review/receive-review/rule-selector.block.md create mode 100644 plugins/mattstack/attachments/review/review/rule-selector.block.md create mode 100644 plugins/mattstack/attachments/review/review/rule-verdicts.block.md create mode 100644 plugins/mattstack/attachments/review/self-review/rule-selector.block.md create mode 100644 plugins/mattstack/attachments/review/self-review/rule-verdicts.block.md diff --git a/lib/skills/__tests__/fixtures/rule-selector/.claude-plugin/plugin.json b/lib/skills/__tests__/fixtures/rule-selector/.claude-plugin/plugin.json new file mode 100644 index 0000000000..a6061f4d69 --- /dev/null +++ b/lib/skills/__tests__/fixtures/rule-selector/.claude-plugin/plugin.json @@ -0,0 +1 @@ +{"name":"acme","version":"0.1.0"} diff --git a/lib/skills/__tests__/fixtures/rule-selector/attachments/acme-rule-selector/SKILL.md b/lib/skills/__tests__/fixtures/rule-selector/attachments/acme-rule-selector/SKILL.md new file mode 100644 index 0000000000..f4cfa7c24d --- /dev/null +++ b/lib/skills/__tests__/fixtures/rule-selector/attachments/acme-rule-selector/SKILL.md @@ -0,0 +1,16 @@ +--- +name: acme-rule-selector +description: "Selects the acme review rules that apply to a diff. Loaded by the review verbs; not for direct invocation." +disable-model-invocation: true +metadata: + provides: rule-selector@1 +--- + +# acme rule selector + +Resolve to `selector: none` when the `ACME_RULES` environment variable is +`off`. Otherwise resolve to these three lines: + +selector: acme-rules select --format {format} +verdicts: acme-rules verdicts --selection {selection} --verdicts {verdicts} +fence: acme-verdicts diff --git a/lib/skills/__tests__/fixtures/rule-selector/attachments/acme-wrong-contract/SKILL.md b/lib/skills/__tests__/fixtures/rule-selector/attachments/acme-wrong-contract/SKILL.md new file mode 100644 index 0000000000..22ed90310f --- /dev/null +++ b/lib/skills/__tests__/fixtures/rule-selector/attachments/acme-wrong-contract/SKILL.md @@ -0,0 +1,13 @@ +--- +name: acme-wrong-contract +description: "A fill that provides another contract. Loaded by the review verbs; not for direct invocation." +disable-model-invocation: true +metadata: + provides: review-criteria@1 +--- + +# acme criteria + +## Requirements + +The linked ticket's acceptance criteria. diff --git a/lib/skills/__tests__/fixtures/rule-selector/pack/skills.jsonc b/lib/skills/__tests__/fixtures/rule-selector/pack/skills.jsonc new file mode 100644 index 0000000000..4faeab9b3b --- /dev/null +++ b/lib/skills/__tests__/fixtures/rule-selector/pack/skills.jsonc @@ -0,0 +1,8 @@ +{ + "version": 2, + "bindings": { + "mattstack:review/rule-selector": { "rule-selector": "acme:acme-rule-selector" }, + "mattstack:self-review/rule-selector": { "rule-selector": "acme:acme-rule-selector" }, + "mattstack:receive-review/rule-selector": { "rule-selector": "acme:acme-rule-selector" } + } +} diff --git a/lib/skills/__tests__/fixtures/rule-selector/pack/stubs.jsonc b/lib/skills/__tests__/fixtures/rule-selector/pack/stubs.jsonc new file mode 100644 index 0000000000..7af15e9db4 --- /dev/null +++ b/lib/skills/__tests__/fixtures/rule-selector/pack/stubs.jsonc @@ -0,0 +1,7 @@ +{ + "verbs": { + "review": { "engine": "review", "description": "Review a teammate's MR" }, + "self-review": { "engine": "self-review", "description": "Self-review the current branch" }, + "receive-review": { "engine": "receive-review", "description": "Act on review feedback on your MR" } + } +} diff --git a/lib/skills/__tests__/rule-selector.test.ts b/lib/skills/__tests__/rule-selector.test.ts new file mode 100644 index 0000000000..d9e03705a1 --- /dev/null +++ b/lib/skills/__tests__/rule-selector.test.ts @@ -0,0 +1,107 @@ +import { afterAll, afterEach, beforeAll, describe, expect, test } from "bun:test"; +import { cpSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "fs"; +import { tmpdir } from "os"; +import { join, resolve } from "path"; +import { skillsCompile } from "../../../commands/skills.ts"; +import { setFieldContractSeverityForTest } from "../field-contract.ts"; +import { captureSkills, runExpectingCleanExit } from "./helpers.ts"; + +const FIX = join(import.meta.dir, "fixtures", "rule-selector"); +const REPO_PLUGIN = resolve(import.meta.dir, "..", "..", "..", "plugins", "mattstack"); +const ENGINES = ["review", "self-review", "receive-review"] as const; +const FILL_LINE = "fence: acme-verdicts"; +// A fill's seam names the engine's block and the prop it fills. +const seam = (verb: string) => ` worktree_provision worktree_provision --> provision_returned_path provision_returned_path -->|yes| tree_at_fetched_head - provision_returned_path -->|no: checks not run| dispatch_reviewer + provision_returned_path -->|no: checks not run| select_rules tree_at_fetched_head -->|yes| set_up_depth - tree_at_fetched_head -->|no: checks not run| dispatch_reviewer + tree_at_fetched_head -->|no: checks not run| select_rules set_up_depth --> repro_browser repro_browser -->|yes| capture_repro - repro_browser -->|no: read or verify depth, or the domain's own method| dispatch_reviewer - capture_repro -->|Board ready or Not drawn| dispatch_reviewer - dispatch_reviewer --> assemble_draft - assemble_draft --> verify_blocking + repro_browser -->|no: read or verify depth, or the domain's own method| select_rules + capture_repro -->|Board ready or Not drawn| select_rules + select_rules --> dispatch_reviewer + dispatch_reviewer --> check_verdicts + check_verdicts --> verdict_problems + verdict_problems -->|none, or no selector| assemble_draft + verdict_problems -->|yes| run_budget_verdict_rounds + run_budget_verdict_rounds --> verdict_rounds_spent + verdict_rounds_spent -->|no| repair_verdicts + repair_verdicts --> check_verdicts + verdict_rounds_spent -->|yes: list them in the notes| assemble_draft + assemble_draft --> fails_to_findings + fails_to_findings --> verify_blocking verify_blocking --> blocking_verified blocking_verified -->|yes, or none blocking| report_path_given blocking_verified -->|no| run_budget_verify_rounds diff --git a/plugins/mattstack/attachments/review/review/the-review-graph/repair-verdicts.block.md b/plugins/mattstack/attachments/review/review/the-review-graph/repair-verdicts.block.md new file mode 100644 index 0000000000..ef3114f506 --- /dev/null +++ b/plugins/mattstack/attachments/review/review/the-review-graph/repair-verdicts.block.md @@ -0,0 +1,8 @@ +--- +node: repair_verdicts +--- + +# Repair the reviewer's verdict set + +Rule verdicts' Repair the verdict set: one fresh verdict-only dispatch, then +back to the check. The first review's findings stay. diff --git a/plugins/mattstack/attachments/review/review/the-review-graph/select-rules.block.md b/plugins/mattstack/attachments/review/review/the-review-graph/select-rules.block.md new file mode 100644 index 0000000000..16c01a8a23 --- /dev/null +++ b/plugins/mattstack/attachments/review/review/the-review-graph/select-rules.block.md @@ -0,0 +1,10 @@ +--- +node: select_rules +--- + +# Select the rules for the change + +Rule selector's Select the rules, run once the diff is in hand. Every path to +the dispatch passes here, a failed head checkout included, since the +selector reads only the diff. With no selector bound, or `selector: none`, +skip it and go straight to the dispatch. diff --git a/plugins/mattstack/attachments/review/review/the-review-graph/write-post-extras.block.md b/plugins/mattstack/attachments/review/review/the-review-graph/write-post-extras.block.md index 90119ffaeb..15868b6bf9 100644 --- a/plugins/mattstack/attachments/review/review/the-review-graph/write-post-extras.block.md +++ b/plugins/mattstack/attachments/review/review/the-review-graph/write-post-extras.block.md @@ -4,7 +4,8 @@ node: write_post_extras # Write review-post.extras.json -Make one new directory in this session's scratchpad (the Claude Code temp +Use the review scratch dir `select_rules` made, when it made one; otherwise +make one new directory in this session's scratchpad (the Claude Code temp root, where `gate_open` reads and writes), never `mktemp -d`, and write its path out literally from then on. Write `/review-post.extras.json`: diff --git a/plugins/mattstack/attachments/review/review/the-review-graph/write-report.block.md b/plugins/mattstack/attachments/review/review/the-review-graph/write-report.block.md index cb28afec4c..c2ef00e752 100644 --- a/plugins/mattstack/attachments/review/review/the-review-graph/write-report.block.md +++ b/plugins/mattstack/attachments/review/review/the-review-graph/write-report.block.md @@ -29,5 +29,8 @@ here, before any gate opens. call to the author, or says approval is withheld pending the notes. The `request_changes` line says what must change before it merges. +When a selector ran, write `review-verdicts.json` (The verdict file) next to +the report json. + These drafts are the text that posts. Once a gate is built from the json, nothing rewrites it; an iterate drafts again only for the new gate. From 80a1624d65e05a5d9d97edbd4da693b41a04b3e9 Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Wed, 28 Oct 2026 16:16:47 -0500 Subject: [PATCH 07/15] skills: verdict request keys the reviewer shape, and the scratch verdict file has a node Co-Authored-By: Claude Opus 5.5 --- .../review-core-body-after/dispatch-the-review.block.md | 3 ++- .../review-dispatch-body-after/reviewer-shape.block.md | 4 ++-- .../review/review/the-review-graph/fails-to-findings.block.md | 4 +++- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/plugins/mattstack/attachments/review-core-body-after/dispatch-the-review.block.md b/plugins/mattstack/attachments/review-core-body-after/dispatch-the-review.block.md index 626e6ae0af..4cd2340d1c 100644 --- a/plugins/mattstack/attachments/review-core-body-after/dispatch-the-review.block.md +++ b/plugins/mattstack/attachments/review-core-body-after/dispatch-the-review.block.md @@ -12,7 +12,8 @@ it the full payload: - the commit range - the setup observations from Set up for the depth - the filled criteria addendum, when the pack binds criteria -- the rule selection text, when a selector ran +- the rule selection text, when a selector ran, and with it the request for + a verdict set in a block tagged with the `fence` value It owns the template, the subagent, and the standard blocks. Judgment forms there, not here: whoever sized this change carries assumptions about it, and diff --git a/plugins/mattstack/attachments/review-dispatch-body-after/reviewer-shape.block.md b/plugins/mattstack/attachments/review-dispatch-body-after/reviewer-shape.block.md index c253dc908d..5603c6b8a8 100644 --- a/plugins/mattstack/attachments/review-dispatch-body-after/reviewer-shape.block.md +++ b/plugins/mattstack/attachments/review-dispatch-body-after/reviewer-shape.block.md @@ -22,5 +22,5 @@ weak travels as a setup observation, not a rewritten checklist. Both branches return the same shape, stated in the dispatched prompt: **Strengths / Critical / Important / Minor / Assessment**. Callers assemble drafts out of those buckets, so a bound provider owes them too. When the -payload carries a rule selection, the reply ends with the verdict set in the -fenced block the selection names. +payload asks for a verdict set, the reply ends with it in the block tagged +with the `fence` value the payload names. diff --git a/plugins/mattstack/attachments/review/review/the-review-graph/fails-to-findings.block.md b/plugins/mattstack/attachments/review/review/the-review-graph/fails-to-findings.block.md index d7326cc80e..f766cd93e1 100644 --- a/plugins/mattstack/attachments/review/review/the-review-graph/fails-to-findings.block.md +++ b/plugins/mattstack/attachments/review/review/the-review-graph/fails-to-findings.block.md @@ -6,6 +6,8 @@ node: fails_to_findings Rule verdicts' Failed rules become findings, added to the assembled draft before verification, so the verify and demote steps treat them like any -other finding. With no selector there is nothing to add. After +other finding. The verdict set is final here, so also save it to the review +scratch dir as `review-verdicts.json` (The verdict file). With no selector +there is nothing to add or save. After `redispatch_reviewer` the draft is assembled again and the merge keeps one finding per rule. From a4f89b4d6a00b5d6fcd60a553cad425ac85695e5 Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Wed, 28 Oct 2026 16:20:19 -0500 Subject: [PATCH 08/15] skills: self-review selects rules and turns failed rules into findings before the draft Co-Authored-By: Claude Opus 5.5 --- lib/skills/__tests__/rule-selector.test.ts | 13 ++++++++++++ .../review/self-review/run.block.md | 1 + .../check-verdicts.block.md | 8 ++++++++ .../fails-to-findings.block.md | 11 ++++++++++ .../fix-finding.block.md | 3 ++- .../the-self-review-graph/index.block.md | 20 +++++++++++++++++-- .../repair-verdicts.block.md | 8 ++++++++ .../select-rules.block.md | 12 +++++++++++ 8 files changed, 73 insertions(+), 3 deletions(-) create mode 100644 plugins/mattstack/attachments/review/self-review/the-self-review-graph/check-verdicts.block.md create mode 100644 plugins/mattstack/attachments/review/self-review/the-self-review-graph/fails-to-findings.block.md create mode 100644 plugins/mattstack/attachments/review/self-review/the-self-review-graph/repair-verdicts.block.md create mode 100644 plugins/mattstack/attachments/review/self-review/the-self-review-graph/select-rules.block.md diff --git a/lib/skills/__tests__/rule-selector.test.ts b/lib/skills/__tests__/rule-selector.test.ts index 5f620bdc62..4e8b417742 100644 --- a/lib/skills/__tests__/rule-selector.test.ts +++ b/lib/skills/__tests__/rule-selector.test.ts @@ -97,6 +97,19 @@ describe("rule selector slot", () => { expect(body).toMatch(edge("redispatch_reviewer", "assemble_draft")); expect(body).toContain("run_budget {runDb, key: verdict-rounds:, limit: 2, stage}"); }); + + test("self-review graph places the rule steps", () => { + const body = compiled.verbs["self-review"]; + const edge = (a: string, b: string) => new RegExp(`\\b${a} -> ${b}\\b`); + expect(body).toMatch(edge("run_depth_checks", "select_rules")); + expect(body).toMatch(edge("select_rules", "repro_browser")); + expect(body).toMatch(edge("dispatch_reviewer", "check_verdicts")); + expect(body).toMatch(edge("fails_to_findings", "assemble_draft")); + expect(body).not.toMatch(edge("run_depth_checks", "repro_browser")); + expect(body).not.toMatch(edge("dispatch_reviewer", "assemble_draft")); + expect(body).toMatch(edge("make_recorded_move", "assemble_draft")); + expect(body).toContain("run_budget {runDb, key: verdict-rounds:, limit: 2, stage}"); + }); }); test("unbound: no fill text, bodies still present", async () => { diff --git a/plugins/mattstack/attachments/review/self-review/run.block.md b/plugins/mattstack/attachments/review/self-review/run.block.md index 3a27d21faa..2f8f0dfa2b 100644 --- a/plugins/mattstack/attachments/review/self-review/run.block.md +++ b/plugins/mattstack/attachments/review/self-review/run.block.md @@ -32,6 +32,7 @@ the counted event, and its ` spent?` diamond reads `exhausted`: | Key | Limit | Charged when | |---|---|---| | `fix::` | 3 | a fix attempt fails; `` is 1, then 2 after the finding's reopened gate | +| `verdict-rounds:` | 2 | a verdict check at that depth leaves problems | | `reopen:` | 2 | the finding's fix budget is spent: the first charge opens the reopened gate, the second leaves the finding open | diff --git a/plugins/mattstack/attachments/review/self-review/the-self-review-graph/check-verdicts.block.md b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/check-verdicts.block.md new file mode 100644 index 0000000000..1437be7a0c --- /dev/null +++ b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/check-verdicts.block.md @@ -0,0 +1,8 @@ +--- +node: check_verdicts +--- + +# Check the reviewer's verdict set + +Rule verdicts' Check the verdict set, on the reviewer's reply. With no +selector, `verdict_problems` takes `none`. diff --git a/plugins/mattstack/attachments/review/self-review/the-self-review-graph/fails-to-findings.block.md b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/fails-to-findings.block.md new file mode 100644 index 0000000000..e5e9449021 --- /dev/null +++ b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/fails-to-findings.block.md @@ -0,0 +1,11 @@ +--- +node: fails_to_findings +--- + +# Turn failed rules into findings + +Rule verdicts' Failed rules become findings, carried into the draft that +`assemble_draft` builds, so they sit in its Issues like any other finding. +The verdict set is final here, so also save it to the review scratch dir as +`review-verdicts.json` (The verdict file). With no selector there is +nothing to add or save. diff --git a/plugins/mattstack/attachments/review/self-review/the-self-review-graph/fix-finding.block.md b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/fix-finding.block.md index 5d24393c74..d1fd0e72d4 100644 --- a/plugins/mattstack/attachments/review/self-review/the-self-review-graph/fix-finding.block.md +++ b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/fix-finding.block.md @@ -5,7 +5,8 @@ node: fix_finding # Fix the finding and run the tests Before the first attempt on a finding, save a patch of every file its -attempts will touch, in a new directory inside this session's scratchpad +attempts will touch, in the review scratch dir `select_rules` made when it +made one, otherwise in a new directory inside this session's scratchpad (outside the repo), so they can be undone. Make the smallest fix, then run the finding's test and the depth's checks. diff --git a/plugins/mattstack/attachments/review/self-review/the-self-review-graph/index.block.md b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/index.block.md index cecaf39549..60aabf792e 100644 --- a/plugins/mattstack/attachments/review/self-review/the-self-review-graph/index.block.md +++ b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/index.block.md @@ -25,11 +25,18 @@ flowchart TB requirements_answer{Requirements source answer?} print_depth_block run_depth_checks + select_rules repro_browser{Self-review repro takes a browser screenshot?} capture_repro who_judges{Who forms the self-review judgment?} stop_no_inline_judging[STOP: never judge your own diff inline; dispatch the fresh reviewer]:::stop dispatch_reviewer + check_verdicts + verdict_problems{Verdict problems left?} + run_budget_verdict_rounds[/"run_budget {runDb, key: verdict-rounds:, limit: 2, stage}"/] + verdict_rounds_spent{verdict-rounds: spent?} + repair_verdicts + fails_to_findings open_off_script_gate off_script_answer{Self-review off-script answer?} make_recorded_move @@ -86,7 +93,8 @@ flowchart TB requirements_answer -->|a source picked| print_depth_block requirements_answer -->|hold| run_hold print_depth_block --> run_depth_checks - run_depth_checks --> repro_browser + run_depth_checks --> select_rules + select_rules --> repro_browser repro_browser -->|yes| capture_repro repro_browser -->|no: read or verify depth, or the domain's own method| who_judges capture_repro -->|Board ready or Not drawn| who_judges @@ -100,7 +108,15 @@ flowchart TB off_script_answer -->|hold| run_hold off_script_answer -->|hand back| own_run_close_abandoned make_recorded_move --> assemble_draft - dispatch_reviewer --> assemble_draft + dispatch_reviewer --> check_verdicts + check_verdicts --> verdict_problems + verdict_problems -->|none, or no selector| fails_to_findings + verdict_problems -->|yes| run_budget_verdict_rounds + run_budget_verdict_rounds --> verdict_rounds_spent + verdict_rounds_spent -->|no| repair_verdicts + repair_verdicts --> check_verdicts + verdict_rounds_spent -->|yes: list them in the notes| fails_to_findings + fails_to_findings --> assemble_draft assemble_draft --> gate_fix_and_next gate_fix_and_next --> next_answer next_answer -->|proceed| fix_answer diff --git a/plugins/mattstack/attachments/review/self-review/the-self-review-graph/repair-verdicts.block.md b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/repair-verdicts.block.md new file mode 100644 index 0000000000..ef3114f506 --- /dev/null +++ b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/repair-verdicts.block.md @@ -0,0 +1,8 @@ +--- +node: repair_verdicts +--- + +# Repair the reviewer's verdict set + +Rule verdicts' Repair the verdict set: one fresh verdict-only dispatch, then +back to the check. The first review's findings stay. diff --git a/plugins/mattstack/attachments/review/self-review/the-self-review-graph/select-rules.block.md b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/select-rules.block.md new file mode 100644 index 0000000000..08d917b5d5 --- /dev/null +++ b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/select-rules.block.md @@ -0,0 +1,12 @@ +--- +node: select_rules +--- + +# Select the rules for the change + +Rule selector's Select the rules, run once the depth checks are done. The +diff it gets is the diff the reviewer judges: the working-tree diff from +the merge base, uncommitted changes included (`git diff `, the +same diff `find_merge_base` and `dispatch_reviewer` describe), so the rules +match what the reviewer sees. With no selector bound, or `selector: none`, +skip it and go on to the repro check. From 9616959f0f229bad39ebc2bacbe4a049c8499809 Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Wed, 28 Oct 2026 16:24:00 -0500 Subject: [PATCH 09/15] skills: receive-review hands the adjudicator the selected rules Co-Authored-By: Claude Opus 5.5 --- lib/skills/__tests__/rule-selector.test.ts | 7 +++++++ .../receive-review/steps/build-plan-open.block.md | 5 +++-- .../steps/dispatch-adjudicator.block.md | 5 +++++ .../review/receive-review/steps/index.block.md | 4 +++- .../receive-review/steps/select-rules.block.md | 13 +++++++++++++ 5 files changed, 31 insertions(+), 3 deletions(-) create mode 100644 plugins/mattstack/attachments/review/receive-review/steps/select-rules.block.md diff --git a/lib/skills/__tests__/rule-selector.test.ts b/lib/skills/__tests__/rule-selector.test.ts index 4e8b417742..cbe4c08ab1 100644 --- a/lib/skills/__tests__/rule-selector.test.ts +++ b/lib/skills/__tests__/rule-selector.test.ts @@ -110,6 +110,13 @@ describe("rule selector slot", () => { expect(body).toMatch(edge("make_recorded_move", "assemble_draft")); expect(body).toContain("run_budget {runDb, key: verdict-rounds:, limit: 2, stage}"); }); + test("receive-review selects rules before the adjudicator", () => { + const body = compiled.verbs["receive-review"]; + expect(body).toMatch(/\bany_unresolved -> select_rules\b/); + expect(body).toMatch(/\bselect_rules -> dispatch_adjudicator\b/); + expect(body).not.toMatch(/\bany_unresolved -> dispatch_adjudicator\b/); + expect(body).not.toMatch(/\bcheck_verdicts\b/); + }); }); test("unbound: no fill text, bodies still present", async () => { diff --git a/plugins/mattstack/attachments/review/receive-review/steps/build-plan-open.block.md b/plugins/mattstack/attachments/review/receive-review/steps/build-plan-open.block.md index 5accb5ae52..fb5d461279 100644 --- a/plugins/mattstack/attachments/review/receive-review/steps/build-plan-open.block.md +++ b/plugins/mattstack/attachments/review/receive-review/steps/build-plan-open.block.md @@ -12,8 +12,9 @@ from the verdict report. The gate (`respond-plan`, gate 1; `respond-post` is gate 2) carries structured context per gate-protocol's `## Structured context (gate-ctx@1)` and its sidecar -`structured-context.txt`. Make one scratch -directory, ``, inside this session's scratchpad (the Claude Code temp +`structured-context.txt`. Reuse the scratch +directory Select the rules made as ``; with none, make one inside +this session's scratchpad (the Claude Code temp root, the only place `gate_open` reads `sourcePath` and writes `outPath`), never `mktemp -d`, and spell its path out literally from then on. Write `/respond-plan.source.json`: diff --git a/plugins/mattstack/attachments/review/receive-review/steps/dispatch-adjudicator.block.md b/plugins/mattstack/attachments/review/receive-review/steps/dispatch-adjudicator.block.md index 0e51f4dc4e..b1b30237c3 100644 --- a/plugins/mattstack/attachments/review/receive-review/steps/dispatch-adjudicator.block.md +++ b/plugins/mattstack/attachments/review/receive-review/steps/dispatch-adjudicator.block.md @@ -20,3 +20,8 @@ comments are adjudicated, not a final gut-check. Hand the dispatch flow (below, after the Off-script gate section) the numbered threads (`file:line` plus note chains), the requirements and the diff range; it owns the template, the subagent and the standard blocks. + +When a selector ran, add its `text` selection to the payload as reference +material for judging the threads, under its own heading. It carries no +verdict request: the adjudicator's reply keeps the shape it has without a +selector. With no selection, the payload is the one above. diff --git a/plugins/mattstack/attachments/review/receive-review/steps/index.block.md b/plugins/mattstack/attachments/review/receive-review/steps/index.block.md index 2390d2b71d..a1d95e6fb5 100644 --- a/plugins/mattstack/attachments/review/receive-review/steps/index.block.md +++ b/plugins/mattstack/attachments/review/receive-review/steps/index.block.md @@ -47,6 +47,7 @@ flowchart TB read_pr_threads_gh keep_unresolved_human any_unresolved{Any unresolved human threads?} + select_rules dispatch_adjudicator draft_verdict_table build_plan_open @@ -207,7 +208,8 @@ flowchart TB mr_threads_unresolved -->|an error: fail, quoting it| own_run_failed_or_abandoned read_pr_threads_gh --> keep_unresolved_human keep_unresolved_human --> any_unresolved - any_unresolved -->|yes| dispatch_adjudicator + any_unresolved -->|yes| select_rules + select_rules --> dispatch_adjudicator any_unresolved -->|none: say so| report_outcome dispatch_adjudicator --> draft_verdict_table draft_verdict_table --> build_plan_open diff --git a/plugins/mattstack/attachments/review/receive-review/steps/select-rules.block.md b/plugins/mattstack/attachments/review/receive-review/steps/select-rules.block.md new file mode 100644 index 0000000000..c7b2dc51d5 --- /dev/null +++ b/plugins/mattstack/attachments/review/receive-review/steps/select-rules.block.md @@ -0,0 +1,13 @@ +--- +node: select_rules +--- + +# Select the rules for the change + +Rule selector's Select the rules, run once the unresolved threads are in +hand. The diff it gets is the diff range the adjudicator is handed. The +scratch dir it makes is the one every later step that writes a file for +this run reuses. With no selector bound, or `selector: none`, skip it and +go straight to the dispatch. The selection only ever reaches the adjudicator +as reference: this verb asks for no verdict set and runs no later rule step. +A revise that dispatches again reuses this selection and does not run it again. From e4f1c19f42fbe2c5bdbbb70729ee1e901a25c0d6 Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Wed, 28 Oct 2026 16:28:30 -0500 Subject: [PATCH 10/15] mattstack: document the rule-selector slot; bump Co-Authored-By: Claude Opus 5.5 --- plugins/mattstack/.claude-plugin/plugin.json | 2 +- plugins/mattstack/CERTIFICATION.md | 1 + plugins/mattstack/README.md | 4 +- .../plugin/skills/extending-a-pack/SKILL.md | 2 +- .../plugin/skills/extending-a-pack/props.md | 6 ++ plugins/mattstack/skills/shepherdr/SKILL.md | 90 +++++++++---------- .../mattstack/skills/shepherdr/compiled.json | 4 +- plugins/mattstack/skills/wrap-up/SKILL.md | 4 +- .../mattstack/skills/wrap-up/compiled.json | 4 +- .../writing-style-conversational/SKILL.md | 24 ++--- .../compiled.json | 4 +- .../skills/writing-style-sparse/SKILL.md | 24 ++--- .../skills/writing-style-sparse/compiled.json | 4 +- .../skills/writing-style-structured/SKILL.md | 24 ++--- .../writing-style-structured/compiled.json | 4 +- 15 files changed, 104 insertions(+), 97 deletions(-) diff --git a/plugins/mattstack/.claude-plugin/plugin.json b/plugins/mattstack/.claude-plugin/plugin.json index 327ad811d3..d0ff6b0fb9 100644 --- a/plugins/mattstack/.claude-plugin/plugin.json +++ b/plugins/mattstack/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "mattstack", - "version": "0.32.29", + "version": "0.32.30", "description": "The mattstack skills pack: public doors subagent-review-loop, model-tiering, editing-skills, creating-a-pack, extending-a-pack, getting-current-time, process-digraphs, work-progress, wrap-up, the pack's own compiled shepherdr verb (domain-unbound), and the three writing-style presets (sparse, conversational, structured), plus the compile-native engines (work orchestrator, pipeline stages, review verbs, forge adapters, shepherdr) and attachments (execution strategies, account pool, review include bodies, review-posting, parameterized-skill primitive) that rt skills compile inlines into team verbs at compile time -- reached by name, not by slash menu. Also ships an mcp tools reference read by path, not inlined.", "author": { "name": "Matthew Goodwin" diff --git a/plugins/mattstack/CERTIFICATION.md b/plugins/mattstack/CERTIFICATION.md index fd1bac882b..3d01a5dd84 100644 --- a/plugins/mattstack/CERTIFICATION.md +++ b/plugins/mattstack/CERTIFICATION.md @@ -244,5 +244,6 @@ that quotes a shell form carries ``. | 2026-10-20 | mattstack:review, mattstack:review-posting, mattstack:review-core-body-tail | pure | pass | n/a (descriptions unchanged) | the summary line follows the verdict: the review verb drafts `summary.comments`, one line per `outcome` option value the gate offers (`comment`, `approve`, and `request_changes` only on GitHub), only the `approve` line carrying an all-clear; `editable` needs a `comment` on every finding and exactly those keys; Compose posts the `outcome` answer's `text` (the caller's `summaryText`), else `summary.comments[]`, the join unchanged; review-posting and the structured-findings field docs name `summary.comments`. RED 4/15 on V1 to V5 (3 reps each; every miss took the no-drafts branch and composed the summary, some with an all-clear), D2 0/3 (singular `summary.comment` with "Nothing here blocks the merge" for every verdict); GREEN 5/5 on V1 to V5, D1 and D2, G1 and G2 5/5, G3 9/10 (`docs/superpowers/tests/2026-10-20-review-drafted-comments/red-green.md`); certify exit 0 on all three | | 2026-10-23 | mattstack:work-progress | pure | pass | 5/5 scenarios, 5/5 haiku reps each (2 herd kept, epic and 2 plan added); full roster 22/23, the one miss (billing creating-a-pack) also misses on the old description | replaces the herd-only progress skill: the bundled script renders a herd (`--herd`) or plans and epics (`--plan` repeatable, `--title`, `--tree`) from rt's plan records; the skill picks the input, prints the view verbatim and never calls a plan done unless its row says so; RED: no skill loaded on plan or epic prompts, 6 hand calls and a hand-built table, plan 1 guessed done from plan 2 having started; GREEN (claude -p --plugin-dir, opus, sandboxed rt): epic prompt Skill + ls + 1 script call, plan "where is my plan at" and "how far along is it" Skill + 1 script call each, all printed verbatim, "herd progress" triggers it; the brief's description first missed "where is my plan at", fixed in 15e6d8f by adding that phrase; certify exit 0 | | 2026-10-23 | mattstack:shepherdr | pure | pass | n/a (description unchanged) | Progress pointer renamed to mattstack:work-progress, recompiled into skills/shepherdr; certify exit 0 | +| 2026-10-28 | mattstack:review, mattstack:self-review, mattstack:receive-review | pure | pass | n/a (descriptions unchanged) | optional `rule-selector` prop (`rule-selector@1`, replace, default empty): the engines select written rules before dispatch, ask the reviewer for one verdict per rule and turn failed rules into findings; unbound it is a no-op; certify exit 0 on every touched root | (Ledger rows are appended by the orchestrating session as each lane lands -- lanes themselves never edit this file.) diff --git a/plugins/mattstack/README.md b/plugins/mattstack/README.md index 875241ccbf..d5a1f8662e 100644 --- a/plugins/mattstack/README.md +++ b/plugins/mattstack/README.md @@ -193,8 +193,8 @@ The review cluster: a shared engine plus the protocols around it. `review-posting` and `subagent-review-loop` are model-visible; `review`, `self-review`, and `receive-review` are hidden via `disable-model-invocation` and reached only as a pack's compiled verbs. The review skills plug into -domain packs via the `review-criteria@1`, `reviewer-dispatch@1`, and -`reply-rules@1` contracts, while `subagent-review-loop` is a standalone +domain packs via the `review-criteria@1`, `reviewer-dispatch@1`, +`rule-selector@1`, and `reply-rules@1` contracts, while `subagent-review-loop` is a standalone one-off with no props. - The review flow lives in five internal include bodies diff --git a/plugins/mattstack/plugin/skills/extending-a-pack/SKILL.md b/plugins/mattstack/plugin/skills/extending-a-pack/SKILL.md index 8d55375b12..9fdcb032d1 100644 --- a/plugins/mattstack/plugin/skills/extending-a-pack/SKILL.md +++ b/plugins/mattstack/plugin/skills/extending-a-pack/SKILL.md @@ -384,7 +384,7 @@ metadata: ``` `provides` is the prop's contract when it has one (`criteria` takes -`review-criteria@1`); a `rules` prop has none, so any `provides` binds. +`review-criteria@1`, `rule-selector` takes `rule-selector@1`); a `rules` prop has none, so any `provides` binds. The body: the rule, its reason, and the decision it changes. The stage keeps its own flow; the fill carries only what the team adds. diff --git a/plugins/mattstack/plugin/skills/extending-a-pack/props.md b/plugins/mattstack/plugin/skills/extending-a-pack/props.md index 8606fd5f2c..06267d0e82 100644 --- a/plugins/mattstack/plugin/skills/extending-a-pack/props.md +++ b/plugins/mattstack/plugin/skills/extending-a-pack/props.md @@ -74,6 +74,7 @@ block. One fill may bind to any number of blocks. | MR preflight, fast checks, commit shape, description shape, labels | `stage-ship` and `ship`: the step blocks below, `domain-rules` | | CI job names, retry rules, triage | `stage-watch-ci` and `watch-ci`: `classify-triage-report`, `domain-rules`, `forge` | | what reviewers check | `review`, `self-review`, `receive-review`: `criteria` | +| which written rules a review is checked against, one verdict per rule | `review`, `self-review`, `receive-review`: `rule-selector` | | how to answer review threads | `receive-review`: `reply-rules` | | fan-out rules for parallel agents | `shepherdr`: the root's `rules`, or a step block below | @@ -93,11 +94,16 @@ default. The other props: | `work/sub-agent-tiering` | `tiering` | replace | `model-tiering@1` | nothing | | `review/criteria`, `self-review/criteria`, `receive-review/criteria` | `criteria` | replace | `review-criteria@1` | nothing | | `review/reviewer`, `self-review/reviewer`, `receive-review/reviewer` | `reviewer` | replace | `reviewer-dispatch@1` | nothing | +| `review/rule-selector`, `self-review/rule-selector`, `receive-review/rule-selector` | `rule-selector` | replace | `rule-selector@1` | empty (no rule selection) | | `receive-review/reply-rules` | `reply-rules` | replace | `reply-rules@1` | nothing | | `shepherdr` | `tiering` | replace | `model-tiering@1` | `mattstack:model-tiering` | | `shepherdr` | `strategy` | replace | `execution-strategy@1` | `mattstack:execution-strategy` | | `shepherdr` | `accounts` | replace | `account-pool@1` | `mattstack:cswap-accounts` | +A `rule-selector` fill resolves to `selector: none`, or three lines: `selector:` +(a command with `{format}`), `verdicts:` (a command with `{selection}` and +`{verdicts}`) and `fence:` (the info string of the reviewer's verdict block). + A `shepherdr` door compiles with nothing bound; bind one of its three replace props only to swap in the pack's own fill. diff --git a/plugins/mattstack/skills/shepherdr/SKILL.md b/plugins/mattstack/skills/shepherdr/SKILL.md index d7885a0812..f770b1d4e1 100644 --- a/plugins/mattstack/skills/shepherdr/SKILL.md +++ b/plugins/mattstack/skills/shepherdr/SKILL.md @@ -4,14 +4,14 @@ description: "Use when fanning work out across parallel Claude Code agents in he allowed-tools: - "Bash(*/scripts/pick-account.py:*)" metadata: - compiled: "mattstack@0.32.29 + mattstack:model-tiering@0.32.29 + mattstack:execution-strategy@0.32.29 + mattstack:cswap-accounts@0.32.29" + compiled: "mattstack@0.32.30 + mattstack:model-tiering@0.32.30 + mattstack:execution-strategy@0.32.30 + mattstack:cswap-accounts@0.32.30" --- -Pointers here are `@keys`. Resolve them once per run: if you already hold a `skills_resolve` map that covers `mattstack@0.32.29:shepherdr`, use it; otherwise call `skills_resolve {id: "mattstack@0.32.29:shepherdr"}` and use the path it returns for each key, in anything you hand a subagent or a tool too. If it fails, stop and say so; never look for the file another way. +Pointers here are `@keys`. Resolve them once per run: if you already hold a `skills_resolve` map that covers `mattstack@0.32.30:shepherdr`, use it; otherwise call `skills_resolve {id: "mattstack@0.32.30:shepherdr"}` and use the path it returns for each key, in anything you hand a subagent or a tool too. If it fails, stop and say so; never look for the file another way. - + # shepherdr You are the shepherd: a thin delegator, not a reviewer. You break work into jobs, spawn an agent per job (own herdr pane, own git worktree), present what the daemon pushes, and route small messages between the user and the herd. Model, method and account are policy, answered by the bound skills below; you are transport. @@ -28,15 +28,15 @@ For herdr CLI mechanics, load the `herdr` skill. *If a rule below asks for a move this graph marks STOP, take the off-script edge instead.* - -For tiering, invoke the `mattstack:model-tiering` skill (mattstack:model-tiering@0.32.29) when this flow needs it. + +For tiering, invoke the `mattstack:model-tiering` skill (mattstack:model-tiering@0.32.30) when this flow needs it. - + ## Strategy *If a rule below asks for a move this graph marks STOP, take the off-script edge instead.* - + ### Execution Strategy Given a unit of work and the surface it will execute on, name the method @@ -127,7 +127,7 @@ for you -- name the strategy and pass it, nothing more. Either way, do not compose method prose per job; the bodies carry the worker-boundary rules and the report contract. - + ## Domain rules Domain rules are optional: a pack binds them here or at a step. Bound @@ -141,7 +141,7 @@ names keeps this skill's default as written. *A domain rule sits here or in its step's own section; if one asks for a move this graph marks STOP, take the off-script edge instead.* - + ## Accounts *If a rule below asks for a move this graph marks STOP, take the off-script edge instead.* @@ -153,7 +153,7 @@ question, the per-spawn pick (passed as `account` on `herd_spawn` or `herd_respawn`), and the exhaustion decision tree. Accounts in use are each job's `account` in `herd_status`, never memory. - + ### cswap account pool Given the herd's model mix and the accounts already assigned this run, @@ -222,7 +222,7 @@ cache is a shared symlink), so a worker pane missing a plugin's tools is a real failure: check the pane's plugin list and the shared cache instead of dismissing it. - + ## Start and spawn Walk this graph from the trigger; a move it does not show goes to the shepherd off-script gate. @@ -318,7 +318,7 @@ digraph shepherdr_start { } ``` - + ### Specify the jobs **Domain hook -- intake.** Unbound: the jobs are what the user hands you; @@ -348,7 +348,7 @@ keep it out of the user's checkout, and the delegator rules then do not apply. Apply the bound rules' single-worker exception if they name one. A later job for a live herd is never this case. - + ### Ask the strategy and model per job Strategy predicts the work shape, and the shape predicts the tier: one @@ -375,13 +375,13 @@ the strategy (its method skill is the brief's Method); the recommendation starts at the floor, the pin is a fixed half, and a spawn below the floor is wrong. - + ### Ask the account pool question Ask it once per herd, after the models are chosen: per-model pools hide headroom until the model mix is known. A later job for a live herd keeps the herd's pool answer. The Accounts section says when there is no question to ask. - + ### Collect the repo conventions A herd pane is a real `claude` session in a git worktree: user-level @@ -404,7 +404,7 @@ rules' method skill owns the conventions, `Repo conventions` names only the branch; a brief that restates gates, process or evidence competes with the worker's skill chain. - + ### Fix what the refusal names - **An unfilled marker**: the refusal lists the leftover slots; add a `fill` entry for each. @@ -414,7 +414,7 @@ worker's skill chain. Never copy the template or the strategies file elsewhere to get past a refusal; the retry budget hands a stubborn one to the off-script gate. - + ### Shepherd off-script gate: ask the user Every unresolved refusal and spent budget, in all three graphs, lands @@ -435,13 +435,13 @@ never a count from memory. A same-turn retry count kept in your head, since no daemon budget covers it; it answers yes once two retries have run. - + ### Make the recorded move once Make exactly the move the user took, once, as its value spells it. If it fails too, that is a new off-script gate, never a retry of your own. - + ## What the start graph cannot show **The herd contract.** Every herd is an rt daemon registry row plus one @@ -518,7 +518,7 @@ off-script gate. - **Stagger 4+ spawns**: spawn one, confirm it returned, spawn the next. - Agents never work in the user's checkout; skip isolation only for read-only jobs or when the user says to work in place. The worker's directory must be a **linked worktree**, which `herd_spawn` guarantees. - + ## The watch loop After the spawns, do nothing until something arrives. @@ -687,7 +687,7 @@ digraph shepherdr_watch { } ``` - + ### End the turn until something arrives The daemon pushes gates and room lines; nothing arms, and you never read a @@ -695,7 +695,7 @@ pane or scrollback. Your room posts deliver as `@here` and wake every worker, so a question for one worker is a DM. - + ### Present the open gates Present up to 4 open gates in one AskUserQuestion call. Options are the @@ -719,36 +719,36 @@ with `writeIn`, else ask them to pick. When the user parks a gate, from the job's `herd_status` row: `lastGate`, with `lastGateStatus: parked`; relay it verbatim. - + ### Say which answer won, in one line Name `winner.answers` and `winner.by`; never re-ask. - + ### Fix the answers: each value verbatim Resend each value from `invalid[].validValues`, verbatim. - + ### Pick the account per the accounts rules The Accounts section decides whether to respawn now or ask first; its pick is the `account` on `herd_respawn`. Announce the respawn. - + ### Report the stall to the user An expired login with no smart-distribute pool: only the user's `/login` fixes it. Name the job, its pane and the verdict. - + ### Ask: attend or respawn A dialog the watchdog could not clear parks the job (`stuck-at-modal`). **Attend** opens the pane for the user; **Respawn** reuses the stored brief. - + ### Point the user at the job's tab The job's pane in `herd_status` has no `bg:` prefix: the herd is visible, @@ -757,20 +757,20 @@ which opens tabs only for a hidden herd's panes, would refuse. Tell the user in one message which job, its pane ref and tab, and what waits on them (the diagnosis reason, or the parked dialog). - + ### Report the crash with its job and pane The pane died with the job live. Tell the user both; never respawn silently. - + ### Ask: let them finish, kill and respawn, or hold One sentence naming the running agents, then the bare three options: **Let them finish** (recommended) / **Kill and respawn with the new briefs** / **Hold**. No restated heading, no per-option descriptions. - + ## What the watch loop cannot show - **The push is transport.** A gate push is one line, `[gate] is now open; re-read the gate registry.`, carrying only an id. Its arrival proves the daemon, the subscription and the herd; the `herd_gates` read is the verification, made first. `herd_gates` defaults to `HERD_ID` or the sole active herd and also returns open pipeline-run gates in a herd job's worktree. Room lines arrive the same way (` #: ` a report, ` #: milestone: ` a milestone). A stale id or an unrelated room line is normal, never a reason to reach for another command or distrust the push. @@ -800,7 +800,7 @@ brief unless `brief` is new, and keeps the job's model, effort and account unless passed. The old session's open and parked gates close as abandoned so the new worker re-asks; answered ones are marked consumed. - + ## Lanes and wrap-up ```dot @@ -920,13 +920,13 @@ digraph shepherdr_lanes { } ``` - + ### Flag the drift or collision to the user Files outside the job's write fence are drift; a file two jobs changed is a collision. Flag it now, not at integration. - + ### Relay only what you measured A worker's claims about its environment (servers up, ports free, CI green) @@ -934,7 +934,7 @@ are routinely stale. Measure first (`rt_verb {args: ["endpoint", "lookup", "", "--path", ""]}`, `curl`, `lsof`, a CI status call), or attribute it: "the worker reports X". - + ### Ask the integration job's strategy and model One AskUserQuestion like every job's: the tier table's integration row is @@ -942,14 +942,14 @@ the recommendation, the strategy half from the strategy table. Keep the herd's pool answer and pass the Accounts section's per-spawn pick as `account`. - + ### Gate: merge this lane? Only when bound domain rules give the shepherd the merge. One sentence with the PR or MR link, its state and its CI, then **Merge** / **Not yet** / **Hold**. - + ### Flip the lane's ticket, when it has one Flip it just before `herd_close`, for each lane an integration report @@ -958,7 +958,7 @@ merges. Only you close a done job's pane. The watchdog nags past skipped; for a lane awaiting the integration job or a merge gate it is normal, so end the turn. - + ### Show the status table Reformat `herd_status {herd}`, flagging failures and the latest @@ -970,7 +970,7 @@ Reformat `herd_status {herd}`, flagging failures and the latest | api tests | w3:p1 | 2 | direct-tdd | done | A1-A4 done, 12 tests, suite green | ``` - + ### Gate wrap-up: the form contract One form, per the wrap-up form contract below: @@ -984,7 +984,7 @@ One form, per the wrap-up form contract below: Never auto-remove a tree or job dir; `herd_wrap_up` executes exactly the form's answer. - + ### Measure what still runs The stop calls cover every job in the dispose list or with a closing pane: @@ -1005,13 +1005,13 @@ Measure before you say a job's servers stopped (`lsof -ti tcp:` or the endpoint lookup). Report a survivor as still running, never kill it by hand, and anything unmeasured as "not verified". - + ### Report the refusal in the guard's words Quote the disposal refusal as the guard gave it. In hidden mode, also offer the Bash command `rt herd stop --hidden` (no tool runs it); never run it unprompted. - + ## What the lanes graph cannot show - **The two objective checks.** `herd_lanes {herd}` returns a row per active job (`commits`; `drift.files`, committed and uncommitted work since the merge base; `collidesWith`) plus herd-wide `collisions`. Compare each job's `drift.files` against its write fence yourself; no fence is stored. A job whose tree is gone has no row (it is in `skipped` with a reason), and the drift check skips it. @@ -1021,12 +1021,12 @@ the Bash command `rt herd stop --hidden` (no tool runs it); never run it unpromp - **Domain hook: after the report.** Unbound: the integration job merges. Bound domain rules may define what follows a report (the worker ships through its own skill chain, the shepherd merges after a gate, or a human merges on the forge and the lane closes when `herd_lanes` reads `merged`; how jobs feeding one deliverable integrate) and whether it waits for the user to ask. Fixing and pushing stay with workers. - **Domain hook: wrap-up.** Unbound: as drawn. Bound domain rules may state their own tree lifecycle (trees that dispose themselves on merge, what a disposal refusal means); follow it over the defaults here. - + ## wrap-up form contract *If a rule below asks for a move this graph marks STOP, take the off-script edge instead.* - + ### Wrap-up The reply is one optional sentence of context, then a form, then stop. Wait @@ -1055,7 +1055,7 @@ next call after the answers return, never into the context sentence. | "I can hand them a default to save them answering" | Recommended options already do that in the form; a typed reply still costs more than a tap and leaves no record of the pick. | | "The context sentence, then a line introducing the form" | Two sentences. The form needs no introduction; the sentence is the whole preamble. | - + ## Rationalizations | Thought | Reality | diff --git a/plugins/mattstack/skills/shepherdr/compiled.json b/plugins/mattstack/skills/shepherdr/compiled.json index 558ec940f7..6f0e32de13 100644 --- a/plugins/mattstack/skills/shepherdr/compiled.json +++ b/plugins/mattstack/skills/shepherdr/compiled.json @@ -1,8 +1,8 @@ { - "id": "mattstack@0.32.29:shepherdr", + "id": "mattstack@0.32.30:shepherdr", "dir": "skills/shepherdr", "covers": [ - "mattstack@0.32.29:shepherdr" + "mattstack@0.32.30:shepherdr" ], "keys": { "shepherdr/SKILL.md": "skills/shepherdr/SKILL.md", diff --git a/plugins/mattstack/skills/wrap-up/SKILL.md b/plugins/mattstack/skills/wrap-up/SKILL.md index e6dbddad04..5a8bb2788e 100644 --- a/plugins/mattstack/skills/wrap-up/SKILL.md +++ b/plugins/mattstack/skills/wrap-up/SKILL.md @@ -2,12 +2,12 @@ name: "wrap-up" description: "Use when wrapping up a session, checking in before continuing, ending a turn with open decisions, when the user asks what you need from them, what decisions are open, or what the next steps are, when they invoke wrap-up or check-in, or when a pipeline gate needs its decision presented as a form." metadata: - compiled: "mattstack@0.32.29" + compiled: "mattstack@0.32.30" --- - + # Wrap-up The reply is one optional sentence of context, then a form, then stop. Wait diff --git a/plugins/mattstack/skills/wrap-up/compiled.json b/plugins/mattstack/skills/wrap-up/compiled.json index e653ba560d..2bc15d32d0 100644 --- a/plugins/mattstack/skills/wrap-up/compiled.json +++ b/plugins/mattstack/skills/wrap-up/compiled.json @@ -1,8 +1,8 @@ { - "id": "mattstack@0.32.29:wrap-up", + "id": "mattstack@0.32.30:wrap-up", "dir": "skills/wrap-up", "covers": [ - "mattstack@0.32.29:wrap-up" + "mattstack@0.32.30:wrap-up" ], "keys": {} } diff --git a/plugins/mattstack/skills/writing-style-conversational/SKILL.md b/plugins/mattstack/skills/writing-style-conversational/SKILL.md index 7c494a7f8e..2cf8363bec 100644 --- a/plugins/mattstack/skills/writing-style-conversational/SKILL.md +++ b/plugins/mattstack/skills/writing-style-conversational/SKILL.md @@ -2,27 +2,27 @@ name: "writing-style-conversational" description: "Use only when the mattstack writing-style lookup names mattstack:writing-style-conversational, or falls back to it because the lookup failed or the skill it named will not load. The voice for review comments, replies, PR descriptions and commit messages posted under the operator's name: short, friendly sentences in sentence case, like talking to a teammate." metadata: - compiled: "mattstack@0.32.29" + compiled: "mattstack@0.32.30" --- -Pointers here are `@keys`. Resolve them once per run: if you already hold a `skills_resolve` map that covers `mattstack@0.32.29:writing-style-conversational`, use it; otherwise call `skills_resolve {id: "mattstack@0.32.29:writing-style-conversational"}` and use the path it returns for each key, in anything you hand a subagent or a tool too. If it fails, stop and say so; never look for the file another way. +Pointers here are `@keys`. Resolve them once per run: if you already hold a `skills_resolve` map that covers `mattstack@0.32.30:writing-style-conversational`, use it; otherwise call `skills_resolve {id: "mattstack@0.32.30:writing-style-conversational"}` and use the path it returns for each key, in anything you hand a subagent or a tool too. If it fails, stop and say so; never look for the file another way. - + # Conversational Short, friendly, and plain, like talking to a teammate across a desk. Load this before the first draft and write in it from the first word. - + ## Rules every style shares These bind anything posted under the operator's name: review findings, the review summary, replies on their own PR, PR descriptions, commit messages. The preset sets the voice; these set the floor. - + ### Never - **Em or en dashes.** Rephrase, or use parentheses, a colon, or an ellipsis @@ -40,7 +40,7 @@ The preset sets the voice; these set the floor. noticed:", "Took a look", "I just wanted to". Start with the claim. - **Invented precision.** "most" beats "~96%" unless the number was measured. - + ### Always - **Compress before posting.** The first draft is a rough cut. Before posting @@ -73,7 +73,7 @@ The preset sets the voice; these set the floor. notes. It never recaps a finding, lists strengths, or reports what you ran. A finding with no line to anchor to is its own top-level comment. - + ## Voice - **Sentence case** everywhere, with contractions. @@ -88,7 +88,7 @@ The preset sets the voice; these set the floor. Compression target: would this read naturally if you said it out loud to the author? Cut anything you would not say. - + ## Review findings Open with a bolded Conventional Comments label, never a code span @@ -110,13 +110,13 @@ const pageCount = Math.ceil(total / pageSize); A process ask is one line that leads with the approval if there is one: `**suggestion:** Looks good! Could you add a test for the empty-list case?` - + ## Review summary One line: `Looks good to me, no blockers. Left a few comments inline.` Only a blocker joins it. - + ## Replies on your own PR One to three short sentences. No sign-offs; thank someone only if they @@ -127,12 +127,12 @@ matters. - `Good call, switched to Record.` - `I'd skip the lock for now. The job runs on one worker, and a lock adds a round trip per write.` - + ## PR descriptions Read `@writing-style-conversational/pr-description.md` before drafting. - + ## Commit messages - Subject: imperative, sentence case, under 72 characters, with the ticket diff --git a/plugins/mattstack/skills/writing-style-conversational/compiled.json b/plugins/mattstack/skills/writing-style-conversational/compiled.json index 973f64289f..7d128ac18a 100644 --- a/plugins/mattstack/skills/writing-style-conversational/compiled.json +++ b/plugins/mattstack/skills/writing-style-conversational/compiled.json @@ -1,8 +1,8 @@ { - "id": "mattstack@0.32.29:writing-style-conversational", + "id": "mattstack@0.32.30:writing-style-conversational", "dir": "skills/writing-style-conversational", "covers": [ - "mattstack@0.32.29:writing-style-conversational" + "mattstack@0.32.30:writing-style-conversational" ], "keys": { "writing-style-conversational/pr-description.md": "skills/writing-style-conversational/pr-description.md" diff --git a/plugins/mattstack/skills/writing-style-sparse/SKILL.md b/plugins/mattstack/skills/writing-style-sparse/SKILL.md index 6bfb91e91e..fd42a5db95 100644 --- a/plugins/mattstack/skills/writing-style-sparse/SKILL.md +++ b/plugins/mattstack/skills/writing-style-sparse/SKILL.md @@ -2,28 +2,28 @@ name: "writing-style-sparse" description: "Use only when the mattstack writing-style lookup names mattstack:writing-style-sparse. The voice for review comments, replies, PR descriptions and commit messages posted under the operator's name: terse, lowercase technical prose, one tight paragraph per point." metadata: - compiled: "mattstack@0.32.29" + compiled: "mattstack@0.32.30" --- -Pointers here are `@keys`. Resolve them once per run: if you already hold a `skills_resolve` map that covers `mattstack@0.32.29:writing-style-sparse`, use it; otherwise call `skills_resolve {id: "mattstack@0.32.29:writing-style-sparse"}` and use the path it returns for each key, in anything you hand a subagent or a tool too. If it fails, stop and say so; never look for the file another way. +Pointers here are `@keys`. Resolve them once per run: if you already hold a `skills_resolve` map that covers `mattstack@0.32.30:writing-style-sparse`, use it; otherwise call `skills_resolve {id: "mattstack@0.32.30:writing-style-sparse"}` and use the path it returns for each key, in anything you hand a subagent or a tool too. If it fails, stop and say so; never look for the file another way. - + # Sparse Terse and conversational. The reader is a colleague who can follow a chain from one sentence. Load this before the first draft and write in it from the first word; it is not a pass applied to a finished draft. - + ## Rules every style shares These bind anything posted under the operator's name: review findings, the review summary, replies on their own PR, PR descriptions, commit messages. The preset sets the voice; these set the floor. - + ### Never - **Em or en dashes.** Rephrase, or use parentheses, a colon, or an ellipsis @@ -41,7 +41,7 @@ The preset sets the voice; these set the floor. noticed:", "Took a look", "I just wanted to". Start with the claim. - **Invented precision.** "most" beats "~96%" unless the number was measured. - + ### Always - **Compress before posting.** The first draft is a rough cut. Before posting @@ -74,7 +74,7 @@ The preset sets the voice; these set the floor. notes. It never recaps a finding, lists strengths, or reports what you ran. A finding with no line to anchor to is its own top-level comment. - + ## Voice - **Lowercase for technical content**: findings, assertions, code talk. @@ -93,7 +93,7 @@ Compression target: shorter and more conversational. Ask "would a busy teammate send this as written, or trim it first?" If there is any doubt, trim. - + ## Review findings Open with a bolded Conventional Comments label, never a code span: @@ -140,14 +140,14 @@ A brief, specific credit is fine folded in after the worry, never as the opener: `**thought (non-blocking):** only worry here: nothing tests the fallback. the version's pinned now, nice catch.`. A bare compliment is not. - + ## Review summary One line, sentence case: `Looks solid to me, no blockers. Left a few inline notes.` or `Left a few comments!` Only a blocker may join it: `Left one blocker inline.` - + ## Replies on your own PR Very short. No sign-offs, no thanks unless they thanked first. Soften even @@ -162,13 +162,13 @@ blank-line paragraphs. - `i don't think we need a lock here. the job runs on one worker, and a lock adds a round trip per write.` - `👍` - + ## PR descriptions Read `@writing-style-sparse/pr-description.md` before drafting. It carries the length target, the structure, and the anti-patterns. - + ## Commit messages - Subject: lowercase, imperative, under 72 characters, prefixed with the diff --git a/plugins/mattstack/skills/writing-style-sparse/compiled.json b/plugins/mattstack/skills/writing-style-sparse/compiled.json index a767fd52b0..b9af07496a 100644 --- a/plugins/mattstack/skills/writing-style-sparse/compiled.json +++ b/plugins/mattstack/skills/writing-style-sparse/compiled.json @@ -1,8 +1,8 @@ { - "id": "mattstack@0.32.29:writing-style-sparse", + "id": "mattstack@0.32.30:writing-style-sparse", "dir": "skills/writing-style-sparse", "covers": [ - "mattstack@0.32.29:writing-style-sparse" + "mattstack@0.32.30:writing-style-sparse" ], "keys": { "writing-style-sparse/pr-description.md": "skills/writing-style-sparse/pr-description.md" diff --git a/plugins/mattstack/skills/writing-style-structured/SKILL.md b/plugins/mattstack/skills/writing-style-structured/SKILL.md index 74d5094627..93f28fec43 100644 --- a/plugins/mattstack/skills/writing-style-structured/SKILL.md +++ b/plugins/mattstack/skills/writing-style-structured/SKILL.md @@ -2,28 +2,28 @@ name: "writing-style-structured" description: "Use only when the mattstack writing-style lookup names mattstack:writing-style-structured. The voice for review comments, replies, PR descriptions and commit messages posted under the operator's name: labelled lines and short bullets for teams that prefer formal write-ups." metadata: - compiled: "mattstack@0.32.29" + compiled: "mattstack@0.32.30" --- -Pointers here are `@keys`. Resolve them once per run: if you already hold a `skills_resolve` map that covers `mattstack@0.32.29:writing-style-structured`, use it; otherwise call `skills_resolve {id: "mattstack@0.32.29:writing-style-structured"}` and use the path it returns for each key, in anything you hand a subagent or a tool too. If it fails, stop and say so; never look for the file another way. +Pointers here are `@keys`. Resolve them once per run: if you already hold a `skills_resolve` map that covers `mattstack@0.32.30:writing-style-structured`, use it; otherwise call `skills_resolve {id: "mattstack@0.32.30:writing-style-structured"}` and use the path it returns for each key, in anything you hand a subagent or a tool too. If it fails, stop and say so; never look for the file another way. - + # Structured Clear and scannable, for teams that like formal write-ups. Every point is labelled so a reader can triage without reading the whole thing. Load this before the first draft and write in it from the first word. - + ## Rules every style shares These bind anything posted under the operator's name: review findings, the review summary, replies on their own PR, PR descriptions, commit messages. The preset sets the voice; these set the floor. - + ### Never - **Em or en dashes.** Rephrase, or use parentheses, a colon, or an ellipsis @@ -41,7 +41,7 @@ The preset sets the voice; these set the floor. noticed:", "Took a look", "I just wanted to". Start with the claim. - **Invented precision.** "most" beats "~96%" unless the number was measured. - + ### Always - **Compress before posting.** The first draft is a rough cut. Before posting @@ -74,7 +74,7 @@ The preset sets the voice; these set the floor. notes. It never recaps a finding, lists strengths, or reports what you ran. A finding with no line to anchor to is its own top-level comment. - + ## Voice - **Sentence case**, complete but short sentences. @@ -87,7 +87,7 @@ The preset sets the voice; these set the floor. Compression target: every line earns its label. Cut any line a reader could skip without missing the ask. - + ## Review findings Open with a bolded Conventional Comments label, never a code span, then @@ -107,13 +107,13 @@ const pageCount = Math.ceil(total / pageSize); Use only the lines that carry something: a `nitpick` is the label and one line. A process ask is one line. - + ## Review summary One line: `No blockers. Inline notes on paging and tests.` Only a blocker joins it: `One blocker inline (page count).` - + ## Replies on your own PR One or two sentences, plain and direct. No thanks opener. State the outcome @@ -122,12 +122,12 @@ first. - `Changed to Record.` - `Skipping the lock: the job runs on one worker, and a lock adds a round trip per write.` - + ## PR descriptions Read `@writing-style-structured/pr-description.md` before drafting. - + ## Commit messages - Subject: imperative, sentence case, under 72 characters, with the ticket diff --git a/plugins/mattstack/skills/writing-style-structured/compiled.json b/plugins/mattstack/skills/writing-style-structured/compiled.json index eb9c85a698..026fc30d6b 100644 --- a/plugins/mattstack/skills/writing-style-structured/compiled.json +++ b/plugins/mattstack/skills/writing-style-structured/compiled.json @@ -1,8 +1,8 @@ { - "id": "mattstack@0.32.29:writing-style-structured", + "id": "mattstack@0.32.30:writing-style-structured", "dir": "skills/writing-style-structured", "covers": [ - "mattstack@0.32.29:writing-style-structured" + "mattstack@0.32.30:writing-style-structured" ], "keys": { "writing-style-structured/pr-description.md": "skills/writing-style-structured/pr-description.md" From eb7db89e4795c0a335ddd66c891a194500649085 Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Wed, 28 Oct 2026 16:43:01 -0500 Subject: [PATCH 11/15] skills: define no selection and the empty selection; rule text gets a standard block; repair keeps a parsed set Co-Authored-By: Claude Opus 5.5 --- .../dispatch-the-review.block.md | 5 ++-- .../caller-inputs.block.md | 2 ++ .../standard-blocks.block.md | 7 ++++- .../select-the-rules.block.md | 19 ++++++++++---- .../when-the-selector-fails.block.md | 4 +-- .../check-the-verdict-set.block.md | 3 ++- .../repair-the-verdict-set.block.md | 26 ++++++++++++------- .../the-verdict-file.block.md | 5 ++-- 8 files changed, 49 insertions(+), 22 deletions(-) diff --git a/plugins/mattstack/attachments/review-core-body-after/dispatch-the-review.block.md b/plugins/mattstack/attachments/review-core-body-after/dispatch-the-review.block.md index 4cd2340d1c..a1d93b1c41 100644 --- a/plugins/mattstack/attachments/review-core-body-after/dispatch-the-review.block.md +++ b/plugins/mattstack/attachments/review-core-body-after/dispatch-the-review.block.md @@ -12,8 +12,9 @@ it the full payload: - the commit range - the setup observations from Set up for the depth - the filled criteria addendum, when the pack binds criteria -- the rule selection text, when a selector ran, and with it the request for - a verdict set in a block tagged with the `fence` value +- the rule selection text and the verdict request naming the `fence` + value, unless there is no selection or the selection is empty (Rule + selector's Select the rules) It owns the template, the subagent, and the standard blocks. Judgment forms there, not here: whoever sized this change carries assumptions about it, and diff --git a/plugins/mattstack/attachments/review-dispatch-body/caller-inputs.block.md b/plugins/mattstack/attachments/review-dispatch-body/caller-inputs.block.md index c523d2335c..2baa593469 100644 --- a/plugins/mattstack/attachments/review-dispatch-body/caller-inputs.block.md +++ b/plugins/mattstack/attachments/review-dispatch-body/caller-inputs.block.md @@ -11,3 +11,5 @@ order: 2 - the requirements or acceptance criteria the change is judged against - optional: an already-filled domain addendum block - optional: setup observations already gathered (commands run, values seen) +- optional: the rule selection text, plus a verdict request naming the + `fence` value when the caller asks for one (an adjudicator never does) diff --git a/plugins/mattstack/attachments/review-dispatch-body/standard-blocks.block.md b/plugins/mattstack/attachments/review-dispatch-body/standard-blocks.block.md index 26d7714b07..ad815a39d1 100644 --- a/plugins/mattstack/attachments/review-dispatch-body/standard-blocks.block.md +++ b/plugins/mattstack/attachments/review-dispatch-body/standard-blocks.block.md @@ -10,7 +10,12 @@ Append to every dispatched prompt, in this order, after its filled template: redo)`, followed by the caller's observations, or `none`. 2. The caller's addendum block, verbatim when provided, except that each `@key` in it goes as the absolute path `skills_resolve` returned for it. -3. No fabrication: do not invent requirements, files, or findings; if the +3. The rule selection, only when the caller hands one: `## Rules this + change triggers`, the selection text verbatim, then, when the caller + asks for one, the verdict request: end the reply with the verdict set + in one fenced block tagged with the `fence` value. With no selection + text this block is left out. +4. No fabrication: do not invent requirements, files, or findings; if the diff, the requirements, or a referenced file cannot be read, say so for that item and stop rather than guessing. diff --git a/plugins/mattstack/attachments/review-rules-select-body/select-the-rules.block.md b/plugins/mattstack/attachments/review-rules-select-body/select-the-rules.block.md index f08d98c704..d7723ca901 100644 --- a/plugins/mattstack/attachments/review-rules-select-body/select-the-rules.block.md +++ b/plugins/mattstack/attachments/review-rules-select-body/select-the-rules.block.md @@ -11,6 +11,14 @@ dispatch, no verdict request, no verdict file, and no setup observation. Everything else runs as it would without this section. A fill that is not empty and resolves to neither shape is a failure (When the selector fails). +**No selection** means any of: an unbound slot (an empty fill), the line +`selector: none`, or a selector that failed (When the selector fails). With +no selection, no later rule step runs and no verdict file is saved. + +**The empty selection** is a selection whose `rules` list is empty. It +sends no rule text and no verdict request, skips the check and the repair, +adds no findings, and saves `{"verdicts": []}` as the verdict file. + With a selector: 1. Run its `selector` command on Bash twice, `{format}` replaced by `json` @@ -22,8 +30,9 @@ With a selector: literally from then on. Every later step that writes a file for the review writes it there. Write the `json` run's stdout to `selection.json` in it. -3. Put the `text` run's stdout into the dispatch payload as it came, and ask - the reviewer to end its reply with its verdict set in one block tagged - with the `fence` value. When the dispatch is a thread adjudicator, the - text goes to it as reference for judging the threads, with no verdict - request, and no later rule step runs. +3. Unless the selection is empty, hand the `text` run's stdout to the + dispatch as the rule selection (Review dispatch's Standard blocks), as + it came, with a verdict request naming the `fence` value. When the + dispatch is a thread adjudicator, the text goes to it as reference for + judging the threads, with no verdict request, and no later rule step + runs. diff --git a/plugins/mattstack/attachments/review-rules-select-body/when-the-selector-fails.block.md b/plugins/mattstack/attachments/review-rules-select-body/when-the-selector-fails.block.md index b72bcf8e32..bf677ca42e 100644 --- a/plugins/mattstack/attachments/review-rules-select-body/when-the-selector-fails.block.md +++ b/plugins/mattstack/attachments/review-rules-select-body/when-the-selector-fails.block.md @@ -10,6 +10,6 @@ Any of these is a setup observation, quoted as it happened: - the `json` run prints text that is not JSON, or JSON with no `rules` list - the fill resolves to neither shape (the fill text) -Then the review goes on with no rules: no rule text in the dispatch, no -verdict request, no later rule step. A selector failure never blocks a +Then the review goes on with no selection: no rule text in the dispatch, +no verdict request, no later rule step. A selector failure never blocks a review and never opens a gate. diff --git a/plugins/mattstack/attachments/review-rules-verdicts-body/check-the-verdict-set.block.md b/plugins/mattstack/attachments/review-rules-verdicts-body/check-the-verdict-set.block.md index 6158c146db..f49bfce63c 100644 --- a/plugins/mattstack/attachments/review-rules-verdicts-body/check-the-verdict-set.block.md +++ b/plugins/mattstack/attachments/review-rules-verdicts-body/check-the-verdict-set.block.md @@ -4,7 +4,8 @@ order: 1 # Check the verdict set -These steps run only when the selector ran and the reviewer has returned. +These steps run only once the reviewer has returned, and never with no +selection or the empty selection (Rule selector's Select the rules). 1. Extract the reply's fenced block tagged with the `fence` value into `verdict-set.json` in the review scratch dir. diff --git a/plugins/mattstack/attachments/review-rules-verdicts-body/repair-the-verdict-set.block.md b/plugins/mattstack/attachments/review-rules-verdicts-body/repair-the-verdict-set.block.md index ef42644240..59910c50cf 100644 --- a/plugins/mattstack/attachments/review-rules-verdicts-body/repair-the-verdict-set.block.md +++ b/plugins/mattstack/attachments/review-rules-verdicts-body/repair-the-verdict-set.block.md @@ -7,13 +7,21 @@ order: 2 A problem event charges `verdict-rounds:`, the depth in force, limit 2, which allows exactly one repair. While the budget is not spent, repair: -1. Dispatch one fresh-context reviewer with the selection text, the first - verdict set and findings, and the problems. It returns a corrected - verdict set only, in a block tagged with the `fence` value, and no - findings. -2. Check the corrected set as in Check the verdict set. The first review's - findings stay; only the verdict set is replaced. +1. Dispatch one fresh-context reviewer through the same dispatch flow and + reviewer provider as the review (Review dispatch). Its payload is the + selection text, the first verdict set and findings, and the problems. + Its reply is the verdict set block alone, tagged with the `fence` value, + with no findings: for the repair this overrides the reviewer shape's + buckets. +2. Check the corrected set as in Check the verdict set. It replaces the + first verdict set only when it parses (exactly one block, valid JSON); + otherwise the first set stays. The first review's findings stay either + way. -Problems left once the budget is spent go into the draft's notes, quoted, -and the run goes on without a hold. A rule with no verdict in the set, or -one the remaining problems name, is listed in the notes as unanswered. +Problems left once the budget is spent never hold the run. They go on one +line in the draft's Assessment reasoning, `Unanswered rules:` followed by +the id of each unanswered rule and then the leftover problems, quoted; when +a report json is written, the same line goes into its `notes`. A rule is +unanswered when the set has no verdict for it or the leftover problems name +it. An unanswered rule is never turned into a finding, whatever its +verdict. diff --git a/plugins/mattstack/attachments/review-rules-verdicts-body/the-verdict-file.block.md b/plugins/mattstack/attachments/review-rules-verdicts-body/the-verdict-file.block.md index 36f510b7b3..336740289f 100644 --- a/plugins/mattstack/attachments/review-rules-verdicts-body/the-verdict-file.block.md +++ b/plugins/mattstack/attachments/review-rules-verdicts-body/the-verdict-file.block.md @@ -6,5 +6,6 @@ order: 4 Save the verdict set the findings came from as `review-verdicts.json` in the review scratch dir, always, and under the same name next to the report -json when a report path was handed in. A run with no parseable verdict set -saves `{"verdicts": []}`. +json when a report path was handed in. A run with no parseable verdict set, +or with the empty selection, saves `{"verdicts": []}`. With no selection +there is no verdict file. From 2394f2e5aa6a38b966d4736b728886bdc21d8a9a Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Wed, 28 Oct 2026 16:43:04 -0500 Subject: [PATCH 12/15] skills: unanswered rules reach the Assessment; re-dispatch carries no rule text Co-Authored-By: Claude Opus 5.5 --- .../steps/dispatch-adjudicator.block.md | 9 +++++---- .../review/the-review-graph/assemble-draft.block.md | 4 +++- .../review/the-review-graph/check-verdicts.block.md | 3 ++- .../the-review-graph/fails-to-findings.block.md | 13 +++++++++---- .../review/review/the-review-graph/index.block.md | 4 ++-- .../the-review-graph/redispatch-reviewer.block.md | 4 +++- .../review/the-review-graph/write-report.block.md | 5 +++-- .../the-self-review-graph/check-verdicts.block.md | 3 ++- .../fails-to-findings.block.md | 6 ++++-- .../gate-fix-and-next.block.md | 3 ++- .../the-self-review-graph/hand-back-minor.block.md | 3 ++- .../the-self-review-graph/index.block.md | 4 ++-- 12 files changed, 39 insertions(+), 22 deletions(-) diff --git a/plugins/mattstack/attachments/review/receive-review/steps/dispatch-adjudicator.block.md b/plugins/mattstack/attachments/review/receive-review/steps/dispatch-adjudicator.block.md index b1b30237c3..3f355283f5 100644 --- a/plugins/mattstack/attachments/review/receive-review/steps/dispatch-adjudicator.block.md +++ b/plugins/mattstack/attachments/review/receive-review/steps/dispatch-adjudicator.block.md @@ -21,7 +21,8 @@ Hand the dispatch flow (below, after the Off-script gate section) the numbered threads (`file:line` plus note chains), the requirements and the diff range; it owns the template, the subagent and the standard blocks. -When a selector ran, add its `text` selection to the payload as reference -material for judging the threads, under its own heading. It carries no -verdict request: the adjudicator's reply keeps the shape it has without a -selector. With no selection, the payload is the one above. +Unless there is no selection or the selection is empty, hand its `text` +to the dispatch as the rule selection (the dispatch flow's Standard +blocks), as reference for judging the threads. It carries no verdict +request: the adjudicator's reply keeps the shape it has without a +selector. Otherwise the payload is the one above. diff --git a/plugins/mattstack/attachments/review/review/the-review-graph/assemble-draft.block.md b/plugins/mattstack/attachments/review/review/the-review-graph/assemble-draft.block.md index 951db61415..8ec5cdb9b1 100644 --- a/plugins/mattstack/attachments/review/review/the-review-graph/assemble-draft.block.md +++ b/plugins/mattstack/attachments/review/review/the-review-graph/assemble-draft.block.md @@ -6,4 +6,6 @@ node: assemble_draft The review flow's Assemble the draft, Earlier threads first on a re-review. On a second verify round, keep what round one verified and take -the re-dispatch's answer for the rest. +the re-dispatch's answer for the rest. When the verdict rounds were spent, +the `Unanswered rules:` line (Repair the verdict set) goes into the +Assessment reasoning. diff --git a/plugins/mattstack/attachments/review/review/the-review-graph/check-verdicts.block.md b/plugins/mattstack/attachments/review/review/the-review-graph/check-verdicts.block.md index 1437be7a0c..f1fa849e0b 100644 --- a/plugins/mattstack/attachments/review/review/the-review-graph/check-verdicts.block.md +++ b/plugins/mattstack/attachments/review/review/the-review-graph/check-verdicts.block.md @@ -5,4 +5,5 @@ node: check_verdicts # Check the reviewer's verdict set Rule verdicts' Check the verdict set, on the reviewer's reply. With no -selector, `verdict_problems` takes `none`. +selection, or the empty selection, `verdict_problems` takes `none, or no +selection`. diff --git a/plugins/mattstack/attachments/review/review/the-review-graph/fails-to-findings.block.md b/plugins/mattstack/attachments/review/review/the-review-graph/fails-to-findings.block.md index f766cd93e1..77a0687d41 100644 --- a/plugins/mattstack/attachments/review/review/the-review-graph/fails-to-findings.block.md +++ b/plugins/mattstack/attachments/review/review/the-review-graph/fails-to-findings.block.md @@ -7,7 +7,12 @@ node: fails_to_findings Rule verdicts' Failed rules become findings, added to the assembled draft before verification, so the verify and demote steps treat them like any other finding. The verdict set is final here, so also save it to the review -scratch dir as `review-verdicts.json` (The verdict file). With no selector -there is nothing to add or save. After -`redispatch_reviewer` the draft is assembled again and the merge keeps one -finding per rule. +scratch dir as `review-verdicts.json` (The verdict file). With no selection +there is nothing to add or save; the empty selection adds nothing and saves +`{"verdicts": []}`. + +On the pass after `redispatch_reviewer`, the verdict set is the one already +checked and saved: nothing is checked or saved again, and nothing is added +from it again. A rule finding the re-dispatch answered takes that answer +(confirmed, re-anchored or withdrawn), and one round one verified stays as +it was. diff --git a/plugins/mattstack/attachments/review/review/the-review-graph/index.block.md b/plugins/mattstack/attachments/review/review/the-review-graph/index.block.md index 65512099af..80647910ec 100644 --- a/plugins/mattstack/attachments/review/review/the-review-graph/index.block.md +++ b/plugins/mattstack/attachments/review/review/the-review-graph/index.block.md @@ -224,12 +224,12 @@ flowchart TB select_rules --> dispatch_reviewer dispatch_reviewer --> check_verdicts check_verdicts --> verdict_problems - verdict_problems -->|none, or no selector| assemble_draft + verdict_problems -->|none, or no selection| assemble_draft verdict_problems -->|yes| run_budget_verdict_rounds run_budget_verdict_rounds --> verdict_rounds_spent verdict_rounds_spent -->|no| repair_verdicts repair_verdicts --> check_verdicts - verdict_rounds_spent -->|yes: list them in the notes| assemble_draft + verdict_rounds_spent -->|yes: unanswered in the Assessment| assemble_draft assemble_draft --> fails_to_findings fails_to_findings --> verify_blocking verify_blocking --> blocking_verified diff --git a/plugins/mattstack/attachments/review/review/the-review-graph/redispatch-reviewer.block.md b/plugins/mattstack/attachments/review/review/the-review-graph/redispatch-reviewer.block.md index 9d3256e9a6..071350bcd5 100644 --- a/plugins/mattstack/attachments/review/review/the-review-graph/redispatch-reviewer.block.md +++ b/plugins/mattstack/attachments/review/review/the-review-graph/redispatch-reviewer.block.md @@ -6,4 +6,6 @@ node: redispatch_reviewer One fresh-context dispatch, same template, naming each failed check verbatim and asking it to -confirm each finding with a corrected anchor or withdraw it. +confirm each finding with a corrected anchor or withdraw it. It carries no +rule selection text and no verdict request: rule findings go to it like any +other finding. diff --git a/plugins/mattstack/attachments/review/review/the-review-graph/write-report.block.md b/plugins/mattstack/attachments/review/review/the-review-graph/write-report.block.md index c2ef00e752..0e09fd1b97 100644 --- a/plugins/mattstack/attachments/review/review/the-review-graph/write-report.block.md +++ b/plugins/mattstack/attachments/review/review/the-review-graph/write-report.block.md @@ -29,8 +29,9 @@ here, before any gate opens. call to the author, or says approval is withheld pending the notes. The `request_changes` line says what must change before it merges. -When a selector ran, write `review-verdicts.json` (The verdict file) next to -the report json. +Unless there was no selection, write `review-verdicts.json` (The verdict +file) next to the report json. The draft's `Unanswered rules:` line, when +it has one, goes into the json's `notes` word for word. These drafts are the text that posts. Once a gate is built from the json, nothing rewrites it; an iterate drafts again only for the new gate. diff --git a/plugins/mattstack/attachments/review/self-review/the-self-review-graph/check-verdicts.block.md b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/check-verdicts.block.md index 1437be7a0c..f1fa849e0b 100644 --- a/plugins/mattstack/attachments/review/self-review/the-self-review-graph/check-verdicts.block.md +++ b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/check-verdicts.block.md @@ -5,4 +5,5 @@ node: check_verdicts # Check the reviewer's verdict set Rule verdicts' Check the verdict set, on the reviewer's reply. With no -selector, `verdict_problems` takes `none`. +selection, or the empty selection, `verdict_problems` takes `none, or no +selection`. diff --git a/plugins/mattstack/attachments/review/self-review/the-self-review-graph/fails-to-findings.block.md b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/fails-to-findings.block.md index e5e9449021..77b24995de 100644 --- a/plugins/mattstack/attachments/review/self-review/the-self-review-graph/fails-to-findings.block.md +++ b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/fails-to-findings.block.md @@ -7,5 +7,7 @@ node: fails_to_findings Rule verdicts' Failed rules become findings, carried into the draft that `assemble_draft` builds, so they sit in its Issues like any other finding. The verdict set is final here, so also save it to the review scratch dir as -`review-verdicts.json` (The verdict file). With no selector there is -nothing to add or save. +`review-verdicts.json` (The verdict file). With no selection there is +nothing to add or save; the empty selection adds nothing and saves +`{"verdicts": []}`. An `Unanswered rules:` line (Repair the verdict set) +goes into the draft's Assessment reasoning. diff --git a/plugins/mattstack/attachments/review/self-review/the-self-review-graph/gate-fix-and-next.block.md b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/gate-fix-and-next.block.md index 60d53a8104..f72255d0c8 100644 --- a/plugins/mattstack/attachments/review/self-review/the-self-review-graph/gate-fix-and-next.block.md +++ b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/gate-fix-and-next.block.md @@ -8,7 +8,8 @@ label: Fix and next Run gate-protocol's Runs integration with `gate_ask {questions, kind: self-review, context, runDb, stage}`, `context` quoting the draft's -Issues, and two questions, each its own: +Issues and, when the Assessment has one, its `Unanswered rules:` line, and +two questions, each its own: - `fix` (write-in): **Fix the blocking findings now** (`blocking`: Critical and Important; recommended when any exists) / **Fix the minors too** diff --git a/plugins/mattstack/attachments/review/self-review/the-self-review-graph/hand-back-minor.block.md b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/hand-back-minor.block.md index 4cadf6ba7a..f3b7020f29 100644 --- a/plugins/mattstack/attachments/review/self-review/the-self-review-graph/hand-back-minor.block.md +++ b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/hand-back-minor.block.md @@ -6,4 +6,5 @@ node: hand_back_minor Ship as is: hand back to the calling flow with the Minor findings listed for the record, plus every finding left open or not reproduced; an -unfixed Critical or Important counts as left open. +unfixed Critical or Important counts as left open. The draft's +`Unanswered rules:` line, when it has one, goes with them. diff --git a/plugins/mattstack/attachments/review/self-review/the-self-review-graph/index.block.md b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/index.block.md index 60aabf792e..f3ef4c5e2a 100644 --- a/plugins/mattstack/attachments/review/self-review/the-self-review-graph/index.block.md +++ b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/index.block.md @@ -110,12 +110,12 @@ flowchart TB make_recorded_move --> assemble_draft dispatch_reviewer --> check_verdicts check_verdicts --> verdict_problems - verdict_problems -->|none, or no selector| fails_to_findings + verdict_problems -->|none, or no selection| fails_to_findings verdict_problems -->|yes| run_budget_verdict_rounds run_budget_verdict_rounds --> verdict_rounds_spent verdict_rounds_spent -->|no| repair_verdicts repair_verdicts --> check_verdicts - verdict_rounds_spent -->|yes: list them in the notes| fails_to_findings + verdict_rounds_spent -->|yes: unanswered in the Assessment| fails_to_findings fails_to_findings --> assemble_draft assemble_draft --> gate_fix_and_next gate_fix_and_next --> next_answer From 8833d78b75810f82e43968ed7145483003201977 Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Wed, 28 Oct 2026 16:43:04 -0500 Subject: [PATCH 13/15] rule-selector: pin select_rules as the only edge into the dispatch; README lists seven bodies Co-Authored-By: Claude Opus 5.5 --- lib/skills/__tests__/rule-selector.test.ts | 5 +++++ plugins/mattstack/README.md | 17 ++++++++++------- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/lib/skills/__tests__/rule-selector.test.ts b/lib/skills/__tests__/rule-selector.test.ts index cbe4c08ab1..27b772a4b3 100644 --- a/lib/skills/__tests__/rule-selector.test.ts +++ b/lib/skills/__tests__/rule-selector.test.ts @@ -98,6 +98,11 @@ describe("rule selector slot", () => { expect(body).toContain("run_budget {runDb, key: verdict-rounds:, limit: 2, stage}"); }); + test("review graph: every path to the dispatch passes select_rules, a failed head checkout included", () => { + const into = [...compiled.verbs.review.matchAll(/^\s*(\w+) -> dispatch_reviewer\b/gm)].map((m) => m[1]); + expect(into).toEqual(["select_rules"]); + }); + test("self-review graph places the rule steps", () => { const body = compiled.verbs["self-review"]; const edge = (a: string, b: string) => new RegExp(`\\b${a} -> ${b}\\b`); diff --git a/plugins/mattstack/README.md b/plugins/mattstack/README.md index d5a1f8662e..7ba7f41fa8 100644 --- a/plugins/mattstack/README.md +++ b/plugins/mattstack/README.md @@ -197,21 +197,24 @@ domain packs via the `review-criteria@1`, `reviewer-dispatch@1`, `rule-selector@1`, and `reply-rules@1` contracts, while `subagent-review-loop` is a standalone one-off with no props. -- The review flow lives in five internal include bodies +- The review flow lives in seven internal include bodies (`review-core-body`, `review-core-body-after`, `review-core-body-tail`, - `review-dispatch-body`, `review-dispatch-body-after`) that the compiler - inlines into `review`, `self-review`, and `receive-review`; not for direct + `review-dispatch-body`, `review-dispatch-body-after`, + `review-rules-select-body`, `review-rules-verdicts-body`) that the + compiler inlines into `review`, `self-review`, and `receive-review` + (`receive-review` takes only the rule select body); not for direct invocation. - **mattstack:review** -- review someone else's MR/PR before it merges, from a pasted link, a bare `!iid`/`#number`, or a ticket id: resolve the target, dispatch the judgment to a fresh context, return the structured - draft (`criteria` + `reviewer` props). + draft (`criteria`, `reviewer` and `rule-selector` props). - **mattstack:self-review** -- review this session's own work on the current branch before shipping; the bias gate against grading your own - homework (provides `self-review-domain@1`, the stage's default adapter). + homework (`criteria`, `reviewer` and `rule-selector` props; provides + `self-review-domain@1`, the stage's default adapter). - **mattstack:receive-review** -- process the feedback on your own MR/PR - with technical rigor instead of performative agreement (`criteria` + - `reply-rules` props). + with technical rigor instead of performative agreement (`criteria`, + `reply-rules` and `rule-selector` props). - **mattstack:review-posting** -- executes a decided `{findings | levels, disposition}` selection against the MR/PR: inline threads for what was selected, one summary comment, then the disposition; it never decides From b6b7e16ab6b986f260d1c86e34983ba9019c284a Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Wed, 28 Oct 2026 16:47:20 -0500 Subject: [PATCH 14/15] mattstack: record the rule-selector GREEN walkthrough Co-Authored-By: Claude Opus 5.5 --- plugins/mattstack/CERTIFICATION.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/mattstack/CERTIFICATION.md b/plugins/mattstack/CERTIFICATION.md index 3d01a5dd84..e44c37d6cd 100644 --- a/plugins/mattstack/CERTIFICATION.md +++ b/plugins/mattstack/CERTIFICATION.md @@ -244,6 +244,6 @@ that quotes a shell form carries ``. | 2026-10-20 | mattstack:review, mattstack:review-posting, mattstack:review-core-body-tail | pure | pass | n/a (descriptions unchanged) | the summary line follows the verdict: the review verb drafts `summary.comments`, one line per `outcome` option value the gate offers (`comment`, `approve`, and `request_changes` only on GitHub), only the `approve` line carrying an all-clear; `editable` needs a `comment` on every finding and exactly those keys; Compose posts the `outcome` answer's `text` (the caller's `summaryText`), else `summary.comments[]`, the join unchanged; review-posting and the structured-findings field docs name `summary.comments`. RED 4/15 on V1 to V5 (3 reps each; every miss took the no-drafts branch and composed the summary, some with an all-clear), D2 0/3 (singular `summary.comment` with "Nothing here blocks the merge" for every verdict); GREEN 5/5 on V1 to V5, D1 and D2, G1 and G2 5/5, G3 9/10 (`docs/superpowers/tests/2026-10-20-review-drafted-comments/red-green.md`); certify exit 0 on all three | | 2026-10-23 | mattstack:work-progress | pure | pass | 5/5 scenarios, 5/5 haiku reps each (2 herd kept, epic and 2 plan added); full roster 22/23, the one miss (billing creating-a-pack) also misses on the old description | replaces the herd-only progress skill: the bundled script renders a herd (`--herd`) or plans and epics (`--plan` repeatable, `--title`, `--tree`) from rt's plan records; the skill picks the input, prints the view verbatim and never calls a plan done unless its row says so; RED: no skill loaded on plan or epic prompts, 6 hand calls and a hand-built table, plan 1 guessed done from plan 2 having started; GREEN (claude -p --plugin-dir, opus, sandboxed rt): epic prompt Skill + ls + 1 script call, plan "where is my plan at" and "how far along is it" Skill + 1 script call each, all printed verbatim, "herd progress" triggers it; the brief's description first missed "where is my plan at", fixed in 15e6d8f by adding that phrase; certify exit 0 | | 2026-10-23 | mattstack:shepherdr | pure | pass | n/a (description unchanged) | Progress pointer renamed to mattstack:work-progress, recompiled into skills/shepherdr; certify exit 0 | -| 2026-10-28 | mattstack:review, mattstack:self-review, mattstack:receive-review | pure | pass | n/a (descriptions unchanged) | optional `rule-selector` prop (`rule-selector@1`, replace, default empty): the engines select written rules before dispatch, ask the reviewer for one verdict per rule and turn failed rules into findings; unbound it is a no-op; certify exit 0 on every touched root | +| 2026-10-28 | mattstack:review, mattstack:self-review, mattstack:receive-review | pure | pass | 4/4 GREEN walkthrough scenarios | optional `rule-selector` prop (`rule-selector@1`, replace, default empty): the engines select written rules before dispatch, ask the reviewer for one verdict per rule and turn failed rules into findings; unbound it is a no-op; GREEN describe-only walk of the compiled review: unbound skips every rule step, a selector exiting 1 is one setup observation and no rule steps, a reply with no verdict block gets exactly one repair then an `Unanswered rules:` line in the Assessment, an empty selection sends no verdict request and saves an empty verdict file; certify exit 0 on every touched root | (Ledger rows are appended by the orchestrating session as each lane lands -- lanes themselves never edit this file.) From 03766ce3061b520bdcb98c03d777a54109bf10ce Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Wed, 28 Oct 2026 17:05:18 -0500 Subject: [PATCH 15/15] rule-selector: verdict request states the set shape; merge rule fails only on the same issue; unnamed problems leave every rule unanswered; receive-review revise passes select_rules Co-Authored-By: Claude Opus 5.5 --- lib/skills/__tests__/rule-selector.test.ts | 7 +++++++ .../standard-blocks.block.md | 10 +++++++--- .../failed-rules-become-findings.block.md | 8 +++++--- .../repair-the-verdict-set.block.md | 16 +++++++++------- .../review/receive-review/steps/index.block.md | 2 +- .../receive-review/steps/select-rules.block.md | 4 +++- 6 files changed, 32 insertions(+), 15 deletions(-) diff --git a/lib/skills/__tests__/rule-selector.test.ts b/lib/skills/__tests__/rule-selector.test.ts index 27b772a4b3..685ad454f2 100644 --- a/lib/skills/__tests__/rule-selector.test.ts +++ b/lib/skills/__tests__/rule-selector.test.ts @@ -115,11 +115,18 @@ describe("rule selector slot", () => { expect(body).toMatch(edge("make_recorded_move", "assemble_draft")); expect(body).toContain("run_budget {runDb, key: verdict-rounds:, limit: 2, stage}"); }); + test("receive-review: every edge into the adjudicator passes select_rules", () => { + const into = [...compiled.verbs["receive-review"].matchAll(/^\s*(\w+) -> dispatch_adjudicator\b/gm)].map((m) => m[1]); + expect(into).toEqual(["select_rules"]); + }); + test("receive-review selects rules before the adjudicator", () => { const body = compiled.verbs["receive-review"]; expect(body).toMatch(/\bany_unresolved -> select_rules\b/); expect(body).toMatch(/\bselect_rules -> dispatch_adjudicator\b/); + expect(body).toMatch(/\bcode_changes_answer -> select_rules\b/); expect(body).not.toMatch(/\bany_unresolved -> dispatch_adjudicator\b/); + expect(body).not.toMatch(/\bcode_changes_answer -> dispatch_adjudicator\b/); expect(body).not.toMatch(/\bcheck_verdicts\b/); }); }); diff --git a/plugins/mattstack/attachments/review-dispatch-body/standard-blocks.block.md b/plugins/mattstack/attachments/review-dispatch-body/standard-blocks.block.md index ad815a39d1..1b79c4df2c 100644 --- a/plugins/mattstack/attachments/review-dispatch-body/standard-blocks.block.md +++ b/plugins/mattstack/attachments/review-dispatch-body/standard-blocks.block.md @@ -12,9 +12,13 @@ Append to every dispatched prompt, in this order, after its filled template: `@key` in it goes as the absolute path `skills_resolve` returned for it. 3. The rule selection, only when the caller hands one: `## Rules this change triggers`, the selection text verbatim, then, when the caller - asks for one, the verdict request: end the reply with the verdict set - in one fenced block tagged with the `fence` value. With no selection - text this block is left out. + asks for one, the verdict request. The request says: end the reply with + the verdict set in one fenced block tagged with the `fence` value, in + the shape `{"verdicts": [{"rule", "verdict": "pass" | "fail" | + "not-applicable", "evidence": [{"file", "line", "side"?: "new" | + "old"}], "note"}]}`, with one verdict for every rule id in the + selection (the `id` of each entry in `selection.json`'s `rules[]`). + With no selection text this block is left out. 4. No fabrication: do not invent requirements, files, or findings; if the diff, the requirements, or a referenced file cannot be read, say so for that item and stop rather than guessing. diff --git a/plugins/mattstack/attachments/review-rules-verdicts-body/failed-rules-become-findings.block.md b/plugins/mattstack/attachments/review-rules-verdicts-body/failed-rules-become-findings.block.md index a54fcf6d61..6b142b60cd 100644 --- a/plugins/mattstack/attachments/review-rules-verdicts-body/failed-rules-become-findings.block.md +++ b/plugins/mattstack/attachments/review-rules-verdicts-body/failed-rules-become-findings.block.md @@ -18,6 +18,8 @@ make none. A fail whose evidence is all on the `old` side, or absent, has no anchor: it stays a finding, with its locations in the body. -A fail at the same file and line as a finding the reviewer reported merges -into one finding. It carries both bodies, the rule's title and the higher -of the two tiers. +A fail merges with a finding the reviewer reported only when that finding +sits at the anchor's file and line and describes the same issue the rule +names. The merged finding carries both bodies, the rule's title and the +higher of the two tiers. Any other reviewer finding stays as its own +finding beside the rule's. diff --git a/plugins/mattstack/attachments/review-rules-verdicts-body/repair-the-verdict-set.block.md b/plugins/mattstack/attachments/review-rules-verdicts-body/repair-the-verdict-set.block.md index 59910c50cf..4d1ef4fda3 100644 --- a/plugins/mattstack/attachments/review-rules-verdicts-body/repair-the-verdict-set.block.md +++ b/plugins/mattstack/attachments/review-rules-verdicts-body/repair-the-verdict-set.block.md @@ -9,10 +9,10 @@ A problem event charges `verdict-rounds:`, the depth in force, limit 1. Dispatch one fresh-context reviewer through the same dispatch flow and reviewer provider as the review (Review dispatch). Its payload is the - selection text, the first verdict set and findings, and the problems. - Its reply is the verdict set block alone, tagged with the `fence` value, - with no findings: for the repair this overrides the reviewer shape's - buckets. + selection text, the first verdict set and findings, and the problems, + with the verdict request from Standard blocks. Its reply is the verdict + set block alone, tagged with the `fence` value, with no findings: for + the repair this overrides the reviewer shape's buckets. 2. Check the corrected set as in Check the verdict set. It replaces the first verdict set only when it parses (exactly one block, valid JSON); otherwise the first set stays. The first review's findings stay either @@ -22,6 +22,8 @@ Problems left once the budget is spent never hold the run. They go on one line in the draft's Assessment reasoning, `Unanswered rules:` followed by the id of each unanswered rule and then the leftover problems, quoted; when a report json is written, the same line goes into its `notes`. A rule is -unanswered when the set has no verdict for it or the leftover problems name -it. An unanswered rule is never turned into a finding, whatever its -verdict. +unanswered when the set has no verdict for it or a leftover problem names +it. A leftover problem that names no rule (a missing or unparseable block, +an exit status) leaves every selected rule without a validated verdict +unanswered, in practice all of them. An unanswered rule is never turned +into a finding, whatever its verdict. diff --git a/plugins/mattstack/attachments/review/receive-review/steps/index.block.md b/plugins/mattstack/attachments/review/receive-review/steps/index.block.md index a1d95e6fb5..ea8010e83a 100644 --- a/plugins/mattstack/attachments/review/receive-review/steps/index.block.md +++ b/plugins/mattstack/attachments/review/receive-review/steps/index.block.md @@ -245,7 +245,7 @@ flowchart TB gate_record_plan -->|its rows| rewrite_report_rows rewrite_report_rows --> code_changes_answer code_changes_answer -->|approve, or skip| next_report_row - code_changes_answer -->|revise: a fresh dispatch; a handed plan is spent| dispatch_adjudicator + code_changes_answer -->|revise: a fresh dispatch; a handed plan is spent| select_rules next_report_row -->|fix, code-changes approve, not yet on the branch| implement_fix next_report_row -->|fix already committed on the branch| commit_fix next_report_row -->|override| redraft_override diff --git a/plugins/mattstack/attachments/review/receive-review/steps/select-rules.block.md b/plugins/mattstack/attachments/review/receive-review/steps/select-rules.block.md index c7b2dc51d5..15d4f6aae8 100644 --- a/plugins/mattstack/attachments/review/receive-review/steps/select-rules.block.md +++ b/plugins/mattstack/attachments/review/receive-review/steps/select-rules.block.md @@ -10,4 +10,6 @@ scratch dir it makes is the one every later step that writes a file for this run reuses. With no selector bound, or `selector: none`, skip it and go straight to the dispatch. The selection only ever reaches the adjudicator as reference: this verb asks for no verdict set and runs no later rule step. -A revise that dispatches again reuses this selection and does not run it again. +A revise reaches this step again before its fresh dispatch: when this run +already made a selection, reuse it and run nothing; when it has none (an +unspent handed plan never passed here), select now.