diff --git a/.changeset/wise-hawks-howl.md b/.changeset/wise-hawks-howl.md new file mode 100644 index 000000000..ea1c1502e --- /dev/null +++ b/.changeset/wise-hawks-howl.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4545 +--- +**`/gsd-new-milestone --ws ` now correctly scopes every downstream operation to the requested workstream** — the parsed `--ws` flag was silently dropped by every step after the one that parsed it (each workflow step runs in its own shell), so `init.new-milestone`, `state.milestone-switch`, `phases.clear`, the phase-archive `git add`, and the requirements/roadmap/milestone-start commits all operated on the wrong (ambient or root) scope instead of the explicitly requested workstream. diff --git a/gsd-core/workflows/new-milestone.md b/gsd-core/workflows/new-milestone.md index bc6ca4a0d..8306d0728 100644 --- a/gsd-core/workflows/new-milestone.md +++ b/gsd-core/workflows/new-milestone.md @@ -34,6 +34,10 @@ _GSD_SHIM_NAME="gsd-tools.cjs"; _GSD_RUNTIME_ROOT="${RUNTIME_DIR:-$(git rev-pars GSD_WS="" echo "$ARGUMENTS" | grep -qE -- '--ws[[:space:]]+[A-Za-z0-9._-]+' && GSD_WS=$(echo "$ARGUMENTS" | grep -oE -- '--ws[[:space:]]+[A-Za-z0-9._-]+') MILESTONE_ARG=$(echo "$ARGUMENTS" | sed -E 's/--ws[[:space:]]+[A-Za-z0-9._-]+//g' | xargs) +# #4456: persist GSD_WS to a file so later steps' bash fences (each a +# separate shell) can forward it — the same cross-fence problem Step 5/6 +# already solve for OUTGOING_MILESTONE via .gsd-outgoing-milestone. +printf '%s' "$GSD_WS" > .planning/.gsd-ws-arg 2>/dev/null || true RESPONSE_LANGUAGE=$(gsd_run query config-get response_language --raw --default "" 2>/dev/null || echo "") # #2994: EARLY, section-manifest-only init.new-milestone call — needed here # (before Step 4) to gate the project-md-milestone-write section. This is @@ -44,7 +48,11 @@ RESPONSE_LANGUAGE=$(gsd_run query config-get response_language --raw --default " # fields too early and corrupt the roadmapper's phase-numbering context. # init.new-milestone is a pure read (no mutation), so calling it twice is # safe; only `section_manifest` is consumed from this early call. -INIT_EARLY=$(gsd_run query init.new-milestone) +# #4456: $GSD_WS forwarded (same fence as the parse above, no round-trip +# needed here) so the section manifest — and the shared PROJECT.md write +# guard it gates — reflects the EXPLICITLY requested workstream, not +# whatever ambient GSD_WORKSTREAM/session pointer happens to be active. +INIT_EARLY=$(gsd_run query init.new-milestone $GSD_WS) if [[ "$INIT_EARLY" == @file:* ]]; then INIT_EARLY=$(cat "${INIT_EARLY#@file:}"); fi ``` @@ -195,10 +203,11 @@ blockers, todos) is preserved across the switch — symmetric with `milestone.complete`. ```bash -OUTGOING_MILESTONE=$(gsd_run query state.get milestone --raw 2>/dev/null || true) +GSD_WS_ARG=$(cat .planning/.gsd-ws-arg 2>/dev/null || true) +OUTGOING_MILESTONE=$(gsd_run query state.get milestone --raw $GSD_WS_ARG 2>/dev/null || true) printf '%s' "$OUTGOING_MILESTONE" > .planning/.gsd-outgoing-milestone 2>/dev/null || true echo "Outgoing milestone (phase history archives under THIS version in step 6): ${OUTGOING_MILESTONE:-}" -gsd_run query state.milestone-switch --milestone "v[X.Y]" --name "[Name]" +gsd_run query state.milestone-switch --milestone "v[X.Y]" --name "[Name]" $GSD_WS_ARG ``` **Capture the outgoing version now.** The lines above read the *current* (previous) milestone @@ -239,11 +248,12 @@ the captured value into the command, so untrusted STATE.md content cannot be re- shell: ```bash +GSD_WS_ARG=$(cat .planning/.gsd-ws-arg 2>/dev/null || true) OUTGOING_MILESTONE=$(cat .planning/.gsd-outgoing-milestone 2>/dev/null || true) if [ -n "$OUTGOING_MILESTONE" ]; then - gsd_run query phases.clear --confirm --archive-version "$OUTGOING_MILESTONE" + gsd_run query phases.clear --confirm --archive-version "$OUTGOING_MILESTONE" $GSD_WS_ARG else - gsd_run query phases.clear --confirm + gsd_run query phases.clear --confirm $GSD_WS_ARG fi rm -f .planning/.gsd-outgoing-milestone 2>/dev/null || true ``` @@ -258,31 +268,48 @@ Stage the phase archive move + source removal so they land in the same commit as ```bash COMMIT_DOCS=$(gsd_run query config-get commit_docs --raw 2>/dev/null || echo "true") +GSD_WS_ARG=$(cat .planning/.gsd-ws-arg 2>/dev/null || true) +INIT_STAGE=$(gsd_run query init.new-milestone $GSD_WS_ARG) +if [[ "$INIT_STAGE" == @file:* ]]; then INIT_STAGE=$(cat "${INIT_STAGE#@file:}"); fi +_gsd_field() { node -e "const o=JSON.parse(process.argv[1]); const v=o[process.argv[2]]; process.stdout.write(v==null?'':String(v))" "$1" "$2"; } +ARCHIVE_DIR=$(_gsd_field "$INIT_STAGE" archive_dir) +PHASES_DIR=$(_gsd_field "$INIT_STAGE" phases_dir) if [ "$COMMIT_DOCS" != "false" ]; then - git add .planning/milestones/ .planning/phases/ 2>/dev/null || true + git add "$ARCHIVE_DIR/" "$PHASES_DIR/" 2>/dev/null || true fi ``` When `commit_docs` is false, the archive move and phase removals are deliberately left unstaged here — not a bug — since Step 6's commit is skipped too. -Stage PROJECT.md in both modes. Step 4's Part A guard — not this commit — is what protects the shared `## Current Milestone` heading (#2308): when a workstream is active Part A never writes it, so the only change PROJECT.md can carry here is Part B's idempotent `## Evolution` backfill, which must be committed rather than stranded as a dangling edit. Do NOT reintroduce a `[ -n "$GSD_WS" ]` branch around this commit: `GSD_WS` is set in Step 1's shell and each step's bash block runs in its own shell (the same reason Step 5 round-trips `OUTGOING_MILESTONE` through a file), so such a guard reads an unset variable, always takes the flat-mode branch, and only appears to work. +Stage PROJECT.md in both modes. Step 4's Part A guard — not this commit — is what protects the shared `## Current Milestone` heading (#2308): when a workstream is active Part A never writes it, so the only change PROJECT.md can carry here is Part B's idempotent `## Evolution` backfill, which must be committed rather than stranded as a dangling edit. Do NOT reintroduce a `[ -n "$GSD_WS" ]` branch around this commit: `GSD_WS` is set in Step 1's shell and each step's bash block runs in its own shell (the same reason Step 5 round-trips `OUTGOING_MILESTONE` through a file), so such a guard reads an unset variable, always takes the flat-mode branch, and only appears to work. STATE.md, unlike PROJECT.md, IS workstream-scoped (Step 5's switch just wrote the workstream's own copy) — resolved below via `init.new-milestone` rather than a literal `.planning/STATE.md`, which would commit the wrong (or a stale, unrelated) file under an active workstream. ```bash -gsd_run query commit "docs: start milestone v[X.Y] [Name]" --files .planning/PROJECT.md .planning/STATE.md +GSD_WS_ARG=$(cat .planning/.gsd-ws-arg 2>/dev/null || true) +INIT_COMMIT=$(gsd_run query init.new-milestone $GSD_WS_ARG) +if [[ "$INIT_COMMIT" == @file:* ]]; then INIT_COMMIT=$(cat "${INIT_COMMIT#@file:}"); fi +_gsd_field() { node -e "const o=JSON.parse(process.argv[1]); const v=o[process.argv[2]]; process.stdout.write(v==null?'':String(v))" "$1" "$2"; } +STATE_PATH=$(_gsd_field "$INIT_COMMIT" state_path) +PROJECT_PATH=$(_gsd_field "$INIT_COMMIT" project_path) +gsd_run query commit "docs: start milestone v[X.Y] [Name]" --files "$PROJECT_PATH" "$STATE_PATH" ``` ## 7. Load Context and Resolve Models ```bash RESET_PHASE_NUMBERS_PARAM=""; if [[ "$ARGUMENTS" =~ (^|[[:space:]])--reset-phase-numbers([[:space:]]|$) ]]; then RESET_PHASE_NUMBERS_PARAM="--reset-phase-numbers"; fi -INIT=$(gsd_run query init.new-milestone $RESET_PHASE_NUMBERS_PARAM) +GSD_WS_ARG=$(cat .planning/.gsd-ws-arg 2>/dev/null || true) +INIT=$(gsd_run query init.new-milestone $RESET_PHASE_NUMBERS_PARAM $GSD_WS_ARG) if [[ "$INIT" == @file:* ]]; then INIT=$(cat "${INIT#@file:}"); fi AGENT_SKILLS_RESEARCHER=$(gsd_run query agent-skills gsd-project-researcher) AGENT_SKILLS_SYNTHESIZER=$(gsd_run query agent-skills gsd-research-synthesizer) AGENT_SKILLS_ROADMAPPER=$(gsd_run query agent-skills gsd-roadmapper) ``` + -Extract from init JSON: `researcher_model`, `synthesizer_model`, `roadmapper_model`, `commit_docs`, `research_enabled`, `current_milestone`, `project_exists`, `roadmap_exists`, `latest_completed_milestone`, `phase_dir_count`, `phase_archive_path`, `agents_installed`, `missing_agents`, `project_path`, `roadmap_path`, `requirements_path`, `config_path`, `research_dir`, `milestones_path`. +Extract from init JSON: `researcher_model`, `synthesizer_model`, `roadmapper_model`, `commit_docs`, `research_enabled`, `current_milestone`, `project_exists`, `roadmap_exists`, `latest_completed_milestone`, `phase_dir_count`, `phase_archive_path`, `agents_installed`, `missing_agents`, `project_path`, `roadmap_path`, `requirements_path`, `config_path`, `research_dir`, `milestones_path`, `phases_dir`, `archive_dir`. **If `agents_installed` is false:** Display a warning before proceeding: ``` @@ -495,7 +522,12 @@ If "adjust": Return to scoping. **Commit requirements:** ```bash -gsd_run query commit "docs: define milestone v[X.Y] requirements" --files .planning/REQUIREMENTS.md +GSD_WS_ARG=$(cat .planning/.gsd-ws-arg 2>/dev/null || true) +INIT_REQ=$(gsd_run query init.new-milestone $GSD_WS_ARG) +if [[ "$INIT_REQ" == @file:* ]]; then INIT_REQ=$(cat "${INIT_REQ#@file:}"); fi +_gsd_field() { node -e "const o=JSON.parse(process.argv[1]); const v=o[process.argv[2]]; process.stdout.write(v==null?'':String(v))" "$1" "$2"; } +REQUIREMENTS_PATH=$(_gsd_field "$INIT_REQ" requirements_path) +gsd_run query commit "docs: define milestone v[X.Y] requirements" --files "$REQUIREMENTS_PATH" ``` ## 10. Create Roadmap @@ -579,7 +611,17 @@ Success criteria: **Commit roadmap** (after approval): ```bash -gsd_run query commit "docs: create milestone v[X.Y] roadmap ([N] phases)" --files .planning/ROADMAP.md .planning/STATE.md .planning/REQUIREMENTS.md +GSD_WS_ARG=$(cat .planning/.gsd-ws-arg 2>/dev/null || true) +INIT_ROADMAP=$(gsd_run query init.new-milestone $GSD_WS_ARG) +if [[ "$INIT_ROADMAP" == @file:* ]]; then INIT_ROADMAP=$(cat "${INIT_ROADMAP#@file:}"); fi +_gsd_field() { node -e "const o=JSON.parse(process.argv[1]); const v=o[process.argv[2]]; process.stdout.write(v==null?'':String(v))" "$1" "$2"; } +ROADMAP_PATH=$(_gsd_field "$INIT_ROADMAP" roadmap_path) +STATE_PATH=$(_gsd_field "$INIT_ROADMAP" state_path) +REQUIREMENTS_PATH=$(_gsd_field "$INIT_ROADMAP" requirements_path) +gsd_run query commit "docs: create milestone v[X.Y] roadmap ([N] phases)" --files "$ROADMAP_PATH" "$STATE_PATH" "$REQUIREMENTS_PATH" +# #4456: true last consumer of the persisted --ws in this workflow — the +# round-trip file is no longer needed after this commit. +rm -f .planning/.gsd-ws-arg 2>/dev/null || true ``` ## 10.5. Link Pending Todos to Roadmap Phases diff --git a/scripts/lint-workflow-shellcheck-baseline.json b/scripts/lint-workflow-shellcheck-baseline.json index b0a488825..515173f22 100644 --- a/scripts/lint-workflow-shellcheck-baseline.json +++ b/scripts/lint-workflow-shellcheck-baseline.json @@ -494,6 +494,56 @@ "code": "2012", "message": "Use find instead of ls to better handle non-alphanumeric filenames." }, + { + "file": "gsd-core/workflows/new-milestone.md", + "code": "2086", + "message": "Double quote to prevent globbing and word splitting." + }, + { + "file": "gsd-core/workflows/new-milestone.md", + "code": "2086", + "message": "Double quote to prevent globbing and word splitting." + }, + { + "file": "gsd-core/workflows/new-milestone.md", + "code": "2086", + "message": "Double quote to prevent globbing and word splitting." + }, + { + "file": "gsd-core/workflows/new-milestone.md", + "code": "2086", + "message": "Double quote to prevent globbing and word splitting." + }, + { + "file": "gsd-core/workflows/new-milestone.md", + "code": "2086", + "message": "Double quote to prevent globbing and word splitting." + }, + { + "file": "gsd-core/workflows/new-milestone.md", + "code": "2086", + "message": "Double quote to prevent globbing and word splitting." + }, + { + "file": "gsd-core/workflows/new-milestone.md", + "code": "2086", + "message": "Double quote to prevent globbing and word splitting." + }, + { + "file": "gsd-core/workflows/new-milestone.md", + "code": "2086", + "message": "Double quote to prevent globbing and word splitting." + }, + { + "file": "gsd-core/workflows/new-milestone.md", + "code": "2086", + "message": "Double quote to prevent globbing and word splitting." + }, + { + "file": "gsd-core/workflows/new-milestone.md", + "code": "2086", + "message": "Double quote to prevent globbing and word splitting." + }, { "file": "gsd-core/workflows/new-project.md", "code": "2012", diff --git a/src/init.cts b/src/init.cts index 733ecd12a..ae761d7d6 100644 --- a/src/init.cts +++ b/src/init.cts @@ -1003,6 +1003,17 @@ function cmdInitExecutePhase( // #3188: null when the file is absent (parity with patterns_path/context_path). state_path: fs.existsSync(statePath) ? toPosixPath(statePath) : null, roadmap_path: fs.existsSync(roadmapPath) ? toPosixPath(roadmapPath) : null, + // #4456 correction: an isolated review pass initially "fixed" this to + // planningDir(cwd, null) on the assumption that config.json is shared + // like PROJECT.md (workstream-flag.md's directory diagram marks it + // `# Shared`) — but ADR-0006's own tests (tests/init.test.cjs, "init + // handlers honor GSD_WORKSTREAM") assert config_path IS workstream-scoped + // for execute-phase/new-project/new-milestone/progress, and gsd-test + // caught the regression immediately. The diagram is stale for + // config.json specifically (same class of staleness already found for + // `milestones/` during the #4455 follow-up) — reverted to the + // workstream-aware planningDir(cwd), matching the established, + // ADR-governed, tested contract. config_path: toPosixPath(path.join(planningDir(cwd), 'config.json')), // #2376: execute-phase.md's verify_phase_goal step reads this instead of // hardcoding '.planning/REQUIREMENTS.md' into the gsd-verifier spawn prompt. @@ -1402,6 +1413,17 @@ function cmdInitNewProject(cwd: string, raw: boolean, options: Record { + // #4456: executed fences below use the runtime-launcher preamble, which + // resolves gsd-tools.cjs via `${RUNTIME_DIR:-$(git rev-parse --show-toplevel + // || pwd)}`. Any fence run with an ISOLATED `cwd` (a tmpDir outside the + // repo, needed once Step 1's fence started performing a real + // `.gsd-ws-arg` write) has no git repo to discover, and CI benches have no + // global `gsd_run` on PATH to fall back to — so RUNTIME_DIR must be set + // explicitly wherever an isolated `cwd` is used, or the preamble fails + // with "gsd-tools.cjs not found" (a real bench failure this fix hit). + const REPO_ROOT = path.join(__dirname, '..'); + const runtimeDirEnv = { ...process.env, RUNTIME_DIR: REPO_ROOT }; const workflowPath = path.join(__dirname, '..', 'gsd-core', 'workflows', 'new-milestone.md'); // readFileNormalized() strips \r\n -> \n before either extractor below slices // a fence out of `content` — both fences are handed to execFileSync('bash', ...) @@ -668,11 +678,20 @@ describe('new-milestone.md: workstream-aware PROJECT.md guard (#2308)', () => { describe('step 1: --ws parsing is real, executable shell (not prose)', () => { const step1Fence = extractFenceBetween(content, '## 1. Load Context', '## 2. Gather Milestone Goals'); + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); function runStep1(argumentsValue) { const script = `ARGUMENTS=${JSON.stringify(argumentsValue)}\n${step1Fence}\n` + 'printf \'GSD_WS=[%s]\\nMILESTONE_ARG=[%s]\\n\' "$GSD_WS" "$MILESTONE_ARG"'; - const r = runHookSeam('-c', [script], { interpreter: 'bash' }); + const r = runHookSeam('-c', [script], { interpreter: 'bash', cwd: tmpDir, env: runtimeDirEnv }); throwIfFailed(r, 'bash '); const out = r.stdout; return { @@ -692,48 +711,314 @@ describe('new-milestone.md: workstream-aware PROJECT.md guard (#2308)', () => { assert.strictEqual(gsdWs, ''); assert.strictEqual(milestoneArg, 'v2.0 Search'); }); + + // #4456: GSD_WS is persisted to .planning/.gsd-ws-arg so later steps' + // separate shells can forward it — without this file the fix is inert. + test('#4456: persists GSD_WS to .planning/.gsd-ws-arg for later steps to read back', () => { + runStep1('--ws search v2.0 Search'); + const persisted = fs.readFileSync(path.join(tmpDir, '.planning', '.gsd-ws-arg'), 'utf8'); + assert.strictEqual(persisted, '--ws search'); + }); + + test('#4456: persists an empty file in flat mode (regression guard)', () => { + runStep1('v2.0 Search'); + const persisted = fs.readFileSync(path.join(tmpDir, '.planning', '.gsd-ws-arg'), 'utf8'); + assert.strictEqual(persisted, ''); + }); }); - describe('step 6: commit stages PROJECT.md in both modes, with no cross-step guard', () => { - const step6CommitFence = extractFenceContaining( - content, - '## 6. Cleanup and Commit', - '## 7. Load Context and Resolve Models', - 'docs: start milestone v[X.Y] [Name]' - ); + describe('#4456: new-milestone.md forwards --ws to every downstream gsd_run call', () => { + let tmpDir; - function runStep6Commit(argumentsValue) { - const gsdRunStub = 'gsd_run() { printf "%s\\n" "gsd_run_call:$*"; }\n'; - const script = `ARGUMENTS=${JSON.stringify(argumentsValue)}\n${gsdRunStub}${step6CommitFence}`; - const r = runHookSeam('-c', [script], { interpreter: 'bash' }); - throwIfFailed(r, 'bash '); - return r.stdout; + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + /** + * Stub gsd_run: `query init.new-milestone [--ws ]` returns a canned + * JSON payload keyed by whether --ws was present in ITS OWN argv (so a + * fence that forgets to forward $GSD_WS_ARG gets caught — the stub + * doesn't just trust a variable, it inspects the actual call). Every + * other call is recorded verbatim (gsd_run_call:) for assertion. + */ + function stubGsdRun(rootPaths, wsPaths) { + const rootJson = JSON.stringify(rootPaths).replace(/'/g, "'\\''"); + const wsJson = JSON.stringify(wsPaths).replace(/'/g, "'\\''"); + return [ + 'gsd_run() {', + ' if [ "$1" = "query" ] && [ "$2" = "init.new-milestone" ]; then', + ' case " $* " in', + ` *" --ws "*) printf '%s' '${wsJson}' ;;`, + ` *) printf '%s' '${rootJson}' ;;`, + ' esac', + ' else', + ' printf "gsd_run_call:%s\\n" "$*"', + ' fi', + '}', + ].join('\n') + '\n'; } - // Step 4 Part A's guard — not this commit — is what protects the shared - // heading. Part B's Evolution backfill DOES write PROJECT.md in workstream - // mode, so a ws-mode branch that dropped PROJECT.md from --files would - // strand that edit uncommitted. - for (const [mode, args] of [['ws', '--ws search v2.0 Search'], ['flat', 'v2.0 Search']]) { - test(`${mode} mode: --files stages PROJECT.md so Part B's Evolution backfill is committed`, () => { - const out = runStep6Commit(args); + describe('step 5: state.get / state.milestone-switch', () => { + const step5Fence = extractFenceBetween(content, '## 5. Update STATE.md', '## 6. Cleanup and Commit'); + + function runStep5(gsdWsArg) { + fs.writeFileSync(path.join(tmpDir, '.planning', '.gsd-ws-arg'), gsdWsArg); + const gsdRunStub = 'gsd_run() { printf "gsd_run_call:%s\\n" "$*"; }\n'; + const r = runHookSeam('-c', [gsdRunStub + step5Fence], { interpreter: 'bash', cwd: tmpDir, env: runtimeDirEnv }); + throwIfFailed(r, 'bash '); + return r.stdout; + } + + test('ws mode: forwards --ws to both state.get and state.milestone-switch', () => { + const out = runStep5('--ws search'); + assert.match(out, /gsd_run_call:query state\.get milestone --raw --ws search/, + `expected state.get to forward --ws search, got: ${out}`); + assert.match(out, /gsd_run_call:query state\.milestone-switch --milestone v\[X\.Y\] --name \[Name\] --ws search/, + `expected state.milestone-switch to forward --ws search, got: ${out}`); + }); + + test('flat mode: no stray --ws tokens on either call', () => { + const out = runStep5(''); + assert.match(out, /gsd_run_call:query state\.get milestone --raw\s*$/m, + `expected no trailing tokens after --raw in flat mode, got: ${out}`); + assert.match(out, /gsd_run_call:query state\.milestone-switch --milestone v\[X\.Y\] --name \[Name\]\s*$/m, + `expected no trailing tokens on state.milestone-switch in flat mode, got: ${out}`); + }); + }); + + describe('step 6: phases.clear forwards --ws', () => { + const phasesClearFence = extractFenceContaining( + content, '## 6. Cleanup and Commit', '## 7. Load Context and Resolve Models', 'phases.clear', + ); + + function runPhasesClearFence(gsdWsArg) { + fs.writeFileSync(path.join(tmpDir, '.planning', '.gsd-ws-arg'), gsdWsArg); + const gsdRunStub = 'gsd_run() { printf "gsd_run_call:%s\\n" "$*"; }\n'; + const r = runHookSeam('-c', [gsdRunStub + phasesClearFence], { interpreter: 'bash', cwd: tmpDir, env: runtimeDirEnv }); + throwIfFailed(r, 'bash '); + return r.stdout; + } + + test('ws mode, no outgoing milestone: forwards --ws to the plain phases.clear branch', () => { + const out = runPhasesClearFence('--ws search'); + assert.match(out, /gsd_run_call:query phases\.clear --confirm --ws search\s*$/m, + `expected phases.clear to forward --ws search, got: ${out}`); + }); + + test('ws mode, with outgoing milestone: forwards --ws alongside --archive-version', () => { + fs.writeFileSync(path.join(tmpDir, '.planning', '.gsd-outgoing-milestone'), 'v1.0'); + const out = runPhasesClearFence('--ws search'); + assert.match(out, /gsd_run_call:query phases\.clear --confirm --archive-version v1\.0 --ws search\s*$/m, + `expected phases.clear to forward both --archive-version and --ws, got: ${out}`); + }); + + test('flat mode: no stray --ws token', () => { + const out = runPhasesClearFence(''); + assert.match(out, /gsd_run_call:query phases\.clear --confirm\s*$/m, + `expected no trailing tokens in flat mode, got: ${out}`); + }); + }); + + describe('step 6: git add stages the resolved (workstream-scoped) archive/phases dirs', () => { + const gitAddFence = extractFenceContaining( + content, '## 6. Cleanup and Commit', '## 7. Load Context and Resolve Models', 'git add', + ); + + function runGitAddFence(gsdWsArg, rootPaths, wsPaths) { + fs.writeFileSync(path.join(tmpDir, '.planning', '.gsd-ws-arg'), gsdWsArg); + const script = stubGsdRun(rootPaths, wsPaths) + gitAddFence + + '\necho "GIT_ADD_CALL: git add \\"$ARCHIVE_DIR/\\" \\"$PHASES_DIR/\\""'; + const r = runHookSeam('-c', [script], { interpreter: 'bash', cwd: tmpDir, env: runtimeDirEnv }); + throwIfFailed(r, 'bash '); + return r.stdout; + } + + test('ws mode: stages the workstream-scoped archive_dir/phases_dir, not root', () => { + const rootPaths = { archive_dir: '/root/milestones', phases_dir: '/root/phases' }; + const wsPaths = { archive_dir: '/ws/milestones', phases_dir: '/ws/phases' }; + const out = runGitAddFence('--ws search', rootPaths, wsPaths); + assert.ok(out.includes('GIT_ADD_CALL: git add "/ws/milestones/" "/ws/phases/"'), + `expected the workstream-scoped dirs to be staged, got: ${out}`); + }); + + test('flat mode: stages the root archive_dir/phases_dir', () => { + const rootPaths = { archive_dir: '/root/milestones', phases_dir: '/root/phases' }; + const wsPaths = { archive_dir: '/ws/milestones', phases_dir: '/ws/phases' }; + const out = runGitAddFence('', rootPaths, wsPaths); + assert.ok(out.includes('GIT_ADD_CALL: git add "/root/milestones/" "/root/phases/"'), + `expected the root dirs to be staged, got: ${out}`); + }); + }); + + describe('step 7: init.new-milestone forwards --ws (round-trip file survives — steps 9/10 still need it)', () => { + const step7Fence = extractFenceContaining( + content, '## 7. Load Context and Resolve Models', 'Extract from init JSON', 'init.new-milestone', + ); + + test('ws mode: forwards --ws alongside --reset-phase-numbers', () => { + fs.writeFileSync(path.join(tmpDir, '.planning', '.gsd-ws-arg'), '--ws search'); + const gsdRunStub = 'gsd_run() { if [ "$1" = "query" ] && [ "$2" = "init.new-milestone" ]; then printf "gsd_run_call:%s\\n" "$*" >&2; echo "{}"; else echo "{}"; fi; }\n'; + const script = `ARGUMENTS="--reset-phase-numbers"\n${gsdRunStub}${step7Fence}`; + const r = runHookSeam('-c', [script], { interpreter: 'bash', cwd: tmpDir, env: runtimeDirEnv }); + throwIfFailed(r, 'bash '); + assert.match(r.stderr, /gsd_run_call:query init\.new-milestone --reset-phase-numbers --ws search\s*$/m, + `expected --ws to be forwarded alongside --reset-phase-numbers, got: ${r.stderr}`); + }); + + // #4456 code-review finding: Steps 9 and 10 (requirements/roadmap + // commits) run AFTER step 7 and still need to re-read .gsd-ws-arg — + // deleting it here (the original implementation) left them with no + // way to resolve REQUIREMENTS.md/ROADMAP.md/STATE.md under a + // workstream. See the "step 7 no longer deletes .gsd-ws-arg" test + // below and the step 10 describe block for the corrected cleanup. + test('does NOT remove .planning/.gsd-ws-arg — steps 9/10 still need it', () => { + fs.writeFileSync(path.join(tmpDir, '.planning', '.gsd-ws-arg'), '--ws search'); + const gsdRunStub = 'gsd_run() { echo "{}"; }\n'; + const r = runHookSeam('-c', [gsdRunStub + step7Fence], { interpreter: 'bash', cwd: tmpDir, env: runtimeDirEnv }); + throwIfFailed(r, 'bash '); + assert.ok(fs.existsSync(path.join(tmpDir, '.planning', '.gsd-ws-arg')), + '.gsd-ws-arg must survive step 7 — steps 9 and 10 run after it and still need to read the file'); + }); + }); + + describe('step 6: commit resolves PROJECT.md/STATE.md through init.new-milestone, not literal paths', () => { + const step6CommitFence = extractFenceContaining( + content, + '## 6. Cleanup and Commit', + '## 7. Load Context and Resolve Models', + 'docs: start milestone v[X.Y] [Name]' + ); + + function runStep6Commit(gsdWsArg, rootPaths, wsPaths) { + fs.writeFileSync(path.join(tmpDir, '.planning', '.gsd-ws-arg'), gsdWsArg); + const script = stubGsdRun(rootPaths, wsPaths) + step6CommitFence; + const r = runHookSeam('-c', [script], { interpreter: 'bash', cwd: tmpDir, env: runtimeDirEnv }); + throwIfFailed(r, 'bash '); + return r.stdout; + } + + // PROJECT.md is shared (stays root in both modes, #4455 follow-up); + // STATE.md is workstream-scoped and must resolve accordingly. + const rootPaths = { project_path: '/root/PROJECT.md', state_path: '/root/STATE.md' }; + const wsPaths = { project_path: '/root/PROJECT.md', state_path: '/ws/STATE.md' }; + + test('ws mode: --files uses the resolved workstream STATE.md, shared root PROJECT.md', () => { + const out = runStep6Commit('--ws search', rootPaths, wsPaths); assert.ok( - out.includes('--files .planning/PROJECT.md .planning/STATE.md'), - `expected PROJECT.md + STATE.md --files in ${mode} mode, got: ${out}` + out.includes('gsd_run_call:query commit docs: start milestone v[X.Y] [Name] --files /root/PROJECT.md /ws/STATE.md'), + `expected the resolved ws-mode paths in --files, got: ${out}` ); }); - } - test('does not guard the commit on GSD_WS — a cross-step variable is always empty here', () => { - // Regression guard for the inert-guard trap: GSD_WS is assigned in Step - // 1's shell, and each step's bash block runs in its own shell (the same - // reason Step 5 round-trips OUTGOING_MILESTONE through a file). A - // `[ -n "$GSD_WS" ]` branch here reads an unset variable, always takes - // the flat branch, and only appears to work. - assert.ok( - !/\[\s*-n\s*"\$GSD_WS"\s*\]/.test(step6CommitFence), - `step 6 must not branch on a cross-step GSD_WS; got fence:\n${step6CommitFence}` + test('flat mode: --files uses the resolved root paths for both', () => { + const out = runStep6Commit('', rootPaths, wsPaths); + assert.ok( + out.includes('gsd_run_call:query commit docs: start milestone v[X.Y] [Name] --files /root/PROJECT.md /root/STATE.md'), + `expected the resolved flat-mode paths in --files, got: ${out}` + ); + }); + + test('does not guard the commit on a bare cross-step GSD_WS variable (regression guard)', () => { + // Regression guard for the inert-guard trap: GSD_WS is assigned in Step + // 1's shell, and each step's bash block runs in its own shell. A + // `[ -n "$GSD_WS" ]` branch here would read an unset variable, always + // take the flat branch, and only appear to work — the fix instead + // reads $GSD_WS_ARG back from the persisted file. + assert.ok( + !/\[\s*-n\s*"\$GSD_WS"\s*\]/.test(step6CommitFence), + `step 6 must not branch on a bare cross-step GSD_WS; got fence:\n${step6CommitFence}` + ); + }); + }); + + // #4456 code-review finding: Steps 9 and 10 ALSO commit workstream-scoped + // files (REQUIREMENTS.md, ROADMAP.md, STATE.md) via literal root paths — + // the same bug class as Step 6, missed in the first pass. Because these + // steps run AFTER Step 7 (where .gsd-ws-arg was previously being deleted), + // fixing them required moving the round-trip file's cleanup to Step 10 — + // its true last consumer — instead of Step 7. + describe('step 9: requirements commit resolves REQUIREMENTS.md through init.new-milestone', () => { + const step9Fence = extractFenceContaining( + content, '## 9. Define Requirements', '## 10. Create Roadmap', 'docs: define milestone', ); + + function runStep9(gsdWsArg, rootPaths, wsPaths) { + fs.writeFileSync(path.join(tmpDir, '.planning', '.gsd-ws-arg'), gsdWsArg); + const script = stubGsdRun(rootPaths, wsPaths) + step9Fence; + const r = runHookSeam('-c', [script], { interpreter: 'bash', cwd: tmpDir, env: runtimeDirEnv }); + throwIfFailed(r, 'bash '); + return r.stdout; + } + + const rootPaths = { requirements_path: '/root/REQUIREMENTS.md' }; + const wsPaths = { requirements_path: '/ws/REQUIREMENTS.md' }; + + test('ws mode: --files uses the resolved workstream REQUIREMENTS.md', () => { + const out = runStep9('--ws search', rootPaths, wsPaths); + assert.ok( + out.includes('gsd_run_call:query commit docs: define milestone v[X.Y] requirements --files /ws/REQUIREMENTS.md'), + `expected the resolved ws-mode path, got: ${out}` + ); + }); + + test('flat mode: --files uses the resolved root REQUIREMENTS.md', () => { + const out = runStep9('', rootPaths, wsPaths); + assert.ok( + out.includes('gsd_run_call:query commit docs: define milestone v[X.Y] requirements --files /root/REQUIREMENTS.md'), + `expected the resolved flat-mode path, got: ${out}` + ); + }); + }); + + describe('step 10: roadmap commit resolves ROADMAP/STATE/REQUIREMENTS through init.new-milestone, then cleans up .gsd-ws-arg', () => { + const step10Fence = extractFenceContaining( + content, '## 10. Create Roadmap', '## 10.5.', 'docs: create milestone v[X.Y] roadmap', + ); + + function runStep10(gsdWsArg, rootPaths, wsPaths) { + fs.writeFileSync(path.join(tmpDir, '.planning', '.gsd-ws-arg'), gsdWsArg); + const script = stubGsdRun(rootPaths, wsPaths) + step10Fence; + const r = runHookSeam('-c', [script], { interpreter: 'bash', cwd: tmpDir, env: runtimeDirEnv }); + throwIfFailed(r, 'bash '); + return r.stdout; + } + + const rootPaths = { roadmap_path: '/root/ROADMAP.md', state_path: '/root/STATE.md', requirements_path: '/root/REQUIREMENTS.md' }; + const wsPaths = { roadmap_path: '/ws/ROADMAP.md', state_path: '/ws/STATE.md', requirements_path: '/ws/REQUIREMENTS.md' }; + + test('ws mode: --files uses all three resolved workstream paths', () => { + const out = runStep10('--ws search', rootPaths, wsPaths); + assert.ok( + out.includes('gsd_run_call:query commit docs: create milestone v[X.Y] roadmap ([N] phases) --files /ws/ROADMAP.md /ws/STATE.md /ws/REQUIREMENTS.md'), + `expected the resolved ws-mode paths, got: ${out}` + ); + }); + + test('flat mode: --files uses all three resolved root paths', () => { + const out = runStep10('', rootPaths, wsPaths); + assert.ok( + out.includes('gsd_run_call:query commit docs: create milestone v[X.Y] roadmap ([N] phases) --files /root/ROADMAP.md /root/STATE.md /root/REQUIREMENTS.md'), + `expected the resolved flat-mode paths, got: ${out}` + ); + }); + + test('removes .planning/.gsd-ws-arg after this commit (the true last consumer, not step 7)', () => { + runStep10('--ws search', rootPaths, wsPaths); + assert.ok(!fs.existsSync(path.join(tmpDir, '.planning', '.gsd-ws-arg')), + '.gsd-ws-arg should be cleaned up here, since steps 9 and 10 still need it after step 7'); + }); + }); + + test('step 7 no longer deletes .gsd-ws-arg (steps 9/10 still need it)', () => { + const step7Fence = extractFenceContaining( + content, '## 7. Load Context and Resolve Models', 'Extract from init JSON', 'init.new-milestone', + ); + assert.ok(!step7Fence.includes('rm -f .planning/.gsd-ws-arg'), + 'step 7 must not delete .gsd-ws-arg — steps 9 and 10 run after it and still need to read the file'); }); });