From c1df4249b83347a8fa7b611f4a8b9a0931f06abe Mon Sep 17 00:00:00 2001 From: Mikael Hugo Date: Mon, 11 May 2026 03:34:48 +0200 Subject: [PATCH] =?UTF-8?q?fix(state):=20Cluster=20F=20=E2=80=94=20fail-cl?= =?UTF-8?q?osed=20SUMMARY=20checks=20in=20state.js=20and=20dispatch-guard.?= =?UTF-8?q?js?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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> --- src/resources/extensions/sf/dispatch-guard.js | 27 ++++++++++++- src/resources/extensions/sf/state.js | 40 +++++++++++++------ 2 files changed, 52 insertions(+), 15 deletions(-) diff --git a/src/resources/extensions/sf/dispatch-guard.js b/src/resources/extensions/sf/dispatch-guard.js index 47a86b3cb..26e680055 100644 --- a/src/resources/extensions/sf/dispatch-guard.js +++ b/src/resources/extensions/sf/dispatch-guard.js @@ -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); diff --git a/src/resources/extensions/sf/state.js b/src/resources/extensions/sf/state.js index f809f2772..343bb62e5 100644 --- a/src/resources/extensions/sf/state.js +++ b/src/resources/extensions/sf/state.js @@ -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) {