diff --git a/packages/core/src/__test-utils__/pg-test-harness.ts b/packages/core/src/__test-utils__/pg-test-harness.ts index 6845f30fd1..f48b0a864a 100644 --- a/packages/core/src/__test-utils__/pg-test-harness.ts +++ b/packages/core/src/__test-utils__/pg-test-harness.ts @@ -45,7 +45,7 @@ import { mkdtemp, rm, writeFile } from "node:fs/promises"; import { basename, join } from "node:path"; import { tmpdir } from "node:os"; import { describe as vitestDescribe } from "vitest"; -import postgres from "postgres"; +import postgres, { type Sql } from "postgres"; import { drizzle, type PostgresJsDatabase } from "drizzle-orm/postgres-js"; import { sql } from "drizzle-orm"; import type { ResolvedBackend } from "../postgres/backend-resolver.js"; @@ -209,6 +209,17 @@ export interface PgTestHarness { readonly layer: AsyncDataLayer; /** A separate admin Drizzle connection for direct row inspection/seeding. */ readonly adminDb: PostgresJsDatabase; + /* + FNXC:PgTestHarness 2026-07-27-18:55: + The RAW tagged-template client behind `adminDb`. Needed because several persisted fields are + stamped by the store on every write (`updatedAt`, `columnMovedAt`), so a fixture that must + present an AGED row — anything testing a staleness threshold — cannot express it through + `updateTask` at all: the patch is accepted and the value silently replaced with `now`. Consumers + outside `@fusion/core` also cannot reach `adminDb` usefully, since driving it needs `drizzle-orm` + and the table schema, neither of which is a dependency of the engine package. + Seeding only — never a substitute for asserting through the real read path. + */ + readonly adminSql: Sql; /** The temp rootDir used for filesystem-backed operations. */ readonly rootDir: string; /** The unique test database name (for diagnostics). */ @@ -812,6 +823,7 @@ export async function createTaskStoreForTest(options?: { store, layer, adminDb, + adminSql, rootDir, dbName, testUrl, @@ -892,6 +904,8 @@ export interface SharedPgTaskStoreHarness { readonly store: () => TaskStore; readonly layer: () => AsyncDataLayer; readonly adminDb: () => PostgresJsDatabase; + /** Raw admin SQL client — see the note on {@link PgTestHarness.adminSql}. */ + readonly adminSql: () => Sql; readonly beforeAll: () => Promise; readonly beforeEach: () => Promise; readonly afterEach: () => Promise; @@ -968,6 +982,10 @@ export function createSharedPgTaskStoreTestHarness(options?: { if (!harness) throw new Error("SharedPgTaskStoreHarness: beforeAll not called yet"); return harness.adminDb; }, + adminSql: () => { + if (!harness) throw new Error("SharedPgTaskStoreHarness: beforeAll not called yet"); + return harness.adminSql; + }, beforeAll: async () => { if (harness) return; // FNXC:PgTestTemplateDb 2026-07-19-17:20: diff --git a/packages/engine/src/__tests__/workflow-lifecycle-live-e2e.pg.test.ts b/packages/engine/src/__tests__/workflow-lifecycle-live-e2e.pg.test.ts index 79259872c3..a2129ceaf3 100644 --- a/packages/engine/src/__tests__/workflow-lifecycle-live-e2e.pg.test.ts +++ b/packages/engine/src/__tests__/workflow-lifecycle-live-e2e.pg.test.ts @@ -36,7 +36,7 @@ throwaway per-file database; never port 4040; no temp-root walk. */ import { beforeAll, beforeEach, afterEach, afterAll, expect, it, describe } from "vitest"; import "@fusion/core"; // registers the built-in column traits into the shared registry -import type { Settings, Task, TaskDetail, WorkflowIr } from "@fusion/core"; +import type { Settings, Task, TaskDetail, TaskStore, WorkflowIr } from "@fusion/core"; import { getWorkflowEventBus, resetWorkflowEventBusForTesting, @@ -53,6 +53,7 @@ import { WorkflowGraphTaskRunner, type WorkflowColumnBoundaryHooks } from "../wo import { createExecutorColumnBoundaryHooks } from "../workflow-column-boundary-hooks.js"; import { runHoldReleaseSweep } from "../hold-release.js"; import { SelfHealingManager } from "../self-healing.js"; +import { reconcileRecovery } from "../recovery-reconciler.js"; /** The four lifecycle roles this program's guards are supposed to resolve by TRAIT, not by id. */ interface Vocabulary { @@ -95,6 +96,10 @@ function lifecycleIr(v: Vocabulary, id: string): WorkflowIr { id: v.hold, name: "Hold", traits: [{ trait: "hold", config: { release: "capacity" } }], + /* U4 workflow-declared recovery policy (#2478). Declared on the HOLD column of both + vocabularies from the one builder, so the reconciler's role resolution is exercised + against a renamed column with nothing else differing. */ + recovery: { stalenessMs: HOLD_STALENESS_MS, onStale: { action: "surface", code: "e2e-stale-hold" } }, }, { id: v.wip, @@ -131,6 +136,8 @@ function lifecycleIr(v: Vocabulary, id: string): WorkflowIr { } as WorkflowIr; } +const HOLD_STALENESS_MS = 60 * 60_000; + const OK = { outcome: "success" as const }; /** Records which seams actually ran, so "exactly once" is asserted on real invocations. */ @@ -455,99 +462,284 @@ pgDescribe("live lifecycle E2E: real graph + real PostgreSQL store", () => { }); /* - The one converted lifecycle-mutating sweep on this tip (slice B3.1 — U4) run against a REAL - store and a REAL renamed workflow. Its conversion claim is that the sweep now resolves the hold - column from the card's own workflow instead of the `todo` literal in BOTH halves (query and - guard). Nothing so far has run it against a workflow that has no `todo` column at all. + FNXC:WorkflowLifecycleColumns 2026-07-27-18:30 (converted-sweep table): - The recovery callback performs a REAL `moveTask`, so the assertion is on the card's persisted - column afterwards — not on whether the callback was invoked. + A TABLE, not a scenario per sweep. The U4 conversion keeps ADDING callers of the lifecycle-role + resolution — the count went from one to two while PR #2475 was in review — so a suite with a + bespoke `describe` per sweep is a coverage claim that quietly goes stale. Adding a converted + sweep here is one entry. + + Each entry declares four things and the shared driver derives four assertions from them: + + seed — put a card in a given column in whatever state the sweep keys on + run — invoke the REAL sweep on a REAL store + acted — did the sweep act on this card? READ FROM THE PERSISTED ROW, never from a callback + roles — the lifecycle ROLE it must act on, and one it must stay inert in + + Derived per entry: {default vocabulary, renamed vocabulary} x {positive, negative}. The default + half is the regression floor; the renamed half is the claim the conversion exists for; the + negative half is what stops "resolve the column per task" from degrading into "act on everything". + + DELIBERATELY ROLE-TYPED. `actsOnRole`/`inertRole` are keys of `Vocabulary`, not column strings, so + an entry cannot accidentally hardcode `todo` and pass for the wrong reason. + + IF A SWEEP DOES NOT FIT. Report it rather than hand-rolling a one-off beside the table — a sweep + whose outcome cannot be observed on the persisted row is itself a finding about that sweep. See + the UNPROVEN SITES ledger at the foot of this file for what is not covered and why. */ - describe("converted self-healing sweep — recoverStrandedCompletedTodoTasks", () => { - async function seedStrandedTask(taskId: string, v: Vocabulary, workflowId: string, column: string) { + interface SweepCaseContext { + readonly store: TaskStore; + readonly taskId: string; + readonly vocab: Vocabulary; + readonly column: string; + readonly workflowId: string; + } + + interface ConvertedSweepCase { + /** The sweep method under test. */ + readonly sweep: string; + /** Where the converted lifecycle-role resolution lives, for the ledger. */ + readonly site: string; + /** Put a card in `ctx.column` in whatever state this sweep keys on. */ + readonly seed: (ctx: SweepCaseContext) => Promise; + /** Project settings this sweep needs in order to run at all. */ + readonly settings?: Record; + /** Invoke the REAL sweep. Returns whatever it produces, for decision-only entries. */ + readonly run: (store: TaskStore, vocab: Vocabulary) => Promise; + /* + WHERE THE OUTCOME IS OBSERVED. `persisted-row` is the strong form and the default expectation: + the sweep changed the row, and `acted` reads it back through `getTask`. `returned-decision` is + WEAKER EVIDENCE and is recorded as such — the sweep produces a decision it does not apply, so + there is no row to read. An entry must not silently use the weak form: naming it here is what + keeps the ledger at the foot of this file truthful about what "covered" means per site. + */ + readonly observability: "persisted-row" | "returned-decision"; + /** Did the sweep act? For `persisted-row` entries, read the row and ignore `runResult`. */ + readonly acted: (task: TaskDetail, vocab: Vocabulary, runResult: unknown) => boolean; + /** The lifecycle role the sweep must act on. */ + readonly actsOnRole: keyof Vocabulary; + /** A role of the SAME workflow the sweep must leave alone. */ + readonly inertRole: keyof Vocabulary; + } + + /** Promote through the store's real transition policy: hold → wip → review. + * A DIRECT hold → review move is refused ("Invalid transition: 'backlog' → 'checking'. Valid + * targets: building") and that refusal is itself workflow-resolved — the renamed board's only + * legal target is its own `building` — so it is honored rather than bypassed. */ + async function promoteThroughPolicy(store: TaskStore, taskId: string, v: Vocabulary): Promise { + for (const target of [v.wip, v.review]) { + await store.moveTask(taskId, target, { + moveSource: "engine", + bypassGuards: true, + preserveProgress: true, + allowDirectInReviewMove: true, + skipMergeBlocker: true, + } as never); + } + return true; + } + + const STALE_PAUSED_THRESHOLD_MS = 24 * 60 * 60_000; + const STALE_PAUSED_MARKER = "Stale paused todo surfaced ["; + + const CONVERTED_SWEEPS: ConvertedSweepCase[] = [ + { + sweep: "recoverStrandedCompletedTodoTasks", + observability: "persisted-row", + site: "self-healing.ts — resolveLifecycleColumns(...).hold (slice B3.1, U4)", + actsOnRole: "hold", + inertRole: "wip", + seed: async ({ store, taskId }) => { + // Fully-complete implementation steps are this sweep's entry condition. + await store.updateTask(taskId, { steps: [{ name: "only step", status: "done" }] } as never); + }, + run: async (store, vocab) => { + const manager = new SelfHealingManager(store, { + recoverCompletedTask: async (task: Task) => promoteThroughPolicy(store, task.id, vocab), + } as never); + await manager.recoverStrandedCompletedTodoTasks(); + }, + // Acted iff the card actually left the hold column for the review column. + acted: (task, vocab) => task.column === vocab.review, + }, + { + sweep: "surfaceStalePausedTodos", + observability: "persisted-row", + site: "self-healing.ts — resolveLifecycleColumns(...).hold (PR #2470 review, P1)", + actsOnRole: "hold", + inertRole: "wip", + settings: { stalePausedTodoThresholdMs: STALE_PAUSED_THRESHOLD_MS }, + seed: async ({ store, taskId }) => { + /* The store STAMPS `updatedAt`/`columnMovedAt` on every write, so `updateTask` cannot + express an aged row at all — the patch is accepted and the value silently replaced with + `now`, and the sweep then correctly finds nothing. Discovered by this case failing on + BOTH vocabularies, which is what distinguishes a broken fixture from a broken guard. + The age is therefore written through the harness's raw admin client. Seeding only: the + assertion still reads back through the real `getTask` path. */ + await store.updateTask(taskId, { paused: true, pausedReason: "e2e-hold" } as never); + const aged = new Date(Date.now() - STALE_PAUSED_THRESHOLD_MS * 3).toISOString(); + await h.adminSql()` + UPDATE project.tasks + SET column_moved_at = ${aged}, updated_at = ${aged} + WHERE id = ${taskId} + `; + store.taskCache.delete(taskId); + }, + run: async (store) => { + await new SelfHealingManager(store, {} as never).surfaceStalePausedTodos(); + }, + /* This sweep does not move the card — it writes an operator-facing log entry. That entry IS + persisted state (`task.log`), so it is still an observed-outcome assertion rather than a + spy; a sweep whose only effect were in-memory would not belong in this table at all. */ + acted: (task) => (task.log ?? []).some((e) => (e.action ?? "").startsWith(STALE_PAUSED_MARKER)), + }, + { + /* + Landed in #2478 AFTER the ledger below was first written — which is precisely the drift a + table exists to absorb: covering it was one entry, not a new describe block. + + WHAT THIS ROW ACTUALLY PROVES, established by mutation rather than by reading the code. The + census flagged `recovery-reconciler.ts:198` as a `resolveLifecycleColumns` call site, so the + row was first labelled as covering it. It does not: destroying the role resolution in + `resolveRoleRecovery` leaves all 18 tests GREEN, because `decideRecovery` looks the policy up + by COLUMN ID (`resolveColumnRecovery(ir, task.column)`) and never consults a role. + `resolveRoleRecovery` turns out to have NO production caller at all — see the ledger. + + What the row does prove, and what it is mutation-verified against: the reconciler resolves and + matches a column-declared recovery policy on a REAL renamed board — real store, real persisted + workflow definition, real `resolveWorkflowIrForTask`. Keying that lookup on the `todo` literal + fails exactly this row's renamed test. + + OBSERVABILITY CAVEAT, stated rather than hidden: `reconcileRecovery` DECIDES and does not + APPLY. The slice deliberately stops before the writer, so there is no persisted effect to read + and `acted` must inspect the returned decision. That is weaker evidence than every other row + here. Switch this to `persisted-row` the moment the applier lands. + */ + sweep: "reconcileRecovery (recovery-reconciler.ts)", + observability: "returned-decision", + site: "recovery-reconciler.ts — resolveColumnRecovery policy lookup on a renamed board (U4, #2478)", + actsOnRole: "hold", + inertRole: "wip", + seed: async ({ store, taskId }) => { + // Rest the card in its column long enough to pass the policy's stalenessMs. + const aged = new Date(Date.now() - HOLD_STALENESS_MS * 3).toISOString(); + await h.adminSql()` + UPDATE project.tasks + SET column_moved_at = ${aged}, updated_at = ${aged} + WHERE id = ${taskId} + `; + store.taskCache.delete(taskId); + }, + run: async (store) => { + const tasks = await store.listTasks({ includeArchived: false }); + return reconcileRecovery(store, tasks, { now: () => Date.now() }); + }, + acted: (task, _vocab, runResult) => + (runResult as Array<{ taskId: string; code: string }>).some( + (d) => d.taskId === task.id && d.code === "e2e-stale-hold", + ), + }, + ]; + + describe.each(CONVERTED_SWEEPS)("converted sweep — $sweep", (testCase) => { + /** Seed one card under `vocab` resting in `column`, then run the real sweep and report the + * PERSISTED outcome. */ + async function driveCase(taskId: string, vocab: Vocabulary, roleKey: keyof Vocabulary, key: string) { const store = h.store(); + if (testCase.settings) await store.updateSettings(testCase.settings as never); + const { workflowId } = await seedWorkflow(vocab, key); + const column = vocab[roleKey]; await store.createTaskWithReservedId( - { description: `stranded ${taskId}`, column } as never, + { description: `${testCase.sweep} ${taskId}`, column } as never, { taskId, applyDefaultWorkflowSteps: false } as never, ); await store.writeTaskWorkflowSelection(taskId, workflowId, []); - // Fully-complete implementation steps are the sweep's entry condition. - await store.updateTask(taskId, { - steps: [{ name: "only step", status: "done" }], - } as never); + await testCase.seed({ store, taskId, vocab, column, workflowId }); store.taskCache.delete(taskId); + + const runResult = await testCase.run(store, vocab); + + store.taskCache.delete(taskId); + const persisted = (await store.getTask(taskId)) as TaskDetail; + return { persisted, acted: testCase.acted(persisted, vocab, runResult) }; } - /** Run the real sweep with a recovery callback that performs REAL column moves. - * The move goes hold → wip → review because the store's transition policy REFUSES a direct - * hold → review move ("Invalid transition: 'backlog' → 'checking'. Valid targets: building"). - * That refusal is itself workflow-resolved — the renamed board's only legal target is its own - * `building`, not `in-progress` — so it is left in place rather than bypassed. */ - async function runSweep(v: Vocabulary): Promise<{ recovered: number; promoted: string[]; moveErrors: string[] }> { - const store = h.store(); - const promoted: string[] = []; - const moveErrors: string[] = []; - const manager = new SelfHealingManager(store, { - recoverCompletedTask: async (task: Task) => { - promoted.push(task.id); - try { - for (const target of [v.wip, v.review]) { - await store.moveTask(task.id, target, { - moveSource: "engine", - bypassGuards: true, - preserveProgress: true, - allowDirectInReviewMove: true, - skipMergeBlocker: true, - } as never); - } - } catch (e) { - moveErrors.push(e instanceof Error ? e.message : String(e)); - return false; - } - return true; - }, - } as never); - const recovered = await manager.recoverStrandedCompletedTodoTasks(); - return { recovered, promoted, moveErrors }; - } - - it("promotes a completed card stranded in a RENAMED hold column", async () => { - const v = RENAMED_VOCAB; - const { workflowId } = await seedWorkflow(v, "stranded-renamed"); - await seedStrandedTask("FN-E2E-8", v, workflowId, v.hold); - - const { recovered, promoted, moveErrors } = await runSweep(v); - - expect(moveErrors).toEqual([]); - expect(promoted).toContain("FN-E2E-8"); - expect(recovered).toBe(1); - // Observed state, not the callback: the card actually left the hold column. - expect(await persistedColumn("FN-E2E-8")).toBe(v.review); + it("acts on a card in the RENAMED lifecycle column", async () => { + const r = await driveCase("FN-SW-1", RENAMED_VOCAB, testCase.actsOnRole, "sweep-renamed"); + expect(r.acted).toBe(true); }); - it("does NOT promote a completed card resting in a non-hold column of the same renamed workflow", async () => { - /* The negative half. Dropping the column filter without a correct per-task hold resolution - turns this sweep into "promote every completed card anywhere", which is a louder failure - than the silence it replaces. */ - const v = RENAMED_VOCAB; - const { workflowId } = await seedWorkflow(v, "stranded-renamed-neg"); - await seedStrandedTask("FN-E2E-9", v, workflowId, v.wip); - - const { promoted } = await runSweep(v); - - expect(promoted).not.toContain("FN-E2E-9"); - expect(await persistedColumn("FN-E2E-9")).toBe(v.wip); + it("stays INERT for a card in a non-target column of the same renamed workflow", async () => { + const r = await driveCase("FN-SW-2", RENAMED_VOCAB, testCase.inertRole, "sweep-renamed-neg"); + expect(r.acted).toBe(false); + // and the card is untouched where it stands + expect(r.persisted.column).toBe(RENAMED_VOCAB[testCase.inertRole]); }); - it("still promotes a default-vocabulary card in `todo` (regression floor)", async () => { - const v = DEFAULT_VOCAB; - const { workflowId } = await seedWorkflow(v, "stranded-default"); - await seedStrandedTask("FN-E2E-10", v, workflowId, v.hold); + it("still acts on a DEFAULT-vocabulary card (regression floor)", async () => { + const r = await driveCase("FN-SW-3", DEFAULT_VOCAB, testCase.actsOnRole, "sweep-default"); + expect(r.acted).toBe(true); + }); - const { promoted } = await runSweep(v); - - expect(promoted).toContain("FN-E2E-10"); - expect(await persistedColumn("FN-E2E-10")).toBe(v.review); + it("stays INERT for a default-vocabulary card in a non-target column", async () => { + const r = await driveCase("FN-SW-4", DEFAULT_VOCAB, testCase.inertRole, "sweep-default-neg"); + expect(r.acted).toBe(false); }); }); }); + +/* +FNXC:WorkflowLifecycleColumns 2026-07-28-16:10 — UNPROVEN SITES LEDGER. + +Kept deliberately, and kept HONEST: the difference between "the E2E covers the conversion" and +"the E2E covers N of M sites" is this list. Census re-taken against main at b133d521c4 (it had +already drifted once — #2478 landed a new site between the first census and this one, which is why +the coverage above is a table). + +Census method: every caller of `resolveLifecycleColumns`, `resolveCompleteColumn`, +`resolveMergeOrchestrationColumn`, `resolveReboundTarget`, `resolveTaskLifecycleColumns`, +`columnHasFlag`, `columnsWithFlag` outside tests and the barrel re-exports. + +PROVEN end to end against a live renamed workflow by this file (each mutation-verified, and for the +two self-healing sweeps verified PER SITE — reverting one fails exactly its own row): + - self-healing.ts recoverStrandedCompletedTodoTasks (table row, persisted-row) + - self-healing.ts surfaceStalePausedTodos (table row, persisted-row) + - recovery-reconciler.ts column-declared policy lookup (table row, returned-decision — WEAKER) + - hold-release.ts isHeldTask / the capacity release (spine) + - the graph column boundary + store.moveTask + the post-commit bus (spine) + +FINDING — AN UNREACHABLE EXPORT. `resolveRoleRecovery` (recovery-reconciler.ts:194) is the ONLY +use of `resolveLifecycleColumns` in that file, and it has NO production caller: `decideRecovery` +looks policy up by column id. Established by mutation — destroying the role resolution leaves all +18 tests here green, and a repo-wide grep finds no caller outside this file. So that census line is +not a live converted site; it is an exported helper written ahead of its consumer. Either its +consumer is still to land, or it should be deleted. Not resolved here: it is production code owned +by the U4 slice, and guessing which is a decision for its author. + +NOT PROVEN end to end — real callers this suite does not reach: + - merger.ts:324-326 resolveCompleteColumn / resolveMergeOrchestrationColumn / resolveReboundTarget + - merger-ai.ts:1022,1039 resolveReboundTarget, resolveLifecycleColumns + - auto-merge-finalization.ts:20-22 completeColumn / mergeColumn / isCompleteColumn + - executor.ts:1763,6339,6341 rebound target, merge-orchestration probe, complete column + - self-healing.ts:713,6732 resolveReboundTarget (two distinct rebound paths) + - mesh-lease-manager.ts:61 resolveReboundTarget + - task-agent-sync.ts:59 resolveTaskLifecycleColumns + - core/task-store/reads.ts:130 listTasks hydration + - core/live-agent-count.ts:63-75 five columnHasFlag classifications + - dashboard register-task-workflow-routes.ts:151,166,175,1797 + +WHY, and what each would take: + - The merge/rebound family (merger, merger-ai, auto-merge-finalization, the executor rebound path, + mesh-lease-manager) needs a REAL git worktree, branch, and squash. This suite deliberately has + none — `merge-gate` is pure policy and the `merge` seam is scripted. They need an engine-slow + real-git lane, not another table row. + - The dashboard sites need an HTTP route test with a live store: reachable, different lane. + - `reads.ts:130` and `live-agent-count.ts` are read/hydration paths already covered at store level + by core's `store-stale-paused-renamed-hold.pg.test.ts`; what is missing is the end-to-end claim, + not the unit one. + +TABLE FIT. Three rows fit. The merge/rebound family does NOT — not because the table is too rigid, +but because those sweeps have no observable persisted effect without a real repository, so `acted` +cannot be written against the row at all. That is a finding about the lane they need, not a reason +to hand-roll a scenario beside the table. +*/