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

fix(core): shared workflow-IR resolver, prod branch persistence + pruning, latest-run JOIN, sweep reservation-leak fix (#1402 #1407 #1412 #1413)

Browse Source
This commit is contained in:
gsxdsm
2026-06-04 07:34:02 -07:00
parent 45ce189548
commit cfc9bb5fd5
15 changed files with 577 additions and 168 deletions
Download Patch File Download Diff File Expand all files Collapse all files

31
packages/engine/src/__tests__/hold-release.test.ts
View File

@@ -164,27 +164,18 @@ describe("hold-release sweep (U6)", () => {
// (b) Reservation accounting across the racing sweeps.
//
// PRODUCTION BUG CAPTURED HERE (report only — prod is owned by another agent):
// The desired safety invariant is `reserveCount - releaseCount <= 1` (at most
// one live reservation, backing the single occupant). Under two overlapping
// sweeps with one held card + one slot, that invariant is VIOLATED: both
// sweeps read the same snapshot, both pass the pre-check, both reserve a slot
// (reserveCount === 2), and BOTH moveTask calls succeed — the second is an
// idempotent same-column move (todo→in-progress on an already-released card)
// whose in-txn capacity count includes the card as its own occupant, so it
// never throws capacity-exhausted and `issueRelease` never calls
// reservation.release(). Result: releaseCount === 0, leaking the loser's
// semaphore/worktree reservation.
//
// We assert the OBSERVED (leaking) behavior so the suite stays green while the
// leak is documented. Tighten this to `<= 1` once the prod fix lands (e.g.
// re-read the card's column inside issueRelease and skip/release when it is
// already at target).
// Both sweeps read the same snapshot, both pass the pre-check, and both
// reserve a slot (reserveCount === 2). The winning sweep commits the move;
// the losing sweep, after acquiring its reservation, re-reads the card's
// current column inside `issueRelease`, sees it already at the target (the
// winner moved it), and releases its reservation without issuing a redundant
// same-column move. The safety invariant therefore holds: at most one live
// reservation backs the single occupant.
expect(reserveCount).toBe(2);
expect(releaseCount).toBe(0);
// The net leaked reservations (2) is the bug; single board occupancy (asserted
// above) is still preserved, so no double card placement occurs.
expect(reserveCount - releaseCount).toBe(2);
// The loser releases its reservation, so the net live reservations is exactly
// one (the winner's), backing the single in-progress occupant — no leak.
expect(releaseCount).toBe(1);
expect(reserveCount - releaseCount).toBe(1);
});
it("sweep release into a full column is rejected by the in-txn check (capacity is not a guard, scheduler bypasses guards)", async () => {

62
packages/engine/src/__tests__/workflow-graph-task-runner.test.ts
View File

@@ -242,4 +242,66 @@ describe("WorkflowGraphTaskRunner (CU-U2)", () => {
const result = await runner.run(task, flagOn);
expect(result.disposition).toBe("completed");
});
// #1407/#1412: the runner forwards its injected branchPersistence into the
// WorkflowGraphExecutor, which writes per-branch state and prunes stale runs.
// Uses a real in-memory persistence whose method shape matches the store-
// backed adapter the production executor builds (saveBranchState /
// loadBranchStates / clearStaleBranchStates) — no mock of a nonexistent API.
function fanoutIr(): WorkflowIr {
return {
version: "v1",
name: "fanout",
nodes: [
{ id: "start", kind: "start" },
{ id: "split", kind: "split" },
{ id: "a", kind: "prompt", config: { prompt: "a" } },
{ id: "b", kind: "prompt", config: { prompt: "b" } },
{ id: "join", kind: "join", config: { mode: "all" } },
{ id: "zend", kind: "end" },
],
edges: [
{ from: "start", to: "split" },
{ from: "split", to: "a" },
{ from: "split", to: "b" },
{ from: "a", to: "join" },
{ from: "b", to: "join" },
{ from: "join", to: "zend", condition: "success" },
],
};
}
it("forwards branchPersistence to the executor: writes branch state and prunes stale runs", async () => {
const saved: Array<{ branchId: string; currentNodeId: string; status: string }> = [];
const pruneCalls: Array<{ taskId: string; keepRunId: string }> = [];
const persistence = {
saveBranchState: (s: { branchId: string; currentNodeId: string; status: string }) => {
saved.push({ branchId: s.branchId, currentNodeId: s.currentNodeId, status: s.status });
},
loadBranchStates: () => [],
clearStaleBranchStates: (taskId: string, keepRunId: string) => {
pruneCalls.push({ taskId, keepRunId });
},
};
const runner = new WorkflowGraphTaskRunner({
store: storeWith(definition(fanoutIr())),
seams: recordingSeams([]),
runCustomNode: async () => ({ outcome: "success" }),
branchPersistence: persistence,
});
const result = await runner.run(task, flagOn);
expect(result.disposition).toBe("completed");
// Both branches persisted, and each reached "completed" at the join.
expect(saved.some((s) => s.branchId === "a")).toBe(true);
expect(saved.some((s) => s.branchId === "b")).toBe(true);
expect(saved.some((s) => s.status === "completed")).toBe(true);
// Prune ran (on start AND completion) keyed by the runner's runId.
expect(pruneCalls.length).toBeGreaterThanOrEqual(2);
expect(pruneCalls.every((c) => c.taskId === task.id)).toBe(true);
expect(pruneCalls.every((c) => c.keepRunId === `${task.id}:WF-001`)).toBe(true);
});
});

28
packages/engine/src/executor.ts
View File

@@ -17,6 +17,7 @@ import {
type WorkflowRunObservation,
} from "@fusion/core";
import { WorkflowGraphTaskRunner, type WorkflowGraphTaskRunResult } from "./workflow-graph-task-runner.js";
import type { WorkflowBranchPersistence, WorkflowBranchRunState } from "./workflow-graph-branches.js";
import { observeWorkflowParity, WORKFLOW_INTERPRETER_DUAL_OBSERVE_FLAG } from "./workflow-parity-observer.js";
import type { WorkflowLegacySeams } from "./workflow-node-handlers.js";
import type { WorkflowNodeResult } from "./workflow-graph-executor.js";
@@ -3259,6 +3260,12 @@ export class TaskExecutor {
seams: this.createGraphSeams(settings),
runCustomNode: (node, nodeTask) => this.runGraphCustomNode(node, nodeTask, settings),
onEvent: (event) => executorLog.log(`[workflow-graph] ${event.type} ${event.taskId}: ${event.detail}`),
// Wire SQLite-backed per-branch persistence in production (#1407): the
// executor writes each branch's currentNodeId/status to
// workflow_run_branches so fan-out crash-resume and the U9 badges have
// real data, and prunes stale runs (#1412). Adapter degrades to no-op
// when the store predates these methods (additive guard).
branchPersistence: this.buildBranchPersistence(),
});
let result: WorkflowGraphTaskRunResult;
try {
@@ -3285,6 +3292,27 @@ export class TaskExecutor {
}
}
/**
* Build the store-backed WorkflowBranchPersistence wired into production
* fan-out runs (#1407/#1412). Returns undefined when the store predates the
* persistence methods (older embedded DBs) so the runner stays fully
* in-memory — purely additive. Each adapter method is itself guarded so a
* mixed/partial store never throws into the run.
*/
private buildBranchPersistence(): WorkflowBranchPersistence | undefined {
const store = this.store as unknown as {
saveWorkflowRunBranch?: (state: WorkflowBranchRunState) => void;
loadWorkflowRunBranches?: (taskId: string, runId: string) => WorkflowBranchRunState[];
clearWorkflowRunBranches?: (taskId: string, keepRunId: string) => void;
};
if (typeof store.saveWorkflowRunBranch !== "function") return undefined;
return {
saveBranchState: (state) => store.saveWorkflowRunBranch?.(state),
loadBranchStates: (taskId, runId) => store.loadWorkflowRunBranches?.(taskId, runId) ?? [],
clearStaleBranchStates: (taskId, keepRunId) => store.clearWorkflowRunBranches?.(taskId, keepRunId),
};
}
/**
* Dual-observe parity (CU-U5): for a workflow-selected task, compare the
* selected graph's routing against the legacy authoritative run for the SAME

78
packages/engine/src/hold-release.ts
View File

@@ -43,10 +43,7 @@ import {
resolveColumnAdjacency,
DEFAULT_WORKFLOW_POOL_ID,
TransitionRejectionError,
BUILTIN_CODING_WORKFLOW_IR,
getBuiltinWorkflow,
isBuiltinWorkflowId,
parseWorkflowIr,
resolveWorkflowIrForTask,
type TaskStore,
type Task,
type WorkflowIr,
@@ -92,40 +89,10 @@ export interface HoldReleaseResult {
held: Array<{ taskId: string; reason: string }>;
}
// ── Workflow IR resolution (read-only, mirrors store + merge-trait) ───────────
async function resolveTaskWorkflowIr(
store: TaskStore,
taskId: string,
// Optional per-sweep cache keyed by workflowId so each distinct workflow's IR
// is resolved (and its definition fetched) at most once per sweep.
irCache?: Map<string, WorkflowIr>,
): Promise<WorkflowIr> {
let workflowId: string | undefined;
try {
workflowId = store.getTaskWorkflowSelection(taskId)?.workflowId;
} catch {
workflowId = undefined;
}
if (!workflowId) return BUILTIN_CODING_WORKFLOW_IR;
const cached = irCache?.get(workflowId);
if (cached) return cached;
if (isBuiltinWorkflowId(workflowId)) {
const builtin = getBuiltinWorkflow(workflowId);
const ir = builtin?.ir ?? BUILTIN_CODING_WORKFLOW_IR;
irCache?.set(workflowId, ir);
return ir;
}
try {
const def = await store.getWorkflowDefinition(workflowId);
if (!def) return BUILTIN_CODING_WORKFLOW_IR;
const ir = typeof def.ir === "string" ? parseWorkflowIr(def.ir) : def.ir;
irCache?.set(workflowId, ir);
return ir;
} catch {
return BUILTIN_CODING_WORKFLOW_IR;
}
}
// ── Workflow IR resolution (read-only) ────────────────────────────────────────
// The selection → builtin/custom → default rule lives in @fusion/core's
// resolveWorkflowIrForTask (GitHub #1402); the optional per-sweep irCache Map is
// threaded straight through.
function effectiveWorkflowId(store: TaskStore, taskId: string): string {
try {
@@ -220,7 +187,7 @@ function legacyDependencySatisfied(dep: Task): boolean {
* audit-diff event is logged.
*/
async function dependencySatisfied(store: TaskStore, dep: Task): Promise<boolean> {
const ir = await resolveTaskWorkflowIr(store, dep.id);
const ir = await resolveWorkflowIrForTask(store, dep.id);
const column = findColumn(ir, dep.column);
const completeFlag = column ? resolveColumnFlags(column).complete === true : false;
@@ -358,7 +325,7 @@ export async function runHoldReleaseSweep(
continue;
}
const ir = await resolveTaskWorkflowIr(store, task.id, irCache);
const ir = await resolveWorkflowIrForTask(store, task.id, irCache);
if (!isHeldTask(ir, task)) continue;
const column = findColumn(ir, task.column);
@@ -452,14 +419,37 @@ async function issueRelease(
}
}
// A concurrent sweep (or explicit promote) can win the move for this same card
// while we hold a reservation. The store serializes the move under a per-task
// lock and resolves a redundant same-column move to a silent no-op: it returns
// the card already at the target WITHOUT re-allocating a slot or emitting a
// `task:moved`. A snapshot/pre-read can't tell winner from loser (both reads
// race ahead of either commit on the per-task lock). Instead we attribute the
// transition by OBJECT IDENTITY: a real move emits `task:moved` with the very
// Task object it then returns, whereas a no-op returns a freshly-read object
// and emits nothing. So the call whose `moveTask` result IS the emitted task is
// the real mover; any other call that reserved performed a redundant no-op and
// must release the slot it grabbed (FN-1415).
const movedTaskObjects = new Set<object>();
const onMoved = (data: { task: object; to: string }): void => {
if (data.to === target) movedTaskObjects.add(data.task);
};
store.on("task:moved", onMoved);
try {
await store.moveTask(task.id, target, {
const result = await store.moveTask(task.id, target, {
moveSource: "scheduler",
allocateWorktree:
targetIsProcessing && deps.allocateWorktree
? (reservedNames) => deps.allocateWorktree!(task, reservedNames)
: undefined,
});
if (reservation && !movedTaskObjects.has(result)) {
// Same-column no-op: a racing sweep already moved this card to the target.
reservation.release();
schedulerLog.log(`Hold release for ${task.id} skipped — already at ${target} (racing sweep won)`);
return false;
}
return true;
} catch (error) {
if (error instanceof TransitionRejectionError && error.rejection.code === "capacity-exhausted") {
@@ -474,6 +464,8 @@ async function issueRelease(
`Hold release for ${task.id} into ${target} failed: ${error instanceof Error ? error.message : String(error)}`,
);
return false;
} finally {
store.off("task:moved", onMoved);
}
}
@@ -495,7 +487,7 @@ export async function promoteHeldTask(
const task = await store.getTask(taskId);
if (!task) return { released: false, rejection: "task-not-found" };
const ir = await resolveTaskWorkflowIr(store, taskId);
const ir = await resolveWorkflowIrForTask(store, taskId);
if (!isHeldTask(ir, task)) {
return { released: false, rejection: "not-held" };
}
@@ -527,7 +519,7 @@ export async function releaseHeldTaskByEvent(
const task = await store.getTask(taskId);
if (!task) return { released: false, rejection: "task-not-found" };
const ir = await resolveTaskWorkflowIr(store, taskId);
const ir = await resolveWorkflowIrForTask(store, taskId);
const column = findColumn(ir, task.column);
const holdConfig = column ? resolveHoldConfig(column) : undefined;
if (!column || !holdConfig || holdConfig.release !== "external-event") {

41
packages/engine/src/merge-trait.ts
View File

@@ -38,12 +38,9 @@
*/
import {
BUILTIN_CODING_WORKFLOW_IR,
getBuiltinWorkflow,
isBuiltinWorkflowId,
isWorkflowColumnsEnabled,
parseWorkflowIr,
registerTraitHookImpl,
resolveWorkflowIrForTask,
type DirectMergeCommitStrategy,
type Settings,
type Task,
@@ -83,37 +80,9 @@ export interface ResolvedMergePolicy {
}
// ── Workflow IR resolution (read-only, flag-gated) ───────────────────────────
/**
* Resolve the task's workflow IR. Mirrors the store's private
* `resolveTaskWorkflowIrSync` resolution rule (selection → builtin/custom →
* default) but stays read-only and engine-side. A missing/corrupt definition
* degrades to the default workflow so policy resolution never throws.
*/
async function resolveTaskWorkflowIr(store: TaskStore, taskId: string): Promise<WorkflowIr> {
let workflowId: string | undefined;
try {
workflowId = store.getTaskWorkflowSelection(taskId)?.workflowId;
} catch {
workflowId = undefined;
}
if (!workflowId) return BUILTIN_CODING_WORKFLOW_IR;
if (isBuiltinWorkflowId(workflowId)) {
const builtin = getBuiltinWorkflow(workflowId);
return builtin?.ir ?? BUILTIN_CODING_WORKFLOW_IR;
}
try {
const def = await store.getWorkflowDefinition(workflowId);
if (!def) return BUILTIN_CODING_WORKFLOW_IR;
// `def.ir` is already a parsed WorkflowIr; reparse defensively only if a
// raw string ever slips through.
return typeof def.ir === "string" ? parseWorkflowIr(def.ir) : def.ir;
} catch {
return BUILTIN_CODING_WORKFLOW_IR;
}
}
// The selection → builtin/custom → default rule is shared via @fusion/core's
// resolveWorkflowIrForTask (GitHub #1402); a missing/corrupt definition degrades
// to the default workflow so policy resolution never throws.
/** Find the column the task currently sits in (by id). */
function findColumn(ir: WorkflowIr, columnId: string): WorkflowIrColumn | undefined {
@@ -178,7 +147,7 @@ export async function resolveMergePolicy(
let config: Record<string, unknown> | undefined;
try {
const ir = await resolveTaskWorkflowIr(store, task.id);
const ir = await resolveWorkflowIrForTask(store, task.id);
config = readMergeTraitConfig(findColumn(ir, task.column));
} catch {
config = undefined;

37
packages/engine/src/plugin-runner.ts
View File

@@ -36,10 +36,7 @@ import { Type } from "@earendil-works/pi-ai";
import { isAbsolute } from "node:path";
import {
getTraitRegistry,
parseWorkflowIr,
BUILTIN_CODING_WORKFLOW_IR,
getBuiltinWorkflow,
isBuiltinWorkflowId,
resolveWorkflowIrForTask,
} from "@fusion/core";
import { createLogger, executorLog } from "./logger.js";
import type { WorkflowCustomNodeRunner } from "./workflow-node-handlers.js";
@@ -539,33 +536,13 @@ export class PluginRunner {
}
/**
* Resolve a task's workflow IR through the public store API (selection +
* workflow definition). Mirrors the store's private resolver but stays on the
* public surface so the adapter never reaches into store internals. Falls back
* to the built-in default workflow on any miss.
* Resolve a task's workflow IR through the shared @fusion/core resolver
* (selection → builtin/custom → default fallback) on the public store surface
* — the adapter never reaches into store internals (GitHub #1402; previously a
* divergent raw-SQL copy via getDatabase()).
*/
private resolveTaskWorkflowIr(taskId: string): WorkflowIr | undefined {
const store = this.options.taskStore;
let workflowId: string | undefined;
try {
workflowId = store.getTaskWorkflowSelection?.(taskId)?.workflowId;
} catch {
return BUILTIN_CODING_WORKFLOW_IR;
}
if (!workflowId) return BUILTIN_CODING_WORKFLOW_IR;
if (isBuiltinWorkflowId(workflowId)) {
return getBuiltinWorkflow(workflowId)?.ir ?? BUILTIN_CODING_WORKFLOW_IR;
}
try {
const db = store.getDatabase();
const row = db.prepare("SELECT ir FROM workflows WHERE id = ?").get(workflowId) as
| { ir: string }
| undefined;
if (!row) return BUILTIN_CODING_WORKFLOW_IR;
return parseWorkflowIr(row.ir);
} catch {
return BUILTIN_CODING_WORKFLOW_IR;
}
private resolveTaskWorkflowIr(taskId: string): Promise<WorkflowIr> {
return resolveWorkflowIrForTask(this.options.taskStore, taskId);
}
getPluginWorkflowStepTemplates(): Array<{ pluginId: string; template: WorkflowStepTemplate }> {

7
packages/engine/src/plugin-trait-adapter.ts
View File

@@ -210,8 +210,9 @@ export class PluginTraitHasDependentsError extends Error {
*/
export async function findLivePluginTraitDependents(params: {
store: Pick<TaskStore, "listTasks">;
/** Resolve the (already-parsed) workflow IR for a task id. */
resolveTaskWorkflowIr: (taskId: string) => WorkflowIr | undefined;
/** Resolve the (already-parsed) workflow IR for a task id. May resolve
* asynchronously (the shared @fusion/core resolver awaits the definition). */
resolveTaskWorkflowIr: (taskId: string) => WorkflowIr | undefined | Promise<WorkflowIr | undefined>;
/** The registry ids of the plugin's traits to check for. */
pluginTraitIds: string[];
}): Promise<PluginTraitDependent[]> {
@@ -222,7 +223,7 @@ export async function findLivePluginTraitDependents(params: {
const dependents: PluginTraitDependent[] = [];
const tasks = await store.listTasks({ slim: true, includeArchived: false });
for (const task of tasks) {
const ir = resolveTaskWorkflowIr(task.id);
const ir = await resolveTaskWorkflowIr(task.id);
if (!ir) continue;
const column = findWorkflowColumn(ir, task.column);
if (!column) continue;

6
packages/engine/src/workflow-graph-branches.ts
View File

@@ -39,6 +39,12 @@ export interface WorkflowBranchPersistence {
saveBranchState?(state: WorkflowBranchRunState): void | Promise<void>;
/** Load any persisted branch states for a run (used on resume). */
loadBranchStates?(taskId: string, runId: string): WorkflowBranchRunState[] | Promise<WorkflowBranchRunState[]>;
/**
* Prune stale branch rows for a task, keeping only `keepRunId` (#1412).
* Called on run start and run completion to bound unbounded growth across a
* long-lived task's repeated runs.
*/
clearStaleBranchStates?(taskId: string, keepRunId: string): void | Promise<void>;
}
/**

17
packages/engine/src/workflow-graph-executor.ts
View File

@@ -119,6 +119,11 @@ export class WorkflowGraphExecutor {
);
}
// Prune prior-run branch rows on run start (#1412). Done after the resume
// load so this run's own (taskId, runId) rows survive while every stale run
// is removed. Never throws into the run.
await this.pruneStaleBranches(task.id, runId);
// Shared branch environment: built lazily so the sequential path pays nothing.
const branchEnv = (): BranchEnvironment => ({
task,
@@ -206,6 +211,9 @@ export class WorkflowGraphExecutor {
};
const terminal = await walk(startNode.id);
// Prune again on run completion (#1412): keeps only this run's rows so the
// table does not accumulate historical runs for a long-lived task.
await this.pruneStaleBranches(task.id, runId);
return {
executed: true,
outcome: terminal.outcome,
@@ -214,6 +222,15 @@ export class WorkflowGraphExecutor {
};
}
/** Best-effort prune of stale-run branch rows; never throws into the run. */
private async pruneStaleBranches(taskId: string, keepRunId: string): Promise<void> {
try {
await this.deps.branchPersistence?.clearStaleBranchStates?.(taskId, keepRunId);
} catch {
// Pruning is additive bookkeeping — a failure must not affect the run.
}
}
private shouldTraverseEdge(edge: WorkflowIrEdge, sourceResult: WorkflowNodeResult): boolean {
if (!edge.condition) return sourceResult.outcome === "success";
if (edge.condition === "success") return sourceResult.outcome === "success";