Files
AgentHub/skills/ship-feature/SKILL.md
T
nntrivi2001 eab7c437df feat(skills,pipelines): make the review node review, and make a long run followable and affordable
Closes #24.

**The review node ran nothing.** Both templates declared
`/superpowers:requesting-code-review` — the skill about how to HAND OVER a
review, not one that reviews anything — so `agenthub lanes node review`
performed no review at all. They now declare the bare `/code-review`, Claude
Code's own. It carries no `plugin:` prefix, so `requiredSkillPlugins` skips it
and the doctor's required-plugin row is unchanged (verified against the live
function: still exactly the five superpowers skills the templates name).

**The rest is the brainstorm → implement → review span, made cheaper to run and
harder to lose track of.** `ship-feature` gains four sections it never had:

- **Right-size the run.** The pipeline is not free — every dispatching phase
  costs a model call and minutes, and the fix-loop pays it again each lap. A
  typo does not enter the pipeline at all; a small single-layer change skips the
  plan-debate, independent-test and gate agents and lets the user's acceptance
  be the gate; medium/large runs everything. Sizing is a claim made out loud,
  and a task that grows re-sizes upward.
- **Dispatching a judgement.** The fresh-context/FOREGROUND/never-`fork`/
  hand-only-verifiable-artifacts/one-parseable-line contract, written once
  instead of re-argued in three phases — plus the rule that was missing
  entirely: never dispatch what a command answers. An agent is for judgement,
  not for typing.
- **Resuming after a compaction.** Long runs outlive the context window; the
  lane skill had this and this one did not. Re-derive position from the declared
  stage and the diff, and treat a phase whose evidence you cannot find as one
  that did not happen.
- **A `Leave when:` line on every phase.** Objective exit criteria are what stop
  both the silent skip and the endless polish: intake leaves when the answers
  are in hand, review leaves when the latest pass over the CURRENT diff is
  clean, the gate leaves when verification really ran AND the user said yes.

