fix: validate paused-session milestone before restoring it (#1664) (#1800)

This commit is contained in:
TÂCHES 2026-03-21 11:40:40 -06:00 committed by GitHub
parent 3e4be6babf
commit 48c25dc853
2 changed files with 165 additions and 10 deletions

View file

@ -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 {

View file

@ -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);
}
});