fix: make task closeout crash-safe by unchecking orphaned checkboxes (#1650) (#1759)

When the process crashes between marking a task [x] in PLAN.md and
writing SUMMARY.md, the task appears done but has no summary. The doctor
previously papered over this by creating a stub summary, silently losing
the task. Now it unchecks the task so it re-executes on next run.

- Add markTaskUndoneInPlan to roadmap-mutations.ts
- Change doctor task_done_missing_summary fix: uncheck instead of stub
- Add markTaskUndoneInPlan helper to doctor.ts for async file ops
- Add test coverage for both the mutation and doctor behavior

Closes #1650

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
TÂCHES 2026-03-21 09:04:55 -06:00 committed by GitHub
parent 4367ea36c4
commit c68c6331ad
3 changed files with 229 additions and 21 deletions

View file

@ -265,6 +265,21 @@ async function markTaskDoneInPlan(basePath: string, milestoneId: string, sliceId
}
}
async function markTaskUndoneInPlan(basePath: string, milestoneId: string, sliceId: string, taskId: string, fixesApplied: string[]): Promise<void> {
const planPath = resolveSliceFile(basePath, milestoneId, sliceId, "PLAN");
if (!planPath) return;
const content = await loadFile(planPath);
if (!content) return;
const updated = content.replace(
new RegExp(`^(\\s*-\\s+)\\[x\\]\\s+\\*\\*${taskId}:`, "mi"),
`$1[ ] **${taskId}:`,
);
if (updated !== content) {
await saveFile(planPath, updated);
fixesApplied.push(`unchecked ${taskId} in ${planPath} (missing summary — task will re-execute)`);
}
}
async function markSliceDoneInRoadmap(basePath: string, milestoneId: string, sliceId: string, fixesApplied: string[]): Promise<void> {
const roadmapPath = resolveMilestoneFile(basePath, milestoneId, "ROADMAP");
if (!roadmapPath) return;
@ -769,30 +784,13 @@ export async function runGSDDoctor(basePath: string, options?: { fix?: boolean;
code: "task_done_missing_summary",
scope: "task",
unitId: taskUnitId,
message: `Task ${task.id} is marked done but summary is missing`,
file: relTaskFile(basePath, milestoneId, slice.id, task.id, "SUMMARY"),
message: `Task ${task.id} is marked done but summary is missing — unchecking so it re-executes`,
file: relSliceFile(basePath, milestoneId, slice.id, "PLAN"),
fixable: true,
});
dryRunCanFix("task_done_missing_summary", `create stub summary for ${taskUnitId}`);
dryRunCanFix("task_done_missing_summary", `uncheck ${task.id} in plan for ${taskUnitId}`);
if (shouldFix("task_done_missing_summary")) {
const stubPath = join(
basePath, ".gsd", "milestones", milestoneId, "slices", slice.id, "tasks",
`${task.id}-SUMMARY.md`,
);
const stubContent = [
`---`,
`status: done`,
`result: unknown`,
`doctor_generated: true`,
`---`,
``,
`# ${task.id}: ${task.title || "Unknown"}`,
``,
`Summary stub generated by \`/gsd doctor\` \u2014 task was marked done but no summary existed.`,
``,
].join("\n");
await saveFile(stubPath, stubContent);
fixesApplied.push(`created stub summary for ${taskUnitId}`);
await markTaskUndoneInPlan(basePath, milestoneId, slice.id, task.id, fixesApplied);
}
}

View file

@ -93,3 +93,29 @@ export function markTaskDoneInPlan(basePath: string, planPath: string, tid: stri
clearParseCache();
return true;
}
/**
* Mark a task as not done ([ ]) in the slice plan.
* Idempotent no-op if already unchecked or if the task isn't found.
*
* @returns true if the plan was modified, false if no change was needed
*/
export function markTaskUndoneInPlan(basePath: string, planPath: string, tid: string): boolean {
let content: string;
try {
content = readFileSync(planPath, "utf-8");
} catch {
return false;
}
const updated = content.replace(
new RegExp(`^(\\s*-\\s+)\\[x\\]\\s+\\*\\*${tid}:`, "mi"),
`$1[ ] **${tid}:`,
);
if (updated === content) return false;
atomicWriteSync(planPath, updated);
clearParseCache();
return true;
}

View file

