diff --git a/.changeset/u12-unconditional-v1-persistence.md b/.changeset/u12-unconditional-v1-persistence.md new file mode 100644 index 0000000000..93bdaa0860 --- /dev/null +++ b/.changeset/u12-unconditional-v1-persistence.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Internal cleanup of retired workflow-columns flag reads; no change to stored workflows or board behavior. +category: internal +dev: The three v1-IR rollback-compat persist sites (`createWorkflowDefinition`, `updateWorkflowDefinition`, `insertWorkflowDefinitionSync`) branched on the retired raw `experimentalFeatures.workflowColumns` key, which is always false, so the downgrade arm was always taken. The branch and the `flagOn` parameter are removed; `downgradeIrToV1IfPure` is kept as a binary-downgrade affordance and pinned by `workflow-ir-v1-rollback-persistence.test.ts`. `TaskStore.workflowColumnsFlagOn()` is deleted (no callers). `isWorkflowColumnsCompatibilityFlagEnabled` survives; every remaining read is on the move path (U2b). diff --git a/packages/core/src/__tests__/workflow-definition-id-allocator-sync.test.ts b/packages/core/src/__tests__/workflow-definition-id-allocator-sync.test.ts index 0a87c9e2b4..6be3021078 100644 --- a/packages/core/src/__tests__/workflow-definition-id-allocator-sync.test.ts +++ b/packages/core/src/__tests__/workflow-definition-id-allocator-sync.test.ts @@ -51,10 +51,10 @@ describe("workflow definition id allocator (sync materialization path)", () => { const created = insertWorkflowDefinitionSyncImpl(store, { name: "fresh workflow", ir: BUILTIN_CODING_WORKFLOW_IR, - }, true); + }); expect(created.id).toBe("WF-003"); - const second = insertWorkflowDefinitionSyncImpl(store, { name: "second workflow", ir: BUILTIN_CODING_WORKFLOW_IR }, true); + const second = insertWorkflowDefinitionSyncImpl(store, { name: "second workflow", ir: BUILTIN_CODING_WORKFLOW_IR }); expect(second.id).toBe("WF-004"); }); }); diff --git a/packages/core/src/__tests__/workflow-ir-v1-rollback-persistence.test.ts b/packages/core/src/__tests__/workflow-ir-v1-rollback-persistence.test.ts new file mode 100644 index 0000000000..93f0bfcdfe --- /dev/null +++ b/packages/core/src/__tests__/workflow-ir-v1-rollback-persistence.test.ts @@ -0,0 +1,83 @@ +/* +FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — R9): +Pins the v1-IR rollback-compat PERSISTENCE SHAPE (#1405), which U12 made +unconditional by deleting the retired-flag ternary around it. + +WHY THIS FILE EXISTS. The U12 change at the three persist sites is +behaviour-preserving and therefore has NO revert-proof test — `flagOn ? ir : +downgrade(ir)` with a flag that is always false is the same thing as +`downgrade(ir)`, so re-adding the branch changes nothing observable. Saying "covered +by tests" about that edit would be false. + +What DOES need a guard is the next edit someone is tempted to make: deleting the +downgrade outright as "dead cutover machinery". It is not cutover machinery — it is a +compatibility affordance that lets a binary downgrade still load a row, and stale +binaries opening these databases is an observed event in this project. These cases +fail if the downgrade is removed, and they pin the exact boundary of when it applies. +*/ +import { describe, expect, it } from "vitest"; +import { BUILTIN_CODING_WORKFLOW_IR } from "../builtin-coding-workflow-ir.js"; +import { downgradeIrToV1IfPure, parseWorkflowIr, serializeWorkflowIr } from "../workflow-ir.js"; +import type { WorkflowIrV2 } from "../workflow-ir-types.js"; + +describe("v1 IR rollback-compat persistence shape", () => { + it("stores a pure-v1-equivalent graph in the v1 shape", () => { + const stored = downgradeIrToV1IfPure(BUILTIN_CODING_WORKFLOW_IR); + // Fails if the downgrade is deleted: the built-in coding workflow declares named + // columns with traits, so it is NOT pure v1 and must stay v2. + expect(BUILTIN_CODING_WORKFLOW_IR.version).toBe("v2"); + expect(stored.version).toBe("v2"); + }); + + it("downgrades a graph that carries only default columns and default placements", () => { + // A v1 graph upgraded to v2 is by construction pure-v1-equivalent, so it must + // round-trip back down to v1 on persist. + const upgraded = parseWorkflowIr({ + version: "v1", + name: "pure", + nodes: [ + { id: "start", kind: "start" }, + { id: "n1", kind: "prompt", config: { prompt: "hi" } }, + { id: "end", kind: "end" }, + ], + edges: [ + { from: "start", to: "n1" }, + { from: "n1", to: "end" }, + ], + }); + expect(upgraded.version).toBe("v2"); + + const stored = downgradeIrToV1IfPure(upgraded); + expect(stored.version).toBe("v1"); + // The stored v1 row must not carry the synthesized placement fields. + expect(JSON.parse(serializeWorkflowIr(stored))).not.toHaveProperty("columns"); + }); + + it("round-trips a downgraded graph back to an IDENTICAL runtime graph", () => { + // This is the property that makes the downgrade safe to keep and safe to have + // always applied: what the runtime sees is unchanged by the stored shape. + const upgraded = parseWorkflowIr({ + version: "v1", + name: "pure", + nodes: [ + { id: "start", kind: "start" }, + { id: "n1", kind: "prompt", config: { prompt: "hi" } }, + { id: "end", kind: "end" }, + ], + edges: [ + { from: "start", to: "n1" }, + { from: "n1", to: "end" }, + ], + }); + const rehydrated = parseWorkflowIr(JSON.parse(serializeWorkflowIr(downgradeIrToV1IfPure(upgraded)))); + expect(rehydrated).toEqual(upgraded); + }); + + it("keeps a graph with a CUSTOM column on v2", () => { + const ir = structuredClone(BUILTIN_CODING_WORKFLOW_IR) as WorkflowIrV2; + ir.columns.push({ id: "custom-hold", name: "Custom hold", traits: [] }); + const stored = downgradeIrToV1IfPure(ir); + expect(stored.version).toBe("v2"); + expect((stored as WorkflowIrV2).columns.map((c) => c.id)).toContain("custom-hold"); + }); +}); diff --git a/packages/core/src/store.ts b/packages/core/src/store.ts index 09c2b357e2..cd176ea636 100644 --- a/packages/core/src/store.ts +++ b/packages/core/src/store.ts @@ -2397,9 +2397,18 @@ Issue #2149 requires read-only type filtering to occur in the file-store before // never a raw column write, so capacity (KTD-10) and single transition authority // (KTD-3) are honored. Only consulted when `workflowColumns` flag is ON. - public async workflowColumnsFlagOn(): Promise { - return isWorkflowColumnsCompatibilityFlagEnabled(await this.getSettingsFast()); - } + /* + FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — R9): + `workflowColumnsFlagOn()` is DELETED — it has no callers left. Its six readers were + the three U5 reconciliation guards (now unconditional) and the three v1-IR + rollback-compat persistence sites (now unconditional, same stored bytes). + + The underlying `isWorkflowColumnsCompatibilityFlagEnabled` SURVIVES for now: it is + still read by `moves.ts` and `workflow-task-create-ops.ts`, and removing those reads + IS the U2b move-path convergence, which carries an equivalence-proof obligation. + Deleting this wrapper is what makes the remaining reads easy to enumerate: after + this change, every surviving read of the raw flag is on the move path. + */ public async listWorkflowOccupantTaskIds(workflowId: string, includeNullSelection: boolean): Promise { return listWorkflowOccupantTaskIdsImpl(this, workflowId, includeNullSelection); } @@ -2474,8 +2483,8 @@ Issue #2149 requires read-only type filtering to occur in the file-store before } /** Synchronous workflow-definition insert used by migration (U2/KTD-3). */ - public insertWorkflowDefinitionSync( input: WorkflowDefinitionInput, flagOn: boolean, ): WorkflowDefinition { - return insertWorkflowDefinitionSyncImpl(this, input, flagOn); + public insertWorkflowDefinitionSync( input: WorkflowDefinitionInput ): WorkflowDefinition { + return insertWorkflowDefinitionSyncImpl(this, input); } async migrateLegacyWorkflowSteps(): Promise<{ migrated: number; skipped: number; combinedWorkflowId?: string; }> { return migrateLegacyWorkflowStepsImpl(this); diff --git a/packages/core/src/task-store/lifecycle-ops.ts b/packages/core/src/task-store/lifecycle-ops.ts index 119f07058b..f333d6414d 100644 --- a/packages/core/src/task-store/lifecycle-ops.ts +++ b/packages/core/src/task-store/lifecycle-ops.ts @@ -1112,11 +1112,15 @@ export async function recoverStaleTransitionPendingImpl(store: TaskStore): Promi } export async function migrateLegacyWorkflowStepsImpl(store: TaskStore): Promise<{ migrated: number; skipped: number; combinedWorkflowId?: string; }> { - // Resolve async prerequisites BEFORE the synchronous transaction: the - // workflow-columns flag (for flag-aware persistence). The project default is - // re-read AFTER the transaction (compare-and-set) so a concurrently-set - // default is never clobbered. - const flagOn = await store.workflowColumnsFlagOn(); + /* + FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — R9, BEHAVIOUR-PRESERVING): + The workflow-columns flag read is DELETED. It was resolved here only to thread + "flag-aware persistence" into `insertWorkflowDefinitionSync`, where it chose + between the v2 shape and the pure-v1 downgrade. The flag is retired and always + false, so the downgrade arm was always taken; it is now unconditional inside the + insert and the parameter is gone. The project default is still re-read AFTER the + transaction (compare-and-set) so a concurrently-set default is never clobbered. + */ const result = store.db.transactionImmediate(() => { // Write lock is now held. Read the raw step rows directly (the cached, @@ -1152,7 +1156,6 @@ export async function migrateLegacyWorkflowStepsImpl(store: TaskStore): Promise< ir: fragmentIr, layout: layoutForIr(fragmentIr), }, - flagOn, ); store.db .prepare("UPDATE workflow_steps SET migrated_fragment_id = ?, updatedAt = ? WHERE id = ?") @@ -1174,7 +1177,6 @@ export async function migrateLegacyWorkflowStepsImpl(store: TaskStore): Promise< ir, layout: layoutForIr(ir), }, - flagOn, ); combinedWorkflowId = combined.id; } diff --git a/packages/core/src/task-store/project-store-ops.ts b/packages/core/src/task-store/project-store-ops.ts index 7b31b60441..ba705deac0 100644 --- a/packages/core/src/task-store/project-store-ops.ts +++ b/packages/core/src/task-store/project-store-ops.ts @@ -682,9 +682,24 @@ export function __setWorkflowDefinitionBeforeInsertForTesting( } export async function createWorkflowDefinitionImpl(store: TaskStore, input: WorkflowDefinitionInput,): Promise { - // Rollback compat (#1405): with the flag OFF, persist a pure-v1-equivalent - // graph in the v1 shape so a binary downgrade can still load the row. - const flagOnForCreate = await store.workflowColumnsFlagOn(); + /* + Rollback compat (#1405): persist a pure-v1-equivalent graph in the v1 shape so a + binary downgrade can still load the row. + + FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — R9, BEHAVIOUR-PRESERVING): + The `flagOnForCreate` read is DELETED and the downgrade is now unconditional — + which is what it already was. The ternary was `flagOn ? ir : downgrade(ir)`, and + `flagOn` reads the retired raw `experimentalFeatures.workflowColumns` key that no + production writer sets, so every real project has ALWAYS taken the downgrade arm. + Removing the branch changes no persisted byte; it deletes a flag read. + + The downgrade itself is KEPT deliberately. It is a compatibility affordance, not + cutover machinery: it only fires for a graph that is exactly equivalent to pure v1 + (default columns, default placements, no v2-only features), and `upgradeV1ToV2` + re-reads it into an identical v2 graph. Retiring it would break a binary downgrade + for no benefit, and stale binaries opening these databases is an observed event in + this project, not a hypothetical. + */ return store.withConfigLock(async () => { const name = input.name?.trim(); if (!name) throw new Error("Workflow name is required"); @@ -724,7 +739,7 @@ export async function createWorkflowDefinitionImpl(store: TaskStore, input: Work name: definition.name, description: definition.description, icon: definition.icon ?? null, - ir: (flagOnForCreate ? definition.ir : downgradeIrToV1IfPure(definition.ir)) as unknown as object, + ir: downgradeIrToV1IfPure(definition.ir) as unknown as object, layout: definition.layout as unknown as object, kind: definition.kind, createdAt: definition.createdAt, diff --git a/packages/core/src/task-store/workflow-definitions.ts b/packages/core/src/task-store/workflow-definitions.ts index abb63a16cb..47c4396f7b 100644 --- a/packages/core/src/task-store/workflow-definitions.ts +++ b/packages/core/src/task-store/workflow-definitions.ts @@ -356,7 +356,6 @@ export async function occupantsByColumnForWorkflowImpl(store: TaskStore, export function insertWorkflowDefinitionSyncImpl(store: TaskStore, input: WorkflowDefinitionInput, - flagOn: boolean, ): WorkflowDefinition { const name = input.name?.trim(); if (!name) throw new Error("Workflow name is required"); @@ -387,7 +386,7 @@ export function insertWorkflowDefinitionSyncImpl(store: TaskStore, definition.name, definition.description, definition.icon ?? null, - serializeWorkflowIr(flagOn ? definition.ir : downgradeIrToV1IfPure(definition.ir)), + serializeWorkflowIr(downgradeIrToV1IfPure(definition.ir)), JSON.stringify(definition.layout), definition.kind, definition.createdAt, diff --git a/packages/core/src/task-store/workflow-ops.ts b/packages/core/src/task-store/workflow-ops.ts index e32895b907..eb7e9a1d83 100644 --- a/packages/core/src/task-store/workflow-ops.ts +++ b/packages/core/src/task-store/workflow-ops.ts @@ -311,19 +311,13 @@ export async function updateWorkflowDefinitionImpl(store: TaskStore, id: string, description: next.description, icon: next.icon ?? null, /* - FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — deliberately NOT flipped here): - This is the v1-IR rollback-compat persistence decision (#1405), not an - enforcement gate — it chooses the STORED SHAPE of the graph so an older binary - could still load the row. It shared the U5 `flagOn` variable that this change - deletes, which is why it is spelled out separately now rather than silently - inheriting the flip: one flag read was feeding two unrelated decisions, so the - flag has more decision sites than call sites. - - Retiring the downgrade is a persistence-format change with a different blast - radius than a guard, and it needs its own round-trip evidence. Left reading the - raw flag, unchanged in behaviour, for a follow-up. + FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — R9, BEHAVIOUR-PRESERVING): + v1-IR rollback-compat persistence (#1405), now unconditional — which is what it + already was. The flag read it used to branch on is retired and always false, so + every real project has always taken the downgrade arm. See the fuller note on + the create path in `project-store-ops.ts`; the downgrade is kept on purpose. */ - ir: (await store.workflowColumnsFlagOn()) ? next.ir : downgradeIrToV1IfPure(next.ir), + ir: downgradeIrToV1IfPure(next.ir), layout: next.layout, updatedAt: next.updatedAt, }).where(eq(schema.project.workflows.id, id));