fix(doctor): gate roadmap checkbox on summary existing on disk, not issue detection (#1915)

The roadmap-done condition checked whether the missing-summary issue was
detected in the issues array, but at fixLevel="task" the summary is
detected and never fixed (deferred via COMPLETION_TRANSITION_CODES).
This caused the roadmap checkbox to be marked without the summary on
disk, making deriveState() skip the summarizing phase and hard-stop at
validating-milestone.

Replace the issues.some() fallback with an existsSync re-check so the
roadmap is only marked when the summary actually exists — either
pre-existing or created earlier in the same doctor run.

Fixes #1910

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tom Boucher 2026-03-21 23:06:53 -04:00 committed by GitHub
parent 53d7350e0d
commit 09f3a5f970
4 changed files with 188 additions and 15 deletions

View file

@ -960,7 +960,7 @@ export async function runGSDDoctor(basePath: string, options?: { fix?: boolean;
fixable: true,
});
dryRunCanFix("all_tasks_done_roadmap_not_checked", `mark ${slice.id} done in roadmap`);
if (shouldFix("all_tasks_done_roadmap_not_checked") && (hasSliceSummary || issues.some(issue => issue.code === "all_tasks_done_missing_slice_summary" && issue.unitId === unitId))) {
if (shouldFix("all_tasks_done_roadmap_not_checked") && (hasSliceSummary || existsSync(join(slicePath, `${slice.id}-SUMMARY.md`)))) {
await markSliceDoneInRoadmap(basePath, milestoneId, slice.id, fixesApplied);
}
}

View file

@ -80,7 +80,7 @@ test("COMPLETION_TRANSITION_CODES only contains slice summary code", () => {
);
});
test("fixLevel:task — fixes roadmap checkbox and UAT stub immediately, defers only summary (#1808)", async () => {
test("fixLevel:task — fixes UAT stub immediately, defers summary and roadmap checkbox (#1808, #1910)", async () => {
const tmp = makeTmp("partial-deferral");
try {
buildScaffold(tmp);
@ -101,15 +101,16 @@ test("fixLevel:task — fixes roadmap checkbox and UAT stub immediately, defers
const sliceUatPath = join(tmp, ".gsd", "milestones", "M001", "slices", "S01", "S01-UAT.md");
assert.ok(existsSync(sliceUatPath), "should have created UAT stub immediately");
// Roadmap checkbox SHOULD be marked done (mechanical bookkeeping, no longer deferred)
// Roadmap checkbox must NOT be checked without summary on disk (#1910).
// Checking it without the summary causes deriveState() to skip complete-slice.
const roadmapContent = readFileSync(join(tmp, ".gsd", "milestones", "M001", "M001-ROADMAP.md"), "utf8");
assert.ok(roadmapContent.includes("- [x] **S01"), "roadmap should show S01 as checked");
assert.ok(roadmapContent.includes("- [ ] **S01"), "roadmap must NOT be checked without summary on disk (#1910)");
} finally {
rmSync(tmp, { recursive: true, force: true });
}
});
test("fixLevel:task — session crash after last task leaves roadmap and UAT consistent (#1808)", async () => {
test("fixLevel:task — session crash after last task leaves UAT consistent, roadmap deferred with summary (#1808, #1910)", async () => {
const tmp = makeTmp("crash-consistency");
try {
buildScaffold(tmp);
@ -121,13 +122,7 @@ test("fixLevel:task — session crash after last task leaves roadmap and UAT con
// A new session starts and runs doctor again at task level.
const report2 = await runGSDDoctor(tmp, { fix: true, fixLevel: "task" });
// The only remaining issue should be the deferred summary.
// Roadmap and UAT should already be fixed from the first run.
const remainingCodes = report2.issues.map(i => i.code);
assert.ok(
!remainingCodes.includes("all_tasks_done_roadmap_not_checked"),
"roadmap should already be fixed from first doctor run"
);
assert.ok(
!remainingCodes.includes("all_tasks_done_missing_slice_uat"),
"UAT should already be fixed from first doctor run"
@ -137,6 +132,16 @@ test("fixLevel:task — session crash after last task leaves roadmap and UAT con
remainingCodes.includes("all_tasks_done_missing_slice_summary"),
"summary should still be detected as missing (deferred)"
);
// Roadmap should still be unchecked because summary doesn't exist (#1910)
assert.ok(
remainingCodes.includes("all_tasks_done_roadmap_not_checked"),
"roadmap should still be unchecked — summary does not exist on disk (#1910)"
);
// Must NOT produce the cascade error from checking roadmap without summary
assert.ok(
!remainingCodes.includes("slice_checked_missing_summary"),
"must not produce slice_checked_missing_summary (#1910)"
);
} finally {
rmSync(tmp, { recursive: true, force: true });
}

View file

@ -63,7 +63,7 @@ Done.
`);
}
test("fixLevel:task — defers only summary stub, fixes roadmap and UAT immediately (#1808)", async () => {
test("fixLevel:task — defers summary stub and roadmap checkbox, fixes UAT immediately (#1808, #1910)", async () => {
const tmp = makeTmp("task-level");
try {
buildScaffold(tmp);
@ -79,13 +79,14 @@ test("fixLevel:task — defers only summary stub, fixes roadmap and UAT immediat
const sliceSummaryPath = join(tmp, ".gsd", "milestones", "M001", "slices", "S01", "S01-SUMMARY.md");
assert.ok(!existsSync(sliceSummaryPath), "should NOT have created summary stub");
// Roadmap SHOULD be marked done (mechanical bookkeeping, no longer deferred)
// Roadmap must NOT be checked without summary on disk (#1910)
const roadmapContent = readFileSync(join(tmp, ".gsd", "milestones", "M001", "M001-ROADMAP.md"), "utf8");
assert.ok(roadmapContent.includes("- [x] **S01"), "roadmap should show S01 as checked");
assert.ok(roadmapContent.includes("- [ ] **S01"), "roadmap must NOT be checked without summary (#1910)");
// Fixes applied should NOT include summary but SHOULD include roadmap
// Fixes applied should NOT include summary or roadmap
for (const f of report.fixesApplied) {
assert.ok(!f.includes("SUMMARY"), `should not have fixed summary: ${f}`);
assert.ok(!f.includes("ROADMAP") && !f.includes("roadmap"), `should not have fixed roadmap: ${f}`);
}
} finally {
rmSync(tmp, { recursive: true, force: true });

View file

@ -0,0 +1,167 @@
/**
* Regression test for #1910: Doctor marks roadmap checkbox at fixLevel="task"
* without summary on disk, causing deriveState() to skip complete-slice and
* hard-stop at validating-milestone.
*
* The roadmap checkbox must only be marked when the slice summary actually
* exists on disk (either pre-existing or created in the current doctor run).
* At fixLevel="task", the summary is deferred (COMPLETION_TRANSITION_CODES),
* so the roadmap checkbox must also be deferred.
*/
import { mkdirSync, writeFileSync, rmSync, readFileSync, existsSync } from "node:fs";
import { join } from "node:path";
import { tmpdir } from "node:os";
import test from "node:test";
import assert from "node:assert/strict";
import { runGSDDoctor } from "../doctor.ts";
function makeTmp(name: string): string {
const dir = join(tmpdir(), `doctor-roadmap-summary-${name}-${Date.now()}-${Math.random().toString(36).slice(2)}`);
mkdirSync(dir, { recursive: true });
return dir;
}
/**
* Build a minimal .gsd structure: milestone with one slice, one task
* marked done with a summary but no slice summary and roadmap unchecked.
* This is the state after the last task completes.
*/
function buildScaffold(base: string) {
const gsd = join(base, ".gsd");
const m = join(gsd, "milestones", "M001");
const s = join(m, "slices", "S01", "tasks");
mkdirSync(s, { recursive: true });
writeFileSync(join(m, "M001-ROADMAP.md"), `# M001: Test
## Slices
- [ ] **S01: Test Slice** \`risk:low\` \`depends:[]\`
> Demo text
`);
writeFileSync(join(m, "slices", "S01", "S01-PLAN.md"), `# S01: Test Slice
**Goal:** test
## Tasks
- [x] **T01: Do stuff** \`est:5m\`
`);
writeFileSync(join(s, "T01-SUMMARY.md"), `---
id: T01
parent: S01
milestone: M001
duration: 5m
verification_result: passed
completed_at: 2026-01-01
---
# T01: Do stuff
Done.
`);
}
test("fixLevel:task — must NOT mark roadmap checkbox when summary does not exist on disk (#1910)", async () => {
const tmp = makeTmp("no-roadmap-without-summary");
try {
buildScaffold(tmp);
const report = await runGSDDoctor(tmp, { fix: true, fixLevel: "task" });
// Doctor should detect both issues
const codes = report.issues.map(i => i.code);
assert.ok(codes.includes("all_tasks_done_missing_slice_summary"), "should detect missing summary");
assert.ok(codes.includes("all_tasks_done_roadmap_not_checked"), "should detect unchecked roadmap");
// Summary should NOT exist (deferred at task level)
const sliceSummaryPath = join(tmp, ".gsd", "milestones", "M001", "slices", "S01", "S01-SUMMARY.md");
assert.ok(!existsSync(sliceSummaryPath), "summary should NOT be created (deferred)");
// CRITICAL: Roadmap checkbox must NOT be checked without summary on disk.
// If it is checked, deriveState() sees the milestone as complete and skips
// the summarizing phase, causing a hard-stop at validating-milestone.
const roadmapContent = readFileSync(join(tmp, ".gsd", "milestones", "M001", "M001-ROADMAP.md"), "utf8");
assert.ok(
roadmapContent.includes("- [ ] **S01"),
"roadmap must NOT mark S01 as checked when summary does not exist on disk"
);
} finally {
rmSync(tmp, { recursive: true, force: true });
}
});
test("fixLevel:task — consecutive runs must not produce slice_checked_missing_summary (#1910)", async () => {
const tmp = makeTmp("no-cascade-error");
try {
buildScaffold(tmp);
// First doctor run at task level
await runGSDDoctor(tmp, { fix: true, fixLevel: "task" });
// Second doctor run — if the first run incorrectly checked the roadmap,
// this run would detect slice_checked_missing_summary (the cascade error
// described in the issue's forensic evidence).
const report2 = await runGSDDoctor(tmp, { fix: true, fixLevel: "task" });
const codes2 = report2.issues.map(i => i.code);
assert.ok(
!codes2.includes("slice_checked_missing_summary"),
"must not produce slice_checked_missing_summary — roadmap should not have been checked without summary"
);
} finally {
rmSync(tmp, { recursive: true, force: true });
}
});
test("fixLevel:all — roadmap checkbox IS marked because summary is created in same run (#1910)", async () => {
const tmp = makeTmp("all-level-creates-both");
try {
buildScaffold(tmp);
const report = await runGSDDoctor(tmp, { fix: true });
// At fixLevel:all, summary stub is created first, then roadmap is checked.
// Both should be fixed.
const sliceSummaryPath = join(tmp, ".gsd", "milestones", "M001", "slices", "S01", "S01-SUMMARY.md");
assert.ok(existsSync(sliceSummaryPath), "summary should be created at fixLevel:all");
const roadmapContent = readFileSync(join(tmp, ".gsd", "milestones", "M001", "M001-ROADMAP.md"), "utf8");
assert.ok(roadmapContent.includes("- [x] **S01"), "roadmap should show S01 as checked at fixLevel:all");
} finally {
rmSync(tmp, { recursive: true, force: true });
}
});
test("fixLevel:task — roadmap IS marked when summary already exists on disk (#1910)", async () => {
const tmp = makeTmp("summary-preexists");
try {
buildScaffold(tmp);
// Pre-create the slice summary (as if complete-slice already ran)
const sliceSummaryPath = join(tmp, ".gsd", "milestones", "M001", "slices", "S01", "S01-SUMMARY.md");
writeFileSync(sliceSummaryPath, `---
id: S01
milestone: M001
---
# S01: Test Slice
Summary content.
`);
const report = await runGSDDoctor(tmp, { fix: true, fixLevel: "task" });
// Summary exists, so roadmap SHOULD be checked even at task level
const roadmapContent = readFileSync(join(tmp, ".gsd", "milestones", "M001", "M001-ROADMAP.md"), "utf8");
assert.ok(
roadmapContent.includes("- [x] **S01"),
"roadmap should be checked when summary already exists on disk"
);
} finally {
rmSync(tmp, { recursive: true, force: true });
}
});