diff --git a/scripts/__tests__/reconcile-task-state-consistency.test.mjs b/scripts/__tests__/reconcile-task-state-consistency.test.mjs index b133b2b87f..4404a2037c 100644 --- a/scripts/__tests__/reconcile-task-state-consistency.test.mjs +++ b/scripts/__tests__/reconcile-task-state-consistency.test.mjs @@ -104,3 +104,65 @@ test("apply reconciles done task and emits exactly one note", async () => { assert.equal(task.recoveryRetryCount, undefined); assert.equal(task.nextRecoveryAt, undefined); }); + +/* +FNXC:OperatorScriptLaneAssumptions 2026-07-30-26:10: +THE INVARIANT: both consistency checks ask the task's OWN lanes, and they failed in OPPOSITE directions. + +Keyed on the literals, `hasDoneTransient` (`column === "done"`) NEVER fires on a renamed board, so a +finished card still holding a worktree and `status:"failed"` goes unreported and unnormalized. Meanwhile +`failed-status-outside-in-review` (`column !== "in-review"`) fires for EVERY failed card, because no +column equals the literal — a report listing the whole board, which looks like the tool working. + +Reverted, the first case returns [] (the miss) and the second returns the spurious flag (the flood). +*/ +test("reports stale transient state in a RENAMED complete lane", () => { + /* No `status:"failed"` here on purpose: it would ALSO trip the second check (a failed card outside + the review lane is genuinely flagged), which would blur which of the two this case is pinning. */ + const task = { id: "FN-R1", column: "shipped", worktree: "/tmp/wt" }; + + assert.deepEqual( + findTaskStateInconsistencies(task, { complete: "shipped", review: "checking" }), + ["done-task-has-transient-failure-state"], + ); +}); + +test("does NOT flag a failed card that is sitting in the board's own review lane", () => { + const task = { id: "FN-R2", column: "checking", status: "failed" }; + + assert.deepEqual(findTaskStateInconsistencies(task, { complete: "shipped", review: "checking" }), []); +}); + +test("still flags a failed card outside the resolved review lane", () => { + const task = { id: "FN-R3", column: "building", status: "failed" }; + + assert.deepEqual( + findTaskStateInconsistencies(task, { complete: "shipped", review: "checking" }), + ["failed-status-outside-in-review"], + ); +}); + +test("unresolved lanes keep exactly the legacy behaviour", () => { + assert.deepEqual( + findTaskStateInconsistencies({ id: "FN-L", column: "done", status: "failed" }), + ["done-task-has-transient-failure-state", "failed-status-outside-in-review"], + ); +}); + +test("runReconciliation normalizes a renamed complete lane by moving the card to its OWN column", async () => { + const moves = []; + const store = { + async listTasks() { return [{ id: "FN-R4", column: "shipped", status: "failed", worktree: "/tmp/w" }]; }, + async moveTask(id, toColumn) { moves.push([id, toColumn]); return { id }; }, + async logEntry() { return { id: "FN-R4" }; }, + }; + + const result = await runReconciliation({ + store, + dryRun: false, + resolveLanes: async () => ({ complete: "shipped", review: "checking" }), + }); + + assert.deepEqual(moves, [["FN-R4", "shipped"]]); + assert.equal(result.actions[0].action, "reconciled"); +}); diff --git a/scripts/lib/backend-db.mjs b/scripts/lib/backend-db.mjs index 631fa11bee..370864155a 100644 --- a/scripts/lib/backend-db.mjs +++ b/scripts/lib/backend-db.mjs @@ -38,7 +38,10 @@ function ensureMigrationsStaged() { } } -async function importCore() { +/* FNXC:OperatorScriptLaneAssumptions 2026-07-30-26:10: exported so operator scripts can reach core + helpers (lane resolution) through the SAME staged-dist seam `openBackend` already uses, rather than + each growing its own dist path — `@fusion/core` is not resolvable from the repo-root `scripts/`. */ +export async function importCore() { ensureMigrationsStaged(); try { return await import(pathToFileURL(resolve(repoRoot, "packages/core/dist/index.js")).href); diff --git a/scripts/reconcile-task-state-consistency.mjs b/scripts/reconcile-task-state-consistency.mjs index 8aba15a57b..a9db9c0a86 100644 --- a/scripts/reconcile-task-state-consistency.mjs +++ b/scripts/reconcile-task-state-consistency.mjs @@ -1,12 +1,33 @@ #!/usr/bin/env node import process from "node:process"; -import { openBackend } from "./lib/backend-db.mjs"; +import { openBackend, importCore } from "./lib/backend-db.mjs"; const DEFAULT_NOTE = "FN-4000 reconciliation: cleared stale transient failure state using TaskStore done-normalization so database and task JSON remain synchronized."; -export function findTaskStateInconsistencies(task) { +/* +FNXC:OperatorScriptLaneAssumptions 2026-07-30-26:10: +Both checks ask this task's OWN lanes, because keyed on the literals they fail in BOTH directions. + + `hasDoneTransient` gated on `column === "done"`. On a board whose complete lane is named anything + else it NEVER fires, so a finished card still carrying `status:"failed"`, a worktree, a blockedBy or + live recovery counters is never reported and never normalized — the exact stale state FN-4000 exists + to clear. + + `failed-status-outside-in-review` gated on `column !== "in-review"`, and fails the OPPOSITE way: on a + renamed board no column equals the literal, so EVERY failed task is flagged. A reconciliation report + listing the whole board is as useless as one listing nothing, and it is the more dangerous of the + two because it looks like the tool is working. + +Lanes arrive resolved from the caller rather than being resolved here, so this stays pure and testable +without a database or a built dist. +*/ +export function findTaskStateInconsistencies(task, lanes = {}) { + /* DELIBERATE-LITERAL — the degraded default when the caller resolved no lanes. */ + const completeColumn = lanes.complete ?? "done"; + /* DELIBERATE-LITERAL — as above. */ + const reviewColumn = lanes.review ?? "in-review"; const findings = []; - const hasDoneTransient = task.column === "done" && ( + const hasDoneTransient = task.column === completeColumn && ( task.status === "failed" || Boolean(task.error) || Boolean(task.worktree) @@ -19,20 +40,30 @@ export function findTaskStateInconsistencies(task) { findings.push("done-task-has-transient-failure-state"); } - if (task.status === "failed" && task.column !== "in-review") { + if (task.status === "failed" && task.column !== reviewColumn) { findings.push("failed-status-outside-in-review"); } return findings; } -export async function runReconciliation({ store, dryRun = true, noteByTaskId = {} }) { +/* +FNXC:OperatorScriptLaneAssumptions 2026-07-30-26:10: +`resolveLanes` is INJECTED, not built here, and `main` below supplies the real one. + +Resolving inside this function would drag `importCore()` — and therefore a built `packages/core/dist` +— into every unit test of a pure reconciliation loop. Injection keeps the tests database-free and +build-free while the production entry point still wires a real resolver, which is the wiring that +matters: an optional lane parameter no caller fills is the inert shape this migration keeps finding. +*/ +export async function runReconciliation({ store, dryRun = true, noteByTaskId = {}, resolveLanes = null }) { const tasks = await store.listTasks({ includeArchived: false }); const findings = []; const actions = []; for (const task of tasks) { - const issues = findTaskStateInconsistencies(task); + const lanes = resolveLanes ? ((await resolveLanes(task.id)) ?? {}) : {}; + const issues = findTaskStateInconsistencies(task, lanes); if (issues.length === 0) continue; findings.push({ taskId: task.id, column: task.column, status: task.status ?? null, issues }); @@ -42,8 +73,10 @@ export async function runReconciliation({ store, dryRun = true, noteByTaskId = { continue; } - if (task.column === "done") { - await store.moveTask(task.id, "done"); + if (task.column === (lanes.complete ?? "done")) { + /* Its OWN column: this move exists to trigger the store's done-normalization, so naming the + destination by literal was only ever a way of spelling "where it already is". */ + await store.moveTask(task.id, task.column); const note = noteByTaskId[task.id] ?? DEFAULT_NOTE; await store.logEntry(task.id, "FN-4000 reconciliation", note); actions.push({ taskId: task.id, action: "reconciled", issues }); @@ -62,6 +95,13 @@ function readFlagValue(argv, flag) { return argv[index + 1]; } +async function buildLaneResolver(store) { + const { resolveTaskLifecycleColumns } = await importCore(); + /* One resolution per WORKFLOW, not per task — the cache is what keeps this loop cheap on a big board. */ + const irCache = new Map(); + return (taskId) => resolveTaskLifecycleColumns(store, taskId, irCache); +} + export async function main(argv = process.argv.slice(2), deps = {}) { const dryRun = !argv.includes("--apply"); const projectDir = readFlagValue(argv, "--project-dir") ?? process.cwd(); @@ -74,7 +114,10 @@ export async function main(argv = process.argv.slice(2), deps = {}) { try { /* FNXC:PostgresOperationalScripts 2026-07-14-18:18: Consistency reconciliation must inspect and repair the authoritative PostgreSQL rows. */ - const result = await runReconciliation({ store, dryRun, noteByTaskId }); + /* Wired only when we opened a real backend: a caller injecting its own store (tests) has no + staged dist to import, and falls back to the documented legacy literals. */ + const resolveLanes = deps.resolveLanes ?? (backend ? await buildLaneResolver(store) : null); + const result = await runReconciliation({ store, dryRun, noteByTaskId, resolveLanes }); console.log(JSON.stringify({ dryRun, ...result }, null, 2)); return 0; } finally {