fix: treat summary as terminal artifact even when roadmap slices are unchecked (#1632)
When a milestone has a roadmap with unchecked slice checkboxes AND a summary file, deriveState() incorrectly treated it as incomplete. The summary check only ran inside the `if (isMilestoneComplete(roadmap))` branch, so it was never reached when checkboxes weren't ticked. This caused auto-mode to pick an already-completed milestone as active, ignoring the actual current milestone entirely. The fix adds summary-existence checks to all three resolution paths: 1. `getActiveMilestoneId()` — now checks for summary before returning a milestone as incomplete 2. Phase 1 pre-scan in `deriveState()` — now adds milestones with unchecked roadmaps + summaries to `completeMilestoneIds` 3. Phase 2 registry builder — now checks for summary before falling through to the active/pending logic This is consistent with the existing principle that the summary is the terminal artifact (#864), which was already stated in a comment but not enforced for the unchecked-roadmap case. Adds two tests: - Unchecked roadmap + summary → status is 'complete', next milestone is active - Unchecked roadmap + summary satisfies depends_on for downstream milestones
This commit is contained in:
parent
6f15ddcbf7
commit
0ec2ae020f
2 changed files with 82 additions and 21 deletions
|
|
@ -126,7 +126,12 @@ export async function getActiveMilestoneId(basePath: string): Promise<string | n
|
|||
// A draft milestone is still "active" — this function only determines which milestone is current.
|
||||
}
|
||||
const roadmap = parseRoadmap(content);
|
||||
if (!isMilestoneComplete(roadmap)) return mid;
|
||||
if (!isMilestoneComplete(roadmap)) {
|
||||
// Summary is the terminal artifact — if it exists, the milestone is
|
||||
// complete even when roadmap checkboxes weren't ticked (#864).
|
||||
const summaryFile = resolveMilestoneFile(basePath, mid, "SUMMARY");
|
||||
if (!summaryFile) return mid;
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
|
@ -258,7 +263,13 @@ async function _deriveStateImpl(basePath: string): Promise<GSDState> {
|
|||
}
|
||||
const rmap = parseRoadmap(rc);
|
||||
roadmapCache.set(mid, rmap);
|
||||
if (!isMilestoneComplete(rmap)) continue;
|
||||
if (!isMilestoneComplete(rmap)) {
|
||||
// Summary is the terminal artifact — if it exists, the milestone is
|
||||
// complete even when roadmap checkboxes weren't ticked (#864).
|
||||
const sf = resolveMilestoneFile(basePath, mid, "SUMMARY");
|
||||
if (sf) completeMilestoneIds.add(mid);
|
||||
continue;
|
||||
}
|
||||
const sf = resolveMilestoneFile(basePath, mid, "SUMMARY");
|
||||
if (sf) completeMilestoneIds.add(mid);
|
||||
}
|
||||
|
|
@ -357,26 +368,33 @@ async function _deriveStateImpl(basePath: string): Promise<GSDState> {
|
|||
} else {
|
||||
registry.push({ id: mid, title, status: 'complete' });
|
||||
}
|
||||
} else if (!activeMilestoneFound) {
|
||||
// Check milestone-level dependencies before promoting to active
|
||||
const contextFile = resolveMilestoneFile(basePath, mid, "CONTEXT");
|
||||
const contextContent = contextFile ? await cachedLoadFile(contextFile) : null;
|
||||
const deps = parseContextDependsOn(contextContent);
|
||||
const depsUnmet = deps.some(dep => !completeMilestoneIds.has(dep));
|
||||
if (depsUnmet) {
|
||||
registry.push({ id: mid, title, status: 'pending', dependsOn: deps });
|
||||
// Do NOT set activeMilestoneFound — let the loop continue to the next milestone
|
||||
} else {
|
||||
activeMilestone = { id: mid, title };
|
||||
activeRoadmap = roadmap;
|
||||
activeMilestoneFound = true;
|
||||
registry.push({ id: mid, title, status: 'active', ...(deps.length > 0 ? { dependsOn: deps } : {}) });
|
||||
}
|
||||
} else {
|
||||
const contextFile2 = resolveMilestoneFile(basePath, mid, "CONTEXT");
|
||||
const contextContent2 = contextFile2 ? await cachedLoadFile(contextFile2) : null;
|
||||
const deps2 = parseContextDependsOn(contextContent2);
|
||||
registry.push({ id: mid, title, status: 'pending', ...(deps2.length > 0 ? { dependsOn: deps2 } : {}) });
|
||||
// Roadmap slices not all checked — but if a summary exists, the milestone
|
||||
// is still complete. The summary is the terminal artifact (#864).
|
||||
const summaryFile = resolveMilestoneFile(basePath, mid, "SUMMARY");
|
||||
if (summaryFile) {
|
||||
registry.push({ id: mid, title, status: 'complete' });
|
||||
} else if (!activeMilestoneFound) {
|
||||
// Check milestone-level dependencies before promoting to active
|
||||
const contextFile = resolveMilestoneFile(basePath, mid, "CONTEXT");
|
||||
const contextContent = contextFile ? await cachedLoadFile(contextFile) : null;
|
||||
const deps = parseContextDependsOn(contextContent);
|
||||
const depsUnmet = deps.some(dep => !completeMilestoneIds.has(dep));
|
||||
if (depsUnmet) {
|
||||
registry.push({ id: mid, title, status: 'pending', dependsOn: deps });
|
||||
// Do NOT set activeMilestoneFound — let the loop continue to the next milestone
|
||||
} else {
|
||||
activeMilestone = { id: mid, title };
|
||||
activeRoadmap = roadmap;
|
||||
activeMilestoneFound = true;
|
||||
registry.push({ id: mid, title, status: 'active', ...(deps.length > 0 ? { dependsOn: deps } : {}) });
|
||||
}
|
||||
} else {
|
||||
const contextFile2 = resolveMilestoneFile(basePath, mid, "CONTEXT");
|
||||
const contextContent2 = contextFile2 ? await cachedLoadFile(contextFile2) : null;
|
||||
const deps2 = parseContextDependsOn(contextContent2);
|
||||
registry.push({ id: mid, title, status: 'pending', ...(deps2.length > 0 ? { dependsOn: deps2 } : {}) });
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -779,6 +779,49 @@ slice: S01
|
|||
}
|
||||
}
|
||||
|
||||
// ─── Test: unchecked roadmap slices + summary → complete (summary is terminal) ────
|
||||
console.log('\n=== unchecked roadmap slices + summary → complete (summary is terminal) ===');
|
||||
{
|
||||
const base = createFixtureBase();
|
||||
try {
|
||||
// M001: roadmap has unchecked slices but a summary exists — should be complete
|
||||
writeRoadmap(base, 'M001', `# M001: First Milestone\n\n**Vision:** Already done.\n\n## Slices\n\n- [ ] **S01: Unchecked slice** \`risk:low\` \`depends:[]\`\n > Work was done but checkbox never ticked.\n- [ ] **S02: Another unchecked** \`risk:low\` \`depends:[]\`\n > Same.\n`);
|
||||
writeMilestoneSummary(base, 'M001', '---\nid: M001\n---\n\n# M001: First Milestone\n\n**Completed despite unchecked roadmap.**');
|
||||
// M002: genuinely incomplete — should be the active milestone
|
||||
writeRoadmap(base, 'M002', `# M002: 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);
|
||||
const m001Entry = state.registry.find(e => e.id === 'M001');
|
||||
assertEq(m001Entry?.status, 'complete', 'M001 with unchecked roadmap + summary is complete');
|
||||
assertEq(state.activeMilestone?.id, 'M002', 'active milestone is M002, not M001');
|
||||
} finally {
|
||||
cleanup(base);
|
||||
}
|
||||
}
|
||||
|
||||
// ─── Test: unchecked roadmap + summary counts toward completeMilestoneIds (deps) ────
|
||||
console.log('\n=== unchecked roadmap + summary satisfies dependency ===');
|
||||
{
|
||||
const base = createFixtureBase();
|
||||
try {
|
||||
// M001: unchecked roadmap + summary → complete
|
||||
writeRoadmap(base, 'M001', `# M001: Foundation\n\n**Vision:** Done.\n\n## Slices\n\n- [ ] **S01: Setup** \`risk:low\` \`depends:[]\`\n > Done.\n`);
|
||||
writeMilestoneSummary(base, 'M001', '---\nid: M001\n---\n\n# M001: Foundation\n\n**Done.**');
|
||||
// M002: depends on M001 — should be active since M001 is complete
|
||||
writeRoadmap(base, 'M002', `# M002: Dependent\n\n**Vision:** Depends on M001.\n\n## Slices\n\n- [ ] **S01: Work** \`risk:low\` \`depends:[]\`\n > Work.\n`);
|
||||
const contextDir = join(base, '.gsd', 'milestones', 'M002');
|
||||
mkdirSync(contextDir, { recursive: true });
|
||||
writeFileSync(join(contextDir, 'M002-CONTEXT.md'), '---\ndepends_on:\n - M001\n---\n\n# M002 Context\n\nDepends on M001.');
|
||||
|
||||
const state = await deriveState(base);
|
||||
assertEq(state.activeMilestone?.id, 'M002', 'M002 is active — M001 dependency satisfied via summary');
|
||||
const m002Entry = state.registry.find(e => e.id === 'M002');
|
||||
assertEq(m002Entry?.status, 'active', 'M002 status is active, not pending');
|
||||
} finally {
|
||||
cleanup(base);
|
||||
}
|
||||
}
|
||||
|
||||
report();
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue