singularity/singularity-forge
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

fix(state): Cluster F — fail-closed SUMMARY checks in state.js and dispatch-guard.js

Browse source 
Three fail-open bugs allowed unreadable (null) SUMMARY files to be treated as
terminal, incorrectly marking milestones as complete when the content could not
be read.

Gap 1 — dispatch-guard.js line 50:
  Any SUMMARY file existence = milestone complete (fail-open).
  Fix: DB-first check via getMilestone()+isClosedStatus(); filesystem fallback
  reads SUMMARY content and calls classifyMilestoneSummaryContent() so only
  non-failure summaries skip the milestone.

Gap 2 — state.js getActiveMilestoneId():
  'if (summaryFile) continue' skipped any milestone with ANY SUMMARY.
  'if (!summaryFile) return mid' fell through incorrectly for failure SUMMARYs.
  Fix: read content; only skip/continue if sc != null && isTerminal(sc).

Gap 3 — state.js _deriveStateImpl() Phase 1 + Phase 2:
  '!sc || isTerminalMilestoneSummaryContent(sc)' — null content = fail-open.
  Fix: 'sc && isTerminalMilestoneSummaryContent(sc)' — null content = fail-closed.
  Applied to all 6 occurrences (lines 1233, 1247, 1257, 1284, 1356, 1391).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Mikael Hugo 2026-05-11 03:34:48 +02:00
parent 70afabedb7
commit c1df4249b8
2 changed files with 52 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

27
src/resources/extensions/sf/dispatch-guard.js
View file

