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

FN-6644: preserve finalized graph aborts

Browse Source 
Keep no-commit completion handoffs in review when teardown reclassifies their abort provenance.

- Track completed finalize-to-review handoffs with a durable executor marker.
- Suppress false operator-action graph failures after hard-cancel teardown overwrites completion-finalize provenance.
- Cover preserved user/global pause, merge-seam, hard-cancel, terminal, and redispatch cleanup behavior.
- Document the finalized completion abort exception and add a patch changeset.

Files changed:
 .changeset/fn-6644-finalize-to-review-abort-overwrite.md  |   5 +
 docs/architecture.md                                      |   2 +-
 packages/engine/src/__tests__/executor-recovery.test.ts   | 300 +++++++++++++++++++++
 packages/engine/src/executor.ts                           |  40 ++-
 4 files changed, 342 insertions(+), 5 deletions(-)

Fusion-Task-Id: FN-6644
Fusion-Task-Lineage: 569fa84f-2cbe-45ae-b12c-874dd45cea73
This commit is contained in:
gsxdsm
2026-06-18 11:18:03 -07:00
parent b1e57d38a0
commit 3b32b535d0
4 changed files with 342 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/fn-6644-finalize-to-review-abort-overwrite.md Normal file
View File

@@ -0,0 +1,5 @@
---
"@runfusion/fusion": patch
---
Completed/no-commit executions that finalize to review no longer get re-parked failed when later teardown overwrites completion-finalize abort provenance with a hard-cancel marker. Genuine user/global pauses, merge-seam retry routing, and active-execution hard-cancel behavior are preserved.

2
docs/architecture.md
View File

