fix: doctor post-hook no longer preempts complete-slice dispatch
Doctor's fix:true mode was creating summary stubs and marking slices done in the roadmap during the post-hook after every task. This short-circuited the complete-slice dispatch unit — by the time dispatchNextUnit ran, the slice was already 'done' and the merge guard merged it to main, so complete-slice (which writes the real compressed summary) never got a chance to run. Root cause: doctor conflated two responsibilities — task-level bookkeeping (marking checkboxes) and completion state transitions (summary stubs, roadmap marking). The post-hook should only do the former. Fix: added fixLevel option to runGSDDoctor. fixLevel:'task' (used by post-hook) skips completion transition codes. fixLevel:'all' (default, used by manual gsd doctor and resume) preserves existing recovery behavior. Completion transition codes gated by fixLevel: - all_tasks_done_missing_slice_summary - all_tasks_done_missing_slice_uat - all_tasks_done_roadmap_not_checked
This commit is contained in:
parent
d4a46d36dd
commit
f6d56991df
3 changed files with 198 additions and 8 deletions
|
|
@ -526,14 +526,15 @@ export async function handleAgentEnd(
|
|||
}
|
||||
|
||||
// Post-hook: fix mechanical bookkeeping the LLM may have skipped.
|
||||
// 1. Doctor handles: checkbox marking, stub summaries/UATs.
|
||||
// 1. Doctor handles: checkbox marking (task-level bookkeeping).
|
||||
// 2. STATE.md is always rebuilt from disk state (purely derived, no LLM needed).
|
||||
// This is more reliable than prompt instructions for mechanical tasks.
|
||||
// Scope to slice level (M001/S01) so doctor checks all tasks within the slice.
|
||||
// fixLevel:"task" ensures doctor only fixes task-level issues (e.g. marking
|
||||
// checkboxes). Slice/milestone completion transitions (summary stubs,
|
||||
// roadmap [x] marking) are left for the complete-slice dispatch unit.
|
||||
try {
|
||||
const scopeParts = currentUnit.id.split("/").slice(0, 2);
|
||||
const doctorScope = scopeParts.join("/");
|
||||
const report = await runGSDDoctor(basePath, { fix: true, scope: doctorScope });
|
||||
const report = await runGSDDoctor(basePath, { fix: true, scope: doctorScope, fixLevel: "task" });
|
||||
if (report.fixesApplied.length > 0) {
|
||||
ctx.ui.notify(`Post-hook: applied ${report.fixesApplied.length} fix(es).`, "info");
|
||||
}
|
||||
|
|
|
|||
|
|
@ -422,10 +422,29 @@ export function formatDoctorIssuesForPrompt(issues: DoctorIssue[]): string {
|
|||
}).join("\n");
|
||||
}
|
||||
|
||||
export async function runGSDDoctor(basePath: string, options?: { fix?: boolean; scope?: string }): Promise<DoctorReport> {
|
||||
export async function runGSDDoctor(basePath: string, options?: { fix?: boolean; scope?: string; fixLevel?: "task" | "all" }): Promise<DoctorReport> {
|
||||
const issues: DoctorIssue[] = [];
|
||||
const fixesApplied: string[] = [];
|
||||
const fix = options?.fix === true;
|
||||
const fixLevel = options?.fixLevel ?? "all";
|
||||
|
||||
// Issue codes that represent completion state transitions — creating summary
|
||||
// stubs, marking slices/milestones done in the roadmap. These belong to the
|
||||
// dispatch lifecycle (complete-slice, complete-milestone units), not to
|
||||
// mechanical post-hook bookkeeping. When fixLevel is "task", these are
|
||||
// detected and reported but never auto-fixed.
|
||||
const completionTransitionCodes = new Set<DoctorIssueCode>([
|
||||
"all_tasks_done_missing_slice_summary",
|
||||
"all_tasks_done_missing_slice_uat",
|
||||
"all_tasks_done_roadmap_not_checked",
|
||||
]);
|
||||
|
||||
/** Whether a given issue code should be auto-fixed at the current fixLevel. */
|
||||
const shouldFix = (code: DoctorIssueCode): boolean => {
|
||||
if (!fix) return false;
|
||||
if (fixLevel === "task" && completionTransitionCodes.has(code)) return false;
|
||||
return true;
|
||||
};
|
||||
|
||||
const prefs = loadEffectiveGSDPreferences();
|
||||
if (prefs) {
|
||||
|
|
@ -606,7 +625,7 @@ export async function runGSDDoctor(basePath: string, options?: { fix?: boolean;
|
|||
file: relSliceFile(basePath, milestoneId, slice.id, "SUMMARY"),
|
||||
fixable: true,
|
||||
});
|
||||
if (fix) await ensureSliceSummaryStub(basePath, milestoneId, slice.id, fixesApplied);
|
||||
if (shouldFix("all_tasks_done_missing_slice_summary")) await ensureSliceSummaryStub(basePath, milestoneId, slice.id, fixesApplied);
|
||||
}
|
||||
|
||||
if (allTasksDone && !hasSliceUat) {
|
||||
|
|
@ -619,7 +638,7 @@ export async function runGSDDoctor(basePath: string, options?: { fix?: boolean;
|
|||
file: `${relSlicePath(basePath, milestoneId, slice.id)}/${slice.id}-UAT.md`,
|
||||
fixable: true,
|
||||
});
|
||||
if (fix) await ensureSliceUatStub(basePath, milestoneId, slice.id, fixesApplied);
|
||||
if (shouldFix("all_tasks_done_missing_slice_uat")) await ensureSliceUatStub(basePath, milestoneId, slice.id, fixesApplied);
|
||||
}
|
||||
|
||||
if (allTasksDone && !slice.done) {
|
||||
|
|
@ -632,7 +651,7 @@ export async function runGSDDoctor(basePath: string, options?: { fix?: boolean;
|
|||
file: relMilestoneFile(basePath, milestoneId, "ROADMAP"),
|
||||
fixable: true,
|
||||
});
|
||||
if (fix && (hasSliceSummary || issues.some(issue => issue.code === "all_tasks_done_missing_slice_summary" && issue.unitId === unitId))) {
|
||||
if (shouldFix("all_tasks_done_roadmap_not_checked") && (hasSliceSummary || issues.some(issue => issue.code === "all_tasks_done_missing_slice_summary" && issue.unitId === unitId))) {
|
||||
await markSliceDoneInRoadmap(basePath, milestoneId, slice.id, fixesApplied);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
170
src/resources/extensions/gsd/tests/doctor-fixlevel.test.ts
Normal file
170
src/resources/extensions/gsd/tests/doctor-fixlevel.test.ts
Normal file
|
|
@ -0,0 +1,170 @@
|
|||
/**
|
||||
* Tests that doctor's fixLevel option correctly separates task-level
|
||||
* bookkeeping from completion state transitions.
|
||||
*
|
||||
* fixLevel:"task" — fixes task checkboxes, does NOT create slice summary
|
||||
* stubs, UAT stubs, or mark slices done in the roadmap.
|
||||
* fixLevel:"all" (default) — fixes everything including completion transitions.
|
||||
*/
|
||||
|
||||
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-fixlevel-${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 exactly 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 — detects completion issues but does NOT create summary stub or mark roadmap", async () => {
|
||||
const tmp = makeTmp("task-level");
|
||||
try {
|
||||
buildScaffold(tmp);
|
||||
|
||||
const report = await runGSDDoctor(tmp, { fix: true, fixLevel: "task" });
|
||||
|
||||
// Should detect the 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");
|
||||
|
||||
// Should NOT have fixed them
|
||||
const sliceSummaryPath = join(tmp, ".gsd", "milestones", "M001", "slices", "S01", "S01-SUMMARY.md");
|
||||
assert.ok(!existsSync(sliceSummaryPath), "should NOT have created summary stub");
|
||||
|
||||
const roadmapContent = readFileSync(join(tmp, ".gsd", "milestones", "M001", "M001-ROADMAP.md"), "utf8");
|
||||
assert.ok(roadmapContent.includes("- [ ] **S01"), "roadmap should still show S01 as unchecked");
|
||||
|
||||
// Fixes applied should NOT include completion artifacts
|
||||
for (const f of report.fixesApplied) {
|
||||
assert.ok(!f.includes("SUMMARY"), `should not have fixed summary: ${f}`);
|
||||
assert.ok(!f.includes("roadmap"), `should not have fixed roadmap: ${f}`);
|
||||
}
|
||||
} finally {
|
||||
rmSync(tmp, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("fixLevel:all (default) — detects AND fixes completion issues", async () => {
|
||||
const tmp = makeTmp("all-level");
|
||||
try {
|
||||
buildScaffold(tmp);
|
||||
|
||||
const report = await runGSDDoctor(tmp, { fix: true });
|
||||
|
||||
// Should detect the 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");
|
||||
|
||||
// SHOULD have fixed them
|
||||
const sliceSummaryPath = join(tmp, ".gsd", "milestones", "M001", "slices", "S01", "S01-SUMMARY.md");
|
||||
assert.ok(existsSync(sliceSummaryPath), "should have created summary stub");
|
||||
|
||||
const roadmapContent = readFileSync(join(tmp, ".gsd", "milestones", "M001", "M001-ROADMAP.md"), "utf8");
|
||||
assert.ok(roadmapContent.includes("- [x] **S01"), "roadmap should show S01 as checked");
|
||||
} finally {
|
||||
rmSync(tmp, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("fixLevel:task — still fixes task-level bookkeeping (checkbox marking)", async () => {
|
||||
const tmp = makeTmp("task-checkbox");
|
||||
try {
|
||||
const gsd = join(tmp, ".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
|
||||
`);
|
||||
|
||||
// Task NOT checked in plan but has a summary — doctor should mark it done
|
||||
writeFileSync(join(m, "slices", "S01", "S01-PLAN.md"), `# S01: Test Slice
|
||||
|
||||
**Goal:** test
|
||||
|
||||
## Tasks
|
||||
|
||||
- [ ] **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.
|
||||
`);
|
||||
|
||||
const report = await runGSDDoctor(tmp, { fix: true, fixLevel: "task" });
|
||||
|
||||
// Should have fixed the task checkbox
|
||||
const planContent = readFileSync(join(m, "slices", "S01", "S01-PLAN.md"), "utf8");
|
||||
assert.ok(planContent.includes("- [x] **T01"), "should have marked T01 done in plan");
|
||||
|
||||
// Should NOT have touched slice-level completion
|
||||
const sliceSummaryPath = join(tmp, ".gsd", "milestones", "M001", "slices", "S01", "S01-SUMMARY.md");
|
||||
assert.ok(!existsSync(sliceSummaryPath), "should NOT have created summary stub");
|
||||
} finally {
|
||||
rmSync(tmp, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue