From 51e108b4f08f68725ffa1eff25a338efea54e66c Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 29 Jul 2026 22:39:50 -0700 Subject: [PATCH] core/workflow-ir-resolver: let callers tell a RESOLVED workflow from a GUESSED one (unblocks the triage census) (#2618) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../workflow-ir-resolution-provenance.test.ts | 172 ++++++++++++++++++ packages/core/src/index.gate.ts | 3 + packages/core/src/index.ts | 3 + packages/core/src/workflow-ir-resolver.ts | 124 +++++++++++-- 4 files changed, 291 insertions(+), 11 deletions(-) create mode 100644 packages/core/src/__tests__/workflow-ir-resolution-provenance.test.ts diff --git a/packages/core/src/__tests__/workflow-ir-resolution-provenance.test.ts b/packages/core/src/__tests__/workflow-ir-resolution-provenance.test.ts new file mode 100644 index 0000000000..5120c8c64d --- /dev/null +++ b/packages/core/src/__tests__/workflow-ir-resolution-provenance.test.ts @@ -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"); + }); +}); diff --git a/packages/core/src/index.gate.ts b/packages/core/src/index.gate.ts index 23df150ee0..6fd6343091 100644 --- a/packages/core/src/index.gate.ts +++ b/packages/core/src/index.gate.ts @@ -508,6 +508,9 @@ export { } from "./builtin-completion-summary-node.js"; export { resolveWorkflowIrForTask, + resolveWorkflowIrForTaskWithProvenance, + type ResolvedWorkflowIr, + type WorkflowIrResolutionSource, resolveWorkflowIrById, resolveSeamPromptFromIr, resolvePlanningPromptFromIr, diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 705e933fbf..7192e5f76a 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -561,6 +561,9 @@ export { } from "./builtin-completion-summary-node.js"; export { resolveWorkflowIrForTask, + resolveWorkflowIrForTaskWithProvenance, + type ResolvedWorkflowIr, + type WorkflowIrResolutionSource, resolveWorkflowIrById, resolveSeamPromptFromIr, resolvePlanningPromptFromIr, diff --git a/packages/core/src/workflow-ir-resolver.ts b/packages/core/src/workflow-ir-resolver.ts index 12489477b6..96fca37fec 100644 --- a/packages/core/src/workflow-ir-resolver.ts +++ b/packages/core/src/workflow-ir-resolver.ts @@ -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)[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, -): Promise { +): Promise { 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, +): Promise { + /* + * 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; }