@ -1,9 +1,10 @@
// SF Dispatch Guard — prevents out-of-order slice dispatch
import { readFileSync } from "node:fs";
import { findMilestoneIds } from "./milestone-ids.js";
import { classifyMilestoneSummaryContent } from "./milestone-summary-classifier.js";
import { parseRoadmap } from "./parsers.js";
import { resolveMilestoneFile } from "./paths.js";
import { getMilestoneSlices, isDbAvailable } from "./sf-db.js";
import { getMilestone, getMilestoneSlices, isDbAvailable } from "./sf-db.js";
import { isClosedStatus } from "./status-guards.js";
import { parseUnitId } from "./unit-id.js";
@ -47,7 +48,29 @@ export function getPriorSliceCompletionBlocker(
const milestoneIds = allIds.slice(0, targetIdx + 1);
for (const mid of milestoneIds) {
if (resolveMilestoneFile(base, mid, "PARKED")) continue;
if (resolveMilestoneFile(base, mid, "SUMMARY")) continue;
// DB-first: check milestone status authoritatively; only skip if DB confirms closed.
// Filesystem fallback: read SUMMARY and classify — skip only if it's a success/unknown
// summary, NOT a failure summary. Fail-closed: unreadable SUMMARY does not count as done.
if (isDbAvailable()) {
const milestoneRow = getMilestone(mid);
if (milestoneRow && isClosedStatus(milestoneRow.status)) continue;
} else {
const summaryPath = resolveMilestoneFile(base, mid, "SUMMARY");
if (summaryPath) {
let summaryContent = null;
try {
summaryContent = readFileSync(summaryPath, "utf-8");
} catch {
// unreadable — fail-closed: don't treat as complete
}
if (
summaryContent != null &&
classifyMilestoneSummaryContent(summaryContent) !== "failure"
) {
continue;
}
}
}
let slices = null;
if (isDbAvailable()) {
const rows = getMilestoneSlices(mid);

40
src/resources/extensions/sf/state.js
View file

@ -237,14 +237,23 @@ export async function getActiveMilestoneId(basePath) {
const content = roadmapFile ? await loadFile(roadmapFile) : null;
if (!content) {
const summaryFile = resolveMilestoneFile(basePath, mid, "SUMMARY");
if (summaryFile) continue;
if (summaryFile) {
// Fail-closed: only skip if SUMMARY is readable AND terminal (not a failure report).
const sc = await loadFile(summaryFile);
if (sc != null && isTerminalMilestoneSummaryContent(sc)) continue;
}
if (isGhostMilestone(basePath, mid)) continue;
return mid;
}
const roadmap = parseRoadmap(content);
if (!isMilestoneComplete(roadmap)) {
const summaryFile = resolveMilestoneFile(basePath, mid, "SUMMARY");
if (!summaryFile) return mid;
if (summaryFile) {
// Fail-closed: only treat as complete if readable AND terminal.
const sc = await loadFile(summaryFile);
if (sc != null && isTerminalMilestoneSummaryContent(sc)) continue;
}
return mid;
}
}
return null;
@ -1221,7 +1230,8 @@ export async function _deriveStateImpl(basePath) {
const sf = resolveMilestoneFile(basePath, mid, "SUMMARY");
if (sf) {
const sc = await cachedLoadFile(sf);
if (!sc || isTerminalMilestoneSummaryContent(sc))
// Fail-closed: only count as complete if SUMMARY is readable AND terminal.
if (sc && isTerminalMilestoneSummaryContent(sc))
completeMilestoneIds.add(mid);
}
continue;
@ -1234,7 +1244,8 @@ export async function _deriveStateImpl(basePath) {
const sf = resolveMilestoneFile(basePath, mid, "SUMMARY");
if (sf) {
const sc = await cachedLoadFile(sf);
if (!sc || isTerminalMilestoneSummaryContent(sc))
// Fail-closed: only count as complete if SUMMARY is readable AND terminal.
if (sc && isTerminalMilestoneSummaryContent(sc))
completeMilestoneIds.add(mid);
}
continue;
@ -1242,7 +1253,8 @@ export async function _deriveStateImpl(basePath) {
const sf = resolveMilestoneFile(basePath, mid, "SUMMARY");
if (sf) {
const sc = await cachedLoadFile(sf);
if (!sc || isTerminalMilestoneSummaryContent(sc))
// Fail-closed: only count as complete if SUMMARY is readable AND terminal.
if (sc && isTerminalMilestoneSummaryContent(sc))
completeMilestoneIds.add(mid);
}
}
@ -1266,18 +1278,17 @@ export async function _deriveStateImpl(basePath) {
const summaryFile = resolveMilestoneFile(basePath, mid, "SUMMARY");
if (summaryFile) {
const summaryContent = await cachedLoadFile(summaryFile);
// Fail-closed: only mark complete if SUMMARY is readable AND terminal.
if (
!summaryContent ||
summaryContent &&
isTerminalMilestoneSummaryContent(summaryContent)
) {
const summaryTitle = summaryContent
? parseSummary(summaryContent).title || mid
: mid;
const summaryTitle = parseSummary(summaryContent).title || mid;
registry.push({ id: mid, title: summaryTitle, status: "complete" });
completeMilestoneIds.add(mid);
continue;
}
// Failure summary — milestone is not yet done; fall through to active/pending logic
// Failure summary or unreadable — milestone is not yet done; fall through
}
// Ghost milestone (only META.json, no CONTEXT/ROADMAP/SUMMARY) — skip entirely
if (isGhostMilestone(basePath, mid)) continue;
@ -1339,15 +1350,16 @@ export async function _deriveStateImpl(basePath) {
!validationTerminal || verdict === "needs-remediation";
if (summaryFile) {
const summaryContent = await cachedLoadFile(summaryFile);
// Fail-closed: only mark complete if SUMMARY is readable AND terminal.
if (
!summaryContent ||
summaryContent &&
isTerminalMilestoneSummaryContent(summaryContent)
) {
// Terminal summary → milestone is complete. The summary is the terminal artifact (#864).
registry.push({ id: mid, title, status: "complete" });
continue;
}
// Failure summary — fall through to re-validation / active logic below
// Failure summary or unreadable — fall through to re-validation / active logic below
}
if (needsRevalidation && !activeMilestoneFound) {
// No terminal summary and needs (re-)validation → validating-milestone
@ -1374,9 +1386,11 @@ export async function _deriveStateImpl(basePath) {
const summaryContent = summaryFile
? await cachedLoadFile(summaryFile)
: null;
// Fail-closed: only mark complete if SUMMARY is readable AND terminal.
if (
summaryFile &&
(!summaryContent || isTerminalMilestoneSummaryContent(summaryContent))
summaryContent &&
isTerminalMilestoneSummaryContent(summaryContent)
) {
registry.push({ id: mid, title, status: "complete" });
} else if (!activeMilestoneFound) {