fix: completed milestone with summary re-entered as active on resume (#864) (#868)

This commit is contained in:
Tom Boucher 2026-03-17 10:22:16 -04:00 committed by GitHub
parent 9fe046d3fa
commit 4c7c64f7f5
3 changed files with 86 additions and 8 deletions

View file

@ -294,19 +294,26 @@ async function _deriveStateImpl(basePath: string): Promise<GSDState> {
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;

View file

@ -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();
}

View file

@ -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 () => {