@ -0,0 +1,184 @@
/**
* Tests for atomic task closeout (#1650):
* 1. Doctor unmarks task checkbox when summary is missing (instead of creating stub)
* 2. markTaskUndoneInPlan correctly unchecks a task in the slice plan
*/
import { mkdirSync, writeFileSync, readFileSync, rmSync, 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";
import { markTaskUndoneInPlan } from "../roadmap-mutations.ts";
function makeTmp(name: string): string {
const dir = join(tmpdir(), `atomic-closeout-${name}-${Date.now()}-${Math.random().toString(36).slice(2)}`);
mkdirSync(dir, { recursive: true });
return dir;
}
// ── markTaskUndoneInPlan ─────────────────────────────────────────────────────
test("markTaskUndoneInPlan unchecks a checked task", () => {
const base = makeTmp("uncheck");
const planPath = join(base, "PLAN.md");
writeFileSync(planPath, `# S01: Demo
## Tasks
- [x] **T01: First task** \`est:5m\`
- [ ] **T02: Second task** \`est:10m\`
`);
const changed = markTaskUndoneInPlan(base, planPath, "T01");
assert.ok(changed, "should return true when plan was modified");
const content = readFileSync(planPath, "utf-8");
assert.ok(content.includes("- [ ] **T01:"), "T01 should be unchecked");
assert.ok(content.includes("- [ ] **T02:"), "T02 should remain unchecked");
rmSync(base, { recursive: true, force: true });
});
test("markTaskUndoneInPlan is idempotent on already-unchecked task", () => {
const base = makeTmp("uncheck-noop");
const planPath = join(base, "PLAN.md");
writeFileSync(planPath, `# S01: Demo
## Tasks
- [ ] **T01: First task** \`est:5m\`
`);
const changed = markTaskUndoneInPlan(base, planPath, "T01");
assert.ok(!changed, "should return false when no change needed");
rmSync(base, { recursive: true, force: true });
});
test("markTaskUndoneInPlan handles indented checkboxes", () => {
const base = makeTmp("uncheck-indent");
const planPath = join(base, "PLAN.md");
writeFileSync(planPath, `# S01: Demo
## Tasks
- [x] **T01: First task** \`est:5m\`
`);
const changed = markTaskUndoneInPlan(base, planPath, "T01");
assert.ok(changed, "should handle indented checkboxes");
const content = readFileSync(planPath, "utf-8");
assert.ok(content.includes("[ ] **T01:"), "T01 should be unchecked");
rmSync(base, { recursive: true, force: true });
});
// ── Doctor: task_done_missing_summary unchecks instead of stubbing ────────────
test("doctor unchecks task when checkbox is marked but summary is missing", async () => {
const base = makeTmp("doctor-uncheck");
const gsd = join(base, ".gsd");
const m = join(gsd, "milestones", "M001");
const s = join(m, "slices", "S01");
const t = join(s, "tasks");
mkdirSync(t, { recursive: true });
writeFileSync(join(m, "M001-ROADMAP.md"), `# M001: Test
## Slices
- [ ] **S01: Test Slice** \`risk:low\` \`depends:[]\`
> Demo
`);
// Task is marked [x] in plan but has no summary file
writeFileSync(join(s, "S01-PLAN.md"), `# S01: Test Slice
**Goal:** test
## Tasks
- [x] **T01: Do stuff** \`est:5m\`
- [ ] **T02: Other stuff** \`est:5m\`
`);
// T02 has no summary either, but it's unchecked — should be left alone
// Run doctor in diagnose mode first
const diagnoseReport = await runGSDDoctor(base, { fix: false });
const issue = diagnoseReport.issues.find(i => i.code === "task_done_missing_summary");
assert.ok(issue, "should detect task_done_missing_summary");
assert.equal(issue!.severity, "error");
// Run doctor in fix mode
const fixReport = await runGSDDoctor(base, { fix: true });
const fixApplied = fixReport.fixesApplied.some(f => f.includes("unchecked T01"));
assert.ok(fixApplied, "should have unchecked T01 in the fix log");
// Verify the plan now has T01 unchecked
const planContent = readFileSync(join(s, "S01-PLAN.md"), "utf-8");
assert.ok(planContent.includes("- [ ] **T01:"), "T01 should be unchecked after doctor fix");
assert.ok(planContent.includes("- [ ] **T02:"), "T02 should remain unchecked");
// Verify no stub summary was created
const stubPath = join(t, "T01-SUMMARY.md");
assert.ok(
!existsSync(stubPath),
"should NOT create a stub summary — task should re-execute instead",
);
rmSync(base, { recursive: true, force: true });
});
test("doctor does not touch task with checkbox AND summary both present", async () => {
const base = makeTmp("doctor-ok");
const gsd = join(base, ".gsd");
const m = join(gsd, "milestones", "M001");
const s = join(m, "slices", "S01");
const t = join(s, "tasks");
mkdirSync(t, { recursive: true });
writeFileSync(join(m, "M001-ROADMAP.md"), `# M001: Test
## Slices
- [ ] **S01: Test Slice** \`risk:low\` \`depends:[]\`
> Demo
`);
writeFileSync(join(s, "S01-PLAN.md"), `# S01: Test Slice
**Goal:** test
## Tasks
- [x] **T01: Do stuff** \`est:5m\`
`);
writeFileSync(join(t, "T01-SUMMARY.md"), `---
id: T01
parent: S01
milestone: M001
duration: 5m
verification_result: passed
completed_at: 2026-01-01
---
# T01: Do stuff
Done.
`);
const report = await runGSDDoctor(base, { fix: true });
const hasTaskIssue = report.issues.some(i => i.code === "task_done_missing_summary");
assert.ok(!hasTaskIssue, "should not flag task_done_missing_summary when both exist");
// Plan should still have T01 checked
const planContent = readFileSync(join(s, "S01-PLAN.md"), "utf-8");
assert.ok(planContent.includes("- [x] **T01:"), "T01 should remain checked");
rmSync(base, { recursive: true, force: true });
});