diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 0000000..8ab3b79 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,6 @@ +{ + "enabledPlugins": { + "feature-dev-workflow@feature-dev-workflow": true, + "superpowers@claude-plugins-official": true + } +} diff --git a/.claude/skills/developing-a-feature/SKILL.md b/.claude/skills/developing-a-feature/SKILL.md deleted file mode 100644 index 2cf0444..0000000 --- a/.claude/skills/developing-a-feature/SKILL.md +++ /dev/null @@ -1,182 +0,0 @@ ---- -name: developing-a-feature -description: - Use when starting implementation against a written plan and the - tracking issue(s) from planning-a-feature, or when handed a plan - by someone else. ---- - -# developing-a-feature - -## When to invoke - -When the plan from `planning-a-feature` (or equivalent) is committed and you're about to start writing code. Skip for -ad-hoc fixes — those go directly through `superpowers:test-driven-development` and `opening-a-pull-request`. - -## Workflow - -### 1. Read the state file first, then plan + spec - -The orchestration state file (`docs/superpowers/plans/--state.md`, created by `planning-a-feature` Step 8) -is the entry point — it points at the plan, the spec, the tracking issue, the open PRs, the worktrees, and any -bubble-up concerns logged so far. Read it in full before anything else; follow the file's "Resume checklist" -section to verify reality against the recorded state. - -Then open the plan and spec it references. Note: - -- The PR breakdown (one PR or multiple). -- The contract table in the plan's `## Contracts` section (if any). -- The dependency ordering — what must land first. -- Each contract's Realization strategy (pre-merge stub PR / stub-on-producer-branch / data-only). - -If the plan is missing, stale, or the state file's recorded state doesn't match reality (a PR's actual status has -drifted from the row), STOP and reconcile — re-invoke `planning-a-feature` Step 7 if the plan needs to change, or -update the state file's rows to match reality before continuing. - -### 2. Decide: single-PR or multi-PR (feature-branch model) - -- **Single PR** → one worktree on the `feature/` branch `planning-a-feature` created, one Claude session, one - PR from it targeting main. Skip the integration-PR step at the end. -- **Multi-PR** → feature-branch model: - - `planning-a-feature` already created `feature/` (off `origin/main`) and committed the spec + plan + state - file onto it. The orchestrator **reuses** that branch — it does not re-create it — attaching the integration - worktree at `.claude/worktrees/` (recorded as `feature_branch` + `feature_worktree` in the state file's - frontmatter). - - Every sub-PR is a real GitHub PR targeting `feature/`, not main. Each sub-worktree is created off the - feature branch with `git worktree add .claude/worktrees/-- -b feature/` - (raw git is the simplest path here; `EnterWorktree` defaults to branching from origin/main). - - When a sub-PR is ready, the orchestrator runs a self-review pass, then **self-merges** the sub-PR into - `feature/`. The dispatching agent owns this merge — sub-agents don't merge their own PRs. - - Sub-issue closure: `Fixes #` / `Closes #` only auto-fires on merge to the **default - branch**. Sub-PRs into the feature branch therefore use `Towards #` (the explicit "keep this issue - open" keyword); the orchestrator runs `gh issue close ` after each self-merge. - - When every sub-PR has been self-merged into the feature branch, the orchestrator opens the **integration PR** - `feature/` → `main`, with `Closes #` in its body, for external review and the final merge. - -For sequential single-PR work, skip to Step 4. For multi-PR work, dispatch parallel subagents in Step 3 — but -first, ask the user how sub-PR approval should work. - -**Sub-PR approval mode (multi-PR only).** Before any sub-worktree work starts, the orchestrator presents the operator -with a two-option choice via `AskUserQuestion`: - -- **Autonomous sub-worktree approval** — the orchestrator reviews each sub-PR with the `review` skill, self-merges it - into the feature branch, and closes its sub-issue automatically. Fastest fan-out; the operator only sees the - integration PR at the end. Suitable when the integration PR's external-review pass is the operator's intended - inspection point. -- **Manual sub-worktree approval** — the orchestrator still runs the `review` skill on each sub-PR, but then pauses - to ask the operator for explicit approval before `gh pr merge` runs. One round-trip per sub-PR, but the operator - inspects every diff before it lands on the feature branch. - -Record the choice in the state file's frontmatter as `sub_pr_approval: autonomous` or `sub_pr_approval: manual`. The -fan-out skill reads this field at every sub-PR ripening to decide whether to gate on operator approval. Default if -the field is missing in an older state file: `autonomous` (preserves the original behaviour). - -### 3. Set up the implementation environment - -- **Multi-PR (feature-branch model)** — `feature/` already exists, created and pushed by `planning-a-feature` - and carrying the committed spec/plan/state. **Reuse it; never re-create it** off `origin/main` — that errors - (`fatal: a branch named 'feature/' already exists`) and would orphan the planning artifacts. If planning - already made the integration worktree at `.claude/worktrees/`, just `cd` into it. Otherwise attach one to the - existing branch: - - ``` - git fetch origin - git switch main # vacate feature/ if planning left you on it - git worktree add .claude/worktrees/ feature/ - cd .claude/worktrees/ - ``` - - (Fallback only if planning was skipped and `feature/` exists nowhere: `git worktree add - .claude/worktrees/ -b feature/ origin/main && git -C .claude/worktrees/ push -u origin - feature/`.) Update the state file's `feature_branch` + `feature_worktree` frontmatter fields to point here. - Sub-worktrees off this branch are created later by `fanning-out-with-worktrees`. - -- **Single-PR** — `planning-a-feature` created `feature/` and committed the planning artifacts onto it; this is - the only branch, and the PR opens from it. Reuse it the same way — if planning made a worktree, `cd` in; otherwise - attach one to the existing branch: - - ``` - git fetch origin - git switch main # vacate feature/ if planning left you on it - git worktree add .claude/worktrees/ feature/ - cd .claude/worktrees/ - ``` - - (Fallback if planning was skipped: `git worktree add .claude/worktrees/ -b feature/ origin/main`.) - Skip the integration-PR step at the end; this is the only PR. - -### 4. Implement - -- **Multi-PR** — **REQUIRED SUB-SKILL:** `fanning-out-with-worktrees`. The skill owns parallel dispatch, multi-wave - ordering, the orchestrator watch loop, per-sub-PR review (via the `review` skill, orchestrator-driven — the - worktree subagent does not review its own PR), self-merge into the feature branch, manual sub-issue close, and - state-file maintenance. Returns control here when every sub-PR is self-merged and every contract is `locked`. - -- **Single-PR** — the orchestrator implements directly in the worktree from Step 3: - - **REQUIRED SUB-SKILL:** `superpowers:test-driven-development` for every code change. - - **REQUIRED SUB-SKILL:** `testing-a-feature` for the assertion shape — black-box against the contract, not - implementation. - - Commits follow CLAUDE.md conventions: `(): (#)`. - - Run `make test` and `make lint` (and `cd frontend && npm run typecheck` if frontend touched) before claiming - work is done. - -### 5. Checkpoint review before opening the final PR - -- **Single-PR feature** → **REQUIRED SUB-SKILL:** `superpowers:verification-before-completion`. Run `make test`, - `make lint`, and (if frontend touched) `cd frontend && npm run typecheck`. Paste the output. Forbids claiming - "done" without evidence. -- **Multi-PR feature** → **REQUIRED SUB-SKILL:** `reviewing-feature-progress`. The orchestrator's checkpoint skill - re-reads spec + plan + state, walks every self-merged sub-PR against acceptance criteria, checks state-file - integrity, and runs end-to-end verification on the main feature worktree (the feature branch as a whole, not just - per-sub-PR CI). Catches drift and integration-only failures before the external-review surface opens. If the - checkpoint finds gaps, route back through `developing-a-feature` Step 4 (follow-up sub-PR) or `planning-a-feature` - Steps 6/7 (plan/issue refinement) before continuing. - -### 6. Open the PR - -**REQUIRED SUB-SKILL:** `opening-a-pull-request`. Base + body keyword depend on which model is in play: - -- **Single-PR feature** → PR targets `main` from `feature/`. Body opens with `Fixes #` (bug) or - `Closes #` (feature/task) so the issue auto-closes on merge. -- **Multi-PR integration PR** → PR targets `main` from `feature/` (`gh pr create --base main --head - feature/`). Body opens with `Closes #` so the epic auto-closes on merge. This is the PR external - reviewers see; the diff is the whole feature. - -Sub-PRs into the feature branch are owned by `fanning-out-with-worktrees`, not this step. - -### 7. Tear down the planning artifacts - -Delete the plan + state file once the work is genuinely done. The spec stays — it's the durable ADR. The plan and -state file are scratch; leaving them committed past readiness pollutes the repo with stale operational state that -future `grep`s have to wade through. - -**Do NOT tear down until the integration PR's CI is green.** The state file is the resume contract for exactly the -case where the PR's CI comes back red and you have to fix forward — tear it down before CI confirms and a failed run -leaves you fixing forward with no recorded state. So the teardown is the **last commit on the feature branch, pushed -only after the integration PR's CI passes** — never as the commit *before opening* the PR, and never on "local green" -or "flipped ready" alone (those are not the CI gate). For single-PR features, same rule: fold the deletion in only -once the PR's CI is green. Until then, keep updating the state file as reality moves. - -## Anti-patterns - -- **Mixing single-PR and multi-PR flows mid-feature.** Once the plan declares multi-PR, the feature-branch model is - on. Don't quietly merge "just this small fix" directly to main while the feature branch is live — it skips - external review on the integration PR and forks the work. -- **Skipping `verification-before-completion` because "tests passed in my package".** `make test` runs the whole - tree because launcher cross-package wiring breaks on edits that look local. -- **Letting the state file drift from reality.** A resumed session reads the state file as ground truth. Update it - on every transition (worktree assigned, PR opened, sub-PR self-merged, phase changed, feature shipped). -- **Re-implementing fan-out logic inline.** Parallel dispatch, multi-wave ordering, the watch loop, per-sub-PR - self-review and self-merge — all of that is in `fanning-out-with-worktrees`. Don't paste it into the dispatch - prompt or the developing-a-feature flow; reference the sub-skill instead. - -## Red flags - -| Thought | Reality | -| -------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------ | -| "I'll just open one big PR, the plan is overcomplicating this" | The PR-shape decision happened during planning. Reopening it here means re-running `planning-a-feature` Step 4, not skipping the model. | -| "Tests pass, I'll skip make lint" | Lint is a CI gate. Running it locally is the cheapest place to catch the failure. | -| "The state file is for the planner, I don't need to update it during dev" | The state file is the resume contract. Every transition is your responsibility while dev is in flight. | -| "I'll open the integration PR before the last sub-PR is self-merged" | The integration PR's diff is supposed to be the whole feature. An in-flight sub-PR means the integration PR will be re-pushed mid-review. Wait. | -| "I'll create `feature/` off `origin/main` in step 3" | Planning already created it and committed the spec/plan/state onto it. `-b feature/` errors ("already exists") and re-creating off `origin/main` orphans the planning artifacts. Reuse the existing branch; attach a worktree to it. | -| "Tests pass locally and the PR is ready, so I'll tear down plan/state now" | Local green and "ready" aren't the CI gate. If the integration PR's CI comes back red you fix forward — with no state file if you deleted it. Tear down only after the PR's CI is green. | diff --git a/.claude/skills/fanning-out-with-worktrees/SKILL.md b/.claude/skills/fanning-out-with-worktrees/SKILL.md deleted file mode 100644 index ad685a1..0000000 --- a/.claude/skills/fanning-out-with-worktrees/SKILL.md +++ /dev/null @@ -1,206 +0,0 @@ ---- -name: fanning-out-with-worktrees -description: - Use when an orchestrator needs to dispatch parallel subagents into - per-PR worktrees off a feature branch — typically invoked from - developing-a-feature for multi-PR features. ---- - -# fanning-out-with-worktrees - -## When to invoke - -When you're the orchestrator for multi-PR feature work and these prerequisites are met: - -- The `feature/` integration branch and the main feature worktree exist (set up by `developing-a-feature` before - invoking this skill). -- The plan (`docs/superpowers/plans/--plan.md`) has a `## Contracts` section with a Realization strategy - per row. -- The state file (`docs/superpowers/plans/--state.md`) has rows for each sub-issue. - -Skip for single-PR features — there's no fan-out, just one branch off main. - -## Why this exists - -Parallel fan-out compounds in complexity quickly: contract handoff, isolation, wave dependencies, propagation, state -tracking, per-PR ripening. Wrapping the choreography in one skill keeps `developing-a-feature` focused on the -single-vs-multi decision and the integration-PR endgame, and gives any future use case (parallel migration work, -parallel refactor, cross-module sweep) a reusable orchestration entry point. - -## Workflow - -### 1. Plan the waves - -Look at the plan's `## Contracts` table. Sub-PRs that are contract **producers** go in the FOUNDATIONAL wave; sub-PRs -that are contract **consumers** go in subsequent waves, keyed by when their contract is realized: - -- `data-only` and `pre-merge stub PR` realizations → consumers can start as soon as the producer's stub PR has merged - into the feature branch (or the data-only contract is documented). -- `stub-on-producer-branch` → consumers branch from the producer's branch (not the feature branch); dispatch is timed - to after the producer's PR is open with the stub in place. - -Sub-PRs with no contract dependency → all in one wave. - -Record the wave assignments in the state file's `## Phases` section before dispatching. - -### 2. Create sub-worktrees and dispatch wave N - -For each sub-PR in the wave, the orchestrator creates the worktree first: - -``` -git worktree add .claude/worktrees/-- -b -``` - -`` is `feature/` for default sub-PRs, `feature/` after the stub PR merged (for `pre-merge stub -PR` consumers), or the producer's branch (for `stub-on-producer-branch` consumers). - -Then dispatch one subagent per sub-PR. **REQUIRED SUB-SKILL:** `superpowers:dispatching-parallel-agents`. - -Each dispatch prompt MUST include: - -1. **Isolation verification as the first action.** `cd && pwd && git branch --show-current` — the - subagent confirms it's on the sub-branch in the right worktree before any edit. Commits land on the wrong branch - otherwise. -2. **Context handoff.** State file path, plan path, spec path, the issue number it's working, and the relevant - contract row(s) (Name + Producer + Consumer + Shape + Realization). The subagent implements **against the - contract** — it does not re-discover or re-design it. -3. **Implementation skills.** `superpowers:test-driven-development` + `testing-a-feature` for every change. -4. **PR completion.** When the implementation is done and verified, the subagent invokes `opening-a-pull-request` - with `--base feature/` and `Towards #` in the body (NOT `Fixes` / `Closes` — those don't fire on - non-default-branch merges; `Towards` is the explicit "keep this issue open until the orchestrator closes it - manually" keyword). The subagent reports the PR URL back to the orchestrator. - -### 3. Update the state file as subagents start work - -As each subagent surfaces its worktree path and branch, the orchestrator fills in the row in the state file's -`## PRs / worktrees` table. When a subagent opens its draft PR, the orchestrator fills in the PR column with the -base ref (`# → feature/`) and flips status to `draft`. - -A stale row is worse than no row — a resumed session reads the state file as ground truth. - -### 4. Watch loop - -While subagents run, the orchestrator is the integration point — not an idle waiter. Watch for concerns that bubble -up from any subagent and propagate the resolution across every subagent the concern touches. Silent divergence is -the failure mode this watch loop exists to prevent. - -Categories of concerns to watch for: - -- **Contract drift.** A row in the `## Contracts` table needs to shift (reviewer feedback, an edge case the producer - hit). Pause every affected consumer, update the plan's row, propagate. -- **Spec ambiguity surfaced mid-implementation.** A subagent hits a case the spec didn't cover. Surface to the user, - get a decision, amend the spec (or add a note to the plan), and propagate to every subagent whose scope touches - the same surface. -- **Discovered cross-PR dependency.** A subagent finds it needs a helper, type, or behaviour from another PR that the - plan didn't enumerate. Decide whether the helper becomes a new contract (file an issue, add a contract row), - inlines into the current PR, or is something one of the other subagents is already producing. -- **Test failure in shared infrastructure.** One subagent breaks a test that another subagent's PR relies on. - Coordinate the fix into the right PR; don't let both subagents fix it independently. -- **External dependency change.** A Go module bump, a frontend lib update, an API shift — affects every running - subagent. -- **Resource conflict.** Two subagents both editing the same file or symbol. Re-scope one to avoid the collision, or - serialize the work. - -How to propagate the resolution: - -- **Subagent still running** → `SendMessage` with the subagent's id to push the resolution with full context. The - subagent resumes with the update applied. -- **Subagent finished, PR still open** → re-dispatch a focused follow-up with the PR number and the specific change. -- **Subagent not yet dispatched (later wave)** → update its dispatch prompt's context block before launching. - -Append a dated entry to the state file's `## Bubble-up log` (newest at top) naming the concern, the resolution, and -the propagation path used. The orchestrator owns propagation; a concern raised by one subagent and not propagated to -the others is how this whole pattern fails. - -### 5. Per-sub-PR ripening — all orchestrator-driven - -When a sub-PR is ready (subagent reports `ready` and the relevant verification commands pass), the **orchestrator** -takes over for the rest of the lifecycle. Every action below — review, merge, sub-issue close, state-file update — -is the orchestrator's, not the worktree subagent's. Three reasons this is the right division of labour: - -- **Review independence.** The subagent that wrote the code is the wrong reviewer for the same code; the - orchestrator's distance from the implementation is the whole point. -- **Global view.** Only the orchestrator holds the merge-order context (which contract rows are `locked`, which - sibling PRs are still in flight, which wave we're in). A subagent merging on its own would commit to ordering it - can't see. -- **Worktree topology.** Subagents live in their per-sub-PR worktrees; only the orchestrator's main feature worktree - has `feature/` checked out, so the merge naturally happens on the orchestrator's side. - -Per sub-PR, in order: - -1. **Review.** **REQUIRED SUB-SKILL:** the `review` skill — invoke it against the sub-PR number to walk the diff with - full PR context. This review is weaker than external review (which lands at the integration PR) but stronger than - nothing; it catches issues that would otherwise pile onto the integration PR reviewer. -2. **Approval gate, per the state file's `sub_pr_approval` mode.** Every gate covers the **bundle**: merge + sub-issue - close + state-file update. The close is bodyless (no `--comment` flag) — GitHub automatically cross-references - the sub-issue from the merge commit via the sub-PR's body keyword, so no custom comment is needed and there's - no "specific body about to land" for the close mutation. - - **`autonomous`** (default) — proceed straight through the bundle in steps 3-5. The operator opted into the - mechanical bundle (review → merge → bodyless close → state update) in `developing-a-feature` Step 2. - - **`manual`** — pause and ask the operator for explicit approval before the bundle. The prompt MUST surface: - a one-line summary of the review findings ("review clean" / " findings, none blocking" / specific concerns), - the PR's title and diff size, and a note that closing sub-issue `#` follows the merge. Wait for an - explicit yes. On push-back, route the concern back to the worktree subagent via `SendMessage` instead of - merging. -3. **Merge into the feature branch.** First **push any local state-file commits to `origin/feature/`** — the - merge happens on GitHub's side and lands on origin's current tip, so unpushed local commits make the post-merge - `git -C pull --ff-only` fail with "Not possible to fast-forward" (local and origin have - diverged; recover with `git rebase origin/feature/`). Keep local == origin at every merge boundary. Then - `gh pr merge --merge --repo sourcehawk/triagent` (or `--squash` / `--rebase` per project preference), and - `git -C fetch origin && git merge --ff-only origin/feature/` to bring the merge back. -4. **Close the sub-issue.** `gh issue close --repo sourcehawk/triagent`. Sub-PRs into a non-default - branch don't trigger `Fixes`/`Closes` — manual close is the workaround. The body's `Towards #` - keyword left the issue open precisely so the orchestrator can close it here; the cross-reference from the merge - commit (which references `#`, which references `#`) is preserved automatically without a - custom comment. -5. **Update the state file.** Flip the row's status to `self-merged`. If the sub-PR was the realization of a - contract (e.g. a pre-merge stub PR), flip the contract row's status to `locked` and fill in the `Realized in` - pointer. - -### 6. Checkpoint review, then dispatch wave N+1 - -When every sub-PR in wave N is `self-merged` AND every contract tied to wave N's producers is `locked`: - -1. **REQUIRED SUB-SKILL:** `reviewing-feature-progress` — run the alignment checkpoint at the wave boundary. Drift - accumulated across wave N is cheapest to fix here, not at the integration PR. If the checkpoint surfaces gaps, - address them (follow-up sub-PR in wave N, plan/issue refinement, or both) before moving on. -2. Once the checkpoint is clean, dispatch wave N+1. Wave N+1's subagents pick up the realized contracts because the - orchestrator updated the state file's `## Contracts` table. - -Repeat Steps 2 → 6 for each wave. - -### 7. All waves complete → hand back - -When every wave is complete (every sub-issue closed, every row `self-merged`, every contract `locked`), update the -state file's frontmatter `status:` to `review` and return control to `developing-a-feature`. The next step is the -integration PR (`feature/` → `main` with `Closes #`) which `developing-a-feature` owns. - -## Anti-patterns - -- **Dispatching parallel agents without contracts.** "Two subagents on these two PRs" with no contract = divergent - implementations that block at integration. If the plan didn't define a contract, don't dispatch — go back to - `planning-a-feature` Step 6 and add one. -- **Skipping wave dependencies.** Consumers dispatched before producers' contracts are realized produce code against - an imagined shape. Dispatch wave N+1 only after wave N is fully self-merged and contracts locked. -- **Self-merging without orchestrator review.** The integration PR is where external review lands, but sub-PRs still - need a review pass before going into the feature branch. Skipping it dumps issues onto the integration PR reviewer. -- **Letting the worktree subagent review its own PR.** A subagent reviewing the code it just wrote has the same - blind spots in review that it had in implementation. The orchestrator owns sub-PR review precisely because it - didn't write the code. -- **Letting a bubble-up concern die in one subagent.** Propagation is the whole point of the watch loop. -- **Forgetting to manually close the sub-issue.** The keyword doesn't fire on non-default-branch merges; the issue - page silently shows "open" even though the work shipped. -- **Letting the state file go stale.** A resumed session reads it as ground truth. Update every row as reality moves; - commit **and push** the state-file diff per phase transition — an unpushed local commit diverges the feature branch - the moment the next sub-PR squash-merges on GitHub (see Step 5.3). - -## Red flags - -| Thought | Reality | -| -------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------ | -| "The plan says parallel but I don't see contracts — I'll just guess" | Plan is incomplete. Stop and define the contract, or sequence the work. | -| "I'll dispatch wave 2 now, wave 1 is almost done" | Almost-done producers haven't `locked` their contract rows. Wave 2 will diverge. Wait. | -| "The subagent will figure out the worktree on its own" | Create the worktree yourself and embed `cd && pwd && git branch --show-current` in the dispatch. | -| "Self-review feels redundant, the integration PR catches everything" | Integration PR reviewer can't catch every issue across 5 sub-PRs in one sitting. Catch them per PR. | -| "The subagent already verified the diff, no need for the orchestrator to review again" | Verification (tests pass) is not review (does the diff express intent cleanly). Different signals. | -| "I'll close the sub-issue once the integration PR merges" | Issues that read "open" while their work has shipped clutter the triage view. Close manually at self-merge. | diff --git a/.claude/skills/opening-a-pull-request/SKILL.md b/.claude/skills/opening-a-pull-request/SKILL.md deleted file mode 100644 index cacc27f..0000000 --- a/.claude/skills/opening-a-pull-request/SKILL.md +++ /dev/null @@ -1,121 +0,0 @@ ---- -name: opening-a-pull-request -description: - Use when about to call `gh pr create` or `gh pr edit` against - sourcehawk/triagent. ---- - -# opening-a-pull-request - -## When to invoke - -Two moments: - -- **Opening a draft PR** when work is still in flight and you want an early surface for reviewers to flag direction - issues. Body follows the draft shape (below). -- **Flipping to ready, or opening a PR straight to ready**, when the work is done. Body follows the ready shape. - -## Templates - -Two templates carry the shape and the per-section guidance: - -- `templates/pull-request-draft.md`: draft PR body. -- `templates/pull-request-ready.md`: ready-for-review PR body. - -Copy the appropriate template, fill in each section per its `` guidance, then pass the body to -`gh pr create` (opening) or `gh pr edit` (flipping or editing) via a `--body "$(cat <<'EOF' ... EOF)"` heredoc. -GitHub doesn't render HTML comments, so leaving the template guidance in place is harmless — don't burn a step -removing it. - -## PR title - -Set the title once when opening and don't rename it. Match the project's commit-message convention from `CLAUDE.md`: - -``` -(): -``` - -Types: `feat`, `fix`, `refactor`, `test`, `chore`, `docs`. Area mirrors the module path (`mcp/strategies`, -`internal/server`, `frontend`, `watches`). When the PR bundles unrelated areas, lead with the headline change and -acknowledge the others in the body — don't try to encode both in the title. - -**Do not suffix the title with lifecycle wording** (`wip`, `draft`, `plan`, `scaffolding`, etc.). GitHub's draft / ready -chip carries the lifecycle state. A single title that survives from open through merge avoids renames and avoids -shipping stale wording into the merged record. - -## Linking the tracking issue - -When the PR has a tracking issue, link it as the **first line of the body's opening section** — `## What lands here` -for draft, `## Description` for ready. The exact keyword depends on **whether the PR should close the issue on -merge**: - -- `Fixes #` — bug-fix issue; GitHub auto-closes the issue when the PR merges to the default branch. -- `Closes #` — feature/task issue; same auto-close semantics, neutral phrasing. -- `Towards #` — the PR contributes to the issue but should NOT close it on merge. GitHub creates the back-link - but doesn't auto-close. Use when the issue will be closed by a sibling PR, by a later PR, or by the orchestrator - manually (see below). - -Which keyword belongs depends on **which branch the PR targets**: - -- **PR targets `main` (the default branch)** — use `Fixes` / `Closes` if the merge should close the issue; - use `Towards` if the issue should stay open. -- **PR targets a feature branch** (`feature/` in the multi-PR feature-branch model — see `developing-a-feature`) - — use `Towards #`. `Fixes` / `Closes` keywords only auto-trigger on merges to the default branch, so - writing them on a feature-branch-bound PR creates a misleading promise that nothing will fulfill. The sub-issue is - closed manually by the orchestrator after the self-merge. The integration PR (feature → main) gets `Closes #` - because that PR does merge to main. - -If there is no tracking issue, drop the line entirely and open the section with prose. - -Don't carry the issue link as a bold-line metadata header at the top of the body — the keyword form is what GitHub -uses to thread the cross-link and it reads as a natural opener to the implementation summary. - -Other related PRs / issues (siblings, follow-ups, prior art) belong under `## Related` — that section is **only** for -links that aren't the tracking issue, since the tracking issue is already cross-linked via the keyword above. - -## Core principle: user-in-the-loop for every GitHub mutation - -Don't run `gh pr create` or `gh pr edit` without an explicit confirmation **for the specific body about to land**. -Generic intent earlier ("yes please open a PR") is not standing consent for the body now. - -Every confirmation shows the user: - -- The exact target (`sourcehawk/triagent`, or `#` for edits). -- The full proposed body. - -Wait for an explicit "yes" before any `gh pr` call. Treat absence of objection as a no. - -## Steps when flipping a draft to ready - -1. **Rewrite the body from `templates/pull-request-ready.md`.** The shapes are different — the draft asks "review the - direction"; the ready asks "review the implementation." Don't ship the draft body forward unchanged. -2. **Confirm both mutations in one prompt, body inline.** Marking the PR ready is a separate GitHub mutation from - editing the body, and the user-in-the-loop rule applies to both. Phrase the confirmation as: "About to update - #'s body to the version below AND flip it from draft to ready. Confirm?" — then paste the body. Wait for - an explicit yes. Splitting confirmation across two prompts is fine; running `gh pr ready` on the strength of the - body confirmation is not. -3. **Run `gh pr edit --body "$(cat <<'EOF' … EOF)"` then `gh pr ready `** once the user confirms. - -## Anti-patterns - -- **Lifecycle suffix in PR titles** (`... wip`, `... draft`, `... scaffolding`). The title outlives the state that - named it. The body and GitHub's chip carry lifecycle; the title doesn't need to. -- **Flipping ready with the draft body unchanged.** Different shape, different audience. Rewrite from the ready - template. -- **Marking ready before the Testing section is filled in.** That section is what gives the reviewer confidence the - PR is shippable; leaving it blank silently drops the claim. -- **Running `gh pr create` / `gh pr edit` on inferred consent.** Every body is a fresh confirmation. The cost of - pausing is low; the cost of an unwanted public mutation is high. - -## Red flags: STOP before flipping ready or publishing - -These thoughts mean the PR isn't actually ready to publish or flip: - -| Thought | Reality | -| ------------------------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------- | -| "The draft description is fine, no need to rewrite" | Different shape, different audience. Rewrite from `templates/pull-request-ready.md`. | -| "Marking ready now, will fix the body in a follow-up edit" | The body is what the reviewer reads in the first 10 seconds. Fix it first, then `gh pr ready`. | -| "The user said yes a turn ago, this is the same thing" | Bodies change between turns. Confirm the exact body about to land. | -| "I'll just append a note and they can edit later if needed" | They shouldn't have to clean up after the agent. Confirm first. | - -All of these mean: rewrite the body from the right template, paste it inline in chat, and wait for an explicit yes. diff --git a/.claude/skills/opening-a-pull-request/templates/pull-request-draft.md b/.claude/skills/opening-a-pull-request/templates/pull-request-draft.md deleted file mode 100644 index 0bf232c..0000000 --- a/.claude/skills/opening-a-pull-request/templates/pull-request-draft.md +++ /dev/null @@ -1,37 +0,0 @@ - - -## What lands here - - - -## Related - - - diff --git a/.claude/skills/opening-a-pull-request/templates/pull-request-ready.md b/.claude/skills/opening-a-pull-request/templates/pull-request-ready.md deleted file mode 100644 index 137de44..0000000 --- a/.claude/skills/opening-a-pull-request/templates/pull-request-ready.md +++ /dev/null @@ -1,62 +0,0 @@ - - -## Description - - - -## Changes - - - -## Challenges - - - -## Related - - - -## Testing - - diff --git a/.claude/skills/planning-a-feature/SKILL.md b/.claude/skills/planning-a-feature/SKILL.md deleted file mode 100644 index 0163cfe..0000000 --- a/.claude/skills/planning-a-feature/SKILL.md +++ /dev/null @@ -1,204 +0,0 @@ ---- -name: planning-a-feature -description: - Use at feature conception — before writing any code, before filing - any issue, before drafting any plan. ---- - -# planning-a-feature - -## When to invoke - -At the start of any feature larger than a one-off fix or refactor. If you're about to brainstorm, write a spec, file -a tracking issue, or write an implementation plan, this skill is the choreography that sequences those tools so the -artifacts land in the right order. - -Skip for: -- One-off bug fixes (file a `bug` issue directly via `writing-github-issues`). -- Single-line refactors or test additions. -- Work that's already plan-shaped from someone else — jump to `developing-a-feature`. - -## Why this exists - -A senior engineering team's pre-implementation pipeline is **visible, fast, and parallelizable** — not a linear queue -of "brainstorm → spec → tickets → plan → code" run by one person in their head. Each step is explicit, hands off -between sub-skills cleanly, and surfaces the parallelism decision as a first-class output so implementation can fan -out instead of dragging. - -## Workflow - -### 1. Brainstorm intent - -**REQUIRED SUB-SKILL:** Invoke `superpowers:brainstorming` to nail down intent, scope, and design choices before any -artifact is written. The brainstorming skill explores the problem — it does **not** author files. - -### 2. Author the spec — and any cross-cutting ADR - -**Spec.** Capture the brainstorm's conclusions in a spec at `docs/superpowers/specs/YYYY-MM-DD--design.md`. The -spec is the feature's design record — WHAT we're building and WHY, with goals, non-goals, design choices, risks, -alternatives considered. It does **not** enumerate PRs, commits, or parallelism contracts (those live in the plan, -see step 6). Slug is short ("multi-tenant-profiles", not "implement-multi-tenant-profile-system"); date is the -conception date. - -**ADR (only if the brainstorm surfaced a project-wide architectural decision).** A spec captures feature-scoped design. -An ADR captures **cross-cutting** decisions that affect multiple features — choosing one architectural approach over -another for reasons that future readers will need. Examples of ADR-worthy decisions: introducing a new IPC -channel, picking a state-of-the-art pattern over the project's current default, rejecting a tempting alternative. -Examples of NOT ADR-worthy: feature-specific design (that's the spec), operational conventions (CLAUDE.md), file -naming (CLAUDE.md). - -If the brainstorm surfaced an ADR-worthy decision, author the ADR at `docs/adrs/-.md` using the format in -`docs/adrs/README.md` (Decision / Context / Consequences). Number sequentially — pick the next free `NNNN`. Add an -entry to the index in `docs/adrs/README.md`. Most features don't produce an ADR; if you're forcing one, the -decision probably belongs in the spec instead. - -Both the spec and any ADR(s) are tracked source artifacts (`docs/superpowers/specs/` and `docs/adrs/` are not -gitignored) — but **never commit a planning artifact to `main`.** The feature owns its branch from birth. The branch -name is the same whether the work ships as one PR or many (the step-4 PR-shape decision doesn't change it), so create -it off `origin/main` before the first commit: - -``` -git fetch origin main -git switch -c feature/ origin/main -``` - -For a non-trivial (multi-PR) feature, run planning from a dedicated worktree on that branch instead, so the whole -feature — planning docs included — lives in one isolated checkout: - -``` -git fetch origin main -git worktree add .claude/worktrees/ -b feature/ origin/main -cd .claude/worktrees/ -``` - -Either way, stay on `feature/` for the rest of planning; every artifact commit lands there. Don't push yet — -that happens at step 8, after the user approves the spec. `developing-a-feature` reuses this branch (and the -worktree, if you created one); it never re-creates it off `origin/main`, and `main` receives the feature only through -the integration/feature PR. - -### 3. User reviews the spec (and any ADR) - -Pause. Surface the spec path and wait for explicit "approved" or redirection before moving on. A spec the user hasn't -read is a draft, and drafts don't get tickets filed against them. - -### 4. PR-shape decision - -Decide whether the work ships as: - -- **One PR** — single self-contained change, one reviewer pass, one merge to main. -- **Multiple PRs** — multiple feature-sized chunks, each independently reviewable, possibly parallelizable. Multi-PR - features land via the **feature-branch model**: a long-lived `feature/` branch off main; every sub-PR is a - real GitHub PR targeting `feature/` (not main); when every sub-PR has been self-merged into the feature - branch, a final **integration PR** from `feature/` to main collects the whole feature for external review. - Main stays shippable throughout the work; each sub-PR retains full GitHub visibility (comments, reviews, history). - -The PR-shape judgment is grounded in **reviewer cost**: a 2000-line PR is unreviewable even if the work is "one -thing". If you can name two independent surfaces that ship value separately, that's two PRs and the feature-branch -model applies. - -The decision is noted in chat (or as a one-line `## Implementation breakdown` paragraph in the spec naming the PRs — -NOT the contracts between them). The spec stays a clean ADR otherwise. - -### 5. File the tracking issue(s) - -**REQUIRED SUB-SKILL:** Invoke `writing-github-issues` with the PR-shape decision in hand. - -- Single PR → file one `feature` (or `bug`) issue. -- Multiple PRs → file an `epic` with one sub-issue per PR. - -Sub-issues reference only the parent epic in their `## Context` section — GitHub's sub-issue linkage threads them and -the epic is the design hub. Don't link the spec or plan from any issue: issues are durable, those files move and get -deleted, and the spec is referenced from the plan, not the ticket. - -### 6. Write the plan (and discover contracts) - -**REQUIRED SUB-SKILL:** Invoke `superpowers:writing-plans` to produce -`docs/superpowers/plans/YYYY-MM-DD--plan.md`. The plan is HOW: ordered tasks, dependencies, review checkpoints, -PR-by-PR breakdown. - -**During plan-writing, identify parallelism and define contracts.** For every pair of PRs that could run in parallel, -ask: what does each side need to know about the other's wire shape, API surface, or data layout to start work -without blocking? That's a contract. Document each in a `## Contracts` section of the plan: - -| Name | Producer (issue) | Consumer (issue) | Shape | Realization | -| -------------------------- | ---------------- | ---------------- | ----------------------------------------------------- | ------------------------ | -| `profile-storage-layout` | #22 | #24, #25 | path `~/.config/triagent//sessions//...` | data-only | -| `profile-loader-signature` | #22 | #24, #25 | `profile.Resolve(name string) (*Profile, error)` | pre-merge stub PR (#21) | - -Each contract names the wire / signature / layout the consumer can write code against today, without waiting for the -producer's implementation. "TBD" is not a contract — block on it until it's concrete. - -**Realization strategy.** A contract row is conceptual; for parallel work to actually compile, the interface has to -exist as code or as data before either side starts. Pick one per row and put it in the Realization column: - -- **Pre-merge stub PR** — file a tiny scaffold PR that exports the symbol the consumers need (Go interface or - function signature with `panic("unimplemented")` body, TypeScript type, HTTP path constant). It targets the - feature branch (in multi-PR features) or main (in single-PR features) and merges BEFORE the implementation PRs - branch off. Producer and consumers then all branch from the post-stub state and import the real symbol. Best - default for code-shaped contracts (Go signatures, TS types); costs one trivial extra PR; payoff is both sides - compile from day one. Reference the stub PR's number in the Realization column once it's open. -- **Stub-on-producer-branch** — the producer's own PR opens with just the interface + panic-bodies; consumers branch - from the producer's branch (not main) and rebase as the producer fills in the body. Avoids the extra PR but - couples consumers to the producer's branch lifetime — rebase pain when the interface evolves. Use only when the - interface and one implementation are inherently coupled (shared unexported package, sibling files in one module). -- **Data-only** — when the contract is a path layout, file format, wire protocol, or env-var name, no code stub is - needed. Consumers write against the contract row directly — strings and paths are just strings and paths. Mark - the row `data-only`. - -Sequential work doesn't need contracts. Only document them where two workers genuinely run in parallel. - -### 7. Refine issues if planning surfaced changes - -Planning often surfaces scope changes. If the issues from step 5 no longer match the plan's breakdown, update them -**now**, before implementation starts. - -**REQUIRED SUB-SKILL:** Re-invoke `writing-github-issues` Step 2B (update) for any issue whose acceptance criteria, -scope, or dependencies shifted. Present and confirm each affected issue **one at a time** — one issue's gap list + -proposed body per confirmation prompt, then its `gh issue edit`, then the next. Don't batch several issues into a -single "yes to all": a wall of bodies gets rubber-stamped instead of read. - -### 8. Initialize the orchestration state file - -Before handing off, create `docs/superpowers/plans/YYYY-MM-DD--state.md` from -`templates/feature-state.md`. This is the **orchestration state** — phases, PRs, worktrees, contract realization, -bubble-up log. It's what a future Claude session reads first to resume the work without a massive user prompt. - -The state file is scratch (same lifecycle as the plan): tracked in git so it survives sessions / worktrees / -machines, and deleted in the orchestrator's last commit once every sub-issue is closed and the feature has shipped. -Update it as the work progresses — see `developing-a-feature` for the update choreography. - -Commit the spec, the plan, and the state file together as the planning artifact set on `feature/` (created in -step 2). Confirm you're not on `main`, then publish the branch so `developing-a-feature` can attach its integration -worktree to it: - -``` -git branch --show-current # must be feature/, never main -git push -u origin feature/ -``` - -### 9. Hand off to implementation - -Spec + plan + state file committed and pushed on `feature/`, issues aligned, contracts written → invoke -`developing-a-feature` to start the work. The state file is the entry-point artifact for every session that touches this feature afterward. - -## Anti-patterns - -- **Putting PR breakdown or parallelism contracts in the spec.** Spec is a durable ADR. PR breakdown is short-lived - scaffolding; contracts are implementation coordination. Both belong in the plan. -- **Filing issues before the spec is reviewed.** Wastes time when scope shifts during review. -- **Writing a plan before deciding PR shape.** The plan's structure depends on whether you're shipping one PR or many. -- **Declaring "we'll parallelize this" without writing the contract.** "Two workers in parallel" turns into "two - workers blocked on each other after day one" without explicit shapes. -- **Treating "no parallelism possible" as a defeat.** Sequential work is fine. The cost of inventing fake parallelism - exceeds the wall-clock saved. - -## Red flags - -| Thought | Reality | -| ------------------------------------------------------------------ | ---------------------------------------------------------------------------------------------------- | -| "The spec already covers ordering, we don't need a plan" | Spec is WHAT/WHY. Plan is HOW. Different artifacts, different lifecycles. | -| "I'll just write the plan and figure out parallelism later" | Parallelism not in the plan never happens. Identify it during plan-writing. | -| "Filing issues is busywork, I'll just start coding" | Without issues, the work isn't reviewable in chunks; the PR will be one giant blob. | -| "The contract is obvious, no need to write it down" | Two parallel workers reading the same "obvious" thing produce divergent implementations. Write. | -| "I'll skip user review on the spec, it's just an internal doc" | An unreviewed spec is a draft. Drafts don't get tickets filed against them. | -| "The feature branch is created later in `developing-a-feature`, so the spec/plan/state commit to `main` first" | Planning owns the branch's birth. Create `feature/` in step 2 before the first commit; `developing-a-feature` reuses it. Nothing about the feature touches `main` except the final integration/feature PR. | diff --git a/.claude/skills/planning-a-feature/templates/feature-state.md b/.claude/skills/planning-a-feature/templates/feature-state.md deleted file mode 100644 index cab12a8..0000000 --- a/.claude/skills/planning-a-feature/templates/feature-state.md +++ /dev/null @@ -1,98 +0,0 @@ - - ---- -feature: -spec: docs/superpowers/specs/YYYY-MM-DD--design.md -plan: docs/superpowers/plans/YYYY-MM-DD--plan.md -tracking_issue: sourcehawk/triagent# -feature_branch: feature/ # omit for single-PR features -feature_worktree: .claude/worktrees/ # the main integration worktree; omit for single-PR -sub_pr_approval: autonomous # autonomous | manual; omit for single-PR (see developing-a-feature Step 2) -integration_pr: sourcehawk/triagent# # filled in once the feature → main PR opens -status: planning ---- - -# — orchestration state - -## Phases - - - -- **Phase 1 (foundational)** — `sourcehawk/triagent#`, `sourcehawk/triagent#` -- **Phase 2 (consumers)** — `sourcehawk/triagent#`, `sourcehawk/triagent#` - -## PRs / worktrees - - - -| Issue | Branch | Worktree path | PR (→ base) | Status | -| --------------------------- | ---------------------------- | ---------------------------------------- | ------------------------------------ | ------------- | -| sourcehawk/triagent# | | .claude/worktrees/-- | sourcehawk/triagent# → feature/ | not-started | - -## Contracts - - - -| Name | Realization | Realized in | Status | -| ----------------- | -------------------------------------------------------- | ------------------------------------ | ----------------- | -| `` | pre-merge stub PR / stub-on-producer-branch / data-only | sourcehawk/triagent# or "n/a" | pending / locked | - -## Bubble-up log - - - -- _No concerns yet._ - -## Resume checklist - -For a fresh Claude session resuming this work: - -1. Read this state file in full. -2. Read the plan at the path in the `plan:` frontmatter. -3. Read the spec at the path in the `spec:` frontmatter. -4. Verify each open PR's actual state via `gh pr view --repo sourcehawk/triagent`. -5. For each `in-progress` or `draft` row, `cd` to the worktree path and check `git status` + `git log --oneline main..HEAD`. -6. Re-dispatch subagents as needed per `developing-a-feature` (parallel waves still in flight; the orchestrator watch - loop continues). diff --git a/.claude/skills/reviewing-feature-progress/SKILL.md b/.claude/skills/reviewing-feature-progress/SKILL.md deleted file mode 100644 index 2a1bf08..0000000 --- a/.claude/skills/reviewing-feature-progress/SKILL.md +++ /dev/null @@ -1,137 +0,0 @@ ---- -name: reviewing-feature-progress -description: - Use at feature-development orchestration checkpoints — between - fan-out waves, before opening the integration PR, and before - flipping the integration PR ready. ---- - -# reviewing-feature-progress - -## When to invoke - -Three checkpoints during a feature in flight: - -1. **Between fan-out waves.** After wave N is fully self-merged and before wave N+1 dispatches — catches drift while - it's still cheap to fix. -2. **Before opening the integration PR.** The integration PR is the external-review surface; its first impression - has to be right. -3. **Before flipping a draft integration PR to ready.** Any feedback addressed since the draft opened needs to be - re-aligned against the spec/plan. - -Skip for ad-hoc fixes that didn't go through `planning-a-feature`. - -## Why this exists - -Per-sub-PR review (`review` skill, run by the orchestrator inside `fanning-out-with-worktrees`) checks one diff at -one moment. It doesn't check whether all sub-PRs together still implement what the spec promised, whether the -acceptance criteria are covered, or whether the feature branch as a whole compiles and tests cleanly. Drift -accumulates silently across waves; this skill catches it at the boundary. - -## Workflow - -### 1. Re-read the artifacts - -Open these in order: - -- The state file (`docs/superpowers/plans/--state.md`). -- The plan (`docs/superpowers/plans/--plan.md`), with focus on the `## Contracts` section and the - PR-by-PR breakdown. -- The spec (`docs/superpowers/specs/--design.md`), with focus on goals + non-goals. -- Each closed sub-issue's `## Acceptance criteria` section (`gh issue view --repo sourcehawk/triagent`). - -### 2. Walk each self-merged sub-PR against the plan - -For every row in the state file's `## PRs / worktrees` table with status `self-merged`: - -- **Diff vs plan.** Use the `review` skill against the sub-PR number to walk the diff with full context. Cross-check - against the sub-issue's acceptance criteria — does the diff cover every bullet? -- **Contract realization.** If the sub-PR was a contract producer, inspect the symbol / wire / data layout that - actually shipped against what the contract row in the plan documented. The contract row's `Status` should be - `locked` and `Realized in` should point at the merged sub-PR. - -### 3. Acceptance-criteria coverage - -For every sub-issue (whether `self-merged` or still open): - -- Is every bullet in the issue's `## Acceptance criteria` section now testable / observable in the feature branch? -- If a criterion isn't covered, classify: - - **Missing implementation** — file or surface a follow-up sub-PR; do not open the integration PR yet. - - **De-scoped during planning** — update the issue body to remove the criterion (via `writing-github-issues` Step - 2B, with confirmation); do not silently drop it. - - **Rephrased / equivalent** — the implementation satisfies the criterion under a different name; update the - criterion to match the language used in the diff. - -### 4. State-file integrity check - -Walk the state file and verify reality against record: - -- Every `self-merged` row's PR has actually merged into the feature branch (`gh pr view --json mergedAt --jq - .mergedAt`). -- Every `locked` contract row's `Realized in` PR is in fact merged. -- Every `## Bubble-up log` entry has a propagation path recorded — no concerns left unresolved. -- The `feature_branch` and `feature_worktree` frontmatter still point at real things on disk - (`git rev-parse --verify feature/` + `ls `). - -If anything is out of sync, fix the state file before continuing — the resumed-session contract depends on it. - -### 5. End-to-end verification on the feature branch - -**REQUIRED SUB-SKILL:** `superpowers:verification-before-completion`. Run the project-wide checks on the main feature -worktree (which holds the integration state — sub-worktrees only hold their own sub-branch): - -``` -cd -git pull origin feature/ -make test -make lint -``` - -If any sub-PR in the feature touched `frontend/`, also: - -``` -cd frontend && npm run typecheck -``` - -Paste the output. The feature branch must be green end to end before the integration PR opens — a sub-PR's isolated -CI passing doesn't guarantee the integration compiles, since each sub-PR's tests ran against its own branch state, -not the post-merge state. - -### 6. Synthesize the gap list - -Produce a short summary for the orchestrator (and the user, if this is a pre-integration-PR or pre-ready checkpoint): - -- **Drift found** — one bullet per discrepancy between spec/plan and what the diffs actually do. -- **Acceptance criteria uncovered** — one bullet per missing or rephrased criterion, with the classification from - Step 3. -- **State-file fixes** — one bullet per row corrected. -- **Verification status** — `make test` / `make lint` / typecheck pass/fail. - -Decide: - -- **All clean** → open the integration PR (or flip ready) per `developing-a-feature`. -- **Coverable by a follow-up sub-PR** → re-enter `developing-a-feature` Step 4 and dispatch a follow-up subagent into - a new sub-worktree. Update the state file with the new row. -- **Needs spec/plan refinement** → re-invoke `planning-a-feature` Steps 6/7 (write/update the plan, refine the - issues), surface the changes to the user, then continue. - -## Anti-patterns - -- **Skipping the check at phase transitions.** Each wave's drift compounds; surfacing it at the boundary is the - cheapest place to fix it. -- **Running verification on a sub-worktree instead of the main feature worktree.** Sub-worktrees only have their own - sub-branch checked out. The feature branch — where the integration shows up — lives in the main feature worktree. -- **Marking the integration PR ready without re-running this checkpoint after external feedback.** Reviewer-requested - changes can re-introduce drift the original review missed. -- **Treating the acceptance-criteria gap as a docs problem.** A missing criterion is either missing implementation or - a planning oversight. Don't silently delete it; classify and act. - -## Red flags - -| Thought | Reality | -| ------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------ | -| "The sub-PRs all passed CI, the integration PR will too" | Per-sub-PR CI runs against the sub-branch, not the feature branch. The integration is new state. | -| "The spec is old, no point checking against it" | Spec is the durable ADR. If it's stale, fix the spec — don't ignore it. | -| "Acceptance criteria mismatch is fine, the spirit is the same" | Acceptance criteria are checkable conditions. Either they're met or they're not. Update the issue if the criterion changed. | -| "I'll skip the state-file walk, I've been keeping it current" | The resumed-session contract is what the state file SAYS — verify it; don't trust your memory. | -| "make lint passed in my sub-worktree, no need to re-run on feature" | Lint is per-package; cross-package issues only surface on the integrated branch. | diff --git a/.claude/skills/testing-a-feature/SKILL.md b/.claude/skills/testing-a-feature/SKILL.md deleted file mode 100644 index 0dc4181..0000000 --- a/.claude/skills/testing-a-feature/SKILL.md +++ /dev/null @@ -1,118 +0,0 @@ ---- -name: testing-a-feature -description: - Use when writing tests for any non-trivial code change — before - deciding which assertions to add. ---- - -# testing-a-feature - -## When to invoke - -Whenever you write a test. Especially when: -- You just wrote a function and are about to "make sure it works." -- You're tempted to copy an assertion from an existing test "to match the style." -- You're about to rewrite a test because the implementation changed. - -Skip for generated test scaffolds where the assertions come straight from a tool you trust. - -## Core principle - -**Tests assert intent, not implementation.** The intent lives in the docstring, the function comment, the spec, the -issue's acceptance criteria — the public contract of the surface under test. The implementation is the body of the -function. - -When an implementation changes but the contract doesn't, the tests should not change. When the contract changes, the -tests change first (per TDD) and the implementation follows. - -This is what makes a docstring valuable: it's the **black box** the tests verify against. Without an intentional -docstring, you have nothing but the code to test against, and tests degrade into change-detectors. - -## Workflow - -### 1. Re-read the contract - -Before adding any assertion, open the surface under test and read its docstring / contract. If the docstring is -absent or vague, that's the first bug to fix — a function without a contract can't be tested against intent. - -For triagent specifically: -- Go exported functions: the godoc comment is the contract. -- Frontend exported hooks / utils: the JSDoc / type signature is the contract. -- MCP tools: the `Description:` field in `specs.go` (or `mcp.AddTool(...)` `Description`) is the contract for callers. -- Walkers / playbooks: the YAML `description:` field is the contract. - -### 2. Self-review the docstring against intent - -A clean docstring should already enumerate the surface's promises. If reading it raises questions ("what happens when -N is zero?", "does this retry on failure?", "is the result sorted?"), the docstring is incomplete. **Fix the -docstring first**, then write the tests. Docstring-first surfaces the edge cases before any assertion is written. - -### 3. List edge cases - -For each behavior the contract promises, ask: - -- **Happy path** — the documented success case. Always test. -- **Boundary inputs** — zero, one, many; empty string, single char, max length; nil vs empty slice. -- **Error paths** — every error the contract names. Every error the contract _doesn't_ name but the code clearly - can return (then either name it in the docstring or make the code not return it). -- **Invariants** — what should never happen, regardless of input. Concurrency safety, idempotency, atomicity. -- **Failure recovery** — partial failure mid-call, retry semantics, what state survives. - -Don't test for behavior the contract doesn't promise. If you're tempted to, the contract is incomplete — fix the -contract first. - -### 4. Write one test per edge case - -Each test name reads as a contract statement: `TestStore_PersistsMultipleSessionsAcrossRestart`, -`TestGetChannelID_MissingScopeIsActionable`. The name describes the intent being verified, not the function being -called. Each test's assertion is what the contract promises, not what the implementation happens to do today. - -### 5. When tempted to rewrite a test - -Stop. Ask: did the **contract** change, or just the implementation? - -- **Contract changed** → rewrite the test first, watch it fail for the right reason, then update the implementation. -- **Implementation changed** → the test should still pass. If it doesn't, either the test was coupled to - implementation details (bad test, fix it) or the implementation regressed the contract (bad change, revert it). - -Rewriting a test "to match" a new implementation when the contract is unchanged decouples the test from intent and -silently weakens the suite. - -## Edge-case discovery checklist - -Apply per surface, per change: - -- [ ] **Happy path** — documented success case. -- [ ] **Empty / zero / nil inputs** — what does the contract say happens? If it doesn't say, fix the contract. -- [ ] **Boundary values** — first / last / single-element / off-by-one neighbors. -- [ ] **Error paths named in the contract** — each one triggered and asserted. -- [ ] **Concurrency** — race detector clean (Go: `-race`); concurrent writers if the contract promises safety. -- [ ] **Idempotency** — does calling twice produce the same result the contract promises? -- [ ] **Persistence** — does on-disk / on-network state survive restart, if the contract says so? -- [ ] **Partial failure** — what state survives a mid-call error? - -## Anti-patterns - -- **Testing the implementation, not the intent.** A test that breaks when a private helper is renamed is testing - implementation, not contract. -- **Copy-pasting an assertion "to match the file's style"** without re-verifying that the asserted behavior is what - the contract under test actually promises. -- **One mega-test per function.** One assertion per intent. Mega-tests fail with one signal even when N intents are - broken. -- **Testing private functions directly.** If the contract is private, the test is testing implementation. Drive the - private code via the public surface. -- **Rewriting a test to make it pass against a changed implementation, without verifying the contract changed.** This - is how regressions slip through. -- **Skipping edge cases because "they're unlikely."** A reviewer asks "what happens when N=0?" — the test should - answer. - -## Red flags - -| Thought | Reality | -| ---------------------------------------------------------------- | -------------------------------------------------------------------------------------------------- | -| "The function is too small to test" | If it's worth writing, its contract is worth asserting. | -| "The test is fragile, let me loosen the assertion" | Fragile = coupled to implementation. Tighten the contract or rewrite the test against intent. | -| "I'll skip the nil case, the caller will always pass non-nil" | The contract should say "never call with nil" then. If it doesn't, test the nil case. | -| "The docstring is wrong but the implementation is right" | Fix the docstring (or the implementation, if the docstring is the source of truth) before testing. | -| "Rewriting the test to match the new code is faster" | And weakens the suite silently. Verify the contract changed first. | -| "Edge cases can wait for the next PR" | They get forgotten. List them now even if you don't write them all. | diff --git a/.claude/skills/writing-github-issues/SKILL.md b/.claude/skills/writing-github-issues/SKILL.md deleted file mode 100644 index 0143a77..0000000 --- a/.claude/skills/writing-github-issues/SKILL.md +++ /dev/null @@ -1,260 +0,0 @@ ---- -name: writing-github-issues -description: - Use when about to call `gh issue create` or `gh issue edit` against - sourcehawk/triagent, or immediately after concluding a brainstorming - session that needs an issue to hold the work. ---- - -# writing-github-issues - -## When to invoke - -Whenever you're about to file or edit an issue on `sourcehawk/triagent`, **or** immediately after a brainstorming -session lands a decision that needs an issue to hold the work. - -Three branches: - -- **No issue yet**: create one (§Step 2A). -- **Issue exists but is missing context**: update it (§Step 2B). -- **Issue exists and is sufficient**: no GitHub-side action (§Step 2C). - -## Core principle: user-in-the-loop for every GitHub mutation - -Don't create, modify, or link an issue without an explicit confirmation **for the specific body about to land**. This -applies even if the user said "file an issue" earlier in the conversation; generic intent earlier is not standing -consent for the specific body now. - -By default, assign the user to any issue created or touched (`--assignee @me`, which `gh` resolves to the authenticated -user). Surface the assignment in the same confirmation prompt; if they decline, note it and move on. - -Every confirmation shows the user: - -- The exact target (`sourcehawk/triagent`, or `sourcehawk/triagent#` for updates). -- The full proposed body. -- Which label(s) will be attached. -- Whether you intend to assign them (default: yes). - -Wait for an explicit "yes" before any `gh issue create / edit` or sub-issue-linkage call. Treat absence of objection as -a no. - -GitHub doesn't render HTML comments, so leaving the template guidance in place is harmless — don't burn a step -removing it. - -## Step 1: pick the template - -Three templates live under `templates/`. The choice is a single judgment call — make it explicit; don't default to -the smallest shape. - -``` -Is the work one self-contained change that ships in one PR? -├─ No → It spans several feature-sized chunks ─────────────────→ EPIC -└─ Yes → Is it fixing unintended behaviour (regression, broken contract)? - ├─ Yes ────────────────────────────────────────────────→ BUG - └─ No → Is the change user-observable (new flag, UI, - API surface)? - ├─ Yes ─────────────────────────────────────────→ FEATURE (label: feature) - └─ No (plumbing, refactor, test work) ─────────→ FEATURE template, label: task -``` - -For brainstorm output specifically: ask whether the outcome spans multiple feature-sized chunks. If yes, file an -**epic** plus one **feature/task/bug** per chunk; link each child as a sub-issue. If no, file a single **feature** -(or bug) and break it into PR-sized phases inside the Approach section. - -**Tiebreaker for feature vs task** (when work touches operator-observable state but reads mostly as plumbing): ask -whether the headline change a no-context reader would describe is user-observable. Yes → `feature`. No → `task`. -Example: a launcher-state migration that changes the on-disk layout is `task` (the headline is "refactor the layout"); -the switcher UI that lets the operator pick a profile is `feature` (the headline is "you can now pick a profile"). - -| Template | When | Label | -| ----------------- | ------------------------------------------------------------------------------------ | ------------------ | -| `epic.md` | Multi-chunk work; parent of feature/task/bug sub-issues | `epic` | -| `feature.md` | One self-contained capability or refactor | `feature` or `task`| -| `bug.md` | Unintended behaviour; broken contract; regression | `bug` | - -## Step 2A: No issue yet (create) - -1. **Draft the issue body** from the chosen template. Each section earns its keep — read the `` guidance in - the template for what belongs where. The cross-template common shape: - - **Title**: human-readable sentence a no-context reader can parse. - - **Problem**: a few sentences opening with the elevator pitch (the what) and naming the operational reason it - matters (the why). No solution; that belongs in the PR description (bug) or in Approach / Design overview - (feature / epic). - - Template-specific sections: see the relevant template file. - -2. **Confirm with the user, with the body inline.** Paste the drafted body into chat under a "About to create a - `