fix(events): carry resolved lanes on task:moved so listener guards stop being inert (#3109)
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) <noreply@anthropic.com>
This commit is contained in:
7
.changeset/task-moved-carries-lanes.md
Normal file
7
.changeset/task-moved-carries-lanes.md
Normal file
@@ -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.
|
||||
@@ -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 {
|
||||
|
||||
@@ -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];
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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<string> },
|
||||
lanes: TaskMoveLanes | undefined,
|
||||
): { hold: string; intake: string; wip: string; review: string; complete: string; archived: string; terminal: ReadonlySet<string> } {
|
||||
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<string> } {
|
||||
const legacy = { hold: "todo", intake: "triage", wip: "in-progress", review: "in-review", complete: "done", archived: "archived" };
|
||||
const legacyTerminal: ReadonlySet<string> = 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}`);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user