From 8dddaaf20cc4f08587d5baa3db6c4dae9c371cb5 Mon Sep 17 00:00:00 2001 From: Samuel Denani Date: Mon, 17 Aug 2026 15:44:03 -0300 Subject: [PATCH 01/16] docs: design spec for grill-rfc architect + slice review phases Adds an architect phase (boundary-only mermaid, posted as an issue comment, user-approved), a reviewed slice phase (set-reviewer before parallel slice-reviewers, drafts as files until approval), and a native task spine for run visibility. Co-Authored-By: Claude Opus 5 (1M context) --- ...l-rfc-architect-and-slice-review-design.md | 378 ++++++++++++++++++ 1 file changed, 378 insertions(+) create mode 100644 docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-slice-review-design.md diff --git a/docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-slice-review-design.md b/docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-slice-review-design.md new file mode 100644 index 0000000..17b1e72 --- /dev/null +++ b/docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-slice-review-design.md @@ -0,0 +1,378 @@ +# Grill-RFC: an architect phase, a reviewed slice phase, and a task spine + +**Date:** 2026-08-17 +**Status:** Approved design, pending implementation plan + +## Problem + +`/grill-rfc` goes from a settled design tree straight to task sub-issues in +one unreviewed step. Three gaps follow from that. + +**No visual record of structure.** The grill settles boundaries — what talks +to what, who owns which state, what crosses a process line — and then records +them only as prose. Boundaries are the part of a design most worth seeing and +the part prose hides best. + +**The breakdown is a single unchecked judgment.** It is made by the session +that just spent an hour grilling, which is the context most committed to its +own conclusions and least able to notice what it omitted. An oversized task is +expensive downstream: it becomes a plan whose steps `/execute-issue` loops +over one dispatch at a time, and a fat task is exactly where the orchestrator +loses the thread on context size. Fixing that in the execution phase costs +coder dispatches that read the codebase; fixing it here costs a prose review. + +**The run is opaque while it happens.** Long stretches of agent work pass with +no indication of where the session is. + +## Goal + +Three additions to `/grill-rfc`: + +1. An **architect phase** that draws boundaries — and only boundaries — as + mermaid, for visual approval. +2. A **reviewed slice phase**: sub-issues are drafted granular by + construction, then reviewed by agents before any issue is created. +3. A **native task spine** so the run is legible while it runs. + +**Non-goals.** No changes to `/execute-issue` or `/babysit-pr`. No +architecture committed to the repo. No resume ledger for `grill-rfc` (see +Known limitations). + +## Decisions made + +- **Architecture is a gate, not documentation.** The agent draws; the user + reviews visually and approves. A correction that reveals an unsettled + decision reopens that branch on the grill frontier. +- **The contract test decides what earns a diagram**, and diagrams group by + boundary rather than by decision. Grouping is what caps the count. +- **Architecture lives in an issue comment and nowhere else** — not in the + refined RFC body, not in the repo. +- **The set reviewer runs before the per-task reviewers**, not in parallel + with them. +- **Task size is "one seam, one vertical slice"**, with a stated fallback when + the architect phase finds no boundaries. +- **Reviewers review only.** The orchestrator session adjudicates and rewrites. + +## Trust model + +Three reviewer agents are added. The reason that is not just a regress — +agents checking agents — is that a second opinion only buys something when its +failure mode differs from the first's. Same model, same input, same framing +produces agreement, not verification. Four rules make the difference concrete, +and they are binding on the implementation. + +**1. Every added reviewer must have an independent failure mode.** Rated +against that standard: + +| Reviewer | Independence | Basis | +|---|---|---| +| `set-reviewer` | Strong | Sees what a per-task view structurally cannot contain | +| `slice-reviewer` | Medium | Fresh context, no sunk cost, and its rule is countable | +| `boundary-reviewer` | Weak | Same input, same model, only the objective inverted | + +**2. Verdicts must be falsifiable.** Every finding names a specific artifact — +a draft file, a numbered acceptance criterion, a named seam, a line of the +RFC's scope — and the rule it violates. A finding that cannot point at one is +not a finding. `no change` is a valid and expected verdict; a reviewer must +never manufacture a finding to appear useful. + +**3. The human review surface stays flat regardless of agent count.** Reviewer +verdicts go to the orchestrator and never reach the user raw. The user sees +artifacts and a one-line changelog per reviewer round. Exactly **one approval +gate is added** — the diagrams; the final-set approval already existed. +Everything else the user receives is output they do not have to answer: the +ledger, the draft set, and the changelog lines. + +**4. `boundary-reviewer` is the designated first deletion candidate.** It is +the weakest of the three. Its changelog line is the evidence: if it reports +`no change` across three RFCs, delete it and rely on the mandatory exclusion +list alone. This is a note to the maintainer, not runtime behaviour — the only +runtime enforcement is rule 2, which stops it inventing findings to survive. + +### The economic bet + +Everything added here reads prose: issue text, a ledger, draft task bodies. +None of it reads the codebase. `/execute-issue` dispatches a planner plus a +coder plus a reviewer *per plan step*, all of which do. If granular slicing +prevents even one five-round fix loop over there, this entire review layer is +paid for several times over. + +That trade is asserted, not measured. It becomes falsifiable through the +changelog lines, which record what each reviewer actually changed. + +## Phase 2c — Architect + +Runs after the reverse grill (§2b), only once both directions hold: the +frontier is empty and the user's answers match the settled design. + +### Step 1 — Extract the boundary ledger + +Walk the settled design tree. A decision earns a place only if it creates or +changes something two parties must agree on: + +- a new or changed module, service, or process +- a public interface or API shape +- a data contract or schema +- ownership of persisted state +- a deployment or trust boundary + +Algorithm choice, library choice, naming, and file layout never qualify. +**When a decision is arguably internal, it is internal.** The phase is biased +toward producing output; the rule exists to counteract that. + +Group the qualifying decisions **by seam**, not by decision. Several decisions +touching `app ↔ Redis` collapse into one boundary. + +The ledger names both lists: + +``` +BOUNDARIES + app <-> Redis — decisions: workers read from Redis; Redis holds job state + client <-> API — decisions: expose GET /jobs/:id + +EXCLUDED (internal) + use a worker pool — no party outside the module observes it + retry with exponential backoff — implementation of an existing contract +``` + +The exclusion list is **mandatory and reasoned**. It converts a silent +judgment into an explicit claim the user reads in five seconds during the +approval they are already making. + +**Zero boundaries is a legitimate outcome.** Report it plainly and skip to +§3 — many RFCs (tuning gate thresholds, restructuring docs) are correctly +boundary-free. + +### Step 2 — Review the extraction + +Dispatch `boundary-reviewer` with the settled design tree and the ledger. It +judges the extraction only: is anything excluded actually a seam, is anything +included actually internal, are two listed boundaries the same seam. It is +told the drafting session's bias explicitly. It reviews only; the orchestrator +adjudicates and revises the ledger. + +Running before the drawing means a boundary that should not exist is never +drawn and never shown. + +### Step 3 — Draw + +One diagram per boundary, type chosen to show what was actually decided: + +- `flowchart` — structural seams: who talks to what, what crosses +- `sequenceDiagram` — the decision was about ordering or handshake across the seam +- `erDiagram` — the boundary is a data contract + +Each diagram carries two to four lines: what crosses the seam, who owns what, +and which settled decisions it encodes. + +**Conservative mermaid subset**, because no validator exists in this +environment and invalid syntax renders as a broken code block on GitHub at +exactly the moment the user is meant to be approving: + +- alphanumeric node ids only +- no `%%{init}%%` directives +- no `classDef`, `style`, or `click` +- no nested subgraphs +- quote any label containing punctuation + +### Step 4 — Post and gate + +All diagrams post as **one comment** on the RFC issue; revisions edit that +comment in place rather than stacking new ones. The user receives the ledger +and the comment link in the terminal. + +Two gates: + +- **The user's.** Approve, or say what is wrong. A correction that reveals a + decision which was never actually settled reopens that branch on the §2 + frontier. +- **The phase's own.** If a seam cannot be drawn without inventing a fact + nobody decided, that is a frontier question. Stop and ask; do not guess. + +### Why the comment and not the body + +`planner.md` reads the parent RFC via `gh issue view`, which returns the body +and not comments. Keeping architecture out of the body means a drifted diagram +physically cannot become the input a later planner slices from — the failure +mode of treating ADRs as durable planning input. + +The architecture is **scaffolding**: its consumers are the slice phase, which +needs the seams, and the user's eyes. Its job ends when the sub-issues are +created. Nothing downstream is supposed to read it, so it cannot mislead when +it rots. Re-running `/grill-rfc` posts a newer comment; latest wins. + +**Accepted cost:** someone reading only the RFC body never sees a diagram. + +## Phase 4 — Drafting and reviewing the slices + +Replaces the current §4. + +### Drafts are files, not issues + +Everything before approval lives in scratch files (`draft-.md`), never +on GitHub. Splitting and merging real issues leaves orphans and broken +relationships; splitting a file is free. Reviewers receive a **path**, not +pasted content — cheaper, and it keeps their context small. + +Issues are created only after user approval, via the existing `gh issue +create` plus GraphQL `addSubIssue` / `addBlockedBy` mechanics. + +If the user rejects the set at that final approval, their objection is treated +as a set-level finding: the orchestrator applies it, re-runs `set-reviewer` +once on the revised set, slice-reviews only newly-born tasks, and presents +again. This is the same convergence rule as the re-entry above, and it is not +subject to the one-re-entry cap — a user objection is never parked. + +### The size rule + +Applied at **drafting** time first and review time second. The loops this +design exists to minimise are cheapest to avoid by not drafting a fat task. +Where the ledger has seams, they are the slice lines. + +A draft task is correctly sized when all three hold: + +1. **One seam.** It crosses at most one boundary from the ledger. + *Zero-boundary fallback:* it changes exactly one observable behaviour of + the system, and every acceptance criterion describes that one behaviour. +2. **Co-true criteria.** Three to seven acceptance checkboxes that are all + true or all false together. +3. **Vertical slice.** Test plus implementation plus wiring, shippable alone. + Never a horizontal layer such as "define all the types". + +### The review pipeline + +``` +draft (granular by construction) + └─ log the full set to the user ← visibility, no approval asked +set-reviewer (refined RFC + ledger + whole draft set) + └─ orchestrator adjudicates → rewrite → changelog line +slice-reviewers (N in parallel; each: ONE draft + RFC + ledger) + └─ orchestrator adjudicates → rewrite → changelog line +if membership changed (any split or merge): + set-reviewer once more on the revised set + + slice-review for newly-born tasks only + └─ cap: one re-entry +user approval → create issues → link parent / blockedBy +``` + +**Set before slices, deliberately.** Set findings change *which tasks exist*. +Run the two in parallel and every slice review of a doomed task is wasted +while the task the set reviewer invents gets no review at all — a second round +becomes guaranteed whenever the set reviewer finds anything. Sequential makes +the second round the exception. It also raises slice-review accuracy: "is this +one seam?" is ambiguous while the task is still a merge candidate. + +**Convergence.** A split or merge from the slice round is itself a membership +change, so the set reviewer runs once more on the revised set and newly-born +tasks get their one slice review. Splits are local, so this converges fast. +Capped at one re-entry; the orchestrator then adjudicates residuals and +surfaces them in the approval message. + +### What each reviewer judges + +**`slice-reviewer`** — input: one draft file, the refined RFC, the ledger. It +does **not** see the other drafts; that is what keeps its context small and +its judgment independent, and coverage is explicitly not its job. It judges: +the three size-rule clauses, criteria testability, and faithfulness to RFC +intent. + +**`set-reviewer`** — input: the refined RFC, the ledger, the whole draft set. +It judges the set and only the set: + +1. **Coverage** — every line of the RFC's Scope maps to at least one task. +2. **Overlap** — no two tasks claim the same change. +3. **Minimality** — two tasks that always ship together across the same seam + are one task. This counter-pressure matters: without it "granular" drifts + into twenty tasks, and every task costs a branch, a PR, and a babysit loop. +4. **Edges** — `blockedBy` reflects real code or data dependency, not + narrative order. + +### Verdict format + +Every reviewer returns: + +``` +FINDING : — — +... +VERDICT: no change | findings +``` + +Legal artifacts: a draft file path, `:criterion `, a seam name from +the ledger, or a quoted line of the RFC's Scope section. + +### Adjudication and the changelog + +Reviewers never edit. The orchestrator applies, rejects with a reason, or +parks each finding. What the user sees is one line per reviewer round: + +``` +set-reviewer: 2 gaps, 1 overlap → added draft-migrate-jobs, merged draft-b + draft-c +slice-reviewers (6): 1 split → draft-api split into draft-api-read, draft-api-write +boundary-reviewer: no change +``` + +## The native task spine + +Created at §1 load, so the shape of the run is visible before the first +question: + +`Grill` · `Reverse grill` · `Architect` · `Rewrite RFC body` · +`Draft sub-issues` · `Review sub-issues` · `Create sub-issues` + +**One task per agent dispatch, and nowhere else.** Agent fan-outs are the only +stretches where visibility is genuinely missing — the user is not in the dark +during the grill, they are being asked questions. So: `boundary-reviewer` +under Architect; `set-reviewer` and the N parallel `Review: ` tasks +under Review sub-issues, created at dispatch when the count is known. The +re-entry round adds its own, suffixed `(round 2)`. + +**A skipped phase still completes.** On a zero-boundary RFC, `Architect` +completes with its label recording why ("no boundaries — 7 decisions internal") +rather than being deleted. A phase vanishing from the spine mid-run reads as a +bug; a phase that completed having done nothing reads as the answer it is. + +**Grill rounds get no tasks.** A task per round is unbounded and would bury +the spine. `Grill` stays one task carrying the round in its live label — +"Grilling: round 3, 4 questions open". `Architect` does the same with the seam +being drawn. + +**No dependency edges.** The spine is strictly sequential and reads that way +by id; the fan-out is parallel by definition. Wiring `blockedBy` would cost a +call per task and encode nothing already invisible. + +**Completed means adjudicated, not returned.** A reviewer's task completes +when the orchestrator has ruled on its findings. Otherwise the list goes green +while the work is still open. + +The task list answers *where*. The changelog answers *what changed*. Neither +repeats the other. + +## Files + +**New** — `.claude/agents/boundary-reviewer.md`, +`.claude/agents/set-reviewer.md`, `.claude/agents/slice-reviewer.md`. All +three are read-only, report-never-fix, matching the existing `reviewer.md` +contract shape. + +**Modified** — `.claude/skills/grill-rfc/SKILL.md` (new §2c, rewritten §4, +task spine in §1). `docs/loop-harness.md` (flow diagram and Pieces table). + +**Unchanged** — `execute-issue`, `babysit-pr`, `planner.md`, `coder.md`, +`reviewer.md`, the quality gate, the issue templates. + +## Known limitations + +**No resume.** Unlike `/execute-issue`, this skill keeps no ledger, so the +task spine dies with the session. Partial recovery exists — the refined issue, +the architecture comment, and the draft files survive — but a crashed grill +restarts the grill. Adding a ledger is a larger change than this design +covers. + +**Architecture goes stale.** Nothing updates the diagrams once execution +reveals the design was slightly off. This is accepted rather than solved: the +comment placement means nothing downstream reads them, and re-running +`/grill-rfc` refreshes them. + +**Mermaid is unvalidated.** The conservative subset reduces the risk; it does +not eliminate it. If broken renders show up in practice, the fix is a +validator, not more prose rules. From 8a9a42ff8246aa872fbbec1d768f0cac406dee9d Mon Sep 17 00:00:00 2001 From: Samuel Denani Date: Mon, 17 Aug 2026 15:53:55 -0300 Subject: [PATCH 02/16] docs: rename slice-reviewer to sub-issue-reviewer in the grill-rfc spec "slice" collides with existing vocabulary for features. The agent, the phase and the doc filename now say sub-issue; "vertical slice" stays in the size rule as the standard term for the property. Co-Authored-By: Claude Opus 5 (1M context) --- ...-architect-and-sub-issue-review-design.md} | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) rename docs/superpowers/specs/{2026-08-17-grill-rfc-architect-and-slice-review-design.md => 2026-08-17-grill-rfc-architect-and-sub-issue-review-design.md} (91%) diff --git a/docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-slice-review-design.md b/docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-sub-issue-review-design.md similarity index 91% rename from docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-slice-review-design.md rename to docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-sub-issue-review-design.md index 17b1e72..ba91882 100644 --- a/docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-slice-review-design.md +++ b/docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-sub-issue-review-design.md @@ -1,4 +1,4 @@ -# Grill-RFC: an architect phase, a reviewed slice phase, and a task spine +# Grill-RFC: an architect phase, a reviewed sub-issue phase, and a task spine **Date:** 2026-08-17 **Status:** Approved design, pending implementation plan @@ -30,7 +30,7 @@ Three additions to `/grill-rfc`: 1. An **architect phase** that draws boundaries — and only boundaries — as mermaid, for visual approval. -2. A **reviewed slice phase**: sub-issues are drafted granular by +2. A **reviewed sub-issue phase**: sub-issues are drafted granular by construction, then reviewed by agents before any issue is created. 3. A **native task spine** so the run is legible while it runs. @@ -67,7 +67,7 @@ against that standard: | Reviewer | Independence | Basis | |---|---|---| | `set-reviewer` | Strong | Sees what a per-task view structurally cannot contain | -| `slice-reviewer` | Medium | Fresh context, no sunk cost, and its rule is countable | +| `sub-issue-reviewer` | Medium | Fresh context, no sunk cost, and its rule is countable | | `boundary-reviewer` | Weak | Same input, same model, only the objective inverted | **2. Verdicts must be falsifiable.** Every finding names a specific artifact — @@ -93,7 +93,7 @@ runtime enforcement is rule 2, which stops it inventing findings to survive. Everything added here reads prose: issue text, a ledger, draft task bodies. None of it reads the codebase. `/execute-issue` dispatches a planner plus a -coder plus a reviewer *per plan step*, all of which do. If granular slicing +coder plus a reviewer *per plan step*, all of which do. If a granular breakdown prevents even one five-round fix loop over there, this entire review layer is paid for several times over. @@ -193,17 +193,17 @@ Two gates: `planner.md` reads the parent RFC via `gh issue view`, which returns the body and not comments. Keeping architecture out of the body means a drifted diagram -physically cannot become the input a later planner slices from — the failure +physically cannot become the input a later planner plans from — the failure mode of treating ADRs as durable planning input. -The architecture is **scaffolding**: its consumers are the slice phase, which +The architecture is **scaffolding**: its consumers are the sub-issue phase, which needs the seams, and the user's eyes. Its job ends when the sub-issues are created. Nothing downstream is supposed to read it, so it cannot mislead when it rots. Re-running `/grill-rfc` posts a newer comment; latest wins. **Accepted cost:** someone reading only the RFC body never sees a diagram. -## Phase 4 — Drafting and reviewing the slices +## Phase 4 — Drafting and reviewing the sub-issues Replaces the current §4. @@ -219,7 +219,7 @@ create` plus GraphQL `addSubIssue` / `addBlockedBy` mechanics. If the user rejects the set at that final approval, their objection is treated as a set-level finding: the orchestrator applies it, re-runs `set-reviewer` -once on the revised set, slice-reviews only newly-born tasks, and presents +once on the revised set, runs sub-issue review on newly-born tasks only, and presents again. This is the same convergence rule as the re-entry above, and it is not subject to the one-re-entry cap — a user objection is never parked. @@ -227,7 +227,7 @@ subject to the one-re-entry cap — a user objection is never parked. Applied at **drafting** time first and review time second. The loops this design exists to minimise are cheapest to avoid by not drafting a fat task. -Where the ledger has seams, they are the slice lines. +Where the ledger has seams, they are the split lines. A draft task is correctly sized when all three hold: @@ -246,31 +246,31 @@ draft (granular by construction) └─ log the full set to the user ← visibility, no approval asked set-reviewer (refined RFC + ledger + whole draft set) └─ orchestrator adjudicates → rewrite → changelog line -slice-reviewers (N in parallel; each: ONE draft + RFC + ledger) +sub-issue-reviewers (N in parallel; each: ONE draft + RFC + ledger) └─ orchestrator adjudicates → rewrite → changelog line if membership changed (any split or merge): set-reviewer once more on the revised set - + slice-review for newly-born tasks only + + sub-issue review for newly-born tasks only └─ cap: one re-entry user approval → create issues → link parent / blockedBy ``` -**Set before slices, deliberately.** Set findings change *which tasks exist*. -Run the two in parallel and every slice review of a doomed task is wasted +**Set before sub-issues, deliberately.** Set findings change *which tasks exist*. +Run the two in parallel and every sub-issue review of a doomed task is wasted while the task the set reviewer invents gets no review at all — a second round becomes guaranteed whenever the set reviewer finds anything. Sequential makes -the second round the exception. It also raises slice-review accuracy: "is this +the second round the exception. It also raises sub-issue review accuracy: "is this one seam?" is ambiguous while the task is still a merge candidate. -**Convergence.** A split or merge from the slice round is itself a membership +**Convergence.** A split or merge from the sub-issue round is itself a membership change, so the set reviewer runs once more on the revised set and newly-born -tasks get their one slice review. Splits are local, so this converges fast. +tasks get their one sub-issue review. Splits are local, so this converges fast. Capped at one re-entry; the orchestrator then adjudicates residuals and surfaces them in the approval message. ### What each reviewer judges -**`slice-reviewer`** — input: one draft file, the refined RFC, the ledger. It +**`sub-issue-reviewer`** — input: one draft file, the refined RFC, the ledger. It does **not** see the other drafts; that is what keeps its context small and its judgment independent, and coverage is explicitly not its job. It judges: the three size-rule clauses, criteria testability, and faithfulness to RFC @@ -307,7 +307,7 @@ parks each finding. What the user sees is one line per reviewer round: ``` set-reviewer: 2 gaps, 1 overlap → added draft-migrate-jobs, merged draft-b + draft-c -slice-reviewers (6): 1 split → draft-api split into draft-api-read, draft-api-write +sub-issue-reviewers (6): 1 split → draft-api split into draft-api-read, draft-api-write boundary-reviewer: no change ``` @@ -350,7 +350,7 @@ repeats the other. ## Files **New** — `.claude/agents/boundary-reviewer.md`, -`.claude/agents/set-reviewer.md`, `.claude/agents/slice-reviewer.md`. All +`.claude/agents/set-reviewer.md`, `.claude/agents/sub-issue-reviewer.md`. All three are read-only, report-never-fix, matching the existing `reviewer.md` contract shape. From 56826bbc9f18a9b4faa6dded250ec5b4f2cc255a Mon Sep 17 00:00:00 2001 From: Samuel Denani Date: Mon, 17 Aug 2026 16:19:37 -0300 Subject: [PATCH 03/16] docs: implementation plan for grill-rfc architect + sub-issue review Seven tasks: three read-only reviewer agents, three grill-rfc sections (architect phase, drafting/review pipeline, task spine), and the loop-harness doc update. Co-Authored-By: Claude Opus 5 (1M context) --- ...rill-rfc-architect-and-sub-issue-review.md | 1020 +++++++++++++++++ 1 file changed, 1020 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-17-grill-rfc-architect-and-sub-issue-review.md diff --git a/docs/superpowers/plans/2026-08-17-grill-rfc-architect-and-sub-issue-review.md b/docs/superpowers/plans/2026-08-17-grill-rfc-architect-and-sub-issue-review.md new file mode 100644 index 0000000..7199770 --- /dev/null +++ b/docs/superpowers/plans/2026-08-17-grill-rfc-architect-and-sub-issue-review.md @@ -0,0 +1,1020 @@ +# Grill-RFC Architect Phase & Sub-Issue Review 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:** Add a boundary-only architect phase, an agent-reviewed sub-issue drafting phase, and a native task spine to the `/grill-rfc` skill. + +**Architecture:** Three new read-only reviewer agents in `.claude/agents/` plus a rewritten `/grill-rfc` skill that dispatches them. Architecture diagrams live in an RFC issue comment and never in the issue body, so a stale diagram cannot become planner input. Sub-issues are drafted as scratch files, reviewed set-first then per-draft, and only created on GitHub after user approval. + +**Tech Stack:** Markdown prompt documents (`.claude/agents/*.md`, `.claude/skills/*/SKILL.md`), `gh` CLI, GitHub GraphQL, mermaid, Claude Code native tasks (TaskCreate/TaskUpdate). + +**Spec:** `docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-sub-issue-review-design.md` + +## Global Constraints + +- **These deliverables are prose, not code.** No file in this plan is reached by `npm run typecheck`, `npm run lint`, or `npm test`. Verification is mechanical shell assertion (does the file exist, does the frontmatter parse, do cross-references resolve) plus reading the result. Do not invent unit tests for markdown. +- **Agent frontmatter shape** must match the existing `.claude/agents/reviewer.md`: `name`, `description`, `tools`, `model` — in that order, `---` delimited, `name` identical to the filename stem. +- **All three new agents are read-only.** `tools: Read, Grep, Glob` — no `Write`, no `Edit`, no `Bash`. The read-only property is load-bearing: the spec's trust model requires reviewers that cannot edit. +- **Model assignment** (decided in this plan, not the spec — flag it at review): `boundary-reviewer` opus, `set-reviewer` opus, `sub-issue-reviewer` sonnet. Rationale: the sub-issue reviewer runs N-in-parallel and is the cost driver, and its rule is the most mechanical of the three. +- **Verdict format is identical across all three agents**, verbatim: + ``` + FINDING : — — + VERDICT: no change | findings + ``` +- **`no change` must be stated as valid and expected** in every reviewer prompt. This is the runtime half of the spec's "falsifiable verdicts" rule and the only thing stopping a reviewer inventing findings to look useful. +- **Never add architecture to the RFC issue body.** Comment only. +- Commit messages end with: + ``` + Co-Authored-By: Claude Opus 5 (1M context) + ``` + +## File Structure + +| File | Responsibility | +|---|---| +| `.claude/agents/boundary-reviewer.md` (create) | Judges the boundary ledger's extraction only — manufactured seams, missed seams, duplicates, unreasoned exclusions. Never judges diagrams. | +| `.claude/agents/set-reviewer.md` (create) | Judges a draft set as a set — coverage, overlap, minimality, edges. The only agent that sees every draft. | +| `.claude/agents/sub-issue-reviewer.md` (create) | Judges ONE draft against the size rule and RFC intent. Never sees siblings. | +| `.claude/skills/grill-rfc/SKILL.md` (modify) | §1 gains the task spine; new §2c architect phase; §4 replaced by draft → review → approve → create; §5 report updated. | +| `docs/loop-harness.md` (modify) | Flow diagram and Pieces table reflect the new phases and agents. | + +--- + +### Task 1: `boundary-reviewer` agent + +**Files:** +- Create: `.claude/agents/boundary-reviewer.md` +- Reference (read first, do not modify): `.claude/agents/reviewer.md` + +**Interfaces:** +- Consumes: nothing — first task. +- Produces: agent name `boundary-reviewer`, dispatched by Task 4. Receives the settled design tree and the boundary ledger as prompt text. Returns the shared `FINDING`/`VERDICT` format, where each finding names a `BOUNDARIES` seam or an `EXCLUDED` decision. + +- [ ] **Step 1: Write the failing assertion** + +Run this now; it must fail because the file does not exist yet. + +```bash +cd /Users/samuel/Code/loopwright +test -f .claude/agents/boundary-reviewer.md \ + && grep -qx 'name: boundary-reviewer' .claude/agents/boundary-reviewer.md \ + && grep -qx 'tools: Read, Grep, Glob' .claude/agents/boundary-reviewer.md \ + && grep -qx 'model: opus' .claude/agents/boundary-reviewer.md \ + && grep -q 'VERDICT: no change' .claude/agents/boundary-reviewer.md \ + && echo PASS || echo FAIL +``` + +Expected: `FAIL` + +- [ ] **Step 2: Create the agent file** + +Write `.claude/agents/boundary-reviewer.md` with exactly this content: + +```markdown +--- +name: boundary-reviewer +description: Reviews the boundary ledger produced by grill-rfc's architect phase — judges the extraction only (what was included, what was excluded), never the diagrams. Read-only; reports findings, never fixes. Dispatched by grill-rfc. +tools: Read, Grep, Glob +model: opus +--- + +You review one thing: a **boundary ledger** extracted from a settled RFC +design tree, before any diagram is drawn. + +You receive the settled design tree (the decisions the grill produced) and the +ledger, which has two lists — BOUNDARIES (each a named seam with the decisions +folded into it) and EXCLUDED (each an internal decision with the reason it was +judged internal). + +## The bias you exist to correct + +The session that wrote this ledger just spent an hour settling the design and +is about to draw diagrams. It is motivated to find boundaries, because a +ledger with entries justifies the phase. **Your default suspicion is that +BOUNDARIES contains something manufactured** — though you check both lists. + +## The test + +A decision belongs in BOUNDARIES only if it creates or changes something two +parties must agree on: + +- a new or changed module, service, or process +- a public interface or API shape +- a data contract or schema +- ownership of persisted state +- a deployment or trust boundary + +Algorithm choice, library choice, naming and file layout never qualify. When a +decision is arguably internal, it **is** internal. + +## What to judge + +1. **Manufactured boundaries** — a BOUNDARIES entry whose decisions all fail + the test above. +2. **Missed boundaries** — an EXCLUDED entry that does meet the test. Name the + party on the other side of the seam. +3. **Duplicate or conflated seams** — two entries that are the same seam under + different names, or one entry that is really two seams. +4. **Unreasoned exclusions** — an EXCLUDED entry whose stated reason does not + explain why no second party observes it. + +You do **not** judge diagram type, wording, the design itself, or whether the +decisions are good ones. Only the extraction. + +## Output + +``` +FINDING : — — +VERDICT: no change | findings +``` + +Every finding must name an entry that literally appears in the ledger. A +finding you cannot anchor to one is not a finding — drop it. + +**"The ledger looks reasonable" is `VERDICT: no change`, and that is a good, +expected result.** Never manufacture a finding to appear useful: a reviewer +that invents work is worse than one that reports nothing. + +You never edit anything and you have no write tools. The orchestrator +adjudicates. +``` + +- [ ] **Step 3: Re-run the assertion** + +```bash +cd /Users/samuel/Code/loopwright +test -f .claude/agents/boundary-reviewer.md \ + && grep -qx 'name: boundary-reviewer' .claude/agents/boundary-reviewer.md \ + && grep -qx 'tools: Read, Grep, Glob' .claude/agents/boundary-reviewer.md \ + && grep -qx 'model: opus' .claude/agents/boundary-reviewer.md \ + && grep -q 'VERDICT: no change' .claude/agents/boundary-reviewer.md \ + && echo PASS || echo FAIL +``` + +Expected: `PASS` + +- [ ] **Step 4: Assert it has no write tools** + +```bash +cd /Users/samuel/Code/loopwright +grep -qE '^tools:.*(Write|Edit|Bash)' .claude/agents/boundary-reviewer.md \ + && echo "FAIL - has write tools" || echo "PASS - read-only" +``` + +Expected: `PASS - read-only` + +- [ ] **Step 5: Commit** + +```bash +cd /Users/samuel/Code/loopwright +git add .claude/agents/boundary-reviewer.md +git commit -F - <<'EOF' +feat: add boundary-reviewer agent + +Reviews grill-rfc's boundary ledger extraction before any diagram is +drawn. Read-only; adjudication stays with the orchestrator. + +Co-Authored-By: Claude Opus 5 (1M context) +EOF +``` + +--- + +### Task 2: `set-reviewer` agent + +**Files:** +- Create: `.claude/agents/set-reviewer.md` + +**Interfaces:** +- Consumes: the `FINDING`/`VERDICT` format established in Task 1. +- Produces: agent name `set-reviewer`, dispatched by Task 5. Receives the refined RFC body, the boundary ledger (possibly empty), and the paths of every `draft-.md`. Findings name a draft path or quote a line of the RFC's Scope section. + +- [ ] **Step 1: Write the failing assertion** + +```bash +cd /Users/samuel/Code/loopwright +test -f .claude/agents/set-reviewer.md \ + && grep -qx 'name: set-reviewer' .claude/agents/set-reviewer.md \ + && grep -qx 'model: opus' .claude/agents/set-reviewer.md \ + && grep -q 'minimality' .claude/agents/set-reviewer.md \ + && grep -q 'VERDICT: no change' .claude/agents/set-reviewer.md \ + && echo PASS || echo FAIL +``` + +Expected: `FAIL` + +- [ ] **Step 2: Create the agent file** + +Write `.claude/agents/set-reviewer.md` with exactly this content: + +```markdown +--- +name: set-reviewer +description: Reviews a whole set of draft sub-issues for grill-rfc — coverage gaps, overlaps, minimality and dependency edges. The only reviewer that sees the entire set. Read-only; reports findings, never fixes. Dispatched by grill-rfc. +tools: Read, Grep, Glob +model: opus +--- + +You review a **set** of draft sub-issues as a set. You are the only reviewer +positioned to say "nothing here builds the migration" — the per-draft +reviewers each see one file and cannot detect a gap by construction. + +You receive the refined RFC body, the boundary ledger (which may be empty), +and the path of every draft file (`draft-.md`). Read all of them before +judging anything. + +## What to judge + +1. **Coverage** — every line of the RFC's Scope section maps to at least one + draft. Quote the uncovered line. +2. **Overlap** — no two drafts claim the same change. Name both drafts and the + change they share. +3. **Minimality** — two drafts that would always ship together across the same + seam are one draft. This matters as much as splitting does: every extra + sub-issue costs a branch, a PR and a babysit loop, so an over-split set is + a real defect, not a safe one. +4. **Edges** — each proposed "blocked by" edge reflects a real code or data + dependency, not narrative order. "B reads the table A creates" is an edge; + "B feels like it comes second" is not. Flag invented edges and missing ones + with equal weight. + +You do **not** judge an individual draft's size, its acceptance criteria or +its wording. That is the sub-issue-reviewer's job, and duplicating it wastes +the one perspective only you have. + +## Output + +``` +FINDING : — — +VERDICT: no change | findings +``` + +Every finding names a draft path or quotes a line of the RFC. A finding you +cannot anchor to one is not a finding — drop it. + +`VERDICT: no change` is a valid and expected result. Never manufacture a +finding to appear useful. + +You never edit anything and you have no write tools. The orchestrator +adjudicates and rewrites the drafts. +``` + +- [ ] **Step 3: Re-run the assertion** + +```bash +cd /Users/samuel/Code/loopwright +test -f .claude/agents/set-reviewer.md \ + && grep -qx 'name: set-reviewer' .claude/agents/set-reviewer.md \ + && grep -qx 'model: opus' .claude/agents/set-reviewer.md \ + && grep -q 'minimality' .claude/agents/set-reviewer.md \ + && grep -q 'VERDICT: no change' .claude/agents/set-reviewer.md \ + && echo PASS || echo FAIL +``` + +Expected: `PASS` + +- [ ] **Step 4: Assert read-only** + +```bash +cd /Users/samuel/Code/loopwright +grep -qE '^tools:.*(Write|Edit|Bash)' .claude/agents/set-reviewer.md \ + && echo "FAIL - has write tools" || echo "PASS - read-only" +``` + +Expected: `PASS - read-only` + +- [ ] **Step 5: Commit** + +```bash +cd /Users/samuel/Code/loopwright +git add .claude/agents/set-reviewer.md +git commit -F - <<'EOF' +feat: add set-reviewer agent + +Judges a draft sub-issue set as a set — coverage, overlap, minimality +and dependency edges. The only reviewer that sees every draft. + +Co-Authored-By: Claude Opus 5 (1M context) +EOF +``` + +--- + +### Task 3: `sub-issue-reviewer` agent + +**Files:** +- Create: `.claude/agents/sub-issue-reviewer.md` + +**Interfaces:** +- Consumes: the `FINDING`/`VERDICT` format from Task 1. +- Produces: agent name `sub-issue-reviewer`, dispatched N-in-parallel by Task 5. Receives exactly one draft path plus the refined RFC and the ledger. Findings name the draft path or `:criterion `. When the verdict is "too big", the finding names the cut line. + +- [ ] **Step 1: Write the failing assertion** + +```bash +cd /Users/samuel/Code/loopwright +test -f .claude/agents/sub-issue-reviewer.md \ + && grep -qx 'name: sub-issue-reviewer' .claude/agents/sub-issue-reviewer.md \ + && grep -qx 'model: sonnet' .claude/agents/sub-issue-reviewer.md \ + && grep -q 'You do not see the other drafts' .claude/agents/sub-issue-reviewer.md \ + && grep -q 'VERDICT: no change' .claude/agents/sub-issue-reviewer.md \ + && echo PASS || echo FAIL +``` + +Expected: `FAIL` + +- [ ] **Step 2: Create the agent file** + +Write `.claude/agents/sub-issue-reviewer.md` with exactly this content: + +```markdown +--- +name: sub-issue-reviewer +description: Reviews ONE draft sub-issue for grill-rfc against the size rule and the RFC's intent. Never sees the other drafts. Read-only; reports findings, never fixes. Dispatched by grill-rfc, N in parallel. +tools: Read, Grep, Glob +model: sonnet +--- + +You review exactly **one** draft sub-issue. + +You receive the path to one draft file (`draft-.md`), the refined RFC +body, and the boundary ledger (which may be empty). + +You do not see the other drafts, and that is deliberate. Coverage, overlap and +dependency edges belong to the set-reviewer. Never speculate about drafts you +were not given. + +## The size rule + +The draft is correctly sized only if all three hold: + +1. **One seam.** It crosses at most one boundary from the ledger. + *If the ledger is empty:* it changes exactly one observable behavior of the + system, and every acceptance criterion describes that one behavior. +2. **Co-true criteria.** Between three and seven acceptance checkboxes, all + true together or all false together. A draft whose criteria could + plausibly be half-satisfied is two drafts. +3. **Vertical slice.** Test plus implementation plus wiring, shippable on its + own. A horizontal layer ("define all the types", "add the interfaces") is a + finding even when it is small. + +## Also judge + +4. **Criteria testability** — each checkbox states an observable outcome + someone could verify, not an activity. "Refactor the parser" is not a + criterion; "the parser accepts trailing commas" is. +5. **Faithfulness** — the draft's goal is something the RFC actually asked + for. Flag invented scope and goals that contradict the RFC alike. + +## Output + +``` +FINDING : or :criterion — — +VERDICT: no change | findings +``` + +When the finding is "too big", name **where to cut** — the seam or the +behavior boundary the split should follow. The orchestrator makes the call; +you supply the line. + +Every finding names the draft path or a numbered criterion within it. A +finding you cannot anchor to one is not a finding — drop it. + +`VERDICT: no change` is valid and expected. Never manufacture a finding to +appear useful. + +You never edit anything and you have no write tools. +``` + +- [ ] **Step 3: Re-run the assertion** + +```bash +cd /Users/samuel/Code/loopwright +test -f .claude/agents/sub-issue-reviewer.md \ + && grep -qx 'name: sub-issue-reviewer' .claude/agents/sub-issue-reviewer.md \ + && grep -qx 'model: sonnet' .claude/agents/sub-issue-reviewer.md \ + && grep -q 'You do not see the other drafts' .claude/agents/sub-issue-reviewer.md \ + && grep -q 'VERDICT: no change' .claude/agents/sub-issue-reviewer.md \ + && echo PASS || echo FAIL +``` + +Expected: `PASS` + +- [ ] **Step 4: Assert all three agents are read-only and consistently shaped** + +```bash +cd /Users/samuel/Code/loopwright +for a in boundary-reviewer set-reviewer sub-issue-reviewer; do + f=".claude/agents/$a.md" + grep -qE '^tools:.*(Write|Edit|Bash)' "$f" && echo "FAIL $a: write tools" && continue + grep -qx "name: $a" "$f" || { echo "FAIL $a: name mismatch"; continue; } + grep -q 'VERDICT: no change' "$f" || { echo "FAIL $a: no-change clause missing"; continue; } + echo "PASS $a" +done +``` + +Expected: three `PASS` lines. + +- [ ] **Step 5: Commit** + +```bash +cd /Users/samuel/Code/loopwright +git add .claude/agents/sub-issue-reviewer.md +git commit -F - <<'EOF' +feat: add sub-issue-reviewer agent + +Judges one draft sub-issue against the size rule and RFC intent, blind +to its siblings. Runs N-in-parallel, so it is on sonnet. + +Co-Authored-By: Claude Opus 5 (1M context) +EOF +``` + +--- + +### Task 4: Architect phase (`grill-rfc` §2c) + +**Files:** +- Modify: `.claude/skills/grill-rfc/SKILL.md` — insert a new section between §2b (ends at the line `...until the user confirms you have reached a shared understanding.`) and `## 3. Rewrite the issue`. + +**Interfaces:** +- Consumes: `boundary-reviewer` from Task 1. +- Produces: the **boundary ledger** — two lists, `BOUNDARIES` (named seams, each with the decisions folded into it) and `EXCLUDED` (internal decisions, each with a reason). Tasks 5 and 6 both consume it: the size rule's clause 1 counts seams in it, and it is passed to `set-reviewer` and `sub-issue-reviewer` on every dispatch. An empty `BOUNDARIES` list is a valid ledger and triggers the size rule's fallback. + +- [ ] **Step 1: Write the failing assertion** + +```bash +cd /Users/samuel/Code/loopwright +grep -q '^## 2c\. Architect the boundaries' .claude/skills/grill-rfc/SKILL.md \ + && grep -q 'boundary-reviewer' .claude/skills/grill-rfc/SKILL.md \ + && grep -q 'never enters the RFC body' .claude/skills/grill-rfc/SKILL.md \ + && echo PASS || echo FAIL +``` + +Expected: `FAIL` + +- [ ] **Step 2: Insert the new section** + +Insert immediately before the line `## 3. Rewrite the issue`: + +````markdown +## 2c. Architect the boundaries + +Only once both directions hold. Mark `Architect` in_progress. + +### Extract the ledger + +Walk the settled design tree. A decision earns a place only if it creates or +changes something **two parties must agree on**: + +- a new or changed module, service, or process +- a public interface or API shape +- a data contract or schema +- ownership of persisted state +- a deployment or trust boundary + +Algorithm choice, library choice, naming and file layout never qualify. When a +decision is arguably internal, it **is** internal — this phase is biased +toward producing output, and the rule exists to counteract that. + +Group qualifying decisions **by seam**, not by decision: several decisions +touching `app <-> Redis` collapse into one boundary. That grouping is what +caps the diagram count. + +``` +BOUNDARIES + app <-> Redis — decisions: workers read from Redis; Redis holds job state + client <-> API — decisions: expose GET /jobs/:id + +EXCLUDED (internal) + use a worker pool — no party outside the module observes it + retry with exponential backoff — implementation of an existing contract +``` + +The EXCLUDED list is **mandatory and reasoned**. It turns a silent judgment +into an explicit claim the user can scan in seconds. + +**Zero boundaries is a legitimate outcome.** Report it plainly, complete +`Architect` with the reason in its label ("no boundaries — 7 decisions +internal"), and go to §3. + +### Review the extraction + +Dispatch the **boundary-reviewer** agent (its own native task) with the +settled design tree and the ledger. Adjudicate every finding yourself — apply, +reject with a reason, or park. It reviews; you rewrite. + +Running it before the drawing means a boundary that should not exist is never +drawn and never shown. + +### Draw + +One diagram per boundary, type chosen to show what was decided: + +| Type | Use when | +|---|---| +| `flowchart` | structural seam — who talks to what, what crosses | +| `sequenceDiagram` | the decision was about ordering or handshake across the seam | +| `erDiagram` | the boundary is a data contract | + +Each diagram carries two to four lines: what crosses the seam, who owns what, +and which settled decisions it encodes. + +**Stay inside a conservative mermaid subset.** No validator exists here, and +invalid syntax renders as a broken code block on GitHub at exactly the moment +the user is meant to be approving: + +- alphanumeric node ids only +- no `%%{init}%%` directives +- no `classDef`, `style` or `click` +- no nested subgraphs +- quote any label containing punctuation + +### Post and gate + +All diagrams go in **one** comment on the RFC, so revisions edit it in place +instead of stacking: + +```bash +gh issue comment --body-file architecture.md # first post +gh issue comment --edit-last --body-file architecture.md # revisions +``` + +`--edit-last` targets your most recent comment. If the user has commented +since, capture the id from the URL the first post printed +(`...#issuecomment-`) and patch it directly instead: + +```bash +gh api --method PATCH /repos///issues/comments/ -F body=@architecture.md +``` + +Give the user the ledger and the comment URL, and ask for approval. + +Two gates: + +- **Theirs.** Approve, or say what is wrong. A correction that reveals a + decision which was never actually settled reopens that branch on the §2 + frontier — go back and grill it. +- **Yours.** If a seam cannot be drawn without inventing a fact nobody + decided, that is a frontier question. Stop and ask; never guess. + +The architecture stays in the comment and **never enters the RFC body**. The +planner in `/execute-issue` reads the body via `gh issue view`, which does not +return comments — so a diagram that drifts cannot become the input a later +planner plans from. The architecture is scaffolding: its consumers are the +sub-issue phase, which needs the seams, and the user's eyes. Its job ends when +the sub-issues are created. +```` + +- [ ] **Step 3: Re-run the assertion** + +```bash +cd /Users/samuel/Code/loopwright +grep -q '^## 2c\. Architect the boundaries' .claude/skills/grill-rfc/SKILL.md \ + && grep -q 'boundary-reviewer' .claude/skills/grill-rfc/SKILL.md \ + && grep -q 'never enters the RFC body' .claude/skills/grill-rfc/SKILL.md \ + && echo PASS || echo FAIL +``` + +Expected: `PASS` + +- [ ] **Step 4: Assert section order and that the agent it names exists** + +```bash +cd /Users/samuel/Code/loopwright +grep -n '^## ' .claude/skills/grill-rfc/SKILL.md +test -f .claude/agents/boundary-reviewer.md && echo "PASS agent exists" || echo "FAIL agent missing" +grep -q 'Architecture' .claude/skills/grill-rfc/SKILL.md && grep -q 'Refined RFC structure' .claude/skills/grill-rfc/SKILL.md \ + && grep -A3 'Refined RFC structure' .claude/skills/grill-rfc/SKILL.md | grep -q 'Architecture' \ + && echo "FAIL - architecture leaked into the RFC body structure" || echo "PASS - body structure unchanged" +``` + +Expected: sections in order `2c` before `3`; `PASS agent exists`; `PASS - body structure unchanged`. + +- [ ] **Step 5: Commit** + +```bash +cd /Users/samuel/Code/loopwright +git add .claude/skills/grill-rfc/SKILL.md +git commit -F - <<'EOF' +feat: add architect phase to grill-rfc + +Extracts a reasoned boundary ledger, has boundary-reviewer check the +extraction before anything is drawn, then posts mermaid diagrams as one +issue comment for visual approval. Never enters the RFC body, so a stale +diagram cannot become planner input. + +Co-Authored-By: Claude Opus 5 (1M context) +EOF +``` + +--- + +### Task 5: Sub-issue drafting and review (`grill-rfc` §4) + +**Files:** +- Modify: `.claude/skills/grill-rfc/SKILL.md` — replace the whole of `## 4. Break into task sub-issues` (from its heading down to, but not including, `## 5. Report`) with three sections `4`, `4b`, `4c`. + +**Interfaces:** +- Consumes: the boundary ledger from Task 4; `set-reviewer` from Task 2; `sub-issue-reviewer` from Task 3. +- Produces: draft files named `draft-.md`, and the changelog line format Task 6's spine text refers to. Preserves the existing `gh issue create` and GraphQL `addSubIssue`/`addBlockedBy` calls verbatim — they move into §4c unchanged. + +- [ ] **Step 1: Write the failing assertion** + +```bash +cd /Users/samuel/Code/loopwright +grep -q '^## 4b\. Review the sub-issues' .claude/skills/grill-rfc/SKILL.md \ + && grep -q '^## 4c\. Approve, then create' .claude/skills/grill-rfc/SKILL.md \ + && grep -q 'set-reviewer' .claude/skills/grill-rfc/SKILL.md \ + && grep -q 'sub-issue-reviewer' .claude/skills/grill-rfc/SKILL.md \ + && grep -q 'addBlockedBy' .claude/skills/grill-rfc/SKILL.md \ + && echo PASS || echo FAIL +``` + +Expected: `FAIL` + +- [ ] **Step 2: Replace §4 with the three new sections** + +````markdown +## 4. Draft the sub-issues + +Mark `Draft sub-issues` in_progress. Drafts are **files**, not issues — +`draft-.md` in a scratch directory. Nothing reaches GitHub until the +user approves: splitting or merging real issues leaves orphaned +relationships, while splitting a file is free. + +Draft them **granular from the start**. The review rounds below are a safety +net, not the mechanism — the cheapest way to minimise loops is to not draft a +fat sub-issue in the first place. Where the ledger has seams, they are the +split lines. + +A draft is correctly sized when all three hold: + +1. **One seam** — it crosses at most one boundary from the ledger. If the + ledger is empty: it changes exactly one observable behavior, and every + acceptance criterion describes that one behavior. +2. **Co-true criteria** — three to seven acceptance checkboxes, all true + together or all false together. +3. **Vertical slice** — test plus implementation plus wiring, shippable + alone. Never a horizontal layer such as "define all the types". + +Draft body: goal, acceptance criteria (checkboxes), pointers into the RFC, +and which drafts it is blocked by. + +Then **log the whole set to the user** — titles, one-line goals and the +dependency edges. This is visibility, not an approval gate: do not wait. + +## 4b. Review the sub-issues + +Mark `Review sub-issues` in_progress. Reviewers review only. You adjudicate +every finding — apply, reject with a reason, or park — and you do all the +rewriting. + +**First the set.** Dispatch the **set-reviewer** agent (its own native task) +with the refined RFC, the ledger and every draft path. It judges coverage, +overlap, minimality and edges. Adjudicate, then rewrite the drafts. + +Set findings change *which drafts exist*, which is why this runs first: in +parallel, every per-draft review of a doomed draft is wasted and the draft the +set-reviewer invents gets no review at all. + +**Then the drafts.** Dispatch one **sub-issue-reviewer** per draft, in +parallel, one native task each (`Review: `). Each gets exactly one +draft path plus the RFC and the ledger — never the other drafts. Adjudicate, +then rewrite. + +**Converge.** If the draft round produced any split or merge, the set changed: +run the set-reviewer once more on the revised set, and give each newly-born +draft its one sub-issue review. Cap at **one** re-entry; then adjudicate the +residuals yourself and carry them into the approval message. + +After each round give the user one line — never the raw verdicts: + +``` +set-reviewer: 2 gaps, 1 overlap → added draft-migrate-jobs, merged draft-b + draft-c +sub-issue-reviewers (6): 1 split → draft-api split into draft-api-read, draft-api-write +boundary-reviewer: no change +``` + +## 4c. Approve, then create + +Present the final set for approval — titles, one-line goals, dependency edges, +and any residual findings you parked. + +If the user rejects it, treat their objection as a set-level finding: apply +it, re-run the set-reviewer once on the revised set, sub-issue-review only +newly-born drafts, and present again. A user objection is never parked and is +not subject to the re-entry cap. + +On approval, mark `Create sub-issues` in_progress and create each one: + +```bash +gh issue create --title "Task: <title>" --label task --body-file draft-<slug>.md +``` + +Then link relationships via GraphQL (write queries to a temp file and use +`-F query=@file` — inline quoting breaks in fish): + +```bash +# node IDs +gh api graphql -F query='query { repository(owner:"<owner>", name:"<repo>") { issue(number:<N>) { id } } }' + +# parent/child (RFC -> task) +gh api graphql -F query='mutation { addSubIssue(input:{issueId:"<rfc-id>", subIssueId:"<task-id>"}) { issue { number } } }' + +# dependency (task blocked by another task) +gh api graphql -F query='mutation { addBlockedBy(input:{issueId:"<blocked-id>", blockingIssueId:"<blocker-id>"}) { issue { number } } }' +``` +```` + +- [ ] **Step 3: Re-run the assertion** + +```bash +cd /Users/samuel/Code/loopwright +grep -q '^## 4b\. Review the sub-issues' .claude/skills/grill-rfc/SKILL.md \ + && grep -q '^## 4c\. Approve, then create' .claude/skills/grill-rfc/SKILL.md \ + && grep -q 'set-reviewer' .claude/skills/grill-rfc/SKILL.md \ + && grep -q 'sub-issue-reviewer' .claude/skills/grill-rfc/SKILL.md \ + && grep -q 'addBlockedBy' .claude/skills/grill-rfc/SKILL.md \ + && echo PASS || echo FAIL +``` + +Expected: `PASS` + +- [ ] **Step 4: Assert the GraphQL mechanics survived the rewrite and every named agent exists** + +```bash +cd /Users/samuel/Code/loopwright +for m in addSubIssue addBlockedBy 'gh issue create'; do + grep -q "$m" .claude/skills/grill-rfc/SKILL.md && echo "PASS kept: $m" || echo "FAIL lost: $m" +done +for a in boundary-reviewer set-reviewer sub-issue-reviewer; do + grep -q "$a" .claude/skills/grill-rfc/SKILL.md \ + && test -f ".claude/agents/$a.md" \ + && echo "PASS resolves: $a" || echo "FAIL dangling: $a" +done +``` + +Expected: three `PASS kept` lines and three `PASS resolves` lines. + +- [ ] **Step 5: Commit** + +```bash +cd /Users/samuel/Code/loopwright +git add .claude/skills/grill-rfc/SKILL.md +git commit -F - <<'EOF' +feat: review sub-issue drafts before creating them in grill-rfc + +Drafts land as scratch files, get reviewed set-first then per-draft in +parallel, and only reach GitHub after approval. Size rule is a drafting +constraint first so the review rounds stay the exception. + +Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> +EOF +``` + +--- + +### Task 6: Native task spine (`grill-rfc` §1 and §5) + +**Files:** +- Modify: `.claude/skills/grill-rfc/SKILL.md` — append to `## 1. Load`, and rewrite `## 5. Report`. + +**Interfaces:** +- Consumes: phase names established by Tasks 4 and 5 — the spine's labels must match the phases those tasks named (`Architect`, `Draft sub-issues`, `Review sub-issues`, `Create sub-issues`). +- Produces: nothing downstream. Final task. + +- [ ] **Step 1: Write the failing assertion** + +```bash +cd /Users/samuel/Code/loopwright +grep -q 'native task spine' .claude/skills/grill-rfc/SKILL.md \ + && grep -q 'adjudicated' .claude/skills/grill-rfc/SKILL.md \ + && echo PASS || echo FAIL +``` + +Expected: `FAIL` + +- [ ] **Step 2: Append the spine to §1** + +Add at the end of `## 1. Load`, after the "design tree below — its central decision is the root." paragraph: + +```markdown +Then create the run's **native task spine** (TaskCreate), in this order: +`Grill` · `Reverse grill` · `Architect` · `Rewrite RFC body` · +`Draft sub-issues` · `Review sub-issues` · `Create sub-issues`. No dependency +edges — the spine is sequential and reads that way by id. It exists so the +user can see the shape of the run before the first question. + +Keep it live for the rest of the session. A phase goes in_progress when it +starts and completed when its output is **adjudicated**, not when an agent +replies — otherwise the list goes green while the work is still open. A phase +that legitimately did nothing still completes, carrying the reason in its +label ("no boundaries — 7 decisions internal"); a phase vanishing mid-run +reads as a bug. + +**One task per agent dispatch, and nowhere else.** Agent fan-outs are the only +stretches where the user is in the dark — during the grill they are being +asked questions. So `Grill` and `Architect` carry their progress in the task +label ("Grilling: round 3, 4 questions open") rather than spawning a task per +round: a task per round is unbounded and would bury the spine. +``` + +- [ ] **Step 3: Rewrite §5** + +Replace the whole of `## 5. Report` with: + +```markdown +## 5. Report + +Complete the last spine task, then give the user: the link to the refined RFC, +the link to the architecture comment (or "no boundaries" if the phase found +none), the created sub-issues with their dependency edges, every residual +finding you parked, and which task is unblocked and ready for +`/execute-issue`. +``` + +- [ ] **Step 4: Re-run the assertion and check spine labels match the phases** + +```bash +cd /Users/samuel/Code/loopwright +grep -q 'native task spine' .claude/skills/grill-rfc/SKILL.md \ + && grep -q 'adjudicated' .claude/skills/grill-rfc/SKILL.md \ + && echo PASS || echo FAIL +for p in 'Architect' 'Draft sub-issues' 'Review sub-issues' 'Create sub-issues'; do + c=$(grep -c "$p" .claude/skills/grill-rfc/SKILL.md) + test "$c" -ge 2 && echo "PASS spine+phase agree: $p ($c)" || echo "FAIL orphan label: $p ($c)" +done +``` + +Expected: `PASS`, then four `PASS spine+phase agree` lines (each name appears both in the spine list and in the phase that uses it). + +- [ ] **Step 5: Commit** + +```bash +cd /Users/samuel/Code/loopwright +git add .claude/skills/grill-rfc/SKILL.md +git commit -F - <<'EOF' +feat: add a native task spine to grill-rfc + +Seven phase tasks created at load, plus one task per agent dispatch. +Grill rounds stay in the task label rather than spawning tasks, and a +phase completes only once its output is adjudicated. + +Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> +EOF +``` + +--- + +### Task 7: Update `docs/loop-harness.md` + +**Files:** +- Modify: `docs/loop-harness.md` — the `RFC issue (label: rfc)` block of the Flow diagram, and the Agents row of the Pieces table. + +**Interfaces:** +- Consumes: everything. Documentation task, runs last. +- Produces: nothing. + +- [ ] **Step 1: Write the failing assertion** + +```bash +cd /Users/samuel/Code/loopwright +grep -q 'boundary-reviewer' docs/loop-harness.md \ + && grep -q 'set-reviewer' docs/loop-harness.md \ + && grep -q 'sub-issue-reviewer' docs/loop-harness.md \ + && echo PASS || echo FAIL +``` + +Expected: `FAIL` + +- [ ] **Step 2: Update the flow diagram** + +Replace these three lines in the `RFC issue (label: rfc)` block: + +``` + │ /grill-rfc <N> — agent interrogates, rewrites the body as the refined + │ RFC (original preserved as a comment), creates task sub-issues with + │ dependencies (native parent/sub-issue + blocked-by relationships) +``` + +with: + +``` + │ /grill-rfc <N> — agent interrogates, then: + │ architect phase → boundary ledger, boundary-reviewer checks the + │ extraction, mermaid per seam posted as ONE issue + │ comment for visual approval (never the body) + │ rewrites the body as the refined RFC (original kept as a comment) + │ drafts sub-issues as files → set-reviewer (whole set) → N + │ sub-issue-reviewers (one draft each, parallel) → your approval + │ creates the sub-issues with native parent + blocked-by edges +``` + +- [ ] **Step 3: Update the Pieces table** + +Replace this row: + +``` +| Agents (planner / coder / reviewer) | `.claude/agents/` | +``` + +with: + +``` +| Execution agents (planner / coder / reviewer) | `.claude/agents/` | +| Refinement agents (boundary-reviewer / set-reviewer / sub-issue-reviewer) | `.claude/agents/` | +``` + +- [ ] **Step 4: Add a rule to "Rules of the loop"** + +Append as a new bullet at the end of the `## Rules of the loop` list: + +```markdown +- **Architecture is scaffolding, not a spec.** The architect phase's mermaid + lives in an RFC issue comment and never in the issue body, because + `planner.md` reads the body via `gh issue view` and comments are not + returned. Its consumers are the sub-issue slicing step and the human eye; + its job ends when the sub-issues are created. Nothing downstream reads it, + so it cannot mislead when it drifts. +``` + +- [ ] **Step 5: Re-run the assertion and verify the whole feature is coherent** + +```bash +cd /Users/samuel/Code/loopwright +grep -q 'boundary-reviewer' docs/loop-harness.md \ + && grep -q 'set-reviewer' docs/loop-harness.md \ + && grep -q 'sub-issue-reviewer' docs/loop-harness.md \ + && echo PASS || echo FAIL + +# every agent named anywhere in .claude/ resolves to a file +for a in planner coder reviewer boundary-reviewer set-reviewer sub-issue-reviewer; do + test -f ".claude/agents/$a.md" && echo "PASS $a" || echo "FAIL $a missing" +done + +# the gate still passes — these are prose files, so it must be untouched +npm run quality +``` + +Expected: `PASS`, six `PASS <agent>` lines, and a green quality gate. + +- [ ] **Step 6: Commit** + +```bash +cd /Users/samuel/Code/loopwright +git add docs/loop-harness.md +git commit -F - <<'EOF' +docs: document the architect and sub-issue review phases + +Flow diagram and Pieces table cover the three refinement agents, plus a +loop rule recording why architecture lives in a comment and not the RFC +body. + +Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> +EOF +``` + +--- + +## Self-Review + +**1. Spec coverage** + +| Spec section | Task | +|---|---| +| Trust model — independence table | Tasks 1–3 (model + tools + scope per agent) | +| Trust model — falsifiable verdicts | Tasks 1–3, `VERDICT: no change` asserted in every agent's Step 3 | +| Trust model — flat human surface | Task 5 (changelog lines, never raw verdicts) | +| Trust model — boundary-reviewer probation | **Gap → see below** | +| Phase 2c step 1, ledger + exclusions | Task 4 | +| Phase 2c step 2, extraction review | Tasks 1 + 4 | +| Phase 2c step 3, diagram types + mermaid subset | Task 4 | +| Phase 2c step 4, one comment + two gates | Task 4 | +| Why comment not body | Tasks 4 and 7 | +| Drafts are files | Task 5 | +| Size rule (3 clauses + fallback) | Tasks 3 and 5 | +| Review pipeline, set-before-drafts | Tasks 2, 3, 5 | +| Convergence + one re-entry cap | Task 5 | +| Verdict format | Tasks 1–3 | +| Adjudication + changelog | Task 5 | +| User rejection path | Task 5 (§4c) | +| Task spine | Task 6 | +| Skipped phase still completes | Task 6 | +| Files section | Tasks 1–7 | + +**Gap found and closed:** the spec's probation note for `boundary-reviewer` had no task. It is a one-sentence maintainer note, so rather than a task of its own it is folded into Task 7 Step 4 — add this second bullet alongside the architecture rule: + +```markdown +- **`boundary-reviewer` is the first thing to cut.** It is the weakest of the + three refinement agents: same input and model as the session that wrote the + ledger, only the objective inverted. Its changelog line is the evidence — if + it reports `no change` across three RFCs, delete it and rely on the ledger's + mandatory reasoned exclusion list alone. +``` + +**2. Placeholder scan:** no TBD/TODO. Every file's full content is inline; no task says "similar to Task N". + +**3. Type consistency:** agent names are identical across frontmatter, dispatch text, assertions and docs (`boundary-reviewer`, `set-reviewer`, `sub-issue-reviewer` — never `slice-reviewer`). Draft filenames are `draft-<slug>.md` in Tasks 5 and 3. Phase labels in Task 6's spine match the phases Tasks 4 and 5 mark in_progress, and Task 6 Step 4 asserts that mechanically. The verdict format string is byte-identical in Tasks 1, 2 and 3. + +**Known risk not solved here:** Task 4's `--edit-last` targets the dispatcher's most recent comment, which is wrong if the user comments in between. The fallback PATCH-by-id is documented in the same step; if this misfires in practice, make the id capture mandatory rather than the fallback. From ce5f66432a880fdfa5007f0d9ab9a37edd9ba7cd Mon Sep 17 00:00:00 2001 From: Samuel Denani <samuel.denani@outlook.com> Date: Mon, 17 Aug 2026 16:23:58 -0300 Subject: [PATCH 04/16] feat: add boundary-reviewer agent Reviews grill-rfc's boundary ledger extraction before any diagram is drawn. Read-only; adjudication stays with the orchestrator. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --- .claude/agents/boundary-reviewer.md | 66 +++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 .claude/agents/boundary-reviewer.md diff --git a/.claude/agents/boundary-reviewer.md b/.claude/agents/boundary-reviewer.md new file mode 100644 index 0000000..4e1d32d --- /dev/null +++ b/.claude/agents/boundary-reviewer.md @@ -0,0 +1,66 @@ +--- +name: boundary-reviewer +description: Reviews the boundary ledger produced by grill-rfc's architect phase — judges the extraction only (what was included, what was excluded), never the diagrams. Read-only; reports findings, never fixes. Dispatched by grill-rfc. +tools: Read, Grep, Glob +model: opus +--- + +You review one thing: a **boundary ledger** extracted from a settled RFC +design tree, before any diagram is drawn. + +You receive the settled design tree (the decisions the grill produced) and the +ledger, which has two lists — BOUNDARIES (each a named seam with the decisions +folded into it) and EXCLUDED (each an internal decision with the reason it was +judged internal). + +## The bias you exist to correct + +The session that wrote this ledger just spent an hour settling the design and +is about to draw diagrams. It is motivated to find boundaries, because a +ledger with entries justifies the phase. **Your default suspicion is that +BOUNDARIES contains something manufactured** — though you check both lists. + +## The test + +A decision belongs in BOUNDARIES only if it creates or changes something two +parties must agree on: + +- a new or changed module, service, or process +- a public interface or API shape +- a data contract or schema +- ownership of persisted state +- a deployment or trust boundary + +Algorithm choice, library choice, naming and file layout never qualify. When a +decision is arguably internal, it **is** internal. + +## What to judge + +1. **Manufactured boundaries** — a BOUNDARIES entry whose decisions all fail + the test above. +2. **Missed boundaries** — an EXCLUDED entry that does meet the test. Name the + party on the other side of the seam. +3. **Duplicate or conflated seams** — two entries that are the same seam under + different names, or one entry that is really two seams. +4. **Unreasoned exclusions** — an EXCLUDED entry whose stated reason does not + explain why no second party observes it. + +You do **not** judge diagram type, wording, the design itself, or whether the +decisions are good ones. Only the extraction. + +## Output + +``` +FINDING <n>: <seam name or excluded decision> — <what is wrong> — <manufactured|missed|duplicate|unreasoned> +VERDICT: no change | <N> findings +``` + +Every finding must name an entry that literally appears in the ledger. A +finding you cannot anchor to one is not a finding — drop it. + +**"The ledger looks reasonable" is `VERDICT: no change`, and that is a good, +expected result.** Never manufacture a finding to appear useful: a reviewer +that invents work is worse than one that reports nothing. + +You never edit anything and you have no write tools. The orchestrator +adjudicates. From 600ebed8621d9bc70703ef1ed71db6b3d00572ad Mon Sep 17 00:00:00 2001 From: Samuel Denani <samuel.denani@outlook.com> Date: Mon, 17 Aug 2026 16:24:20 -0300 Subject: [PATCH 05/16] feat: add set-reviewer agent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Judges a draft sub-issue set as a set — coverage, overlap, minimality and dependency edges. The only reviewer that sees every draft. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --- .claude/agents/set-reviewer.md | 49 ++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 .claude/agents/set-reviewer.md diff --git a/.claude/agents/set-reviewer.md b/.claude/agents/set-reviewer.md new file mode 100644 index 0000000..e6a8832 --- /dev/null +++ b/.claude/agents/set-reviewer.md @@ -0,0 +1,49 @@ +--- +name: set-reviewer +description: Reviews a whole set of draft sub-issues for grill-rfc — coverage gaps, overlaps, minimality and dependency edges. The only reviewer that sees the entire set. Read-only; reports findings, never fixes. Dispatched by grill-rfc. +tools: Read, Grep, Glob +model: opus +--- + +You review a **set** of draft sub-issues as a set. You are the only reviewer +positioned to say "nothing here builds the migration" — the per-draft +reviewers each see one file and cannot detect a gap by construction. + +You receive the refined RFC body, the boundary ledger (which may be empty), +and the path of every draft file (`draft-<slug>.md`). Read all of them before +judging anything. + +## What to judge + +1. **Coverage** — every line of the RFC's Scope section maps to at least one + draft. Quote the uncovered line. +2. **Overlap** — no two drafts claim the same change. Name both drafts and the + change they share. +3. **Minimality** — two drafts that would always ship together across the same + seam are one draft. This matters as much as splitting does: every extra + sub-issue costs a branch, a PR and a babysit loop, so an over-split set is + a real defect, not a safe one. +4. **Edges** — each proposed "blocked by" edge reflects a real code or data + dependency, not narrative order. "B reads the table A creates" is an edge; + "B feels like it comes second" is not. Flag invented edges and missing ones + with equal weight. + +You do **not** judge an individual draft's size, its acceptance criteria or +its wording. That is the sub-issue-reviewer's job, and duplicating it wastes +the one perspective only you have. + +## Output + +``` +FINDING <n>: <draft path(s) or quoted RFC scope line> — <what is wrong> — <coverage|overlap|minimality|edge> +VERDICT: no change | <N> findings +``` + +Every finding names a draft path or quotes a line of the RFC. A finding you +cannot anchor to one is not a finding — drop it. + +`VERDICT: no change` is a valid and expected result. Never manufacture a +finding to appear useful. + +You never edit anything and you have no write tools. The orchestrator +adjudicates and rewrites the drafts. From 94280dafc0688364574cedf47f21e41a4193d6f0 Mon Sep 17 00:00:00 2001 From: Samuel Denani <samuel.denani@outlook.com> Date: Mon, 17 Aug 2026 16:24:47 -0300 Subject: [PATCH 06/16] feat: add sub-issue-reviewer agent Judges one draft sub-issue against the size rule and RFC intent, blind to its siblings. Runs N-in-parallel, so it is on sonnet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --- .claude/agents/sub-issue-reviewer.md | 56 ++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 .claude/agents/sub-issue-reviewer.md diff --git a/.claude/agents/sub-issue-reviewer.md b/.claude/agents/sub-issue-reviewer.md new file mode 100644 index 0000000..12d3cb5 --- /dev/null +++ b/.claude/agents/sub-issue-reviewer.md @@ -0,0 +1,56 @@ +--- +name: sub-issue-reviewer +description: Reviews ONE draft sub-issue for grill-rfc against the size rule and the RFC's intent. Never sees the other drafts. Read-only; reports findings, never fixes. Dispatched by grill-rfc, N in parallel. +tools: Read, Grep, Glob +model: sonnet +--- + +You review exactly **one** draft sub-issue. + +You receive the path to one draft file (`draft-<slug>.md`), the refined RFC +body, and the boundary ledger (which may be empty). + +You do not see the other drafts, and that is deliberate. Coverage, overlap and +dependency edges belong to the set-reviewer. Never speculate about drafts you +were not given. + +## The size rule + +The draft is correctly sized only if all three hold: + +1. **One seam.** It crosses at most one boundary from the ledger. + *If the ledger is empty:* it changes exactly one observable behavior of the + system, and every acceptance criterion describes that one behavior. +2. **Co-true criteria.** Between three and seven acceptance checkboxes, all + true together or all false together. A draft whose criteria could + plausibly be half-satisfied is two drafts. +3. **Vertical slice.** Test plus implementation plus wiring, shippable on its + own. A horizontal layer ("define all the types", "add the interfaces") is a + finding even when it is small. + +## Also judge + +4. **Criteria testability** — each checkbox states an observable outcome + someone could verify, not an activity. "Refactor the parser" is not a + criterion; "the parser accepts trailing commas" is. +5. **Faithfulness** — the draft's goal is something the RFC actually asked + for. Flag invented scope and goals that contradict the RFC alike. + +## Output + +``` +FINDING <n>: <draft path> or <draft path>:criterion <n> — <what is wrong> — <one-seam|co-true|vertical|testability|faithfulness> +VERDICT: no change | <N> findings +``` + +When the finding is "too big", name **where to cut** — the seam or the +behavior boundary the split should follow. The orchestrator makes the call; +you supply the line. + +Every finding names the draft path or a numbered criterion within it. A +finding you cannot anchor to one is not a finding — drop it. + +`VERDICT: no change` is valid and expected. Never manufacture a finding to +appear useful. + +You never edit anything and you have no write tools. From 69ee1086917b2ed9c1a0220264c5e9dc519b7fc1 Mon Sep 17 00:00:00 2001 From: Samuel Denani <samuel.denani@outlook.com> Date: Mon, 17 Aug 2026 16:26:55 -0300 Subject: [PATCH 07/16] docs: widen the architecture-leak check in plan task 4 Pre-flight ruling A: the -A3 window was narrower than the "Refined RFC structure" paragraph, so a leak past line 3 would pass unnoticed. The spec's binding rule is that architecture never enters the RFC body; the check has to actually enforce it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --- .../2026-08-17-grill-rfc-architect-and-sub-issue-review.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/superpowers/plans/2026-08-17-grill-rfc-architect-and-sub-issue-review.md b/docs/superpowers/plans/2026-08-17-grill-rfc-architect-and-sub-issue-review.md index 7199770..79c1b94 100644 --- a/docs/superpowers/plans/2026-08-17-grill-rfc-architect-and-sub-issue-review.md +++ b/docs/superpowers/plans/2026-08-17-grill-rfc-architect-and-sub-issue-review.md @@ -585,7 +585,7 @@ cd /Users/samuel/Code/loopwright grep -n '^## ' .claude/skills/grill-rfc/SKILL.md test -f .claude/agents/boundary-reviewer.md && echo "PASS agent exists" || echo "FAIL agent missing" grep -q 'Architecture' .claude/skills/grill-rfc/SKILL.md && grep -q 'Refined RFC structure' .claude/skills/grill-rfc/SKILL.md \ - && grep -A3 'Refined RFC structure' .claude/skills/grill-rfc/SKILL.md | grep -q 'Architecture' \ + && grep -A8 'Refined RFC structure' .claude/skills/grill-rfc/SKILL.md | grep -q 'Architecture' \ && echo "FAIL - architecture leaked into the RFC body structure" || echo "PASS - body structure unchanged" ``` From 3842adb91b7fa38bb5823ce54b683ab86bee052e Mon Sep 17 00:00:00 2001 From: Samuel Denani <samuel.denani@outlook.com> Date: Mon, 17 Aug 2026 16:30:54 -0300 Subject: [PATCH 08/16] feat: add architect phase to grill-rfc Extracts a reasoned boundary ledger, has boundary-reviewer check the extraction before anything is drawn, then posts mermaid diagrams as one issue comment for visual approval. Never enters the RFC body, so a stale diagram cannot become planner input. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --- .claude/skills/grill-rfc/SKILL.md | 107 ++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/.claude/skills/grill-rfc/SKILL.md b/.claude/skills/grill-rfc/SKILL.md index 445d93d..7190ab3 100644 --- a/.claude/skills/grill-rfc/SKILL.md +++ b/.claude/skills/grill-rfc/SKILL.md @@ -86,6 +86,113 @@ The session is done when both directions hold: your frontier is empty AND the user's answers match the design. Do not move to step 3 until the user confirms you have reached a shared understanding. +## 2c. Architect the boundaries + +Only once both directions hold. Mark `Architect` in_progress. + +### Extract the ledger + +Walk the settled design tree. A decision earns a place only if it creates or +changes something **two parties must agree on**: + +- a new or changed module, service, or process +- a public interface or API shape +- a data contract or schema +- ownership of persisted state +- a deployment or trust boundary + +Algorithm choice, library choice, naming and file layout never qualify. When a +decision is arguably internal, it **is** internal — this phase is biased +toward producing output, and the rule exists to counteract that. + +Group qualifying decisions **by seam**, not by decision: several decisions +touching `app <-> Redis` collapse into one boundary. That grouping is what +caps the diagram count. + +``` +BOUNDARIES + app <-> Redis — decisions: workers read from Redis; Redis holds job state + client <-> API — decisions: expose GET /jobs/:id + +EXCLUDED (internal) + use a worker pool — no party outside the module observes it + retry with exponential backoff — implementation of an existing contract +``` + +The EXCLUDED list is **mandatory and reasoned**. It turns a silent judgment +into an explicit claim the user can scan in seconds. + +**Zero boundaries is a legitimate outcome.** Report it plainly, complete +`Architect` with the reason in its label ("no boundaries — 7 decisions +internal"), and go to §3. + +### Review the extraction + +Dispatch the **boundary-reviewer** agent (its own native task) with the +settled design tree and the ledger. Adjudicate every finding yourself — apply, +reject with a reason, or park. It reviews; you rewrite. + +Running it before the drawing means a boundary that should not exist is never +drawn and never shown. + +### Draw + +One diagram per boundary, type chosen to show what was decided: + +| Type | Use when | +|---|---| +| `flowchart` | structural seam — who talks to what, what crosses | +| `sequenceDiagram` | the decision was about ordering or handshake across the seam | +| `erDiagram` | the boundary is a data contract | + +Each diagram carries two to four lines: what crosses the seam, who owns what, +and which settled decisions it encodes. + +**Stay inside a conservative mermaid subset.** No validator exists here, and +invalid syntax renders as a broken code block on GitHub at exactly the moment +the user is meant to be approving: + +- alphanumeric node ids only +- no `%%{init}%%` directives +- no `classDef`, `style` or `click` +- no nested subgraphs +- quote any label containing punctuation + +### Post and gate + +All diagrams go in **one** comment on the RFC, so revisions edit it in place +instead of stacking: + +```bash +gh issue comment <N> --body-file architecture.md # first post +gh issue comment <N> --edit-last --body-file architecture.md # revisions +``` + +`--edit-last` targets your most recent comment. If the user has commented +since, capture the id from the URL the first post printed +(`...#issuecomment-<id>`) and patch it directly instead: + +```bash +gh api --method PATCH /repos/<owner>/<repo>/issues/comments/<id> -F body=@architecture.md +``` + +Give the user the ledger and the comment URL, and ask for approval. + +Two gates: + +- **Theirs.** Approve, or say what is wrong. A correction that reveals a + decision which was never actually settled reopens that branch on the §2 + frontier — go back and grill it. +- **Yours.** If a seam cannot be drawn without inventing a fact nobody + decided, that is a frontier question. Stop and ask; never guess. + +The architecture stays in the comment and **never enters the RFC body**. The +planner in `/execute-issue` reads the body via `gh issue view`, which does not +return comments — so a diagram that drifts cannot become the input a later +planner plans from. The architecture is scaffolding: its consumers are the +sub-issue phase, which needs the seams, and the user's eyes. Its job ends when +the sub-issues are created. + ## 3. Rewrite the issue First preserve the original deliberation (audit trail), then replace the From 63181715fb8792307fa2f7ab44dc85b5b89f8d42 Mon Sep 17 00:00:00 2001 From: Samuel Denani <samuel.denani@outlook.com> Date: Mon, 17 Aug 2026 16:38:03 -0300 Subject: [PATCH 09/16] fix: review the boundary ledger even when it is empty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The zero-boundary path in §2c previously skipped straight to §3, bypassing boundary-reviewer entirely. But the reviewer's charter explicitly covers a missed boundary hiding in the EXCLUDED list, and declaring everything internal is the cheapest way to produce a wrongly-empty ledger — exactly the case most needing that check. Now an all-empty BOUNDARIES list is still reviewed before the phase reports it and moves on. Applied to the shipped skill and to the spec/plan that argue for it, so all three agree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --- .claude/skills/grill-rfc/SKILL.md | 12 +++++++++--- ...08-17-grill-rfc-architect-and-sub-issue-review.md | 12 +++++++++--- ...rill-rfc-architect-and-sub-issue-review-design.md | 8 +++++--- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/.claude/skills/grill-rfc/SKILL.md b/.claude/skills/grill-rfc/SKILL.md index 7190ab3..5e1e3f5 100644 --- a/.claude/skills/grill-rfc/SKILL.md +++ b/.claude/skills/grill-rfc/SKILL.md @@ -122,9 +122,11 @@ EXCLUDED (internal) The EXCLUDED list is **mandatory and reasoned**. It turns a silent judgment into an explicit claim the user can scan in seconds. -**Zero boundaries is a legitimate outcome.** Report it plainly, complete -`Architect` with the reason in its label ("no boundaries — 7 decisions -internal"), and go to §3. +**Zero boundaries is a legitimate outcome** — but an all-empty BOUNDARIES +list is still an extraction, and declaring everything internal is the +cheapest way to skip this phase. Review it anyway (next step). Only once the +reviewer confirms: report it plainly, complete `Architect` with the reason in +its label ("no boundaries — 7 decisions internal"), and go to §3. ### Review the extraction @@ -132,6 +134,10 @@ Dispatch the **boundary-reviewer** agent (its own native task) with the settled design tree and the ledger. Adjudicate every finding yourself — apply, reject with a reason, or park. It reviews; you rewrite. +Dispatch it even when BOUNDARIES is empty. A wrongly-empty ledger is exactly +what this reviewer is best placed to catch — a missed seam surfaces as an +EXCLUDED entry that meets the test. + Running it before the drawing means a boundary that should not exist is never drawn and never shown. diff --git a/docs/superpowers/plans/2026-08-17-grill-rfc-architect-and-sub-issue-review.md b/docs/superpowers/plans/2026-08-17-grill-rfc-architect-and-sub-issue-review.md index 79c1b94..06e9329 100644 --- a/docs/superpowers/plans/2026-08-17-grill-rfc-architect-and-sub-issue-review.md +++ b/docs/superpowers/plans/2026-08-17-grill-rfc-architect-and-sub-issue-review.md @@ -494,9 +494,11 @@ EXCLUDED (internal) The EXCLUDED list is **mandatory and reasoned**. It turns a silent judgment into an explicit claim the user can scan in seconds. -**Zero boundaries is a legitimate outcome.** Report it plainly, complete -`Architect` with the reason in its label ("no boundaries — 7 decisions -internal"), and go to §3. +**Zero boundaries is a legitimate outcome** — but an all-empty BOUNDARIES +list is still an extraction, and declaring everything internal is the +cheapest way to skip this phase. Review it anyway (next step). Only once the +reviewer confirms: report it plainly, complete `Architect` with the reason in +its label ("no boundaries — 7 decisions internal"), and go to §3. ### Review the extraction @@ -504,6 +506,10 @@ Dispatch the **boundary-reviewer** agent (its own native task) with the settled design tree and the ledger. Adjudicate every finding yourself — apply, reject with a reason, or park. It reviews; you rewrite. +Dispatch it even when BOUNDARIES is empty. A wrongly-empty ledger is exactly +what this reviewer is best placed to catch — a missed seam surfaces as an +EXCLUDED entry that meets the test. + Running it before the drawing means a boundary that should not exist is never drawn and never shown. diff --git a/docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-sub-issue-review-design.md b/docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-sub-issue-review-design.md index ba91882..1658990 100644 --- a/docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-sub-issue-review-design.md +++ b/docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-sub-issue-review-design.md @@ -139,9 +139,11 @@ The exclusion list is **mandatory and reasoned**. It converts a silent judgment into an explicit claim the user reads in five seconds during the approval they are already making. -**Zero boundaries is a legitimate outcome.** Report it plainly and skip to -§3 — many RFCs (tuning gate thresholds, restructuring docs) are correctly -boundary-free. +**Zero boundaries is a legitimate outcome** — many RFCs (tuning gate +thresholds, restructuring docs) are correctly boundary-free. It is still +reviewed: an all-empty BOUNDARIES list is an extraction like any other, and +declaring everything internal is the cheapest way to skip the phase. Only +after `boundary-reviewer` confirms does the phase report it and skip to §3. ### Step 2 — Review the extraction From fe2082f747e26648261f3d69afddbe28184c3f1b Mon Sep 17 00:00:00 2001 From: Samuel Denani <samuel.denani@outlook.com> Date: Mon, 17 Aug 2026 16:42:34 -0300 Subject: [PATCH 10/16] feat: review sub-issue drafts before creating them in grill-rfc Drafts land as scratch files, get reviewed set-first then per-draft in parallel, and only reach GitHub after approval. Size rule is a drafting constraint first so the review rounds stay the exception. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --- .claude/skills/grill-rfc/SKILL.md | 77 ++++++++++++++++++++++++++++--- 1 file changed, 70 insertions(+), 7 deletions(-) diff --git a/.claude/skills/grill-rfc/SKILL.md b/.claude/skills/grill-rfc/SKILL.md index 5e1e3f5..e09364e 100644 --- a/.claude/skills/grill-rfc/SKILL.md +++ b/.claude/skills/grill-rfc/SKILL.md @@ -215,19 +215,82 @@ tree, with the reasoning that settled contested branches) · **Scope** / to be created, with dependency edges) · **Open questions** (the explicitly deferred branches). -## 4. Break into task sub-issues +## 4. Draft the sub-issues -Propose the breakdown to the user first — titles, one-line goals, and the -dependency edges — and get approval before creating anything. Each task must -be one PR's worth of work with testable acceptance criteria. +Mark `Draft sub-issues` in_progress. Drafts are **files**, not issues — +`draft-<slug>.md` in a scratch directory. Nothing reaches GitHub until the +user approves: splitting or merging real issues leaves orphaned +relationships, while splitting a file is free. -For each approved task: +Draft them **granular from the start**. The review rounds below are a safety +net, not the mechanism — the cheapest way to minimise loops is to not draft a +fat sub-issue in the first place. Where the ledger has seams, they are the +split lines. + +A draft is correctly sized when all three hold: + +1. **One seam** — it crosses at most one boundary from the ledger. If the + ledger is empty: it changes exactly one observable behavior, and every + acceptance criterion describes that one behavior. +2. **Co-true criteria** — three to seven acceptance checkboxes, all true + together or all false together. +3. **Vertical slice** — test plus implementation plus wiring, shippable + alone. Never a horizontal layer such as "define all the types". + +Draft body: goal, acceptance criteria (checkboxes), pointers into the RFC, +and which drafts it is blocked by. + +Then **log the whole set to the user** — titles, one-line goals and the +dependency edges. This is visibility, not an approval gate: do not wait. + +## 4b. Review the sub-issues + +Mark `Review sub-issues` in_progress. Reviewers review only. You adjudicate +every finding — apply, reject with a reason, or park — and you do all the +rewriting. + +**First the set.** Dispatch the **set-reviewer** agent (its own native task) +with the refined RFC, the ledger and every draft path. It judges coverage, +overlap, minimality and edges. Adjudicate, then rewrite the drafts. + +Set findings change *which drafts exist*, which is why this runs first: in +parallel, every per-draft review of a doomed draft is wasted and the draft the +set-reviewer invents gets no review at all. + +**Then the drafts.** Dispatch one **sub-issue-reviewer** per draft, in +parallel, one native task each (`Review: <title>`). Each gets exactly one +draft path plus the RFC and the ledger — never the other drafts. Adjudicate, +then rewrite. + +**Converge.** If the draft round produced any split or merge, the set changed: +run the set-reviewer once more on the revised set, and give each newly-born +draft its one sub-issue review. Cap at **one** re-entry; then adjudicate the +residuals yourself and carry them into the approval message. + +After each round give the user one line — never the raw verdicts: + +``` +set-reviewer: 2 gaps, 1 overlap → added draft-migrate-jobs, merged draft-b + draft-c +sub-issue-reviewers (6): 1 split → draft-api split into draft-api-read, draft-api-write +boundary-reviewer: no change +``` + +## 4c. Approve, then create + +Present the final set for approval — titles, one-line goals, dependency edges, +and any residual findings you parked. + +If the user rejects it, treat their objection as a set-level finding: apply +it, re-run the set-reviewer once on the revised set, sub-issue-review only +newly-born drafts, and present again. A user objection is never parked and is +not subject to the re-entry cap. + +On approval, mark `Create sub-issues` in_progress and create each one: ```bash -gh issue create --title "Task: <title>" --label task --body-file task-N.md +gh issue create --title "Task: <title>" --label task --body-file draft-<slug>.md ``` -Task body: goal, acceptance criteria (checkboxes), pointers into the RFC. Then link relationships via GraphQL (write queries to a temp file and use `-F query=@file` — inline quoting breaks in fish): From 915ecff987aa066270d6d21abf091810e236df56 Mon Sep 17 00:00:00 2001 From: Samuel Denani <samuel.denani@outlook.com> Date: Mon, 17 Aug 2026 16:50:22 -0300 Subject: [PATCH 11/16] feat: add a native task spine to grill-rfc Seven phase tasks created at load, plus one task per agent dispatch. Grill rounds stay in the task label rather than spawning tasks, and a phase completes only once its output is adjudicated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --- .claude/skills/grill-rfc/SKILL.md | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/.claude/skills/grill-rfc/SKILL.md b/.claude/skills/grill-rfc/SKILL.md index e09364e..1c47fe7 100644 --- a/.claude/skills/grill-rfc/SKILL.md +++ b/.claude/skills/grill-rfc/SKILL.md @@ -19,6 +19,25 @@ gh issue view <N> --json number,title,body,labels,url Abort unless the issue has the `rfc` label. The deliberation in the body seeds the design tree below — its central decision is the root. +Then create the run's **native task spine** (TaskCreate), in this order: +`Grill` · `Reverse grill` · `Architect` · `Rewrite RFC body` · +`Draft sub-issues` · `Review sub-issues` · `Create sub-issues`. No dependency +edges — the spine is sequential and reads that way by id. It exists so the +user can see the shape of the run before the first question. + +Keep it live for the rest of the session. A phase goes in_progress when it +starts and completed when its output is **adjudicated**, not when an agent +replies — otherwise the list goes green while the work is still open. A phase +that legitimately did nothing still completes, carrying the reason in its +label ("no boundaries — 7 decisions internal"); a phase vanishing mid-run +reads as a bug. + +**One task per agent dispatch, and nowhere else.** Agent fan-outs are the only +stretches where the user is in the dark — during the grill they are being +asked questions. So `Grill` and `Architect` carry their progress in the task +label ("Grilling: round 3, 4 questions open") rather than spawning a task per +round: a task per round is unbounded and would bury the spine. + ## 2. Grill Interview the user relentlessly until you reach a shared understanding. Map @@ -307,6 +326,8 @@ gh api graphql -F query='mutation { addBlockedBy(input:{issueId:"<blocked-id>", ## 5. Report -Final message: link to the refined RFC, the list of created sub-issues with -their dependency edges, and which task is unblocked and ready for +Complete the last spine task, then give the user: the link to the refined RFC, +the link to the architecture comment (or "no boundaries" if the phase found +none), the created sub-issues with their dependency edges, every residual +finding you parked, and which task is unblocked and ready for `/execute-issue`. From 3e3c5335bbef92e5858ed6b427d0b0553c7990e7 Mon Sep 17 00:00:00 2001 From: Samuel Denani <samuel.denani@outlook.com> Date: Mon, 17 Aug 2026 16:58:15 -0300 Subject: [PATCH 12/16] fix: close the Architect phase explicitly at the approval gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mainline (non-zero-boundary) path through §2c left completion to inference: §1's general "output is adjudicated" rule and the zero-boundary shortcut's explicit "complete `Architect`" (right after the boundary-reviewer's findings are adjudicated) together trained a fresh agent to close the phase mid-way, before Draw and the two gates ran. Add an explicit "complete `Architect`" at the end of "Post and gate" in the skill, spec and plan so the mainline path is no longer left to inference and names the wrong point it must not close at. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --- .claude/skills/grill-rfc/SKILL.md | 5 +++++ .../2026-08-17-grill-rfc-architect-and-sub-issue-review.md | 5 +++++ ...-08-17-grill-rfc-architect-and-sub-issue-review-design.md | 4 ++++ 3 files changed, 14 insertions(+) diff --git a/.claude/skills/grill-rfc/SKILL.md b/.claude/skills/grill-rfc/SKILL.md index 1c47fe7..75fdaa6 100644 --- a/.claude/skills/grill-rfc/SKILL.md +++ b/.claude/skills/grill-rfc/SKILL.md @@ -211,6 +211,11 @@ Two gates: - **Yours.** If a seam cannot be drawn without inventing a fact nobody decided, that is a frontier question. Stop and ask; never guess. +On approval, complete `Architect`. The phase's output is the approved +diagrams, so it closes here — not back when the boundary-reviewer's findings +were adjudicated, which is mid-phase with the drawing and both gates still +ahead. + The architecture stays in the comment and **never enters the RFC body**. The planner in `/execute-issue` reads the body via `gh issue view`, which does not return comments — so a diagram that drifts cannot become the input a later diff --git a/docs/superpowers/plans/2026-08-17-grill-rfc-architect-and-sub-issue-review.md b/docs/superpowers/plans/2026-08-17-grill-rfc-architect-and-sub-issue-review.md index 06e9329..51f4db6 100644 --- a/docs/superpowers/plans/2026-08-17-grill-rfc-architect-and-sub-issue-review.md +++ b/docs/superpowers/plans/2026-08-17-grill-rfc-architect-and-sub-issue-review.md @@ -564,6 +564,11 @@ Two gates: - **Yours.** If a seam cannot be drawn without inventing a fact nobody decided, that is a frontier question. Stop and ask; never guess. +On approval, complete `Architect`. The phase's output is the approved +diagrams, so it closes here — not back when the boundary-reviewer's findings +were adjudicated, which is mid-phase with the drawing and both gates still +ahead. + The architecture stays in the comment and **never enters the RFC body**. The planner in `/execute-issue` reads the body via `gh issue view`, which does not return comments — so a diagram that drifts cannot become the input a later diff --git a/docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-sub-issue-review-design.md b/docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-sub-issue-review-design.md index 1658990..773a9c4 100644 --- a/docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-sub-issue-review-design.md +++ b/docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-sub-issue-review-design.md @@ -191,6 +191,10 @@ Two gates: - **The phase's own.** If a seam cannot be drawn without inventing a fact nobody decided, that is a frontier question. Stop and ask; do not guess. +On approval the phase completes. Its output is the approved diagrams, so +`Architect` closes at this gate — not at the earlier point where the +boundary-reviewer's findings were adjudicated. + ### Why the comment and not the body `planner.md` reads the parent RFC via `gh issue view`, which returns the body From 8c58347c75f3eef57426136093b8b13c9c2cce75 Mon Sep 17 00:00:00 2001 From: Samuel Denani <samuel.denani@outlook.com> Date: Mon, 17 Aug 2026 17:03:48 -0300 Subject: [PATCH 13/16] docs: document the architect and sub-issue review phases Flow diagram and Pieces table cover the three refinement agents, plus a loop rule recording why architecture lives in a comment and not the RFC body. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --- docs/loop-harness.md | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/docs/loop-harness.md b/docs/loop-harness.md index 6c43c73..0a4b298 100644 --- a/docs/loop-harness.md +++ b/docs/loop-harness.md @@ -9,9 +9,14 @@ half. ``` RFC issue (label: rfc) │ human writes the initial deliberation - │ /grill-rfc <N> — agent interrogates, rewrites the body as the refined - │ RFC (original preserved as a comment), creates task sub-issues with - │ dependencies (native parent/sub-issue + blocked-by relationships) + │ /grill-rfc <N> — agent interrogates, then: + │ architect phase → boundary ledger, boundary-reviewer checks the + │ extraction, mermaid per seam posted as ONE issue + │ comment for visual approval (never the body) + │ rewrites the body as the refined RFC (original kept as a comment) + │ drafts sub-issues as files → set-reviewer (whole set) → N + │ sub-issue-reviewers (one draft each, parallel) → your approval + │ creates the sub-issues with native parent + blocked-by edges ▼ Task sub-issue (label: task) │ /execute-issue <N> — accepts a task OR the RFC itself. @@ -58,12 +63,24 @@ Ready PR, gate green, reviews addressed → human merges and the PR conversation. Any session (or human) can pick up a half-done task from these alone. - **Merging is always the human's decision.** +- **Architecture is scaffolding, not a spec.** The architect phase's mermaid + lives in an RFC issue comment and never in the issue body, because + `planner.md` reads the body via `gh issue view` and comments are not + returned. Its consumers are the sub-issue slicing step and the human eye; + its job ends when the sub-issues are created. Nothing downstream reads it, + so it cannot mislead when it drifts. +- **`boundary-reviewer` is the first thing to cut.** It is the weakest of the + three refinement agents: same input and model as the session that wrote the + ledger, only the objective inverted. Its changelog line is the evidence — if + it reports `no change` across three RFCs, delete it and rely on the ledger's + mandatory reasoned exclusion list alone. ## Pieces | Piece | Where | |---|---| -| Agents (planner / coder / reviewer) | `.claude/agents/` | +| Execution agents (planner / coder / reviewer) | `.claude/agents/` | +| Refinement agents (boundary-reviewer / set-reviewer / sub-issue-reviewer) | `.claude/agents/` | | Skills (grill-rfc / execute-issue / babysit-pr) | `.claude/skills/` | | Specs | `docs/specs/issue-<N>.md` | | Quality gate | `scripts/quality-gate.mjs`, `docs/quality-gate.md` | From e99ca8224e74f34f454a048b79689212316eae61 Mon Sep 17 00:00:00 2001 From: Samuel Denani <samuel.denani@outlook.com> Date: Mon, 17 Aug 2026 17:18:21 -0300 Subject: [PATCH 14/16] fix: close the gaps the whole-branch review found MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two matter most: boundary-reviewer's prompt still suspected only BOUNDARIES even though grill-rfc §2c now dispatches it when BOUNDARIES is empty too — it now inverts its suspicion toward EXCLUDED in that case. And the RFC body's Task breakdown, written in §3 before drafting, was never reconciled after the review rounds change which sub-issues exist — §4c now rewrites it to the issues actually created after linking their edges. The other eight: draft-slug "blocked by" references get swapped for real issue numbers as issues are created in dependency order; boundary-reviewer gets its changelog line in §2c so its "cut it after three no-changes" rule has evidence to point at; re-entry round tasks are suffixed "(round 2)"; architecture.md's scratch-directory location is made explicit; §3 gets its missing "mark in_progress"; §1's "one task per dispatch" rule is amended to explicitly cover §2's fact-finding sub-agents; README's agents table row is split to match loop-harness.md; and loop-harness.md's "state lives in artifacts, not sessions" bullet now notes the grill phase is the session-scoped exception. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --- .claude/agents/boundary-reviewer.md | 4 ++ .claude/skills/grill-rfc/SKILL.md | 37 +++++++++++++++++-- README.md | 3 +- docs/loop-harness.md | 3 +- ...c-architect-and-sub-issue-review-design.md | 6 +++ 5 files changed, 48 insertions(+), 5 deletions(-) diff --git a/.claude/agents/boundary-reviewer.md b/.claude/agents/boundary-reviewer.md index 4e1d32d..810c2b2 100644 --- a/.claude/agents/boundary-reviewer.md +++ b/.claude/agents/boundary-reviewer.md @@ -20,6 +20,10 @@ is about to draw diagrams. It is motivated to find boundaries, because a ledger with entries justifies the phase. **Your default suspicion is that BOUNDARIES contains something manufactured** — though you check both lists. +When BOUNDARIES is empty, invert this. An all-internal ledger is the cheapest +way to skip the phase entirely, so scrutinise EXCLUDED first and hardest — an +empty ledger is an extraction like any other, and you are the only check on it. + ## The test A decision belongs in BOUNDARIES only if it creates or changes something two diff --git a/.claude/skills/grill-rfc/SKILL.md b/.claude/skills/grill-rfc/SKILL.md index 75fdaa6..13de8ae 100644 --- a/.claude/skills/grill-rfc/SKILL.md +++ b/.claude/skills/grill-rfc/SKILL.md @@ -32,7 +32,9 @@ that legitimately did nothing still completes, carrying the reason in its label ("no boundaries — 7 decisions internal"); a phase vanishing mid-run reads as a bug. -**One task per agent dispatch, and nowhere else.** Agent fan-outs are the only +**One task per agent dispatch, and nowhere else** — including the +fact-finding sub-agents §2 dispatches, which get a task like any other +dispatch. Agent fan-outs are the only stretches where the user is in the dark — during the grill they are being asked questions. So `Grill` and `Architect` carry their progress in the task label ("Grilling: round 3, 4 questions open") rather than spawning a task per @@ -153,6 +155,10 @@ Dispatch the **boundary-reviewer** agent (its own native task) with the settled design tree and the ledger. Adjudicate every finding yourself — apply, reject with a reason, or park. It reviews; you rewrite. +Give the user one changelog line for this round — `boundary-reviewer: no +change`, or what you changed and why — never the raw verdict. That line is +the standing evidence for whether this reviewer earns its place. + Dispatch it even when BOUNDARIES is empty. A wrongly-empty ledger is exactly what this reviewer is best placed to catch — a missed seam surfaces as an EXCLUDED entry that meets the test. @@ -186,7 +192,8 @@ the user is meant to be approving: ### Post and gate All diagrams go in **one** comment on the RFC, so revisions edit it in place -instead of stacking: +instead of stacking. Write `architecture.md` in the same scratch directory as +the drafts, and never commit it to the repo: ```bash gh issue comment <N> --body-file architecture.md # first post @@ -225,6 +232,8 @@ the sub-issues are created. ## 3. Rewrite the issue +Mark `Rewrite RFC body` in_progress. + First preserve the original deliberation (audit trail), then replace the body: @@ -291,6 +300,9 @@ run the set-reviewer once more on the revised set, and give each newly-born draft its one sub-issue review. Cap at **one** re-entry; then adjudicate the residuals yourself and carry them into the approval message. +Suffix the re-entry round's native tasks `(round 2)` so they do not collide +with the first round's by name. + After each round give the user one line — never the raw verdicts: ``` @@ -309,7 +321,14 @@ it, re-run the set-reviewer once on the revised set, sub-issue-review only newly-born drafts, and present again. A user objection is never parked and is not subject to the re-entry cap. -On approval, mark `Create sub-issues` in_progress and create each one: +On approval, mark `Create sub-issues` in_progress. + +Create in dependency order, blockers first, so every issue number exists +before something needs to reference it. As you create each one, replace its +draft-slug "blocked by" line with the real issue numbers — a published issue +must never reference a scratch filename. The native `addBlockedBy` edge below +is what actually encodes the dependency; the line in the body is only for a +human reading it. ```bash gh issue create --title "Task: <title>" --label task --body-file draft-<slug>.md @@ -329,6 +348,18 @@ gh api graphql -F query='mutation { addSubIssue(input:{issueId:"<rfc-id>", subIs gh api graphql -F query='mutation { addBlockedBy(input:{issueId:"<blocked-id>", blockingIssueId:"<blocker-id>"}) { issue { number } } }' ``` +Finally, reconcile the RFC body. §3's **Task breakdown** was written before +drafting, and the review rounds exist to change which sub-issues there are — +so by now it is very likely wrong. Rewrite that section to the issues actually +created, with their real numbers and edges, and push it: + +```bash +gh issue edit <N> --body-file refined.md +``` + +The body is the surface `/execute-issue`'s planner reads. A stale task +breakdown there is the same defect the architecture comment exists to avoid. + ## 5. Report Complete the last spine task, then give the user: the link to the refined RFC, diff --git a/README.md b/README.md index bd9f827..3e89b98 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,8 @@ lands, and fill in the `TODO(template)` markers in `CLAUDE.md`. |---|---| | Flow overview | `docs/loop-harness.md` | | Gate design | `docs/quality-gate.md` | -| Agents (planner / coder / reviewer) | `.claude/agents/` | +| Execution agents (planner / coder / reviewer) | `.claude/agents/` | +| Refinement agents (boundary-reviewer / set-reviewer / sub-issue-reviewer) | `.claude/agents/` | | Skills (grill-rfc / execute-issue / babysit-pr) | `.claude/skills/` | | Gate engine | `scripts/quality-gate.mjs`, `quality-gate.config.json` | | CI | `.github/workflows/` | diff --git a/docs/loop-harness.md b/docs/loop-harness.md index 0a4b298..85c4377 100644 --- a/docs/loop-harness.md +++ b/docs/loop-harness.md @@ -61,7 +61,8 @@ Ready PR, gate green, reviews addressed → human merges - **State lives in artifacts, not sessions**: the refined RFC and its comment trail on the issue, the committed spec in `docs/specs/`, per-step commits, and the PR conversation. Any session (or human) can pick up a half-done - task from these alone. + task from these alone — except the grill phase itself, whose task spine and + drafts are session-scoped, so a crashed grill restarts. - **Merging is always the human's decision.** - **Architecture is scaffolding, not a spec.** The architect phase's mermaid lives in an RFC issue comment and never in the issue body, because diff --git a/docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-sub-issue-review-design.md b/docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-sub-issue-review-design.md index 773a9c4..081cb9c 100644 --- a/docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-sub-issue-review-design.md +++ b/docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-sub-issue-review-design.md @@ -261,6 +261,12 @@ if membership changed (any split or merge): user approval → create issues → link parent / blockedBy ``` +After creation, the RFC body's **Task breakdown** is rewritten to the issues +actually created. It was authored in §3 before drafting, and the review rounds +are expected to change set membership — leaving it stale would put exactly the +kind of drifted artifact the comment-only architecture rule exists to prevent +into the one surface the planner reads. + **Set before sub-issues, deliberately.** Set findings change *which tasks exist*. Run the two in parallel and every sub-issue review of a doomed task is wasted while the task the set reviewer invents gets no review at all — a second round From 3b997a07f063b5337f577c83a7891f5507109319 Mon Sep 17 00:00:00 2001 From: Samuel Denani <samuel.denani@outlook.com> Date: Mon, 17 Aug 2026 17:33:24 -0300 Subject: [PATCH 15/16] fix: stop the pre-commit hook tripping on gate fixtures, ignore local .claude state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hook scanned every staged JS/TS addition for gated shortcuts with no path exclusion, so merging main into a branch failed on .loopwright/tests/fixtures/src-sample/legacy.jsx — a fixture that carries it.skip precisely so the gate's skipped-test detector has something to detect. The authoritative gate never saw it (integrity.skippedTests 0), because .loopwright/config.json sets sources.ignore to **/fixtures/**. The hook now mirrors that exclusion, which is the rule it was always meant to preview. Verified: the fixture no longer matches, and a .skip added outside fixtures/ is still caught. .gitignore regains the per-machine Claude Code entries. They predate the vendorable-layer restructure, which rewrote this file (coverage/ + reports/ became .loopwright/reports/), so they were rebased onto the new contents rather than merged. .claude/agents and .claude/skills stay tracked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --- .githooks/pre-commit | 6 +++++- .gitignore | 5 +++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 40cce90..e1bd586 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -11,7 +11,11 @@ fail() { echo "pre-commit: $1" >&2; exit 1; } # --- gated shortcuts in staged additions ------------------------------------- # Each of these blocks the PR in CI; rejecting them here saves the round-trip. -added=$(git diff --cached -U0 -- '*.ts' '*.tsx' '*.js' '*.jsx' '*.mjs' | grep '^+[^+]' || true) +# Fixtures are excluded to mirror `sources.ignore` in .loopwright/config.json: +# they exist to give the gate's detectors something to detect, so a fixture +# carrying a .skip or an @ts-ignore is the point, not a defect. Without this, +# merging main into a branch trips the hook on files nobody edited. +added=$(git diff --cached -U0 -- '*.ts' '*.tsx' '*.js' '*.jsx' '*.mjs' ':(exclude)**/fixtures/**' | grep '^+[^+]' || true) if [ -n "$added" ]; then declare -a patterns=( '\.only *\(' 'a focused test (.only)' diff --git a/.gitignore b/.gitignore index 67be44c..f0d0e02 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,8 @@ node_modules/ dist/ *.log .DS_Store + +# Claude Code: keep .claude/agents and .claude/skills tracked, ignore per-machine state +.claude/worktrees/ +.claude/settings.local.json +.claude/*.local.json From a07db829053169a9d41b0a883b357b19d10113ee Mon Sep 17 00:00:00 2001 From: Samuel Denani <samuel.denani@outlook.com> Date: Mon, 17 Aug 2026 17:48:47 -0300 Subject: [PATCH 16/16] fix: address PR review findings on paths, task rule, and hook pathspec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Update docs/loop-harness.md references to docs/loopwright/loop-harness.md in the plan and spec, which predate the vendorable-layer restructure that moved the file. - Fix the self-contradicting task-per-dispatch rule in grill-rfc/SKILL.md §1: fact-finding sub-agents fold into the Grill task's label, while the bounded review fan-outs get their own tasks. - Widen the pre-commit hook's fixtures exclusion to cover both a root-level fixtures/ and a nested **/fixtures/**, since git pathspec's leading **/ requires at least one directory component. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --- .claude/skills/grill-rfc/SKILL.md | 11 ++++++---- .githooks/pre-commit | 6 ++++-- ...rill-rfc-architect-and-sub-issue-review.md | 20 +++++++++---------- ...c-architect-and-sub-issue-review-design.md | 2 +- 4 files changed, 22 insertions(+), 17 deletions(-) diff --git a/.claude/skills/grill-rfc/SKILL.md b/.claude/skills/grill-rfc/SKILL.md index 13de8ae..6365e8f 100644 --- a/.claude/skills/grill-rfc/SKILL.md +++ b/.claude/skills/grill-rfc/SKILL.md @@ -32,10 +32,13 @@ that legitimately did nothing still completes, carrying the reason in its label ("no boundaries — 7 decisions internal"); a phase vanishing mid-run reads as a bug. -**One task per agent dispatch, and nowhere else** — including the -fact-finding sub-agents §2 dispatches, which get a task like any other -dispatch. Agent fan-outs are the only -stretches where the user is in the dark — during the grill they are being +**One task per agent dispatch, with one exception** — the fact-finding +sub-agents §2 dispatches during the grill. Those are per-round and unbounded +like the rounds themselves, so they fold into the `Grill` task's label rather +than spawning their own. The review fan-outs (`boundary-reviewer` in §2c, +`set-reviewer` and the per-draft `sub-issue-reviewer`s in §4b) each get their +own native task — those are bounded, and they're the stretches where the user +is genuinely in the dark, unlike the grill itself where the user is being asked questions. So `Grill` and `Architect` carry their progress in the task label ("Grilling: round 3, 4 questions open") rather than spawning a task per round: a task per round is unbounded and would bury the spine. diff --git a/.githooks/pre-commit b/.githooks/pre-commit index e1bd586..48d0fc7 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -14,8 +14,10 @@ fail() { echo "pre-commit: $1" >&2; exit 1; } # Fixtures are excluded to mirror `sources.ignore` in .loopwright/config.json: # they exist to give the gate's detectors something to detect, so a fixture # carrying a .skip or an @ts-ignore is the point, not a defect. Without this, -# merging main into a branch trips the hook on files nobody edited. -added=$(git diff --cached -U0 -- '*.ts' '*.tsx' '*.js' '*.jsx' '*.mjs' ':(exclude)**/fixtures/**' | grep '^+[^+]' || true) +# merging main into a branch trips the hook on files nobody edited. Both +# exclude forms are needed: a git pathspec's leading `**/` requires at least +# one directory component, so it alone would miss a root-level `fixtures/`. +added=$(git diff --cached -U0 -- '*.ts' '*.tsx' '*.js' '*.jsx' '*.mjs' ':(exclude)fixtures/**' ':(exclude)**/fixtures/**' | grep '^+[^+]' || true) if [ -n "$added" ]; then declare -a patterns=( '\.only *\(' 'a focused test (.only)' diff --git a/docs/superpowers/plans/2026-08-17-grill-rfc-architect-and-sub-issue-review.md b/docs/superpowers/plans/2026-08-17-grill-rfc-architect-and-sub-issue-review.md index 51f4db6..74c4a1e 100644 --- a/docs/superpowers/plans/2026-08-17-grill-rfc-architect-and-sub-issue-review.md +++ b/docs/superpowers/plans/2026-08-17-grill-rfc-architect-and-sub-issue-review.md @@ -36,7 +36,7 @@ | `.claude/agents/set-reviewer.md` (create) | Judges a draft set as a set — coverage, overlap, minimality, edges. The only agent that sees every draft. | | `.claude/agents/sub-issue-reviewer.md` (create) | Judges ONE draft against the size rule and RFC intent. Never sees siblings. | | `.claude/skills/grill-rfc/SKILL.md` (modify) | §1 gains the task spine; new §2c architect phase; §4 replaced by draft → review → approve → create; §5 report updated. | -| `docs/loop-harness.md` (modify) | Flow diagram and Pieces table reflect the new phases and agents. | +| `docs/loopwright/loop-harness.md` (modify) | Flow diagram and Pieces table reflect the new phases and agents. | --- @@ -878,10 +878,10 @@ EOF --- -### Task 7: Update `docs/loop-harness.md` +### Task 7: Update `docs/loopwright/loop-harness.md` **Files:** -- Modify: `docs/loop-harness.md` — the `RFC issue (label: rfc)` block of the Flow diagram, and the Agents row of the Pieces table. +- Modify: `docs/loopwright/loop-harness.md` — the `RFC issue (label: rfc)` block of the Flow diagram, and the Agents row of the Pieces table. **Interfaces:** - Consumes: everything. Documentation task, runs last. @@ -891,9 +891,9 @@ EOF ```bash cd /Users/samuel/Code/loopwright -grep -q 'boundary-reviewer' docs/loop-harness.md \ - && grep -q 'set-reviewer' docs/loop-harness.md \ - && grep -q 'sub-issue-reviewer' docs/loop-harness.md \ +grep -q 'boundary-reviewer' docs/loopwright/loop-harness.md \ + && grep -q 'set-reviewer' docs/loopwright/loop-harness.md \ + && grep -q 'sub-issue-reviewer' docs/loopwright/loop-harness.md \ && echo PASS || echo FAIL ``` @@ -954,9 +954,9 @@ Append as a new bullet at the end of the `## Rules of the loop` list: ```bash cd /Users/samuel/Code/loopwright -grep -q 'boundary-reviewer' docs/loop-harness.md \ - && grep -q 'set-reviewer' docs/loop-harness.md \ - && grep -q 'sub-issue-reviewer' docs/loop-harness.md \ +grep -q 'boundary-reviewer' docs/loopwright/loop-harness.md \ + && grep -q 'set-reviewer' docs/loopwright/loop-harness.md \ + && grep -q 'sub-issue-reviewer' docs/loopwright/loop-harness.md \ && echo PASS || echo FAIL # every agent named anywhere in .claude/ resolves to a file @@ -974,7 +974,7 @@ Expected: `PASS`, six `PASS <agent>` lines, and a green quality gate. ```bash cd /Users/samuel/Code/loopwright -git add docs/loop-harness.md +git add docs/loopwright/loop-harness.md git commit -F - <<'EOF' docs: document the architect and sub-issue review phases diff --git a/docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-sub-issue-review-design.md b/docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-sub-issue-review-design.md index 081cb9c..46927f4 100644 --- a/docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-sub-issue-review-design.md +++ b/docs/superpowers/specs/2026-08-17-grill-rfc-architect-and-sub-issue-review-design.md @@ -367,7 +367,7 @@ three are read-only, report-never-fix, matching the existing `reviewer.md` contract shape. **Modified** — `.claude/skills/grill-rfc/SKILL.md` (new §2c, rewritten §4, -task spine in §1). `docs/loop-harness.md` (flow diagram and Pieces table). +task spine in §1). `docs/loopwright/loop-harness.md` (flow diagram and Pieces table). **Unchanged** — `execute-issue`, `babysit-pr`, `planner.md`, `coder.md`, `reviewer.md`, the quality gate, the issue templates.