diff --git a/skills/developing-a-feature/SKILL.md b/skills/developing-a-feature/SKILL.md index 640a5d9..eac7322 100644 --- a/skills/developing-a-feature/SKILL.md +++ b/skills/developing-a-feature/SKILL.md @@ -29,13 +29,13 @@ If the plan is missing, stale, or the state file's recorded state doesn't match ### 2. Decide: single-PR or multi-PR (feature-branch model) -- **Single PR** → one worktree on the `feature/` branch `feature-dev-workflow:planning-a-feature` created, one Claude session, one PR from it targeting main. Skip the integration-PR step at the end. +- **Single PR** → one worktree on the `/` branch `feature-dev-workflow:planning-a-feature` created, one Claude session, one PR from it targeting main. Skip the integration-PR step at the end. - **Multi-PR** → two sub-models, selected by the Sub-PR target model question below. The default is the **feature-branch model**: - - `feature-dev-workflow: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. + - `feature-dev-workflow:planning-a-feature` already created `/` (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 `/`, not main. Each sub-worktree is created off the feature branch with `git worktree add .claude/worktrees/-- -b /-- /` (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 `/`. 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. + - When every sub-PR has been self-merged into the feature branch, the orchestrator opens the **integration PR** `/` → `main`, with `Closes #` in its body, for external review and the final merge. The alternative sub-model — sub-PRs targeting main directly, no integration PR — is configured by the Sub-PR target model question below. 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. @@ -56,34 +56,34 @@ Record the choice as `sub_pr_review_loop: on` or `sub_pr_review_loop: off`. The **Sub-PR target model (multi-PR only).** Immediately after the review-loop choice, ask a third `AskUserQuestion`: should sub-PRs target the feature branch (with one integration PR to main at the end) or main directly (with each sub-PR reviewed and merged independently)? -- **Feature branch (default)** — sub-PRs target `feature/`; the orchestrator self-merges each one into the feature branch; the feature culminates in a single integration PR (`feature/` → `main`) with `Closes #` that closes the epic and gives external reviewers a combined diff. Choose this when sub-PRs are coupled by shared contracts and a unified review surface matters. +- **Feature branch (default)** — sub-PRs target `/`; the orchestrator self-merges each one into the feature branch; the feature culminates in a single integration PR (`/` → `main`) with `Closes #` that closes the epic and gives external reviewers a combined diff. Choose this when sub-PRs are coupled by shared contracts and a unified review surface matters. - **Directly to main** — sub-PRs target `main` directly; each is reviewed and merged on its own timeline; no integration PR at the end. Choose this when sub-PRs are truly independent and individually deliverable, and a combined review surface isn't needed. Record the choice as `sub_pr_target: feature-branch` or `sub_pr_target: main`. The fan-out skill reads this field to set the base ref for sub-worktrees and the `--base` flag for each PR. Default if the field is missing in an older state file: `feature-branch` (preserves the original behaviour). ### 3. Set up the implementation environment -- **Multi-PR (feature-branch model)** — `feature/` already exists, created and pushed by `feature-dev-workflow: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: +- **Multi-PR (feature-branch model)** — `/` already exists, created and pushed by `feature-dev-workflow: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 '/' 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/ + git switch main # vacate / if planning left you on it + git worktree add .claude/worktrees/ / 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 `feature-dev-workflow:fanning-out-with-worktrees`. +(Fallback only if planning was skipped and `/` exists nowhere: `git worktree add .claude/worktrees/ -b / origin/main && git -C .claude/worktrees/ push -u origin /`.) Update the state file's `feature_branch` + `feature_worktree` frontmatter fields to point here. Sub-worktrees off this branch are created later by `feature-dev-workflow:fanning-out-with-worktrees`. -- **Single-PR** — `feature-dev-workflow: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: +- **Single-PR** — `feature-dev-workflow:planning-a-feature` created `/` 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/ + git switch main # vacate / if planning left you on it + git worktree add .claude/worktrees/ / 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. +(Fallback if planning was skipped: `git worktree add .claude/worktrees/ -b / origin/main`.) Skip the integration-PR step at the end; this is the only PR. ### 4. Implement @@ -114,8 +114,8 @@ Record the choice as `sub_pr_target: feature-branch` or `sub_pr_target: main`. T **REQUIRED SUB-SKILL:** `feature-dev-workflow: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 (feature-branch) 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. +- **Single-PR feature** → PR targets `main` from `/`. Body opens with `Fixes #` (bug) or `Closes #` (feature/task) so the issue auto-closes on merge. +- **Multi-PR (feature-branch) integration PR** → PR targets `main` from `/` (`gh pr create --base main --head /`). Body opens with `Closes #` so the epic auto-closes on merge. This is the PR external reviewers see; the diff is the whole feature. - **Multi-PR (directly to main)** → there is no integration PR and nothing to open here. Epic closure already happened at fan-out hand-back — `feature-dev-workflow:fanning-out-with-worktrees` Step 7 owns the `gh issue close ` (sub-PR keywords only close sub-issues, never the epic). Verify it with `gh issue view --json state`, then proceed to Step 7. Sub-PRs into the feature branch are owned by `feature-dev-workflow:fanning-out-with-worktrees`, not this step. @@ -175,7 +175,7 @@ The teardown does not change where the flow ends. In the models that end in a fi | "They wrote 'merge it / merge to the main worktree' earlier — that's authorization" | Merge-ish phrasing in passing isn't the merge button; it usually means "bring the result into the local checkout and verify it". Stop at ready-to-merge; if they want you to merge, they'll say so against the open PR. | | "`Closes #` is in the body — merging just completes the design" | The keyword describes what happens when the *user* merges. It is not an instruction to merge. | | "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. | +| "I'll create `/` off `origin/main` in step 3" | Planning already created it and committed the spec/plan/state onto it. `-b /` 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" | When CI runs on the PR, local green and "ready" aren't the gate — if it comes back red you fix forward, with no state file if you deleted it. Tear down on the PR's checks going green. (Repo has no CI configured for this branch? Then the local suite *is* the gate — proceed.) | | "The spec stays — it's the durable record" | Only in repos with no decision-record convention of their own. Where `docs/adrs/` (or equivalent) exists, distill the lasting decisions into it — with user review — and delete the spec with the other scratch artifacts. The canonical record is what future engineers actually read. | | "`gh pr checks` reports no checks, so I'll keep polling until CI shows up" | Zero checks reported isn't the same as CI pending. Inspect the repo's CI configuration: if no pipeline runs on this branch, none will ever appear and polling just stalls the workflow. Proceed on the local suite you already pasted. | diff --git a/skills/fanning-out-with-worktrees/SKILL.md b/skills/fanning-out-with-worktrees/SKILL.md index 03c12bf..f6da13d 100644 --- a/skills/fanning-out-with-worktrees/SKILL.md +++ b/skills/fanning-out-with-worktrees/SKILL.md @@ -12,7 +12,7 @@ description: 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 `feature-dev-workflow:developing-a-feature` before invoking this skill). +- The `/` integration branch and the main feature worktree exist (set up by `feature-dev-workflow: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 plan also has a `## Conventions` block (directory layout, naming scheme, locked vocabulary) — it's mandatory dispatch context for every subagent (Step 2), so the fan-out can't start without it. - The state file (`docs/superpowers/states/--state.md`) has rows for each sub-issue. @@ -41,10 +41,14 @@ Record the wave assignments in the state file's `## Phases` section before dispa For each sub-PR in the wave, the orchestrator creates the worktree first: ``` -git worktree add .claude/worktrees/-- -b +git worktree add .claude/worktrees/-- -b /-- ``` -When the state file's frontmatter has `sub_pr_target: main`, `` is `origin/main` for every sub-PR in every wave (fetch `origin/main` first so wave N+1 picks up the commits that wave N merged). When `sub_pr_target` is `feature-branch` (the default), `` 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). +**The sub-branch name is derived, not invented.** It is `/--`: the parent's slug, so every branch of one feature sorts together in the remote's branch list, then `--` for this slice. `` is the commit type of *this* sub-PR's headline change, which is often not the parent's — a feature branch's slices routinely include a `refactor` that clears the way and a `test` that backfills coverage. The `` follows the same rule as the parent slug (`feature-dev-workflow:planning-a-feature` step 2): it names the slice's subject and effect in two to four words, never `part-2`, `phase-b`, or the plan's organizing label — those are positions in a plan the reader does not have, and inheriting them violates the naming firewall in `feature-dev-workflow:maintaining-architectural-coherence`. + +Record the branch in the state file's PR-table row as you create it; the resumed-session contract in `feature-dev-workflow:resuming-a-feature` reads that row as ground truth. + +When the state file's frontmatter has `sub_pr_target: main`, `` is `origin/main` for every sub-PR in every wave (fetch `origin/main` first so wave N+1 picks up the commits that wave N merged). When `sub_pr_target` is `feature-branch` (the default), `` is `/` for default sub-PRs, `/` 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`. @@ -53,11 +57,11 @@ 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, the relevant contract row(s) (Name + Producer + Consumer + Shape + Realization), **and the plan's `## Conventions` block**. The subagent implements **against the contract and the conventions** — it does not re-discover or re-design either, and it does not invent its own directory layout or naming scheme. A subagent handed contracts but not conventions will name and structure locally, and the merged feature reads as written by a committee (see `feature-dev-workflow:maintaining-architectural-coherence`). 3. **Implementation skills.** `superpowers:test-driven-development` + `feature-dev-workflow:testing-a-feature` for every change. -4. **PR completion.** When the implementation is done and verified, the subagent invokes `feature-dev-workflow:opening-a-pull-request`. The base and body keyword depend on `sub_pr_target` in the state file: when `feature-branch`, use `--base feature/` and `Towards #` in the body (`Fixes`/`Closes` don't fire on non-default-branch merges; `Towards` keeps the issue open until the orchestrator closes it manually after the merge); when `main`, use `--base main` and the type-appropriate closing keyword in the body — `Fixes #` for a bug sub-issue, `Closes #` otherwise; either fires automatically on merge to the default branch (see `feature-dev-workflow:opening-a-pull-request` for the distinction). Epic closure in main mode is not handled by sub-PR keywords — the orchestrator closes the epic manually in Step 7. The subagent reports the PR URL back to the orchestrator. +4. **PR completion.** When the implementation is done and verified, the subagent invokes `feature-dev-workflow:opening-a-pull-request`. The base and body keyword depend on `sub_pr_target` in the state file: when `feature-branch`, use `--base /` and `Towards #` in the body (`Fixes`/`Closes` don't fire on non-default-branch merges; `Towards` keeps the issue open until the orchestrator closes it manually after the merge); when `main`, use `--base main` and the type-appropriate closing keyword in the body — `Fixes #` for a bug sub-issue, `Closes #` otherwise; either fires automatically on merge to the default branch (see `feature-dev-workflow:opening-a-pull-request` for the distinction). Epic closure in main mode is not handled by sub-PR keywords — the orchestrator closes the epic manually in Step 7. 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/` or `# → main`, per the `sub_pr_target` setting) and flips status to `draft`. +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 (`# → /` or `# → main`, per the `sub_pr_target` setting) and flips status to `draft`. A stale row is worse than no row — a resumed session reads the state file as ground truth. @@ -89,7 +93,7 @@ When a sub-PR is ready (subagent reports `ready` and the relevant verification c - **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. +- **Worktree topology.** Subagents live in their per-sub-PR worktrees; only the orchestrator's main feature worktree has `/` checked out, so the merge naturally happens on the orchestrator's side. Per sub-PR, in order: @@ -101,9 +105,9 @@ Per sub-PR, in order: 3. **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 4-6. The user opted into the mechanical bundle (review → merge → bodyless close → state update) in `feature-dev-workflow:developing-a-feature` Step 2. - **`manual`** — pause and ask the user 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. -4. **Merge.** Run `gh pr merge --merge` (or `--squash` / `--rebase` per project preference). Before merging, push any local state-file commits to their remote — for `feature-branch` that is `origin/feature/`; for `main` that is `origin/feature/` as well (the orchestrator stays on the feature branch for state management). The merge itself lands on GitHub's remote, and the pull-back differs by target: - - **`sub_pr_target: feature-branch`**: after the merge, `git -C fetch origin && git merge --ff-only origin/feature/` to bring the merge commit back into the feature worktree. Keep local == origin at every merge boundary (unpushed local commits cause a "Not possible to fast-forward" failure; recover with `git rebase origin/feature/`). - - **`sub_pr_target: main`**: the merge lands on `main`. The orchestrator remains on `feature/` for state file management; no pull-back into the feature worktree is needed. Before dispatching wave N+1, run `git fetch origin` so the next wave's worktrees branch from the freshly updated `origin/main`. +4. **Merge.** Run `gh pr merge --merge` (or `--squash` / `--rebase` per project preference). Before merging, push any local state-file commits to their remote — for `feature-branch` that is `origin//`; for `main` that is `origin//` as well (the orchestrator stays on the feature branch for state management). The merge itself lands on GitHub's remote, and the pull-back differs by target: + - **`sub_pr_target: feature-branch`**: after the merge, `git -C fetch origin && git merge --ff-only origin//` to bring the merge commit back into the feature worktree. Keep local == origin at every merge boundary (unpushed local commits cause a "Not possible to fast-forward" failure; recover with `git rebase origin//`). + - **`sub_pr_target: main`**: the merge lands on `main`. The orchestrator remains on `/` for state file management; no pull-back into the feature worktree is needed. Before dispatching wave N+1, run `git fetch origin` so the next wave's worktrees branch from the freshly updated `origin/main`. 5. **Close the sub-issue.** - **`sub_pr_target: feature-branch`**: `gh issue close `. 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. - **`sub_pr_target: main`**: no manual close needed. The closing keyword in the PR body (`Fixes`/`Closes`) fires on merge to the default branch and auto-closes the sub-issue. Confirm it closed before marking the state-file row `self-merged`. @@ -121,7 +125,7 @@ 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 `feature-dev-workflow:developing-a-feature`. -- **`sub_pr_target: feature-branch`**: the next step is the integration PR (`feature/` → `main` with `Closes #`), which `feature-dev-workflow:developing-a-feature` Step 6 owns — opening and review-looping only; the merge to main itself is the user's (see that skill's merge guard). +- **`sub_pr_target: feature-branch`**: the next step is the integration PR (`/` → `main` with `Closes #`), which `feature-dev-workflow:developing-a-feature` Step 6 owns — opening and review-looping only; the merge to main itself is the user's (see that skill's merge guard). - **`sub_pr_target: main`**: the sub-PRs were already the deliverables to main; there is no integration PR. `feature-dev-workflow:developing-a-feature` passes through Step 6 (verification only — no PR to open) and proceeds to Step 7 (teardown). Close the epic manually with `gh issue close ` before handing back — sub-PR closing keywords (`Fixes`/`Closes`) only close sub-issues, so the epic does not auto-close. ## Anti-patterns diff --git a/skills/opening-a-pull-request/SKILL.md b/skills/opening-a-pull-request/SKILL.md index 95058f9..b1601eb 100644 --- a/skills/opening-a-pull-request/SKILL.md +++ b/skills/opening-a-pull-request/SKILL.md @@ -33,6 +33,17 @@ Set the title once when opening and don't rename it. Match the project's commit- Types: `feat`, `fix`, `refactor`, `test`, `chore`, `docs`. Area mirrors the module path (`api`, `auth`, `ui`, `db`). 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. +**The summary names the effect or the cause, whichever a human recognizes faster.** Choose one: + +| Shape | Reads as | Use when | +| --- | --- | --- | +| Impact-first | `fix(api): stop stale sessions from blocking every newer one` | The reader cares what starts or stops happening. | +| Cause-first | `fix(api): expire sessions whose backend lookup never answers` | The trigger is the surprising part and names the bug better than its effect does. | + +If both read equally well, take impact-first. The title is the squash-merge subject and the line that appears in every notification, release note, and `git log`, read by people who know the codebase but have never seen this diff. Keep the whole thing inside roughly 72 characters so it survives those surfaces without truncation — if it won't fit, the summary is carrying detail that belongs in the body. + +The failure to avoid is the third shape, the mechanic: `fix(api): move the expiry check above the backend lookup`. It names the code motion that achieved the fix. It is accurate, it satisfies every part of the convention above, and it still leaves the reader unable to tell whether the PR matters to them — because the code motion is the one thing the diff already shows. Check the drafted summary against this: does it say what changed **for the system**, or only what changed **in the code**? A title whose meaning only arrives once the diff is open has failed. + **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 @@ -46,7 +57,7 @@ When the PR has a tracking issue, link it as the **first line of the body's open 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. This includes sub-PRs in the `sub_pr_target: main` model (see `feature-dev-workflow:fanning-out-with-worktrees`): each sub-PR targets main directly with the type-appropriate closing keyword (`Fixes #` for a bug sub-issue, `Closes #` otherwise); the epic is closed manually by the orchestrator after all sub-PRs merge. -- **PR targets a feature branch** (`feature/` in the multi-PR feature-branch model — see `feature-dev-workflow: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. +- **PR targets a feature branch** (`/` in the multi-PR feature-branch model — see `feature-dev-workflow: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 (`/` → `main`) gets `Closes #` because that PR does merge to main. - **PR temporarily targets a sibling branch and will be retargeted to `main`** (stacked sub-PRs in the `sub_pr_target: main` model: each draft opens against its parent branch so its diff shows only its own commits, then retargets to `main` when the parent merges) — use the type-appropriate closing keyword (`Fixes #` for a bug sub-issue, `Closes #` otherwise) from the start. It describes the merge that will eventually happen on `main`, and it survives the retarget with no body edit; a `Towards` placed "because the base isn't main yet" has to be remembered and upgraded at every retarget, and a forgotten upgrade means the sub-issue never auto-closes. Know what the keyword does NOT do while the base is a sibling branch: GitHub only creates the issue's linked-PR association (and only auto-closes) for closing keywords on default-branch-base PRs, so until the retarget the sub-issue shows a plain timeline mention and **no linked PR** — deferred, not broken. At each retarget, verify the linkage materialized (`gh pr view --json closingIssuesReferences` lists the sub-issue); if it stays empty, re-save the body so GitHub re-evaluates the keyword against the new base. If there is no tracking issue, drop the line entirely and open the section with prose. @@ -89,6 +100,8 @@ The mechanic: ## 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. +- **Mechanic-first titles** (`... move the check above the lookup`, `... reorder the gates`, `... extract the handler`). They describe the code motion, which is the one thing the diff already shows. Lead with the effect or the cause (§PR title). +- **`Challenges` as the body's overflow section.** Consequences, tradeoffs, follow-up work, and the history of review rounds all get filed there because the heading is sitting in the template. Each has a home: the Description's third part, `Related`, or the commits. The section earns its place only when the diff hides a system fact a reviewer needs — otherwise the heading gets deleted. - **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. @@ -106,5 +119,8 @@ These thoughts mean the PR isn't actually ready to publish or flip: | "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. | | "The PR's already open/ready, the stale body isn't worth re-editing" | The body is what the reviewer reads first; once the diff moves past it, it misleads. Reconcile the body to match the diff — body only, no comment (§Reconciling an open PR's body with reality). | | "The sub-issue shows no linked PR, the keyword must be wrong" | Closing-keyword linkage only materializes while the PR's base is the default branch. On a stacked draft it's deferred until retarget — verify with `gh pr view --json closingIssuesReferences` then, not before. | +| "The title is accurate and follows the convention, so it's fine" | Accuracy is not the bar; recognition is. If the summary names the code motion, a reader can't tell whether the PR matters to them without opening the diff. Lead with the effect or the cause. | +| "This part was genuinely hard, so it belongs in `Challenges`" | Hard for the author is not the test. The test is whether the diff hides a system fact the reviewer needs. Difficulty you already resolved, review rounds, and wrong turns live in the commits. | +| "Better to include it than leave the reviewer guessing" | A reviewer's attention is finite and spending it on filler costs the sections that matter. Every fact needs a section that's actually for it, or it comes out. | 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/skills/opening-a-pull-request/templates/pull-request-ready.md b/skills/opening-a-pull-request/templates/pull-request-ready.md index 72e727a..2d6f613 100644 --- a/skills/opening-a-pull-request/templates/pull-request-ready.md +++ b/skills/opening-a-pull-request/templates/pull-request-ready.md @@ -13,7 +13,12 @@ Open with ONE of these as the FIRST line of this section, depending on what shou - `Towards #` — the PR contributes to the issue but does NOT auto-close it; the issue stays open. Used for sub-PRs into a feature branch (where the orchestrator closes the sub-issue manually after self-merge) and for any other "in progress on this, not finishing it" case. Omit the line entirely if there is no tracking issue. -Then follow with a short, human-readable summary accessible to a dev coming in without context. Not overly technical. Two-to-four sentences answering: what does this PR achieve, and why now. Lean on Changes (below) for the "what specifically". +Then the summary, at most four sentences, in this order: + 1. The effect, or the cause — the same one the title leads with. A reader who knows the codebase but not this work should be able to stop after this sentence and know whether the PR concerns them. + 2. What this PR does about it. + 3. The consequence, when this PR leaves one behind — a new failure mode, an alert that will start firing, a case it deliberately does not fix, work deferred to a follow-up. One sentence, and it belongs HERE, not further down: it is the part a reviewer most needs before approving, and the Description is the only section they are guaranteed to read. + +Write it for a dev arriving without context. Lean on Changes (below) for the "what specifically" — everything a reviewer needs that doesn't fit those three parts has a section of its own further down. --> ## Changes @@ -25,13 +30,26 @@ Over-arching changes that affect behavior or user-visible surface. Don't list "r ## Challenges ## Related ## Testing diff --git a/skills/planning-a-feature/SKILL.md b/skills/planning-a-feature/SKILL.md index d944693..c6a5c6d 100644 --- a/skills/planning-a-feature/SKILL.md +++ b/skills/planning-a-feature/SKILL.md @@ -40,18 +40,37 @@ Both the spec and any ADR(s) are tracked source artifacts (`docs/superpowers/spe ``` git fetch origin main -git switch -c feature/ origin/main +git switch -c / 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 +git worktree add .claude/worktrees/ -b / 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. `feature-dev-workflow: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. +Either way, stay on `/` 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. `feature-dev-workflow: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. + +**The branch name has two parts, and both carry information.** + +`` is the commit type of the headline change — `feat`, `fix`, `refactor`, `test`, `chore`, `docs` — the same type the PR title will carry. A bug fix lives on `fix/…`, not on `feat/…` and not on a generic `feature/…`; whoever scans `git branch` or the remote's branch list learns what kind of work this is before reading further. Pick the type from the change, not from the workflow that produced it: running this feature-development flow does not make the work a feature. + +`` names the subject and what changes about it, in two to four words. The subject alone is not enough, and the code motion that achieves it is the wrong half — that is what the diff shows. + +**The slug is only the part after the slash, and it never contains one.** The type prefix belongs to the branch name and nothing else: `` on its own is interpolated into the worktree path (`.claude/worktrees/`) and into every artifact filename (`docs/superpowers/specs/YYYY-MM-DD--design.md`), so a slug carrying its own type would nest the worktree a directory deeper and put a slash in a filename. For a branch `fix/stale-sessions-block-new-ones`, the slug is `stale-sessions-block-new-ones`. + +| Branch name | Verdict | +| --- | --- | +| `fix/stale-sessions-block-new-ones` | Names the subject and its effect. A reader knows what is wrong. | +| `feat/multi-tenant-profiles` | Good for a feature: the subject *is* the change. | +| `fix/session-controller-work` | Subject with no effect. Says nothing a reader can act on. | +| `fix/reorder-expiry-check` | The mechanic. Describes the patch, not the problem. | +| `feature/session-fix` | Wrong type, and the slug is a placeholder. | +| `fix/fix-stale-sessions` | The type is duplicated into the slug. The slug is `stale-sessions…`, not `fix-…`. | + +The name is set once, at birth, and every sub-branch, worktree path, and state-file row inherits it, so a vague slug is expensive to live with and awkward to change later. ### 3. User reviews the spec (and any ADR) @@ -62,7 +81,7 @@ Pause. Surface the spec path and wait for explicit "approved" or redirection bef 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). +- **Multiple PRs** — multiple feature-sized chunks, each independently reviewable, possibly parallelizable. Multi-PR features land via the **feature-branch model**: a long-lived `/` branch off main; every sub-PR is a real GitHub PR targeting `/` (not main); when every sub-PR has been self-merged into the feature branch, a final **integration PR** from `/` 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. @@ -112,16 +131,16 @@ Before handing off, create `docs/superpowers/states/YYYY-MM-DD--state.md` 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 `feature-dev-workflow: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 `feature-dev-workflow:developing-a-feature` can attach its integration worktree to it: +Commit the spec, the plan, and the state file together as the planning artifact set on `/` (created in step 2). Confirm you're not on `main`, then publish the branch so `feature-dev-workflow:developing-a-feature` can attach its integration worktree to it: ``` -git branch --show-current # must be feature/, never main -git push -u origin feature/ +git branch --show-current # must be /, never main +git push -u origin / ``` ### 9. Hand off to implementation -Spec + plan + state file committed and pushed on `feature/`, issues aligned, contracts written → invoke `feature-dev-workflow:developing-a-feature` to start the work. The state file is the entry-point artifact for every session that touches this feature afterward. +Spec + plan + state file committed and pushed on `/`, issues aligned, contracts written → invoke `feature-dev-workflow: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 @@ -142,4 +161,6 @@ Spec + plan + state file committed and pushed on `feature/`, issues aligne | "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 `feature-dev-workflow: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; `feature-dev-workflow:developing-a-feature` reuses it. Nothing about the feature touches `main` except the final integration/feature PR. | +| "The feature branch is created later in `feature-dev-workflow:developing-a-feature`, so the spec/plan/state commit to `main` first" | Planning owns the branch's birth. Create `/` in step 2 before the first commit; `feature-dev-workflow:developing-a-feature` reuses it. Nothing about the feature touches `main` except the final integration/feature PR. | +| "This is running the feature-development flow, so the branch is `feature/…`" | The type comes from the change, not from the workflow. A bug fix planned through this flow is still `fix/…`. | +| "The slug can be refined once I see how the work lands" | Every sub-branch, worktree path, and state-file row derives from it. Name the subject and its effect now (step 2); renaming later touches all of them. | diff --git a/skills/planning-a-feature/templates/feature-state.md b/skills/planning-a-feature/templates/feature-state.md index f4f1057..fd2fd2b 100644 --- a/skills/planning-a-feature/templates/feature-state.md +++ b/skills/planning-a-feature/templates/feature-state.md @@ -11,10 +11,10 @@ feature: spec: docs/superpowers/specs/YYYY-MM-DD--design.md plan: docs/superpowers/plans/YYYY-MM-DD--plan.md tracking_issue: # -feature_branch: feature/ # omit for single-PR features +feature_branch: / # 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: # # filled in once the feature → main PR opens +integration_pr: # # filled in once the / → main PR opens status: planning --- @@ -32,16 +32,16 @@ Implementation phases as the plan defines them. Each phase names the sub-issues ## PRs / worktrees -| Issue | Branch | Worktree path | PR (→ base) | Status | -| --------------------------- | ---------------------------- | ---------------------------------------- | ------------------------------------ | ------------- | -| # | | .claude/worktrees/-- | # → feature/ | not-started | +| Issue | Branch | Worktree path | PR (→ base) | Status | +| --------------------------- | ----------------------------- | ---------------------------------------- | ------------------------------------ | ------------- | +| # | /-- | .claude/worktrees/-- | # → / | not-started | ## Contracts diff --git a/skills/reviewing-feature-progress/SKILL.md b/skills/reviewing-feature-progress/SKILL.md index cba44f7..b107061 100644 --- a/skills/reviewing-feature-progress/SKILL.md +++ b/skills/reviewing-feature-progress/SKILL.md @@ -73,7 +73,7 @@ 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 `). +- The `feature_branch` and `feature_worktree` frontmatter still point at real things on disk (`git rev-parse --verify /` + `ls `). If anything is out of sync, fix the state file before continuing — the resumed-session contract depends on it. @@ -87,7 +87,7 @@ With the feature whole and its behavior settled, write or update the public-faci ``` cd -git pull origin feature/ +git pull origin / # then run the project's full test + lint suite (and typecheck, if it has one), # discovered from the project's CLAUDE.md / AGENTS.md or build config ``` diff --git a/skills/writing-github-issues/SKILL.md b/skills/writing-github-issues/SKILL.md index abecbf8..50d4a96 100644 --- a/skills/writing-github-issues/SKILL.md +++ b/skills/writing-github-issues/SKILL.md @@ -183,7 +183,7 @@ When implementation diverges from what the tracking issue currently states — a **Which issue.** Single-PR feature → the one tracking issue. Multi-PR → the **sub-issue** when the change is scoped to that sub-PR's work; the **epic** when the change is to the design-overview-level shape the epic carries. -**Commit it first.** The record links the commit that embodies the change, so the change must be committed and pushed before this branch runs. In a multi-PR feature that commit is on `feature/`, which is pushed and linkable on GitHub before the feature merges. +**Commit it first.** The record links the commit that embodies the change, so the change must be committed and pushed before this branch runs. In a multi-PR feature that commit is on `/`, which is pushed and linkable on GitHub before the feature merges. The record and the reconcile are one logical change, confirmed together: