diff --git a/skills/subagent-driven-development/SKILL.md b/skills/subagent-driven-development/SKILL.md index aac35b91..cd007c0a 100644 --- a/skills/subagent-driven-development/SKILL.md +++ b/skills/subagent-driven-development/SKILL.md @@ -135,7 +135,7 @@ a ledger file, not only in todos. - Each plan owns a workspace: at skill start, run this skill's `scripts/sdd-workspace PLAN_FILE` — it prints the plan's git-ignored - directory (`/.superpowers/sdd//`), home to + directory (under `/.superpowers/sdd/`), home to every artifact for THIS plan: ledger, briefs, reports, review packages. Another plan's directory is never yours to read or write. - Check for this plan's ledger at `/progress.md`. If its first diff --git a/skills/subagent-driven-development/scripts/sdd-workspace b/skills/subagent-driven-development/scripts/sdd-workspace index 4e2d1680..ff6b9839 100755 --- a/skills/subagent-driven-development/scripts/sdd-workspace +++ b/skills/subagent-driven-development/scripts/sdd-workspace @@ -8,6 +8,16 @@ # artifacts. A stale ledger misread as current progress makes controllers # skip whole task sequences — plan-scoping removes that failure structurally. # +# Basename slugs collide when two plans share a filename (docs/alpha/plan.md +# vs docs/beta/plan.md), so each workspace records its owning plan's path in +# a plan-path marker (repo-relative in-repo, absolute outside). A workspace +# owned by a different plan is skipped and the slug disambiguated with the +# plan's parent-directory name, then a counter. A workspace with no marker +# predates the marker scheme and is adopted for the current plan so in-flight +# workspaces keep resolving — which means the first collision on such a +# legacy workspace adopts instead of detecting; acceptable, marker-less +# workspaces age out as plans finish. +# # The workspace lives in the working tree (not under .git/) because Claude Code # treats .git/ as a protected path and denies agent writes there — which blocks # an implementer subagent from writing its report file. A self-ignoring @@ -34,7 +44,39 @@ slug=$(basename "$plan" .md) root=$(git rev-parse --show-toplevel) base="$root/.superpowers/sdd" + +# Normalize the plan path (physical directory, so relative/absolute/../ +# spellings of one plan compare equal) and express it as the marker value: +# repo-relative when the plan lives under the repo root, absolute otherwise. +plan_dir=$(CDPATH= cd -- "$(dirname "$plan")" && pwd -P) +plan_abs="$plan_dir/$(basename "$plan")" +case "$plan_abs" in + "$root"/*) plan_id=${plan_abs#"$root"/} ;; + *) plan_id=$plan_abs ;; +esac + +# True when the workspace at $1 is (or becomes) this plan's: an existing +# marker must name this plan; a missing marker means a new workspace or a +# pre-marker legacy one, and either way the plan claims it by writing one. +owns() { + if [ -e "$1/plan-path" ]; then + [ "$(cat "$1/plan-path")" = "$plan_id" ] + else + mkdir -p "$1" + printf '%s\n' "$plan_id" > "$1/plan-path" + fi +} + dir="$base/$slug" -mkdir -p "$dir" +if ! owns "$dir"; then + parent=$(basename "$plan_dir") + dir="$base/$slug-$parent" + if ! owns "$dir"; then + n=2 + while ! owns "$base/$slug-$parent-$n"; do n=$((n + 1)); done + dir="$base/$slug-$parent-$n" + fi +fi + printf '*\n' > "$base/.gitignore" -cd "$dir" && pwd +CDPATH= cd -- "$dir" && pwd diff --git a/tests/claude-code/test-sdd-workspace.sh b/tests/claude-code/test-sdd-workspace.sh index 84172301..1a86fe70 100755 --- a/tests/claude-code/test-sdd-workspace.sh +++ b/tests/claude-code/test-sdd-workspace.sh @@ -189,6 +189,128 @@ PLAN echo " status: $wt_status" fi + # --- Ownership markers: two plans with the same basename (#2045) --- + mkdir -p "$repo/docs/alpha" "$repo/docs/beta" + cat > "$repo/docs/alpha/plan.md" <<'PLAN' +# Alpha Plan + +## Task 1: Alpha work + +Alpha-only requirement text. +PLAN + cat > "$repo/docs/beta/plan.md" <<'PLAN' +# Beta Plan + +## Task 1: Beta work + +Beta-only requirement text. +PLAN + + local dir_alpha dir_beta + dir_alpha="$(cd "$repo" && "$SDD_SCRIPTS/sdd-workspace" docs/alpha/plan.md)" + dir_beta="$(cd "$repo" && "$SDD_SCRIPTS/sdd-workspace" docs/beta/plan.md)" + if [[ "$dir_alpha" != "$dir_beta" ]]; then + pass "same-basename plans resolve to distinct workspaces" + else + fail "same-basename plans resolve to distinct workspaces" + echo " alpha: $dir_alpha" + echo " beta: $dir_beta" + fi + + ( cd "$repo" && "$SDD_SCRIPTS/task-brief" docs/alpha/plan.md 1 >/dev/null ) + ( cd "$repo" && "$SDD_SCRIPTS/task-brief" docs/beta/plan.md 1 >/dev/null ) + if grep -q "Alpha-only requirement text." "$dir_alpha/task-1-brief.md" 2>/dev/null \ + && grep -q "Beta-only requirement text." "$dir_beta/task-1-brief.md" 2>/dev/null; then + pass "same-basename plans keep both task briefs intact" + else + fail "same-basename plans keep both task briefs intact" + echo " alpha brief: $(cat "$dir_alpha/task-1-brief.md" 2>/dev/null)" + echo " beta brief: $(cat "$dir_beta/task-1-brief.md" 2>/dev/null)" + fi + + # --- Legacy adoption: pre-existing workspace without a marker --- + printf '# Foo\n\n## Task 1: Foo\n\nFoo.\n' > "$repo/foo.md" + mkdir -p "$repo/.superpowers/sdd/foo" + printf 'ledger\n' > "$repo/.superpowers/sdd/foo/progress.md" + local dir_foo + dir_foo="$(cd "$repo" && "$SDD_SCRIPTS/sdd-workspace" foo.md)" + if [[ "$dir_foo" == "$repo/.superpowers/sdd/foo" \ + && -f "$dir_foo/progress.md" \ + && "$(cat "$dir_foo/plan-path" 2>/dev/null)" == "foo.md" ]]; then + pass "legacy markerless workspace is adopted in place and marked" + else + fail "legacy markerless workspace is adopted in place and marked" + echo " dir: $dir_foo" + echo " marker: $(cat "$dir_foo/plan-path" 2>/dev/null)" + fi + + # --- Ownership conflict: marker names a different plan --- + printf '# Bar\n\n## Task 1: Bar\n\nBar.\n' > "$repo/bar.md" + mkdir -p "$repo/.superpowers/sdd/bar" + printf 'somewhere-else/bar.md\n' > "$repo/.superpowers/sdd/bar/plan-path" + printf 'other ledger\n' > "$repo/.superpowers/sdd/bar/progress.md" + local dir_bar + dir_bar="$(cd "$repo" && "$SDD_SCRIPTS/sdd-workspace" bar.md)" + if [[ "$dir_bar" == "$repo/.superpowers/sdd/bar-repo" \ + && "$(cat "$dir_bar/plan-path" 2>/dev/null)" == "bar.md" ]]; then + pass "owned workspace disambiguates with parent-dir suffix" + else + fail "owned workspace disambiguates with parent-dir suffix" + echo " got: $dir_bar" + fi + if [[ "$(cat "$repo/.superpowers/sdd/bar/plan-path")" == "somewhere-else/bar.md" \ + && "$(cat "$repo/.superpowers/sdd/bar/progress.md")" == "other ledger" ]]; then + pass "conflicting plan leaves the original workspace untouched" + else + fail "conflicting plan leaves the original workspace untouched" + fi + + # --- Counter fallback: parent-suffixed workspace is owned too --- + printf '# Baz\n\n## Task 1: Baz\n\nBaz.\n' > "$repo/baz.md" + mkdir -p "$repo/.superpowers/sdd/baz" "$repo/.superpowers/sdd/baz-repo" + printf 'one/baz.md\n' > "$repo/.superpowers/sdd/baz/plan-path" + printf 'two/baz.md\n' > "$repo/.superpowers/sdd/baz-repo/plan-path" + local dir_baz + dir_baz="$(cd "$repo" && "$SDD_SCRIPTS/sdd-workspace" baz.md)" + if [[ "$dir_baz" == "$repo/.superpowers/sdd/baz-repo-2" \ + && "$(cat "$dir_baz/plan-path" 2>/dev/null)" == "baz.md" ]]; then + pass "double conflict falls back to a counter suffix" + else + fail "double conflict falls back to a counter suffix" + echo " got: $dir_baz" + fi + + # --- Same plan spelled differently resolves to one workspace --- + local dir_rel dir_abs dir_dotdot + dir_rel="$(cd "$repo" && "$SDD_SCRIPTS/sdd-workspace" docs/alpha/plan.md)" + dir_abs="$(cd "$repo" && "$SDD_SCRIPTS/sdd-workspace" "$repo/docs/alpha/plan.md")" + dir_dotdot="$(cd "$repo/docs/beta" && "$SDD_SCRIPTS/sdd-workspace" ../alpha/plan.md)" + if [[ "$dir_rel" == "$dir_abs" && "$dir_rel" == "$dir_dotdot" \ + && "$(cat "$dir_rel/plan-path" 2>/dev/null)" == "docs/alpha/plan.md" ]]; then + pass "relative, absolute, and ../ spellings share one workspace and marker" + else + fail "relative, absolute, and ../ spellings share one workspace and marker" + echo " rel: $dir_rel" + echo " abs: $dir_abs" + echo " dotdot: $dir_dotdot" + echo " marker: $(cat "$dir_rel/plan-path" 2>/dev/null)" + fi + + # --- Out-of-repo plans keep working, marker holds the absolute path --- + mkdir -p "$TEST_ROOT/outside" + printf '# Remote\n\n## Task 1: Remote\n\nRemote.\n' > "$TEST_ROOT/outside/remote-plan.md" + local outside_abs dir_out + outside_abs="$(cd "$TEST_ROOT/outside" && pwd -P)/remote-plan.md" + dir_out="$(cd "$repo" && "$SDD_SCRIPTS/sdd-workspace" "$TEST_ROOT/outside/remote-plan.md")" + if [[ "$dir_out" == "$repo/.superpowers/sdd/remote-plan" \ + && "$(cat "$dir_out/plan-path" 2>/dev/null)" == "$outside_abs" ]]; then + pass "out-of-repo plan gets a basename slug and an absolute-path marker" + else + fail "out-of-repo plan gets a basename slug and an absolute-path marker" + echo " dir: $dir_out" + echo " marker: $(cat "$dir_out/plan-path" 2>/dev/null)" + fi + echo "" if [[ "$FAILURES" -ne 0 ]]; then echo "FAILED: $FAILURES assertion(s)."