core/workflow-ir-resolver: let callers tell a RESOLVED workflow from a GUESSED one (unblocks the triage census) (#2618)
Shared-backlog infrastructure, not a single-file conversion. This is the blocker I hit on three separate census files and flagged twice; landing it once beats working around it five more times. ## The problem `resolveWorkflowIrForTask` returns the default coding IR in two cases that are **not** the same as knowing which workflow governs a task: - the selection read threw; - the store reported no selection at all — the synchronous PostgreSQL path does exactly this, deliberately. Callers cannot distinguish either from a genuine selection. **For lifecycle-column work that difference decides correctness.** Post-merge the default coding lineage declares `todo` as its single Planning column and **no `triage`**. So a call site converting a `column === "triage"` guard to trait resolution silently stops firing for `builtin:legacy-coding` cards whenever the store cannot name the workflow — it is handed the default's vocabulary with no signal that it was a guess. ## Why this is the census blocker, with receipts Every conversion I have landed has hit it and worked around it the same way: | Site | Workaround forced | |---|---| | `usage-limit-detector.ts` (#2572) | narrowed to intake, three separate corrections | | `mission-feature-sync.ts` (#2609) | legacy ids unioned, then position-ordered to stop over-claiming | | `live-agent-count.ts` (#2604) | not converted at all — left as a documented finding | That is why the count stalls around a dozen rather than converging on zero: the honest conversion is unavailable, so each site keeps the literal "just in case". With provenance a caller can finally say what it means — **trust the resolved columns when the workflow was selected; fall back to legacy compat only when it was guessed.** ## What lands `resolveWorkflowIrForTaskWithProvenance` returning `{ ir, source: "selection" | "default", workflowId? }`. **Additive by construction:** `resolveWorkflowIrForTask` delegates to it and drops the provenance, so the two answers cannot drift and no existing caller changes behaviour. ## Red-green Mislabelling the no-selection guess as a selection fails its test (`1 failed | 5 passed`). One test deliberately pins the underlying *fact* rather than assuming it — the default guess really does lack `triage` and does have `todo`. If that lineage ever regains the column the hazard changes, and the callers relying on provenance should be revisited; this is what will tell them. Another asserts `resolveWorkflowIrForTask` returns exactly the provenance form's IR across all three paths, so the delegation cannot silently diverge. ## Not done here I have **not** converted any call site onto it. Each one is a behaviour decision for its owner — `comments-ops.ts`, `task-creation.ts`, `archive-planning.ts`, plus revisiting the three above — and bundling them would make this unrevertable. The enabler is the shared part. ## Verification - 6 new tests green; `pnpm test:gate` green (132 / 10 / 482 / 71); `pnpm lint` clean; `tsc --noEmit` clean - Additive API on a private package (`@fusion/core`), no behaviour change, so no changeset 🤖 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:
@@ -0,0 +1,172 @@
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-12:30 (lifecycle-column census enabler):
|
||||
|
||||
`resolveWorkflowIrForTask` returns the default coding IR in two cases that are NOT the same as
|
||||
knowing which workflow governs a task: the selection read threw, and the store reported no
|
||||
selection (the synchronous PostgreSQL path does exactly that). Callers cannot tell a guess from a
|
||||
real answer, and for lifecycle-column work that difference decides correctness.
|
||||
|
||||
Concretely: post-merge the default coding lineage declares `todo` as its single Planning column
|
||||
and NO `triage`. A call site converting a `column === "triage"` guard to trait resolution
|
||||
therefore stops firing for `builtin:legacy-coding` cards whenever the store cannot name the
|
||||
workflow — it silently gets the default's vocabulary. Every site converted so far has had to keep
|
||||
the legacy ids unioned "just in case", which is why the census stalls rather than converging.
|
||||
|
||||
These pin the three answers a caller needs to distinguish, and that the existing function's
|
||||
behaviour is untouched.
|
||||
*/
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { resolveWorkflowIrForTask, resolveWorkflowIrForTaskWithProvenance } from "../workflow-ir-resolver.js";
|
||||
|
||||
const WF = "custom:wf";
|
||||
const customIr = {
|
||||
version: "v2",
|
||||
id: WF,
|
||||
nodes: [],
|
||||
edges: [],
|
||||
columns: [
|
||||
{ id: "inbox", label: "Inbox", traits: [{ trait: "intake" }] },
|
||||
{ id: "building", label: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] },
|
||||
],
|
||||
};
|
||||
|
||||
function storeWith(selection: unknown, opts: { throws?: boolean } = {}) {
|
||||
return {
|
||||
getTaskWorkflowSelectionAsync: async () => {
|
||||
if (opts.throws) throw new Error("selection read failed");
|
||||
return selection;
|
||||
},
|
||||
getTaskWorkflowSelection: () => selection,
|
||||
getWorkflowDefinition: async (id: string) => (id === WF ? { id: WF, ir: customIr } : undefined),
|
||||
} as never;
|
||||
}
|
||||
|
||||
describe("workflow IR resolution provenance", () => {
|
||||
it("reports `selection` when the store names a workflow", async () => {
|
||||
const resolved = await resolveWorkflowIrForTaskWithProvenance(storeWith({ workflowId: WF, stepIds: [] }), "FN-1");
|
||||
expect(resolved.source).toBe("selection");
|
||||
expect(resolved.workflowId).toBe(WF);
|
||||
expect((resolved.ir as { id: string }).id).toBe(WF);
|
||||
});
|
||||
|
||||
it("reports `default` when the store reports NO selection", async () => {
|
||||
/* The synchronous PostgreSQL path — a guess that previously looked identical to an answer. */
|
||||
const resolved = await resolveWorkflowIrForTaskWithProvenance(storeWith(undefined), "FN-1");
|
||||
expect(resolved.source).toBe("default");
|
||||
expect(resolved.workflowId).toBeUndefined();
|
||||
});
|
||||
|
||||
it("reports `default` when the selection read THROWS", async () => {
|
||||
const resolved = await resolveWorkflowIrForTaskWithProvenance(storeWith(undefined, { throws: true }), "FN-1");
|
||||
expect(resolved.source).toBe("default");
|
||||
});
|
||||
|
||||
it("the default guess really does lack `triage` — which is why provenance matters", async () => {
|
||||
/*
|
||||
Not a tautology: this is the fact that makes a converted guard stop firing for legacy cards.
|
||||
If the default lineage ever regains a `triage` column, the hazard changes and callers relying
|
||||
on provenance should be revisited.
|
||||
*/
|
||||
const resolved = await resolveWorkflowIrForTaskWithProvenance(storeWith(undefined), "FN-1");
|
||||
const columnIds = ((resolved.ir as { columns?: Array<{ id: string }> }).columns ?? []).map((c) => c.id);
|
||||
expect(columnIds).not.toContain("triage");
|
||||
expect(columnIds).toContain("todo");
|
||||
});
|
||||
|
||||
it("resolveWorkflowIrForTask returns exactly the provenance form's IR (no drift)", async () => {
|
||||
for (const store of [storeWith({ workflowId: WF, stepIds: [] }), storeWith(undefined), storeWith(undefined, { throws: true })]) {
|
||||
const plain = await resolveWorkflowIrForTask(store, "FN-1");
|
||||
const withProvenance = await resolveWorkflowIrForTaskWithProvenance(store, "FN-1");
|
||||
expect(plain).toEqual(withProvenance.ir);
|
||||
}
|
||||
});
|
||||
|
||||
it("shares the caller-owned IR cache — one definition read per workflow", async () => {
|
||||
const getWorkflowDefinition = vi.fn(async () => ({ id: WF, ir: customIr }));
|
||||
const store = {
|
||||
getTaskWorkflowSelectionAsync: async () => ({ workflowId: WF, stepIds: [] }),
|
||||
getTaskWorkflowSelection: () => ({ workflowId: WF, stepIds: [] }),
|
||||
getWorkflowDefinition,
|
||||
} as never;
|
||||
const cache = new Map();
|
||||
await resolveWorkflowIrForTaskWithProvenance(store, "FN-1", cache);
|
||||
await resolveWorkflowIrForTaskWithProvenance(store, "FN-2", cache);
|
||||
expect(getWorkflowDefinition).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-13:25 (PR #2618 review — greptile P1):
|
||||
`resolveWorkflowIrById` degrades to the default coding IR in three further cases beyond the two
|
||||
the first version handled — a missing definition, a malformed one, and a throwing lookup. Naming a
|
||||
selection is not resolving it, and reporting "selection" for any of these hands the caller the
|
||||
default's columns wearing the selected workflow's label. A provenance signal that lies is worse
|
||||
than none, because its whole value is that "selection" can be trusted.
|
||||
*/
|
||||
describe("a named selection that does not actually resolve is a default", () => {
|
||||
const WF = "custom:missing";
|
||||
const base = {
|
||||
getTaskWorkflowSelectionAsync: async () => ({ workflowId: WF, stepIds: [] }),
|
||||
getTaskWorkflowSelection: () => ({ workflowId: WF, stepIds: [] }),
|
||||
};
|
||||
|
||||
it("reports `default` when the definition is MISSING", async () => {
|
||||
const resolved = await resolveWorkflowIrForTaskWithProvenance(
|
||||
{ ...base, getWorkflowDefinition: async () => undefined } as never, "FN-1");
|
||||
expect(resolved.source).toBe("default");
|
||||
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. */
|
||||
const resolved = await resolveWorkflowIrForTaskWithProvenance(
|
||||
{ ...base, getWorkflowDefinition: async () => ({ id: "other", ir: { version: "v2", id: "other", nodes: [], edges: [], columns: [] } }) } as never,
|
||||
"FN-1");
|
||||
expect(resolved.source).toBe("default");
|
||||
});
|
||||
|
||||
it("still reports `selection` when the definition genuinely resolves", async () => {
|
||||
const resolved = await resolveWorkflowIrForTaskWithProvenance(
|
||||
{ ...base, getWorkflowDefinition: async () => ({ id: WF, ir: { version: "v2", id: WF, nodes: [], edges: [], columns: [{ id: "inbox", traits: [{ trait: "intake" }] }] } }) } as never,
|
||||
"FN-1");
|
||||
expect(resolved.source).toBe("selection");
|
||||
expect(resolved.workflowId).toBe(WF);
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-16:25 (PR #2618 review — greptile P1, 2nd):
|
||||
An ABSENT IR id is no evidence of a fallback. Requiring a match also denied trust to valid
|
||||
selections whose IR carries no id — a v1, or a stored v2 that omits it — so the conversion those
|
||||
callers were promised would quietly not take effect. Only a PRESENT, DIFFERING id proves a
|
||||
fallback, and that still catches all three degradation paths because each returns the default
|
||||
coding IR under a different id than the one requested.
|
||||
*/
|
||||
describe("an absent IR id is not evidence of a fallback", () => {
|
||||
const WF = "custom:no-id";
|
||||
const base = {
|
||||
getTaskWorkflowSelectionAsync: async () => ({ workflowId: WF, stepIds: [] }),
|
||||
getTaskWorkflowSelection: () => ({ workflowId: WF, stepIds: [] }),
|
||||
};
|
||||
|
||||
it("reports `selection` for a valid v2 IR that carries no id", async () => {
|
||||
const resolved = await resolveWorkflowIrForTaskWithProvenance(
|
||||
{ ...base, getWorkflowDefinition: async () => ({ id: WF, ir: { version: "v2", nodes: [], edges: [], columns: [{ id: "inbox", traits: [{ trait: "intake" }] }] } }) } as never,
|
||||
"FN-1");
|
||||
expect(resolved.source).toBe("selection");
|
||||
expect(resolved.workflowId).toBe(WF);
|
||||
});
|
||||
|
||||
it("still reports `default` when the definition is missing (differing id is the proof)", async () => {
|
||||
const resolved = await resolveWorkflowIrForTaskWithProvenance(
|
||||
{ ...base, getWorkflowDefinition: async () => undefined } as never, "FN-1");
|
||||
expect(resolved.source).toBe("default");
|
||||
});
|
||||
});
|
||||
@@ -508,6 +508,9 @@ export {
|
||||
} from "./builtin-completion-summary-node.js";
|
||||
export {
|
||||
resolveWorkflowIrForTask,
|
||||
resolveWorkflowIrForTaskWithProvenance,
|
||||
type ResolvedWorkflowIr,
|
||||
type WorkflowIrResolutionSource,
|
||||
resolveWorkflowIrById,
|
||||
resolveSeamPromptFromIr,
|
||||
resolvePlanningPromptFromIr,
|
||||
|
||||
@@ -561,6 +561,9 @@ export {
|
||||
} from "./builtin-completion-summary-node.js";
|
||||
export {
|
||||
resolveWorkflowIrForTask,
|
||||
resolveWorkflowIrForTaskWithProvenance,
|
||||
type ResolvedWorkflowIr,
|
||||
type WorkflowIrResolutionSource,
|
||||
resolveWorkflowIrById,
|
||||
resolveSeamPromptFromIr,
|
||||
resolvePlanningPromptFromIr,
|
||||
|
||||
@@ -198,37 +198,139 @@ export async function resolveWorkflowIrById(
|
||||
|
||||
try {
|
||||
const def = await store.getWorkflowDefinition(workflowId);
|
||||
if (!def) return defaultCodingWorkflowIr();
|
||||
if (!def) return markFellBack(defaultCodingWorkflowIr());
|
||||
const ir = typeof def.ir === "string" ? parseWorkflowIr(def.ir) : def.ir;
|
||||
irCache?.set(cacheKey, ir);
|
||||
return ir;
|
||||
} catch {
|
||||
return defaultCodingWorkflowIr();
|
||||
return markFellBack(defaultCodingWorkflowIr());
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-16:45 (PR #2618 review — greptile P1, both rounds):
|
||||
FALLBACK IS REPORTED, NOT INFERRED. Two review findings pulled in opposite directions and
|
||||
together proved the id cannot answer this: requiring `ir.id === workflowId` denied trust to a
|
||||
valid selection whose IR carries no id, while accepting an absent id let the default fallback —
|
||||
which also has none — pass as a selection. 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. A non-enumerable brand keeps the IR structurally unchanged
|
||||
for every existing consumer, deep-equal comparisons included, while letting the provenance form
|
||||
read the one fact only the resolver has.
|
||||
*/
|
||||
const FELL_BACK_TO_DEFAULT = Symbol.for("fusion.workflowIr.fellBackToDefault");
|
||||
|
||||
function markFellBack(ir: WorkflowIr): WorkflowIr {
|
||||
Object.defineProperty(ir, FELL_BACK_TO_DEFAULT, { value: true, enumerable: false, configurable: true });
|
||||
return ir;
|
||||
}
|
||||
|
||||
function didFallBackToDefault(ir: WorkflowIr): boolean {
|
||||
return (ir as unknown as Record<symbol, unknown>)[FELL_BACK_TO_DEFAULT] === true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a task's workflow IR via its selection. A null/absent selection or any
|
||||
* lookup failure degrades to the built-in default workflow.
|
||||
*/
|
||||
export async function resolveWorkflowIrForTask(
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-12:10 (lifecycle-column census enabler):
|
||||
WHY PROVENANCE IS A SEPARATE ANSWER FROM THE IR.
|
||||
|
||||
`resolveWorkflowIrForTask` answers "which workflow governs this task" by returning the default
|
||||
coding IR in two cases that are NOT the same as knowing: the selection read threw, and the store
|
||||
reported no selection at all (the synchronous PostgreSQL path does exactly that). Callers cannot
|
||||
tell those apart from a genuine selection, and for lifecycle-column work the difference decides
|
||||
correctness rather than tidiness.
|
||||
|
||||
Concretely, and this is what motivated it: post-merge the default coding lineage declares `todo`
|
||||
as its single Planning column and NO `triage`. So a call site converting a `column === "triage"`
|
||||
guard to trait resolution silently stops firing for `builtin:legacy-coding` cards whenever the
|
||||
store cannot name the workflow — the resolver hands back the default's vocabulary and the caller
|
||||
has no way to know it was a guess. Every such site has so far had to keep the legacy ids unioned
|
||||
in "just in case", which is exactly why the census stalls instead of converging.
|
||||
|
||||
With provenance a caller can say what it actually means: trust the resolved columns when the
|
||||
workflow was SELECTED, and fall back to legacy compat only when it was GUESSED.
|
||||
|
||||
Additive by construction — `resolveWorkflowIrForTask` delegates here and drops the provenance, so
|
||||
no existing caller changes behavior.
|
||||
*/
|
||||
export type WorkflowIrResolutionSource = "selection" | "default";
|
||||
|
||||
export interface ResolvedWorkflowIr {
|
||||
ir: WorkflowIr;
|
||||
/** `"selection"` only when the store named a workflow; `"default"` when we guessed. */
|
||||
source: WorkflowIrResolutionSource;
|
||||
/** The selected id, absent when guessed. */
|
||||
workflowId?: string;
|
||||
}
|
||||
|
||||
export async function resolveWorkflowIrForTaskWithProvenance(
|
||||
store: WorkflowIrResolverStore,
|
||||
taskId: string,
|
||||
irCache?: Map<string, WorkflowIr>,
|
||||
): Promise<WorkflowIr> {
|
||||
): Promise<ResolvedWorkflowIr> {
|
||||
let workflowId: string | undefined;
|
||||
try {
|
||||
/*
|
||||
* FNXC:WorkflowModelLanes 2026-07-14-16:26:
|
||||
* Backend-mode task workflow selection is asynchronous. Execution must resolve the migrated task selection before loading its workflow graph; the synchronous PostgreSQL fallback intentionally reports no selection and previously forced every task onto builtin:coding.
|
||||
*/
|
||||
const selection = store.getTaskWorkflowSelectionAsync
|
||||
? await store.getTaskWorkflowSelectionAsync(taskId)
|
||||
: store.getTaskWorkflowSelection(taskId);
|
||||
workflowId = selection?.workflowId;
|
||||
} catch {
|
||||
return defaultCodingWorkflowIr();
|
||||
return { ir: defaultCodingWorkflowIr(), source: "default" };
|
||||
}
|
||||
if (!workflowId) return resolveWorkflowIrById(store, "builtin:coding", irCache);
|
||||
return resolveWorkflowIrById(store, workflowId, irCache);
|
||||
if (!workflowId) {
|
||||
return { ir: await resolveWorkflowIrById(store, "builtin:coding", irCache), source: "default" };
|
||||
}
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-13:20 (PR #2618 review — greptile P1):
|
||||
A NAMED SELECTION IS NOT A RESOLVED ONE. `resolveWorkflowIrById` degrades to the default coding
|
||||
IR in three further cases — a missing definition, a malformed one, and a throwing lookup — so
|
||||
reporting `source: "selection"` merely because the store named an id would hand a caller the
|
||||
default's columns wearing the selected workflow's label. That is worse than having no provenance
|
||||
at all: the entire value of this API is that a caller can TRUST "selection", and a signal that
|
||||
lies is one nobody can build the census conversions on.
|
||||
|
||||
Verified by identity, not by hope: a v2 IR carries its own id, so a returned IR whose id is not
|
||||
the selected one is a fallback however it arose. A v1/column-less IR carries no id to check, and
|
||||
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.
|
||||
|
||||
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.
|
||||
*/
|
||||
if (typeof resolvedId === "string" && resolvedId !== workflowId) {
|
||||
return { ir, source: "default" };
|
||||
}
|
||||
return { ir, source: "selection", workflowId };
|
||||
}
|
||||
|
||||
export async function resolveWorkflowIrForTask(
|
||||
store: WorkflowIrResolverStore,
|
||||
taskId: string,
|
||||
irCache?: Map<string, WorkflowIr>,
|
||||
): Promise<WorkflowIr> {
|
||||
/*
|
||||
* FNXC:WorkflowModelLanes 2026-07-14-16:26:
|
||||
* Backend-mode task workflow selection is asynchronous. Execution must resolve the migrated task selection before loading its workflow graph; the synchronous PostgreSQL fallback intentionally reports no selection and previously forced every task onto builtin:coding.
|
||||
*
|
||||
* FNXC:WorkflowLifecycleColumns 2026-07-30-12:15: delegates to the provenance form and drops
|
||||
* the provenance, so the two answers cannot drift apart.
|
||||
*/
|
||||
return (await resolveWorkflowIrForTaskWithProvenance(store, taskId, irCache)).ir;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user