Compare commits

...

1 Commits

Author SHA1 Message Date
Drew Ritter
db61f9dab5 fix: remove global worktree path fallback 2026-05-05 12:38:55 -07:00
8 changed files with 117 additions and 66 deletions

View File

@@ -275,23 +275,16 @@ If no native tool is available, create a worktree manually using git.
Follow this priority order:
1. **Check existing directories:**
1. **Check your instructions for a worktree directory preference.** If specified, use it without asking.
2. **Check existing project-local directories:**
```bash
ls -d .worktrees 2>/dev/null # Preferred (hidden)
ls -d worktrees 2>/dev/null # Alternative
```
If found, use that directory. If both exist, `.worktrees` wins.
2. **Check for existing global directory:**
```bash
project=$(basename "$(git rev-parse --show-toplevel)")
ls -d ~/.config/superpowers/worktrees/$project 2>/dev/null
```
If found, use it (backward compatibility with legacy global path).
3. **Check your instructions for a worktree directory preference.** If specified, use it without asking.
4. **Default to `.worktrees/`.**
3. **Default to `.worktrees/`.**
#### Safety Verification (project-local directories only)
@@ -305,16 +298,11 @@ git check-ignore -q .worktrees 2>/dev/null || git check-ignore -q worktrees 2>/d
**Why critical:** Prevents accidentally committing worktree contents to repository.
Global directories (`~/.config/superpowers/worktrees/`) need no verification.
#### Create the Worktree
```bash
project=$(basename "$(git rev-parse --show-toplevel)")
# Determine path based on chosen location
# For project-local: path="$LOCATION/$BRANCH_NAME"
# For global: path="~/.config/superpowers/worktrees/$project/$BRANCH_NAME"
path="$LOCATION/$BRANCH_NAME"
git worktree add "$path" -b "$BRANCH_NAME"
cd "$path"
@@ -387,7 +375,6 @@ Ready to implement <feature-name>
| `worktrees/` exists | Use it (verify ignored) |
| Both exist | Use `.worktrees/` |
| Neither exists | Check instruction file, then default `.worktrees/` |
| Global path exists | Use it (backward compat) |
| Directory not ignored | Add to .gitignore + commit |
| Permission error on create | Sandbox fallback, work in place |
| Tests fail during baseline | Report failures + ask |
@@ -464,7 +451,7 @@ git commit -m "feat: rewrite using-git-worktrees with detect-and-defer (PRI-974)
Step 0: GIT_DIR != GIT_COMMON detection (skip if already isolated)
Step 0 consent: opt-in prompt before creating worktree (#991)
Step 1a: native tool preference (short, first, declarative)
Step 1b: git worktree fallback with hooks symlink and legacy path compat
Step 1b: git worktree fallback with project-local directory policy
Submodule guard prevents false detection
Platform-neutral instruction file references (#1049)"
```
@@ -663,7 +650,7 @@ WORKTREE_PATH=$(git rev-parse --show-toplevel)
**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.
**If worktree path is under `.worktrees/` or `worktrees/`:** Superpowers created this worktree — we own cleanup.
```bash
MAIN_ROOT=$(git -C "$(git rev-parse --git-common-dir)/.." rev-parse --show-toplevel)
@@ -707,7 +694,7 @@ git worktree prune # Self-healing: clean up any stale registrations
**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/`
- **Fix:** Only clean up worktrees under `.worktrees/` or `worktrees/`
**No confirmation for discard**
- **Problem:** Accidentally delete work

View File

