From 8661b739ffb4f5ddd67c4b8f0c84d1037561ef7b Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 31 Jul 2026 10:47:15 -0700 Subject: [PATCH] fix(scheduler): a board with TWO complete columns left dependents waiting forever (#3210) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## The defect On a board declaring more than one complete-trait column — a merged lane and a shipped lane, say — a card landing in the **second** one was never recognised as finished, so nothing unblocked its dependents. Silent: no error, the dependent just waits. Two problems, the same shape: 1. **`TaskMoveLanes` carried one id per role.** That is right for *"where should this card go"* and wrong for *"is this column one of the finished lanes"*, which is a **membership** question. The payload could not express such a board at all. 2. **`mergeParkedColumns` rebuilt `terminal` as `new Set([complete, archived])`** — discarding `base.terminal`, which the sync IR path had already resolved correctly, and narrowing a membership set back to first-match-per-role. Point 2 contradicted the note sitting directly above it in the same file: > `terminal` is a MEMBERSHIP set, and it is not the same question as `complete`/`archived`. […] A workflow may declare more than one complete-trait column […] and `to === parked.complete` sees only the first and silently skips the rest. The reasoning was already written down. The overlay added later didn't honour it. ## Fix `TaskMoveLanes.terminal?: readonly string[]`, filled from `columnsWithFlag(ir, "complete"|"archived")` rather than the first-match `resolveLifecycleColumns`, and the merge is now a **union** of base, payload, and the single lanes. Optional, so all 12 emitters and every listener keep compiling — a listener that ignores it is exactly as correct as before. Union is the direction `scheduler.ts` already argues for at line ~422: a superset costs one extra query; a subset **silently withholds work from a finished card**. `complete` deliberately stays first-match — a set would be the wrong shape for a move *target*. Both questions now coexist rather than one replacing the other. ## How it was found, and what it corrects Supplying `task:moved` lanes fixed every *other* renamed-board case in `scheduler-renamed-hold-events` — measured **10 passed / 1 failed** — and left exactly this one broken. That same measurement is why I narrowed my earlier claim on #3082: the other behaviours were never broken in production, because all 12 emitters already carry lanes. This is the residue that was genuinely broken. ## Tests — both with anti-vacuity controls | control | result | |---|---| | revert `toTaskMoveLanes` | **2 of 4** core tests fail (the terminal pair) | | revert the scheduler union | the new engine test fails, **and only it** (1 failed / 11 passed) | | both restored | 4 passed, 12 passed | The 2 core tests that pass either way are shape invariants asserted on purpose (`complete` must stay first-match; a column-less IR returns `undefined` rather than an invented lane) — flagging that so the control isn't read as 4-of-4. The pre-existing scheduler case emits **without** lanes, which no production emitter does, so it exercises the sync fallback. The new one emits `toTaskMoveLanes(ir)` — the shape that actually ships. ## Measured | check | result | |---|---| | `@fusion/core` / `@fusion/engine` tsc | exit 0 / exit 0 | | eslint | clean | | `census --strict`, `check:fnxc-future-dates`, `check:changesets` | exit 0 | | every `TaskMoveLanes` consumer | 24 passed | | `pnpm test:gate` | **exit 0 — 744 tests, up 12** | ## Census No guard converted; this is a payload-shape fix. Backlog unchanged at 11, all deferred. --- .changeset/move-lanes-terminal-set.md | 7 +++ .../task-move-lanes-terminal-set.test.ts | 50 +++++++++++++++++++ .../core/src/workflow-lifecycle-traits.ts | 23 ++++++++- .../scheduler-renamed-hold-events.test.ts | 29 +++++++++++ packages/engine/src/scheduler.ts | 14 +++++- 5 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 .changeset/move-lanes-terminal-set.md create mode 100644 packages/core/src/__tests__/task-move-lanes-terminal-set.test.ts diff --git a/.changeset/move-lanes-terminal-set.md b/.changeset/move-lanes-terminal-set.md new file mode 100644 index 0000000000..2f9084384f --- /dev/null +++ b/.changeset/move-lanes-terminal-set.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Cards on boards with two "complete" columns now unblock their dependents correctly. +category: fix +dev: `TaskMoveLanes` gains an optional `terminal?: readonly string[]` carrying every complete/archived-trait column id; `toTaskMoveLanes` fills it from `columnsWithFlag` rather than the first-match `resolveLifecycleColumns`. The scheduler's `mergeParkedColumns` now unions `base.terminal`, the payload's set, and the single lanes instead of rebuilding from `[complete, archived]`. diff --git a/packages/core/src/__tests__/task-move-lanes-terminal-set.test.ts b/packages/core/src/__tests__/task-move-lanes-terminal-set.test.ts new file mode 100644 index 0000000000..9954824a09 --- /dev/null +++ b/packages/core/src/__tests__/task-move-lanes-terminal-set.test.ts @@ -0,0 +1,50 @@ +/* +FNXC:WorkflowResolvedColumns 2026-07-31-11:15 (u12 — the payload could not express a two-complete board): +Pins the ONE renamed-board behaviour that supplying `task:moved` lanes did NOT fix when measured +(10 passed / 1 failed): a card landing in a SECOND complete-trait column left its dependents waiting +forever, because `TaskMoveLanes` carried one id per role and the scheduler rebuilt `terminal` from it. +*/ +import { describe, expect, it } from "vitest"; + +import { toTaskMoveLanes } from "../workflow-lifecycle-traits.js"; +import type { WorkflowIr } from "../workflow-ir.js"; + +const twoCompleteBoard = { + version: "v2", id: "wf", + columns: [ + { id: "inbox", name: "inbox", traits: [{ trait: "intake" }] }, + { id: "building", name: "building", traits: [{ trait: "wip" }] }, + { id: "done", name: "done", traits: [{ trait: "complete" }] }, + { id: "released", name: "released", traits: [{ trait: "complete" }] }, + { id: "archived", name: "archived", traits: [{ trait: "archived" }] }, + ], + nodes: [], edges: [], +} as unknown as WorkflowIr; + +describe("the task:moved payload carries the terminal SET, not just the first complete lane", () => { + it("includes BOTH complete-trait columns", () => { + const lanes = toTaskMoveLanes(twoCompleteBoard); + + expect(lanes?.terminal).toContain("done"); + // The half that was impossible before: a second complete lane in the payload at all. + expect(lanes?.terminal).toContain("released"); + }); + + it("still reports a single first-match `complete` for move TARGETS", () => { + // `complete` answers "where does a finished card go" and must stay first-match — a set would be + // the wrong shape there. Both questions coexist; that is the point of the new field. + const lanes = toTaskMoveLanes(twoCompleteBoard); + + expect(lanes?.complete).toBe("done"); + }); + + it("includes the archived lane in terminal", () => { + expect(toTaskMoveLanes(twoCompleteBoard)?.terminal).toContain("archived"); + }); + + it("omits terminal for an IR with no lifecycle columns rather than inventing one", () => { + const bare = { version: "v2", id: "wf", columns: [], nodes: [], edges: [] } as unknown as WorkflowIr; + + expect(toTaskMoveLanes(bare)).toBeUndefined(); + }); +}); diff --git a/packages/core/src/workflow-lifecycle-traits.ts b/packages/core/src/workflow-lifecycle-traits.ts index 6cc9abf2d5..efe251e545 100644 --- a/packages/core/src/workflow-lifecycle-traits.ts +++ b/packages/core/src/workflow-lifecycle-traits.ts @@ -249,6 +249,21 @@ export interface TaskMoveLanes { readonly review?: string; readonly complete?: string; readonly archived?: string; + /* + FNXC:WorkflowResolvedColumns 2026-07-31-11:05 (u12 — a role SET cannot be carried by a role ID): + Every other field answers "which column IS this role", which is first-match-per-role. `terminal` + answers a different question — "is this column ONE OF the finished lanes" — and a workflow may + declare more than one complete-trait column (a merged lane and a shipped lane). + + Without this the payload could not express that board at all: `scheduler.ts` rebuilt its terminal + set as `new Set([complete, archived])`, so a card landing in a SECOND complete column was not seen + as finished and its dependents were never unblocked. That is a card that waits forever, and it is + the one behaviour that supplying lanes did NOT fix when measured (10 passed / 1 failed). + + Optional, so every existing emitter and listener keeps compiling; a listener that ignores it is + exactly as correct as it was before. + */ + readonly terminal?: readonly string[]; } /** Resolve the `task:moved` lane payload for a task's own workflow. Returns undefined if unresolvable. */ @@ -256,7 +271,13 @@ export function toTaskMoveLanes(ir: WorkflowIr | undefined): TaskMoveLanes | und if (!ir) return undefined; const l = resolveLifecycleColumns(ir); if (!l) return undefined; - return { hold: l.hold, intake: l.intake, wip: l.wip, review: l.review, complete: l.complete, archived: l.archived }; + /* Read from the trait flags, NOT from `l`: `resolveLifecycleColumns` is first-match-per-role and + would collapse a second complete-trait column, which is the whole defect this field exists for. */ + const terminal = [...new Set([...columnsWithFlag(ir, "complete"), ...columnsWithFlag(ir, "archived")])]; + return { + hold: l.hold, intake: l.intake, wip: l.wip, review: l.review, complete: l.complete, archived: l.archived, + ...(terminal.length > 0 ? { terminal } : {}), + }; } export function resolveReboundTarget(ir: WorkflowIr): string | undefined { 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 5d47a40140..880f5003d2 100644 --- a/packages/engine/src/__tests__/scheduler-renamed-hold-events.test.ts +++ b/packages/engine/src/__tests__/scheduler-renamed-hold-events.test.ts @@ -30,6 +30,7 @@ Written against the literal implementation and observed FAILING first. */ import { describe, expect, it, vi } from "vitest"; import type { TaskStore, WorkflowIr } from "@fusion/core"; +import { toTaskMoveLanes } from "@fusion/core"; import { Scheduler } from "../scheduler.js"; import { evaluateParkedAgentTaskLink } from "../task-agent-sync.js"; import { flushAsyncHandlers } from "./_flush-async-handlers.js"; @@ -226,6 +227,34 @@ describe("scheduler event handlers under a renamed hold column", () => { const queried = listTasks.mock.calls.map((c) => (c[0] as { column?: string } | undefined)?.column); expect(queried).toContain("drafting"); }); + + /* + FNXC:WorkflowResolvedColumns 2026-07-31-11:25 (u12 — the PRODUCTION emit shape): + The case above emits WITHOUT lanes, which no production emitter does — all 12 call + `toTaskMoveLanes` — so it exercises the sync-resolver fallback rather than the shipped path. + Measured on the shipped path, supplying lanes fixed every other renamed-board case in this file + (10 passed / 1 failed) and left exactly this one broken, because `TaskMoveLanes` carried one id + per role and the scheduler rebuilt `terminal` from it. This asserts the fix at the seam that + actually runs: a card landing in a SECOND complete-trait column unblocks its dependents. + */ + it("treats a second complete-trait column as terminal when the emitter supplies lanes", async () => { + const twoCompleteLanes = renamedIr(); + (twoCompleteLanes as unknown as { columns: Record[] }).columns.push({ + id: "released", name: "released", traits: [{ trait: "complete" }], + }); + + const dependent = task({ id: "FN-DEP", column: "drafting", dependencies: ["FN-BLOCK"], blockedBy: "FN-BLOCK" }); + const blocker = task({ id: "FN-BLOCK", column: "released" }); + const { emit, listTasks } = createScheduler([dependent, blocker], {}, twoCompleteLanes); + + await emit("task:moved", { + task: blocker, from: "building", to: "released", source: "engine", + lanes: toTaskMoveLanes(twoCompleteLanes), + }); + + const queried = listTasks.mock.calls.map((c) => (c[0] as { column?: string } | undefined)?.column); + expect(queried).toContain("drafting"); + }); }); describe("agent link (wrong here DROPS a live agent's task link)", () => { diff --git a/packages/engine/src/scheduler.ts b/packages/engine/src/scheduler.ts index 61c410f500..cb2859dc27 100644 --- a/packages/engine/src/scheduler.ts +++ b/packages/engine/src/scheduler.ts @@ -445,7 +445,19 @@ function mergeParkedColumns( review: lanes.review ?? base.review, complete, archived, - terminal: new Set([complete, archived]), + /* + FNXC:WorkflowResolvedColumns 2026-07-31-11:10 (u12 — the overlay NARROWED a membership set): + This rebuilt `terminal` as `new Set([complete, archived])`, which is first-match-per-role and so + contradicted the note above ("`terminal` is a MEMBERSHIP set, and it is not the same question as + `complete`/`archived`"). Two losses in one line: it DISCARDED `base.terminal`, which the sync IR + path had already resolved correctly, and it had no way to express a second complete-trait column + even when the emitter knew about one. + + Now a UNION of everything either side proved terminal. That is the direction this file already + argues for at line ~422: a superset makes the reconciliation run on a move it would otherwise + ignore — one extra query — while a subset silently withholds work from a card that is finished. + */ + terminal: new Set([...base.terminal, ...(lanes.terminal ?? []), complete, archived]), }; }