add-feature

Files (5)

• SKILL.md
• code-review-checklist.md · references/
• performance-review-checklist.md · references/
• security-review-checklist.md · references/
• subagent-playbook.md · references/

SKILL.md (first lines)

> **Tool mapping (OpenCode):** This skill uses:
> - `question` tool for user prompts (not `AskUserQuestion`)
> - `task` tool for subagents (not `Agent` or `Task` subagent syntax)
> - `skill({ name: "..." })` to load other skills (not `Skill(skill="...", args="...")`)
> - `codegraph_explore`, `codegraph_search`, `codegraph_context` for codebase exploration (preferred)
> - `todowrite` for phase/progress tracking
> - `grep`, `glob`, `read`, `bash` as fallback when `.codegraph/` not initialized
>
> **Context rule:** This skill supersedes any prior skill instructions. Follow ONLY these instructions now. Read the user's goal and any mode/include/skip from the conversation context.

Read `mode=`, `include=`, and `skip=` from the conversation context. Default to the mode specified in this skill if not provided.


# Feature Delivery

Drive a non-trivial feature from request to merged code without the four failures this skill exists to prevent: **shallow clarification, unreviewed code, bad code merged, duplication.**

The workflow is gated. Do not skip the approval checkpoint. Do not skip the review phase. Reviews themselves are gated — run only the ones the diff actually requires.

---

## Modes

This skill accepts a `mode=` argument that controls depth of execution. Default — when no `mode=` is specified — is `production`: the full 8-phase pipeline below.

| Mode | Phases that run | Phases skipped |
|---|---|---|
| `production` (default) | 1, 2, 3, 4, 5, 6, 7a, 7b, 7c-7h (gated), 8, post-steps | none — full pipeline |
| `balanced` | 1, 2, 3, 5, 6, 7a, 7b, 7e (gated), 7f (gated), 7g (gated), 8 (conditional), post-steps | 4 (Plan-gate), 7c (Security), 7d (Perf), 7h (Data Integrity) unless explicitly included |
| `fast` | 5, 6 | 1, 2, 3, 4, 7a-7h, 8, post-steps |

**`include=` / `skip=` overrides.** Add or remove specific phases on top of the mode default — `mode=fast include=8` runs implement + verify + tests; `mode=production skip=7c` skips the security review even when the gate would have triggered. `include=` wins over the mode default; `skip=` wins over both.

**Conditional tests in balanced mode.** Phase 8 runs in `balanced` whenever the diff changes backend logic, data transformations, permissions, contracts, persisted data, non-trivial branching, async behavior, or bug-prone business rules. Skip Phase 8 in `balanced` only for trivial UI wiring, one-line wrappers, pure config, copy/styling-only changes, or projects where the user explicitly says "no tests."

**Logic-first lane.** If Phase 3 identifies a deterministic parser, validator, pricing rule, permission rule, data transform, state machine, or API/data contract, write the expected behavior test before Phase 5 implementation. The test should fail against the current behavior or encode the new contract before the production code lands. This lane is mandatory in `production`, conditional in `balanced`, and opt-in via `include=logic-first` in `fast`.

**Integration-first lane.** If the feature touches HTTP/webhook dispatch, queues, jobs, cron, IPC, MCP tool calls, file writes, env-var injection, spawned processes, external APIs, email/SMS/push, imports/exports, or cache invalidation, plan the runtime contract and observation point in Phase 3. Dispatch the **`runtime-contract-tracer`** subagent via `task({ subagent_type: "runtime-contract-tracer", description: "Trace runtime contract for integration", prompt: "Trace the trigger → dispatch → receive → observe flow for the integration <name>. Return file:line refs and flag silent-failure sites." })` with the integration name to get the trigger → dispatch → receive → observe trace with file:line refs and silent-failure sites flagged — this becomes the input to Phase 3's planning. After Phase 6, invoke `skill({ name: "add-observability" })` unless the project already has equivalent structured evidence and you can name where it lives.

**Mode safety override.** If `mode=fast` is requested for work that touches auth, permissions, payments, secrets, schema migrations, destructive deletes, background jobs, queues, cron, external APIs, email/SMS/push, imports/exports, file writes, spawned processes, IPC, caching/invalidation, feature flags, analytics/business reporting, concurrency-sensitive mutations, or external webhooks, pause and surface the conflict via `question` tool: *"Detected high-risk signals. You requested fast mode — that skips clarify, plan, reviews, and tests. Confirm fast anyway, or upgrade to production?"* The `/ship` orchestrator enforces this upstream, but direct manual callers may not — don't silently honor a dangerous override.

**Phase-gated NEVER scope.** Several rules in the NEVER section below are phase-gated. When the active mode skips a phase, the corresponding NEVER is explicitly suspended for that run:

- `mode=fast` suspends: *"NEVER write feature code before Phase 4 approval"*, *"NEVER skip clarifying questions"*, *"NEVER skip the duplication scan"*, *"NEVER ship a new field to only one CRUD surface"*, *"NEVER ship a new UI instance without sibling audit"*, and all adjunct-skill routing unless explicitly included
- `mode=balanced` suspends: *"NEVER write feature code before Phase 4 approval"*

The universal NEVERs stay in force in every mode: no fan-out for tightly coupled work, declare-done only after running the code, no irrelevant security/perf review, no `--no-verify` bypass.

---

## Phase 1 — Clarify

Use `todowrite` to mark this phase as `in_progress`. Mark it `completed` when done.

Most failures originate here. The user's first message is almost never enough.

Before asking anything, write down what you currently understand and what you don't:

```
## Understanding so far
**Goal:** ...
**User-visible behavior:** ...
**Touchpoints (best guess):** ...
**Open questions:** ...
**Impact on existing system:** ...
```

