From 9a9183a680d23a57c0342428f7c1d03f3d99215b Mon Sep 17 00:00:00 2001 From: tom Date: Tue, 11 Aug 2026 20:11:05 +0200 Subject: [PATCH 1/2] Restructure the product task workflow around vertical subtasks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tasks that fit in a single subtask no longer get a spec: they are grilled, implemented, and opened as a PR in one session, with the reasoning in the PR description. The size vocabulary goes with them — a deferred subtask is marked by having only a brief.md, which is what "large" used to record. A subtask is now a vertical slice, bounded to one fresh context window and one commit; its leaves stay layer-shaped, one project skill each (ADR 0002). Review and commit move from the leaf to the subtask, so an unattended chain no longer spends a subagent per axis on every step. Acceptance criteria replace the [verify] tag — a (human) criterion is what pauses the chain — and Blocked by edges replace implicit ordering, which keeps subtask numbers as identity and lets a deferred subtask expand into appended siblings. Nesting and the sub-branch/sub-PR scheme are gone. to-spec is now a pure writer: Slack harvesting is dropped and outreach moves to grill-the-task, where the questions are born and where a spec-less task needs it too. The spec axis of review-changes is gated on a spec existing, and create-pr carries the why for the PRs that have no spec behind them. Co-Authored-By: Claude Opus 5 --- .agents/AGENTS.md | 6 +- .../adr/0002-layer-shaped-subtask-leaves.md | 57 +++++ .agents/skills/create-pr/SKILL.md | 24 ++- .agents/skills/grill-the-task/SKILL.md | 161 +++++++++------ .agents/skills/implement-task/SKILL.md | 194 +++++++++--------- .agents/skills/resolve-review/SKILL.md | 2 +- .agents/skills/review-changes/SKILL.md | 44 ++-- .../skills/review-changes/review-template.md | 8 +- .agents/skills/to-spec/SKILL.md | 168 ++++++--------- .agents/skills/to-spec/spec-template.md | 63 ++---- .agents/skills/to-spec/subtask-template.md | 69 +++++++ .agents/tasks/README.md | 175 +++++++++------- 12 files changed, 560 insertions(+), 411 deletions(-) create mode 100644 .agents/adr/0002-layer-shaped-subtask-leaves.md create mode 100644 .agents/skills/to-spec/subtask-template.md diff --git a/.agents/AGENTS.md b/.agents/AGENTS.md index d7eb15b8039..02f9a2af792 100644 --- a/.agents/AGENTS.md +++ b/.agents/AGENTS.md @@ -68,6 +68,7 @@ Decisions with repo-wide consequences are recorded in `.agents/adr/`, named evidence and the trade-off, so it answers "why is it like this?" without a git archaeology session. - `0001-webpack-for-production-builds.md` — why production bundles are built with webpack while dev stays on Turbopack. +- `0002-layer-shaped-subtask-leaves.md` — why a product task's subtasks cut vertically while the leaves inside them run along layers. Add a new record (next free number, and a line here) whenever a decision is expensive to rediscover: it constrains future work, was reached by measurement or an investigation worth not repeating, or @@ -77,8 +78,9 @@ looks wrong without its context. Supersede rather than rewrite — flip the old ## Product task workflow Product tasks (GitHub issues) are worked through a spec-driven workflow — interview, spec, agent -implementation, code review. Specs accumulate in `.agents/tasks/` as a permanent record. See -`./tasks/README.md` for the lifecycle, the skills that run it, and the spec conventions. +implementation, code review. A task small enough to finish in its grilling session skips the spec +entirely. Specs accumulate in `.agents/tasks/` as a permanent record. See `./tasks/README.md` for the +lifecycle, the skills that run it, and the spec conventions. ## Editing this instruction set diff --git a/.agents/adr/0002-layer-shaped-subtask-leaves.md b/.agents/adr/0002-layer-shaped-subtask-leaves.md new file mode 100644 index 00000000000..7c3856132a4 --- /dev/null +++ b/.agents/adr/0002-layer-shaped-subtask-leaves.md @@ -0,0 +1,57 @@ +# 0002 — subtasks cut vertically, leaves run along layers + +| | | +| --- | --- | +| Status | accepted | +| Date | 2026-08-11 | +| Deciders | @tom2drum | +| Supersedes | — | + +## Decision + +**A subtask is a vertical slice; the leaves inside it are layer-shaped.** + +A subtask cuts a narrow but complete path through every layer it touches and is verifiable on its own. Its +leaves do the opposite — each is one project skill's worth of work in one layer: `add-api-resource`, then +`add-new-page`, then the `[human]` styling. + +``` +subtasks/01-cross-chain-list/ ← vertical: demoable, one context window, one commit + leaf 1 [agent] add-api-resource — declare the resource + leaf 2 [agent] add-new-page — tab route + scaffold + leaf 3 [agent] wire the resource into the table + leaf 4 [human] style to mockup +``` + +The full model lives in "The subtask model" in `.agents/tasks/README.md`; this record holds only the +reasoning, which that file should not have to carry. + +## Why + +The tracer-bullet norm says every unit of work should be a vertical slice, all the way down. Ours stops one +level short, deliberately. + +**Layer-shaped leaves are what make execution mechanical.** The project skills are layer-shaped by +construction — `add-api-resource` declares a resource, `add-new-page` scaffolds a route. A leaf that mapped +one skill to one step lets `implement-task` execute it without deciding anything: open the skill, read the +`inputs:` the grilling session already collected, run. Force a leaf to be vertical and it spans three +skills, so the executor has to compose them itself — the interesting decisions move from the spec, where a +human reviewed them, into an unattended run. + +**Vertical subtasks are what make review and verification meaningful.** An API resource reviewed alone +cannot be judged against the thing that consumes it, and a scaffold with no data cannot be verified by +looking at the running product. Grouping the leaves into a slice that renders gives both a real target: the +review reads one coherent diff, and a `(human)` acceptance criterion has something to be true about. + +So the two levels answer two different questions. *What can an agent execute without judgement?* — a leaf. +*What can a human judge?* — a subtask. Aligning both to the same axis would sacrifice one of them. + +## Consequences + +- Leaves are not run boundaries. A run executes a whole subtask; leaf checkboxes exist so a run interrupted + by a `[human]` leaf can resume, since nothing is committed until the subtask finishes. +- The review unit is the subtask, so a leaf's code can be wrong for as long as it takes the slice to + finish. Accepted deliberately: reviewing every leaf spent three subagents per step, and most of what it + caught was churn the next leaf rewrote anyway. +- Nesting is unnecessary. Work too big for one subtask becomes more subtasks with blocking edges between + them, never subtasks inside subtasks — which is what let the sub-branch and sub-PR machinery go. diff --git a/.agents/skills/create-pr/SKILL.md b/.agents/skills/create-pr/SKILL.md index 904c0ed6938..370fe15a7bb 100644 --- a/.agents/skills/create-pr/SKILL.md +++ b/.agents/skills/create-pr/SKILL.md @@ -28,7 +28,7 @@ Check the current branch (`git branch --show-current`) and its open PR (`gh pr l If the signals conflict or are ambiguous, ask the user which mode they mean. -**Unattended finalize.** When Mode B is reached from `implement-task --auto` clearing a task's last leaf, the +**Unattended finalize.** When Mode B is reached from `implement-task --auto` clearing a task's last subtask, the `--auto` flag already granted the push and the flip — so proceed without the confirmation steps below, and report what was done instead of asking. Every other invocation keeps them. @@ -54,13 +54,11 @@ description is a placeholder pointing at the plan: - `Resolves #` when the branch matches `issue-\d+` (ad-hoc spec branches have no issue — omit). - One short paragraph: the task's goal, taken from the spec's **Context & goal**. - - A link to the spec file on this branch — the main `.agents/tasks//spec.md`, or the subtask's - `.agents/tasks//subtasks/-/spec.md` when this is a step sub-branch (`issue-N-step-M`). + - A link to the spec file on this branch: `.agents/tasks//spec.md`. - A note that this is a **spec-first draft**: the branch will receive the task's work subtask by subtask, and the final description will be written when the PR is marked ready for review. 3. **Confirm with the user**, then create as draft: `gh pr create --draft --title "..." --body-file ...`. - Title per "PR title" above (not "spec for..."; the PR becomes the task's/subtask's PR) — a feature - branch's PR describes the whole task, a step sub-branch's PR describes just that subtask. + Title per "PR title" above (not "spec for..."; the PR becomes the task's PR, describing the whole task). 4. **Labels** — copy the issue's labels (`gh issue view --json labels`). Skip ENVs/dependencies labels — nothing is implemented yet; Mode B adds them from the real diff. 5. Link the created PR in the output. @@ -79,13 +77,14 @@ description is a placeholder pointing at the plan: 4. **Confirm with the user**, then flip: `gh pr ready `. (On flipping, the Checks workflow runs — drafts skip it by design.) 5. Link the PR in the output, and **nudge the whole-task review**: now that the PR is ready, `review-changes` - run by hand posts inline comments for the pass no per-leaf review could make (see Land in + run by hand posts inline comments for the pass no per-subtask review could make (see Land in `.agents/tasks/README.md`). ## Mode C — Regular PR (work already done) 1. **Prepare the branch** — as Mode B step 1, plus commit any outstanding changes (with the user's - approval, clear message). + approval, clear message). When the work sits on `main`, create the branch first: `issue-` when + it came from an issue, otherwise a kebab-case slug naming the change. 2. **Write the description** — see "Writing the description" below. 3. **Confirm with the user**, then create: `gh pr create --title "..." --body-file ...` (add `--draft` only if the user asked for it). @@ -99,6 +98,17 @@ description is a placeholder pointing at the plan: issue (`gh issue view `), and start the **Description** section with `Resolves #`. - **Summary of changes:** clear and concise, at most two paragraphs; bullet points if needed. Be precise; keep it short. This is the **Description** section. +- **The why, whenever there is no spec to hold it.** A diff shows *what* changed; the Description is the + only place the reasoning survives, and most PRs through this skill have no spec behind them — work done + by hand, and tasks small enough to finish inside their own grilling session. Add the problem the change + solves and any decision a reader would otherwise have to reverse-engineer, sourced from wherever it + actually is: + - **This conversation**, when the work happened here — the decisions and the alternatives ruled out are + already in context; use them. + - **The issue**, when the branch names one — its body states the problem the diff only implies. + - **The diff and the surrounding code**, otherwise. Infer the intent and write it plainly, then let the + user correct it at the confirmation step — that is what the confirmation is for. Where the reasoning + genuinely cannot be recovered, ask the user for it rather than inventing a rationale. - **Environment variables:** if any env vars were added, changed, or removed, compare or read `./docs/ENVS.md` (and the validator/ENVS docs if relevant) and fill the **Environment variables** section with each variable change and its **purpose** (write "None" if there are none): diff --git a/.agents/skills/grill-the-task/SKILL.md b/.agents/skills/grill-the-task/SKILL.md index 42b6ead0724..abfe401d3e5 100644 --- a/.agents/skills/grill-the-task/SKILL.md +++ b/.agents/skills/grill-the-task/SKILL.md @@ -1,8 +1,9 @@ --- name: grill-the-task description: >- - Grill a product task (GitHub issue) into an implementable spec — research first, then a - one-question-at-a-time interview; also elaborates sub-specs for deferred subtasks of large tasks. + Grill a product task (GitHub issue) into implementable work — research first, then a + one-question-at-a-time interview, then a breakdown quizzed with the developer; also scopes deferred + subtasks. disable-model-invocation: true --- @@ -10,17 +11,19 @@ disable-model-invocation: true Product task issues arrive thin — a title and a couple of links. This skill closes the gap: research everything researchable, then grill the developer about everything that is a *decision*, tracking what they -can't answer as open questions for the responsible people. The output is a spec, written by the `to-spec` -skill. +can't answer as open questions for the responsible people. **Two modes.** -- **Task mode** (default): input is a GitHub issue URL; output is the task's main spec. -- **Subtask mode**: input is an existing spec plus a subtask number (one that has only a `brief.md`, no - `spec.md` yet); the session scopes research and questions to that subtask, reads its folder's `brief.md` - (plus any `research.md` / prototype notes gathered since) as the starting point, and writes its sub-spec - (`subtasks/-/spec.md`). Run it just-in-time, right before the subtask starts, against the - by-then-current code. +- **Task mode** (default): input is a GitHub issue URL. +- **Subtask mode**: input is an existing spec plus a subtask that has only a `brief.md`. The session scopes + research and questions to that subtask, reads its folder's `brief.md` (plus any `research.md` / + prototype notes gathered since) as the starting point, and produces its `spec.md` — along with any + further subtasks the spike revealed, which are appended as siblings. Run it just-in-time, right before + the subtask starts, against the by-then-current code. + +The output is work the developer can act on: a **single-subtask** task is implemented in this same session +and opened as a PR; anything larger is written up by the `to-spec` skill. ## Step 1 — Research @@ -41,7 +44,7 @@ Gather, in roughly this order: is production-deployed or staging-only. 4. **Figma mockups** — via the Figma MCP tools, **enumerate-only**: list screens/frames, their elements, columns, states, and record a node link per screen. Do **not** extract visual/styling details — appearance - stays with the mockups and the `[human]` style subtasks (see `.agents/delegation.md`). If the Figma + stays with the mockups and the `[human]` style leaves (see `.agents/delegation.md`). If the Figma MCP is not connected, have the developer describe the mockups instead. Then run two mechanical cross-checks; every mismatch becomes an open question for the backend owner or PM: @@ -56,42 +59,22 @@ Then run two mechanical cross-checks; every mismatch becomes an open question fo Research is complete when every linked source is read or flagged inaccessible, every named endpoint has a real sample response, and both cross-checks have run with each mismatch recorded as an open question. -## Step 2 — Classify the size - -Propose a size to the developer and confirm it: - -- **small** — one step; a single `spec.md`, no `subtasks/` folder. Implementable by an agent or a user - right after this session. -- **medium** — a breakdown of subtasks, each in its own folder `subtasks/-/`. The main spec is a - slim index; every subtask is scoped now (its `spec.md` written up front). -- **large** — same folder-per-subtask layout, but some subtasks are too big to specify up front. For - those, this session writes only a `brief.md` (the context it gathered + what still needs research, - prototyping, or decisions) into the folder — no `spec.md` — and each gets its own just-in-time - subtask-mode session later that writes the sub-spec. - -As the breakdown takes shape, decide each subtask's readiness with the developer — scoped now (write its -`spec.md`) or deferred (write a `brief.md`, no `spec.md`). **A task with any deferred subtask is `large`; if -every subtask is scoped now, it's `medium`.** The presence of a `spec.md` is the signal that a subtask is -scoped; the main spec's index carries only the done checkbox. - -## Step 3 — The interview +## Step 2 — The interview **Invoke the `grilling` skill** and run the interview under its discipline: one question at a time with a -recommended answer, decisions put to the developer while facts are looked up, and no enactment (Step 4) -until shared understanding is confirmed. Skip anything the research already answered. +recommended answer, decisions put to the developer while facts are looked up, and no enactment until shared +understanding is confirmed. Skip anything the research already answered. **Start by picking the task's contacts**: for each relevant team in `.agents/TEAM.md`, ask which member owns this task, recommending the member marked ✓ in that team's Default column — and record that ✓ member -whenever the developer has no task-specific pick. These go into the spec header, and `to-spec` routes each -open question to the contact that owns it. Don't ask what can be -inferred: when the issue's author maps to a roster member of the relevant team (match the GitHub handle in -`.agents/TEAM.md`), record them as that team's contact without asking — the PM slot in particular is -usually just the task's author. Ask about a **dedicated Slack channel** only for **large** tasks — big -features often get one, and it changes where open questions are sent (see the `to-spec` skill); small and -medium tasks always use the default routing (frontend channel or DMs), so record "—" without asking. When -the developer doesn't know an answer, don't press — record the question with the owning contact and move on. +whenever the developer has no task-specific pick. These go into the spec header, and Step 4 routes each open +question to the contact that owns it. Don't ask what can be inferred: when the issue's author maps to a +roster member of the relevant team (match the GitHub handle in `.agents/TEAM.md`), record them as that +team's contact without asking — the PM slot in particular is usually just the task's author. When the +developer doesn't know an answer, don't press — record the question with the owning contact and move on. -Cover these domains: +Cover these domains, each only where the task actually reaches it — a one-line bug fix touches almost none +of them, and marching through all six regardless is how a five-minute task turns into a twenty-minute one: 1. **Goal & users** — what problem, for whom. 2. **Env gating** — does the feature sit behind a new `NEXT_PUBLIC_*` env var or not. (Just the decision — @@ -107,25 +90,77 @@ Cover these domains: 6. **Delivery** — one question: deploy a demo after completion or not (executed via the `deploy-demo` skill as a final subtask if yes). -Testing is **not** an interview domain — the capability boundary in `.agents/delegation.md` settles it. Neither is -code review or human verification: review is `implement-task`'s call at run time (always under `--auto`, the -developer's choice in a manual run), and the `[verify]` tag follows the standing rule in the **Subtask tags** -section of `.agents/tasks/README.md`. Only ask when a leaf sits genuinely on the line. - -**Front-load the executor skills' inputs.** Once the task breakdown has taken shape, go through every -`[agent]` subtask that will run a project skill (`add-new-page`, `add-api-resource`, `add-env-var`, …): -**open that skill and run its user-facing interview now** (e.g. `add-new-page` Step 0), from the skill's -current text — don't work from memory of its questions. The answers are recorded with the subtask in its -own `spec.md`, so `implement-task` can later execute without stopping to ask. Do this in whichever session -scopes the subtask: here for a subtask specced now, in the just-in-time subtask session for a deferred one -(the one that starts from a `brief.md`). - -The interview is complete when every domain is covered or explicitly skipped as research-answered, the -contacts and channel are settled, every unanswered question has an owner, and every fully-specified -`[agent]` subtask has its executor skill's inputs collected. - -## Step 4 — Hand off to `to-spec` - -Invoke the **`to-spec`** skill. It writes the spec (or sub-spec, in subtask mode), tags subtasks per the -Subtask tags section of `.agents/tasks/README.md`, and runs the open-question outreach (grouping by owner, -drafting Slack messages for the developer's approval, recording thread permalinks). +Testing is **not** an interview domain — the capability boundary in `.agents/delegation.md` settles it. +Neither is code review or human verification: review is `implement-task`'s call at run time (always under +`--auto`, the developer's choice in a manual run), and which acceptance criteria are `(human)` follows the +standing rule in "The subtask model" in `.agents/tasks/README.md`. Only ask when one sits genuinely on the +line. + +The interview is complete when every domain the task reaches is covered or explicitly skipped as +research-answered, the contacts are settled, and every unanswered question has an owner. + +## Step 3 — Quiz the breakdown + +Propose the work as a numbered list of **subtasks** — vertical slices, per "The subtask model" in +`.agents/tasks/README.md`. For each, show the title, what end-to-end behaviour it delivers, and its +`Blocked by:` edges. Then put the breakdown itself to the developer and iterate until they approve it: + +- **Granularity** — too coarse or too fine? The bound is hard: a subtask that does not fit in one fresh + context window is two subtasks. +- **Edges** — does each subtask depend only on the subtasks that genuinely gate it? +- **Merge or split** — anything that should be one slice, or three? + +Two things to look for while drafting it: + +- **Prefactor first.** *Make the change easy, then make the easy change.* When the current code fights the + feature, the first subtask reshapes it with no behaviour change — which also makes it the ideal opener for + an unattended chain, since it earns no `(human)` acceptance criteria and never pauses. +- **Defer what can't be scoped.** A subtask blocked on a prototype, a spike, or an answer nobody has yet + gets a `brief.md` and no `spec.md`; a just-in-time subtask-mode session scopes it later. + +**Then front-load the executor skills' inputs — only when the breakdown has more than one subtask**, since +that is what makes a later session execute it blind. Go through every `[agent]` leaf that will run a project +skill (`add-new-page`, `add-api-resource`, `add-env-var`, …): **open that skill and run its user-facing +interview now** (e.g. `add-new-page` Step 0), from the skill's current text — don't work from memory of its +questions. The answers are recorded with the subtask in its own `spec.md`, so `implement-task` never stops +to ask. A single-subtask task skips this entirely: this session runs the skill itself, so it can just ask +as it goes. + +## Step 4 — Send open questions + +Route every question the session couldn't answer to the person who owns it. + +1. Group them by owner. +2. Pick each group's destination. Ask whether the task has a **dedicated Slack channel** only when the + breakdown came out as several subtasks — big features often get one, and it changes the routing; + otherwise assume there is none. + - Task has a **dedicated feature channel** → **all** questions go there, API ones included. + - Otherwise, **product questions go to the frontend channel** (see `.agents/TEAM.md`) — never a DM — so + colleagues from other teams (QA in particular) build the same understanding of the feature. + - Other questions (API, design) default to a DM with the owner. + - When posting to a channel, **always mention the addressee** — `<@member ID>` from `.agents/TEAM.md` + (people missing from the roster: resolve by name via `slack_search_users` and suggest adding them). +3. Draft one message per owner: brief task context (issue link), the questions, and why they block progress. + Write all Slack messages in **Russian** — the team's internal language (the spec itself stays in English). +4. **Show every draft (with its destination) to the user and wait for explicit approval** — never send + unreviewed outreach. +5. Send (`slack_send_message`), then keep each thread's permalink for the question's spec entry. + +If the Slack MCP tools are unavailable, record the questions with owners anyway and tell the user to route +them manually. + +Outreach is complete when every question has a recorded permalink — or an explicit note that the developer +routes it manually. + +## Step 5 — Hand off + +**One subtask** — this session finishes the job. Create the feature branch (`issue-` off `main`) +with the developer's approval, implement the work, and hand off to the `create-pr` skill, which writes the +reasoning from this conversation into the PR description. No spec is written: nothing is being handed to a +session that wasn't in the room. If a `pending` question blocks the work, wait for the reply and pick the +implementation back up in this same session — and write a spec instead only if the developer asks for one, +which is worth doing when the work will sit before it starts. + +**Several subtasks** — invoke the **`to-spec`** skill. It writes the `spec.md` index plus every subtask +folder, records the Slack permalinks from Step 4, and walks the developer through branch, first commit, and +the draft PR. diff --git a/.agents/skills/implement-task/SKILL.md b/.agents/skills/implement-task/SKILL.md index 29a05d63150..5bb587f4691 100644 --- a/.agents/skills/implement-task/SKILL.md +++ b/.agents/skills/implement-task/SKILL.md @@ -1,8 +1,8 @@ --- name: implement-task description: >- - Execute a product-task spec one leaf subtask per run — [agent] subtasks via the project skills, - [human] subtasks handed off to the developer. `--auto` code-reviews, fixes, commits and chains unattended. + Execute a product-task spec one subtask per run — `[agent]` leaves via the project skills, `[human]` + leaves handed off to the developer. `--auto` code-reviews, fixes, commits and chains unattended. disable-model-invocation: true --- @@ -13,100 +13,99 @@ machine: each run starts a fresh session, picks up where the spec says work stop subtask, updates the spec, and stops so the developer can verify and commit. Any colleague can resume the task from the branch alone. -`--auto` makes the run unattended: it code-reviews each leaf, resolves the findings, commits, and **chains** -to the next leaf without the developer. A manual run does none of that by default — the developer decides per -run whether they want the agentic review or would rather read the diff themselves. +A **subtask** is the unit of a run, a review, and a commit — see "The subtask model" in +`.agents/tasks/README.md`. Its **leaves** are the steps inside it; they are not run boundaries, and their +checkboxes exist so a run can resume mid-subtask, where no commit has been made yet. + +`--auto` makes the run unattended: it code-reviews each finished subtask, resolves the findings, commits, +and **chains** to the next. A manual run does none of that by default — the developer decides per run +whether they want the agentic review or would rather read the diff themselves. ## Invocation -- `/implement-task` — no arguments, the usual case: infer the spec (and the current subtask, from a - `-step-` branch) from the branch name, per the Branch model below. Execute the next eligible subtask - (Step 3). +- `/implement-task` — no arguments, the usual case: infer the spec from the branch name, per the Branch + model below. Execute the next eligible subtask (Step 3). - `/implement-task 4` — a **specific** subtask, out of order. Pending questions still refuse the run; - unchecked dependencies are pointed out and need the developer's explicit confirmation to proceed. -- `/implement-task 2.3` — when a subtask's own spec has a multi-step breakdown, address its **leaf steps** - with dotted numbers: step 3 inside subtask 2's sub-spec (`subtasks/02-/spec.md`). The sub-spec's own - breakdown is the checklist for that subtask; its header status is maintained like a spec's. + unmet blocking edges are pointed out and need the developer's explicit confirmation to proceed. - An explicit task dir as the first argument (e.g. `/implement-task 3219-cross-chain-views 4`) overrides branch inference — needed when not on the feature branch yet. -- `--auto` — chain leaves unattended: review, resolve findings, commit, next leaf, until a stop condition - (Step 9). `--auto 3-5` bounds the chain to those leaves. Typing the flag is what **grants the run - commit authority**; without it nothing is ever committed. - -The unit of one run is always a **leaf**: "next eligible subtask" descends — if the next subtask has its -own multi-step sub-spec, execute the next eligible leaf step *inside* it (one step, then stop). Under -`--auto` the run does not stop after one leaf; it keeps taking leaves until a stop condition fires. +- `--auto` — chain subtasks unattended: review, resolve findings, commit, next subtask, until a stop + condition (Step 9). `--auto 3-5` bounds the chain to those subtasks. Typing the flag is what **grants the + run commit authority**; without it nothing is ever committed. ## Branch model -One **feature branch** holds the whole product task; it lands in `main` as one PR when the task is done. -Within it: a big subtask (several commits) gets its own sub-branch and a PR into the feature branch; a simple -subtask is a single commit directly on the feature branch. Remind the developer of this when a subtask -starts, but **never commit, push, or open PRs yourself** — the developer reviews the diff and commits between -runs. +One **feature branch** holds the whole product task and lands in `main` as one PR when the task is done. +Each subtask is a single commit on it: a subtask is bounded to one context window, so the branch carries the +whole task and every subtask lands directly on it. Under a manual run, **never commit, push, or open PRs +yourself**: the developer reviews the diff and commits between runs. **The one exception is `--auto`**, where the flag itself is the developer's grant: the run commits each -cleared leaf (Step 9), and pushes only when it finalizes the task's last leaf. It still never amends and -never force-pushes, so a per-leaf commit chain with no rewriting means unwinding a bad run is a plain +cleared subtask (Step 9), and pushes only when it finalizes the task's last subtask. It still never amends +and never force-pushes, so a per-subtask commit chain with no rewriting means unwinding a bad run is a plain `git reset`. **PR timing (developer's action — prompt, don't do):** a draft PR opens as soon as the spec is the branch's -first commit (feature branch → `main`; a big subtask's sub-branch → feature branch, with its sub-spec as -the first commit) and flips to ready for review when its breakdown's last box is checked. Nudge accordingly: -on a first run with no PR yet, suggest opening the draft; when checking off the final subtask (or a big -subtask's final leaf step), suggest finalizing it via the `create-pr` skill (finalize-draft mode: real -description from the diff, labels, then ready for review). - -**Branch names carry the addressing.** The feature branch is `issue-` (e.g. `issue-3219`); a big -subtask's sub-branch adds a `-step-` postfix (e.g. `issue-3219-step-2`). An **ad-hoc** spec's branch is its -task-dir slug (`.agents/tasks//` → branch ``). Dash postfixes, **not** slashes — git forbids -`X` and `X/…` coexisting. The names are fully mechanical, which is what lets the skill construct branches -itself and infer the spec (and the current subtask) with no arguments: `issue-` matches the task dir by -issue number, any other branch matches by exact dir name. +first commit, and flips to ready for review when the index's last box is checked. Nudge accordingly: on a +first run with no PR yet, suggest opening the draft; when checking off the final subtask, suggest finalizing +it via the `create-pr` skill (finalize-draft mode: real description from the diff, labels, then ready for +review). + +**Branch names carry the addressing.** The feature branch is `issue-` (e.g. `issue-3219`); an +**ad-hoc** spec's branch is its task-dir slug (`.agents/tasks//` → branch ``). The names are +fully mechanical, which is what lets the skill construct the branch itself and infer the spec with no +arguments: `issue-` matches the task dir by issue number, any other branch matches by exact dir name. ## Workflow ### Step 1 — Load state Resolve the spec per the Invocation section (branch inference by default, explicit task dir wins); if -neither yields a match in `.agents/tasks/`, ask. Read the main spec — and the target subtask's -`subtasks/-/spec.md`, when a `-step-` branch or a dotted target selects one — plus -`.agents/delegation.md`. If the header has no feature branch yet, construct it from the convention -above (`issue-`), confirm with the developer, create it, and record it in the header. +neither yields a match in `.agents/tasks/`, ask. Read the main spec, the target subtask's +`subtasks/-/spec.md`, and `.agents/delegation.md`. If the header has no feature branch yet, +construct it from the convention above (`issue-`), confirm with the developer, create it, and record +it in the header. ### Step 2 — Reconcile the previous handoff -If the previous subtask in the breakdown is `[human]` and still unchecked, ask the developer whether it's -done before doing anything else — check it off if so, stop if it's still in progress (the order exists for a -reason; don't leapfrog a pending style step unless the developer explicitly says the next subtask is -independent). +A run can arrive mid-subtask, because a `[human]` leaf inside one stops the chain. Before anything else: + +- **An unchecked `[human]` leaf in a subtask already in progress** — ask the developer whether it's done. + Check it off if so; stop if it's still in progress. Then resume that subtask at its next unchecked leaf + rather than picking a new one. +- **A finished subtask left uncommitted for verification** — ask whether the `(human)` acceptance criteria + passed. If they did, the developer commits before this run starts new work; if they didn't, fix what + failed inside that subtask instead of moving on. ### Step 3 — Pick the next subtask -If the invocation named a subtask or leaf step, that's the pick (with the guardrails from the Invocation -section). Otherwise: the first unchecked subtask whose dependencies are all checked **and** whose listed -questions are all `resolved` or `waived`, descending into sub-specs to a leaf step. Then: +If the invocation named a subtask, that's the pick (with the guardrails from the Invocation section). +Otherwise take the **frontier**: the first unchecked subtask whose `Blocked by:` edges are all checked +**and** whose listed questions are all `resolved` or `waived`. Then: - **Nothing unchecked left** — the index's last box is checked → the task is done, so **finalize** rather than reporting nothing to do: hand off to the `create-pr` skill in finalize-draft mode. This is the path a - task takes whenever its final leaf is `[human]` or `[verify]`, which the default UI split makes the common - case; without it a finished task would simply stall. -- **All remaining subtasks blocked by `pending` questions** → tell the developer which questions block what, - and suggest running `to-spec` to harvest Slack answers. Stop. -- **Next subtask is `[human]`** → hand off: state what needs doing, link the Figma node, note that the - scaffold's `TODO (design):` markers are the worklist. Stop. -- **Next subtask has only a `brief.md`, no `spec.md` yet** → it isn't scoped; tell the developer to run - `grill-the-task` in subtask mode for it (it writes the folder's `spec.md` from its `brief.md`). Stop. -- **Next subtask is `[agent]`** → proceed. + task takes whenever its final subtask ends on a `[human]` leaf or a `(human)` criterion, which the default + UI split makes the common case; without it a finished task would simply stall. +- **Every remaining subtask blocked by `pending` questions** → tell the developer which questions block + what, so they can chase the threads. Stop. +- **The subtask's first leaf is `[human]`** → hand off: state what needs doing, link the Figma node, note + that the scaffold's `TODO (design):` markers are the worklist. Stop. +- **The subtask has only a `brief.md`, no `spec.md` yet** → it isn't scoped; tell the developer to run + `grill-the-task` in subtask mode for it. Stop. +- **Otherwise** → proceed. ### Step 4 — Execute (one subtask only) -Do the work, composing the project skills wherever one applies (`add-api-resource`, `add-env-var`, -`add-new-page`, `deploy-demo`, …) and staying inside the delegation boundary — scaffolds get placeholder -presentation and `TODO (design):` markers, never final styling. Follow the sub-spec if the subtask has one. +Work the subtask's leaves in order, checking each box as it completes — that is what lets a later run resume +here, since nothing is committed until Step 9. Compose the project skills wherever one applies +(`add-api-resource`, `add-env-var`, `add-new-page`, `deploy-demo`, …) and stay inside the delegation +boundary — scaffolds get placeholder presentation and `TODO (design):` markers, never final styling. + +**Stop at a `[human]` leaf** and hand off as in Step 3; the subtask resumes in a later run. -The spec should already contain the executing skill's inputs — the grilling session runs each skill's -interview up front, so **skip any of the skill's questions the spec answers** and run uninterrupted. If an +The subtask spec should already contain each executing skill's inputs — the grilling session runs those +interviews up front, so **skip any of the skill's questions the spec answers** and run uninterrupted. If an input is genuinely missing, ask the developer and **backfill the answer into the spec** before proceeding. Write the unit tests and Playwright scaffolds `.agents/delegation.md` assigns to agents (test the behavior that matters, not the obvious — per `.agents/rules/tests-unit.md`). @@ -117,24 +116,27 @@ Run every code-quality check the repo defines (per `.agents/rules/code-quality.m only the ones you remember) plus the relevant unit tests. Intentional scaffold `TODO`s may keep ESLint red in the same way the `add-new-page` skill documents — say so explicitly rather than chasing green. +Then walk the subtask's **acceptance criteria** and confirm each unmarked one holds. A `(human)` criterion +is not yours to judge — it is what Step 9 stops for. + ### Step 6 — Review **Under `--auto` this step always runs** — it is what makes an unattended commit defensible. In a **manual** run, ask once: agentic review, or is the developer reviewing the diff themselves? If they take it, go -straight to Step 8; a review nobody asked for spends three subagents and delays the diff they are waiting to -read. +straight to Step 8; a review nobody asked for spends a subagent per axis and delays the diff they are +waiting to read. Dispatch the **`code-reviewer`** agent, synchronously (`run_in_background: false`) — you need its verdict before you can continue. It follows `.agents/skills/review-changes/SKILL.md` and reviews the **uncommitted** -tree, so do not touch a file while it runs. +tree, which is exactly this subtask's whole diff, so do not touch a file while it runs. -Hand it four things, since it starts with an empty context: the **spec path** for this leaf (the sub-spec if -there is one), the **leaf's number and title** — it names the record's section with them — the **round -number** (≥ 2 is what tells it to arbitrate instead of reviewing afresh), and any check failure Step 5 -declared intentional. +Hand it four things, since it starts with an empty context: the **subtask spec's path** — its acceptance +criteria are what the spec axis checks — the **subtask's number and title** (it names the record's section +with them), the **round number** (≥ 2 is what tells it to arbitrate instead of reviewing afresh), and any +check failure Step 5 declared intentional. Step 5 must be **settled** first — green, or red only for the intentional scaffold `TODO`s Step 5 documents. -Reviewing code that does not compile spends three subagents on noise. Pass those known-intentional failures +Reviewing code that does not compile spends every axis on noise. Pass those known-intentional failures to the agent so it does not report them back as findings. The agent returns the review record's path and an `Outcome`: @@ -158,47 +160,49 @@ Nits never gate: `deferred` findings leave the `Outcome` `clear`. ### Step 8 — Update the spec -Check the box for what you did, with a **one-line** note (files/skills involved) — never a multi-line -changelog; git and the PR carry the detail, and any durable decision (a new dependency, an architectural -choice) is folded into the relevant spec section instead. A finding worth keeping — a gotcha, a bug you hit -— goes to the task folder's `notes.md` as evidence the PR can quote, or graduates to a `CONTEXT.md`, a rule, -or the glossary if it's durable repo knowledge; it never lands in the spec as a report. If the work uncovers -a bug from an **earlier, finished** task, fix it here and note it in *this* task's PR — never reopen or edit -that task's frozen spec. See "What a spec holds" in `.agents/tasks/README.md`. +Check the subtask's box in the main index, with a **one-line** note (files/skills involved) — never a +multi-line changelog; git and the PR carry the detail, and any durable decision (a new dependency, an +architectural choice) is folded into the relevant spec section instead. A finding worth keeping — a gotcha, +a bug you hit — goes to the subtask folder's `notes.md` as evidence the PR can quote, or graduates to a +`CONTEXT.md`, a rule, or the glossary if it's durable repo knowledge; it never lands in the spec as a +report. If the work uncovers a bug from an **earlier, finished** task, fix it here and note it in *this* +task's PR — never reopen or edit that task's frozen spec. See "What a spec holds" in +`.agents/tasks/README.md`. -Keep both checklist levels in sync when the subtask has its own sub-spec: +Keep both levels in sync: -- Check the **leaf step** in the sub-spec's breakdown; set the sub-spec's header `Status` to `in progress` - on its first step and `done` when its last box is checked. -- When a sub-spec goes `done`, check its **subtask line in the main index** too. +- Set the subtask spec's header `Status` to `in progress` on its first leaf and `done` when its last box is + checked, then check its line in the **main index**. - Set the **main** header `Status` to `in progress` on the first executed subtask and `done` when the index's last box is checked. The main index is what "done" and draft-PR finalization key off, so never leave it trailing a completed -sub-spec. +subtask spec. ### Step 9 — Commit and chain, or hand off **Without `--auto`**: summarize and end the run. Verification of the diff, the commit, and the next `implement-task` invocation belong to the developer. -**Under `--auto`**, when the `Outcome` is `clear` and the leaf is **not** tagged `[verify]`: commit the leaf -on the task branch — one commit with the review fixes folded in, a plain descriptive subject, and the repo's -`Co-Authored-By` trailer — then return to **Step 3** for the next leaf. Keep the issue out of the commit: no -`#` or issue URL in the subject or body. GitHub adds a timeline reference to the issue on every push -that names it, so a per-leaf chain spams it; the PR's `Resolves #N` already carries the link and closes the -issue on merge. +**Under `--auto`**, when the `Outcome` is `clear` and the subtask has **no `(human)` acceptance criterion**: +commit it on the task branch — one commit for the whole subtask with the review fixes folded in, a plain +descriptive subject, and the repo's `Co-Authored-By` trailer — then return to **Step 3** for the next +subtask. Keep the issue out of the commit: no `#` or issue URL in the subject or body. GitHub adds a +timeline reference to the issue on every push that names it, so a chain of commits spams it; the PR's +`Resolves #N` already carries the link and closes the issue on merge. Stop the chain, pushing nothing, on any of: -- the leaf is `[verify]` → leave it **uncommitted**; the developer verifies per its `verify:` line, then commits -- the next leaf is `[human]`, or has only a `brief.md` -- a `pending` question blocks the next leaf +- the subtask has a `(human)` acceptance criterion → leave it **uncommitted**; the developer checks it + against the running product per the spec's "How to verify" line, then commits what they verified +- the next leaf inside this subtask is `[human]` +- the next subtask is blocked, or has only a `brief.md` +- a `pending` question blocks every remaining subtask - a `needs-human` finding, or a `disputed` finding at the 3-round cap - verification stays red and the fix is not obvious — never thrash on a red build - the `--auto 3-5` scope is exhausted -When the chain clears the task's **last** leaf, finalize: hand off to the `create-pr` skill in +When the chain clears the task's **last** subtask, finalize: hand off to the `create-pr` skill in finalize-draft mode (push, real description from the diff, labels, then ready for review). That is the only point at which an `--auto` run pushes. The full-task review afterwards is the developer's, run by hand. @@ -206,8 +210,8 @@ point at which an `--auto` run pushes. The full-task review afterwards is the de part needed immediately: 1. Stop reason. -2. Leaves completed, with their commit shas. -3. Findings per leaf, counts by severity. +2. Subtasks completed, with their commit shas. +3. Findings per subtask, counts by severity. 4. **Every `rejected-accepted` finding with its one-line reason** — this is where an agent talked itself out of work while nobody was watching, so it must be impossible to miss. 5. Anything `needs-human` or `disputed`, with links. diff --git a/.agents/skills/resolve-review/SKILL.md b/.agents/skills/resolve-review/SKILL.md index f9f32b9225b..13cffa8e1ff 100644 --- a/.agents/skills/resolve-review/SKILL.md +++ b/.agents/skills/resolve-review/SKILL.md @@ -116,7 +116,7 @@ Implement the confirmed `fix` items only, following the conventions in `.agents/ those files define for the code you touched. Leave `reject`, `answered`, `deferred` and `needs-human` findings untouched. -Do **not** commit. In auto mode `implement-task` owns the commit, folding these fixes into the leaf's +Do **not** commit. In auto mode `implement-task` owns the commit, folding these fixes into the subtask's single commit; in manual mode the developer commits. **Done when**: every confirmed fix is applied and locally verified. diff --git a/.agents/skills/review-changes/SKILL.md b/.agents/skills/review-changes/SKILL.md index 1c02c799530..8712a4fff98 100644 --- a/.agents/skills/review-changes/SKILL.md +++ b/.agents/skills/review-changes/SKILL.md @@ -8,7 +8,7 @@ disable-model-invocation: true # Review changes -Review a change the way a lead reviewer would: three **axes** in parallel, each in its own fresh +Review a change the way a lead reviewer would: the **axes** that apply, in parallel, each in its own fresh subagent context, then one normalized report. You produce **findings** and nothing else. Fixing them is `resolve-review`'s job, so this skill never @@ -21,20 +21,20 @@ Mode is not a choice; it follows from where the code is. | Working tree | Mode | Base | | --- | --- | --- | -| Dirty — an uncommitted leaf | markdown record | `HEAD` | +| Dirty — an uncommitted subtask | markdown record | `HEAD` | | Clean, PR open, `HEAD` = the PR's head sha | inline PR comments | merge-base with the PR's base branch | | Clean, PR open, `HEAD` ≠ the PR's head sha | **stop — the branch must be synced first** | — | | Clean, no PR, inside a task dir | markdown record | last reviewed sha in the record, else merge-base with `main` | | Clean, no PR, no task dir | chat only, no file | merge-base with `main` | **First matching row wins.** A dirty tree short-circuits to a markdown record even on a branch whose draft PR -has existed since spec time — that is what keeps a per-leaf review out of PR mode. +has existed since spec time — that is what keeps a per-subtask review out of PR mode. **An open PR plus an out-of-sync branch stops the run.** Say which way it diverged and what to run — `git push` when `HEAD` is ahead, `git pull` when behind — then stop rather than review. There is no good outcome otherwise: lines that were never pushed are absent from the PR diff, so every anchor fails and the all-or-nothing POST discards the whole review; and a markdown record has nowhere to live once the unit is a whole PR rather -than a leaf. +than a subtask. The skill takes no invocation arguments — every case above is inferred. A dispatcher (`implement-task`) may still hand over context it already knows; the steps below name exactly what. @@ -72,7 +72,7 @@ Collect, in the review's own context: - `git diff --stat ` and `git diff --name-only `, plus untracked files (`git ls-files --others --exclude-standard`) — a new file is the most review-worthy thing in a change and `git diff` alone misses it. -- The subtask's `spec.md` (or the task's, or the sub-spec for a leaf step) and `.agents/delegation.md`. +- The subtask's `spec.md` (or the main `spec.md` in a whole-task review) and `.agents/delegation.md`. - The prior review record, if any. **In markdown and chat modes**, run the repo's checks yourself — do not take "checks pass" on trust, @@ -95,8 +95,16 @@ and the diff is known non-empty. ## 2. Round 1 — spawn the axes -Send **one** message with three `general-purpose` subagents. Each gets: the base ref, the touched-file -list plus untracked files, the check output as established fact, and the paths it must read. +Send **one** message with the `general-purpose` subagents the change actually has axes for. Each gets: the +base ref, the touched-file list plus untracked files, the check output as established fact, and the paths it +must read. + +**The spec axis is gated on a spec existing.** Plenty of changes have none — work done outside the task +workflow, and any task finished inside its own grilling session (see "Not every task needs a spec" in +`.agents/tasks/README.md`). Confirm the file is there before dispatching: the subtask `spec.md` a +dispatcher named, or the task dir's specs in a whole-task review. With no spec, run **two** axes and say so +in the report — the spec axis with nothing to read invents a standard to judge against, which is worse than +the gap it papers over. Standards and correctness carry the review on their own. Every axis returns findings in this shape, and nothing else — no preamble, no summary: @@ -110,14 +118,16 @@ fix: Each report is capped at **400 words**, which forces ranking instead of dumping. -**Spec axis brief.** Read the spec (path given) and the diff. Report: requirements the spec asks for -that are missing or partial; behaviour in the diff the spec never asked for; requirements that look +**Spec axis brief.** Read the spec (path given) and the diff. Its **acceptance criteria** are the primary +target: take each unmarked criterion in turn and report whether the diff actually satisfies it. A criterion +marked `(human)` is out of bounds — only a person judging the running product can rule on it. Then report +what the criteria don't cover: behaviour in the diff the spec never asked for, and requirements that look implemented but are implemented wrongly. Quote the spec line behind each finding. Anything the spec's **Out of scope** section names is not a finding. In a **whole-task** review, read the main `spec.md` *and every* `subtasks/*/spec.md`, and add the one check -no per-leaf review can make: leaves that contradict each other — the same concept named, modelled, or gated -two different ways across subtasks. +no per-subtask review can make: subtasks that contradict each other — the same concept named, modelled, or +gated two different ways across the task. **Standards axis brief.** Read `.agents/rules/*.md` matching the touched file types, every `CONTEXT.md` for directories the diff touches, `.agents/delegation.md`, and **one** smell baseline, picked by what the @@ -142,7 +152,7 @@ Report: logic errors; mishandled loading / empty / error / pagination paths; pla claim something the runtime does not; and tests that assert the framework or the mock rather than real behaviour (per the "What to test (and what not)" section of `.agents/rules/tests-unit.md`). -**Done when**: all three axes have returned, or an axis has failed and you have noted which. +**Done when**: every dispatched axis has returned, or one has failed and you have noted which. ## 3. Round 2+ — arbitration @@ -163,7 +173,7 @@ the current diff. It does exactly three things: ## 4. Normalize -Only this context sees all three axes, so only it can calibrate. Left alone, each axis inflates its own +Only this context sees every axis, so only it can calibrate. Left alone, each axis inflates its own findings to `blocker` because that axis is all it can see. - **Severity.** `blocker` — a spec requirement missing or wrong, a correctness bug, or a rule breach @@ -187,8 +197,8 @@ findings to `blocker` because that axis is all it can see. Zero findings still produces a report with `Outcome: clear` and zeroed counts — a missing record is indistinguishable from a review that never ran. -**Markdown mode.** Write `review.md` beside the spec it was reviewed against: in the subtask folder, or -next to a small task's `spec.md`. One `##` section per reviewed unit — a round never opens a section of its +**Markdown mode.** Write `review.md` beside the spec it was reviewed against, in the subtask folder. One +`##` section per reviewed unit — a round never opens a section of its own; it bumps the header's `Round` and appends exchange lines under the findings it touched. Give each new finding its starting **Status**: `open`, or `deferred` for a nit, which is never auto-fixed. Every later transition belongs to `resolve-review`. Format in [`review-template.md`](review-template.md). @@ -217,7 +227,7 @@ reader to skim past real problems. **Always out of bounds** -- **Any visual or styling judgement.** Presentation belongs to the `[human]` style subtask. +- **Any visual or styling judgement.** Presentation belongs to the `[human]` style leaf. - **Anything the spec's Out of scope section names.** - **Missing Playwright screenshot baselines** — human-generated, per `.agents/delegation.md`. - **Style preferences with no basis** in `.agents/rules/`, a `CONTEXT.md`, or the surrounding code. No @@ -226,7 +236,7 @@ reader to skim past real problems. - **A primitive or shortcut with a why-comment on it.** The comment is an override signal: read it and back off rather than arguing with it. -**Tolerated in a per-leaf review only**, because an `[agent]` scaffold leaf is deliberately unfinished: +**Tolerated in a per-subtask review only**, because an `[agent]` scaffold leaf is deliberately unfinished: - **`TODO (design):` markers** — the scaffold working as designed. This tolerance **expires** at the whole-task review, where the PR is ready for review, belongs to no subtask, and the `[human]` style leaves diff --git a/.agents/skills/review-changes/review-template.md b/.agents/skills/review-changes/review-template.md index 05f8bb428a8..f6300536a83 100644 --- a/.agents/skills/review-changes/review-template.md +++ b/.agents/skills/review-changes/review-template.md @@ -1,8 +1,8 @@ # Review — - -## Step — +## Subtask — | | | | --- | --- | diff --git a/.agents/skills/to-spec/SKILL.md b/.agents/skills/to-spec/SKILL.md index 133fc79e630..fa452f3ac9c 100644 --- a/.agents/skills/to-spec/SKILL.md +++ b/.agents/skills/to-spec/SKILL.md @@ -1,53 +1,50 @@ --- name: to-spec description: >- - Convert the current conversation into a product-task spec in .agents/tasks/, or update an existing - spec — folding in new decisions, harvesting colleague replies from Slack threads, and sending open - questions to their owners. Use at the end of a grilling session, when the user wants to capture any - conversation as a spec, or to sync a spec's open questions with Slack. + Write the current conversation into a product-task spec in .agents/tasks/, or update an existing spec — + folding in decisions taken since, including answers that came back from colleagues. Use at the end of a + grilling session, when the user wants a conversation captured as a spec, or when a spec needs updating. --- # To spec -Turn the current conversation into a spec file — or merge it into one that already exists. The spec is the -single source of truth for a product task: `implement-task` executes from it, humans work from it, and its -open questions drive the Slack round-trip with PMs, designers, and backend engineers. +Turn the current conversation into a spec — or merge it into one that already exists. **Synthesize what +the conversation already settled; do not interview.** The decisions, the breakdown, and the open questions +were made in the session that hands off to this skill (`grill-the-task`, normally); this skill's whole job +is writing them down in the right files. -This skill is **conversation-agnostic**: it is normally invoked at the end of a `grill-the-task` session, -but works from any conversation that contains decisions worth capturing — including an **empty** one. A -fresh session invoking it on an existing spec (e.g. `/to-spec 3219-cross-chain-txs`) is the normal way to -sync Slack replies: there is nothing to convert, so the run is just harvest (Step 2) plus outreach (Step 4). +That boundary cuts both ways. Reading Slack replies is **not** this skill's job either — the developer +reads the threads in their own session and brings the answers into the conversation; what arrives here is +a decision to fold in, like any other. + +The spec is the single source of truth for a product task: `implement-task` executes from it and humans +work from it. Not every task gets one — see "Not every task needs a spec" in `.agents/tasks/README.md`. ## Spec location and structure - With a GitHub issue: `.agents/tasks/-/spec.md` — the bare issue number, then a kebab-case slug naming the task. - Ad-hoc (no issue): `.agents/tasks//spec.md`. -- Every subtask of a medium/large task gets its own folder `subtasks/-/`, holding: - - `brief.md` — the handoff from the initial grilling session for a subtask that isn't scoped yet. Its - presence (with no `spec.md`) marks the subtask as not-yet-scoped. It carries: the subtask's goal in a - sentence or two; the context already gathered (relevant code, endpoints, mockups); the specific unknowns - to resolve (what to research, prototype, or decide) and who owns each; and links (issue, Figma, related - specs) — enough for a `grill-the-task` subtask session to start without re-deriving it. - - `spec.md` — the subtask spec (same template), Status `draft | ready | in progress | done`. Written up - front for a scoped subtask, or by the just-in-time subtask session for a deferred one — filled from the - folder's `brief.md`. - - `research.md` — optional; real research findings or prototype notes produced before the subtask - session, feeding it alongside the brief. - -Use `spec-template.md` (next to this file) for every spec — main and subtask alike. Structure by size: - -- **small** — one `spec.md`, no `subtasks/`; the whole task is a single leaf worklist. -- **medium** — the main spec is a slim index; each subtask is a folder with a fully-specified `spec.md`. -- **large** — same layout; big subtasks are deferred (a `brief.md` now, no `spec.md`; the sub-spec is - written just-in-time later). - -**The main spec is an index, not a container.** Its Task breakdown is one line per subtask (checkbox + -title + folder link) — never inline inputs, requirements, or changelogs; that detail belongs in the -subtask's own `spec.md`. Tag every subtask per the **Subtask tags** section of `.agents/tasks/README.md` — -`[agent]`/`[human]` on every subtask, and `[verify]` (plus its `verify:` line) decided for every `[agent]` -leaf. Never leave a tag implicit; `implement-task` reads them as its state machine. Specs merge with the -task's PR and accumulate in `.agents/tasks/` as precedent. +- Every subtask gets its own folder `subtasks/-/`, holding either: + - `spec.md` — the subtask spec, written from `subtask-template.md` (next to this file). Status + `draft | ready | in progress | done`. + - `brief.md` — for a subtask that **isn't scoped yet**; its presence with no `spec.md` is the only marker + of a deferred subtask. It carries: the subtask's goal in a sentence or two; the context already gathered + (relevant code, endpoints, mockups); the specific unknowns to resolve (what to research, prototype, or + decide) and who owns each; and links (issue, Figma, related specs) — enough for a `grill-the-task` + subtask session to start without re-deriving it. + - `research.md` — optional; research findings or prototype notes produced before the subtask session, + feeding it alongside the brief. + +**The main spec is an index, not a container.** Its Task breakdown is one line per subtask — checkbox, +title, folder link, blocking edges — and never inlines requirements, inputs, or changelogs. Write it from +`spec-template.md`; write every subtask from `subtask-template.md`. The two templates are shaped +differently on purpose: the main spec holds the task's shared facts, a subtask spec holds one vertical +slice's contract. "The subtask model" in `.agents/tasks/README.md` is the definition of both — follow it +rather than restating it, and tag every leaf explicitly, since `implement-task` reads the tags as its state +machine. + +Specs merge with the task's PR and accumulate in `.agents/tasks/` as precedent. ## Workflow @@ -56,87 +53,50 @@ task's PR and accumulate in `.agents/tasks/` as precedent. Derive the path from the issue (or ask for a slug). If the file already exists, this is an **update** run: read the spec first and treat it as hand-editable — developers edit specs directly between runs. -### Step 2 — Harvest Slack answers (update runs only) - -Open questions live in the main `spec.md` **and** in any subtask `spec.md` under `subtasks/*/` — gather -them from all of these files. For every open question with status `pending` and a recorded Slack permalink: - -1. Read the thread with the Slack MCP tools (`slack_read_thread`; parse `channel_id`/`message_ts` from the - permalink as in the `create-issue-from-slack-thread` skill). -2. If there are replies, summarize them and propose a resolution to the user. -3. On acceptance: fold the decision into the affected spec section(s), set the question's status to - `resolved`, and record the answer as a phrase (the decision + date) in its entry — the decision, not the - deliberation, and nothing this public repo shouldn't carry (see "What a spec holds" in - `.agents/tasks/README.md`); the recorded permalink holds the rest. -4. If a reply raises a follow-up: draft it (in Russian, like all outreach), get the user's approval, send it - **into the same thread**, and keep the question `pending`. +### Step 2 — Write or merge -The harvest is complete when every `pending` question with a permalink — across the main spec and every -subtask spec — has had its thread read and is now resolved, followed up, or confirmed still unanswered. - -### Step 3 — Write or merge the spec - -Extract from the conversation: decisions, requirements, data/API facts, UI inventory, size classification, -task breakdown, and unanswered questions with their owners — the per-team contacts picked during the -session (defaults from `.agents/TEAM.md`), recorded in the header. +Extract from the conversation: decisions, requirements, data/API facts, UI inventory, the approved task +breakdown with its blocking edges, and unanswered questions with their owners — the per-team contacts +picked during the session (defaults from `.agents/TEAM.md`), recorded in the header. Record the Slack +permalink of every question the session already sent. **Write to the right file.** Task-level facts (context, shared data/API, overall UI inventory, out-of-scope, -the index breakdown) go in the main `spec.md`; a subtask's own requirements, data, UI, executor-skill -`inputs:`, and leaf worklist go in `subtasks/-/spec.md`. A subtask that isn't scoped yet gets a -`brief.md` instead of a `spec.md`. +the index breakdown) go in the main `spec.md`; a subtask's own what-to-build, acceptance criteria, blocking +edges, executor-skill `inputs:`, and leaf worklist go in `subtasks/-/spec.md`. A subtask that +isn't scoped yet gets a `brief.md` instead. **Merge surgically.** On update runs, never regenerate the file: preserve checked boxes, statuses, hand -edits, and resolved-question records; only add or amend what the conversation actually changed. Show the -user a summary of the changes and confirm before moving on. +edits, and resolved-question records; only add or amend what the conversation actually changed. When an +answer resolves a question, fold the decision into the section it affects, set the question's status to +`resolved`, and record the answer as a phrase plus its date — the decision, not the deliberation, and +nothing this public repo shouldn't carry. Show the user a summary of the changes and confirm before moving +on. -**No changelogs.** Record a subtask's completion as a one-line note on its checkbox, nothing more — the -commit and the PR are the record of what changed. Durable decisions taken during work (a new dependency, an -architectural choice) are folded into the relevant spec section, not appended as a "done: …" block. See -"What a spec holds" in `.agents/tasks/README.md` for what stays in the spec versus what lives in its thread, -the code, or the PR. +**Scoping a deferred subtask.** A subtask session hands off with its folder's `spec.md` to write, plus +whatever else the spike revealed. Append those as new sibling subtasks and retarget the `Blocked by:` edges +that pointed at the deferred one; never renumber, and never nest a subtask inside a subtask. -Status field: a new spec starts as `draft`; set it to `ready` once no `pending` question blocks the first -subtask (per-subtask blocking — unblocked subtasks may proceed while unrelated questions are pending). +**No changelogs.** A subtask's completion is its checked box plus a one-line note — the commit and the PR +are the record of what changed. Durable decisions taken during work (a new dependency, an architectural +choice) are folded into the relevant section, not appended as a "done: …" block. See "What a spec holds" in +`.agents/tasks/README.md` for what stays in the spec versus what lives in its thread, the code, or the PR. -### Step 4 — Send open questions (outreach) - -For `pending` questions that have **no** Slack permalink yet: - -1. Group them by owner. -2. Pick each group's destination: - - Task has a **dedicated feature channel** (spec header) → **all** questions go there, API ones included. - - Otherwise, **product questions go to the frontend channel** (see `.agents/TEAM.md`) — never a DM — so - colleagues from other teams (QA in particular) build the same understanding of the feature. - - Other questions (API, design) default to a DM with the owner. - - When posting to a channel, **always mention the addressee** — `<@member ID>` from `.agents/TEAM.md` - (people missing from the roster: resolve by name via `slack_search_users` and suggest adding them). -3. Draft one message per owner: brief task context (issue link), the questions, and why they block progress. - Write all Slack messages in **Russian** — the team's internal language (the spec itself stays in English). -4. **Show every draft (with its destination) to the user and wait for explicit approval** — never send - unreviewed outreach. -5. Send (`slack_send_message`), then record each thread's permalink in the question's entry. - -If the Slack MCP tools are unavailable, record the questions with owners anyway and tell the user to route -them manually. - -Outreach is complete when every `pending` question has a recorded permalink — or an explicit note that the -developer routes it manually. +Status field: a new spec starts as `draft`; set it to `ready` once no `pending` question blocks the first +subtask — blocking is per-subtask, so unblocked subtasks may proceed while unrelated questions are pending. -### Step 5 — Branch and draft PR (first creation only) +### Step 3 — Branch and draft PR (first creation only) -When this run **created** the spec (or sub-spec), bootstrap the workflow's draft-PR-first policy — each -action only with the developer's explicit approval, never unprompted: +When this run **created** the spec, bootstrap the workflow's draft-PR-first policy — each action only with +the developer's explicit approval, never unprompted: -1. **Branch** — main spec: `issue-` off `main`; sub-spec (subtask mode): `issue--step-` - off the feature branch; **ad-hoc spec** (no issue): the task-dir slug itself (spec in - `.agents/tasks//` → branch ``). Create/switch if needed and record the branch in the spec - header. +1. **Branch** — `issue-` off `main`, or, for an **ad-hoc spec** (no issue), the task-dir slug + itself (spec in `.agents/tasks//` → branch ``). Create/switch if needed and record the + branch in the spec header. 2. **Commit** — propose committing the spec as the branch's first commit; show what will be committed and wait for confirmation. -3. **Draft PR** — suggest opening it right away via the `create-pr` skill (draft-placeholder mode; feature - branch → `main`, sub-branch → feature branch). Why drafts open this early is documented in - `.agents/tasks/README.md`; the PR flips to ready when the breakdown's last box is checked (the - `implement-task` skill nudges at that moment). +3. **Draft PR** — suggest opening it right away via the `create-pr` skill (draft-placeholder mode, feature + branch → `main`). Why drafts open this early is documented in `.agents/tasks/README.md`; the PR flips to + ready when the breakdown's last box is checked (the `implement-task` skill nudges at that moment). For ad-hoc specs the draft PR doubles as a **parking spot**: an idea captured as a spec today can sit in its draft PR and be picked up, refined, or implemented days later — visible on GitHub instead of only in a diff --git a/.agents/skills/to-spec/spec-template.md b/.agents/skills/to-spec/spec-template.md index c062bf69193..2413d3180a4 100644 --- a/.agents/skills/to-spec/spec-template.md +++ b/.agents/skills/to-spec/spec-template.md @@ -4,23 +4,15 @@ | --- | --- | | Issue | | | Status | `draft` \| `ready` \| `in progress` \| `done` | -| Size | `small` \| `medium` \| `large` | | Feature branch | `` | | PM | | | Designer | | | Backend | | | Minimum API version | | -| Slack channel | <#feature-channel if the task has one; otherwise "—" (default routing per `to-spec`)> | +| Slack channel | <#feature-channel if the task has one; otherwise "—" (default routing per `grill-the-task`)> | - - - + ## Context & goal @@ -28,7 +20,8 @@ overrides one. --> ## Functional requirements - + ## Data & API @@ -40,7 +33,7 @@ staging-only) and the backend release version that ships the changes (for releas @@ -51,38 +44,26 @@ spec holds" in `.agents/tasks/README.md`. --> ## Task breakdown - - -- [ ] 1 `[agent]` — skill: `add-api-resource` — questions: Q2 - - inputs: - - <executor-skill answer> -- [ ] 2 `[agent]` `[verify]` <title> — skill: `add-new-page` - - inputs: - - <executor-skill answer> - - verify: `pnpm dev:preset <alias>`, open <route>, confirm <the behaviour a human must judge> -- [ ] 3 `[human]` Style <component> to mockup — [Figma](<node URL>) +<!-- A slim INDEX of vertical slices — one line per subtask, nothing else. No inputs, no leaf steps, no +changelog: that detail lives in the subtask's own `subtasks/<NN>-<slug>/spec.md`. + +Each line carries exactly three things: the done checkbox (the ONLY per-subtask state this file tracks — +readiness is derived, never stored), the title, and the blocking edges. Numbers are identity, not order; +a new subtask is appended and its edges placed, never renumbered. See "The subtask model" in +`.agents/tasks/README.md`. + +A deferred subtask links its `brief.md` instead, and is scoped just-in-time by a `grill-the-task` subtask +session. --> + +- [ ] 01 <title> → [`subtasks/01-<slug>/`](subtasks/01-<slug>/spec.md) — blocked by: none +- [ ] 02 <title> → [`subtasks/02-<slug>/`](subtasks/02-<slug>/brief.md) — blocked by: 01 ## Open questions <!-- One entry per question. Status is the gate `implement-task` checks. The Slack permalink is recorded -when the question is sent, so answers can be harvested later. When resolved, fold the decision into the -section above that it affects AND record it here. --> +when the question is sent, so answers can be folded in later. When resolved, fold the decision into the +section above that it affects AND record it here. A question scoped to one subtask lives in that subtask's +spec instead. --> ### Q1 — <question> diff --git a/.agents/skills/to-spec/subtask-template.md b/.agents/skills/to-spec/subtask-template.md new file mode 100644 index 00000000000..b9dd92c8e02 --- /dev/null +++ b/.agents/skills/to-spec/subtask-template.md @@ -0,0 +1,69 @@ +# <NN> — <Subtask title> + +| | | +| --- | --- | +| Parent spec | `.agents/tasks/<dir>/spec.md` → link it as `../../spec.md`, subtask <NN> of #<issue> | +| Status | `draft` \| `ready` \| `in progress` \| `done` | +| Blocked by | <subtask numbers that must be checked first, or "none"> | + +<!-- People rows are inherited from the parent spec; add one here only to override it. A subtask that +hasn't been scoped yet has NO `spec.md` at all — only a `brief.md` in its folder, which a just-in-time +`grill-the-task` subtask session turns into this file. --> + +## What to build + +<!-- The end-to-end behaviour this subtask makes work, from the user's perspective — a paragraph, not a +layer-by-layer plan. A subtask is a vertical slice: it cuts a narrow but complete path through every layer +it touches and is verifiable on its own. If it does not fit in one fresh context window, it is two +subtasks. --> + +## Acceptance criteria + +<!-- What must be true when this subtask is done. The review checks every unmarked criterion; a `(human)` +criterion is one only a person looking at the running product can judge, and having one is what makes +`implement-task` pause for verification before it commits. Which criteria earn `(human)` is defined in +"The subtask model" in `.agents/tasks/README.md`. Drop the "How to verify" line when nothing is `(human)`. --> + +How to verify: `pnpm dev:preset <alias>`, open <route> + +- [ ] <criterion the review can check from the diff> +- [ ] `(human)` <criterion only a person judging the running product can check> + +## Details + +<!-- OPTIONAL — only what this subtask needs beyond the main spec's Data & API and UI inventory: the +endpoint and `service:name` resource it touches, the Figma node for its screen, a deliberate deviation and +its reason. Point at existing code by symbol name, never by transcribing its values or line numbers. +Delete the section when the main spec already carries everything. --> + +## Leaf worklist + +<!-- The actual steps, each one project skill's worth of work. Leaves run along layers (resource, then +page, then styling) while the subtask cuts across them. + +Tag every leaf `[agent]` or `[human]` — explicitly, never implied; `implement-task` reads the tags as its +state machine. A UI component is two linked leaves (scaffold → style). Record the executing skill's +interview answers as an indented `inputs:` list, so `implement-task` never stops to ask. + +A leaf's checkbox is RESUMPTION STATE — where to pick up inside a subtask that has no commits yet. It is +never a changelog: one line at most, and durable decisions get folded into the sections above instead. --> + +- [ ] 1 `[agent]` <title> — skill: `add-api-resource` + - inputs: + - <executor-skill answer> +- [ ] 2 `[agent]` <title> — skill: `add-new-page` + - inputs: + - <executor-skill answer> +- [ ] 3 `[human]` Style <component> to mockup — [Figma](<node URL>) + +## Open questions + +<!-- Questions scoped to this subtask only; task-wide ones live in the main spec. Same format and the same +gate: this subtask can't start while one is `pending`. --> + +### Q1 — <question> + +- Owner: <role> (<name>) +- Status: `pending` \| `resolved` \| `waived` +- Slack: <permalink, once sent> +- Answer: <the decision as a phrase, + date — the decision, not the deliberation> diff --git a/.agents/tasks/README.md b/.agents/tasks/README.md index 89a9bcadacc..6e416ca3f9f 100644 --- a/.agents/tasks/README.md +++ b/.agents/tasks/README.md @@ -1,9 +1,8 @@ # Product task specs -This directory holds one folder per product task, each with a `spec.md`. A medium/large task also has a -`subtasks/` folder with one sub-folder per subtask (`subtasks/NN-<slug>/`). Specs merge with their task's -PR and **accumulate here as a permanent record** — consult past specs as precedent for how similar tasks -were scoped and split. +This directory holds one folder per specced product task: a `spec.md` index plus one folder per subtask +(`subtasks/NN-<slug>/`). Specs merge with their task's PR and **accumulate here as a permanent record** — +consult past specs as precedent for how similar tasks were scoped and split. ## Why @@ -12,6 +11,13 @@ that a developer fills the gaps with guesswork. The spec workflow fixes the inpu gaps, unanswerable questions get routed to the people who own the answers, and the resulting spec explicitly says which steps an agent does and which a developer does by hand. +## Not every task needs a spec + +A spec exists to **hand work to a session that wasn't in the room**. A task whose breakdown comes out as a +single subtask never leaves the room: it is grilled, implemented, and opened as a PR inside one session, so +it gets no folder here and its reasoning goes into the PR description instead. Ask for a spec anyway when +the work will sit before it starts — the draft PR then parks it somewhere visible. + ## What a spec holds A spec is an **index of decisions**, not a worklog: *what* to build and *why*, pointing at detail instead of @@ -36,90 +42,102 @@ architectural choice): fold it into the section it changes, as revised intent. Once its task is **done**, a spec is frozen — a record of what was decided then. A later task that finds a bug from an earlier one fixes it in its *own* spec and PR; it never rewrites the finished spec. -## Lifecycle +## The subtask model -1. **Grill** — run the `grill-the-task` skill with the issue URL. It researches first (issue, codebase, live - API samples, Figma mockups — enumerate-only), then interviews you one question at a time. What you can't - answer becomes an open question with an owner. -2. **Spec** — the session ends in the `to-spec` skill: it writes a slim index `spec.md` here plus one - `subtasks/NN-<slug>/` folder per subtask (a `spec.md` if it's scoped now, or a `brief.md` if it's - deferred to its own later session), sizes the task (small / medium / large), tags every subtask per - **Subtask tags** below — `[agent]` / `[human]`, plus `[verify]` where a human must judge the running - product — then - drafts the open questions as Slack messages grouped by owner — you approve, it sends, and each thread's - permalink lands in the spec. (`to-spec` also works standalone, from any conversation worth capturing.) - Commit the spec to the feature branch and **open a draft PR right away** (`to-spec` walks you through - branch, commit, and draft PR at the end of the run) — a spec-only draft is the cheap moment to catch a - wrong split or a missed requirement, it links the issue to the work, and CI and demo deploys hang off it - for the rest of the task. -3. **Answers** — when colleagues reply, run `to-spec` on the spec again: it harvests the Slack threads, - proposes resolutions, folds accepted decisions into the spec, and sends approved follow-ups. -4. **Implement** — run the `implement-task` skill repeatedly, one subtask per run: it executes `[agent]` - subtasks (composing `add-api-resource`, `add-new-page`, `add-env-var`, …) and verifies them, or hands - `[human]` subtasks (styling to Figma mockups) over to you. In a manual run you review the diff and commit - between runs; under `--auto` the run commits each cleared leaf itself (step 5). - A subtask can't start while a question blocking it is `pending` — unrelated subtasks can. -5. **Review** — the `review-changes` skill reviews a subtask in a fresh subagent context on three axes (spec - compliance, repo standards, correctness). Findings land in the subtask folder's `review.md`, and - `resolve-review` adjudicates each one — fix, or reject with a written reason that an arbitration round - rules on. `implement-task --auto` runs that whole cycle unattended, then commits the cleared leaf and - chains to the next, stopping at the first `[verify]` leaf, `[human]` subtask, `needs-human` finding, or - unsettled dispute. In a manual run the review is optional: you either ask for it or read the diff - yourself. -6. **Land** — flip the draft PR to **ready for review** when the spec's last box is checked; the feature - branch merges to `main` as one PR, spec included. Big subtasks may have had their own sub-branch + PR - into the feature branch along the way (same pattern: draft when the step starts with its sub-spec as the - first commit, ready when the step's boxes are checked); simple ones are single commits on it. Branch - names carry the addressing — feature branch is `issue-<number>` (`issue-3219`), a big subtask's sub-branch - adds `-step-<N>` (`issue-3219-step-2`) — so `implement-task` needs no arguments on a task branch. Once the - PR is ready, run `review-changes` **by hand** for the whole-task pass: per-subtask reviews structurally - cannot see inconsistency between subtasks, duplicated helpers, or dead scaffolding. On a pushed branch it - posts inline PR comments instead of a file, so your own review comments and the agent's sit side by side — - `resolve-review` closes out both, and never rejects a human's. +**This section is the only definition of the model** — other files point here rather than restating it. + +A **subtask is a vertical slice**: a narrow but complete path through every layer it touches, verifiable on +its own once it lands. Two hard bounds make it the unit everything else keys off: + +- It **fits in one fresh context window** — the sizing test the breakdown is quizzed against. +- It is **one commit**, made once its review comes back clear. + +Inside a subtask, the **leaves** are the actual steps, and they run *along* layers — one project skill each +(`add-api-resource`, then `add-new-page`, then the styling). See +[`../adr/0002-layer-shaped-subtask-leaves.md`](../adr/0002-layer-shaped-subtask-leaves.md) for why the two +levels cut in different directions. -## Task sizes +### Leaves -- **small** — one step; a single `spec.md`, no `subtasks/` folder. An agent or a user can implement it - right after the grilling session. -- **medium** — the main `spec.md` is a slim index; each subtask lives in its own - `subtasks/NN-<slug>/spec.md`, fully specified up front (`ready`). -- **large** — same layout, but big subtasks are deferred: the grilling session drops a `brief.md` in the - folder now (no `spec.md`), and each gets its sub-spec written **just-in-time** via a `grill-the-task` - subtask session right before it starts. +Each leaf carries `[agent]` or `[human]` per the capability boundary in `.agents/delegation.md` — exactly +one, written explicitly, because `implement-task` reads the tag as its state machine. UI work is **two +linked leaves** by default: an `[agent]` scaffold, then a `[human]` style leaf that takes it to the mockup — +layout, spacing, styling, icons — with the exact Figma node linked on that leaf and the scaffold's +`TODO (design):` markers as its worklist. -A subtask is "scoped" once its folder has a `spec.md`; until then it holds only a `brief.md`. The main -spec's breakdown carries only the done checkbox and a link to each subtask folder. +A leaf's checkbox is **resumption state**: a subtask has no commits until it finishes, so the boxes are the +only durable record of the frontier inside it. They say where to pick up, never what was done — the commit +and the PR carry that. -## Subtask tags +### Acceptance criteria -A breakdown line carries its per-subtask state as tags. **This section is the only definition of them** — -other files point here rather than restating. +Every subtask spec carries a checklist of what must be true when it is done. A criterion marked `(human)` +is one only a person looking at the running product can judge; every other criterion is what the review +checks. One `(human)` criterion is what makes `implement-task` pause for verification before it commits — +the list *is* the gate, so nothing separate has to stay in sync with it. -- **`[agent]` / `[human]`** — who does the work, per the capability boundary in `.agents/delegation.md`. - Exactly one per subtask. UI work is **two linked leaves** by default: an `[agent]` scaffold, then a - `[human]` style leaf that takes it to the mockup — layout, spacing, styling, icons — with the exact Figma - node linked on that leaf and the scaffold's `TODO (design):` markers as its worklist. -- **`[verify]`** — on an `[agent]` leaf only: once the code review comes back clear, a **human must verify - the running product** before the leaf is committed. Such a leaf also carries a `verify:` line saying how - to check it. +The test for a `(human)` criterion: *does this change what a user sees or does?* -`to-spec` writes every tag explicitly, never implicitly, because `implement-task` reads them as its state -machine: it hands `[human]` subtasks over, and it leaves a cleared `[verify]` leaf **uncommitted** for the -developer instead of committing it and chaining on. +- **Earns one** — component scaffolds (placeholder ones included), data wiring that renders, page + behaviour, perf-sensitive changes, anything touching CSP or security, and new dependencies. +- **Does not** — env vars, API resources and response types, route plumbing, metadata, sitemap, analytics, + unit tests, glossary and docs, behaviour-preserving refactors. -### When a leaf needs `[verify]` +Getting it wrong costs in both directions: a needless `(human)` criterion stalls an unattended chain, and a +missing one lets `--auto` commit something nobody looked at. -The test: *is there user-visible behaviour only a human can judge?* The review covers the code; it cannot -tell whether the running product is right. +### Order -- **No `[verify]`** — env vars, API resources and response types, route plumbing / metadata / sitemap / - analytics, unit tests, glossary and docs, behaviour-preserving refactors. -- **`[verify]`** — anything that changes what a user sees or does: component scaffolds (placeholder ones - included), data wiring that renders, page behaviour, perf-sensitive changes, anything touching CSP or - security, and new dependencies. +Every subtask declares `Blocked by:` — the subtasks that must be checked before it can start, or `None`. +**Numbers are identity, not order**: the edges carry the order, which is what lets a subtask be appended +without renumbering anything. A subtask may also list blocking question ids; it can't start while one is +`pending`, and unrelated subtasks carry on regardless. -Getting it wrong costs in both directions: a needless `[verify]` stalls an unattended chain, and a missing -one lets `--auto` commit something nobody looked at. +`implement-task` works the **frontier** — any subtask whose blockers are checked and whose questions are +settled. + +### Deferred subtasks + +A subtask that can't be scoped until something happens first — a prototype, a spike, an answer nobody has +yet — gets a `brief.md` in its folder and **no** `spec.md`. That absence is the only marker; nothing labels +the task as a whole. + +A just-in-time `grill-the-task` subtask session scopes it later, against the by-then-current code: the +folder gets its `spec.md`, and whatever else the spike revealed is **appended as new sibling subtasks**, +with `Blocked by:` edges retargeted to match. The structure stays flat — a subtask never contains subtasks. + +## Lifecycle + +1. **Grill** — run the `grill-the-task` skill with the issue URL. It researches first (issue, codebase, + live API samples, Figma mockups — enumerate-only), interviews you one question at a time, quizzes the + breakdown with you, and sends what you couldn't answer to its owner on Slack once you approve the draft. + A **one-subtask** task ends here: the session implements it and `create-pr` opens the finished PR. +2. **Spec** — the session hands off to the `to-spec` skill, which writes the `spec.md` index plus every + subtask folder (a `spec.md` where the subtask is scoped, a `brief.md` where it is deferred) and records + each question's Slack permalink. Commit the spec to the feature branch and **open a draft PR right away** + (`to-spec` walks you through branch, commit, and draft PR at the end of the run) — a spec-only draft is + the cheap moment to catch a wrong split or a missed requirement, it links the issue to the work, and CI + and demo deploys hang off it for the rest of the task. +3. **Answers** — when colleagues reply, read the threads and ask for the spec to be updated. `to-spec` + folds each accepted decision into the section it changes and marks the question `resolved`. +4. **Implement** — run the `implement-task` skill repeatedly, **one subtask per run**: it executes the + subtask's `[agent]` leaves (composing `add-api-resource`, `add-new-page`, `add-env-var`, …), verifies + them, and hands `[human]` leaves over to you. In a manual run you review the diff and commit between + runs; under `--auto` the run reviews, commits, and chains itself (step 5). +5. **Review** — the `review-changes` skill reviews a finished subtask in a fresh subagent context on three + axes (spec compliance, repo standards, correctness). Findings land in the subtask folder's `review.md`, + and `resolve-review` adjudicates each one — fix, or reject with a written reason that an arbitration + round rules on. `implement-task --auto` runs that whole cycle unattended, then commits the subtask and + chains to the next, stopping at a `(human)` acceptance criterion, a `[human]` leaf, a `needs-human` + finding, or an unsettled dispute. In a manual run the review is optional: you either ask for it or read + the diff yourself. +6. **Land** — flip the draft PR to **ready for review** when the index's last box is checked. One feature + branch holds the whole task and lands in `main` as one PR, spec included; it is named `issue-<number>` + (`issue-3219`), which is what lets `implement-task` run with no arguments on a task branch. Once the PR + is ready, run `review-changes` **by hand** for the whole-task pass: per-subtask reviews structurally + cannot see inconsistency between subtasks, duplicated helpers, or dead scaffolding. On a pushed branch it + posts inline PR comments instead of a file, so your own review comments and the agent's sit side by side — + `resolve-review` closes out both, and never rejects a human's. ## Supporting files @@ -128,7 +146,10 @@ one lets `--auto` commit something nobody looked at. PR as the repo gets more agent-friendly, never per task. - `.agents/TEAM.md` — the team roster (members + Slack IDs); the grilling session picks one contact per team for the task and records the picks in the spec header. -- `.agents/skills/to-spec/spec-template.md` — the spec template (used for both main and subtask specs). +- `.agents/skills/to-spec/spec-template.md` — the main spec: header, context, shared facts, and the + subtask index. +- `.agents/skills/to-spec/subtask-template.md` — a subtask spec: what to build, acceptance criteria, + blocking edges, and the leaf worklist. - `.agents/skills/review-changes/` — the reviewer: the three axes, the smell baseline (`smells.md`), the record format (`review-template.md`), and the `gh` surface both review skills share (`gh-commands.md`). - `.agents/skills/resolve-review/SKILL.md` — adjudicating findings and closing them out. From 7f470d41f830aca47bb41818e9e2aca15ef18862 Mon Sep 17 00:00:00 2001 From: tom <tom@ohhhh.me> Date: Tue, 11 Aug 2026 20:40:24 +0200 Subject: [PATCH 2/2] Resolve review findings on the task workflow restructure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The main index checkbox is what `Blocked by:` edges and PR finalization read, so it now moves at commit time rather than in the spec-update step — checking it early let an unverified, uncommitted subtask release its dependents. Three gaps in the handoff paths: `grill-the-task` had no subtask-mode branch, so scoping a deferred subtask matched the single-subtask path and would implement instead of writing the sub-spec; its input front-loading was gated on subtask count rather than on whether a later session executes the work, which skipped every subtask-mode run; and `implement-task` had no route from a `[human]` leaf that ends its subtask into verification and commit, which the default UI split makes the common case. Also drops `ready` from the subtask template — nothing set it, and stored readiness is what the derived model replaced — and trims restatements of the subtask model from the ADR, the subtask template, and AGENTS.md, leaving the definition with `.agents/tasks/README.md` alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --- .agents/AGENTS.md | 5 +-- .../adr/0002-layer-shaped-subtask-leaves.md | 16 +++---- .agents/skills/grill-the-task/SKILL.md | 18 +++++--- .agents/skills/implement-task/SKILL.md | 43 +++++++++++-------- .agents/skills/to-spec/subtask-template.md | 7 ++- 5 files changed, 46 insertions(+), 43 deletions(-) diff --git a/.agents/AGENTS.md b/.agents/AGENTS.md index 02f9a2af792..dbd08f5a020 100644 --- a/.agents/AGENTS.md +++ b/.agents/AGENTS.md @@ -78,9 +78,8 @@ looks wrong without its context. Supersede rather than rewrite — flip the old ## Product task workflow Product tasks (GitHub issues) are worked through a spec-driven workflow — interview, spec, agent -implementation, code review. A task small enough to finish in its grilling session skips the spec -entirely. Specs accumulate in `.agents/tasks/` as a permanent record. See `./tasks/README.md` for the -lifecycle, the skills that run it, and the spec conventions. +implementation, code review. Specs accumulate in `.agents/tasks/` as a permanent record. See +`./tasks/README.md` for the lifecycle, the skills that run it, and the spec conventions. ## Editing this instruction set diff --git a/.agents/adr/0002-layer-shaped-subtask-leaves.md b/.agents/adr/0002-layer-shaped-subtask-leaves.md index 7c3856132a4..48db640190a 100644 --- a/.agents/adr/0002-layer-shaped-subtask-leaves.md +++ b/.agents/adr/0002-layer-shaped-subtask-leaves.md @@ -11,10 +11,6 @@ **A subtask is a vertical slice; the leaves inside it are layer-shaped.** -A subtask cuts a narrow but complete path through every layer it touches and is verifiable on its own. Its -leaves do the opposite — each is one project skill's worth of work in one layer: `add-api-resource`, then -`add-new-page`, then the `[human]` styling. - ``` subtasks/01-cross-chain-list/ ← vertical: demoable, one context window, one commit leaf 1 [agent] add-api-resource — declare the resource @@ -23,8 +19,8 @@ subtasks/01-cross-chain-list/ ← vertical: demoable, one context window, on leaf 4 [human] style to mockup ``` -The full model lives in "The subtask model" in `.agents/tasks/README.md`; this record holds only the -reasoning, which that file should not have to carry. +"The subtask model" in `.agents/tasks/README.md` defines both levels and every rule that follows from them; +this record holds only the reasoning, which that file should not have to carry. ## Why @@ -48,10 +44,10 @@ So the two levels answer two different questions. *What can an agent execute wit ## Consequences -- Leaves are not run boundaries. A run executes a whole subtask; leaf checkboxes exist so a run interrupted - by a `[human]` leaf can resume, since nothing is committed until the subtask finishes. - The review unit is the subtask, so a leaf's code can be wrong for as long as it takes the slice to - finish. Accepted deliberately: reviewing every leaf spent three subagents per step, and most of what it - caught was churn the next leaf rewrote anyway. + finish. Accepted deliberately: reviewing every leaf spent a subagent per axis on every step, and most of + what it caught was churn the next leaf rewrote anyway. +- Leaves stop being run boundaries, which buys the review unit above at the cost of needing a resumption + mechanism inside a subtask that has no commits yet. The workflow layer owns how that works. - Nesting is unnecessary. Work too big for one subtask becomes more subtasks with blocking edges between them, never subtasks inside subtasks — which is what let the sub-branch and sub-PR machinery go. diff --git a/.agents/skills/grill-the-task/SKILL.md b/.agents/skills/grill-the-task/SKILL.md index abfe401d3e5..569ba353686 100644 --- a/.agents/skills/grill-the-task/SKILL.md +++ b/.agents/skills/grill-the-task/SKILL.md @@ -118,13 +118,13 @@ Two things to look for while drafting it: - **Defer what can't be scoped.** A subtask blocked on a prototype, a spike, or an answer nobody has yet gets a `brief.md` and no `spec.md`; a just-in-time subtask-mode session scopes it later. -**Then front-load the executor skills' inputs — only when the breakdown has more than one subtask**, since -that is what makes a later session execute it blind. Go through every `[agent]` leaf that will run a project +**Then front-load the executor skills' inputs, whenever a later session will execute the work** — every +subtask-mode run, and any task-mode breakdown with more than one subtask. That session starts blind, and +these answers are what let it run without stopping. Go through every `[agent]` leaf that will run a project skill (`add-new-page`, `add-api-resource`, `add-env-var`, …): **open that skill and run its user-facing interview now** (e.g. `add-new-page` Step 0), from the skill's current text — don't work from memory of its -questions. The answers are recorded with the subtask in its own `spec.md`, so `implement-task` never stops -to ask. A single-subtask task skips this entirely: this session runs the skill itself, so it can just ask -as it goes. +questions. The answers are recorded with the subtask in its own `spec.md`. Only a task-mode single-subtask +task skips this: this session runs the skill itself, so it can just ask as it goes. ## Step 4 — Send open questions @@ -154,13 +154,17 @@ routes it manually. ## Step 5 — Hand off -**One subtask** — this session finishes the job. Create the feature branch (`issue-<number>` off `main`) +**Subtask mode** — always invoke **`to-spec`**, however small the scoped subtask turns out to be. The work +is being written down for a later `implement-task` run, which is the whole reason the subtask was deferred; +the in-session path below never applies here. + +**Task mode, one subtask** — this session finishes the job. Create the feature branch (`issue-<number>` off `main`) with the developer's approval, implement the work, and hand off to the `create-pr` skill, which writes the reasoning from this conversation into the PR description. No spec is written: nothing is being handed to a session that wasn't in the room. If a `pending` question blocks the work, wait for the reply and pick the implementation back up in this same session — and write a spec instead only if the developer asks for one, which is worth doing when the work will sit before it starts. -**Several subtasks** — invoke the **`to-spec`** skill. It writes the `spec.md` index plus every subtask +**Task mode, several subtasks** — invoke the **`to-spec`** skill. It writes the `spec.md` index plus every subtask folder, records the Slack permalinks from Step 4, and walks the developer through branch, first commit, and the draft PR. diff --git a/.agents/skills/implement-task/SKILL.md b/.agents/skills/implement-task/SKILL.md index 5bb587f4691..d3dde9d8f5d 100644 --- a/.agents/skills/implement-task/SKILL.md +++ b/.agents/skills/implement-task/SKILL.md @@ -71,11 +71,14 @@ it in the header. A run can arrive mid-subtask, because a `[human]` leaf inside one stops the chain. Before anything else: - **An unchecked `[human]` leaf in a subtask already in progress** — ask the developer whether it's done. - Check it off if so; stop if it's still in progress. Then resume that subtask at its next unchecked leaf - rather than picking a new one. -- **A finished subtask left uncommitted for verification** — ask whether the `(human)` acceptance criteria - passed. If they did, the developer commits before this run starts new work; if they didn't, fix what - failed inside that subtask instead of moving on. + Check it off if so; stop if it's still in progress. Then continue that subtask rather than picking a new + one: at its next unchecked leaf, or — when that leaf was the subtask's **last**, which the default UI + split makes the common case — at **Step 5**, so the finished subtask still gets its verification, review, + and commit. +- **A subtask left uncommitted for verification** — every leaf checked, its index box not. Ask whether the + `(human)` acceptance criteria passed. If they did, the developer commits, and this run then checks the + index box and sets that subtask's `Status` to `done` before starting anything new; if they didn't, fix + what failed inside that subtask instead of moving on. ### Step 3 — Pick the next subtask @@ -160,7 +163,8 @@ Nits never gate: `deferred` findings leave the `Outcome` `clear`. ### Step 8 — Update the spec -Check the subtask's box in the main index, with a **one-line** note (files/skills involved) — never a +Record the work in the subtask's **own** spec: its leaf boxes checked, its header `Status` set to +`in progress`, and a **one-line** note (files/skills involved) — never a multi-line changelog; git and the PR carry the detail, and any durable decision (a new dependency, an architectural choice) is folded into the relevant spec section instead. A finding worth keeping — a gotcha, a bug you hit — goes to the subtask folder's `notes.md` as evidence the PR can quote, or graduates to a @@ -169,15 +173,14 @@ report. If the work uncovers a bug from an **earlier, finished** task, fix it he task's PR — never reopen or edit that task's frozen spec. See "What a spec holds" in `.agents/tasks/README.md`. -Keep both levels in sync: +**The main index waits for the commit.** A checked index box means *landed*: `Blocked by:` edges read it to +release dependents, and the last box checked is what finalizes the PR. Checking it here — before Step 9's +`(human)` gate and its commit — would let an unverified, uncommitted subtask release the work that depends +on it. So the index box and the subtask's `done` Status are set **at commit time**: by Step 9 under +`--auto`, and by the next run's Step 2 when the developer commits a verified subtask themselves. -- Set the subtask spec's header `Status` to `in progress` on its first leaf and `done` when its last box is - checked, then check its line in the **main index**. -- Set the **main** header `Status` to `in progress` on the first executed subtask and `done` when the - index's last box is checked. - -The main index is what "done" and draft-PR finalization key off, so never leave it trailing a completed -subtask spec. +Set the **main** header `Status` to `in progress` on the first executed subtask, and `done` once the index's +last box is checked. ### Step 9 — Commit and chain, or hand off @@ -185,16 +188,18 @@ subtask spec. `implement-task` invocation belong to the developer. **Under `--auto`**, when the `Outcome` is `clear` and the subtask has **no `(human)` acceptance criterion**: -commit it on the task branch — one commit for the whole subtask with the review fixes folded in, a plain -descriptive subject, and the repo's `Co-Authored-By` trailer — then return to **Step 3** for the next -subtask. Keep the issue out of the commit: no `#<issue>` or issue URL in the subject or body. GitHub adds a +check its box in the main index, set its `Status` to `done`, and commit it on the task branch — one commit +for the whole subtask with the review fixes and the spec updates folded in, a plain descriptive subject, and +the repo's `Co-Authored-By` trailer — then return to **Step 3** for the next subtask. Keep the issue out of +the commit: no `#<issue>` or issue URL in the subject or body. GitHub adds a timeline reference to the issue on every push that names it, so a chain of commits spams it; the PR's `Resolves #N` already carries the link and closes the issue on merge. Stop the chain, pushing nothing, on any of: -- the subtask has a `(human)` acceptance criterion → leave it **uncommitted**; the developer checks it - against the running product per the spec's "How to verify" line, then commits what they verified +- the subtask has a `(human)` acceptance criterion → leave it **uncommitted, its index box unchecked**; the + developer checks it against the running product per the spec's "How to verify" line, then commits what + they verified, and the next run's Step 2 checks the box - the next leaf inside this subtask is `[human]` - the next subtask is blocked, or has only a `brief.md` - a `pending` question blocks every remaining subtask diff --git a/.agents/skills/to-spec/subtask-template.md b/.agents/skills/to-spec/subtask-template.md index b9dd92c8e02..8e641b27294 100644 --- a/.agents/skills/to-spec/subtask-template.md +++ b/.agents/skills/to-spec/subtask-template.md @@ -3,7 +3,7 @@ | | | | --- | --- | | Parent spec | `.agents/tasks/<dir>/spec.md` → link it as `../../spec.md`, subtask <NN> of #<issue> | -| Status | `draft` \| `ready` \| `in progress` \| `done` | +| Status | `draft` \| `in progress` \| `done` | | Blocked by | <subtask numbers that must be checked first, or "none"> | <!-- People rows are inherited from the parent spec; add one here only to override it. A subtask that @@ -13,9 +13,8 @@ hasn't been scoped yet has NO `spec.md` at all — only a `brief.md` in its fold ## What to build <!-- The end-to-end behaviour this subtask makes work, from the user's perspective — a paragraph, not a -layer-by-layer plan. A subtask is a vertical slice: it cuts a narrow but complete path through every layer -it touches and is verifiable on its own. If it does not fit in one fresh context window, it is two -subtasks. --> +layer-by-layer plan. What makes a well-formed subtask, and the bounds it has to satisfy: "The subtask +model" in `.agents/tasks/README.md`. --> ## Acceptance criteria