From f8155cafd7cf0c51c3d5326059a12f402ef0c989 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 31 Jul 2026 01:08:54 -0700 Subject: [PATCH] fix(cli): the node-override guard saw only the FIRST wip lane (#3023) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #3019, which merged with an incomplete fix. I found this while sitting down to write the test that PR was missing. ## The guard still never fired, one lane over #3019 wired `fn_task_update`'s guard like this: ```ts const nodeOverrideLifecycle = await resolveTaskLifecycleColumns(store, task.id); wipColumns: nodeOverrideLifecycle?.wip ? new Set([nodeOverrideLifecycle.wip]) : undefined, ``` `resolveTaskLifecycleColumns` → `resolveLifecycleColumns`, whose per-role accessor is **first match** (`workflow-lifecycle-traits.ts:353`): ```ts const first = (flag) => resolved.find((c) => c.flags[flag] === true)?.id; ``` The guard's contract is **every** column carrying the trait — its own resolver uses `columnsWithFlag(ir, "countsTowardWip")`. So on a board with a build lane beside a verify lane, a task sitting in the **second** wip lane still slipped the mid-flight check, and an operator could still repoint the node of a running task. That is the defect #3019 set out to close. Interchangeable on any single-wip-lane board, which is exactly why it read as correct — the same arity trap #2975 removed from the surfacing family. ## The fix Use `resolveNodeOverrideLanes`, the guard's own resolver, which `task-update.ts` and `branch-and-pr-entities.ts` already call. All three callers now resolve identically and the V1/unresolvable fallback lives in one place. Needed a one-line re-export from `@fusion/core`. **Mutation:** forcing the resolver to first-match (`.slice(0, 1)`) fails the new case, 1 of 32. The new test names **two** wip lanes, because that is the only shape that separates the two resolutions — a single-wip-lane test passes against both, which is why #3019's gap was invisible and why I would have written a useless test if I had not read the implementation first. ## A gate constraint worth recording My first version passed the resolved object straight through: ```ts validateNodeOverrideChange(task, normalizedNodeId ?? null, overrideLanes) ``` Identical at runtime, and it turned the lane-wiring gate **red**: `check-lane-wiring` matches an object-literal argument and cannot see through a variable, so the correct call reads as UNWIRED. #3019's header records hitting the same constraint — and it is what pushed that PR toward resolving the lanes inline, which is where the first-match bug entered. So the gate's shape requirement steered a correct instinct into a subtly wrong implementation. The fix here spells both keys explicitly, satisfying the gate without the bespoke resolution. Worth someone deciding whether the census should follow a variable to its initializer — but that is a change to a shared ratchet, and I have noted it at the call site rather than making it. **Verified:** 32/32 core guard suite, `tsc` 0 errors for both packages, lane-wiring gate exit 0, FNXC gate exit 0, lint clean. --------- Co-authored-by: Claude Opus 5 (1M context) --- .changeset/node-override-every-wip-lane.md | 7 ++++ packages/cli/src/extension.ts | 32 +++++++++++++++++-- .../src/__tests__/node-override-guard.test.ts | 29 +++++++++++++++++ packages/core/src/index.ts | 1 + 4 files changed, 66 insertions(+), 3 deletions(-) create mode 100644 .changeset/node-override-every-wip-lane.md diff --git a/.changeset/node-override-every-wip-lane.md b/.changeset/node-override-every-wip-lane.md new file mode 100644 index 0000000000..5876f877cc --- /dev/null +++ b/.changeset/node-override-every-wip-lane.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: A running task in a board's second WIP lane can no longer have its node override changed mid-flight. +category: fix +dev: `fn_task_update` resolved lanes with `resolveTaskLifecycleColumns` (first match per role); switched to the guard's own `resolveNodeOverrideLanes` (`columnsWithFlag`, every match), now re-exported from `@fusion/core`. diff --git a/packages/cli/src/extension.ts b/packages/cli/src/extension.ts index 0d30428da0..356ea895f8 100644 --- a/packages/cli/src/extension.ts +++ b/packages/cli/src/extension.ts @@ -42,6 +42,7 @@ import { type SecretScope, declaresAnyLifecycleTrait, resolveTaskLifecycleColumns, + resolveNodeOverrideLanes, resolveLifecycleColumns, resolveWorkflowIrForTaskWithProvenance, resolveWorkflowIrForTask, @@ -1838,10 +1839,35 @@ export default function kbExtension(pi: ExtensionAPI) { scripts/lib/lane-wiring-census.mjs, which matches an object-literal argument and cannot see a ternary. This site was a known-unwired entry in that gate's baseline. */ - const nodeOverrideLifecycle = await resolveTaskLifecycleColumns(store, task.id); + /* + FNXC:WorkflowResolvedColumns 2026-07-31-01:30: + EVERY wip/complete lane, not the FIRST — and via the guard's own resolver, like its two other callers. + + #3019 wired this call with `resolveTaskLifecycleColumns(...).wip`, whose per-role accessor is + `resolved.find(...)` (workflow-lifecycle-traits.ts:353) — the FIRST column carrying the trait. + The guard's contract is every column: `resolveNodeOverrideLanes` builds its sets from + `columnsWithFlag(ir, "countsTowardWip")`. On a board with a build lane beside a verify lane + the two answers differ, and a task sitting in the SECOND wip lane slipped the mid-flight + check — the exact defect #3019 set out to close, still open one lane over. + + Interchangeable on any single-wip-lane board, which is why it read as correct. Same arity trap + #2975 removed from the surfacing family. + + `resolveNodeOverrideLanes` is what `task-update.ts` and `branch-and-pr-entities.ts` already + call, so all three callers now resolve identically and the fallback lives in one place. + */ + const overrideLanes = await resolveNodeOverrideLanes(store, task.id); + /* + Spelled as an object literal naming both keys, not `…, overrideLanes)`. The two are identical + at runtime, but `scripts/lib/lane-wiring-census.mjs` matches an object-literal argument and + cannot see through a variable — passing the resolved object directly reads to that gate as an + UNWIRED call and turns it red. #3019's header records the same constraint, and it is what + pushed that PR toward resolving the lanes inline; the constraint is real, the bespoke + resolution it produced was not required by it. + */ const validation = validateNodeOverrideChange(task, normalizedNodeId ?? null, { - wipColumns: nodeOverrideLifecycle?.wip ? new Set([nodeOverrideLifecycle.wip]) : undefined, - completeColumns: nodeOverrideLifecycle?.complete ? new Set([nodeOverrideLifecycle.complete]) : undefined, + wipColumns: overrideLanes.wipColumns, + completeColumns: overrideLanes.completeColumns, }); if (!validation.allowed) { return { diff --git a/packages/core/src/__tests__/node-override-guard.test.ts b/packages/core/src/__tests__/node-override-guard.test.ts index 6ce163af91..66597891d7 100644 --- a/packages/core/src/__tests__/node-override-guard.test.ts +++ b/packages/core/src/__tests__/node-override-guard.test.ts @@ -53,6 +53,35 @@ describe("resolveNodeOverrideLanes builds the set from traits, with legacy as an expect([...lanes.completeColumns]).toEqual(["shipped"]); }); + /* + FNXC:WorkflowResolvedColumns 2026-07-31-01:35: + EVERY wip lane, not the first — the case that separates this resolver from `resolveLifecycleColumns`. + + A board may carry the wip trait on more than one column (a build lane beside a verify lane). This + resolver answers with all of them (`columnsWithFlag`); `resolveLifecycleColumns` answers with the + FIRST (`resolved.find(...)`, workflow-lifecycle-traits.ts:353). The two are interchangeable on every + single-wip-lane board, so substituting one for the other reads as correct and is not. + + It is not hypothetical: #3019 wired the CLI's `fn_task_update` guard with the first-match resolver, + and a task sitting in the SECOND wip lane went on slipping the mid-flight check the PR set out to + close. A single-wip-lane test passes against both, which is why this case names two. + */ + it("returns EVERY wip lane, not just the first", async () => { + const ir = { + version: "v2", id: "wf", name: "wf", nodes: [], edges: [], + columns: [ + { id: "building", name: "Building", traits: [{ trait: "wip" }] }, + { id: "verifying", name: "Verifying", traits: [{ trait: "wip" }] }, + { id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] }, + ], + }; + const lanes = await resolveNodeOverrideLanes(storeFor(ir), "FN-1"); + + expect([...lanes.wipColumns].sort()).toEqual(["building", "verifying"]); + /* The load-bearing half: a task in the second lane must still be seen as executing. */ + expect(lanes.wipColumns.has("verifying")).toBe(true); + }); + it("falls back to the legacy ids for a V1-UPGRADED board that traits nothing", async () => { const v1 = { version: "v2", id: "wf", name: "wf", nodes: [], edges: [], diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 6605686879..292917fed4 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -1298,6 +1298,7 @@ export { export type { FnBinaryStatus, FnBinaryName } from "./fn-binary.js"; export { validateNodeOverrideChange, + resolveNodeOverrideLanes, type NodeOverrideValidationResult, type NodeOverrideBlockReason, } from "./node-override-guard.js";