fix: guard allSlicesDone against vacuous truth on empty slice array (#2679)

deriveStateFromDb line 565 used activeMilestoneSlices.every() without a
length > 0 guard. In JavaScript, [].every() === true (vacuous truth),
which would cause a premature phase transition to validating-milestone
if the array were empty at that point.

While the current code has an early-return at line 536 that catches
length === 0, the guard is still necessary for consistency with the
identical checks at lines 368 and 413 (which both have the guard),
and to protect against future control-flow changes that might bypass
the early return.

Closes #2667
This commit is contained in:
mastertyko 2026-03-26 23:19:35 +01:00 committed by GitHub
parent c2cd8bcc0a
commit 8d77c40638
2 changed files with 119 additions and 1 deletions

View file

@ -562,7 +562,10 @@ export async function deriveStateFromDb(basePath: string): Promise<GSDState> {
}
// ── All slices done → validating/completing ─────────────────────────
const allSlicesDone = activeMilestoneSlices.every(s => isStatusDone(s.status));
// Guard: [].every() === true (vacuous truth). Without the length check,
// an empty slice array causes a premature phase transition to
// validating-milestone. See: https://github.com/gsd-build/gsd-2/issues/2667
const allSlicesDone = activeMilestoneSlices.length > 0 && activeMilestoneSlices.every(s => isStatusDone(s.status));
if (allSlicesDone) {
const validationFile = resolveMilestoneFile(basePath, activeMilestone.id, "VALIDATION");
const validationContent = validationFile ? await loadFile(validationFile) : null;

View file

@ -0,0 +1,115 @@
/**
* Regression test for #2667: deriveStateFromDb must NOT treat an empty
* slice array as "all slices done" due to JavaScript's vacuous-truth
* behavior of Array.prototype.every on an empty array.
*
* [].every(predicate) === true in JavaScript. Without a length > 0 guard,
* this causes a premature phase transition to validating-milestone when
* the DB returns 0 slices (e.g. after a worktree DB wipe).
*/
import { test } from "node:test";
import assert from "node:assert/strict";
import { mkdtempSync, mkdirSync, rmSync, writeFileSync } from "node:fs";
import { join } from "node:path";
import { tmpdir } from "node:os";
import { deriveStateFromDb, invalidateStateCache } from "../state.ts";
import {
openDatabase,
closeDatabase,
insertMilestone,
insertSlice,
} from "../gsd-db.ts";
test("deriveStateFromDb does NOT skip to validating when slice array is empty (#2667)", async () => {
const base = mkdtempSync(join(tmpdir(), "gsd-vacuous-truth-"));
mkdirSync(join(base, ".gsd", "milestones", "M001"), { recursive: true });
try {
// Set up a milestone with a roadmap that references slices,
// but the DB has NO slice rows (simulating a worktree DB wipe)
writeFileSync(
join(base, ".gsd", "milestones", "M001", "M001-ROADMAP.md"),
[
"# M001: Test Milestone",
"",
"## Slices",
"",
"### S01 — First Slice",
"Do something.",
"",
"### S02 — Second Slice",
"Do another thing.",
].join("\n"),
);
openDatabase(":memory:");
// Milestone exists but NO slices inserted — simulates DB wipe
insertMilestone({ id: "M001", title: "Test Milestone", status: "active" });
invalidateStateCache();
const state = await deriveStateFromDb(base);
// The phase must NOT be "validating-milestone" or "completing-milestone"
// because no slices have been executed — the empty array should not
// trigger the "all slices done" code path.
assert.notEqual(
state.phase,
"validating-milestone",
"empty slice array must not trigger validating-milestone (vacuous truth)",
);
assert.notEqual(
state.phase,
"completing-milestone",
"empty slice array must not trigger completing-milestone (vacuous truth)",
);
closeDatabase();
} finally {
closeDatabase();
rmSync(base, { recursive: true, force: true });
}
});
test("deriveStateFromDb correctly reaches validating when all slices are done (#2667 guard)", async () => {
const base = mkdtempSync(join(tmpdir(), "gsd-vacuous-truth-"));
mkdirSync(join(base, ".gsd", "milestones", "M001", "slices", "S01"), { recursive: true });
try {
writeFileSync(
join(base, ".gsd", "milestones", "M001", "M001-ROADMAP.md"),
[
"# M001: Test Milestone",
"",
"## Slices",
"",
"### S01 — First Slice",
"Do something.",
].join("\n"),
);
// Write a slice summary so the filesystem recognizes it as complete
writeFileSync(
join(base, ".gsd", "milestones", "M001", "slices", "S01", "S01-SUMMARY.md"),
"# S01 Summary\n\nDone.",
);
openDatabase(":memory:");
insertMilestone({ id: "M001", title: "Test Milestone", status: "active" });
insertSlice({ id: "S01", milestoneId: "M001", title: "First Slice", status: "complete", risk: "low", depends: [] });
invalidateStateCache();
const state = await deriveStateFromDb(base);
// With one slice that IS complete, phase should advance
assert.ok(
state.phase === "validating-milestone" || state.phase === "completing-milestone",
`expected validating or completing phase, got "${state.phase}"`,
);
closeDatabase();
} finally {
closeDatabase();
rmSync(base, { recursive: true, force: true });
}
});