From 48c25dc853bdb6991d705b88fefc80494912b8a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=82CHES?= Date: Sat, 21 Mar 2026 11:40:40 -0600 Subject: [PATCH] fix: validate paused-session milestone before restoring it (#1664) (#1800) --- src/resources/extensions/gsd/auto.ts | 32 ++-- .../auto-paused-session-validation.test.ts | 143 ++++++++++++++++++ 2 files changed, 165 insertions(+), 10 deletions(-) create mode 100644 src/resources/extensions/gsd/tests/auto-paused-session-validation.test.ts diff --git a/src/resources/extensions/gsd/auto.ts b/src/resources/extensions/gsd/auto.ts index b995d3d16..ebbbcfbd7 100644 --- a/src/resources/extensions/gsd/auto.ts +++ b/src/resources/extensions/gsd/auto.ts @@ -967,16 +967,28 @@ export async function startAuto( if (existsSync(pausedPath)) { const meta = JSON.parse(readFileSync(pausedPath, "utf-8")); if (meta.milestoneId) { - s.currentMilestoneId = meta.milestoneId; - s.originalBasePath = meta.originalBasePath || base; - s.stepMode = meta.stepMode ?? requestedStepMode; - s.paused = true; - // Clean up the persisted file — we're consuming it - try { unlinkSync(pausedPath); } catch { /* non-fatal */ } - ctx.ui.notify( - `Resuming paused session for ${meta.milestoneId}${meta.worktreePath ? ` (worktree)` : ""}.`, - "info", - ); + // Validate the milestone still exists and isn't already complete (#1664). + const mDir = resolveMilestonePath(base, meta.milestoneId); + const summaryFile = resolveMilestoneFile(base, meta.milestoneId, "SUMMARY"); + if (!mDir || summaryFile) { + // Stale milestone — clean up and fall through to fresh bootstrap + try { unlinkSync(pausedPath); } catch { /* non-fatal */ } + ctx.ui.notify( + `Paused milestone ${meta.milestoneId} is ${!mDir ? "missing" : "already complete"}. Starting fresh.`, + "info", + ); + } else { + s.currentMilestoneId = meta.milestoneId; + s.originalBasePath = meta.originalBasePath || base; + s.stepMode = meta.stepMode ?? requestedStepMode; + s.paused = true; + // Clean up the persisted file — we're consuming it + try { unlinkSync(pausedPath); } catch { /* non-fatal */ } + ctx.ui.notify( + `Resuming paused session for ${meta.milestoneId}${meta.worktreePath ? ` (worktree)` : ""}.`, + "info", + ); + } } } } catch { diff --git a/src/resources/extensions/gsd/tests/auto-paused-session-validation.test.ts b/src/resources/extensions/gsd/tests/auto-paused-session-validation.test.ts new file mode 100644 index 000000000..addbefa22 --- /dev/null +++ b/src/resources/extensions/gsd/tests/auto-paused-session-validation.test.ts @@ -0,0 +1,143 @@ +/** + * auto-paused-session-validation.test.ts — Validates milestone existence + * before restoring from paused-session.json (#1664). + * + * Two layers: + * 1. Source-code regression: ensures auto.ts validates the milestone before + * trusting paused-session.json (guards against accidental removal). + * 2. Filesystem unit: confirms resolveMilestonePath / resolveMilestoneFile + * correctly detect missing and completed milestones. + */ + +import test from "node:test"; +import assert from "node:assert/strict"; +import { mkdirSync, writeFileSync, rmSync, readFileSync } from "node:fs"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { randomUUID } from "node:crypto"; +import { fileURLToPath } from "node:url"; +import { dirname } from "node:path"; + +import { resolveMilestonePath, resolveMilestoneFile } from "../paths.ts"; + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const AUTO_TS_PATH = join(__dirname, "..", "auto.ts"); + +// ─── Source-code regression guard ─────────────────────────────────────────── + +test("auto.ts validates milestone before restoring paused session (#1664)", () => { + const source = readFileSync(AUTO_TS_PATH, "utf-8"); + + // The resume block must call resolveMilestonePath to verify the milestone dir exists + assert.ok( + source.includes('resolveMilestonePath(base, meta.milestoneId)'), + "auto.ts must call resolveMilestonePath to verify paused milestone exists", + ); + + // The resume block must check for a SUMMARY file to detect completed milestones + assert.ok( + source.includes('resolveMilestoneFile(base, meta.milestoneId, "SUMMARY")'), + "auto.ts must check for SUMMARY file to detect completed milestones", + ); +}); + +// ─── Filesystem validation unit tests ─────────────────────────────────────── + +function makeTmpBase(): string { + return join(tmpdir(), `gsd-paused-test-${randomUUID()}`); +} + +function cleanup(base: string): void { + try { rmSync(base, { recursive: true, force: true }); } catch { /* */ } +} + +test("resolveMilestonePath returns null for missing milestone", () => { + const base = makeTmpBase(); + mkdirSync(join(base, ".gsd", "milestones"), { recursive: true }); + try { + const result = resolveMilestonePath(base, "M999"); + assert.equal(result, null, "should return null for non-existent milestone"); + } finally { + cleanup(base); + } +}); + +test("resolveMilestonePath returns path for existing milestone", () => { + const base = makeTmpBase(); + mkdirSync(join(base, ".gsd", "milestones", "M001"), { recursive: true }); + try { + const result = resolveMilestonePath(base, "M001"); + assert.ok(result, "should return a path for existing milestone"); + assert.ok(result.includes("M001"), "path should contain the milestone ID"); + } finally { + cleanup(base); + } +}); + +test("resolveMilestoneFile returns null when no SUMMARY exists", () => { + const base = makeTmpBase(); + mkdirSync(join(base, ".gsd", "milestones", "M001"), { recursive: true }); + try { + const result = resolveMilestoneFile(base, "M001", "SUMMARY"); + assert.equal(result, null, "should return null when no SUMMARY file"); + } finally { + cleanup(base); + } +}); + +test("resolveMilestoneFile returns path when SUMMARY exists (completed)", () => { + const base = makeTmpBase(); + const mDir = join(base, ".gsd", "milestones", "M001"); + mkdirSync(mDir, { recursive: true }); + writeFileSync(join(mDir, "M001-SUMMARY.md"), "# Summary\nDone."); + try { + const result = resolveMilestoneFile(base, "M001", "SUMMARY"); + assert.ok(result, "should return a path when SUMMARY exists"); + assert.ok(result.includes("SUMMARY"), "path should reference SUMMARY"); + } finally { + cleanup(base); + } +}); + +// ─── Combined validation logic (mirrors auto.ts resume guard) ─────────────── + +test("stale milestone: missing dir means paused session should be discarded", () => { + const base = makeTmpBase(); + mkdirSync(join(base, ".gsd", "milestones"), { recursive: true }); + try { + const mDir = resolveMilestonePath(base, "M999"); + const summaryFile = resolveMilestoneFile(base, "M999", "SUMMARY"); + const isStale = !mDir || !!summaryFile; + assert.ok(isStale, "milestone that doesn't exist should be detected as stale"); + } finally { + cleanup(base); + } +}); + +test("stale milestone: completed (has SUMMARY) means paused session should be discarded", () => { + const base = makeTmpBase(); + const mDir = join(base, ".gsd", "milestones", "M001"); + mkdirSync(mDir, { recursive: true }); + writeFileSync(join(mDir, "M001-SUMMARY.md"), "# Summary\nDone."); + try { + const dir = resolveMilestonePath(base, "M001"); + const summaryFile = resolveMilestoneFile(base, "M001", "SUMMARY"); + const isStale = !dir || !!summaryFile; + assert.ok(isStale, "milestone with SUMMARY should be detected as stale"); + } finally { + cleanup(base); + } +}); + +test("valid milestone: exists and has no SUMMARY means paused session is valid", () => { + const base = makeTmpBase(); + mkdirSync(join(base, ".gsd", "milestones", "M001"), { recursive: true }); + try { + const dir = resolveMilestonePath(base, "M001"); + const summaryFile = resolveMilestoneFile(base, "M001", "SUMMARY"); + const isStale = !dir || !!summaryFile; + assert.ok(!isStale, "active milestone should not be detected as stale"); + } finally { + cleanup(base); + } +});