From 6483f9ce2b5f2d2eca6b65c35bb05a8d6334e739 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 31 Jul 2026 05:41:16 -0700 Subject: [PATCH] =?UTF-8?q?fix(scheduler):=20resolve=20task:updated=20/=20?= =?UTF-8?q?task:deleted=20lanes=20asynchronously=20(scheduler=20inert=205?= =?UTF-8?q?=20=E2=86=92=200)=20(#3128)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The last inert guards in `scheduler.ts`. Independent of my other branches. ## Inert-guard ratchet | Scope | Before | After | |---|---:|---:| | `scheduler.ts` | 5 | **0** | | total | 12 | **7** (triage.ts 8 → other worker; executor.ts 4 → #3112) | ## The live bug These read `resolveTaskParkedColumnsSync`, which answers with the **default** workflow in production. On a renamed board the scheduler **never woke** on unpause or planning-finish, and a **deleted blocker never unblocked its dependents** — the card sat behind a task that no longer existed. ## The criterion, restated because I got it wrong before **What blocks a guard is whether its answer is consumed synchronously — not whether the enclosing listener is declared sync.** I assumed the latter earlier in this program and reverted for it. All three fail that test: two only gate `schedule()`, which is itself `async`, fire-and-forget and re-entrance-guarded; the third already sits below an `await getSettings()`. The edge-trigger bookkeeping (`planningTaskIds.delete`) **stays synchronous** on purpose — deferring *that* would let a second update re-enter the branch. ## The union is load-bearing, not defensive Post-U11 the default lineage has no `triage` column, so a **resolved** answer returns `intake: "todo"` where the inert path fell back to `"triage"`. Converting without unioning the legacy ids silently **narrowed** the wake set and stopped waking cards in a legacy-named lane — caught by *"schedules when planning clears in triage"*. **A resolved conversion must be a superset of what it replaces, or it is a behaviour change wearing a vocabulary change's clothes.** That's the reusable lesson here. ## Tests - Drained with the repo's existing **`flushAsyncHandlers`** helper — written for exactly this fire-and-forget shape — rather than loosening any assertion. - **The characterization test flipped, as designed.** `workflow-scheduler-parked-columns-live-e2e.pg.test.ts` asserted *"a dependent in a RENAMED hold column is NEVER unblocked"*, with its author noting: *"expected to flip to null the moment the resolver is fixed — and that flip is the whole point of writing it down."* It flipped. Inverted to a REGRESSION case so the assertion holds the fix rather than the defect; it now matches its own CONTROL arm, which still guards against a vacuous pass. ## Verification - 21 scheduler suites — **361 green**, including the live PostgreSQL e2e - **`pnpm test:gate` green**; eslint and `tsc` clean - Changeset added; `check:changesets` passes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) --- .changeset/scheduler-async-lanes.md | 7 ++ .../scheduler-auto-claim-invalidation.test.ts | 5 +- .../scheduler-planning-finished-wake.test.ts | 12 ++- .../scheduler-renamed-hold-events.test.ts | 6 ++ ...heduler-parked-columns-live-e2e.pg.test.ts | 23 +++-- packages/engine/src/scheduler.ts | 92 +++++++++++++++---- scripts/lib/inert-sync-lane-baseline.json | 3 +- 7 files changed, 115 insertions(+), 33 deletions(-) create mode 100644 .changeset/scheduler-async-lanes.md diff --git a/.changeset/scheduler-async-lanes.md b/.changeset/scheduler-async-lanes.md new file mode 100644 index 0000000000..4f49bf5db7 --- /dev/null +++ b/.changeset/scheduler-async-lanes.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Renamed hold and intake lanes now wake the scheduler and unblock dependents correctly. +category: fix +dev: The `task:updated`/`task:deleted` handlers resolve lanes asynchronously; the wake set unions legacy ids so it stays a superset. diff --git a/packages/engine/src/__tests__/scheduler-auto-claim-invalidation.test.ts b/packages/engine/src/__tests__/scheduler-auto-claim-invalidation.test.ts index 6acdd176f8..b2a5cd922b 100644 --- a/packages/engine/src/__tests__/scheduler-auto-claim-invalidation.test.ts +++ b/packages/engine/src/__tests__/scheduler-auto-claim-invalidation.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it, vi } from "vitest"; import type { TaskStore } from "@fusion/core"; import { Scheduler } from "../scheduler.js"; +import { flushAsyncHandlers } from "./_flush-async-handlers.js"; function createStore() { const listeners = new Map void)[]>(); @@ -114,7 +115,7 @@ describe("Scheduler auto-claim snapshot invalidation", () => { expect(invalidate).toHaveBeenNthCalledWith(2, "task:updated"); }); - it("triggers immediate scheduling when a userPaused-only task is unpaused", () => { + it("triggers immediate scheduling when a userPaused-only task is unpaused", async () => { const { store, emit } = createStore(); const scheduler = new Scheduler(store, {}); const schedule = vi.spyOn(scheduler, "schedule").mockResolvedValue(undefined); @@ -123,6 +124,8 @@ describe("Scheduler auto-claim snapshot invalidation", () => { emit("task:updated", createTask({ userPaused: true })); emit("task:updated", createTask({ userPaused: false })); + await flushAsyncHandlers(); + expect(schedule).toHaveBeenCalledTimes(1); }); diff --git a/packages/engine/src/__tests__/scheduler-planning-finished-wake.test.ts b/packages/engine/src/__tests__/scheduler-planning-finished-wake.test.ts index d5f24b700c..447ecc0c55 100644 --- a/packages/engine/src/__tests__/scheduler-planning-finished-wake.test.ts +++ b/packages/engine/src/__tests__/scheduler-planning-finished-wake.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it, vi } from "vitest"; import type { TaskStore } from "@fusion/core"; import { Scheduler } from "../scheduler.js"; +import { flushAsyncHandlers } from "./_flush-async-handlers.js"; /* FNXC:CodingIdeasWorkflow 2026-07-25-13:10: @@ -73,26 +74,29 @@ function createScheduler() { } describe("Scheduler wakes on the planning -> dispatchable transition", () => { - it("schedules when planning clears in todo", () => { + it("schedules when planning clears in todo", async () => { const { emit, schedule } = createScheduler(); emit("task:updated", createTask({ status: "planning" })); expect(schedule).not.toHaveBeenCalled(); // still planning — nothing to dispatch yet emit("task:updated", createTask({ status: null })); + await flushAsyncHandlers(); expect(schedule).toHaveBeenCalledTimes(1); }); - it("schedules when planning clears in triage", () => { + it("schedules when planning clears in triage", async () => { const { emit, schedule } = createScheduler(); emit("task:updated", createTask({ column: "triage", status: "planning" })); emit("task:updated", createTask({ column: "triage", status: null })); + await flushAsyncHandlers(); + expect(schedule).toHaveBeenCalledTimes(1); }); - it("fires once per transition, not on every later update", () => { + it("fires once per transition, not on every later update", async () => { const { emit, schedule } = createScheduler(); emit("task:updated", createTask({ status: "planning" })); @@ -100,6 +104,8 @@ describe("Scheduler wakes on the planning -> dispatchable transition", () => { emit("task:updated", createTask({ status: null })); emit("task:updated", createTask({ status: null })); + await flushAsyncHandlers(); + expect(schedule).toHaveBeenCalledTimes(1); }); 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 e50ff81440..5d47a40140 100644 --- a/packages/engine/src/__tests__/scheduler-renamed-hold-events.test.ts +++ b/packages/engine/src/__tests__/scheduler-renamed-hold-events.test.ts @@ -32,6 +32,7 @@ import { describe, expect, it, vi } from "vitest"; import type { TaskStore, WorkflowIr } from "@fusion/core"; import { Scheduler } from "../scheduler.js"; import { evaluateParkedAgentTaskLink } from "../task-agent-sync.js"; +import { flushAsyncHandlers } from "./_flush-async-handlers.js"; const WF = "custom:wf"; @@ -144,6 +145,7 @@ describe("scheduler event handlers under a renamed hold column", () => { it("wakes when a card moves INTO the renamed hold column", async () => { const { emit, schedule } = createScheduler(); await emit("task:moved", { task: task(), from: "building", to: "drafting", source: "engine" }); + await flushAsyncHandlers(); expect(schedule).toHaveBeenCalled(); }); @@ -151,6 +153,7 @@ describe("scheduler event handlers under a renamed hold column", () => { /* The negative half: converting must not turn every move into a wake. */ const { emit, schedule } = createScheduler(); await emit("task:moved", { task: task({ column: "building" }), from: "inbox", to: "building", source: "user" }); + await flushAsyncHandlers(); expect(schedule).not.toHaveBeenCalled(); }); }); @@ -160,6 +163,7 @@ describe("scheduler event handlers under a renamed hold column", () => { const { emit, schedule } = createScheduler(); await emit("task:updated", task({ paused: true })); await emit("task:updated", task({ paused: false })); + await flushAsyncHandlers(); expect(schedule).toHaveBeenCalled(); }); @@ -167,6 +171,7 @@ describe("scheduler event handlers under a renamed hold column", () => { const { emit, schedule } = createScheduler(); await emit("task:updated", task({ column: "inbox", status: "planning" })); await emit("task:updated", task({ column: "inbox", status: null })); + await flushAsyncHandlers(); expect(schedule).toHaveBeenCalled(); }); @@ -174,6 +179,7 @@ describe("scheduler event handlers under a renamed hold column", () => { const { emit, schedule } = createScheduler(); await emit("task:updated", task({ column: "building", status: "planning" })); await emit("task:updated", task({ column: "building", status: null })); + await flushAsyncHandlers(); expect(schedule).not.toHaveBeenCalled(); }); }); 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 d02a7e0a8c..3154ad11da 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 @@ -166,20 +166,23 @@ pgDescribe("scheduler parked-column resolution against a live store", () => { expect(await unblockOutcome(store, DEFAULT_VOCAB, "wf-default-parked")).toBeNull(); }); - it("CHARACTERIZATION — a dependent in a RENAMED hold column is NEVER unblocked", async () => { + it("REGRESSION — a dependent in a RENAMED hold column IS unblocked when its blocker is deleted", async () => { /* - The live consequence of the inert sync read. `resolveTaskParkedColumnsSync` answers - `{ hold: "todo" }` for this task even though its workflow's hold column is `backlog`, so the - reconciliation queries a column this board does not have, finds no dependents, and leaves - `blockedBy` pointing at a task that no longer exists. + FNXC:WorkflowResolvedColumns 2026-08-01-02:20 (fleet — the flip this test was written to catch): + This was a CHARACTERIZATION of the inert sync read: `resolveTaskParkedColumnsSync` answered + `{ hold: "todo" }` for a board whose hold column is `backlog`, so the reconciliation queried a + column that does not exist, found no dependents, and left `blockedBy` pointing at a deleted task. - Asserting the defect, not blessing it. Expected to flip to `null` the moment the resolver is - fixed — and that flip is the whole point of writing it down. + Its author wrote "expected to flip to `null` the moment the resolver is fixed — and that flip is + the whole point of writing it down." The `task:deleted` handler now resolves asynchronously, so it + has flipped, and the assertion is inverted to hold the fix rather than deleted. + + It now asserts the SAME thing as the CONTROL above, which is the point: the renamed board and the + default board must behave identically. The control still earns its place — if the settle window + were too short, both would return null and this would pass vacuously. */ const store = h.store(); - const outcome = await unblockOutcome(store, RENAMED_VOCAB, "wf-renamed-parked"); - expect(outcome).not.toBeNull(); - expect(outcome).toMatch(/^[A-Z]+-\d+$/); + expect(await unblockOutcome(store, RENAMED_VOCAB, "wf-renamed-parked")).toBeNull(); }); }); diff --git a/packages/engine/src/scheduler.ts b/packages/engine/src/scheduler.ts index d0c9860914..0c945a6e5d 100644 --- a/packages/engine/src/scheduler.ts +++ b/packages/engine/src/scheduler.ts @@ -449,6 +449,55 @@ function mergeParkedColumns( }; } +/* +FNXC:WorkflowResolvedColumns 2026-08-01-01:40 (fleet): +The ASYNC twin. The sync one below cannot answer for a custom workflow in production, so any guard that +can reach this one must. + +THE CRITERION IS WHETHER THE ANSWER IS CONSUMED SYNCHRONOUSLY, not whether the enclosing listener is +declared sync — I got that wrong earlier in this program and it cost a revert. The three call sites +converted to this all fail that test: two only feed `schedule()`, which is itself `async`, +fire-and-forget and re-entrance-guarded, and the third already sits below an `await getSettings()`. +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. +*/ +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; + return { + hold: l?.hold ?? legacy.hold, + intake: l?.intake ?? legacy.intake, + wip: l?.wip ?? legacy.wip, + review: l?.review ?? legacy.review, + complete, + archived, + terminal: new Set([complete, archived]), + /* + FNXC:WorkflowResolvedColumns 2026-08-01-02:05 (fleet): + The wake set UNIONS the legacy ids rather than replacing them, and that is load-bearing rather + than defensive. Post-U11 the default lineage has no `triage` column, so a RESOLVED answer returns + `intake: "todo"` where the old inert path fell back to `"triage"`. Converting without the union + therefore NARROWS the set and stops waking cards that sit in a legacy-named lane — caught by + `scheduler-planning-finished-wake.test.ts` -> "schedules when planning clears in triage". + + 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]), + }; + } catch { + return { + ...legacy, + terminal: new Set([legacy.complete, legacy.archived]), + wake: new Set([legacy.hold, legacy.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]); @@ -1219,11 +1268,15 @@ export class Scheduler { schedulerLog.warn(`Failed to reset dispatch oscillation state for ${task.id} on unpause: ${error instanceof Error ? error.message : String(error)}`); }); } - const unpausedParked = resolveTaskParkedColumnsSync(this.store, task.id); - if (this.running && (task.column === unpausedParked.hold || task.column === unpausedParked.intake)) { - schedulerLog.log(`Task ${task.id} unpaused — triggering scheduling`); - this.schedule(); - } + /* FNXC:WorkflowResolvedColumns 2026-08-01-01:40 (fleet): the answer only gates `schedule()`, + which is async and fire-and-forget, so resolving it properly costs nothing observable. */ + void (async () => { + const unpausedParked = await resolveTaskParkedColumns(this.store, task.id); + if (this.running && unpausedParked.wake.has(task.column)) { + schedulerLog.log(`Task ${task.id} unpaused — triggering scheduling`); + void this.schedule(); + } + })(); } /* @@ -1246,17 +1299,22 @@ export class Scheduler { this.planningTaskIds.add(task.id); } else if (this.planningTaskIds.has(task.id)) { this.planningTaskIds.delete(task.id); - const planningParked = resolveTaskParkedColumnsSync(this.store, task.id); - if ( - this.running - && !task.status - && !task.paused - && !task.userPaused - && (task.column === planningParked.hold || task.column === planningParked.intake) - ) { - schedulerLog.log(`Task ${task.id} finished planning — triggering scheduling`); - this.schedule(); - } + /* FNXC:WorkflowResolvedColumns 2026-08-01-01:40 (fleet): as with the unpause wake above, the + answer only gates `schedule()`. The `planningTaskIds.delete` stays SYNCHRONOUS — it is the + edge-trigger bookkeeping, and deferring it would let a second update re-enter this branch. */ + void (async () => { + const planningParked = await resolveTaskParkedColumns(this.store, task.id); + if ( + this.running + && !task.status + && !task.paused + && !task.userPaused + && planningParked.wake.has(task.column) + ) { + schedulerLog.log(`Task ${task.id} finished planning — triggering scheduling`); + void this.schedule(); + } + })(); } if (!this.options.prMonitor) return; @@ -1299,7 +1357,7 @@ export class Scheduler { return; } - const deletedParked = resolveTaskParkedColumnsSync(this.store, task.id); + const deletedParked = await resolveTaskParkedColumns(this.store, task.id); /* FNXC:WorkflowLifecycleColumns 2026-08-01-05:00: A HALF-CONVERTED PAIR, one line apart. The hold read above already resolved its lane while diff --git a/scripts/lib/inert-sync-lane-baseline.json b/scripts/lib/inert-sync-lane-baseline.json index 066330a20c..3a8c705f04 100644 --- a/scripts/lib/inert-sync-lane-baseline.json +++ b/scripts/lib/inert-sync-lane-baseline.json @@ -1,8 +1,7 @@ { - "total": 29, + "total": 11, "byFile": { "packages/engine/src/executor.ts": 4, - "packages/engine/src/scheduler.ts": 18, "packages/engine/src/triage.ts": 7 } }