@@ -46,7 +46,7 @@ The skill describes the goal ("ensure work happens in an isolated workspace") an
### Provenance-based ownership
Whoever creates the worktree owns its cleanup. If the harness created it, superpowers doesn't touch it. If superpowers created it (via git fallback), superpowers cleans it up. The heuristic: if the worktree lives under `.worktrees/` or `~/.config/superpowers/worktrees/`, superpowers owns it. Anything else (`.claude/worktrees/`, `~/.codex/worktrees/`, `.gemini/worktrees/`) belongs to the harness.
Whoever creates the worktree owns its cleanup. If the harness created it, superpowers doesn't touch it. If superpowers created it (via git fallback), superpowers cleans it up. The heuristic: if the worktree lives under `.worktrees/` or `worktrees/`, superpowers owns it. Anything else (`.claude/worktrees/`, `~/.codex/worktrees/`, `.gemini/worktrees/`, or old user-global Superpowers paths) belongs to the harness or user and is left alone.
## Design
@@ -110,12 +110,11 @@ File splitting (Step 1b in a separate skill) was tested and proven unnecessary.
When no native tool is available, create a worktree manually.
**Directory selection** (priority order):
1. Check for existing `.worktrees/` or `worktrees/` directory — if found, use it. If both exist, `.worktrees/` wins.
2. Check for existing `~/.config/superpowers/worktrees/<project>/` directory — if found, use it (backward compatibility with legacy global path).
3. Check the project's agent instruction file (CLAUDE.md, GEMINI.md, AGENTS.md, .cursorrules, or equivalent) for a worktree directory preference.
4. Default to `.worktrees/`.
1. Check the project's agent instruction file (CLAUDE.md, GEMINI.md, AGENTS.md, .cursorrules, or equivalent) for a worktree directory preference.
2. Check for existing `.worktrees/` or `worktrees/` directory — if found, use it. If both exist, `.worktrees/` wins.
3. Default to `.worktrees/`.
No interactive directory selection prompt. The global path (`~/.config/superpowers/worktrees/`) is no longer offered as a choice to new users, but existing worktrees at that location are detected and used for backward compatibility.
No interactive directory selection prompt. Old user-global Superpowers worktree paths are not detected or offered; new manual worktrees are project-local unless the user explicitly specifies another location.
**Safety verification** (project-local directories only):
@@ -232,7 +231,7 @@ if GIT_DIR == GIT_COMMON:
# Normal repo, no worktree to clean up
done
if worktree path is under .worktrees/ or ~/.config/superpowers/worktrees/:
if worktree path is under .worktrees/ or worktrees/:
# Superpowers created it — we own cleanup
cd to main repo root # Bug #238 fix
git worktree remove <path>
@@ -318,7 +317,7 @@ As of 2026-04-06, Claude Code is the only harness with an agent-callable mid-ses
### Provenance heuristic
The `.worktrees/` or `~/.config/superpowers/worktrees/` = ours, anything else = hands off` heuristic works for every current harness. If a future harness adopts `.worktrees/` as its convention, we'd have a false positive (superpowers tries to clean up a harness-owned worktree). Similarly, if a user manually runs `git worktree add .worktrees/experiment` without superpowers, we'd incorrectly claim ownership. Both are low risk — every harness uses branded paths, and manual `.worktrees/` creation is unlikely — but worth noting.
The `.worktrees/` or `worktrees/` = ours, anything else = hands off` heuristic works for every current harness. If a future harness adopts one of those project-local directories as its convention, we'd have a false positive (superpowers tries to clean up a harness-owned worktree). Similarly, if a user manually runs `git worktree add .worktrees/experiment` without superpowers, we'd incorrectly claim ownership. Both are low risk — every harness uses branded paths, and manual `.worktrees/` creation is unlikely — but worth noting.
### Detached HEAD finishing

View File

