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

U12 part 3: make the v1-IR persistence unconditional — after this, every raw-flag read is on the move path (U2b) (#2513)

Browse Source 
## U12 part 3 — every remaining raw-flag read is now on the move path

**Stacks on #2512** (shares a line in `workflow-ops.ts`). Merge that
first.

**Behaviour-preserving. Not a single persisted byte changes.**

### What changed

The three v1-IR rollback-compat persist sites (#1405) all read `flagOn ?
ir : downgradeIrToV1IfPure(ir)`, where `flagOn` came from the retired
raw `experimentalFeatures.workflowColumns` key. No production writer
sets it, so **every real project has always taken the downgrade arm**.
Removing the branch is a runtime no-op; it deletes three flag reads.

Sites: `createWorkflowDefinitionImpl`, `updateWorkflowDefinitionImpl`,
and `insertWorkflowDefinitionSyncImpl` — whose `flagOn` *parameter* is
gone too, along with the plumbing that resolved it in
`migrateLegacyWorkflowStepsImpl`.

With those gone, **`TaskStore.workflowColumnsFlagOn()` has no callers
and is deleted.** Its six readers were the three U5 guards (part 2) and
these three persist sites.

### The decision I made, and why I went the other way

I had this slice scoped as "retire the v1 downgrade." **I rejected
that.** It is a compatibility affordance, not cutover machinery: it
fires only for a graph exactly equivalent to pure v1 (default columns,
default placements, no v2-only features), and `upgradeV1ToV2` re-reads
it into an identical v2 graph, so the runtime never sees a difference.
Retiring it would break a binary downgrade for zero benefit — and stale
binaries opening these databases is an **observed event** in this
project, not a hypothetical.

So the slice became the strictly better version of itself: same three
flag reads removed, no compat surface touched.

### Why this matters for sequencing

`isWorkflowColumnsCompatibilityFlagEnabled` survives. It is still read
by `moves.ts:363` and by `workflow-task-create-ops.ts:351`'s move-policy
preflight that feeds it. Removing those reads **is** the U2b move-path
convergence with its equivalence-proof obligation.

The point of deleting the wrapper is that it makes the remainder
enumerable:

```
$ grep -rn isWorkflowColumnsCompatibilityFlagEnabled --include=*.ts packages/ | grep -v __tests__
packages/core/src/store.ts:38                      <- the definition
packages/core/src/task-store/moves.ts:9,363        <- U2b
packages/core/src/task-store/workflow-task-create-ops.ts:11,351  <- U2b (feeds moves.ts)
```

**Every surviving read is on the move path.** U2b deletes the definition
and the unit closes.

### On coverage — stated honestly

This change is behaviour-preserving, so it has **no revert-proof test**,
and I am not going to claim one. `flagOn ? ir : downgrade(ir)` with an
always-false flag *is* `downgrade(ir)`.

What needed a guard is the next edit someone is tempted to make —
deleting `downgradeIrToV1IfPure` as dead cutover machinery. New
`workflow-ir-v1-rollback-persistence.test.ts` fails if it is removed,
and pins the exact boundary: the built-in coding workflow (named columns
+ traits) stays v2; a pure-v1-equivalent graph stores as v1 without the
synthesized `columns`; a downgraded graph re-parses to an **identical**
runtime graph (the property that makes unconditional application safe);
a graph with a custom column stays v2.

### Verification

`pnpm test:gate` (307 + 10 + 71), `pnpm lint`, `pnpm verify:fast` (17
steps), typecheck green. Core workflow-named suites: 383 passed, 1
failed — `workflow-ir-settings.test.ts > moved-key catalog ...`
(`expected 10 to strictly equal 3`), which I confirmed fails identically
on a stashed clean tree. Pre-existing, unrelated. No Fusion instance
booted.


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Improved workflow persistence compatibility by consistently storing
pure v1-equivalent workflows in the compatible format.
* Preserved v2 workflows and custom column information when they are not
v1-equivalent.
* Retired obsolete feature-flag checks without changing stored workflow
or board behavior.

* **Tests**
* Added coverage for workflow version preservation, rollback-compatible
serialization, and custom columns.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-07-28 19:09:06 -07:00
committed by GitHub
parent 2e39763930
commit 063978c289
8 changed files with 141 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
.changeset/u12-unconditional-v1-persistence.md Normal file
View File

@@ -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).

4
packages/core/src/__tests__/workflow-definition-id-allocator-sync.test.ts
View File

@@ -51,10 +51,10 @@ describe("workflow definition id allocator (sync materialization path)", () => {
const created = insertWorkflowDefinitionSyncImpl(store, { const created = insertWorkflowDefinitionSyncImpl(store, {
name: "fresh workflow", name: "fresh workflow",
ir: BUILTIN_CODING_WORKFLOW_IR, ir: BUILTIN_CODING_WORKFLOW_IR,
}, true); });
expect(created.id).toBe("WF-003"); 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"); expect(second.id).toBe("WF-004");
}); });
}); });

83
packages/core/src/__tests__/workflow-ir-v1-rollback-persistence.test.ts Normal file
View File

@@ -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");
});
});

19
packages/core/src/store.ts
View File

@@ -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 // never a raw column write, so capacity (KTD-10) and single transition authority
// (KTD-3) are honored. Only consulted when `workflowColumns` flag is ON. // (KTD-3) are honored. Only consulted when `workflowColumns` flag is ON.
public async workflowColumnsFlagOn(): Promise<boolean> { /*
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<string[]> { public async listWorkflowOccupantTaskIds(workflowId: string, includeNullSelection: boolean): Promise<string[]> {
return listWorkflowOccupantTaskIdsImpl(this, workflowId, includeNullSelection); 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). */ /** Synchronous workflow-definition insert used by migration (U2/KTD-3). */
public insertWorkflowDefinitionSync( input: WorkflowDefinitionInput, flagOn: boolean, ): WorkflowDefinition { public insertWorkflowDefinitionSync( input: WorkflowDefinitionInput ): WorkflowDefinition {
return insertWorkflowDefinitionSyncImpl(this, input, flagOn); return insertWorkflowDefinitionSyncImpl(this, input);
} }
async migrateLegacyWorkflowSteps(): Promise<{ migrated: number; skipped: number; combinedWorkflowId?: string; }> { async migrateLegacyWorkflowSteps(): Promise<{ migrated: number; skipped: number; combinedWorkflowId?: string; }> {
return migrateLegacyWorkflowStepsImpl(this); return migrateLegacyWorkflowStepsImpl(this);

16
packages/core/src/task-store/lifecycle-ops.ts
View File

@@ -1112,11 +1112,15 @@ export async function recoverStaleTransitionPendingImpl(store: TaskStore): Promi
} }
export async function migrateLegacyWorkflowStepsImpl(store: TaskStore): Promise<{ migrated: number; skipped: number; combinedWorkflowId?: string; }> { 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 FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — R9, BEHAVIOUR-PRESERVING):
// re-read AFTER the transaction (compare-and-set) so a concurrently-set The workflow-columns flag read is DELETED. It was resolved here only to thread
// default is never clobbered. "flag-aware persistence" into `insertWorkflowDefinitionSync`, where it chose
const flagOn = await store.workflowColumnsFlagOn(); 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(() => { const result = store.db.transactionImmediate(() => {
// Write lock is now held. Read the raw step rows directly (the cached, // 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, ir: fragmentIr,
layout: layoutForIr(fragmentIr), layout: layoutForIr(fragmentIr),
}, },
flagOn,
); );
store.db store.db
.prepare("UPDATE workflow_steps SET migrated_fragment_id = ?, updatedAt = ? WHERE id = ?") .prepare("UPDATE workflow_steps SET migrated_fragment_id = ?, updatedAt = ? WHERE id = ?")
@@ -1174,7 +1177,6 @@ export async function migrateLegacyWorkflowStepsImpl(store: TaskStore): Promise<
ir, ir,
layout: layoutForIr(ir), layout: layoutForIr(ir),
}, },
flagOn,
); );
combinedWorkflowId = combined.id; combinedWorkflowId = combined.id;
} }

23
packages/core/src/task-store/project-store-ops.ts
View File

@@ -682,9 +682,24 @@ export function __setWorkflowDefinitionBeforeInsertForTesting(
} }
export async function createWorkflowDefinitionImpl(store: TaskStore, input: WorkflowDefinitionInput,): Promise<WorkflowDefinition> { export async function createWorkflowDefinitionImpl(store: TaskStore, input: WorkflowDefinitionInput,): Promise<WorkflowDefinition> {
// 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. Rollback compat (#1405): persist a pure-v1-equivalent graph in the v1 shape so a
const flagOnForCreate = await store.workflowColumnsFlagOn(); 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 () => { return store.withConfigLock(async () => {
const name = input.name?.trim(); const name = input.name?.trim();
if (!name) throw new Error("Workflow name is required"); if (!name) throw new Error("Workflow name is required");
@@ -724,7 +739,7 @@ export async function createWorkflowDefinitionImpl(store: TaskStore, input: Work
name: definition.name, name: definition.name,
description: definition.description, description: definition.description,
icon: definition.icon ?? null, 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, layout: definition.layout as unknown as object,
kind: definition.kind, kind: definition.kind,
createdAt: definition.createdAt, createdAt: definition.createdAt,

3
packages/core/src/task-store/workflow-definitions.ts
View File

@@ -356,7 +356,6 @@ export async function occupantsByColumnForWorkflowImpl(store: TaskStore,
export function insertWorkflowDefinitionSyncImpl(store: TaskStore, export function insertWorkflowDefinitionSyncImpl(store: TaskStore,
input: WorkflowDefinitionInput, input: WorkflowDefinitionInput,
flagOn: boolean,
): WorkflowDefinition { ): WorkflowDefinition {
const name = input.name?.trim(); const name = input.name?.trim();
if (!name) throw new Error("Workflow name is required"); if (!name) throw new Error("Workflow name is required");
@@ -387,7 +386,7 @@ export function insertWorkflowDefinitionSyncImpl(store: TaskStore,
definition.name, definition.name,
definition.description, definition.description,
definition.icon ?? null, definition.icon ?? null,
serializeWorkflowIr(flagOn ? definition.ir : downgradeIrToV1IfPure(definition.ir)), serializeWorkflowIr(downgradeIrToV1IfPure(definition.ir)),
JSON.stringify(definition.layout), JSON.stringify(definition.layout),
definition.kind, definition.kind,
definition.createdAt, definition.createdAt,

18
packages/core/src/task-store/workflow-ops.ts
View File

@@ -311,19 +311,13 @@ export async function updateWorkflowDefinitionImpl(store: TaskStore, id: string,
description: next.description, description: next.description,
icon: next.icon ?? null, icon: next.icon ?? null,
/* /*
FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — deliberately NOT flipped here): FNXC:WorkflowColumns 2026-07-28-00:00 (U12 — R9, BEHAVIOUR-PRESERVING):
This is the v1-IR rollback-compat persistence decision (#1405), not an v1-IR rollback-compat persistence (#1405), now unconditional — which is what it
enforcement gate — it chooses the STORED SHAPE of the graph so an older binary already was. The flag read it used to branch on is retired and always false, so
could still load the row. It shared the U5 `flagOn` variable that this change every real project has always taken the downgrade arm. See the fuller note on
deletes, which is why it is spelled out separately now rather than silently the create path in `project-store-ops.ts`; the downgrade is kept on purpose.
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.
*/ */
ir: (await store.workflowColumnsFlagOn()) ? next.ir : downgradeIrToV1IfPure(next.ir), ir: downgradeIrToV1IfPure(next.ir),
layout: next.layout, layout: next.layout,
updatedAt: next.updatedAt, updatedAt: next.updatedAt,
}).where(eq(schema.project.workflows.id, id)); }).where(eq(schema.project.workflows.id, id));