From 27e6c4e348492627f240c075978a9d3c36babe7a Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Wed, 28 Oct 2026 21:54:21 -0500 Subject: [PATCH 1/3] self-review: diff takes in untracked files; stage review field carries unanswered rules The self-review diff now appends each untracked, non-ignored file as a new-file diff without staging it, and the selector and reviewer both get that diff. When the tree has uncommitted or untracked changes, the reviewer judges a written self-review.diff instead of the commit range. stage-self-review copies the review's Unanswered rules line into the review run field. SKILLS-100 Co-Authored-By: Claude Opus 5.5 --- lib/skills/__tests__/rule-selector.test.ts | 7 ++ plugins/mattstack/.claude-plugin/plugin.json | 2 +- .../follow-domain-review.block.md | 4 +- .../pipeline/stage-self-review/index.block.md | 2 +- .../dispatch-reviewer.block.md | 8 +- .../find-merge-base.block.md | 13 ++- .../select-rules.block.md | 9 +- 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 +- 17 files changed, 127 insertions(+), 104 deletions(-) diff --git a/lib/skills/__tests__/rule-selector.test.ts b/lib/skills/__tests__/rule-selector.test.ts index 685ad454f..14f0d1c2d 100644 --- a/lib/skills/__tests__/rule-selector.test.ts +++ b/lib/skills/__tests__/rule-selector.test.ts @@ -115,6 +115,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("self-review's diff takes in untracked files without staging them", () => { + const body = compiled.verbs["self-review"]; + expect(body).toContain("git ls-files --others --exclude-standard"); + expect(body).toContain("git diff --no-index /dev/null "); + expect(body).toContain("`self-review.diff`"); + }); + 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"]); diff --git a/plugins/mattstack/.claude-plugin/plugin.json b/plugins/mattstack/.claude-plugin/plugin.json index d0ff6b0fb..adbd764fa 100644 --- a/plugins/mattstack/.claude-plugin/plugin.json +++ b/plugins/mattstack/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "mattstack", - "version": "0.32.30", + "version": "0.32.31", "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/attachments/pipeline/stage-self-review/follow-domain-review.block.md b/plugins/mattstack/attachments/pipeline/stage-self-review/follow-domain-review.block.md index 28d830e7d..35b8d1ab9 100644 --- a/plugins/mattstack/attachments/pipeline/stage-self-review/follow-domain-review.block.md +++ b/plugins/mattstack/attachments/pipeline/stage-self-review/follow-domain-review.block.md @@ -11,7 +11,9 @@ props: The domain's own review pass, as it words it. When that pass asks the human about its findings at a gate of its own (a self-review whose gate offers Ship as is), the answer stands: the findings it leaves open go -into `review` by name, and this stage never fixes them. A Hold there +into `review` by name, and this stage never fixes them. When the review's +Assessment carries an `Unanswered rules:` line, that line goes into +`review` as written, on any path that writes it. A Hold there holds this stage too; an Abandon has already failed it, so nothing more is written. The fix rounds (Review rounds = 3? and Fix each blocking finding test-first, commit) run only when no such gate ran. diff --git a/plugins/mattstack/attachments/pipeline/stage-self-review/index.block.md b/plugins/mattstack/attachments/pipeline/stage-self-review/index.block.md index 5730d8028..f5d85665d 100644 --- a/plugins/mattstack/attachments/pipeline/stage-self-review/index.block.md +++ b/plugins/mattstack/attachments/pipeline/stage-self-review/index.block.md @@ -34,7 +34,7 @@ flowchart TB domain_review_gated{How did the domain review's own gate end?} git_log_shas[/git log --format=%h ..HEAD/] run_field_set_commits[/"run_field_set {key: commits, value: , stage: self-review}"/] - run_field_set_review[/"run_field_set {key: review, value: , stage: self-review}"/] + run_field_set_review[/"run_field_set {key: review, value: , stage: self-review}"/] run_stage_fail[/"run_stage {action: fail, stage: self-review, reason: blocking findings after 3 rounds}"/] stage_failed(((Stage failed))) held_domain_gate(((Held at the domain review's gate))) diff --git a/plugins/mattstack/attachments/review/self-review/the-self-review-graph/dispatch-reviewer.block.md b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/dispatch-reviewer.block.md index 6aafc766a..1d281a826 100644 --- a/plugins/mattstack/attachments/review/self-review/the-self-review-graph/dispatch-reviewer.block.md +++ b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/dispatch-reviewer.block.md @@ -4,6 +4,12 @@ node: dispatch_reviewer # Dispatch the fresh reviewer on your own diff -The review flow's Dispatch the review, on the merge-base diff as it stands +The review flow's Dispatch the review, on the self-review diff as it stands now, with the requirements and a label (the branch or task name). One dispatch, never skipped, however small the change. + +The commit range (merge base to HEAD) covers commits only. When the tree +has uncommitted changes or untracked files, write the self-review diff to +`self-review.diff` in the review scratch dir (one new directory in this +session's scratchpad, made here when none exists yet) and pass its path as +a setup observation: the reviewer judges that file, not the range. diff --git a/plugins/mattstack/attachments/review/self-review/the-self-review-graph/find-merge-base.block.md b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/find-merge-base.block.md index c2b3daebb..7959ee944 100644 --- a/plugins/mattstack/attachments/review/self-review/the-self-review-graph/find-merge-base.block.md +++ b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/find-merge-base.block.md @@ -5,5 +5,14 @@ node: find_merge_base # Find the self-review merge-base `git merge-base origin/HEAD HEAD`, or the default branch by name when -`origin/HEAD` is unset. The diff runs from that base through the working -tree, uncommitted changes included. +`origin/HEAD` is unset. **The self-review diff** runs from that base +through the working tree, uncommitted changes and untracked files +included, built read-only: + +1. `git diff `: the commits and the uncommitted changes. +2. Then, for each path `git ls-files --others --exclude-standard` prints, + append `git diff --no-index /dev/null `. It exits 1 when it prints + a diff; that is not a failure. + +Ignored files stay out. Nothing is staged and no file changes: never `git +add` or `git add -N` to make a new file show up. 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 index 08d917b5d..fbdde1b0f 100644 --- 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 @@ -5,8 +5,7 @@ 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. +diff it gets on stdin is the self-review diff (Find the self-review +merge-base), untracked files included, so the rules match what the +reviewer judges. With no selector bound, or `selector: none`, skip it and +go on to the repro check. diff --git a/plugins/mattstack/skills/shepherdr/SKILL.md b/plugins/mattstack/skills/shepherdr/SKILL.md index f770b1d4e..1c69d5260 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.30 + mattstack:model-tiering@0.32.30 + mattstack:execution-strategy@0.32.30 + mattstack:cswap-accounts@0.32.30" + compiled: "mattstack@0.32.31 + mattstack:model-tiering@0.32.31 + mattstack:execution-strategy@0.32.31 + mattstack:cswap-accounts@0.32.31" --- -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. +Pointers here are `@keys`. Resolve them once per run: if you already hold a `skills_resolve` map that covers `mattstack@0.32.31:shepherdr`, use it; otherwise call `skills_resolve {id: "mattstack@0.32.31: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.30) when this flow needs it. + +For tiering, invoke the `mattstack:model-tiering` skill (mattstack:model-tiering@0.32.31) 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 6f0e32de1..3e9abad5e 100644 --- a/plugins/mattstack/skills/shepherdr/compiled.json +++ b/plugins/mattstack/skills/shepherdr/compiled.json @@ -1,8 +1,8 @@ { - "id": "mattstack@0.32.30:shepherdr", + "id": "mattstack@0.32.31:shepherdr", "dir": "skills/shepherdr", "covers": [ - "mattstack@0.32.30:shepherdr" + "mattstack@0.32.31: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 5a8bb2788..6b34ca536 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.30" + compiled: "mattstack@0.32.31" --- - + # 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 2bc15d32d..798571c9f 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.30:wrap-up", + "id": "mattstack@0.32.31:wrap-up", "dir": "skills/wrap-up", "covers": [ - "mattstack@0.32.30:wrap-up" + "mattstack@0.32.31:wrap-up" ], "keys": {} } diff --git a/plugins/mattstack/skills/writing-style-conversational/SKILL.md b/plugins/mattstack/skills/writing-style-conversational/SKILL.md index 2cf8363be..707cc54d5 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.30" + compiled: "mattstack@0.32.31" --- -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. +Pointers here are `@keys`. Resolve them once per run: if you already hold a `skills_resolve` map that covers `mattstack@0.32.31:writing-style-conversational`, use it; otherwise call `skills_resolve {id: "mattstack@0.32.31: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 7d128ac18..39e513104 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.30:writing-style-conversational", + "id": "mattstack@0.32.31:writing-style-conversational", "dir": "skills/writing-style-conversational", "covers": [ - "mattstack@0.32.30:writing-style-conversational" + "mattstack@0.32.31: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 fd42a5db9..9b495c41f 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.30" + compiled: "mattstack@0.32.31" --- -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. +Pointers here are `@keys`. Resolve them once per run: if you already hold a `skills_resolve` map that covers `mattstack@0.32.31:writing-style-sparse`, use it; otherwise call `skills_resolve {id: "mattstack@0.32.31: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 b9af07496..6c14288b8 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.30:writing-style-sparse", + "id": "mattstack@0.32.31:writing-style-sparse", "dir": "skills/writing-style-sparse", "covers": [ - "mattstack@0.32.30:writing-style-sparse" + "mattstack@0.32.31: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 93f28fec4..3ec09e3f7 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.30" + compiled: "mattstack@0.32.31" --- -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. +Pointers here are `@keys`. Resolve them once per run: if you already hold a `skills_resolve` map that covers `mattstack@0.32.31:writing-style-structured`, use it; otherwise call `skills_resolve {id: "mattstack@0.32.31: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 026fc30d6..358c1c77b 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.30:writing-style-structured", + "id": "mattstack@0.32.31:writing-style-structured", "dir": "skills/writing-style-structured", "covers": [ - "mattstack@0.32.30:writing-style-structured" + "mattstack@0.32.31:writing-style-structured" ], "keys": { "writing-style-structured/pr-description.md": "skills/writing-style-structured/pr-description.md" From 8e36f9e2c1a59c295894248f6f3d61179236dc93 Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Wed, 28 Oct 2026 22:10:39 -0500 Subject: [PATCH 2/3] review dispatch: a diff file block; NUL-safe untracked paths; review field scope The dispatch body gains an optional diff file that the reviewer judges over the commit range, and self-review hands self-review.diff through it. Untracked paths come from git ls-files -z and go to git diff --no-index after --. The Unanswered rules line goes into review whenever the stage writes review. The test pins the no-staging sentence. Co-Authored-By: Claude Opus 5.5 --- lib/skills/__tests__/rule-selector.test.ts | 6 ++++-- .../stage-self-review/follow-domain-review.block.md | 2 +- .../dispatch-the-review.block.md | 2 ++ .../review-core-body/caller-inputs.block.md | 3 ++- .../review-dispatch-body/caller-inputs.block.md | 2 ++ .../review-dispatch-body/standard-blocks.block.md | 11 ++++++++--- .../the-self-review-graph/dispatch-reviewer.block.md | 5 +++-- .../the-self-review-graph/find-merge-base.block.md | 11 ++++++----- 8 files changed, 28 insertions(+), 14 deletions(-) diff --git a/lib/skills/__tests__/rule-selector.test.ts b/lib/skills/__tests__/rule-selector.test.ts index 14f0d1c2d..69e2e5d87 100644 --- a/lib/skills/__tests__/rule-selector.test.ts +++ b/lib/skills/__tests__/rule-selector.test.ts @@ -117,9 +117,11 @@ describe("rule selector slot", () => { }); test("self-review's diff takes in untracked files without staging them", () => { const body = compiled.verbs["self-review"]; - expect(body).toContain("git ls-files --others --exclude-standard"); - expect(body).toContain("git diff --no-index /dev/null "); + expect(body).toMatch(/`git ls-files -z --others\s+--exclude-standard`/); + expect(body).toMatch(/`git diff --no-index\s+-- \/dev\/null `/); + expect(body).toContain("never `git add` or `git add -N` to make a new file show up"); expect(body).toContain("`self-review.diff`"); + expect(body).toContain("## The diff to review"); }); test("receive-review: every edge into the adjudicator passes select_rules", () => { diff --git a/plugins/mattstack/attachments/pipeline/stage-self-review/follow-domain-review.block.md b/plugins/mattstack/attachments/pipeline/stage-self-review/follow-domain-review.block.md index 35b8d1ab9..98078d67a 100644 --- a/plugins/mattstack/attachments/pipeline/stage-self-review/follow-domain-review.block.md +++ b/plugins/mattstack/attachments/pipeline/stage-self-review/follow-domain-review.block.md @@ -13,7 +13,7 @@ human about its findings at a gate of its own (a self-review whose gate offers Ship as is), the answer stands: the findings it leaves open go into `review` by name, and this stage never fixes them. When the review's Assessment carries an `Unanswered rules:` line, that line goes into -`review` as written, on any path that writes it. A Hold there +`review` as written whenever this stage writes `review`. A Hold there holds this stage too; an Abandon has already failed it, so nothing more is written. The fix rounds (Review rounds = 3? and Fix each blocking finding test-first, commit) run only when no such gate ran. 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 a1d93b1c4..826e71670 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 @@ -10,6 +10,8 @@ it the full payload: - a one-or-two-sentence description of the change - the requirements - the commit range +- the diff file, when the caller wrote one because the change reaches + past HEAD - the setup observations from Set up for the depth - the filled criteria addendum, when the pack binds criteria - the rule selection text and the verdict request naming the `fence` diff --git a/plugins/mattstack/attachments/review-core-body/caller-inputs.block.md b/plugins/mattstack/attachments/review-core-body/caller-inputs.block.md index 9a47ca56a..77ad91985 100644 --- a/plugins/mattstack/attachments/review-core-body/caller-inputs.block.md +++ b/plugins/mattstack/attachments/review-core-body/caller-inputs.block.md @@ -6,7 +6,8 @@ order: 1 - The diff source, already confirmed to resolve. With no named base: the branch point (`git merge-base origin/HEAD HEAD`, falling back to the - default branch by name), plus uncommitted changes for own-branch flows. + default branch by name), plus uncommitted changes and untracked files + for own-branch flows. - The requirements, from the ticket or task description. - A human-readable label for the change. 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 2baa59346..514e064df 100644 --- a/plugins/mattstack/attachments/review-dispatch-body/caller-inputs.block.md +++ b/plugins/mattstack/attachments/review-dispatch-body/caller-inputs.block.md @@ -11,5 +11,7 @@ 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, reviewer only: a diff file, when the change reaches past the + commit range (uncommitted changes or untracked files) - 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 1b79c4df2..8f15f8c89 100644 --- a/plugins/mattstack/attachments/review-dispatch-body/standard-blocks.block.md +++ b/plugins/mattstack/attachments/review-dispatch-body/standard-blocks.block.md @@ -8,9 +8,14 @@ Append to every dispatched prompt, in this order, after its filled template: 1. Setup observations: `## Already checked before dispatch (fold in; do not redo)`, followed by the caller's observations, or `none`. -2. The caller's addendum block, verbatim when provided, except that each +2. The diff file, only when the caller hands one: `## The diff to review`, + the file's absolute path, and this sentence: "Review this file: it holds + every change, including ones past HEAD that the commit range above does + not cover. Where the two differ, the file wins." With no diff file this + block is left out. +3. 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. The rule selection, only when the caller hands one: `## Rules this +4. 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. The request says: end the reply with the verdict set in one fenced block tagged with the `fence` value, in @@ -19,7 +24,7 @@ Append to every dispatched prompt, in this order, after its filled template: "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 +5. 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/self-review/the-self-review-graph/dispatch-reviewer.block.md b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/dispatch-reviewer.block.md index 1d281a826..31c1a5628 100644 --- a/plugins/mattstack/attachments/review/self-review/the-self-review-graph/dispatch-reviewer.block.md +++ b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/dispatch-reviewer.block.md @@ -11,5 +11,6 @@ dispatch, never skipped, however small the change. The commit range (merge base to HEAD) covers commits only. When the tree has uncommitted changes or untracked files, write the self-review diff to `self-review.diff` in the review scratch dir (one new directory in this -session's scratchpad, made here when none exists yet) and pass its path as -a setup observation: the reviewer judges that file, not the range. +session's scratchpad, made here when none exists yet) and hand it to the +dispatch as the diff file (Review dispatch's Standard blocks), so the +reviewer judges that file, not the range. diff --git a/plugins/mattstack/attachments/review/self-review/the-self-review-graph/find-merge-base.block.md b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/find-merge-base.block.md index 7959ee944..7e637a6e1 100644 --- a/plugins/mattstack/attachments/review/self-review/the-self-review-graph/find-merge-base.block.md +++ b/plugins/mattstack/attachments/review/self-review/the-self-review-graph/find-merge-base.block.md @@ -10,9 +10,10 @@ through the working tree, uncommitted changes and untracked files included, built read-only: 1. `git diff `: the commits and the uncommitted changes. -2. Then, for each path `git ls-files --others --exclude-standard` prints, - append `git diff --no-index /dev/null `. It exits 1 when it prints - a diff; that is not a failure. +2. Then read the NUL-separated paths `git ls-files -z --others + --exclude-standard` prints, and for each one append `git diff --no-index + -- /dev/null `. It exits 1 when it prints a diff; that is not a + failure. -Ignored files stay out. Nothing is staged and no file changes: never `git -add` or `git add -N` to make a new file show up. +Ignored files stay out. Nothing is staged and no file changes: +never `git add` or `git add -N` to make a new file show up. From 539bb19c9d4a7b88329351431a942aaf53a763cd Mon Sep 17 00:00:00 2001 From: Matthew Goodwin Date: Wed, 28 Oct 2026 22:24:37 -0500 Subject: [PATCH 3/3] self-review: the selector's diff is rebuilt on every pass Co-Authored-By: Claude Opus 5.5 --- .../self-review/the-self-review-graph/select-rules.block.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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 index fbdde1b0f..2048dceef 100644 --- 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 @@ -6,6 +6,8 @@ node: select_rules Rule selector's Select the rules, run once the depth checks are done. The diff it gets on stdin is the self-review diff (Find the self-review -merge-base), untracked files included, so the rules match what the -reviewer judges. With no selector bound, or `selector: none`, skip it and +merge-base), untracked files included, built afresh from the tree as it +stands on every pass (an iteration that skips Find the self-review +merge-base reuses its base, never an earlier diff), so the rules match +what the reviewer judges. With no selector bound, or `selector: none`, skip it and go on to the repro check.