@@ -180,7 +180,7 @@ WORKTREE_PATH=$(git rev-parse --show-toplevel)
**If `GIT_DIR == GIT_COMMON`:** Normal repo, no worktree to clean up. Done.
**If worktree path is under `.worktrees/`, `worktrees/`, or `~/.config/superpowers/worktrees/`:** Superpowers created this worktree — we own cleanup.
**If worktree path is under `.worktrees/` or `worktrees/`:** Superpowers created this worktree — we own cleanup.
```bash
MAIN_ROOT=$(git -C "$(git rev-parse --git-common-dir)/.." rev-parse --show-toplevel)
@@ -224,7 +224,7 @@ git worktree prune # Self-healing: clean up any stale registrations
**Cleaning up harness-owned worktrees**
- **Problem:** Removing a worktree the harness created causes phantom state
- **Fix:** Only clean up worktrees under `.worktrees/`, `worktrees/`, or `~/.config/superpowers/worktrees/`
- **Fix:** Only clean up worktrees under `.worktrees/` or `worktrees/`
**No confirmation for discard**
- **Problem:** Accidentally delete work

View File

@@ -73,14 +73,7 @@ Follow this priority order. Explicit user preference always beats observed files
```
If found, use it. If both exist, `.worktrees` wins.
3. **Check for an existing global directory:**
```bash
project=$(basename "$(git rev-parse --show-toplevel)")
ls -d ~/.config/superpowers/worktrees/$project 2>/dev/null
```
If found, use it (backward compatibility with legacy global path).
4. **If there is no other guidance available**, default to `.worktrees/` at the project root.
3. **If there is no other guidance available**, default to `.worktrees/` at the project root.
#### Safety Verification (project-local directories only)
@@ -94,16 +87,11 @@ git check-ignore -q .worktrees 2>/dev/null || git check-ignore -q worktrees 2>/d
**Why critical:** Prevents accidentally committing worktree contents to repository.
Global directories (`~/.config/superpowers/worktrees/`) need no verification.
#### Create the Worktree
```bash
project=$(basename "$(git rev-parse --show-toplevel)")
# Determine path based on chosen location
# For project-local: path="$LOCATION/$BRANCH_NAME"
# For global: path="~/.config/superpowers/worktrees/$project/$BRANCH_NAME"
path="$LOCATION/$BRANCH_NAME"
git worktree add "$path" -b "$BRANCH_NAME"
cd "$path"
@@ -163,7 +151,6 @@ Ready to implement <feature-name>
| `worktrees/` exists | Use it (verify ignored) |
| Both exist | Use `.worktrees/` |
| Neither exists | Check instruction file, then default `.worktrees/` |
| Global path exists | Use it (backward compat) |
| Directory not ignored | Add to .gitignore + commit |
| Permission error on create | Sandbox fallback, work in place |
| Tests fail during baseline | Report failures + ask |
@@ -189,7 +176,7 @@ Ready to implement <feature-name>
### Assuming directory location
- **Problem:** Creates inconsistency, violates project conventions
- **Fix:** Follow priority: existing > global legacy > instruction file > default
- **Fix:** Follow priority: explicit instructions > existing project-local directory > default
### Proceeding with failing tests
@@ -209,7 +196,7 @@ Ready to implement <feature-name>
**Always:**
- Run Step 0 detection first
- Prefer native tools over git fallback
- Follow directory priority: existing > global legacy > instruction file > default
- Follow directory priority: explicit instructions > existing project-local directory > default
- Verify directory is ignored for project-local
- Auto-detect and run project setup
- Verify clean test baseline

View File

@@ -25,7 +25,7 @@ fi
# Parse command line arguments
VERBOSE=false
SPECIFIC_TEST=""
TIMEOUT=300 # Default 5 minute timeout per test
TIMEOUT=600 # Default 10 minute timeout per test
RUN_INTEGRATION=false
while [[ $# -gt 0 ]]; do
@@ -73,6 +73,7 @@ done
# List of skill tests to run (fast unit tests)
tests=(
"test-worktree-path-policy.sh"
"test-subagent-driven-development.sh"
)

View File

@@ -9,14 +9,14 @@ run_claude() {
local allowed_tools="${3:-}"
local output_file=$(mktemp)
# Build command
local cmd="claude -p \"$prompt\""
# Build command as an argv array so timeout wraps claude directly.
local cmd=(claude -p "$prompt")
if [ -n "$allowed_tools" ]; then
cmd="$cmd --allowed-tools=$allowed_tools"
cmd+=(--allowed-tools="$allowed_tools")
fi
# Run Claude in headless mode with timeout
if timeout "$timeout" bash -c "$cmd" > "$output_file" 2>&1; then
if timeout "$timeout" "${cmd[@]}" > "$output_file" 2>&1; then
cat "$output_file"
rm -f "$output_file"
return 0

View File

@@ -6,13 +6,15 @@ set -euo pipefail
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
source "$SCRIPT_DIR/test-helpers.sh"
CLAUDE_PROMPT_TIMEOUT="${CLAUDE_PROMPT_TIMEOUT:-90}"
echo "=== Test: subagent-driven-development skill ==="
echo ""
# Test 1: Verify skill can be loaded
echo "Test 1: Skill loading..."
output=$(run_claude "What is the subagent-driven-development skill? Describe its key steps briefly." 30)
output=$(run_claude "What is the subagent-driven-development skill? Describe its key steps briefly." "$CLAUDE_PROMPT_TIMEOUT")
if assert_contains "$output" "subagent-driven-development\|Subagent-Driven Development\|Subagent Driven" "Skill is recognized"; then
: # pass
@@ -31,9 +33,11 @@ echo ""
# Test 2: Verify skill describes correct workflow order
echo "Test 2: Workflow ordering..."
output=$(run_claude "In the subagent-driven-development skill, what comes first: spec compliance review or code quality review? Be specific about the order." 30)
output=$(run_claude "In the subagent-driven-development skill, what comes first: spec compliance review or code quality review? Answer using exactly this structure:
First: <review type>
Second: <review type>" "$CLAUDE_PROMPT_TIMEOUT")
if assert_order "$output" "spec.*compliance" "code.*quality" "Spec compliance before code quality"; then
if assert_order "$output" "First:.*spec.*compliance" "Second:.*code.*quality" "Spec compliance before code quality"; then
: # pass
else
exit 1
@@ -44,15 +48,17 @@ echo ""
# Test 3: Verify self-review is mentioned
echo "Test 3: Self-review requirement..."
output=$(run_claude "Does the subagent-driven-development skill require implementers to do self-review? What should they check?" 30)
output=$(run_claude "Does the subagent-driven-development skill require implementers to self-review before handoff, and can self-review replace the external reviews? Answer using exactly this structure:
Self-review required: <yes or no>
Self-review replaces external review: <yes or no>" "$CLAUDE_PROMPT_TIMEOUT")
if assert_contains "$output" "self-review\|self review" "Mentions self-review"; then
if assert_contains "$output" "Self-review required:.*yes" "Mentions self-review"; then
: # pass
else
exit 1
fi
if assert_contains "$output" "completeness\|Completeness" "Checks completeness"; then
if assert_contains "$output" "Self-review replaces external review:.*no" "Self-review does not replace external review"; then
: # pass
else
exit 1
@@ -63,7 +69,7 @@ echo ""
# Test 4: Verify plan is read once
echo "Test 4: Plan reading efficiency..."
output=$(run_claude "In subagent-driven-development, how many times should the controller read the plan file? When does this happen?" 30)
output=$(run_claude "In subagent-driven-development, how many times should the controller read the plan file? When does this happen?" "$CLAUDE_PROMPT_TIMEOUT")
if assert_contains "$output" "once\|one time\|single" "Read plan once"; then
: # pass
@@ -82,7 +88,7 @@ echo ""
# Test 5: Verify spec compliance reviewer is skeptical
echo "Test 5: Spec compliance reviewer mindset..."
output=$(run_claude "What is the spec compliance reviewer's attitude toward the implementer's report in subagent-driven-development?" 30)
output=$(run_claude "What is the spec compliance reviewer's attitude toward the implementer's report in subagent-driven-development?" "$CLAUDE_PROMPT_TIMEOUT")
if assert_contains "$output" "not trust\|don't trust\|skeptical\|verify.*independently\|suspiciously" "Reviewer is skeptical"; then
: # pass
@@ -101,7 +107,7 @@ echo ""
# Test 6: Verify review loops
echo "Test 6: Review loop requirements..."
output=$(run_claude "In subagent-driven-development, what happens if a reviewer finds issues? Is it a one-time review or a loop?" 30)
output=$(run_claude "In subagent-driven-development, what happens if a reviewer finds issues? Is it a one-time review or a loop?" "$CLAUDE_PROMPT_TIMEOUT")
if assert_contains "$output" "loop\|again\|repeat\|until.*approved\|until.*compliant" "Review loops mentioned"; then
: # pass
@@ -120,7 +126,9 @@ echo ""
# Test 7: Verify full task text is provided
echo "Test 7: Task context provision..."
output=$(run_claude "In subagent-driven-development, how does the controller provide task information to the implementer subagent? Does it make them read a file or provide it directly?" 30)
output=$(run_claude "In subagent-driven-development, how does the controller provide task information to the implementer subagent? Answer using exactly this structure:
Controller provides: <directly or by file>
Implementer must read plan file: <yes or no>" "$CLAUDE_PROMPT_TIMEOUT")
if assert_contains "$output" "provide.*directly\|full.*text\|paste\|include.*prompt" "Provides text directly"; then
: # pass
@@ -128,7 +136,7 @@ else
exit 1
fi
if assert_not_contains "$output" "read.*file\|open.*file" "Doesn't make subagent read file"; then
if assert_contains "$output" "Implementer must read plan file:.*no" "Doesn't make subagent read file"; then
: # pass
else
exit 1
@@ -139,7 +147,7 @@ echo ""
# Test 8: Verify worktree requirement
echo "Test 8: Worktree requirement..."
output=$(run_claude "What workflow skills are required before using subagent-driven-development? List any prerequisites or required skills." 30)
output=$(run_claude "What workflow skills are required before using subagent-driven-development? List any prerequisites or required skills." "$CLAUDE_PROMPT_TIMEOUT")
if assert_contains "$output" "using-git-worktrees\|worktree" "Mentions worktree requirement"; then
: # pass
@@ -152,7 +160,7 @@ echo ""
# Test 9: Verify main branch warning
echo "Test 9: Main branch red flag..."
output=$(run_claude "In subagent-driven-development, is it okay to start implementation directly on the main branch?" 30)
output=$(run_claude "In subagent-driven-development, is it okay to start implementation directly on the main branch?" "$CLAUDE_PROMPT_TIMEOUT")
if assert_contains "$output" "worktree\|feature.*branch\|not.*main\|never.*main\|avoid.*main\|don't.*main\|consent\|permission" "Warns against main branch"; then
: # pass

View File

@@ -0,0 +1,69 @@
#!/usr/bin/env bash
# Regression check: Superpowers should not route new worktrees through the old
# global worktree directory.
set -euo pipefail
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
REPO_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)"
USING_SKILL="$REPO_ROOT/skills/using-git-worktrees/SKILL.md"
FINISHING_SKILL="$REPO_ROOT/skills/finishing-a-development-branch/SKILL.md"
ROTOTILL_SPEC="$REPO_ROOT/docs/superpowers/specs/2026-04-06-worktree-rototill-design.md"
ROTOTILL_PLAN="$REPO_ROOT/docs/superpowers/plans/2026-04-06-worktree-rototill.md"
failures=0
assert_contains() {
local file="$1"
local pattern="$2"
local label="$3"
if grep -Fq "$pattern" "$file"; then
echo " [PASS] $label"
else
echo " [FAIL] $label"
echo " Expected to find: $pattern"
echo " In file: $file"
failures=$((failures + 1))
fi
}
assert_not_contains() {
local file="$1"
local pattern="$2"
local label="$3"
if grep -Fq "$pattern" "$file"; then
echo " [FAIL] $label"
echo " Did not expect to find: $pattern"
echo " In file: $file"
failures=$((failures + 1))
else
echo " [PASS] $label"
fi
}
echo "=== Worktree Path Policy Test ==="
echo ""
assert_not_contains "$USING_SKILL" "~/.config/superpowers/worktrees" "using-git-worktrees does not mention old global path"
assert_not_contains "$USING_SKILL" "global legacy" "using-git-worktrees does not use unclear global legacy shorthand"
assert_not_contains "$USING_SKILL" "Global path" "using-git-worktrees has no global path quick-reference row"
assert_contains "$USING_SKILL" 'default to `.worktrees/` at the project root' "using-git-worktrees defaults new manual worktrees to .worktrees/"
assert_not_contains "$FINISHING_SKILL" "~/.config/superpowers/worktrees" "finishing-a-development-branch does not treat old global path as owned"
assert_contains "$FINISHING_SKILL" '`.worktrees/` or `worktrees/`' "finishing-a-development-branch keeps project-local cleanup ownership"
assert_not_contains "$ROTOTILL_SPEC" "~/.config/superpowers/worktrees" "rototill spec does not preserve old global path policy"
assert_not_contains "$ROTOTILL_PLAN" "~/.config/superpowers/worktrees" "rototill plan does not preserve old global path policy"
assert_not_contains "$ROTOTILL_PLAN" "legacy path compat" "rototill plan does not advertise legacy path compatibility"
echo ""
if [ "$failures" -gt 0 ]; then
echo "STATUS: FAILED ($failures failures)"
exit 1
fi
echo "STATUS: PASSED"