And, narrower: intake asks only what the repo cannot answer (a question the code
could have answered spends the user's round and earns a worse answer);
implement works the approved plan task by task, re-checks each acceptance
criterion, heartbeats on long work, and treats a plan that turns out wrong as a
return to `plan` rather than a quiet substitution; tests runs the suite the diff
needs rather than both by reflex.
2026-09-18 10:39:01 +07:00

322 lines
19 KiB
Markdown

---
name: ship-feature
description: Implement a feature safely end-to-end in this repository. Use when adding or changing functionality across backend, frontend, or MCP with required verification and documentation updates. Drives the Superpowers workflow skills and declares each phase with `agenthub stage` so the dashboard's pipeline map follows along. Brainstorms WITH the user at intake, stops for their approval of the plan before any code, reviews with Claude Code's own `code-review` plus only the domain reviewers the diff actually needs, and stops again at the gate for their acceptance — a NO-GO loops back to implement or plan rather than shipping.
---
# Ship Feature
Use this workflow for medium or large implementation tasks. It does not restate
how to plan, test, or review — the Superpowers skills own that. What lives here
is the phase order, this repository's own rules, and the stage declaration at
each boundary.
For a feature inside a AgentHub **lane**, use `ship-feature-lane` instead: it adds
the branch/e2e/QC/senior-gate/PR half this skill deliberately leaves out.
## Declaring the stage
Each phase below starts with `agenthub stage <node>`, which is what puts the phase
on the lane's pipeline map. The nodes are the `default` template's:
`intake → plan → implement → tests → review → gate → ship → done`.
Set the template yourself, don't rely on how the lane was created: before your
first `agenthub stage` call, run `agenthub lanes pipeline default` (no id needed —
resolves from `cwd`; a no-op if the lane is already on it). This is what makes
the choice invisible to the human — whichever of `ship-feature` or
`ship-feature-lane` actually runs is what decides the template, not a picker
they have to get right in advance.
`agenthub stage` needs a lane owning the current directory. If it reports no lane,
this repo was never adopted (`agenthub lanes add --cwd $(pwd)` fixes it) — carry on
with the workflow and skip the stage calls; they are reporting, not control
flow. Never skip a phase because its stage call failed.
Declaring beats detection. The dashboard also *infers* these stages from the
Superpowers skill invocations below, but an inference never renders `done` and
never overrides a declaration — and detection cannot move a lane BACKWARD past
a declared stage, so a rework loop is invisible unless you re-declare the phase
you dropped back to.
## Waiting for an agent or a long command
A phase that dispatches a foreground agent (tests, review, gate) or runs a long
command is **already** wired to wake you: you are re-invoked the moment a
backgrounded Agent or helper finishes. So:
- Do not poll for it, and do not invent a timer. **`ScheduleWakeup` is a
`/loop`-only primitive** — this pipeline is not a `/loop` session, so it
neither paces nor sustains anything here, and in `auto` permission mode it can
simply be refused (the safety classifier is a model call, which fails when
that model is briefly unavailable). Never let a phase's progress depend on it.
- Need to pace a wait anyway (a CI run, an external queue)? Background the wait:
`sleep <secs>` with `run_in_background: true`.
- Never narrate a wait you are not actually running. If you cannot sustain one,
say so plainly and stop.
## The shape of a run
Forward: `intake → plan → implement → tests → review → gate → ship → done`.
Two back-edges, and they are the point of the pipeline, not its failure mode:
- **`tests`/`review` → `implement`** — a failure or a finding sends you to the
code, then forward through `tests` and `review` again.
- **`gate` → `plan`** — a NO-GO about the APPROACH (wrong scope, wrong shape,
the requirement meant something else) replans and re-earns the user's
approval. A NO-GO about a DEFECT goes to `implement` instead.
Going back is a declaration: re-run `agenthub stage <node>` for the node you
dropped to. Detection cannot move a lane backward on its own, so a silent
rework leaves the dashboard showing a stage you already left, and the nodes
ahead keep the previous pass's evidence.
## Right-size the run before you start it
This pipeline is not free: every phase that dispatches an agent costs a model
call and minutes of wall clock, and the fix-loop pays it again on each lap.
Match its weight to the work, and say in one line which weight you picked.
| Work | What runs |
|---|---|
| **Trivial** — typo, rename, one-line fix, a question | Not this skill. Do it, run the touched suite, say what you did. |
| **Small** — one layer, a few files, no new interface | `intake` questions → `implement` → `tests` → `review` with `code-review` alone → `gate` (the user's acceptance IS the gate). Skip the plan-debate agent, the independent-test agent and the gate agent. |
| **Medium / large** — multi-file, new route/schema/interface, cross-layer, or anything you would defend in a review | Every phase below, in full. |
Sizing is a claim you make out loud, not a shortcut you take quietly. When
torn, take the heavier one. A small task that grows re-sizes upward mid-run —
re-declare the phase you re-enter and pick up the skipped agents from there.
## Dispatching a judgement (used by `tests`, `review`, `gate`)
One contract, written once, so the phases below don't each re-argue it:
- **Fresh context, FOREGROUND, never `fork`.** A fork inherits your context —
precisely the bias the dispatch exists to escape. Foreground because the phase
gates on the answer, and the agent's completion is what re-invokes you.
- **Hand only what the agent can verify for itself**: the repo path, the
requirement/plan, the `git diff` range, the commands to run. Never your
conclusions — "I already checked X", "this is safe because…" — those are the
very claims it is there to test.
- **One parseable final line** is the verdict (`UNIT-TESTS:`, `REVIEW:`,
`BACKEND-REVIEW:`, `FRONTEND-REVIEW:`, `GATE:`). Read that line; never infer a
pass from prose around it.
- **Never dispatch what a command answers.** Running a suite, reading a log,
grepping a path — do it yourself. An agent is for judgement, not for typing.
- Agents for the same phase go out in **one message** so they run concurrently.
## Resuming after a compaction
A long run outlives the context window. Re-derive your position, don't guess it:
- `agenthub lanes` — the lane's declared stage, status and notes.
- `git status` + `git diff --stat <base>...` — what is actually built.
- The plan file — what was actually approved.
Resume at the declared stage and re-run it from the top; every `agenthub`
command is idempotent. **A phase whose evidence you cannot find did not
happen** — re-run it rather than assuming a past self did.
## Phases
### intake — brainstorm WITH the user, always
- `agenthub stage intake --status running`
- Restate the requirement and its success criteria.
- Explore the impacted modules: `Explore` agent for breadth, then read the key
files yourself. Identify which layers are hit (server / client / docs /
scripts) — `repo-onboarding` if the area is unfamiliar.
- **superpowers:brainstorming is MANDATORY here, not a fallback for a fuzzy
requirement.** Run it before any plan exists. Skipping it because the ask
"seems clear" is the single most common way this pipeline builds the wrong
thing: a requirement typed in one line always leaves scope, edge cases and
acceptance criteria unstated.
- **Ask only what the repo cannot answer.** Read first, ask second: a question
whose answer is in the code, the tests, `CLAUDE.md` or the existing UI costs
the user's time and earns you a worse answer than reading would. What belongs
to the user is intent — scope, priorities, trade-offs, what "done" means —
never facts you could have looked up.
- **Ask the user, and wait for the answer.** Put the open questions to them with
`AskUserQuestion` — batched in one round (up to 4 at a time), each with real
options, exactly the way plan mode asks. Questions that change what you build:
scope boundaries, which surfaces are in/out, behavior on the edge cases you
found, what "done" means. You may not answer them on the user's behalf, and
you may not move to `plan` until they are answered. Answering yourself and
proceeding IS the bug this phase exists to prevent.
- The one exception: the user explicitly said to skip the questions and decide
yourself ("cứ làm đi", "no questions", "you decide"). Then state the
assumptions you are proceeding under, in writing, and go on.
- **Leave when:** the user's answers are in hand (or their explicit "decide
yourself" is), and you can state the scope, the non-goals and what "done"
means in your own words without a guess in any of them.
### plan — a written plan, challenged, then APPROVED by the user
- `agenthub stage plan`
- **superpowers:writing-plans**: approach, files to change, the test strategy
(which behavior each test pins), and how each success criterion is met.
- If the task is a bug rather than a feature: **superpowers:systematic-debugging**
first. Root cause, not symptom — and grep every caller of the function you
are about to change, not just the path the report names.
- Have the plan **challenged by a fresh-context agent** (`Plan` or
`general-purpose`, never `fork`): missed requirements, wrong assumptions, a
simpler approach, unhandled edge cases. Give it the requirement and the plan,
not your reasons for believing the plan. Apply what survives
**superpowers:receiving-code-review** scrutiny.
- **STOP. The plan is presented to the user and implementation does not start
until they approve it in this turn.** Show the plan (or its path plus a short
summary: what changes, which files, what is deliberately out of scope, how it
will be verified) and ask with `AskUserQuestion` — approve / revise / change
approach. "Revise" means edit the plan and ask again; the loop is the point.
Writing a plan and immediately implementing it is exactly the behavior this
phase forbids, however obvious the plan looks to you.
- Approval covers the plan as presented. Work that turns out to need a
materially different approach comes BACK here (`agenthub stage plan` again)
for a fresh approval, rather than drifting under the old one.
- **Leave when:** a written plan exists, it survived the challenge, and the user
approved THIS version of it in this turn.
### implement — smallest coherent diff
- `agenthub stage implementing`
- Entered only from an **approved** plan (or from a gate NO-GO sending work
back here). No approval in this turn → you are still in `plan`.
- **superpowers:test-driven-development**: failing test → minimal code → green.
- Smallest diff that satisfies the requirement. Every changed line traces to it.
- **file-headers** applies to every source file you create or edit.
- Work the approved plan task by task, in its order. Before leaving this phase,
re-read the plan's acceptance criteria and check each one against what you
actually built — that list, not your memory of it, is what `gate` will ask
about.
- **The plan turning out wrong is a result, not a detour.** If implementing
shows the approach cannot work, or the requirement means something else, stop
and go back to `plan` (re-declare it, present the new plan, get it approved
again). Quietly building a different thing than the one approved is the
failure this phase has to avoid.
- Long implementation? Re-declare `agenthub stage implementing` as you go — it
doubles as the heartbeat, so the dashboard doesn't read a working lane as
stalled.
- **Leave when:** every plan task is done, every acceptance criterion has
something concrete behind it, and the diff contains nothing that traces to
neither.
### tests — this repo's verification, not a claim
- `agenthub stage tests --status running`
- **Dispatch the judgement, run the commands yourself.** Only work that needs an
unbiased pair of eyes is worth an agent; a deterministic command is not.
- **Independent unit tests → a fresh-context agent** (`general-purpose`; see
*Dispatching a judgement*). It reads the requirement
and the diff cold and writes the tests the implementation is *missing* —
happy, negative and boundary, in the repo's existing style. It may not touch
non-test files nor weaken existing tests. Final line:
`UNIT-TESTS: PASS — <n> added` or `UNIT-TESTS: FAIL — <test> :: <defect>`.
- **Suite run → plain Bash, here.** Run the suite(s) the diff actually needs:
backend changed → `npm run test:server`; frontend changed →
`npm run test:client`; both → both. A docs-only change needs neither, and
running the other layer's suite proves nothing about this diff. Nothing is judged, so nothing is
delegated: an agent for this buys a second model's tokens to read output you
can read for free. A UI snapshot diff is reported for review, **never
blindly regenerated** (`cd client && npx vitest run -u` is yours to run,
only after you read the diff).
- `FAIL` → fix the **source** (root cause), not the agent's test — then re-run
this phase. Rewriting a failing independent test to pass is the one move that
makes this whole phase worthless.
- Record the outcome: `agenthub stage tests --evidence "<what passed>"`, or
`--result fail` with what failed. A step you could not run is reported as not
run, never as passed.
- **Leave when:** the suites the diff needs are green in output you actually
read, and the independent-test agent (when the size called for one) returned
`PASS`.
### review — the native review, plus the domains this diff actually touched
- `agenthub stage review --status running`
- **Read the diff first, then decide who reviews it.** `git diff --name-only`
against the base this task started from is what picks the reviewers. Spawning
a fixed set regardless of what changed wastes a pass on files nobody touched,
and worse, reads as coverage that never happened.
- **Scale the pass to the diff.** A handful of lines in one layer gets the
`code-review` pass alone — three agents re-reading a 20-line change costs
three times the tokens for one opinion. Add the domain reviewer(s) once the
change is big enough to have seams: multiple files in a layer, a route or
schema or hook touched, or anything you would want a second reader on.
- **Always: Claude Code's own `code-review` skill** (`/code-review`, on the
working diff — it is also what the `review` node declares). Not optional, and
not replaced by an agent reviewing "in its own words": run it in a
fresh-context `Code Reviewer` agent whose job is to invoke that skill and
report what it found (a reviewer that watched you write the code is not a
reviewer). See **superpowers:requesting-code-review** for what to hand over.
Final line: `REVIEW: CLEAN` or `REVIEW: FINDINGS — <n>`.
- **Domain reviewers, conditional on the touched layers**, in the same message
as the above so they run concurrently:
- Any path under `server/` (routes, lib, hooks, db, CLI-facing code) →
`agenthub:backend-reviewer`. No backend path changed → do not launch it.
- Any path under `client/` (components, pages, i18n, styles) →
`agenthub:frontend-reviewer`. No frontend path changed → do not launch it.
- Both touched → both, in the same message so they run concurrently.
- Neither touched (docs-only, scripts-only, config-only) → neither; the
`code-review` pass above is the whole review.
- Hand each ONLY the repo path and the working diff — each scopes itself to
its own domain's files. Each ends with its own parseable line:
`BACKEND-REVIEW:` / `FRONTEND-REVIEW:` — `CLEAN` or `FINDINGS — <n>`.
- Apply what is worth applying — `code-review` plus every domain reviewer that
ran — with **superpowers:receiving-code-review** judgment: verify each point,
neither blind agreement nor blind rejection. A point you reject is answered
with a reason, not silence.
- **Changed code in response? Re-run `tests`, then re-run THIS phase.** Review
is a loop, not a checkpoint: it ends when a pass over the current diff comes
back clean (or with only findings you have explicitly and defensibly
rejected), not when one pass has been performed.
- Record it: `agenthub stage review --evidence "<n> findings across <who ran>,
resolved"`.
- **Leave when:** the latest pass over the CURRENT diff is clean or
argued-through — not when a pass has merely happened.
### gate — evidence, then the user's acceptance
- **superpowers:verification-before-completion**. Commands actually run, output
actually read. This is the phase that stops "should work" from shipping.
- At medium/large size the gate is a **fresh-context agent** too
(`general-purpose`): hand it the requirement, the diff range and the
verification commands — not your evidence — and let it re-run and re-check
them. Final line: `GATE: GO` or `GATE: NO-GO — <what is missing>`. At small
size skip it; the user's acceptance below is the gate.
- **The gate is not yours to wave through. Ask the user.** Present, in a few
lines: what was built, what was verified and how (the commands and their real
outcome), what review found and what you did about it, and anything you could
not run. Then ask with `AskUserQuestion`: accept / send back for fixes /
rethink the approach. Silence is not acceptance, and neither is `GATE: GO` —
the agent gate checks the evidence, the user decides whether this is what they
wanted.
- **NO-GO, or the user does not accept → go BACK, do not proceed.** Pick the
node by the size of what is wrong, and re-declare it so the dashboard follows
the rework (a declaration on an earlier node clears the nodes ahead of it —
that is what makes a loop visible instead of leaving stale `done` evidence):
- A defect, a missed case, a wrong detail → `agenthub stage implementing`, fix
at the root cause, then forward through `tests` and `review` again.
- Wrong approach, wrong scope, a requirement that turns out to mean something
else → `agenthub stage plan`, replan, and get the plan approved again (the
`plan` phase's approval step applies in full, every time through).
- Either way the phases after it run again. Never carry a NO-GO forward into
`ship` with a note about it.
- Only once the user has accepted: `agenthub stage gate --evidence "<what was
verified, and that the user accepted it>"`
- **Leave when:** the verification really ran, its output was really read, and
the user said yes. Two of three is a NO-GO.
### ship — docs, then the user's call
- `agenthub stage ship`
- **update-project-docs** — mandatory for any change to behavior, config,
interfaces, events, schema, CLI commands, or features. Not optional, not
deferred, not "if asked".
- Commit / push / PR **only when the user asks in this turn**. Finishing an
implementation is not authorization to commit.
- **Leave when:** the docs that the change made wrong are right again, and you
have reported plainly what shipped, what was verified, and what you skipped.
- `agenthub stage done --status passed --evidence "<what shipped>"`
## This repository's own rules
Not restated here. `CLAUDE.md` is loaded in every session and already binds
them — backward-compatible API/WebSocket contracts, fail-safe hooks,
migration-safe schema changes, the destructive-lane and git-argv guards, and
the lane boundaries (the console never writes a stage; the runtime never writes
`stage`/`status`/`notes`). A second copy here would only drift out of sync with
the first. Read `CLAUDE.md` and `.claude/rules/` for the area you are touching.
## References
- Checklist template: `references/feature-checklist.md`