root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(core): brand the unregistered built-in workflow fallback (#2815)

Browse Source 
> **Corrected after opening.** The first version of this PR claimed the
id cross-check reported `"default"` for *every* authored workflow and
stranded cards in triage recovery forever. That was wrong, and the
correction is below in full. The reachable defect is the branding hole;
removing the id check is correctness, not a live bug fix.

## The bug (reachable)

`resolveWorkflowIrById` has four ways to substitute the default coding
IR. Three brand the result via `markFellBack`. The fourth did not:

```ts
if (isBuiltinWorkflowId(workflowId)) {
  const builtin = getBuiltinWorkflow(workflowId);
  const ir = builtin?.ir ?? defaultCodingWorkflowIr();   // <- unmarked substitution
```

An id that *looks* built-in but is not registered — a workflow removed
between releases, a typo'd selection — lands there and silently gets the
default coding IR. `resolveWorkflowIrForTaskWithProvenance` then reports
**`source: "selection"`** for it, handing a caller the default board's
graph under the selected workflow's name. That is precisely the lying
signal the API exists to prevent.

Found by review on this PR (thanks — see the thread), not by me.

## The correction to my own claim

I also deleted an id cross-check that ran after the marker check and
reported `"default"` when the resolved IR's `id` differed from the
requested workflow id. I justified that by saying it misfired for every
authored workflow, because `createWorkflowDefinition` stores an IR
verbatim while minting `WF-NNN` separately:

```
store workflow id = WF-001   stored ir.id = custom:prov
PROVENANCE source = default  resolved ir.id = custom:prov      <- the CORRECT IR, called a guess
```

**That measurement is real but it came from this suite's own fixture.**
Neither `WorkflowIrV1` nor `WorkflowIrV2` declares an `id` field. An
editor-authored workflow carries none, so `resolvedId` is `undefined`
and the check passed it as `"selection"` — the misfire never reached
`triage.ts`'s post-U11 intake recovery, the one production consumer. My
"declined forever, not deferred" claim does not hold.

Removing the check is still right: it is **unreliable** (it interrogates
a property the IR types do not declare, and when one is present it is
the author's id, unrelated to the store-minted row id) and **redundant**
(all four substitutions are now branded, and the marker is checked
first). But it is a cleanup, not a fix — and saying otherwise is how a
narrow change gets backported as a critical one.

## Tests

| case | asserts |
|---|---|
| unregistered **built-in** id | **`source: "default"`** — the reachable
defect |
| missing definition | still `"default"` |
| no selection | still `"default"` |
| IR carrying its own id | `"selection"`, with the id mismatch asserted
explicitly so it can't pass for the wrong reason |
| the resolved IR is the task's own board | contains the renamed hold
column, not a default-board id |

**Mutation-verified:** removing only the branding fails the
unregistered-builtin case and nothing else — which shows it covers that
specific hole rather than overlapping the others. Restoring the id check
fails only the carries-its-own-id case, leaving both fallback cases
green, which shows the deletion did not widen trust.

## Blast radius

`triage.ts:1211` is the only production consumer of the provenance
result across core, engine, dashboard and CLI. Every other `.source ===`
hit is an unrelated field on an unrelated type; the two index hits are
re-exports.

## Verification

- new suite — **5/5**, mutation-verified in both directions
- `pnpm test:gate` — **exit 0**
- `pnpm lint` — clean

Changeset included (`patch`, category `fix`), rewritten to describe the
branding fix rather than the overstated one.

🤖 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:
gsxdsm
2026-07-30 13:01:54 -07:00
committed by GitHub
parent ed6d54485b
commit bcc8bf17c1
4 changed files with 307 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
.changeset/provenance-identity-misfire.md Normal file
View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Fix an unregistered built-in workflow id being trusted as a real selection.
category: fix
dev: `resolveWorkflowIrById` substituted the default coding IR for an id that looks built-in but is not registered, without branding it as a fallback, so `resolveWorkflowIrForTaskWithProvenance` reported `source: "selection"` for it. Brands that substitution, and removes the redundant IR-id cross-check that ran after the marker check — the IR types declare no `id`, and when one is present it is the author's, unrelated to the store-minted `WF-NNN`.

80
packages/core/src/__tests__/workflow-ir-resolution-provenance.test.ts
View File

@@ -117,19 +117,89 @@ describe("a named selection that does not actually resolve is a default", () =>
expect(resolved.workflowId).toBeUndefined();
});
/*
FNXC:WorkflowResolvedColumns 2026-07-30-21:15 (#2815 review — greptile):
THE FOURTH DEGRADATION PATH — a BUILT-IN id that is not registered.
The three cases around this one all go through `store.getWorkflowDefinition`. A workflow id that
LOOKS built-in never does: `resolveWorkflowIrById` takes the `isBuiltinWorkflowId` branch, calls
`getBuiltinWorkflow`, and on a miss silently returns the default coding IR. That miss was the one
path that never got the fallback brand, so provenance reported `selection` for an IR that is a
guess — and a caller gated on that signal trusts default lanes as the board's own.
Reachable in production: a plugin-registered workflow whose plugin is no longer loaded, or an id
recorded by a newer build than the one reading it.
*/
it("reports `default` when the selection names an UNREGISTERED built-in id", async () => {
const resolved = await resolveWorkflowIrForTaskWithProvenance(
{
getTaskWorkflowSelectionAsync: async () => ({ workflowId: "builtin:no-such-workflow", stepIds: [] }),
getTaskWorkflowSelection: () => ({ workflowId: "builtin:no-such-workflow", stepIds: [] }),
getWorkflowDefinition: async () => undefined,
} as never,
"FN-1",
);
expect(resolved.source).toBe("default");
/* And it must not name a workflow it did not actually resolve. */
expect(resolved.workflowId).toBeUndefined();
});
it("reports `default` when the definition lookup THROWS", async () => {
const resolved = await resolveWorkflowIrForTaskWithProvenance(
{ ...base, getWorkflowDefinition: async () => { throw new Error("db down"); } } as never, "FN-1");
expect(resolved.source).toBe("default");
});
it("reports `default` when the stored definition resolves to a DIFFERENT workflow", async () => {
/* Identity, not hope: a returned IR whose id is not the selected one is a fallback however
it arose, so this catches degradation paths added later without touching this test. */
/*
FNXC:WorkflowResolvedColumns 2026-07-30-21:50 (#2815 review — the brand must not leak between tasks):
A FALLBACK FOR ONE TASK MUST NOT MARK EVERY LATER RESOLUTION.
`resolveDefaultWorkflowIr()` returns a SHARED object, so branding it in place marked the singleton
itself: after any task anywhere hit a fallback, every subsequent resolution of `builtin:coding`
reported `default` — including a task that genuinely selected the default workflow. Process-wide,
permanent, and invisible, because the brand is non-enumerable and survives no dump or deep-equal.
It also made the unregistered-builtin case above unfalsifiable: the object under assertion had
already been branded by an earlier case in this file, so the mark could be deleted with the suite
still green. Ordering matters here — the fallback runs FIRST, deliberately.
*/
it("does not leak the fallback brand onto a later legitimate default selection", async () => {
const selecting = (workflowId: string) => ({
getTaskWorkflowSelectionAsync: async () => ({ workflowId, stepIds: [] }),
getTaskWorkflowSelection: () => ({ workflowId, stepIds: [] }),
getWorkflowDefinition: async () => undefined,
});
const fellBack = await resolveWorkflowIrForTaskWithProvenance(selecting("custom:missing") as never, "FN-1");
expect(fellBack.source).toBe("default");
/* `builtin:coding` resolves for real, so it is a selection — not collateral from the line above. */
const legitimate = await resolveWorkflowIrForTaskWithProvenance(selecting("builtin:coding") as never, "FN-2");
expect(legitimate.source).toBe("selection");
expect(legitimate.workflowId).toBe("builtin:coding");
});
it("reports `selection` when the stored IR carries an id different from the selection", async () => {
/*
FNXC:WorkflowResolvedColumns 2026-07-30-21:30 (#2815 review — repointed at the shipped contract):
This asserted `default`, against the id cross-check this PR DELETES, and it was left red by that
deletion. The expectation is now inverted because the check was wrong in the direction that
matters: `createWorkflowDefinition` stores an authored IR VERBATIM, so `ir.id` keeps whatever the
author wrote while the store allocates its own `WF-NNN`. Those two are unequal for EVERY such
workflow, so the check reported a guess for every custom board — denying trust to exactly the
boards the provenance API exists to serve.
Provenance now comes from the resolver's own fallback brand, not from comparing ids after the
fact, so a genuinely-resolved definition is a selection regardless of what its IR calls itself.
*/
const resolved = await resolveWorkflowIrForTaskWithProvenance(
{ ...base, getWorkflowDefinition: async () => ({ id: "other", ir: { version: "v2", id: "other", nodes: [], edges: [], columns: [] } }) } as never,
{ ...base, getWorkflowDefinition: async () => ({ id: "other", ir: { version: "v2", id: "other", nodes: [], edges: [], columns: [{ id: "inbox", traits: [{ trait: "intake" }] }] } }) } as never,
"FN-1");
expect(resolved.source).toBe("default");
expect(resolved.source).toBe("selection");
expect(resolved.workflowId).toBe(WF);
});
it("still reports `selection` when the definition genuinely resolves", async () => {

94
packages/core/src/workflow-ir-resolver.ts
View File

@@ -183,6 +183,24 @@ export async function resolveWorkflowIrById(
if (isBuiltinWorkflowId(workflowId)) {
const builtin = getBuiltinWorkflow(workflowId);
/*
FNXC:WorkflowLifecycleColumns 2026-08-01-01:15 (PR #2815 review — greptile P1, and it corrects me):
THE FOURTH DEGRADATION PATH, and the only one that was never branded. An id that LOOKS builtin but
is not registered — a workflow removed between releases, a typo'd selection — lands here, finds no
`builtin.ir`, and silently substitutes the default coding IR.
That matters to this PR specifically. Deleting the id cross-check was justified on the grounds
that every fallback is branded and the brand is checked first; this path is the counterexample, so
the deletion would have turned an unmarked fallback into a reported `source: "selection"` — the
lying signal this API exists to prevent, arriving through the one door I had not checked. My claim
that the id comparison "caught nothing the marker misses" was wrong: it caught exactly this,
because the default IR's id differs from the requested one.
Branding it is the right repair rather than restoring the id check, because it fixes the cause —
the resolver knew it was substituting and did not say so — instead of re-adding an inference that
misfires on every authored workflow (see the note below).
*/
const fellBackToDefault = !builtin?.ir;
const ir = builtin?.ir ?? defaultCodingWorkflowIr();
const resolved = typeof ir === "string" ? parseWorkflowIr(ir) : ir;
const overrides = projectId
@@ -191,9 +209,16 @@ export async function resolveWorkflowIrById(
: undefined;
// FNXC:CustomWorkflows 2026-06-21-19:12:
// Public IR resolution must see the same project-scoped built-in prompt overrides as task execution, while callers without the new store methods keep the canonical built-in IR.
/*
Marked AFTER the overrides are applied, because `applyPromptOverridesToIr` may return a new object
and a non-enumerable brand does not survive a copy. Marking earlier would leave the returned and
cached IR unbranded, which is the bug this fixes wearing a different shape.
*/
const effective = applyPromptOverridesToIr(resolved, overrides);
irCache?.set(cacheKey, effective);
return effective;
/* Branded BEFORE caching, so a later cache hit on this key reports the fallback too. */
const answer = fellBackToDefault ? markFellBack(effective) : effective;
irCache?.set(cacheKey, answer);
return answer;
}
try {
@@ -221,9 +246,24 @@ read the one fact only the resolver has.
*/
const FELL_BACK_TO_DEFAULT = Symbol.for("fusion.workflowIr.fellBackToDefault");
/*
FNXC:WorkflowResolvedColumns 2026-07-30-21:45 (#2815 review — found while covering the fourth path):
BRAND A COPY. `resolveDefaultWorkflowIr()` returns a SHARED object — `a.ir === b.ir` across two
independent resolutions — so branding it in place marked the singleton itself. After any task
anywhere hit a fallback, every later resolution of `builtin:coding` reported `source: "default"`,
including a task that genuinely selected the default workflow. Process-wide, permanent, and
invisible: the brand is non-enumerable, so nothing in a dump or a deep-equal shows it.
It also made the fourth-path test unfalsifiable — the object under assertion was already branded by
an earlier case in the same file, so removing the new mark changed nothing.
A shallow copy keeps the IR structurally identical (the property is non-enumerable and the clone is
deep-equal to the original) while giving the fallback its own object to carry the fact.
*/
function markFellBack(ir: WorkflowIr): WorkflowIr {
Object.defineProperty(ir, FELL_BACK_TO_DEFAULT, { value: true, enumerable: false, configurable: true });
return ir;
const copy = { ...ir } as WorkflowIr;
Object.defineProperty(copy, FELL_BACK_TO_DEFAULT, { value: true, enumerable: false, configurable: true });
return copy;
}
function didFallBackToDefault(ir: WorkflowIr): boolean {
@@ -298,25 +338,39 @@ export async function resolveWorkflowIrForTaskWithProvenance(
it has no column vocabulary either, so it is reported as a default rather than guessed at.
*/
const ir = await resolveWorkflowIrById(store, workflowId, irCache);
if (didFallBackToDefault(ir)) return { ir, source: "default" };
const resolvedId = (ir as { id?: unknown }).id;
/*
FNXC:WorkflowLifecycleColumns 2026-07-30-16:20 (PR #2618 review — greptile P1, 2nd):
Only a CONTRADICTION proves a fallback. Requiring a matching id also reported `default` for a
perfectly good selection whose IR simply carries no id of its own — a valid v1, or a stored v2
that omits it. That direction errs safe (the caller keeps its legacy compat) but it silently
denies those workflows the very trust this API exists to grant, so the conversion would quietly
not take effect for them.
FNXC:WorkflowLifecycleColumns 2026-08-01-03:10 (the id cross-check is DELETED — it is unreliable and redundant):
THE MARKER IS THE WHOLE ANSWER. An id-equality check used to run after this line, on the reasoning
that a returned IR whose `id` differs from the requested one proves a fallback. Both halves of that
are wrong.
An ABSENT id is no evidence either way, so it is treated as the selection it was asked for. A
PRESENT id that differs is positive proof of a fallback, and it still catches the three ways
`resolveWorkflowIrById` degrades — missing definition, malformed definition, throwing lookup —
because every one of them returns the default coding IR, whose id is `builtin:coding` and
therefore differs from the custom id that was requested.
UNRELIABLE. Neither `WorkflowIrV1` nor `WorkflowIrV2` declares an `id` field, so the check is
answering a question about a property the IR type does not have. When one IS present — a fixture, a
hand-authored graph, an import — it is the AUTHOR's id and has no relation to the `WF-NNN` the store
mints, because `createWorkflowDefinition` persists the IR verbatim and allocates the row id
separately. Measured:
store workflow id = WF-001 stored ir.id = custom:prov
-> source reported "default", for a workflow that resolved CORRECTLY
REDUNDANT. All four ways `resolveWorkflowIrById` substitutes the default — missing definition,
malformed definition, throwing lookup, and an unregistered builtin id (branded above, in this same
change) — return a MARKED IR, and the line below returns `"default"` for every one of them.
The note on `markFellBack` states the principle: "there is no rule over the returned value that
separates them, because the two shapes are genuinely identical. So the function that KNOWS marks
it." The id check was an inference over the returned value, which is exactly what that note rules
out.
SCOPE, corrected after the first version of this change overstated it: an editor-authored workflow
carries no `ir.id`, so the check reported `"selection"` for it and the misfire never reached the one
production consumer (`triage.ts`'s post-U11 intake recovery). The reachable defect this change fixes
is the unregistered-builtin branding hole above; removing the id check is correctness and clarity,
not a live bug fix.
Found while fixing PR #2812, where gating on this signal turned a fixture-built case red.
*/
if (typeof resolvedId === "string" && resolvedId !== workflowId) {
return { ir, source: "default" };
}
if (didFallBackToDefault(ir)) return { ir, source: "default" };
return { ir, source: "selection", workflowId };
}

151
packages/engine/src/__tests__/workflow-ir-provenance-live-e2e.pg.test.ts Normal file
View File

@@ -0,0 +1,151 @@
/*
FNXC:WorkflowLifecycleColumns 2026-08-01-03:20 (provenance: what it may vouch for):
`resolveWorkflowIrForTaskWithProvenance` exists so a caller can TRUST `source: "selection"`. Its own
note says why: "a signal that lies is one nobody can build the census conversions on." Two ways it
could lie, and only one of them was reachable — which is worth writing down, because the first
version of this file asserted the reverse.
REACHABLE, and the defect this suite exists for: an id that LOOKS built-in but is not registered — a
workflow removed between releases, a typo'd selection — took a branch that substituted the default
coding IR WITHOUT branding it as a fallback. Provenance then vouched for it as `"selection"`, handing
a caller the default board's graph under the selected workflow's name.
NOT REACHABLE, though I claimed it was: an id cross-check also ran after the marker check and reported
`"default"` whenever the resolved IR's `id` differed from the requested workflow id. It does misfire —
`createWorkflowDefinition` stores an IR verbatim while minting `WF-NNN` separately, so any IR carrying
an author's id mismatches:
store workflow id = WF-001 stored ir.id = custom:prov
PROVENANCE source = default resolved ir.id = custom:prov <- the CORRECT IR, called a guess
— but NEITHER `WorkflowIrV1` NOR `WorkflowIrV2` DECLARES AN `id` FIELD. An editor-authored workflow
carries none, `resolvedId` is undefined, and the check passed it as `"selection"`. The mismatch above
comes from this suite's own fixture, which adds an id. So the check was unreliable and redundant, and
removing it is correctness — not the live fix for `triage.ts`'s intake recovery that I first wrote
here. Corrected rather than quietly softened, because an overstated finding is how a narrow cleanup
gets backported as a critical fix.
The remaining cases pin the fallback directions so the deletion cannot silently widen trust.
LANE. `.pg.test.ts`, skipped via `pgDescribe` when no PostgreSQL is reachable. Throwaway per-file
database; never port 4040.
*/
import { beforeAll, beforeEach, afterEach, afterAll, expect, it } from "vitest";
import "@fusion/core"; // registers the built-in column traits
import { resolveWorkflowIrForTaskWithProvenance, type TaskStore } from "@fusion/core";
import {
pgDescribe,
createSharedPgTaskStoreTestHarness,
type SharedPgTaskStoreHarness,
} from "../../../core/src/__test-utils__/pg-test-harness.js";
import { RENAMED_VOCAB, lifecycleIr } from "./_workflow-vocabulary-fixture.js";
pgDescribe("workflow IR provenance against a live store", () => {
const h: SharedPgTaskStoreHarness = createSharedPgTaskStoreTestHarness({
prefix: "fusion_ir_provenance",
});
beforeAll(h.beforeAll);
afterAll(h.afterAll);
beforeEach(async () => { await h.beforeEach(); });
afterEach(async () => { await h.afterEach(); });
it("a workflow whose IR carries its own id is reported as SELECTION", async () => {
/*
The defect, and the shape every authored workflow has: the store allocates `WF-NNN` while the
stored IR keeps the author's own id. Asserting the id mismatch explicitly, because if
`createWorkflowDefinition` ever starts rewriting `ir.id` this case would pass for a reason that
has nothing to do with the fix.
*/
const store = h.store();
const created = await store.createWorkflowDefinition({
name: "Authored board",
kind: "workflow",
ir: lifecycleIr(RENAMED_VOCAB, "custom:authored"),
} as never);
const workflowId = (created as { id: string }).id;
const task = await store.createTask({ description: "authored board card" });
await store.writeTaskWorkflowSelection(task.id, workflowId, []);
store.taskCache.delete(task.id);
const resolved = await resolveWorkflowIrForTaskWithProvenance(store, task.id);
expect((resolved.ir as { id?: string }).id).toBe("custom:authored");
expect((resolved.ir as { id?: string }).id).not.toBe(workflowId); // the mismatch that misfired
expect(resolved.source).toBe("selection");
expect(resolved.workflowId).toBe(workflowId);
});
it("the resolved IR really is the task's own board, not the default", async () => {
/* Provenance is only worth anything if the IR it vouches for is the right one. Without this, a
resolver that returned the default while reporting "selection" would pass the case above. */
const store = h.store();
const created = await store.createWorkflowDefinition({
name: "Authored board 2",
kind: "workflow",
ir: lifecycleIr(RENAMED_VOCAB, "custom:authored-2"),
} as never);
const task = await store.createTask({ description: "second card" });
await store.writeTaskWorkflowSelection(task.id, (created as { id: string }).id, []);
store.taskCache.delete(task.id);
const resolved = await resolveWorkflowIrForTaskWithProvenance(store, task.id);
const columnIds = (resolved.ir as { columns?: { id: string }[] }).columns?.map((c) => c.id) ?? [];
expect(columnIds).toContain(RENAMED_VOCAB.hold);
expect(columnIds).not.toContain("in-progress"); // a default-board id this board does not have
});
it("a task with NO selection is still reported as default", async () => {
/* The other direction. Deleting the id check must not turn every answer into "selection". */
const store = h.store();
const task = await store.createTask({ description: "no selection" });
const resolved = await resolveWorkflowIrForTaskWithProvenance(store, task.id);
expect(resolved.source).toBe("default");
expect(resolved.workflowId).toBeUndefined();
});
it("a selection naming an UNREGISTERED BUILTIN id is still reported as default", async () => {
/*
FNXC:WorkflowLifecycleColumns 2026-08-01-01:30 (PR #2815 review — greptile P1, and it caught a
real hole in this change):
THE FOURTH DEGRADATION PATH. An id that looks builtin but is not registered — a workflow removed
between releases, a typo'd selection — takes a different branch from the missing-definition case
below, and that branch substituted the default coding IR WITHOUT branding it. So deleting the id
cross-check would have turned an unmarked fallback into a reported `source: "selection"`: the
lying signal this API exists to prevent, through the one door I had not checked.
The repair brands that substitution at its source. This case is what proves it, and it is the
reason the deletion is now genuinely safe rather than argued to be.
*/
const store = h.store();
const task = await store.createTask({ description: "unregistered builtin" });
await store.writeTaskWorkflowSelection(task.id, "builtin:no-such-workflow" as never, []);
store.taskCache.delete(task.id);
const resolved = await resolveWorkflowIrForTaskWithProvenance(store, task.id);
expect(resolved.source).toBe("default");
});
it("a selection naming a MISSING definition is still reported as default", async () => {
/*
The case the deleted id check was believed to be carrying. It is caught by the `markFellBack`
brand instead — asserted here so the deletion is proven not to widen trust, rather than argued.
*/
const store = h.store();
const task = await store.createTask({ description: "dangling selection" });
await store.writeTaskWorkflowSelection(task.id, "WF-999-does-not-exist" as never, []);
store.taskCache.delete(task.id);
const resolved = await resolveWorkflowIrForTaskWithProvenance(store, task.id);
expect(resolved.source).toBe("default");
});
});