fix(scheduler): a board with TWO complete columns left dependents waiting forever (#3210)
## 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.
This commit is contained in:
7
.changeset/move-lanes-terminal-set.md
Normal file
7
.changeset/move-lanes-terminal-set.md
Normal file
@@ -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]`.
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
@@ -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 {
|
||||
|
||||
@@ -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<string, unknown>[] }).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)", () => {
|
||||
|
||||
@@ -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]),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user