Smartgift-AI/AgentHub
14
0
Fork 0
Code Issues 3 Pull Requests Actions Packages Projects Releases 89 Wiki Activity

feat(skills,pipelines): ask the user at the two points being wrong is cheap, and stop paying for reviews the diff does not need

Browse Source 
The `default` 8-node pipeline walked past its own intake: the node declared
`agenthub stage intake`, a stage report with no work attached, so "intake" was
a label the run passed through without ever asking a question. It now declares
`/superpowers:brainstorming` (and detects it — that rule moves off `plan`),
and both driving skills make the brainstorm mandatory, asked with
AskUserQuestion and waited on rather than answered on the user's behalf.

Three stops replace three silent slides:

- After the plan is written and adversarially challenged, it goes to the USER
  for approval. Implementation does not start without it, and work that turns
  out to need a different approach comes back for a fresh one.
- Review picks its reviewers from `git diff --name-only`: Claude Code's own
  `code-review` always, a domain reviewer only for a layer the diff actually
  touched, and — new — only once the change is big enough to have seams. Three
  agents re-reading twenty lines buys one opinion at three times the tokens, on
  every fix-loop lap. Review is also a loop now: it ends on a clean pass over
  the current diff, not on one pass having happened.
- The gate asks the user to accept before anything is claimed done (and in the
  lane pipeline, before the PR is published — publishing is outward-facing).
  A NO-GO re-enters by SIZE: a defect goes back to implement, a wrong approach
  back to plan. Nothing carries a NO-GO forward with a note about it.

Also here, from a real stall seen on a lane: `ship-feature` had no waiting rule
at all, and a session at the gate invented one with `ScheduleWakeup` — a
/loop-only primitive that paces nothing here and is refused outright when
`auto` mode cannot reach its safety classifier. The rule the lane skill already
had now exists in both.

Cost, measured in what each agent is actually for: the suite run leaves its
agent (nothing is judged, so nothing is delegated — an agent there buys a
second model's tokens to read output the session can read for free), and
`backend-reviewer` drops from opus to sonnet like its frontend twin; both read
a scoped diff against a fixed checklist, and the senior gate above them is
still opus.

Skill bugs found in the same sweep: doc-map told agents not to recreate the
removed translations and then, three lines later, to mirror every README change
into them; the docs mapping still carried `+VN+CN` and the punctuation left by
their removal; Procedure jumped 2 → 6; Verify required `scripts/doc-coverage.sh`,
which does not exist in this repo.
This commit is contained in:
nntrivi2001
2026-09-18 10:21:03 +07:00
parent 4227a97ab1
commit 26b0fe53c7
13 changed files with 180 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files
skills/ship-feature/SKILL.md
+120 -42
View File
@@ -1,6 +1,6 @@
---
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.
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
@@ -37,17 +37,47 @@ 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.
## Phases
### intake — understand before touching anything
### 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.
- Requirement fuzzy or open to more than one reading? **superpowers:brainstorming**.
- **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 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.
### plan — a written plan, challenged before it is code
### 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.
@@ -59,30 +89,44 @@ you dropped back to.
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.
### 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.
### tests — this repo's verification, not a claim
- `agenthub stage tests --status running`
- **Dispatch, don't self-grade.** Both halves below run in a **fresh-context
agent** (Agent tool, `subagent_type: general-purpose` — FOREGROUND, and
**never `fork`**: a fork inherits your context, which is the bias you are
trying to escape). Hand it only what it can verify itself — repo path, the
spec/requirement, `git diff` range, the commands — never your own conclusions.
- **Independent unit tests.** 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.** Backend changed → `npm run test:server`. Frontend →
`npm run test:client`.
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).
- **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** (Agent tool,
`subagent_type: general-purpose` — FOREGROUND, and **never `fork`**: a fork
inherits your context, which is the bias you are trying to escape). Hand it
only what it can verify itself — repo path, the spec/requirement, `git diff`
range, the commands — never your own conclusions. 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.** Backend changed → `npm run test:server`.
Frontend → `npm run test:client`. 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.
@@ -90,32 +134,47 @@ you dropped back to.
`--result fail` with what failed. A step you could not run is reported as not
run, never as passed.
### review — a real review pass, not a re-read
### review — the native review, plus the domains this diff actually touched
- `agenthub stage review --status running`
- Hand the review to a **fresh-context agent** (`subagent_type: Code Reviewer`,
FOREGROUND, never `fork`) — a reviewer that watched you write the code is not
a reviewer. It runs the **code-review** skill on the working diff
(see also **superpowers:requesting-code-review** for what to hand over).
Final line: `REVIEW: CLEAN` or `REVIEW: FINDINGS — <n>`.
- **Domain reviewers, in parallel with the above (same message, separate
Agent calls).** Check `git diff --name-only` (against the base this task
started from) for touched layers and launch every matching one —
FOREGROUND, fresh context, never `fork`, addressed as `agenthub:backend-reviewer`
/ `agenthub:frontend-reviewer`:
- Any path under `server/` → `agenthub:backend-reviewer`.
- Any path under `client/` → `agenthub:frontend-reviewer`.
- Neither touched → skip both; the generic Code Reviewer
above is the only gate.
- **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** (the official plugin's, run
as `/code-review` on the working diff). It is not optional and is not replaced
by an agent reviewing "in its own words" — run it in a **fresh-context agent**
(`subagent_type: Code Reviewer`, FOREGROUND, never `fork`; a reviewer that
watched you write the code is not a reviewer), whose job is to invoke that
skill and report its findings. 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**, launched in parallel
with the above (same message, separate Agent calls) — FOREGROUND, fresh
context, never `fork`:
- 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 — the generic review plus every domain
reviewer that ran — with **superpowers:receiving-code-review** judgment:
verify each point, neither blind agreement nor blind rejection.
- Changed code in response? Re-run the `tests` phase.
`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.
### gate — evidence before the completion claim
### 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.
- For a change of any real size, the gate is a **fresh-context agent** too
@@ -124,7 +183,26 @@ you dropped back to.
re-run and re-check them. Final line: `GATE: GO` or `GATE: NO-GO — <what is
missing>`. A one-line fix does not need it; anything you would defend in a
review does.
- `agenthub stage gate --evidence "<what was verified>"`
- **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>"`
### ship — docs, then the user's call
- `agenthub stage ship`