fix(cli): the node-override guard saw only the FIRST wip lane (#3023)
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) <noreply@anthropic.com>
This commit is contained in:
7
.changeset/node-override-every-wip-lane.md
Normal file
7
.changeset/node-override-every-wip-lane.md
Normal file
@@ -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`.
|
||||||
@@ -42,6 +42,7 @@ import {
|
|||||||
type SecretScope,
|
type SecretScope,
|
||||||
declaresAnyLifecycleTrait,
|
declaresAnyLifecycleTrait,
|
||||||
resolveTaskLifecycleColumns,
|
resolveTaskLifecycleColumns,
|
||||||
|
resolveNodeOverrideLanes,
|
||||||
resolveLifecycleColumns,
|
resolveLifecycleColumns,
|
||||||
resolveWorkflowIrForTaskWithProvenance,
|
resolveWorkflowIrForTaskWithProvenance,
|
||||||
resolveWorkflowIrForTask,
|
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
|
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.
|
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, {
|
const validation = validateNodeOverrideChange(task, normalizedNodeId ?? null, {
|
||||||
wipColumns: nodeOverrideLifecycle?.wip ? new Set([nodeOverrideLifecycle.wip]) : undefined,
|
wipColumns: overrideLanes.wipColumns,
|
||||||
completeColumns: nodeOverrideLifecycle?.complete ? new Set([nodeOverrideLifecycle.complete]) : undefined,
|
completeColumns: overrideLanes.completeColumns,
|
||||||
});
|
});
|
||||||
if (!validation.allowed) {
|
if (!validation.allowed) {
|
||||||
return {
|
return {
|
||||||
|
|||||||
@@ -53,6 +53,35 @@ describe("resolveNodeOverrideLanes builds the set from traits, with legacy as an
|
|||||||
expect([...lanes.completeColumns]).toEqual(["shipped"]);
|
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 () => {
|
it("falls back to the legacy ids for a V1-UPGRADED board that traits nothing", async () => {
|
||||||
const v1 = {
|
const v1 = {
|
||||||
version: "v2", id: "wf", name: "wf", nodes: [], edges: [],
|
version: "v2", id: "wf", name: "wf", nodes: [], edges: [],
|
||||||
|
|||||||
@@ -1298,6 +1298,7 @@ export {
|
|||||||
export type { FnBinaryStatus, FnBinaryName } from "./fn-binary.js";
|
export type { FnBinaryStatus, FnBinaryName } from "./fn-binary.js";
|
||||||
export {
|
export {
|
||||||
validateNodeOverrideChange,
|
validateNodeOverrideChange,
|
||||||
|
resolveNodeOverrideLanes,
|
||||||
type NodeOverrideValidationResult,
|
type NodeOverrideValidationResult,
|
||||||
type NodeOverrideBlockReason,
|
type NodeOverrideBlockReason,
|
||||||
} from "./node-override-guard.js";
|
} from "./node-override-guard.js";
|
||||||
|
|||||||
Reference in New Issue
Block a user