diff --git a/src/resources/extensions/gsd/doctor-git-checks.ts b/src/resources/extensions/gsd/doctor-git-checks.ts index 0b8820108..78754fc8f 100644 --- a/src/resources/extensions/gsd/doctor-git-checks.ts +++ b/src/resources/extensions/gsd/doctor-git-checks.ts @@ -14,6 +14,28 @@ import { nativeIsRepo, nativeWorktreeList, nativeWorktreeRemove, nativeBranchLis import { getAllWorktreeHealth } from "./worktree-health.js"; import { loadEffectiveGSDPreferences } from "./preferences.js"; +/** + * Returns true if the directory contains only doctor artifacts + * (e.g. `.gsd/doctor-history.jsonl`). These dirs are created by + * appendDoctorHistory() writing to worktree-scoped paths during the audit + * and should not be flagged as orphaned worktrees (#3105). + */ +function isDoctorArtifactOnly(dirPath: string): boolean { + try { + const entries = readdirSync(dirPath); + // Empty dir — not a doctor artifact, still orphaned + if (entries.length === 0) return false; + // Only a .gsd subdirectory + if (entries.length === 1 && entries[0] === ".gsd") { + const gsdEntries = readdirSync(join(dirPath, ".gsd")); + return gsdEntries.length <= 1 && gsdEntries.every(e => e === "doctor-history.jsonl"); + } + return false; + } catch { + return false; + } +} + export async function checkGitHealth( basePath: string, issues: DoctorIssue[], @@ -314,6 +336,10 @@ export async function checkGitHealth( } catch { continue; } const normalizedFullPath = normalizePath(fullPath); if (!registeredPaths.has(normalizedFullPath)) { + // Skip directories that only contain doctor artifacts (.gsd/doctor-history.jsonl). + // appendDoctorHistory() can recreate these dirs during the audit itself, + // causing a circular false positive (#3105 Bug 1). + if (isDoctorArtifactOnly(fullPath)) continue; issues.push({ severity: "warning", code: "worktree_directory_orphaned", diff --git a/src/resources/extensions/gsd/doctor.ts b/src/resources/extensions/gsd/doctor.ts index 83fc8a754..05630093e 100644 --- a/src/resources/extensions/gsd/doctor.ts +++ b/src/resources/extensions/gsd/doctor.ts @@ -729,8 +729,10 @@ export async function runGSDDoctor(basePath: string, options?: { fix?: boolean; } // Blocker-without-replan detection + // Skip when all tasks are done — the blocker was implicitly resolved + // within the task and the slice is not stuck (#3105 Bug 2). const replanPath = resolveSliceFile(basePath, milestoneId, slice.id, "REPLAN"); - if (!replanPath) { + if (!replanPath && !allTasksDone) { for (const task of plan.tasks) { if (!task.done) continue; const summaryPath = resolveTaskFile(basePath, milestoneId, slice.id, task.id, "SUMMARY"); diff --git a/src/resources/extensions/gsd/parsers-legacy.ts b/src/resources/extensions/gsd/parsers-legacy.ts index c1a00e554..ee0632fd2 100644 --- a/src/resources/extensions/gsd/parsers-legacy.ts +++ b/src/resources/extensions/gsd/parsers-legacy.ts @@ -196,18 +196,28 @@ function _parsePlanImpl(content: string): SlicePlan { const mhSection = extractSection(body, 'Must-Haves'); const mustHaves = mhSection ? parseBullets(mhSection) : []; + // Parse tasks from ## Tasks section first, then scan the full body for any + // task checkboxes that were missed. Multi-task plans can interleave T01 detail + // headings (## Steps, ## Must-Haves) before T02's checkbox, which causes + // extractSection("Tasks") to stop at the first ## heading and miss T02+ (#3105). const tasksSection = extractSection(body, 'Tasks'); const tasks: TaskPlanEntry[] = []; - if (tasksSection) { - const taskLines = tasksSection.split('\n'); + // Parse task entries from a set of lines, appending to `tasks`. + const parseTaskLines = (lines: string[], knownIds: Set): void => { let currentTask: TaskPlanEntry | null = null; - for (const line of taskLines) { + for (const line of lines) { const cbMatch = line.match(/^-\s+\[([ xX])\]\s+\*\*([\w.]+):\s+(.+?)\*\*\s*(.*)/); // Heading-style: ### T01 -- Title, ### T01: Title, ### T01 — Title const hdMatch = !cbMatch ? line.match(/^#{2,4}\s+([\w.]+)\s*(?:--|—|:)\s*(.+)/) : null; if (cbMatch || hdMatch) { + const taskId = cbMatch ? cbMatch[2] : hdMatch![1]; + // Skip tasks already found in the Tasks section + if (knownIds.has(taskId)) { + currentTask = null; + continue; + } if (currentTask) tasks.push(currentTask); if (cbMatch) { @@ -259,8 +269,17 @@ function _parsePlanImpl(content: string): SlicePlan { } } if (currentTask) tasks.push(currentTask); + }; + + if (tasksSection) { + parseTaskLines(tasksSection.split('\n'), new Set()); } + // Second pass: scan the full body for task checkboxes outside ## Tasks. + // This handles interleaved plans where T02+ appear after T01's detail headings. + const foundIds = new Set(tasks.map(t => t.id)); + parseTaskLines(body.split('\n'), foundIds); + const filesSection = extractSection(body, 'Files Likely Touched'); const filesLikelyTouched = filesSection ? parseBullets(filesSection) : []; diff --git a/src/resources/extensions/gsd/tests/integration/doctor-false-positives.test.ts b/src/resources/extensions/gsd/tests/integration/doctor-false-positives.test.ts new file mode 100644 index 000000000..c2189e236 --- /dev/null +++ b/src/resources/extensions/gsd/tests/integration/doctor-false-positives.test.ts @@ -0,0 +1,243 @@ +import { describe, test } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtempSync, mkdirSync, rmSync, writeFileSync, existsSync, readdirSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; + +import { runGSDDoctor } from "../../doctor.js"; +import { parsePlan } from "../../parsers-legacy.js"; + +// ── Helpers ───────────────────────────────────────────────────────────────── + +function makeBase(): { base: string; gsd: string; mDir: string } { + const base = mkdtempSync(join(tmpdir(), "gsd-doctor-fp-")); + const gsd = join(base, ".gsd"); + const mDir = join(gsd, "milestones", "M001"); + mkdirSync(join(mDir, "slices"), { recursive: true }); + return { base, gsd, mDir }; +} + +function writeRoadmap(mDir: string, content: string): void { + writeFileSync(join(mDir, "M001-ROADMAP.md"), content); +} + +function writeSlice(mDir: string, sliceId: string, planContent: string): string { + const sDir = join(mDir, "slices", sliceId); + const tDir = join(sDir, "tasks"); + mkdirSync(tDir, { recursive: true }); + writeFileSync(join(sDir, `${sliceId}-PLAN.md`), planContent); + return sDir; +} + +describe('doctor false-positives (#3105)', async () => { + + // ═══════════════════════════════════════════════════════════════════════════ + // Bug 1: Orphaned worktree directory recreated by appendDoctorHistory + // ═══════════════════════════════════════════════════════════════════════════ + + test('Bug 1: orphaned worktree check ignores dirs containing only .gsd/doctor-history.jsonl', async () => { + // Simulate: a worktree dir that only contains .gsd/doctor-history.jsonl + // (created by appendDoctorHistory writing to the worktree-scoped path). + // The orphan check should NOT warn about this directory. + const { base, gsd } = makeBase(); + writeRoadmap(join(gsd, "milestones", "M001"), `# M001: Test\n\n## Slices\n- [ ] **S01: Slice** \`risk:low\` \`depends:[]\`\n > After this: done\n`); + writeSlice(join(gsd, "milestones", "M001"), "S01", "# S01: Slice\n\n**Goal:** G\n**Demo:** D\n\n## Tasks\n- [ ] **T01: Task** `est:10m`\n Pending.\n"); + + // Create a worktree directory that only has .gsd/doctor-history.jsonl + const wtDir = join(gsd, "worktrees", "M042"); + const wtGsdDir = join(wtDir, ".gsd"); + mkdirSync(wtGsdDir, { recursive: true }); + writeFileSync(join(wtGsdDir, "doctor-history.jsonl"), '{"ts":"2026-01-01","ok":true}\n'); + + const result = await runGSDDoctor(base, { fix: false }); + + // Should NOT produce worktree_directory_orphaned for a dir that only has doctor history + const orphanIssues = result.issues.filter( + i => i.code === "worktree_directory_orphaned" && i.unitId === "M042" + ); + assert.equal(orphanIssues.length, 0, + "should not warn about worktree dir that only contains .gsd/doctor-history.jsonl"); + + rmSync(base, { recursive: true, force: true }); + }); + + // ═══════════════════════════════════════════════════════════════════════════ + // Bug 2: blocker_discovered + all tasks done = unfixable deadlock + // ═══════════════════════════════════════════════════════════════════════════ + + test('Bug 2: blocker_discovered with all tasks done should not warn (implicitly resolved)', async () => { + // Scenario: blocker was discovered and resolved within the same task. + // blocker_discovered: true, no REPLAN, but all tasks are done. + // Neither blocker_discovered_no_replan nor stale_replan_file should fire. + const { base, mDir } = makeBase(); + writeRoadmap(mDir, `# M001: Blocker Test\n\n## Slices\n- [ ] **S01: Slice** \`risk:low\` \`depends:[]\`\n > After this: done\n`); + const sDir = writeSlice(mDir, "S01", + "# S01: Slice\n\n**Goal:** G\n**Demo:** D\n\n## Tasks\n- [x] **T01: Task** `est:10m`\n Done.\n"); + writeFileSync(join(sDir, "tasks", "T01-SUMMARY.md"), `--- +id: T01 +parent: S01 +milestone: M001 +provides: [] +requires: [] +affects: [] +key_files: [] +key_decisions: [] +patterns_established: [] +observability_surfaces: [] +drill_down_paths: [] +duration: 10m +verification_result: passed +completed_at: 2026-01-01T00:00:00Z +blocker_discovered: true +--- + +# T01: Task + +**Done** + +## What Happened +Found a blocker, resolved it in-task. + +## Diagnostics +- log +`); + + const result = await runGSDDoctor(base, { fix: false }); + + // Should NOT produce blocker_discovered_no_replan when all tasks are done + const blockerIssues = result.issues.filter(i => i.code === "blocker_discovered_no_replan"); + assert.equal(blockerIssues.length, 0, + "should not warn about blocker_discovered when all tasks are done (blocker was implicitly resolved)"); + + // Also should NOT produce stale_replan_file (no REPLAN exists, so this shouldn't fire anyway) + const staleReplanIssues = result.issues.filter(i => i.code === "stale_replan_file"); + assert.equal(staleReplanIssues.length, 0, + "should not produce stale_replan_file when no REPLAN exists"); + + rmSync(base, { recursive: true, force: true }); + }); + + test('Bug 2: blocker_discovered with incomplete tasks should still warn', async () => { + // Sanity check: when there IS an incomplete task and blocker_discovered, warn as before. + const { base, mDir } = makeBase(); + writeRoadmap(mDir, `# M001: Blocker Warn Test\n\n## Slices\n- [ ] **S01: Slice** \`risk:low\` \`depends:[]\`\n > After this: done\n`); + const sDir = writeSlice(mDir, "S01", + "# S01: Slice\n\n**Goal:** G\n**Demo:** D\n\n## Tasks\n- [x] **T01: Task A** `est:10m`\n Done.\n- [ ] **T02: Task B** `est:10m`\n Pending.\n"); + writeFileSync(join(sDir, "tasks", "T01-SUMMARY.md"), `--- +id: T01 +parent: S01 +milestone: M001 +provides: [] +requires: [] +affects: [] +key_files: [] +key_decisions: [] +patterns_established: [] +observability_surfaces: [] +drill_down_paths: [] +duration: 10m +verification_result: passed +completed_at: 2026-01-01T00:00:00Z +blocker_discovered: true +--- + +# T01: Task A + +**Done** + +## What Happened +Found blocker, but T02 is still pending. + +## Diagnostics +- log +`); + + const result = await runGSDDoctor(base, { fix: false }); + + const blockerIssues = result.issues.filter(i => i.code === "blocker_discovered_no_replan"); + assert.ok(blockerIssues.length > 0, + "should still warn about blocker_discovered when some tasks are not done"); + + rmSync(base, { recursive: true, force: true }); + }); + + // ═══════════════════════════════════════════════════════════════════════════ + // Bug 3: Multi-task plan — T02+ outside ## Tasks section + // ═══════════════════════════════════════════════════════════════════════════ + + test('Bug 3: parsePlan finds all tasks even when interleaved with detail sections', () => { + // Multi-task plan where T02 checkbox appears after T01's ## Steps heading, + // which ends the ## Tasks section for extractSection(). + const planContent = `# S01: Demo Slice + +**Goal:** Build the demo +**Demo:** Run it + +## Must-Haves +- Feature A + +## Tasks +- [x] **T01: First task** \`est:30m\` + Implement the first thing. +## Steps +1. Step one +2. Step two +## Must-Haves +- Requirement A +- [x] **T02: Second task** \`est:1h\` + Implement the second thing. +## Steps +1. Step one +2. Step two +`; + + const plan = parsePlan(planContent); + const taskIds = plan.tasks.map(t => t.id); + + assert.ok(taskIds.includes("T01"), "should find T01"); + assert.ok(taskIds.includes("T02"), "should find T02 even when after T01 detail headings"); + assert.equal(plan.tasks.length, 2, "should find exactly 2 tasks"); + }); + + test('Bug 3: task_file_not_in_plan should not fire for T02 in multi-task plan', async () => { + const { base, mDir } = makeBase(); + writeRoadmap(mDir, `# M001: Multi-Task Test\n\n## Slices\n- [ ] **S01: Slice** \`risk:low\` \`depends:[]\`\n > After this: done\n`); + + // Plan with interleaved headings (the problematic format) + const sDir = writeSlice(mDir, "S01", `# S01: Demo Slice + +**Goal:** Build the demo +**Demo:** Run it + +## Must-Haves +- Feature A + +## Tasks +- [x] **T01: First task** \`est:30m\` + Implement the first thing. +## Steps +1. Step one +## Must-Haves +- Req A +- [x] **T02: Second task** \`est:1h\` + Implement the second thing. +## Steps +1. Step one +`); + + // Both tasks have summaries on disk + writeFileSync(join(sDir, "tasks", "T01-SUMMARY.md"), "---\nstatus: done\ncompleted_at: 2026-01-01T00:00:00Z\n---\n# T01\nDone.\n"); + writeFileSync(join(sDir, "tasks", "T02-SUMMARY.md"), "---\nstatus: done\ncompleted_at: 2026-01-01T00:00:00Z\n---\n# T02\nDone.\n"); + + const result = await runGSDDoctor(base, { fix: false }); + + // T02 should NOT be flagged as "not in plan" + const notInPlan = result.issues.filter( + i => i.code === "task_file_not_in_plan" && i.message.includes("T02") + ); + assert.equal(notInPlan.length, 0, + "should not report T02 as 'not in plan' when it exists in the interleaved plan format"); + + rmSync(base, { recursive: true, force: true }); + }); +});