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

refactor(state): extract loadTerminalSummary helper, dedup 5 fail-closed SUMMARY checks

Browse source 
The 'read SUMMARY → check if readable AND terminal' pattern appeared five
times in state.js after the Cluster F polarity fix. Extract it to a
private loadTerminalSummary(summaryFile, loadFn) helper so the fail-closed
semantics live in one place and can't drift between call sites.

- loadTerminalSummary returns the content if readable AND terminal, null otherwise
- All 5 call sites replaced: 2 in getActiveMilestoneId(), 3 in _deriveStateImpl()
- Phase 2 'no roadmap' case reuses returned content for parseSummary().title
- isTerminalMilestoneSummaryContent now only referenced inside the helper

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Mikael Hugo 2026-05-11 03:46:36 +02:00
parent 3f0a02fe13
commit 65da855c5e
1 changed files with 40 additions and 62 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -182,6 +182,25 @@ function buildDbRecoveryRequiredState() {
progress: { milestones: { done: 0, total: 0 } },
};
}
/**
* Load SUMMARY content for a milestone and return it only if it is readable
* AND classifies as terminal (success/unknown, not failure).
*
* Purpose: centralise the fail-closed SUMMARY check so the pattern
* "only treat a SUMMARY as terminal if we can read it AND it passes the
* classifier" cannot drift between the five call sites in state derivation.
* Returning null on either an unreadable file or a failure summary ensures
* that a corrupt or failure SUMMARY never silently marks a milestone complete.
*
* Consumer: getActiveMilestoneId() and _deriveStateImpl() Phase 1 + Phase 2.
*/
async function loadTerminalSummary(summaryFile, loadFn) {
if (!summaryFile) return null;
const sc = await loadFn(summaryFile);
if (sc == null || !isTerminalMilestoneSummaryContent(sc)) return null;
return sc;
}
/**
* Invalidate the deriveState() cache. Call this whenever planning files on disk
* may have changed (unit completion, merges, file writes).
@ -237,22 +256,14 @@ export async function getActiveMilestoneId(basePath) {
const content = roadmapFile ? await loadFile(roadmapFile) : null;
if (!content) {
const summaryFile = resolveMilestoneFile(basePath, mid, "SUMMARY");
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 (await loadTerminalSummary(summaryFile, loadFile)) continue;
if (isGhostMilestone(basePath, mid)) continue;
return mid;
}
const roadmap = parseRoadmap(content);
if (!isMilestoneComplete(roadmap)) {
const summaryFile = resolveMilestoneFile(basePath, mid, "SUMMARY");
if (summaryFile) {
// Fail-closed: only treat as complete if readable AND terminal.
const sc = await loadFile(summaryFile);
if (sc != null && isTerminalMilestoneSummaryContent(sc)) continue;
}
if (await loadTerminalSummary(summaryFile, loadFile)) continue;
return mid;
}
}
@ -1228,12 +1239,8 @@ export async function _deriveStateImpl(basePath) {
const rc = rf ? await cachedLoadFile(rf) : null;
if (!rc) {
const sf = resolveMilestoneFile(basePath, mid, "SUMMARY");
if (sf) {
const sc = await cachedLoadFile(sf);
// Fail-closed: only count as complete if SUMMARY is readable AND terminal.
if (sc && isTerminalMilestoneSummaryContent(sc))
completeMilestoneIds.add(mid);
}
if (await loadTerminalSummary(sf, cachedLoadFile))
completeMilestoneIds.add(mid);
continue;
}
const rmap = parseRoadmap(rc);
@ -1242,21 +1249,13 @@ export async function _deriveStateImpl(basePath) {
// Summary is the terminal artifact — if it exists and is terminal, the milestone is
// complete even when roadmap checkboxes weren't ticked (#864).
const sf = resolveMilestoneFile(basePath, mid, "SUMMARY");
if (sf) {
const sc = await cachedLoadFile(sf);
// Fail-closed: only count as complete if SUMMARY is readable AND terminal.
if (sc && isTerminalMilestoneSummaryContent(sc))
completeMilestoneIds.add(mid);
}
if (await loadTerminalSummary(sf, cachedLoadFile))
completeMilestoneIds.add(mid);
continue;
}
const sf = resolveMilestoneFile(basePath, mid, "SUMMARY");
if (sf) {
const sc = await cachedLoadFile(sf);
// Fail-closed: only count as complete if SUMMARY is readable AND terminal.
if (sc && isTerminalMilestoneSummaryContent(sc))
completeMilestoneIds.add(mid);
}
if (await loadTerminalSummary(sf, cachedLoadFile))
completeMilestoneIds.add(mid);
}
// Phase 2: Build registry using cached roadmaps (no re-parsing or re-reading)
const registry = [];
@ -1276,20 +1275,14 @@ export async function _deriveStateImpl(basePath) {
if (!roadmap) {
// No roadmap — check if a terminal summary exists (completed milestone without roadmap)
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 &&
isTerminalMilestoneSummaryContent(summaryContent)
) {
const summaryTitle = parseSummary(summaryContent).title || mid;
registry.push({ id: mid, title: summaryTitle, status: "complete" });
completeMilestoneIds.add(mid);
continue;
}
// Failure summary or unreadable — milestone is not yet done; fall through
const sc = await loadTerminalSummary(summaryFile, cachedLoadFile);
if (sc) {
const summaryTitle = parseSummary(sc).title || mid;
registry.push({ id: mid, title: summaryTitle, status: "complete" });
completeMilestoneIds.add(mid);
continue;
}
// 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;
// No roadmap and no summary — treat as incomplete/active
@ -1348,19 +1341,12 @@ export async function _deriveStateImpl(basePath) {
// needs-remediation is terminal but requires re-validation (#3596)
const needsRevalidation =
!validationTerminal || verdict === "needs-remediation";
if (summaryFile) {
const summaryContent = await cachedLoadFile(summaryFile);
// Fail-closed: only mark complete if SUMMARY is readable AND terminal.
if (
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 or unreadable — fall through to re-validation / active logic below
if (await loadTerminalSummary(summaryFile, cachedLoadFile)) {
// Terminal summary → milestone is complete. The summary is the terminal artifact (#864).
registry.push({ id: mid, title, status: "complete" });
continue;
}
// Failure summary or unreadable — fall through to re-validation / active logic below
if (needsRevalidation && !activeMilestoneFound) {
// No terminal summary and needs (re-)validation → validating-milestone
activeMilestone = { id: mid, title };
@ -1383,15 +1369,7 @@ export async function _deriveStateImpl(basePath) {
// Roadmap slices not all checked — but if a terminal summary exists, the
// milestone is still complete. The summary is the terminal artifact (#864).
const summaryFile = resolveMilestoneFile(basePath, mid, "SUMMARY");
const summaryContent = summaryFile
? await cachedLoadFile(summaryFile)
: null;
// Fail-closed: only mark complete if SUMMARY is readable AND terminal.
if (
summaryFile &&
summaryContent &&
isTerminalMilestoneSummaryContent(summaryContent)
) {
if (await loadTerminalSummary(summaryFile, cachedLoadFile)) {
registry.push({ id: mid, title, status: "complete" });
} else if (!activeMilestoneFound) {
// Check milestone-level dependencies before promoting to active.