Then ask clarifying questions covering — at minimum — these categories. Skip a category only if the answer is genuinely unambiguous from context, and say which you're skipping and why:

1. **Scope** — what's in, what's explicitly out
2. **User experience** — exact UX, error states, empty states, loading
3. **Data** — first decide *whether* to persist before *what schema*. Default to derive-on-read for cheap/small/transient computation; default to persist when any of these triggers hit: per-request scan is expensive (filesystem walks, JSONL/log parsing, external API aggregation), the same inputs are queried repeatedly with mostly-unchanged results, results aggregate across sessions/runs, the source is slow or rate-limited, or the feature needs historical queries the source itself may not preserve (rotation, compaction, deletion). Once persist is chosen, then: new tables/columns, new fields on existing rows, retention, backfill.
4. **API surface** — new endpoints, contract changes to existing ones, auth requirements
5. **Integration points** — what existing features does this touch, change, or risk breaking
6. **CRUD/input surfaces** — enumerate every place a user creates, edits, configures, imports, or duplicates the affected artifact (create dialog AND edit dialog, settings page, bulk import, CSV upload, API client). Dispatch the **`crud-surface-mapper`** subagent via `task({ subagent_type: "crud-surface-mapper", description: "Map CRUD surfaces for artifact", prompt: "Map every create, edit, configure, import, and duplicate surface for the artifact '<name>'. Return a complete inventory with file:line refs." })` with the artifact name to get a complete surface inventory with file:line refs — keeps the search out of the parent's context. For each surface, decide explicitly whether the new field/behavior applies — silently shipping to only one is the most common "incomplete" failure. This is the inward complement to #5 (which looks outward at unrelated features that might break).
7. **Edge cases** — concurrent writes, partial failures, large inputs, permissions
8. **Non-goals** — adjacent things the user does *not* want done now
9. **Done criteria** — how the user will recognize this is finished

**Always use the `question` tool** (multi-question, structured choices) to ask clarifying questions — never present them as a static numbered list the user has to type answers to. Fall back to free-form prose only for a single question whose option space is genuinely open-ended.

**Exit gate:** Restate the feature in your own words including impact on existing code. Get explicit confirmation before Phase 2. Do not proceed on assumed answers.

---

## Phase 2 — Explore Existing Code

Use `todowrite` to mark this phase as `in_progress`. Mark it `completed` when done.

Prefer `codegraph_explore` or `codegraph_search` for codebase exploration. If `.codegraph/` is not initialized, mention the user can run `codegraph init -i` for faster exploration, then fall back to `grep` + `read` + `glob`.

Read before writing. This is where duplication gets prevented and integration points get found.

Map at least:
- Files/modules likely touched
- Existing patterns for similar features (reuse, don't reinvent)
- Shared utilities, hooks, components that the new code should compose with
- Tests that currently cover adjacent behavior
- Data model: schemas, migrations, query patterns

**UI surface parity.** If the feature will create a new instance of a recurring UI surface (modal, dialog, drawer, sheet, popover, form, confirm prompt, command palette, toolbar button, list-row action), dispatch the **`ui-pattern-inspector`** subagent via `task({ subagent_type: "ui-pattern-inspector", description: "Inspect UI pattern siblings", prompt: "Read 2–3 sibling instances of the surface '<name>'. Return a structured inventory: submit/cancel hotkeys, <Kbd> hint placement, autofocus target, loading/disabled states, footer chrome, error display, click-outside behavior. Also return 'consistent across siblings' / 'varies' verdict per property." })` with the surface name. The subagent reads 2–3 sibling instances and returns a structured inventory (submit/cancel hotkeys, `<Kbd>` hint placement, autofocus target, loading/disabled states, footer chrome, error display, click-outside behavior) plus a "consistent across siblings" / "varies" verdict. Record the inventory in the plan; matching consistent conventions is the default, diverging requires a stated reason. The `skill({ name: "propagate-ui-pattern" })` skill is the operational tool when 3+ siblings exist.

**Pre-write utility check — prevent duplication at creation.** For each non-trivial helper/utility/component the plan calls for, dispatch the **`utility-finder`** subagent via `task({ subagent_type: "utility-finder", description: "Find existing utility", prompt: "Search the repo for existing equivalents of a utility that does: '<description>'. Check naming variants. Return verdict: reuse / extend / write-new, with file:line refs for any existing match." })` with the function description. It searches the repo for existing equivalents (across naming variants) and returns a verdict — reuse / extend / write-new. The result short-circuits Phase 7b duplication-scan findings and keeps duplication out of the diff in the first place.

**Subagent fan-out — use when scope is wide.** Spawn parallel `task({ subagent_type: "explore", ... })` agents in a single message when:
- Feature spans 3+ distinct subsystems (e.g., DB + API + UI + jobs)
- You need to survey "is there already a utility for X?" across the repo
- Different areas have independent search criteria

See [`references/subagent-playbook.md`](references/subagent-playbook.md) for fan-out patterns and prompt templates.

**Exit gate:** You can name the existing files this feature will modify, the existing utilities it will reuse, the integration risks, AND every CRUD/input surface for the affected artifact (or confirm only one exists), AND — if a recurring UI surface is being added — you can name the sibling instances and the conventions you'll match. If you can't, keep exploring.

**Realignment boundary:** If exploration shows the request is actually a domain-model rename or lifecycle/state/value realignment with persisted values (enum migration, status rename, vocabulary change, adding/removing a lifecycle state), stop and invoke `skill({ name: "realign" })`. Do not squeeze model realignment through a feature-add plan.

---

## Phase 3 — Design

From Acauhi99/opencode-agent-system, MIT.

More from Acauhi99/opencode-agent-system