From 4c7c64f7f535e1cfb6cb8e5f42f5f480429ce62b Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 17 Mar 2026 10:22:16 -0400 Subject: [PATCH] fix: completed milestone with summary re-entered as active on resume (#864) (#868) --- src/resources/extensions/gsd/state.ts | 17 +++-- .../extensions/gsd/tests/derive-state.test.ts | 70 +++++++++++++++++++ src/tests/github-client.test.ts | 7 +- 3 files changed, 86 insertions(+), 8 deletions(-) diff --git a/src/resources/extensions/gsd/state.ts b/src/resources/extensions/gsd/state.ts index ef76fb648..d99a41bbb 100644 --- a/src/resources/extensions/gsd/state.ts +++ b/src/resources/extensions/gsd/state.ts @@ -294,19 +294,26 @@ async function _deriveStateImpl(basePath: string): Promise { if (complete) { // All slices done — check validation and summary state + const summaryFile = resolveMilestoneFile(basePath, mid, "SUMMARY"); const validationFile = resolveMilestoneFile(basePath, mid, "VALIDATION"); const validationContent = validationFile ? await cachedLoadFile(validationFile) : null; const validationTerminal = validationContent ? isValidationTerminal(validationContent) : false; - const summaryFile = resolveMilestoneFile(basePath, mid, "SUMMARY"); - if (!validationTerminal && !activeMilestoneFound) { - // No terminal validation yet → validating-milestone + if (summaryFile) { + // Summary exists → milestone is complete regardless of validation state. + // The summary is the terminal artifact (#864). + registry.push({ id: mid, title, status: 'complete' }); + } else if (!validationTerminal && !activeMilestoneFound) { + // No summary and no terminal validation → validating-milestone activeMilestone = { id: mid, title }; activeRoadmap = roadmap; activeMilestoneFound = true; registry.push({ id: mid, title, status: 'active' }); - } else if (!summaryFile && !activeMilestoneFound) { - // Validated but no summary written yet → completing-milestone + } else if (!validationTerminal && activeMilestoneFound) { + // No summary and no terminal validation, but another milestone is already active + registry.push({ id: mid, title, status: 'pending' }); + } else if (!activeMilestoneFound) { + // Terminal validation but no summary → completing-milestone activeMilestone = { id: mid, title }; activeRoadmap = roadmap; activeMilestoneFound = true; diff --git a/src/resources/extensions/gsd/tests/derive-state.test.ts b/src/resources/extensions/gsd/tests/derive-state.test.ts index 20f21153d..f924e829b 100644 --- a/src/resources/extensions/gsd/tests/derive-state.test.ts +++ b/src/resources/extensions/gsd/tests/derive-state.test.ts @@ -700,6 +700,76 @@ slice: S01 } } + // ─── Test: completed M001 (summary, no validation) skipped for active M003 (#864) ──── + console.log('\n=== completed milestone with summary but no validation is not active (#864) ==='); + { + const base = createFixtureBase(); + try { + // M001: all slices done, has summary, no validation + writeRoadmap(base, 'M001', `# M001: First Milestone\n\n**Vision:** Done.\n\n## Slices\n\n- [x] **S01: Done slice** \`risk:low\` \`depends:[]\`\n > Completed.\n`); + writeMilestoneSummary(base, 'M001', '---\nid: M001\n---\n\n# M001: First Milestone\n\n**Completed.**'); + // M003: incomplete, should be active + writeRoadmap(base, 'M003', `# M003: Active Milestone\n\n**Vision:** Do stuff.\n\n## Slices\n\n- [ ] **S01: Work slice** \`risk:low\` \`depends:[]\`\n > Needs work.\n`); + + const state = await deriveState(base); + assertEq(state.activeMilestone?.id, 'M003', 'active milestone is M003, not completed M001'); + const m001Entry = state.registry.find(e => e.id === 'M001'); + assertEq(m001Entry?.status, 'complete', 'M001 is marked complete despite no validation'); + } finally { + cleanup(base); + } + } + + // ─── Test: completed M001 with summary AND validation is complete (#864) ──── + console.log('\n=== completed milestone with summary and validation is complete ==='); + { + const base = createFixtureBase(); + try { + writeRoadmap(base, 'M001', `# M001: First Milestone\n\n**Vision:** Done.\n\n## Slices\n\n- [x] **S01: Done slice** \`risk:low\` \`depends:[]\`\n > Completed.\n`); + writeMilestoneSummary(base, 'M001', '---\nid: M001\n---\n\n# M001: First Milestone\n\n**Completed.**'); + writeMilestoneValidation(base, 'M001', 'pass'); + writeRoadmap(base, 'M003', `# M003: Active Milestone\n\n**Vision:** Do stuff.\n\n## Slices\n\n- [ ] **S01: Work slice** \`risk:low\` \`depends:[]\`\n > Needs work.\n`); + + const state = await deriveState(base); + assertEq(state.activeMilestone?.id, 'M003', 'active milestone is M003'); + const m001Entry = state.registry.find(e => e.id === 'M001'); + assertEq(m001Entry?.status, 'complete', 'M001 with both summary and validation is complete'); + } finally { + cleanup(base); + } + } + + // ─── Test: all slices done, no summary, no validation → needs validation (#864) ──── + console.log('\n=== all slices done, no summary, no validation → validating-milestone ==='); + { + const base = createFixtureBase(); + try { + writeRoadmap(base, 'M001', `# M001: First Milestone\n\n**Vision:** Validate me.\n\n## Slices\n\n- [x] **S01: Done slice** \`risk:low\` \`depends:[]\`\n > Completed.\n`); + // No summary, no validation — this should be active for validation + + const state = await deriveState(base); + assertEq(state.activeMilestone?.id, 'M001', 'M001 is active for validation'); + } finally { + cleanup(base); + } + } + + // ─── Test: all slices done, validation pass, no summary → needs completion (#864) ──── + console.log('\n=== all slices done, validation pass, no summary → completing-milestone ==='); + { + const base = createFixtureBase(); + try { + writeRoadmap(base, 'M001', `# M001: First Milestone\n\n**Vision:** Complete me.\n\n## Slices\n\n- [x] **S01: Done slice** \`risk:low\` \`depends:[]\`\n > Completed.\n`); + writeMilestoneValidation(base, 'M001', 'pass'); + // No summary — validated but not yet completed + + const state = await deriveState(base); + assertEq(state.activeMilestone?.id, 'M001', 'M001 is active for completion'); + } finally { + cleanup(base); + } + } + report(); } diff --git a/src/tests/github-client.test.ts b/src/tests/github-client.test.ts index 41b9ed52f..7d9fc5708 100644 --- a/src/tests/github-client.test.ts +++ b/src/tests/github-client.test.ts @@ -137,10 +137,11 @@ describe("createGitHubClient — Octokit instantiation", () => { describe("getRepoInfo — detects repo from git working directory", () => { it("returns owner/repo for the current repository", async () => { const info = await getRepoInfo(process.cwd()); - // This test repo is gsd-build/gsd-2 + // Verifies getRepoInfo can parse the origin remote — owner varies + // depending on whether this is the upstream repo or a fork. assert.notEqual(info, null); - assert.equal(info!.owner, "gsd-build"); - assert.equal(info!.repo, "gsd-2" /* or GSD-2 depending on remote */); + assert.ok(info!.owner.length > 0, "owner should be non-empty"); + assert.ok(info!.repo.toLowerCase().includes("gsd-2"), "repo should contain gsd-2"); }); it("returns null for a non-git directory", async () => {