subagent-driven-development: plan-scoped workspace still collides when two plans share a basename (follow-up to #2012) #2045

New issue

Closed

opened 2026-07-27 03:26:57 +00:00 by CRGDan · 2 comments

CRGDan commented

2026-07-27 03:26:57 +00:00

(Migrated from github.com)

Following up on #2012 as asked: "if you can still reproduce the collision on a release that includes #1943, please reopen with the session details."

I can, on released v6.2.0 (3dcbd5c). #1943 scopes the workspace by plan basename, so it fixes collisions between plan-a.md and plan-b.md but not between docs/alpha/plan.md and docs/beta/plan.md. Any repo where plans are foldered — one directory per feature, per sprint, per subproject — still gets the original silent overwrite. plan.md, implementation-plan.md, and README.md are exactly the basenames that repeat.

Cause

scripts/sdd-workspace:31

slug=$(basename "$plan" .md)

basename discards the directory, which is the only part that distinguishes the two plans.

Reproduction

SP=~/.claude/plugins/marketplaces/superpowers/skills/subagent-driven-development/scripts
T=$(mktemp -d); git init -q -b main "$T/repo"; R=$(cd "$T/repo" && git rev-parse --show-toplevel)
mkdir -p "$R/docs/alpha" "$R/docs/beta"
printf '# Alpha\n\n## Task 1: Alpha thing\n\nAlpha requirements: MAGIC_CONST=7.\n' > "$R/docs/alpha/plan.md"
printf '# Beta\n\n## Task 1: Beta thing\n\nBeta requirements.\n'                    > "$R/docs/beta/plan.md"
cd "$R"
"$SP/task-brief" docs/alpha/plan.md 1
"$SP/task-brief" docs/beta/plan.md  1
cat .superpowers/sdd/plan/task-1-brief.md

## Task 1: Beta thing

Beta requirements.

Both plans resolve to <repo>/.superpowers/sdd/plan/. Alpha's brief is gone, no warning, and the directory is gitignored so there is no history to recover from.

Three consequences, in severity order

1. Briefs and reports are overwritten with no identity check. The ledger has a self-identifying first line, so progress.md collisions are at least detectable. task-N-brief.md and task-N-report.md have no such marker, and SKILL.md:207-209 makes the brief "the single source of requirements" — exact values, magic strings, and signatures appear only there. A subagent dispatched against a clobbered brief implements the wrong plan's task and has no way to notice.

2. A task-brief invocation that fails still destroys the colliding file. scripts/task-brief:34 redirects awk output with >, which truncates before the not-found check at line 36:

awk -v n="$n" '...' "$plan" > "$out"

if [ ! -s "$out" ]; then
  echo "task ${n} not found in ${plan} ..." >&2
  exit 3
fi

So a wrong task number, a ## Step 1: heading style, or a typo'd plan path leaves plan A's brief as a zero-byte file while telling the operator nothing was written:

alpha brief before:  59 bytes
task 1 not found in docs/beta/plan.md (no heading matching 'Task 1')
beta task-brief exit: 3
alpha brief after:    0 bytes

3. The documented remedies both assume the directories are distinct. SKILL.md:132-133 says a ledger naming a different plan "is another plan's progress: leave it in place and start your own, fresh" — but <workspace>/progress.md is a single fixed path inside a shared directory, so there is nowhere fresh to start. And SKILL.md:419-420, rm -rf <workspace> on clean final review, with "sibling directories belong to other plans; leave them alone": under a collision the other plan's artifacts are not in a sibling directory, they are in this one, and they get deleted mid-run.

tests/claude-code/test-sdd-workspace.sh only exercises plan-a.md and plan-b.md in the repo root, so the distinct-basename case is covered and the colliding one is not.

Suggested fix

Derive the slug from the plan's repo-relative path rather than its basename, so the directory is unique for exactly the reason the plan file is:

root=$(git rev-parse --show-toplevel)
abs="$(cd "$(dirname "$plan")" && pwd -P)/$(basename "$plan")"   # -P matches --show-toplevel's symlink resolution
rel=${abs#"$root"/}
slug=$(printf '%s' "${rel%.md}" | tr '/' '-')

docs/alpha/plan.md → docs-alpha-plan, docs/beta/plan.md → docs-beta-plan, and flat.md → flat, so today's flat layout keeps the directory name it already has. Deliberately not git ls-files, which returns empty for a plan that hasn't been committed yet — the normal case when a plan is written and immediately executed.

Two edge cases need a decision rather than a snippet: a plan outside the repo root (the expression above degrades to a long absolute-ish slug — unique, but ugly), and --vs-/ ambiguity between a/b-c.md and a-b/c.md. A short digest suffix, or percent-encoding /, settles both.

Independently worth doing, since it converts silent loss into a visible error even if the slug stays as it is:

task-brief: write to a temp file and mv into place only after the not-found check, and refuse to overwrite an existing file without --force.
sdd-workspace: if <dir>/progress.md exists and its first line names a different plan, exit non-zero instead of returning the directory. That makes the collision loud at the one moment the operator can still act on it.

Environment

superpowers v6.2.0 (3dcbd5c), Claude Code, macOS 26.5.1 (Darwin 25.5.0), bash 3.2.57.

Following up on #2012 as asked: *"if you can still reproduce the collision on a release that includes #1943, please reopen with the session details."* I can, on released **v6.2.0** (`3dcbd5c`). #1943 scopes the workspace by plan **basename**, so it fixes collisions between `plan-a.md` and `plan-b.md` but not between `docs/alpha/plan.md` and `docs/beta/plan.md`. Any repo where plans are foldered — one directory per feature, per sprint, per subproject — still gets the original silent overwrite. `plan.md`, `implementation-plan.md`, and `README.md` are exactly the basenames that repeat. ### Cause `scripts/sdd-workspace:31` ```sh slug=$(basename "$plan" .md) ``` `basename` discards the directory, which is the only part that distinguishes the two plans. ### Reproduction ```sh SP=~/.claude/plugins/marketplaces/superpowers/skills/subagent-driven-development/scripts T=$(mktemp -d); git init -q -b main "$T/repo"; R=$(cd "$T/repo" && git rev-parse --show-toplevel) mkdir -p "$R/docs/alpha" "$R/docs/beta" printf '# Alpha\n\n## Task 1: Alpha thing\n\nAlpha requirements: MAGIC_CONST=7.\n' > "$R/docs/alpha/plan.md" printf '# Beta\n\n## Task 1: Beta thing\n\nBeta requirements.\n' > "$R/docs/beta/plan.md" cd "$R" "$SP/task-brief" docs/alpha/plan.md 1 "$SP/task-brief" docs/beta/plan.md 1 cat .superpowers/sdd/plan/task-1-brief.md ``` ``` ## Task 1: Beta thing Beta requirements. ``` Both plans resolve to `<repo>/.superpowers/sdd/plan/`. Alpha's brief is gone, no warning, and the directory is gitignored so there is no history to recover from. ### Three consequences, in severity order **1. Briefs and reports are overwritten with no identity check.** The ledger has a self-identifying first line, so `progress.md` collisions are at least *detectable*. `task-N-brief.md` and `task-N-report.md` have no such marker, and SKILL.md:207-209 makes the brief "the single source of requirements" — exact values, magic strings, and signatures appear only there. A subagent dispatched against a clobbered brief implements the wrong plan's task and has no way to notice. **2. A `task-brief` invocation that *fails* still destroys the colliding file.** `scripts/task-brief:34` redirects `awk` output with `>`, which truncates before the not-found check at line 36: ```sh awk -v n="$n" '...' "$plan" > "$out" if [ ! -s "$out" ]; then echo "task ${n} not found in ${plan} ..." >&2 exit 3 fi ``` So a wrong task number, a `## Step 1:` heading style, or a typo'd plan path leaves plan A's brief as a zero-byte file while telling the operator nothing was written: ``` alpha brief before: 59 bytes task 1 not found in docs/beta/plan.md (no heading matching 'Task 1') beta task-brief exit: 3 alpha brief after: 0 bytes ``` **3. The documented remedies both assume the directories are distinct.** SKILL.md:132-133 says a ledger naming a different plan "is another plan's progress: leave it in place and start your own, fresh" — but `<workspace>/progress.md` is a single fixed path inside a shared directory, so there is nowhere fresh to start. And SKILL.md:419-420, `rm -rf <workspace>` on clean final review, with "sibling directories belong to other plans; leave them alone": under a collision the other plan's artifacts are not in a sibling directory, they are in this one, and they get deleted mid-run. `tests/claude-code/test-sdd-workspace.sh` only exercises `plan-a.md` and `plan-b.md` in the repo root, so the distinct-basename case is covered and the colliding one is not. ### Suggested fix Derive the slug from the plan's repo-relative path rather than its basename, so the directory is unique for exactly the reason the plan file is: ```sh root=$(git rev-parse --show-toplevel) abs="$(cd "$(dirname "$plan")" && pwd -P)/$(basename "$plan")" # -P matches --show-toplevel's symlink resolution rel=${abs#"$root"/} slug=$(printf '%s' "${rel%.md}" | tr '/' '-') ``` `docs/alpha/plan.md` → `docs-alpha-plan`, `docs/beta/plan.md` → `docs-beta-plan`, and `flat.md` → `flat`, so today's flat layout keeps the directory name it already has. Deliberately not `git ls-files`, which returns empty for a plan that hasn't been committed yet — the normal case when a plan is written and immediately executed. Two edge cases need a decision rather than a snippet: a plan outside the repo root (the expression above degrades to a long absolute-ish slug — unique, but ugly), and `-`-vs-`/` ambiguity between `a/b-c.md` and `a-b/c.md`. A short digest suffix, or percent-encoding `/`, settles both. Independently worth doing, since it converts silent loss into a visible error even if the slug stays as it is: - `task-brief`: write to a temp file and `mv` into place only after the not-found check, and refuse to overwrite an existing file without `--force`. - `sdd-workspace`: if `<dir>/progress.md` exists and its first line names a different plan, exit non-zero instead of returning the directory. That makes the collision loud at the one moment the operator can still act on it. ### Environment superpowers v6.2.0 (`3dcbd5c`), Claude Code, macOS 26.5.1 (Darwin 25.5.0), bash 3.2.57.

obra commented

2026-08-12 23:04:46 +00:00

(Migrated from github.com)

Short names stay: slugs remain basename-derived — docs-superpowers-plans-<name> style path slugs are rejected.
Out-of-repo plans keep working: plans outside the repository remain supported (no exit-2 rejection); path-relative slugging can't represent them, which rules that approach out entirely.
No migration break: existing workspaces must keep resolving for in-flight plans; a controller finding an unexpectedly empty workspace re-dispatches completed tasks, the most expensive known SDD failure.
Ownership marker: sdd-workspace records the owning plan's path (repo-relative in-repo, absolute outside) in the workspace; a lookup that finds a marker naming a different plan triggers disambiguation (parent-dir suffix or counter). Collisions are caught exactly when they exist, lazily, with no rename churn.

Also carrying over from #2120's review, for whoever implements: guard cd against CDPATH leakage (CDPATH= cd -- ...) in any new path logic, and add test assertions that the same plan spelled different ways (absolute/relative/../) resolves to one workspace.

— Claude Fable 5, Claude Code 2.1.228, triaging on behalf of @obra

Triage update: PR #2120 went through the full review pipeline today (security review clean, deterministic tests 20/20, manual reproduction of the collision confirmed on dev and fixed on the branch) but was closed on slug-design grounds. Design constraints for the in-house fix, per @obra: - **Short names stay:** slugs remain `basename`-derived — `docs-superpowers-plans-<name>` style path slugs are rejected. - **Out-of-repo plans keep working:** plans outside the repository remain supported (no exit-2 rejection); path-relative slugging can't represent them, which rules that approach out entirely. - **No migration break:** existing workspaces must keep resolving for in-flight plans; a controller finding an unexpectedly empty workspace re-dispatches completed tasks, the most expensive known SDD failure. - **Ownership marker:** `sdd-workspace` records the owning plan's path (repo-relative in-repo, absolute outside) in the workspace; a lookup that finds a marker naming a *different* plan triggers disambiguation (parent-dir suffix or counter). Collisions are caught exactly when they exist, lazily, with no rename churn. Also carrying over from #2120's review, for whoever implements: guard `cd` against `CDPATH` leakage (`CDPATH= cd -- ...`) in any new path logic, and add test assertions that the same plan spelled different ways (absolute/relative/`../`) resolves to one workspace. — Claude Fable 5, Claude Code 2.1.228, triaging on behalf of @obra

obra commented

2026-08-12 23:04:46 +00:00

(Migrated from github.com)

Short names stay: slugs remain basename-derived — docs-superpowers-plans-<name> style path slugs are rejected.
Out-of-repo plans keep working: plans outside the repository remain supported (no exit-2 rejection); path-relative slugging can't represent them, which rules that approach out entirely.
No migration break: existing workspaces must keep resolving for in-flight plans; a controller finding an unexpectedly empty workspace re-dispatches completed tasks, the most expensive known SDD failure.
Ownership marker: sdd-workspace records the owning plan's path (repo-relative in-repo, absolute outside) in the workspace; a lookup that finds a marker naming a different plan triggers disambiguation (parent-dir suffix or counter). Collisions are caught exactly when they exist, lazily, with no rename churn.