From 5dade1757269342223241f2f86fe1c44505632d4 Mon Sep 17 00:00:00 2001 From: Drew Ritter Date: Mon, 6 Apr 2026 14:30:58 -0700 Subject: [PATCH] feat: rewrite finishing-a-development-branch with detect-and-defer (PRI-974) Step 2: environment detection (GIT_DIR != GIT_COMMON) before presenting menu Detached HEAD: reduced 3-option menu (no merge from detached HEAD) Provenance-based cleanup: .worktrees/ = ours, anything else = hands off Bug #940: Option 2 no longer cleans up worktree Bug #999: merge -> verify -> remove worktree -> delete branch Bug #238: cd to main repo root before git worktree remove Stale worktree pruning after removal (git worktree prune) Co-Authored-By: Claude Opus 4.6 (1M context) --- .../finishing-a-development-branch/SKILL.md | 122 +++++++++++++----- 1 file changed, 90 insertions(+), 32 deletions(-) diff --git a/skills/finishing-a-development-branch/SKILL.md b/skills/finishing-a-development-branch/SKILL.md index c308b43b..00a80aca 100644 --- a/skills/finishing-a-development-branch/SKILL.md +++ b/skills/finishing-a-development-branch/SKILL.md @@ -9,7 +9,7 @@ description: Use when implementation is complete, all tests pass, and you need t Guide completion of development work by presenting clear options and handling chosen workflow. -**Core principle:** Verify tests → Present options → Execute choice → Clean up. +**Core principle:** Verify tests → Detect environment → Present options → Execute choice → Clean up. **Announce at start:** "I'm using the finishing-a-development-branch skill to complete this work." @@ -37,7 +37,24 @@ Stop. Don't proceed to Step 2. **If tests pass:** Continue to Step 2. -### Step 2: Determine Base Branch +### Step 2: Detect Environment + +**Determine workspace state before presenting options:** + +```bash +GIT_DIR=$(cd "$(git rev-parse --git-dir)" 2>/dev/null && pwd -P) +GIT_COMMON=$(cd "$(git rev-parse --git-common-dir)" 2>/dev/null && pwd -P) +``` + +This determines which menu to show and how cleanup works: + +| State | Menu | Cleanup | +|-------|------|---------| +| `GIT_DIR == GIT_COMMON` (normal repo) | Standard 4 options | No worktree to clean up | +| `GIT_DIR != GIT_COMMON`, named branch | Standard 4 options | Provenance-based (see Step 6) | +| `GIT_DIR != GIT_COMMON`, detached HEAD | Reduced 3 options (no merge) | No cleanup (externally managed) | + +### Step 3: Determine Base Branch ```bash # Try common base branches @@ -46,9 +63,9 @@ git merge-base HEAD main 2>/dev/null || git merge-base HEAD master 2>/dev/null Or ask: "This branch split from main - is that correct?" -### Step 3: Present Options +### Step 4: Present Options -Present exactly these 4 options: +**Normal repo and named-branch worktree — present exactly these 4 options:** ``` Implementation complete. What would you like to do? @@ -61,30 +78,43 @@ Implementation complete. What would you like to do? Which option? ``` +**Detached HEAD — present exactly these 3 options:** + +``` +Implementation complete. You're on a detached HEAD (externally managed workspace). + +1. Push as new branch and create a Pull Request +2. Keep as-is (I'll handle it later) +3. Discard this work + +Which option? +``` + **Don't add explanation** - keep options concise. -### Step 4: Execute Choice +### Step 5: Execute Choice #### Option 1: Merge Locally ```bash -# Switch to base branch +# Get main repo root for CWD safety +MAIN_ROOT=$(git -C "$(git rev-parse --git-common-dir)/.." rev-parse --show-toplevel) +cd "$MAIN_ROOT" + +# Merge first — verify success before removing anything git checkout - -# Pull latest git pull - -# Merge feature branch git merge # Verify tests on merged result -# If tests pass +# Only after merge succeeds: remove worktree, then delete branch +# (See Step 6 for worktree cleanup) git branch -d ``` -Then: Cleanup worktree (Step 5) +Then: Cleanup worktree (Step 6) #### Option 2: Push and Create PR @@ -103,7 +133,7 @@ EOF )" ``` -Then: Cleanup worktree (Step 5) +**Do NOT clean up worktree** — user needs it alive to iterate on PR feedback. #### Option 3: Keep As-Is @@ -127,36 +157,46 @@ Wait for exact confirmation. If confirmed: ```bash -git checkout +MAIN_ROOT=$(git -C "$(git rev-parse --git-common-dir)/.." rev-parse --show-toplevel) +cd "$MAIN_ROOT" +``` + +Then: Cleanup worktree (Step 6), then force-delete branch: +```bash git branch -D ``` -Then: Cleanup worktree (Step 5) +### Step 6: Cleanup Workspace -### Step 5: Cleanup Worktree +**Only runs for Options 1 and 4.** Options 2 and 3 always preserve the worktree. -**For Options 1, 2, 4:** - -Check if in worktree: ```bash -git worktree list | grep $(git branch --show-current) +GIT_DIR=$(cd "$(git rev-parse --git-dir)" 2>/dev/null && pwd -P) +GIT_COMMON=$(cd "$(git rev-parse --git-common-dir)" 2>/dev/null && pwd -P) +WORKTREE_PATH=$(git rev-parse --show-toplevel) ``` -If yes: +**If `GIT_DIR == GIT_COMMON`:** Normal repo, no worktree to clean up. Done. + +**If worktree path is under `.worktrees/` or `~/.config/superpowers/worktrees/`:** Superpowers created this worktree — we own cleanup. + ```bash -git worktree remove +MAIN_ROOT=$(git -C "$(git rev-parse --git-common-dir)/.." rev-parse --show-toplevel) +cd "$MAIN_ROOT" +git worktree remove "$WORKTREE_PATH" +git worktree prune # Self-healing: clean up any stale registrations ``` -**For Option 3:** Keep worktree. +**Otherwise:** The host environment (harness) owns this workspace. Do NOT remove it. If your platform provides a workspace-exit tool, use it. Otherwise, leave the workspace in place. ## Quick Reference | Option | Merge | Push | Keep Worktree | Cleanup Branch | |--------|-------|------|---------------|----------------| -| 1. Merge locally | ✓ | - | - | ✓ | -| 2. Create PR | - | ✓ | ✓ | - | -| 3. Keep as-is | - | - | ✓ | - | -| 4. Discard | - | - | - | ✓ (force) | +| 1. Merge locally | yes | - | - | yes | +| 2. Create PR | - | yes | yes | - | +| 3. Keep as-is | - | - | yes | - | +| 4. Discard | - | - | - | yes (force) | ## Common Mistakes @@ -165,13 +205,25 @@ git worktree remove - **Fix:** Always verify tests before offering options **Open-ended questions** -- **Problem:** "What should I do next?" → ambiguous -- **Fix:** Present exactly 4 structured options +- **Problem:** "What should I do next?" is ambiguous +- **Fix:** Present exactly 4 structured options (or 3 for detached HEAD) -**Automatic worktree cleanup** -- **Problem:** Remove worktree when might need it (Option 2, 3) +**Cleaning up worktree for Option 2** +- **Problem:** Remove worktree user needs for PR iteration - **Fix:** Only cleanup for Options 1 and 4 +**Deleting branch before removing worktree** +- **Problem:** `git branch -d` fails because worktree still references the branch +- **Fix:** Merge first, remove worktree, then delete branch + +**Running git worktree remove from inside the worktree** +- **Problem:** Command fails silently when CWD is inside the worktree being removed +- **Fix:** Always `cd` to main repo root before `git worktree remove` + +**Cleaning up harness-owned worktrees** +- **Problem:** Removing a worktree the harness created causes phantom state +- **Fix:** Only clean up worktrees under `.worktrees/` or `~/.config/superpowers/worktrees/` + **No confirmation for discard** - **Problem:** Accidentally delete work - **Fix:** Require typed "discard" confirmation @@ -183,12 +235,18 @@ git worktree remove - Merge without verifying tests on result - Delete work without confirmation - Force-push without explicit request +- Remove a worktree before confirming merge success +- Clean up worktrees you didn't create (provenance check) +- Run `git worktree remove` from inside the worktree **Always:** - Verify tests before offering options -- Present exactly 4 options +- Detect environment before presenting menu +- Present exactly 4 options (or 3 for detached HEAD) - Get typed confirmation for Option 4 - Clean up worktree for Options 1 & 4 only +- `cd` to main repo root before worktree removal +- Run `git worktree prune` after removal ## Integration