From 41cdcc741ebb4e89bb07128d4887835309508c29 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 31 Jul 2026 04:47:09 -0700 Subject: [PATCH] fix(events): carry resolved lanes on task:moved so listener guards stop being inert (#3109) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes the **inert-guard class at its source** instead of one call site at a time. Independent of my other branches. ## The problem `task:moved` listeners run synchronously, so a listener needing a lane answer had to resolve one synchronously — and `resolveTaskWorkflowIrSync` returns the **default** workflow under PostgreSQL, the shipped backend. Every such guard behaved exactly as the literal it replaced, while the census scored it as converted. **Resolving asynchronously inside the listener is not available**, and that is measured rather than assumed. The scheduler's `snapshotManager.invalidate` is asserted to run in the listener's **synchronous prologue**; putting an await ahead of it produced **3 failures across 21 scheduler suites**. ## The fix The emitter carries the answer, which removes the dilemma rather than trading one horn for the other. `moves.ts` is already async and already post-commit, so it resolves the moving task's lanes **once** and hands them to every listener. The guard becomes correct **and** the prologue stays synchronous. This is the file's own recorded preferred fix — *"having the emitter carry the resolved lanes on the event payload so no listener resolves at all"* — now that the audit it was waiting on is done and came back as **one** prologue-dependent consumer, not a class. ## Design choices - **`lanes` is optional and fail-soft to `undefined`** — "unknown", never "legacy". Some emit paths fire from sync contexts or a cached row mid-teardown. Listeners keep their existing fallback, so those paths are no better than before but **no worse**, and they become the exception rather than the rule. - **`mergeParkedColumns` overlays only fields the emitter actually resolved**, so a partial payload cannot blank a lane back to a wrong answer. - **The sync resolver stays** as that fallback. Deleting it would strand the emit paths that cannot resolve. ## Verification - **Revert-proof and it pins the prologue:** the new case asserts invalidation on a **renamed** hold lane with **no `waitFor`**. Ignoring the payload gives **0 calls**. - 21 scheduler suites — **361 green** - self-healing + notification suites — **491 green** - core moves + the `sync-workflow-ir-callsite-allowlist` ratchet — green - **`pnpm test:gate` green** (71) - Changeset added; `check:changesets` passes ## What it unblocks `scheduler.ts`'s 10 allow-listed guards now resolve correctly for every move that goes through `moves.ts` — the path real moves take. Those were already absent from the backlog, so **the census number does not move**; what changes is that they now do what the number claimed. `executor.ts`'s 4 remaining sites can follow the same pattern in a separate PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) --- .changeset/task-moved-carries-lanes.md | 7 ++++ packages/core/src/index.ts | 4 +- packages/core/src/store.ts | 21 +++++++++- packages/core/src/task-store/moves.ts | 16 ++++++- .../core/src/workflow-lifecycle-traits.ts | 22 ++++++++++ .../scheduler-auto-claim-invalidation.test.ts | 27 ++++++++++++ packages/engine/src/scheduler.ts | 42 ++++++++++++++++++- 7 files changed, 132 insertions(+), 7 deletions(-) create mode 100644 .changeset/task-moved-carries-lanes.md diff --git a/.changeset/task-moved-carries-lanes.md b/.changeset/task-moved-carries-lanes.md new file mode 100644 index 0000000000..d34fbcd30b --- /dev/null +++ b/.changeset/task-moved-carries-lanes.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Board renames no longer silently disable scheduler auto-claim invalidation and lane guards. +category: fix +dev: `task:moved` payloads now carry emitter-resolved `lanes` (`TaskMoveLanes`); listeners prefer them over the sync IR resolver, which returns the default workflow under PostgreSQL. diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 292917fed4..86a0d39abd 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -467,8 +467,8 @@ export { createWorkflowEventBus, getWorkflowEventBus, emitWorkflowLifecycleEvent export type { WorkflowEventBus, WorkflowEventSubscriber, WorkflowEventSubscription } from "./workflow-events.js"; export { findWorkflowEventShapeViolations, isIdsOnlyWorkflowEvent, MAX_ID_VALUE_LENGTH, IMPLEMENTATION_EXITS } from "./types/workflow-events.js"; export type { WorkflowLifecycleEvent, WorkflowLifecycleEventType, WorkflowLifecycleEventBase, TaskTransitionedEvent, NodeEnteredEvent, NodeCompletedEvent, RunSuspendedEvent, RunResumedEvent, WorkflowEventShapeViolation, ImplementationExit } from "./types/workflow-events.js"; -export { columnHasFlag, columnsWithFlag, declaresAnyLifecycleTrait, resolveArchiveTargetForTask, resolveCompleteColumn, resolveLifecycleColumns, resolveMergeOrchestrationColumn, resolveReboundTarget, resolveReboundTargetForTask, resolveReviewColumns, resolveTaskLifecycleColumns, resolveTerminalColumns, resolveWipTargetForTask } from "./workflow-lifecycle-traits.js"; -export type { LifecycleColumns } from "./workflow-lifecycle-traits.js"; +export { columnHasFlag, columnsWithFlag, declaresAnyLifecycleTrait, resolveArchiveTargetForTask, resolveCompleteColumn, resolveLifecycleColumns, resolveMergeOrchestrationColumn, resolveReboundTarget, resolveReboundTargetForTask, resolveReviewColumns, resolveTaskLifecycleColumns, resolveTerminalColumns, resolveWipTargetForTask, toTaskMoveLanes } from "./workflow-lifecycle-traits.js"; +export type { LifecycleColumns, TaskMoveLanes } from "./workflow-lifecycle-traits.js"; export { resolveProjectColumnsForRoles, resolveArchivedLanes, REVIEW_ROLES, TERMINAL_ROLES, LEGACY_COLUMN_IDS_BY_ROLE, type ProjectLaneVocabularyStore, type ProjectLaneResolutionOptions } from "./project-lane-vocabulary.js"; export { resolveReviewLevelSteps, applyReviewLevelPreset } from "./review-level-preset.js"; export { diff --git a/packages/core/src/store.ts b/packages/core/src/store.ts index e48c0b16d8..95664c7a78 100644 --- a/packages/core/src/store.ts +++ b/packages/core/src/store.ts @@ -1,4 +1,5 @@ import { EventEmitter } from "node:events"; +import type { TaskMoveLanes } from "./workflow-lifecycle-traits.js"; import { randomUUID } from "node:crypto"; import { join } from "node:path"; import { and, eq, isNull, ne, sql } from "drizzle-orm"; @@ -141,7 +142,25 @@ import type { BranchGroupRow, PrEntityRow, TaskDocumentRow, ArtifactRow, TaskDoc export interface TaskStoreEvents { "task:created": [task: Task]; - "task:moved": [data: { task: Task; from: ColumnId; to: ColumnId; source: "user" | "engine" | "scheduler" }]; + /* + FNXC:WorkflowEvents 2026-07-31-21:00 (fleet — the emitter carries the lanes): + `lanes` is the moving task's RESOLVED lifecycle columns, attached by the emitter. + + Listeners on this event run synchronously, so any that needed a lane answer had to resolve one + synchronously too — and the only sync resolver (`resolveTaskWorkflowIrSync`) returns the DEFAULT + workflow in production, making every such guard inert. Resolving asynchronously in the listener is + not available either: the scheduler's snapshot invalidation is asserted to run in the listener's + synchronous prologue. + + Carrying the answer on the payload removes the dilemma rather than trading one horn for the other: + the emitter is already async and already resolves this task's IR, and the listener gets a correct + lane set with no await at all. + + OPTIONAL because not every emit path can resolve (some fire from sync contexts or from a cached row + mid-teardown). A listener must therefore keep its existing fallback; absent `lanes` is "unknown", + never "legacy". + */ + "task:moved": [data: { task: Task; from: ColumnId; to: ColumnId; source: "user" | "engine" | "scheduler"; lanes?: TaskMoveLanes }]; "task:updated": [task: Task]; "task:deleted": [task: Task, meta?: { githubIssueAction?: GithubIssueAction }]; "task:merged": [result: MergeResult]; diff --git a/packages/core/src/task-store/moves.ts b/packages/core/src/task-store/moves.ts index 8a33d57a96..c3596b82b3 100644 --- a/packages/core/src/task-store/moves.ts +++ b/packages/core/src/task-store/moves.ts @@ -26,7 +26,7 @@ import { evaluateTransitionInvariants, } from "../workflow-transition-policy.js"; import {type DefaultWorkflowMoveContext, applyDefaultWorkflowMoveEffects, isReopenIntoPlanning} from "../default-workflow-hooks.js"; -import {columnsWithFlag, resolveLifecycleColumns, resolveReviewColumns} from "../workflow-lifecycle-traits.js"; +import {columnsWithFlag, resolveLifecycleColumns, resolveReviewColumns, toTaskMoveLanes} from "../workflow-lifecycle-traits.js"; import {resolveWorkflowIrForTask} from "../workflow-ir-resolver.js"; import {makeTransitionRejection, makeTransitionPending} from "../transition-types.js"; import {writeTransitionPendingAsync, clearTransitionPendingAsync} from "./async-transition-pending.js"; @@ -1435,7 +1435,19 @@ export async function moveTaskInternalImpl(store: TaskStore, id: string, toColum } if (fromColumn !== toColumn) { - store.emit("task:moved", { task, from: fromColumn, to: toColumn, source: moveSource }); + /* + FNXC:WorkflowEvents 2026-07-31-21:00 (fleet): + Resolve the moving task's lanes HERE, once, and hand them to every listener. This is the emit + path real moves take, and it is already async and already post-commit, so the resolution costs + one IR read on a transition that has just done database work. + + Listeners could not do this for themselves: they run synchronously, the only sync resolver + answers with the DEFAULT workflow in production, and awaiting inside the listener breaks the + scheduler's synchronous snapshot invalidation. Fail-soft to undefined — "unknown", never + "legacy" — so a listener keeps its own fallback rather than being handed a wrong answer. + */ + const lanes = toTaskMoveLanes(await resolveWorkflowIrForTask(store, task.id).catch(() => undefined)); + store.emit("task:moved", { task, from: fromColumn, to: toColumn, source: moveSource, lanes }); /* FNXC:WorkflowEvents 2026-07-27-11:45 (U3 / R5, R6): THE post-commit emit point for lifecycle transitions. Its position is the diff --git a/packages/core/src/workflow-lifecycle-traits.ts b/packages/core/src/workflow-lifecycle-traits.ts index 369c9c4616..0f3cdfe27a 100644 --- a/packages/core/src/workflow-lifecycle-traits.ts +++ b/packages/core/src/workflow-lifecycle-traits.ts @@ -237,6 +237,28 @@ export function resolveTerminalColumns(ir: WorkflowIr): readonly [string, string * its first column. Returns undefined only for a column-less (v1) IR, where the * caller keeps the legacy literal fallback. For builtin:coding this is `todo`. */ +/* +FNXC:WorkflowEvents 2026-07-31-21:00 (fleet): +The resolved lane answer carried on a `task:moved` payload. A plain data shape, not a resolver: the +whole point is that a listener does not resolve anything. +*/ +export interface TaskMoveLanes { + readonly hold?: string; + readonly intake?: string; + readonly wip?: string; + readonly review?: string; + readonly complete?: string; + readonly archived?: string; +} + +/** Resolve the `task:moved` lane payload for a task's own workflow. Returns undefined if unresolvable. */ +export function toTaskMoveLanes(ir: WorkflowIr | undefined): TaskMoveLanes | undefined { + 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 }; +} + export function resolveReboundTarget(ir: WorkflowIr): string | undefined { const columns = columnsOf(ir); if (columns.length === 0) return undefined; 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 c8054954dd..6acdd176f8 100644 --- a/packages/engine/src/__tests__/scheduler-auto-claim-invalidation.test.ts +++ b/packages/engine/src/__tests__/scheduler-auto-claim-invalidation.test.ts @@ -174,6 +174,33 @@ describe("Scheduler auto-claim snapshot invalidation", () => { expect(internals.wasNodeBlocked.has("FN-1")).toBe(false); }); + /* + FNXC:WorkflowResolvedColumns 2026-07-31-21:00 (fleet): + The lane guard in this listener used to resolve through `resolveTaskWorkflowIrSync`, which returns the + DEFAULT workflow in production — so on a renamed board it matched nothing and the auto-claim snapshot + was never invalidated, silently serving stale claim data. + + It could not simply become async: the assertion below pins `invalidate` to the listener's SYNCHRONOUS + prologue, and an await ahead of it fails that test. The emitter now resolves the lanes and carries them + on the payload, so the guard is correct AND the prologue stays synchronous — which is exactly what this + case checks, by using a hold lane that matches no legacy id. + */ + it("invalidates on a RENAMED hold lane using the lanes the emitter resolved", () => { + const invalidate = vi.fn(); + const { store, emit } = createStore(); + new Scheduler(store, { snapshotManager: { invalidate } as any }); + + emit("task:moved", { + task: createTask({ id: "FN-9", column: "building" }), + from: "backlog", + to: "building", + lanes: { hold: "backlog", wip: "building" }, + }); + + // Synchronous on purpose: no waitFor. The prologue must still run before emit returns. + expect(invalidate).toHaveBeenCalledWith("task:moved:backlog->building"); + }); + it("invalidates task:moved only when todo is source or destination", () => { const invalidate = vi.fn(); const { store, emit } = createStore(); diff --git a/packages/engine/src/scheduler.ts b/packages/engine/src/scheduler.ts index ec823f3570..d0c9860914 100644 --- a/packages/engine/src/scheduler.ts +++ b/packages/engine/src/scheduler.ts @@ -40,6 +40,7 @@ import { StaleTaskReporter } from "./stale-task-reporter.js"; import { BacklogPressureReporter } from "./backlog-pressure-reporter.js"; import { UnlinkedMissionsAdvisoryReporter } from "./unlinked-missions-advisory-reporter.js"; import { createRunAuditor, generateSyntheticRunId } from "./run-audit.js"; +import type { TaskMoveLanes } from "@fusion/core"; import { resolveProjectColumnsForRoles, resolveWorkflowIrForTask, resolveWorkflowIrById, resolveColumnFlags, resolveWorktreeCapacityLimit, resolveLifecycleColumns, isWipColumnRole, isReviewColumnRole, isCompleteColumnRole, columnsWithFlag } from "@fusion/core"; import type { ColumnRoleTraitFlags } from "@fusion/core"; import type { WorkflowIr, WorkflowIrV2 } from "@fusion/core"; @@ -425,6 +426,29 @@ and cannot wrongly withhold work. (Seeding a REFUSAL is the bug — see `node-ov Same sync IR path and same fail-soft legacy default as the single-column answers, so event ordering and unresolvable-workflow behaviour are unchanged. */ +/* +FNXC:WorkflowResolvedColumns 2026-07-31-21:00 (fleet): +Overlay emitter-resolved lanes onto the fail-soft defaults. Only fields the emitter actually resolved +are taken, so a partial payload cannot blank a lane back to a wrong answer. +*/ +function mergeParkedColumns( + base: { hold: string; intake: string; wip: string; review: string; complete: string; archived: string; terminal: ReadonlySet }, + lanes: TaskMoveLanes | undefined, +): { hold: string; intake: string; wip: string; review: string; complete: string; archived: string; terminal: ReadonlySet } { + if (!lanes) return base; + const complete = lanes.complete ?? base.complete; + const archived = lanes.archived ?? base.archived; + return { + hold: lanes.hold ?? base.hold, + intake: lanes.intake ?? base.intake, + wip: lanes.wip ?? base.wip, + review: lanes.review ?? base.review, + complete, + archived, + terminal: new Set([complete, archived]), + }; +} + 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]); @@ -960,9 +984,23 @@ export class Scheduler { 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 }) => { + this.store.on("task:moved", async ({ task, from, to, source, lanes }) => { this.lastAutoClaimFingerprint.set(task.id, computeAutoClaimFingerprint(task)); - const parked = resolveTaskParkedColumnsSync(this.store, task.id); + /* + 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. + + 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. + */ + const parked = mergeParkedColumns(resolveTaskParkedColumnsSync(this.store, task.id), lanes); if (from === parked.hold || to === parked.hold) { this.options.snapshotManager?.invalidate(`task:moved:${from}->${to}`); }