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 876e450c2b..e50ff81440 100644 --- a/packages/engine/src/__tests__/scheduler-renamed-hold-events.test.ts +++ b/packages/engine/src/__tests__/scheduler-renamed-hold-events.test.ts @@ -51,7 +51,7 @@ function renamedIr(): WorkflowIr { } as unknown as WorkflowIr; } -function createStore(tasks: Record[] = []) { +function createStore(tasks: Record[] = [], ir: WorkflowIr = renamedIr()) { const listeners = new Map void)[]>(); const selection = { workflowId: WF, stepIds: [] }; const listTasks = vi.fn(async (opts?: { column?: string }) => @@ -73,8 +73,8 @@ function createStore(tasks: Record[] = []) { getCompletionHandoffAcceptedMarker: vi.fn().mockResolvedValue(null), getTaskWorkflowSelection: vi.fn(() => selection), getTaskWorkflowSelectionAsync: vi.fn(async () => selection), - getWorkflowDefinition: vi.fn(async () => ({ ir: renamedIr() })), - resolveTaskWorkflowIrSync: vi.fn(() => renamedIr()), + getWorkflowDefinition: vi.fn(async () => ({ ir })), + resolveTaskWorkflowIrSync: vi.fn(() => ir), } as unknown as TaskStore; return { @@ -130,8 +130,9 @@ function createAgentStore(agents: Record[], freshRun: unknown = function createScheduler( tasks: Record[] = [], options: Record = {}, + ir: WorkflowIr = renamedIr(), ) { - const { store, emit, listTasks } = createStore(tasks); + const { store, emit, listTasks } = createStore(tasks, ir); const scheduler = new Scheduler(store, options as never); const schedule = vi.spyOn(scheduler, "schedule").mockResolvedValue(undefined); (scheduler as unknown as { running: boolean }).running = true; @@ -193,6 +194,32 @@ describe("scheduler event handlers under a renamed hold column", () => { expect(queried).not.toContain("todo"); expect(queried).toContain("drafting"); }); + + /* + FNXC:WorkflowResolvedColumns 2026-07-31-12:40: + THE SECOND COMPLETE LANE. The guard above used to ask `to === parked.complete`, and + `resolveLifecycleColumns` answers FIRST MATCH PER ROLE — so on a board that declares two + complete-trait columns, a blocker finishing in the second one reconciled nothing and its + dependents waited forever on a blocker that was already done. + + That arity difference is invisible on a single-lane board, which is why the sibling case above + passes either way and this one is needed to hold the membership shape in place. + */ + it("treats a SECOND complete-trait column as terminal, not just the first", 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" }); + + 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 5065f614fb..b767593dd4 100644 --- a/packages/engine/src/scheduler.ts +++ b/packages/engine/src/scheduler.ts @@ -409,13 +409,39 @@ terminal cleanup never ran. None of those error; they simply stop happening. One event, reusing the SAME sync path and the SAME fail-soft legacy defaults, so event ordering and unresolvable-workflow behaviour are both unchanged. */ -function resolveTaskParkedColumnsSync(store: TaskStore, taskId: string): { hold: string; intake: string; wip: string; review: string; complete: string; archived: string } { +/* +FNXC:WorkflowResolvedColumns 2026-07-31-12:30: +`terminal` is a MEMBERSHIP set, and it is not the same question as `complete`/`archived`. + +`resolveLifecycleColumns` answers FIRST MATCH PER ROLE — the right shape for "where should this card +be moved to", the wrong shape for "did this card just reach a finished lane". A workflow may declare +more than one complete-trait column (a merged lane and a shipped lane, say); `to === parked.complete` +sees only the first and silently skips the rest. + +Seeded with the legacy ids. For an INCLUSION that is safe in the direction that matters: a superset +makes the reconciliation below run on a move it would otherwise ignore, which costs one extra query +and cannot wrongly withhold work. (Seeding a REFUSAL is the bug — see `node-override-guard.ts`.) + +Same sync IR path and same fail-soft legacy default as the single-column answers, so event ordering +and unresolvable-workflow behaviour are unchanged. +*/ +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 l = resolveLifecycleColumns(store.resolveTaskWorkflowIrSync(taskId)); - 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 }; + 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; + return { ...legacy, terminal: legacyTerminal }; } } @@ -989,7 +1015,7 @@ export class Scheduler { // 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 (to === parked.complete || to === parked.archived) { + if (parked.terminal.has(to)) { try { const settings = await this.store.getSettings(); if (!settings.globalPause && !settings.enginePaused) { @@ -1068,7 +1094,7 @@ export class Scheduler { } else { this.recentEngineTodoRequeues.delete(task.id); } - } else if (to === parked.review || to === parked.complete || to === parked.archived) { + } else if (to === parked.review || parked.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, { @@ -1085,7 +1111,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 (to === parked.complete || to === parked.hold) { + if (parked.terminal.has(to) || to === parked.hold) { schedulerLog.log(`Task moved to ${to} — triggering scheduling`); this.schedule(); }