From 897cce9d94ac0fcfb97c78ea19d9549f47d628e4 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 31 Jul 2026 22:19:32 -0700 Subject: [PATCH] FN-8656: resolve scheduler lanes for renamed holds Resolve scheduler lane lookup against workflow-defined hold and terminal columns. - Use asynchronous workflow lane resolution after the synchronous event prologue - Preserve legacy lane fallback and recognize all terminal workflow columns - Update scheduler regression coverage, sync-lane guardrails, and release notes Files changed: .changeset/fn-8656-scheduler-renamed-hold-lanes.md | 7 ++ .../sync-workflow-ir-callsite-allowlist.test.ts | 10 +- .../scheduler-renamed-hold-events.test.ts | 20 ++-- ...ow-scheduler-parked-columns-live-e2e.pg.test.ts | 13 ++- ...-sync-role-conversion-inert-live-e2e.pg.test.ts | 8 +- packages/engine/src/scheduler.ts | 121 +++++++++------------ scripts/check-inert-sync-lane-conversions.mjs | 5 + scripts/lib/inert-sync-lane-baseline.json | 3 +- 8 files changed, 92 insertions(+), 95 deletions(-) Fusion-Task-Id: FN-8656 Fusion-Task-Lineage: 389a95a1-289f-4dde-86b3-1e450f8d43db Co-authored-by: Fusion (runfusion.ai) --- .../fn-8656-scheduler-renamed-hold-lanes.md | 7 ++ ...ync-workflow-ir-callsite-allowlist.test.ts | 10 +- .../scheduler-renamed-hold-events.test.ts | 20 +-- ...heduler-parked-columns-live-e2e.pg.test.ts | 13 +- ...-role-conversion-inert-live-e2e.pg.test.ts | 8 +- packages/engine/src/scheduler.ts | 119 +++++++----------- scripts/check-inert-sync-lane-conversions.mjs | 5 + scripts/lib/inert-sync-lane-baseline.json | 3 +- 8 files changed, 91 insertions(+), 94 deletions(-) create mode 100644 .changeset/fn-8656-scheduler-renamed-hold-lanes.md diff --git a/.changeset/fn-8656-scheduler-renamed-hold-lanes.md b/.changeset/fn-8656-scheduler-renamed-hold-lanes.md new file mode 100644 index 0000000000..1617125679 --- /dev/null +++ b/.changeset/fn-8656-scheduler-renamed-hold-lanes.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix scheduler not unblocking dependents on boards with renamed hold or terminal columns. +category: fix +dev: Scheduler now uses async workflow lanes after its synchronous event prologue. diff --git a/packages/core/src/__tests__/sync-workflow-ir-callsite-allowlist.test.ts b/packages/core/src/__tests__/sync-workflow-ir-callsite-allowlist.test.ts index 2649b6b408..363ee0925f 100644 --- a/packages/core/src/__tests__/sync-workflow-ir-callsite-allowlist.test.ts +++ b/packages/core/src/__tests__/sync-workflow-ir-callsite-allowlist.test.ts @@ -24,6 +24,11 @@ PostgreSQL (`docs/multi-project.md`), so a node-local sync cache of the selectio another node writes one — confidently wrong, which is worse than today's uniformly-wrong default. All three are proved and kept honest by `sync-workflow-ir-second-blocker.test.ts`. +FNXC:WorkflowLifecycleColumns 2026-08-01-05:01: +FN-8656 removed `scheduler.ts` from this list. Its synchronous `task:moved` prologue now consumes +emitter lanes over legacy defaults, while its post-await arms and agent-link rollback resolve lanes +asynchronously. Keeping the old exception would hide a reintroduced inert scheduler read. + That makes it the most dangerous tool in this conversion program. A guard written as resolveLifecycleColumns(store.resolveTaskWorkflowIrSync(id))?.hold @@ -66,11 +71,6 @@ const ALLOWED_CALL_SITES: ReadonlyMap = new Map([ + "board a caller branching on that flag is told the lanes are workflow-resolved while being " + "handed the DEFAULT ones.", ], - [ - "packages/engine/src/scheduler.ts", - "`resolveTaskParkedColumnsSync`, called from synchronous `task:moved` / `task:updated` listeners " - + "where introducing an await would reorder handlers against a synchronous emitter.", - ], ]); /** The declaration and the resolver's own module are not call sites. */ diff --git a/packages/engine/src/__tests__/scheduler-renamed-hold-events.test.ts b/packages/engine/src/__tests__/scheduler-renamed-hold-events.test.ts index 3d8d3cb01a..b9b1f71482 100644 --- a/packages/engine/src/__tests__/scheduler-renamed-hold-events.test.ts +++ b/packages/engine/src/__tests__/scheduler-renamed-hold-events.test.ts @@ -53,6 +53,16 @@ function renamedIr(): WorkflowIr { } as unknown as WorkflowIr; } +/* +FNXC:WorkflowResolvedColumns 2026-08-01-05:01: +With no `resolveTaskWorkflowIrSync` fixture, this exact command first produced 3 failed / 9 passed: +renamed-hold wake, renamed-hold dependency lookup, and second complete-trait terminal handling. After +FN-8656 moved await-safe scheduler arms to the async resolver, it produces 12 passed / 0 failed: +`pnpm --filter @fusion/engine exec vitest run src/__tests__/scheduler-renamed-hold-events.test.ts --silent=passed-only --reporter=dot`. + +The lane-less emits below are intentional. Reverting those arms to the sync resolver restores the +same three failures, proving the async fallback rather than a supplied payload resolves this fixture. +*/ function createStore(tasks: Record[] = [], ir: WorkflowIr = renamedIr()) { const listeners = new Map void)[]>(); const selection = { workflowId: WF, stepIds: [] }; @@ -76,16 +86,6 @@ function createStore(tasks: Record[] = [], ir: WorkflowIr = ren getTaskWorkflowSelection: vi.fn(() => selection), getTaskWorkflowSelectionAsync: vi.fn(async () => selection), getWorkflowDefinition: vi.fn(async () => ({ ir })), - /* - FNXC:WorkflowResolvedColumns 2026-08-01-02:07 MASKING: - Deleting the renamed-IR sync fixture and running - `pnpm --filter @fusion/engine exec vitest run src/__tests__/scheduler-renamed-hold-events.test.ts --silent=passed-only --reporter=dot` - produced 3 failed / 9 passed: renamed-hold wake, dependency lookup, and second-terminal tests. - The fake already supplies the authoritative async selection and IR readers, so this is not an - incomplete fake; synchronous scheduler listeners are inert on renamed boards in production. - Keep this logic fixture explicit while FN-8656 restructures the synchronous listener safely. - */ - resolveTaskWorkflowIrSync: vi.fn(() => ir), } as unknown as TaskStore; return { diff --git a/packages/engine/src/__tests__/workflow-scheduler-parked-columns-live-e2e.pg.test.ts b/packages/engine/src/__tests__/workflow-scheduler-parked-columns-live-e2e.pg.test.ts index 2193b614ec..5d1fd10ed9 100644 --- a/packages/engine/src/__tests__/workflow-scheduler-parked-columns-live-e2e.pg.test.ts +++ b/packages/engine/src/__tests__/workflow-scheduler-parked-columns-live-e2e.pg.test.ts @@ -49,11 +49,14 @@ That is also why the two arms share one helper and one settle constant rather th apart. If this ever flakes, the control fails first and says so — the fix is the quarantine ledger, never a larger number here. -CHARACTERIZATION, NOT ENDORSEMENT. The renamed expectation below asserts the WRONG-but-current -behaviour deliberately. When the sync read is replaced (see the PR body for why the obvious `await` -is not free — these run inside synchronous `task:moved`/`task:updated` listeners and an added await -reorders handlers against a synchronous emitter), this is the test that turns red and tells you the -operator-visible outcome moved. +REGRESSION, NOT CHARACTERIZATION. The renamed expectation now pins the corrected outcome: the +asynchronous resolver finds dependents in the workflow's real hold column, so the renamed and default +boards both clear `blockedBy` in the same settle window. + +FNXC:WorkflowResolvedColumns 2026-08-01-05:01: +FN-8656 applies the same async fallback to the await-safe `task:moved` arms. Its synchronous prologue +still consumes emitter lanes over legacy defaults, preserving event ordering; this file continues to +pin the live-store dependency invariant for lane-less or forwarded payloads. LANE. `.pg.test.ts`, skipped via `pgDescribe` when no PostgreSQL is reachable, so the merge gate is unaffected. Throwaway per-file database; never port 4040. diff --git a/packages/engine/src/__tests__/workflow-scheduler-sync-role-conversion-inert-live-e2e.pg.test.ts b/packages/engine/src/__tests__/workflow-scheduler-sync-role-conversion-inert-live-e2e.pg.test.ts index 4072dc6f1f..0ae9064ab1 100644 --- a/packages/engine/src/__tests__/workflow-scheduler-sync-role-conversion-inert-live-e2e.pg.test.ts +++ b/packages/engine/src/__tests__/workflow-scheduler-sync-role-conversion-inert-live-e2e.pg.test.ts @@ -7,6 +7,12 @@ the ten `task:moved` / `task:updated` handler arms by widening `resolveTaskParke `parked.complete` / `parked.archived` / `parked.wip`. The census fell by ten. The behaviour did not change at all, on any board. +FNXC:WorkflowLifecycleColumns 2026-08-01-05:01: +FN-8656 deletes that scheduler helper and removes every scheduler call site. This file remains the +live PostgreSQL proof that the sync resolver itself is inert, but it no longer claims a scheduler arm +consumes its result; the scheduler's await-safe arms now resolve asynchronously and its prologue uses +emitter lanes. + The reason is one line inside that helper: const l = resolveLifecycleColumns(store.resolveTaskWorkflowIrSync(taskId)); @@ -73,7 +79,7 @@ import { import { resolveLifecycleColumns } from "../../../core/src/workflow-lifecycle-traits.js"; import { DEFAULT_VOCAB, RENAMED_VOCAB, lifecycleIr, type Vocabulary } from "./_workflow-vocabulary-fixture.js"; -pgDescribe("scheduler sync-role conversion (#3051) against a live store", () => { +pgDescribe("sync workflow-role resolution remains inert without scheduler consumers", () => { const h: SharedPgTaskStoreHarness = createSharedPgTaskStoreTestHarness({ prefix: "fusion_sched_syncrole", }); diff --git a/packages/engine/src/scheduler.ts b/packages/engine/src/scheduler.ts index 90df991f12..d1ac94e356 100644 --- a/packages/engine/src/scheduler.ts +++ b/packages/engine/src/scheduler.ts @@ -475,20 +475,36 @@ Deferring them by a microtask changes nothing observable. Same fail-soft legacy default as its sync twin, so an unresolvable workflow behaves exactly as before. */ +const LEGACY_PARKED_COLUMNS = { + hold: "todo", + intake: "triage", + wip: "in-progress", + review: "in-review", + complete: "done", + archived: "archived", + terminal: new Set(["done", "archived"]), +}; + async function resolveTaskParkedColumns(store: TaskStore, taskId: string): Promise<{ hold: string; intake: string; wip: string; review: string; complete: string; archived: string; terminal: ReadonlySet; wake: ReadonlySet }> { - const legacy = { hold: "todo", intake: "triage", wip: "in-progress", review: "in-review", complete: "done", archived: "archived" }; try { - const l = resolveLifecycleColumns(await resolveWorkflowIrForTask(store, taskId)); - const complete = l?.complete ?? legacy.complete; - const archived = l?.archived ?? legacy.archived; + const ir = await resolveWorkflowIrForTask(store, taskId); + const l = resolveLifecycleColumns(ir); + const complete = l?.complete ?? LEGACY_PARKED_COLUMNS.complete; + const archived = l?.archived ?? LEGACY_PARKED_COLUMNS.archived; return { - hold: l?.hold ?? legacy.hold, - intake: l?.intake ?? legacy.intake, - wip: l?.wip ?? legacy.wip, - review: l?.review ?? legacy.review, + hold: l?.hold ?? LEGACY_PARKED_COLUMNS.hold, + intake: l?.intake ?? LEGACY_PARKED_COLUMNS.intake, + wip: l?.wip ?? LEGACY_PARKED_COLUMNS.wip, + review: l?.review ?? LEGACY_PARKED_COLUMNS.review, complete, archived, - terminal: new Set([complete, archived]), + terminal: new Set([ + ...LEGACY_PARKED_COLUMNS.terminal, + ...columnsWithFlag(ir, "complete"), + ...columnsWithFlag(ir, "archived"), + complete, + archived, + ]), /* FNXC:WorkflowResolvedColumns 2026-07-31-06:35 (fleet): The wake set UNIONS the legacy ids rather than replacing them, and that is load-bearing rather @@ -500,37 +516,17 @@ async function resolveTaskParkedColumns(store: TaskStore, taskId: string): Promi A resolved conversion must be a superset of what it replaces, or it is a behaviour change wearing a vocabulary change's clothes. */ - wake: new Set([l?.hold ?? legacy.hold, l?.intake ?? legacy.intake, legacy.hold, legacy.intake]), + wake: new Set([l?.hold ?? LEGACY_PARKED_COLUMNS.hold, l?.intake ?? LEGACY_PARKED_COLUMNS.intake, LEGACY_PARKED_COLUMNS.hold, LEGACY_PARKED_COLUMNS.intake]), }; } catch { return { - ...legacy, - terminal: new Set([legacy.complete, legacy.archived]), - wake: new Set([legacy.hold, legacy.intake]), + ...LEGACY_PARKED_COLUMNS, + terminal: new Set(LEGACY_PARKED_COLUMNS.terminal), + wake: new Set([LEGACY_PARKED_COLUMNS.hold, LEGACY_PARKED_COLUMNS.intake]), }; } } -function resolveTaskParkedColumnsSync(store: TaskStore, taskId: string): { hold: string; intake: string; wip: string; review: string; complete: string; archived: string; terminal: ReadonlySet } { - const legacy = { hold: "todo", intake: "triage", wip: "in-progress", review: "in-review", complete: "done", archived: "archived" }; - const legacyTerminal: ReadonlySet = new Set([legacy.complete, legacy.archived]); - try { - const ir = store.resolveTaskWorkflowIrSync(taskId); - const l = resolveLifecycleColumns(ir); - return { - hold: l?.hold ?? legacy.hold, - intake: l?.intake ?? legacy.intake, - wip: l?.wip ?? legacy.wip, - review: l?.review ?? legacy.review, - complete: l?.complete ?? legacy.complete, - archived: l?.archived ?? legacy.archived, - terminal: new Set([...legacyTerminal, ...columnsWithFlag(ir, "complete"), ...columnsWithFlag(ir, "archived")]), - }; - } catch { - return { ...legacy, terminal: legacyTerminal }; - } -} - export function shouldHoldActiveFileScopeLease( task: Task, tasks: Task[], @@ -1033,43 +1029,22 @@ export class Scheduler { * Also handles mission auto-advance: when a linked task completes, * update feature status and potentially activate next pending slice. */ - /* - FNXC:WorkflowLifecycleColumns 2026-07-30-20:10 (fleet — why the arms below are still literals): - - The ten `from`/`to` comparisons here are real backlog, deliberately left counted. The blocker is - ordering, not effort: this handler is `async` but its PROLOGUE is not — there is no `await` - between this line and the terminal-blocker branch (~55 lines down), so the snapshot invalidation, - PR-monitor start/stop, mission hand-off and failed-task tracking all run in the SAME TICK as the - emitter. Hoisting a resolution to convert those arms turns the prologue into a microtask and - reorders this listener against every other synchronous `task:moved` subscriber. That is the - hazard `resolveTaskParkedColumnsSync` exists to avoid — verified, not assumed. - - Lazy resolution inside a branch does not help (the CONDITION needs the lanes). A sync superset - prefilter does not either: it needs a predicate that cannot wrongly EXCLUDE on an unknown - vocabulary, and a renamed board's terminal id is unknown by construction. - - Unblocking it means either auditing every `task:moved` emitter/subscriber for prologue-ordering - dependence and then converting all ten together, or — preferred, since it removes the class - rather than one instance — having the emitter carry the resolved lanes on the event payload so - no listener resolves at all. - */ this.store.on("task:moved", async ({ task, from, to, source, lanes }) => { this.lastAutoClaimFingerprint.set(task.id, computeAutoClaimFingerprint(task)); /* - FNXC:WorkflowResolvedColumns 2026-07-31-21:00 (fleet): - PREFER the lanes the emitter resolved. The sync resolver below is inert in production — it - answers with the DEFAULT workflow under PostgreSQL — so before this it made every lane guard in - this listener behave exactly as the literal it replaced. + FNXC:WorkflowResolvedColumns 2026-08-01-05:01: + The synchronous prologue must remain in the emitter tick: snapshot invalidation, PR monitor + transitions, mission hand-off, and failure tracking are ordered by + `scheduler-auto-claim-invalidation.test.ts`. It therefore reads emitter-carried lanes over + legacy defaults without awaiting. The two SQLite polling-replica emitters that carry no lanes + retain that legacy behavior; PostgreSQL emitters carry lanes. - Resolving here instead was not an option: this listener's synchronous prologue is load-bearing - (`snapshotManager.invalidate` is asserted to run before the emit returns), and an await ahead of - it fails `scheduler-auto-claim-invalidation.test.ts`. Reading the answer off the payload keeps - the prologue synchronous AND makes the guard correct. - - The sync path stays as the fallback for emit sites that cannot resolve. It is no better than it - was, but it is no worse, and it is now the exception rather than the rule. + Every later arm is already behind an await, so it resolves the task's workflow asynchronously + instead of consulting the production-inert sync resolver. This covers forwarded or lane-less + payloads without changing synchronous ordering. Terminal remains a membership union so a second + complete-trait column reconciles dependents too. */ - const parked = mergeParkedColumns(resolveTaskParkedColumnsSync(this.store, task.id), lanes); + const parked = mergeParkedColumns(LEGACY_PARKED_COLUMNS, lanes); if (from === parked.hold || to === parked.hold) { this.options.snapshotManager?.invalidate(`task:moved:${from}->${to}`); } @@ -1119,14 +1094,16 @@ export class Scheduler { } } + const resolvedParked = mergeParkedColumns(await resolveTaskParkedColumns(this.store, task.id), lanes); + // FN-3895/FN-3924: complement periodic stale-blockedBy self-healing with immediate // blocker reconciliation when a potential blocker reaches a terminal completion column. // Invariant: blockedBy must reference a *current* unresolved blocker, else be null. - if (parked.terminal.has(to)) { + if (resolvedParked.terminal.has(to)) { try { const settings = await this.store.getSettings(); if (!settings.globalPause && !settings.enginePaused) { - const todoTasks = await this.store.listTasks({ column: parked.hold, slim: true }); + const todoTasks = await this.store.listTasks({ column: resolvedParked.hold, slim: true }); /* One IR cache for the whole reconciliation, per the caller-owned-cache contract. */ const dependencySatisfactionIrCache = new Map(); for (const dependent of todoTasks) { @@ -1195,13 +1172,13 @@ export class Scheduler { } } - if (from === parked.wip && to === parked.hold) { + if (from === resolvedParked.wip && to === resolvedParked.hold) { if (source === "engine") { this.recentEngineTodoRequeues.set(task.id, task.columnMovedAt ?? new Date().toISOString()); } else { this.recentEngineTodoRequeues.delete(task.id); } - } else if (to === parked.review || parked.terminal.has(to)) { + } else if (to === resolvedParked.review || resolvedParked.terminal.has(to)) { this.recentEngineTodoRequeues.delete(task.id); if (task.dispatchStormCount != null || task.lastDispatchAt != null || task.executeRequeueLoopCount != null || task.executeRequeueLoopSignature != null) { void this.store.updateTask(task.id, { @@ -1218,7 +1195,7 @@ export class Scheduler { // Event-driven scheduling: when a task moves to "done" (completion) or "todo" (retry/manual move), // trigger scheduling immediately so waiting tasks can start without waiting // for the next poll interval (up to 15 seconds). - if (parked.terminal.has(to) || to === parked.hold) { + if (resolvedParked.terminal.has(to) || to === resolvedParked.hold) { schedulerLog.log(`Task moved to ${to} — triggering scheduling`); this.schedule(); } @@ -1802,7 +1779,7 @@ export class Scheduler { which reads as unparked and clears a live agent's link. That invariant, not the conversion, is what the agent-link tests pin. */ - const rollbackParked = resolveTaskParkedColumnsSync(this.store, taskId); + const rollbackParked = await resolveTaskParkedColumns(this.store, taskId); const proof = evaluateParkedAgentTaskLink({ agent, linkedTask: { column: rollbackParked.hold } as Pick, diff --git a/scripts/check-inert-sync-lane-conversions.mjs b/scripts/check-inert-sync-lane-conversions.mjs index c5d66b1b0d..ab82af69ff 100644 --- a/scripts/check-inert-sync-lane-conversions.mjs +++ b/scripts/check-inert-sync-lane-conversions.mjs @@ -41,6 +41,11 @@ re-recording, because from inside this check a fall caused by denser encoding is from a fall caused by a gate being deleted — and only the second is a defect. That check is the standing obligation attached to every `--update-baseline`. +FNXC:LifecycleColumnCensus 2026-08-01-05:01: +FN-8656 re-records the scheduler drop because it removes the inert sync-lane source entirely, not +because it deletes this guard. The synchronous prologue now uses emitter lanes and every await-safe +arm resolves asynchronously, so there are no sync-derived scheduler guards left to census. + WHY A RISE AND NOT ZERO. Existing sync-resolved guards are real, deliberate, and documented — the scheduler's listeners genuinely cannot await today, and their authors said so. Demanding zero would force either a revert or an exemption marker on day one. What must not happen is MORE literals diff --git a/scripts/lib/inert-sync-lane-baseline.json b/scripts/lib/inert-sync-lane-baseline.json index 8413f12a02..de7ab58438 100644 --- a/scripts/lib/inert-sync-lane-baseline.json +++ b/scripts/lib/inert-sync-lane-baseline.json @@ -1,8 +1,7 @@ { - "total": 19, + "total": 6, "byFile": { "packages/engine/src/executor.ts": 2, - "packages/engine/src/scheduler.ts": 13, "packages/engine/src/triage.ts": 4 } }