@@ -1279,7 +1279,7 @@ The columns/traits track moved *board* policy (transitions, capacity, hold, merg
- A `parse-steps` node reads a workflow-declared **artifact** (PROMPT.md is just the default workflow's declared `step-source` artifact) and runs a registry **parser** (`step-headings`, `json-steps`, or a plugin-contributed parser) to write `Task.steps[]`. It is the only graph-side step-list writer and must dominate any `foreach`. Parsers fail closed to a routable `outcome:parse-error`.
- A `foreach(source:"task-steps")` node instantiates an inline template subgraph once per planned step, with `mode` (sequential/parallel) and `isolation` (shared/worktree) as explicit axes and per-instance run-state pinned + persisted for crash-safe resume.
- Resume-limbo graph failures are retried only through a narrow persisted counter (`Task.graphResumeRetryCount`, max 2). The executor classifies a failure as transient only when it happens immediately after the engine restart/unpause resume log marker, reports no graph `reason`, has no completed step progress, and the task has no durable `lastError`/`failureReason`; it clears transient `status`/`error`, logs the auto-retry, and schedules one more graph execution. Any explicit graph reason, completed step progress, durable task error, missing resume marker, or exhausted counter remains a genuine `status:"failed"` disposition and goes to review handoff, preserving the FN-5704 anti-loop contract.
- Paused graph exits are benign only while the task is still in `in-progress`; that is the user-pause/engine-pause state where preserving the pause without requeueing is intentional. If the graph reports a pause/abort exit after the task has already advanced to another live column (for example `in-review` after an unpause/resume race), `TaskExecutor.handleGraphFailure()` surfaces the boundary as operator-actionable failure evidence (`status:"failed"`/`error` when no failure is already present, plus a task-log entry) and does **not** move, rewind, or auto-merge the task. The exception is FN-6625 `completion-finalize` provenance: a completed/no-commit execution whose teardown abort arrives after `handoffTaskToReview(..., "paused-after-completion")` resolves as an already-advanced benign graph exit instead of being re-parked failed. `done` and `archived` remain terminal and keep their column/status, while existing failure details are preserved.
- Paused graph exits are benign only while the task is still in `in-progress`; that is the user-pause/engine-pause state where preserving the pause without requeueing is intentional. If the graph reports a pause/abort exit after the task has already advanced to another live column (for example `in-review` after an unpause/resume race), `TaskExecutor.handleGraphFailure()` surfaces the boundary as operator-actionable failure evidence (`status:"failed"`/`error` when no failure is already present, plus a task-log entry) and does **not** move, rewind, or auto-merge the task. The exception is completed/no-commit finalize-to-review teardown (FN-6625/FN-6644): once `handoffTaskToReview(..., "paused-after-completion")` records durable completion-finalized state, a trailing graph abort resolves as an already-advanced benign graph exit even if later teardown re-marked the abort provenance from `completion-finalize` to `hard-cancel`. Genuine `userPaused`/global-pause exits and active-execution hard-cancels still use the operator-action path. `done` and `archived` remain terminal and keep their column/status, while existing failure details are preserved.
- A `step-review` node surfaces reviewer verdicts (APPROVE/REVISE/RETHINK/UNAVAILABLE) as outcome edges; `rework` edges (the only legal graph cycles, bounded per instance) route REVISE/RETHINK back to `step-execute`, with RETHINK traversal triggering the reset seam.
- A `code` node runs sandboxed TypeScript (esbuild + child process, clamped timeout, no store handle) for arbitrary computed routing/field logic — the same trust tier as project-local script steps.

300
packages/engine/src/__tests__/executor-recovery.test.ts
View File

@@ -1148,6 +1148,58 @@ describe("TaskExecutor bounded recovery retries", () => {
expect(store.handoffToReview).not.toHaveBeenCalled();
});
it("treats completion-finalize graph exits as benign after teardown re-marks hard-cancel (FN-6644)", async () => {
const store = createMockStore();
const steps = [
{ name: "Preflight", status: "done" },
{ name: "Implement", status: "done" },
];
const task = {
id: "FN-001",
title: "Test",
description: "Test",
column: "in-progress",
status: undefined,
dependencies: [],
steps,
currentStep: 1,
log: [{ timestamp: new Date().toISOString(), action: "Execution paused after completion — finalizing to in-review" }],
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
} as Task;
store.getTask.mockResolvedValue({
...task,
column: "in-review",
paused: false,
userPaused: false,
status: undefined,
error: null,
});
const executor = new TaskExecutor(store, "/tmp/test", {});
(executor as any).markCompletionFinalized("FN-001");
await (executor as any).awaitAbortInFlightTaskWork("FN-001", "completion-finalize teardown after handoff");
expect((executor as any).pausedAbortProvenance.get("FN-001")).toBe("hard-cancel");
await (executor as any).handleGraphFailure(task, {
disposition: "failed",
outcome: "failure",
visitedNodeIds: ["execute"],
});
const messages = store.logEntry.mock.calls.map((call) => call[1]).join("\n");
expect(messages).toContain("Workflow graph run ended after task already advanced to 'in-review' — no further action needed");
expect(messages).not.toContain("engine abort during pause/resume");
expect(messages).not.toContain("operator action required");
expect(store.updateTask).not.toHaveBeenCalledWith(
"FN-001",
expect.objectContaining({ status: "failed" }),
expect.anything(),
);
expect(store.moveTask).not.toHaveBeenCalled();
expect(store.handoffToReview).not.toHaveBeenCalled();
});
it("surfaces genuine hard-cancel pausedAborted in-review graph exits as workflow failures", async () => {
const store = createMockStore();
const steps = [
@@ -1339,6 +1391,254 @@ describe("TaskExecutor bounded recovery retries", () => {
});
describe("completion-finalize hard-cancel overwrite classification (FN-6644)", () => {
/*
Surface Enumeration coverage:
- Classifier branch: the overwrite-sequence tests drive `handleGraphFailure` after durable completion-finalized state survives a later `hard-cancel` re-mark and assert the benign already-advanced branch.
- Provenance-overwrite ordering: each benign case calls `markCompletionFinalized(...)` first, then `awaitAbortInFlightTaskWork(...)`, proving `completion-finalize → hard-cancel` cannot re-park a finalized row failed.
- Abort provenance sources: hard-cancel overwrite is reproduced here; user pause/global pause/merge-seam companion tests prove their existing provenance categories still win.
- Both completion-finalize paths: production uses the same `markCompletionFinalized(...)` helper at the graceful-session-exit and finally-block `handoffTaskToReview(task, "paused-after-completion")` sites.
- Failed-node identity: benign overwrite coverage uses both `execute` and a non-execute node so the fix is keyed on durable completion state rather than node id.
- Column/progress states: finalized in-review and already-terminal done rows are benign; active in-progress hard-cancel remains pause-preserved; pending-step in-review hard-cancel remains operator-action failure.
- Data states: userPaused true, global-pause, hard-cancel overwrite after completion, genuine hard-cancel before completion, and merge-seam retry routing are asserted without weakening the already-status/error guard above.
- Shared hooks / cleanup sites: `clearPausedAborted(...)` and `execute(...)` clear the durable marker; the final test asserts a new dispatch drops stale suppression state.
- No leftover shells: durable completion state is in-memory only and is cleared on re-dispatch/backward cleanup, preventing a later genuine run from inheriting suppression.
*/
const makeCompletedTask = (overrides: Partial<Task> = {}) => ({
id: "FN-001",
title: "Test",
description: "Test",
column: "in-progress",
status: undefined,
dependencies: [],
steps: [
{ name: "Preflight", status: "done" },
{ name: "Verify", status: "done" },
],
currentStep: 1,
log: [{ timestamp: new Date().toISOString(), action: "Execution paused after completion — finalizing to in-review" }],
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
...overrides,
}) as Task;
it.each(["execute", "verifySentinel"] as const)(
"treats finalized-completion graph exits as benign after hard-cancel overwrite at node %s",
async (nodeId) => {
const store = createMockStore();
const task = makeCompletedTask();
store.getTask.mockResolvedValue({
...task,
column: "in-review",
paused: false,
userPaused: false,
status: undefined,
error: null,
});
const executor = new TaskExecutor(store, "/tmp/test", {});
(executor as any).markCompletionFinalized("FN-001");
await (executor as any).awaitAbortInFlightTaskWork("FN-001", "completion-finalize teardown after handoff");
expect((executor as any).pausedAbortProvenance.get("FN-001")).toBe("hard-cancel");
await (executor as any).handleGraphFailure(task, {
disposition: "failed",
outcome: "failure",
visitedNodeIds: [nodeId],
});
const messages = store.logEntry.mock.calls.map((call) => call[1]).join("\n");
expect(messages).toContain("Workflow graph run ended after task already advanced to 'in-review' — no further action needed");
expect(messages).not.toContain("engine abort during pause/resume");
expect(messages).not.toContain("operator action required");
expect(store.updateTask).not.toHaveBeenCalledWith(
"FN-001",
expect.objectContaining({ status: "failed" }),
expect.anything(),
);
expect(store.moveTask).not.toHaveBeenCalled();
},
);
it("keeps already-terminal finalized-completion rows benign after hard-cancel overwrite", async () => {
const store = createMockStore();
const task = makeCompletedTask();
store.getTask.mockResolvedValue({
...task,
column: "done",
paused: false,
userPaused: false,
status: undefined,
error: null,
});
const executor = new TaskExecutor(store, "/tmp/test", {});
(executor as any).markCompletionFinalized("FN-001");
await (executor as any).awaitAbortInFlightTaskWork("FN-001", "completion-finalize teardown after done handoff");
await (executor as any).handleGraphFailure(task, {
disposition: "failed",
outcome: "failure",
visitedNodeIds: ["execute"],
});
const messages = store.logEntry.mock.calls.map((call) => call[1]).join("\n");
expect(messages).toContain("Workflow graph run ended after task already advanced to 'done' — no further action needed");
expect(messages).not.toContain("operator action required");
expect(store.updateTask).not.toHaveBeenCalledWith(
"FN-001",
expect.objectContaining({ status: "failed" }),
expect.anything(),
);
expect(store.moveTask).not.toHaveBeenCalled();
});
it("preserves explicit user-pause parking even when durable completion state exists", async () => {
const store = createMockStore();
const task = makeCompletedTask();
store.getTask.mockResolvedValue({
...task,
column: "in-review",
paused: true,
userPaused: true,
status: undefined,
error: null,
});
const executor = new TaskExecutor(store, "/tmp/test", {});
(executor as any).markCompletionFinalized("FN-001");
await (executor as any).awaitAbortInFlightTaskWork("FN-001", "completion-finalize teardown after handoff");
await (executor as any).handleGraphFailure(task, {
disposition: "failed",
outcome: "failure",
visitedNodeIds: ["execute"],
});
const expectedMessage = "Workflow graph failure surfaced after paused explicit user pause in 'in-review' at node 'execute' — operator action required; retry or explicitly unpause/resume after inspecting the task";
expect(store.logEntry).toHaveBeenCalledWith("FN-001", expectedMessage, undefined, undefined);
expect(store.updateTask).toHaveBeenCalledWith("FN-001", { error: expectedMessage, status: "failed" }, undefined);
expect(store.moveTask).not.toHaveBeenCalled();
});
it("preserves global-pause parking even when durable completion state exists", async () => {
const store = createMockStore();
const task = makeCompletedTask();
store.getTask.mockResolvedValue({
...task,
column: "in-review",
paused: false,
userPaused: false,
status: undefined,
error: null,
});
const executor = new TaskExecutor(store, "/tmp/test", {});
(executor as any).markCompletionFinalized("FN-001");
(executor as any).markPausedAborted("FN-001", "global-pause");
await (executor as any).handleGraphFailure(task, {
disposition: "failed",
outcome: "failure",
visitedNodeIds: ["execute"],
});
const expectedMessage = "Workflow graph failure surfaced after paused global pause in 'in-review' at node 'execute' — operator action required; retry or explicitly unpause/resume after inspecting the task";
expect(store.updateTask).toHaveBeenCalledWith("FN-001", { error: expectedMessage, status: "failed" }, undefined);
expect(store.moveTask).not.toHaveBeenCalled();
});
it("preserves merge-seam retry routing when merge provenance coexists with stale durable completion state", async () => {
const store = createMockStore();
const task = makeCompletedTask();
store.getTask.mockResolvedValue({
...task,
column: "in-review",
paused: false,
userPaused: false,
status: undefined,
error: null,
mergeRetries: null,
});
const executor = new TaskExecutor(store, "/tmp/test", {});
const mergeRequester = vi.fn(async () => ({ merged: false, noOp: false, reason: "merge-conflict" }));
executor.setMergeRequester(mergeRequester as any);
(executor as any).markCompletionFinalized("FN-001");
(executor as any).markPausedAborted("FN-001", "merge-seam");
await (executor as any).handleGraphFailure(task, {
disposition: "failed",
outcome: "failure",
visitedNodeIds: ["requestMerge"],
});
const messages = store.logEntry.mock.calls.map((call) => call[1]).join("\n");
expect(messages).toContain("Workflow graph merge failure at node 'requestMerge' routed to bounded auto-merge retry after merge-seam abort");
expect(messages).not.toContain("operator action required");
expect(mergeRequester).toHaveBeenCalledWith("FN-001");
expect(store.updateTask).not.toHaveBeenCalledWith(
"FN-001",
expect.objectContaining({ status: "failed" }),
expect.anything(),
);
});
it("preserves genuine pending-step hard-cancel parking when no completion finalize occurred", async () => {
const store = createMockStore();
const task = makeCompletedTask({
steps: [{ name: "Preflight", status: "pending" }],
currentStep: 0,
log: [{ timestamp: new Date().toISOString(), action: "Resuming execution after unpause" }],
});
store.getTask.mockResolvedValue({
...task,
column: "in-review",
paused: false,
userPaused: false,
status: undefined,
error: null,
});
const executor = new TaskExecutor(store, "/tmp/test", {});
(executor as any).markPausedAborted("FN-001", "hard-cancel");
await (executor as any).handleGraphFailure(task, {
disposition: "failed",
outcome: "failure",
visitedNodeIds: ["execute"],
});
const expectedMessage = "Workflow graph failure surfaced after paused engine abort during pause/resume in 'in-review' at node 'execute' — operator action required; retry or explicitly unpause/resume after inspecting the task";
expect(store.updateTask).toHaveBeenCalledWith("FN-001", { error: expectedMessage, status: "failed" }, undefined);
expect(store.moveTask).not.toHaveBeenCalled();
});
it("clears durable completion state on new execution dispatch so suppression cannot leak across runs", async () => {
const store = createMockStore();
const task = makeCompletedTask({ steps: [{ name: "Preflight", status: "pending" }], currentStep: 0 });
store.getTask.mockResolvedValue({
...task,
column: "in-review",
paused: false,
userPaused: false,
status: undefined,
error: null,
});
const executor = new TaskExecutor(store, "/tmp/test", {
workflowAuthoritativeDispatch: async () => true,
});
(executor as any).markCompletionFinalized("FN-001");
await executor.execute(task);
(executor as any).markPausedAborted("FN-001", "hard-cancel");
await (executor as any).handleGraphFailure(task, {
disposition: "failed",
outcome: "failure",
visitedNodeIds: ["execute"],
});
const expectedMessage = "Workflow graph failure surfaced after paused engine abort during pause/resume in 'in-review' at node 'execute' — operator action required; retry or explicitly unpause/resume after inspecting the task";
expect((executor as any).completionFinalizedTaskIds.has("FN-001")).toBe(false);
expect(store.updateTask).toHaveBeenCalledWith("FN-001", { error: expectedMessage, status: "failed" }, undefined);
});
});
it("keeps genuine in-progress user pauses benign even with partial step progress", async () => {
const store = createMockStore();
const task = {

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

@@ -1487,6 +1487,11 @@ export class TaskExecutor {
* FN-6625 adds completion-finalize provenance for the FN-6614 symptom where a completed/no-commit execution already handed off to in-review, then a trailing graph abort looked like a pause/resume engine abort and re-parked the task failed. Completion-finalize is sibling provenance to FN-6568 merge-seam, not operator pause intent.
*/
private pausedAbortProvenance = new Map<string, "global-pause" | "merge-seam" | "hard-cancel" | "completion-finalize">();
/**
* FNXC:WorkflowLifecycle 2026-06-18-10:56:
* FN-6644 makes completed/no-commit finalize-to-review state durable beyond volatile pause provenance. FN-6641 showed FN-6625 was incomplete because teardown can re-mark `completion-finalize` as `hard-cancel`; this marker keeps the already-finalized handoff from being re-parked as an operator-action pause abort while preserving genuine live pauses and active hard-cancels.
*/
private completionFinalizedTaskIds = new Set<string>();
/** Tasks that had a dependency added mid-execution (abort + discard worktree). */
private depAborted = new Set<string>();
/** Tasks killed by stuck task detector. Value = shouldRequeue (budget not exhausted). */
@@ -1515,9 +1520,15 @@ export class TaskExecutor {
this.pausedAbortProvenance.set(taskId, provenance);
}
private markCompletionFinalized(taskId: string): void {
this.markPausedAborted(taskId, "completion-finalize");
this.completionFinalizedTaskIds.add(taskId);
}
private clearPausedAborted(taskId: string): void {
this.pausedAborted.delete(taskId);
this.pausedAbortProvenance.delete(taskId);
this.completionFinalizedTaskIds.delete(taskId);
}
private setActiveSession(taskId: string, sessionState: ActiveExecutorSessionState, worktreePath: string): void {
@@ -6440,12 +6451,26 @@ export class TaskExecutor {
const abortProvenance = this.pausedAbortProvenance.get(task.id);
const mergeSeamAborted = abortProvenance === "merge-seam";
const completionFinalizeAborted = abortProvenance === "completion-finalize";
// FNXC:WorkflowLifecycle 2026-06-17-23:39: A real live pause still parks even if stale provenance says completion-finalize; completed handoff rows are expected to be unpaused.
const completionFinalized = completionFinalizeAborted || this.completionFinalizedTaskIds.has(task.id);
/*
FNXC:WorkflowLifecycle 2026-06-17-23:39: A real live pause still parks even if stale provenance says completion-finalize; completed handoff rows are expected to be unpaused.
FNXC:WorkflowLifecycle 2026-06-18-10:57:
FN-6644: a completed/no-commit execution that already finalized to in-review must not be re-parked as an operator-action pause abort when later teardown overwrites FN-6625 `completion-finalize` provenance with `hard-cancel` (FN-6641). Only suppress the pause-abort branch for already-finalized, non-in-progress rows with no live user/global pause; active execution hard-cancel and genuine pause/global-pause still park or preserve exactly as before.
*/
const suppressFinalizedCompletionAbort = Boolean(
completionFinalized
&& live.column !== "in-progress"
&& !live.userPaused
&& live.paused !== true
&& abortProvenance !== "global-pause"
&& !mergeSeamAborted,
);
const genuinePauseAbort = Boolean(
live.userPaused
|| abortProvenance === "global-pause"
|| (live.paused && !mergeSeamAborted)
|| (pausedAborted && !mergeSeamAborted && !completionFinalizeAborted),
|| (pausedAborted && !mergeSeamAborted && !completionFinalizeAborted && !suppressFinalizedCompletionAbort),
);
if (genuinePauseAbort) {
/*
@@ -6671,6 +6696,7 @@ export class TaskExecutor {
}
async execute(task: Task): Promise<void> {
this.completionFinalizedTaskIds.delete(task.id);
// Workflow graph interpreter routing (cutover M-C): graph-selected tasks
// are orchestrated by the interpreter. The execute seam re-enters this
// method with a completion interceptor registered (which claims the task
@@ -8154,8 +8180,11 @@ export class TaskExecutor {
/*
FNXC:WorkflowLifecycle 2026-06-17-23:33:
FN-6625: the completed/no-commit handoff may dispose graph execution after the task is already in-review. Mark that abort as completion-finalize so a trailing FN-6614-style graph failure resolves benignly instead of looking like a user/global pause; FN-6568 uses the same provenance seam for merge aborts.
FNXC:WorkflowLifecycle 2026-06-18-10:58:
FN-6644/FN-6641: the graceful-session-exit handoff must also record durable completed-finalize state because a later teardown can re-mark the abort as `hard-cancel`. The classifier uses that durable handoff marker, not the volatile provenance alone, to keep completed no-commit tasks from being re-parked failed.
*/
this.markPausedAborted(task.id, "completion-finalize");
this.markCompletionFinalized(task.id);
await this.handoffTaskToReview(task, "paused-after-completion");
this.clearCompletedTaskWatchdog(task.id);
this.options.onComplete?.(task);
@@ -8705,8 +8734,11 @@ export class TaskExecutor {
/*
FNXC:WorkflowLifecycle 2026-06-17-23:33:
FN-6625: the completed/no-commit handoff may dispose graph execution after the task is already in-review. Mark that abort as completion-finalize so a trailing FN-6614-style graph failure resolves benignly instead of looking like a user/global pause; FN-6568 uses the same provenance seam for merge aborts.
FNXC:WorkflowLifecycle 2026-06-18-10:59:
FN-6644/FN-6641: the finally-block handoff must record durable completed-finalize state because a later teardown can overwrite provenance to `hard-cancel`. The classifier must still resolve that completed no-commit tail failure benignly without weakening genuine pause or active hard-cancel behavior.
*/
this.markPausedAborted(task.id, "completion-finalize");
this.markCompletionFinalized(task.id);
await this.handoffTaskToReview(task, "paused-after-completion");
this.options.onComplete?.(task);
} else {