diff --git a/sdk/src/query/phase-lifecycle.ts b/sdk/src/query/phase-lifecycle.ts index 0e4ebb377..5211c46a6 100644 --- a/sdk/src/query/phase-lifecycle.ts +++ b/sdk/src/query/phase-lifecycle.ts @@ -1256,12 +1256,31 @@ export const phaseComplete: QueryHandler = async (args, projectDir, workstream) let nextPhaseNum: string | null = null; let nextPhaseName: string | null = null; let isLastPhase = true; + // Tracks whether the completed phase belongs to the primary milestone in STATE.md. + // When false (parallel-milestone case, Bug #2676), the milestone filter is bypassed + // for next-phase detection so phases from the same secondary milestone are visible. + let completedPhaseInPrimaryMilestone = true; try { const isDirInMilestone = await getMilestonePhaseFilter(projectDir, workstream); const entries = await readdir(paths.phases, { withFileTypes: true }); - const dirs = entries.filter(e => e.isDirectory()).map(e => e.name) - .filter(isDirInMilestone) + const allDirs = entries.filter(e => e.isDirectory()).map(e => e.name); + + // Guard: if the completed phase's directory is not in the current-milestone filter + // set, the filter was built from a different (primary) milestone in STATE.md. + // In that case skip the filter so we can find the true next phase on disk. + // This handles parallel-milestone workflows where STATE.md's `milestone:` field + // points at the primary milestone but the phase being completed belongs to a + // secondary in-flight milestone. (Bug #2676) + const completedDirInFilter = allDirs.some((d) => { + const dm = d.match(/^(\d+[A-Z]?(?:\.\d+)*)-?/i); + return dm && comparePhaseNum(dm[1], phaseNum) === 0 && isDirInMilestone(d); + }); + completedPhaseInPrimaryMilestone = completedDirInFilter; + const effectiveFilter = completedDirInFilter ? isDirInMilestone : (_d: string) => true; + + const dirs = allDirs + .filter(effectiveFilter) .sort((a, b) => comparePhaseNum(a, b)); for (const dir of dirs) { @@ -1277,11 +1296,16 @@ export const phaseComplete: QueryHandler = async (args, projectDir, workstream) } } catch { /* intentionally empty */ } - // Fallback: check ROADMAP.md for phases not yet scaffolded + // Fallback: check ROADMAP.md for phases not yet scaffolded. + // When the completed phase is from a parallel (non-primary) milestone, scan the + // full ROADMAP rather than the primary-milestone slice so 41.3 is visible when + // completing 41.2 for a secondary milestone. (Bug #2676) if (isLastPhase && existsSync(paths.roadmap)) { try { const roadmapContent = await readFile(paths.roadmap, 'utf-8'); - const roadmapForPhases = await extractCurrentMilestone(roadmapContent, projectDir); + const roadmapForPhases = completedPhaseInPrimaryMilestone + ? await extractCurrentMilestone(roadmapContent, projectDir) + : roadmapContent; const phasePattern = /#{2,4}\s*Phase\s+(\d+[A-Z]?(?:\.\d+)*)\s*:\s*([^\n]+)/gi; let pm: RegExpExecArray | null; while ((pm = phasePattern.exec(roadmapForPhases)) !== null) { diff --git a/tests/bug-2676-parallel-milestone-phase-complete.test.cjs b/tests/bug-2676-parallel-milestone-phase-complete.test.cjs new file mode 100644 index 000000000..9b6ab705a --- /dev/null +++ b/tests/bug-2676-parallel-milestone-phase-complete.test.cjs @@ -0,0 +1,182 @@ +/** + * Regression tests for bug #2676: + * `gsd-sdk query phase.complete ` returns is_last_phase: true + * when the completed phase belongs to a milestone that is not the + * primary milestone recorded in STATE.md's `milestone:` field. + * + * Root cause: Step E of phaseComplete applies getMilestonePhaseFilter + * unconditionally. getMilestonePhaseFilter extracts phases from the + * milestone slice selected by STATE.md's `milestone:` field. When + * completing phase 41.2 (which belongs to vB) but STATE.md points at + * vA, all 41.x directories are excluded from the candidate set and + * the empty set causes isLastPhase = true. + * + * Fix: before applying the filter, check if the completed phase itself + * passes it. If not (parallel-milestone case), skip the filter entirely + * so all filesystem phases are visible for next-phase detection. + */ + +const { describe, test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); +const { createTempProject, cleanup } = require('./helpers.cjs'); + +const SDK_CLI = path.join(__dirname, '..', 'sdk', 'dist', 'cli.js'); +const { execFileSync } = require('child_process'); + +function runSdkQuery(args, cwd) { + try { + const result = execFileSync(process.execPath, [SDK_CLI, 'query', ...args], { + cwd, + encoding: 'utf-8', + stdio: ['pipe', 'pipe', 'pipe'], + }); + const parsed = JSON.parse(result.trim()); + return { success: true, data: parsed }; + } catch (err) { + const stderr = err.stderr?.toString().trim() || ''; + const stdout = err.stdout?.toString().trim() || ''; + try { + const parsed = JSON.parse(stdout); + return { success: true, data: parsed }; + } catch { + /* not JSON */ + } + return { success: false, error: stderr || err.message }; + } +} + +// ROADMAP.md with two active milestones: v1.0 (phases 10, 11) and v2.0 (phases 41.1, 41.2, 41.3). +// Using numeric version IDs so extractCurrentMilestone can correctly detect milestone boundaries. +const PARALLEL_ROADMAP = `# Roadmap + +## v1.0 Milestone (Primary) + +### Phase 10: Foo +**Goal:** Foo work + +### Phase 11: Bar +**Goal:** Bar work + +## v2.0 Milestone (Parallel) + +### Phase 41.1: Baz +**Goal:** Baz work + +### Phase 41.2: Qux +**Goal:** Qux work + +### Phase 41.3: Quux +**Goal:** Quux work +`; + +// STATE.md with milestone pointing at v1.0 (not v2.0). +// Uses YAML frontmatter so extractCurrentMilestone can read the `milestone:` field. +const STATE_POINTING_V1 = `--- +milestone: v1.0 +milestone_name: Primary Milestone +--- +# State + +**Current Phase:** 41.2 +**Status:** In progress +**Last Activity:** 2026-04-25 +**Last Activity Description:** Working on qux +`; + +describe('bug #2676: phase.complete respects parallel milestone routing', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + // Write ROADMAP.md and STATE.md + fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), PARALLEL_ROADMAP); + fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), STATE_POINTING_V1); + + // Create filesystem phase directories for vA (primary milestone) + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '10-foo'), { recursive: true }); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '11-bar'), { recursive: true }); + + // Create filesystem phase directories for vB (parallel milestone) + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '41.1-baz'), { recursive: true }); + + const phase412 = path.join(tmpDir, '.planning', 'phases', '41.2-qux'); + fs.mkdirSync(phase412, { recursive: true }); + fs.writeFileSync(path.join(phase412, '41.2-01-PLAN.md'), '# Plan'); + fs.writeFileSync(path.join(phase412, '41.2-01-SUMMARY.md'), '# Summary'); + + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '41.3-quux'), { recursive: true }); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('phase.complete 41.2 returns is_last_phase: false when 41.3 exists', () => { + // BUG: before the fix this returns is_last_phase: true because the + // milestone filter (built from vA's phases: 10, 11) excludes all 41.x dirs, + // leaving an empty candidate set and defaulting isLastPhase to true. + const result = runSdkQuery(['phase.complete', '41.2'], tmpDir); + + assert.ok(result.success, `phase.complete failed: ${result.error}`); + assert.strictEqual( + result.data.is_last_phase, + false, + `expected is_last_phase: false but got true — parallel milestone filter not bypassed. ` + + `next_phase was: ${result.data.next_phase}` + ); + }); + + test('phase.complete 41.2 returns next_phase pointing at 41.3', () => { + const result = runSdkQuery(['phase.complete', '41.2'], tmpDir); + + assert.ok(result.success, `phase.complete failed: ${result.error}`); + assert.ok( + result.data.next_phase !== null, + `next_phase should not be null when 41.3 exists — got: ${JSON.stringify(result.data.next_phase)}` + ); + // next_phase may be returned as "41.3" or "41" depending on dir name matching + assert.match( + String(result.data.next_phase), + /^41\.3/, + `next_phase should start with 41.3, got: ${result.data.next_phase}` + ); + }); + + test('phase.complete for vA phase still uses milestone filter normally', () => { + // Completing phase 10 (in vA): the filter includes it, so the candidate + // set for next-phase is {10, 11} and next should be 11. + const phase10 = path.join(tmpDir, '.planning', 'phases', '10-foo'); + fs.writeFileSync(path.join(phase10, '10-01-PLAN.md'), '# Plan'); + fs.writeFileSync(path.join(phase10, '10-01-SUMMARY.md'), '# Summary'); + + const result = runSdkQuery(['phase.complete', '10'], tmpDir); + + assert.ok(result.success, `phase.complete 10 failed: ${result.error}`); + assert.strictEqual(result.data.is_last_phase, false, 'phase 10 should not be last (11 follows)'); + assert.match( + String(result.data.next_phase ?? ''), + /^11/, + `next_phase should point to 11, got: ${result.data.next_phase}` + ); + }); + + test('phase.complete for actual last phase of vA still returns is_last_phase: true', () => { + // Completing phase 11 (last in vA): the filter includes phases 10 and 11, + // nothing higher in the vA milestone, so is_last_phase should be true + // (even though 41.x dirs exist on disk for vB). + const phase11 = path.join(tmpDir, '.planning', 'phases', '11-bar'); + fs.writeFileSync(path.join(phase11, '11-01-PLAN.md'), '# Plan'); + fs.writeFileSync(path.join(phase11, '11-01-SUMMARY.md'), '# Summary'); + + const result = runSdkQuery(['phase.complete', '11'], tmpDir); + + assert.ok(result.success, `phase.complete 11 failed: ${result.error}`); + assert.strictEqual( + result.data.is_last_phase, + true, + `phase 11 is last in vA; is_last_phase should be true even with vB dirs on disk` + ); + }); +}); diff --git a/tests/bug-2684-milestone-complete-version.test.cjs b/tests/bug-2684-milestone-complete-version.test.cjs new file mode 100644 index 000000000..aaa449e0d --- /dev/null +++ b/tests/bug-2684-milestone-complete-version.test.cjs @@ -0,0 +1,125 @@ +/** + * Regression tests for bug #2684: + * `gsd-sdk query milestone.complete ` always fails with + * GSDError: version required for phases archive. + * + * Root cause: milestoneComplete extracted version from args[0] but passed + * [] instead of args (or [version]) to phasesArchive, so phasesArchive + * never received the version string and threw immediately. + * + * Fix: pass args (or [version]) when delegating to phasesArchive. + */ + +const { describe, test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); +const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs'); + +const SDK_CLI = path.join(__dirname, '..', 'sdk', 'dist', 'cli.js'); +const { execFileSync } = require('child_process'); + +function runSdkQuery(args, cwd) { + try { + const result = execFileSync(process.execPath, [SDK_CLI, 'query', ...args], { + cwd, + encoding: 'utf-8', + stdio: ['pipe', 'pipe', 'pipe'], + }); + const parsed = JSON.parse(result.trim()); + return { success: true, data: parsed }; + } catch (err) { + const stderr = err.stderr?.toString().trim() || ''; + const stdout = err.stdout?.toString().trim() || ''; + // If the output is JSON despite non-zero exit, parse it + try { + const parsed = JSON.parse(stdout); + return { success: true, data: parsed }; + } catch { + /* not JSON */ + } + return { success: false, error: stderr || err.message }; + } +} + +describe('bug #2684: milestone.complete forwards version to phases.archive', () => { + let tmpDir; + + beforeEach(() => { + tmpDir = createTempProject(); + }); + + afterEach(() => { + cleanup(tmpDir); + }); + + test('milestone.complete v1.0 does not throw version required error', () => { + // Minimal project: ROADMAP.md so milestone filter can run, one phase dir + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap\n\n### Phase 1: Foundation\n**Goal:** Setup\n` + ); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '01-foundation'), { recursive: true }); + + const result = runSdkQuery(['milestone.complete', 'v1.0'], tmpDir); + + assert.ok( + result.success, + `milestone.complete should succeed, got error: ${result.error}` + ); + assert.ok( + !result.error || !result.error.includes('version required'), + `should not throw "version required" — got: ${result.error}` + ); + }); + + test('milestone.complete returns version in response data', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap\n\n### Phase 1: Foundation\n**Goal:** Setup\n` + ); + + const result = runSdkQuery(['milestone.complete', 'v2.5'], tmpDir); + + assert.ok(result.success, `Command failed: ${result.error}`); + assert.strictEqual(result.data.version, 'v2.5', 'version should be echoed in response'); + }); + + test('milestone.complete with --archive-phases forwards version correctly', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap\n\n### Phase 1: Foundation\n**Goal:** Setup\n` + ); + const phaseDir = path.join(tmpDir, '.planning', 'phases', '01-foundation'); + fs.mkdirSync(phaseDir, { recursive: true }); + fs.writeFileSync(path.join(phaseDir, '01-01-PLAN.md'), '# Plan'); + fs.writeFileSync(path.join(phaseDir, '01-01-SUMMARY.md'), '# Summary'); + + // With --archive-phases, the version must reach the archive logic + // Without the fix this would throw "version required for phases archive" + const result = runSdkQuery(['milestone.complete', 'v1.0', '--archive-phases'], tmpDir); + + assert.ok(result.success, `milestone.complete --archive-phases failed: ${result.error}`); + assert.strictEqual(result.data.version, 'v1.0'); + // The archive flag should have moved the phase dir + assert.ok( + result.data.archived.phases === true, + 'phases should be archived when --archive-phases is passed' + ); + const archiveDir = path.join(tmpDir, '.planning', 'milestones', 'v1.0-phases'); + assert.ok(fs.existsSync(archiveDir), 'archive directory should exist'); + }); + + test('phases.archive v1.0 (direct call, workaround) also works', () => { + fs.writeFileSync( + path.join(tmpDir, '.planning', 'ROADMAP.md'), + `# Roadmap\n\n### Phase 1: Foundation\n**Goal:** Setup\n` + ); + fs.mkdirSync(path.join(tmpDir, '.planning', 'phases', '01-foundation'), { recursive: true }); + + const result = runSdkQuery(['phases.archive', 'v1.0'], tmpDir); + + assert.ok(result.success, `phases.archive failed: ${result.error}`); + assert.strictEqual(result.data.version, 'v1.0'); + }); +});