From d280fa6f54e1298020610f1563f5711f55289946 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 19 Aug 2026 19:12:28 -0700 Subject: [PATCH] FN-9166: preserve implementation-incomplete merge failures Keep structured incomplete-implementation failures intact so graph recovery can resume or fail closed without retrying a no-op merge. - Preserve normalized implementation-incomplete reasons during direct merge-attempt classification. - Cover primitive, legacy seam, resumable, fail-closed, and cancellation paths. - Document the merge-boundary invariant and add a patch changeset. Files changed: .../fn-9166-preserve-implementation-incomplete.md | 7 ++ docs/architecture.md | 2 +- .../merge-node-paused-abort-retryable.test.ts | 76 ++++++++++++++++++++++ .../__tests__/workflow-merge-cancellation.test.ts | 15 ++++- .../src/__tests__/workflow-merge-nodes.test.ts | 46 +++++++++++++ .../engine/src/workflows/workflow-merge-nodes.ts | 13 +++- 6 files changed, 155 insertions(+), 4 deletions(-) Fusion-Task-Id: FN-9166 Fusion-Task-Lineage: 1b7946bf-62e3-42f0-8c8c-09230fef22bb Co-authored-by: Fusion (runfusion.ai) --- ...9166-preserve-implementation-incomplete.md | 7 ++ docs/architecture.md | 2 +- .../merge-node-paused-abort-retryable.test.ts | 76 +++++++++++++++++++ .../workflow-merge-cancellation.test.ts | 15 +++- .../__tests__/workflow-merge-nodes.test.ts | 46 +++++++++++ .../src/workflows/workflow-merge-nodes.ts | 13 +++- 6 files changed, 155 insertions(+), 4 deletions(-) create mode 100644 .changeset/fn-9166-preserve-implementation-incomplete.md diff --git a/.changeset/fn-9166-preserve-implementation-incomplete.md b/.changeset/fn-9166-preserve-implementation-incomplete.md new file mode 100644 index 0000000000..cfa9fc9269 --- /dev/null +++ b/.changeset/fn-9166-preserve-implementation-incomplete.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Preserve incomplete implementation failures through workflow merge handling. +category: fix +dev: Keeps the implementation-incomplete merge-node value intact for graph recovery. diff --git a/docs/architecture.md b/docs/architecture.md index e58734a08f..29a665eb26 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -2427,4 +2427,4 @@ Scheduler and autopilot mission reconciliation persist the evaluated alignment o ## Workflow merge-boundary invariant -The bounded auto-merge retry must never repeat a merge-boundary check that has already reported missing proof. `merge-boundary-unproven` is terminal: the engine parks the task as failed, and `shouldHoldActiveFileScopeLease` releases its active file-scope lease because failed rows are not live work. Terminal merge values must survive `classifyMergePrimitiveResult` on both the collapsed synthetic `merge` seam and direct `merge-attempt` runner; do not encode a new terminal value solely as `data.status:"failed"` with an unrecognized reason, because that classifier collapses it to non-terminal `merge-failed`. +The bounded auto-merge retry must never repeat a merge-boundary check that has already reported missing proof. `merge-boundary-unproven` is terminal: the engine parks the task as failed, and `shouldHoldActiveFileScopeLease` releases its active file-scope lease because failed rows are not live work. `implementation-incomplete` is likewise a preserved structured failed reason, allowing its dedicated resumable/fail-closed graph route to run rather than repeating a merge request. Terminal merge values must survive `classifyMergePrimitiveResult` on both the collapsed synthetic `merge` seam and direct `merge-attempt` runner; do not encode a new terminal value solely as `data.status:"failed"` with an unrecognized reason, because that classifier collapses it to non-terminal `merge-failed`. diff --git a/packages/engine/src/__tests__/reliability-interactions/merge-node-paused-abort-retryable.test.ts b/packages/engine/src/__tests__/reliability-interactions/merge-node-paused-abort-retryable.test.ts index 20218d7170..c2fad02804 100644 --- a/packages/engine/src/__tests__/reliability-interactions/merge-node-paused-abort-retryable.test.ts +++ b/packages/engine/src/__tests__/reliability-interactions/merge-node-paused-abort-retryable.test.ts @@ -1,6 +1,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import "../executor-test-helpers.js"; import { TaskExecutor } from "../../executor.js"; +import { runWorkflowMergeAttemptNode } from "../../workflows/workflow-merge-nodes.js"; import { createMockStore, resetExecutorMocks } from "../executor-test-helpers.js"; import type { TaskDetail } from "@fusion/core"; @@ -75,6 +76,28 @@ function logText(store: ReturnType): string { return store.logEntry.mock.calls.map((call: unknown[]) => call[1]).join("\n"); } +async function produceImplementationIncompleteMergeNodeValue(task: TaskDetail): Promise { + const requestMerge = vi.fn().mockResolvedValue({ + outcome: "failure", + value: "implementation-incomplete", + data: { status: "failed", reason: "implementation-incomplete" }, + }); + const result = await runWorkflowMergeAttemptNode({ + primitives: { requestMerge, audit: vi.fn() }, + }, { + run: { runId: "run-implementation-incomplete", taskId: task.id, workflowId: "builtin:coding" }, + node: { node: { id: "merge-attempt", kind: "merge-attempt" } }, + }, task); + + expect(requestMerge).toHaveBeenCalledTimes(1); + expect(result).toMatchObject({ + outcome: "failure", + value: "implementation-incomplete", + contextPatch: { "workflow:merge-status": "implementation-incomplete" }, + }); + return result.value!; +} + describe("merge-node paused-abort retry classification (FN-6735)", () => { beforeEach(() => { resetExecutorMocks(); @@ -320,6 +343,59 @@ describe("merge-node paused-abort retry classification (FN-6735)", () => { "merge-retry", ] as const; + it.each(["merge-attempt", "merge"] as const)("routes primitive-produced implementation-incomplete no-proof failure at node %s without requesting no-op merge", async (nodeId) => { + const { store, task, executor, mergeRequester } = makeHarness({ + steps: [], + currentStep: 0, + branch: null, + worktree: null, + modifiedFiles: undefined, + workflowStepResults: undefined, + paused: false, + } as Partial); + (executor as any).addActiveWorktree(task.id, "/tmp/fusion-fn-9166-fail-closed"); + + const value = await produceImplementationIncompleteMergeNodeValue(task); + await invokeGraphFailure(executor, task, nodeId, value); + + expect(mergeRequester).not.toHaveBeenCalled(); + expect(store.updateTask).toHaveBeenCalledWith( + task.id, + expect.objectContaining({ + status: "failed", + error: expect.stringContaining("implementation incomplete with no executable proof to resume"), + }), + undefined, + ); + expect(logText(store)).toContain(`Workflow graph merge blocked at node '${nodeId}': implementation incomplete with no executable proof to resume — failing instead of retrying merge`); + expect((executor as any).activeWorktrees.has(task.id)).toBe(false); + }); + + it.each(["merge-attempt", "merge"] as const)("routes primitive-produced implementation-incomplete resumable failure at node %s without requesting merge", async (nodeId) => { + const worktreePath = "/tmp/fusion-fn-9166-resumable"; + const { store, task, executor, mergeRequester } = makeHarness({ + steps: [ + { name: "Preflight", status: "done" }, + { name: "Implement", status: "pending" }, + ], + currentStep: 1, + branch: "fusion/fn-9166-resumable", + worktree: worktreePath, + modifiedFiles: undefined, + workflowStepResults: undefined, + paused: false, + } as Partial); + (executor as any).addActiveWorktree(task.id, worktreePath); + + const value = await produceImplementationIncompleteMergeNodeValue(task); + await invokeGraphFailure(executor, task, nodeId, value); + + expect(mergeRequester).not.toHaveBeenCalled(); + expect(store.moveTask).toHaveBeenCalledWith(task.id, "todo", expect.objectContaining({ preserveProgress: true })); + expect(logText(store)).toContain(`Workflow graph failed at node '${nodeId}' (implementation-incomplete) with incomplete steps — moved back to todo for execution resume`); + expect((executor as any).getActiveWorktreePaths(task.id)).toEqual([worktreePath]); + }); + it.each(implementationIncompleteMergeNodes)("fails implementation-incomplete no-proof merge pause abort at node %s without requesting no-op merge", async (nodeId) => { const { store, task, executor, mergeRequester } = makeHarness({ steps: [], diff --git a/packages/engine/src/__tests__/workflow-merge-cancellation.test.ts b/packages/engine/src/__tests__/workflow-merge-cancellation.test.ts index 8a8499375a..0b44959622 100644 --- a/packages/engine/src/__tests__/workflow-merge-cancellation.test.ts +++ b/packages/engine/src/__tests__/workflow-merge-cancellation.test.ts @@ -22,8 +22,11 @@ import { createMockStore, mockedExistsSync, resetExecutorMocks } from "./executo const now = "2026-07-15T00:00:00.000Z"; -/** A task shaped to clear the merge boundary's implementation-proof gates, so the - * cancellation race — not a pre-flight rejection — is what the assertion observes. */ +/** + * FNXC:WorkflowCancellation 2026-08-20-01:20: + * FN-9157 requires terminal pre-merge evidence before a merge attempt reaches the requester. + * Keep this fixture merge-ready so cancellation, not boundary admission, is the observed contract. + */ function mergeReadyTask(overrides = {}) { return { id: "FN-CANCEL", @@ -38,6 +41,14 @@ function mergeReadyTask(overrides = {}) { branch: null, worktree: null, enabledWorkflowSteps: [], + workflowStepResults: [{ + workflowStepId: "execute", + workflowStepName: "Execute", + source: "node", + phase: "pre-merge", + status: "passed", + completedAt: now, + }], prompt: "# Task\n\n## Steps\n\n### Step 1: Decide\n- [ ] Record no-code decision", createdAt: now, updatedAt: now, diff --git a/packages/engine/src/__tests__/workflow-merge-nodes.test.ts b/packages/engine/src/__tests__/workflow-merge-nodes.test.ts index 88d4351d9a..e5c43caf3f 100644 --- a/packages/engine/src/__tests__/workflow-merge-nodes.test.ts +++ b/packages/engine/src/__tests__/workflow-merge-nodes.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi } from "vitest"; import type { TaskDetail } from "@fusion/core"; +import { createMergeAttemptHandler } from "../workflow-node-runners/merge-runner.js"; import { classifyMergePrimitiveResult, runWorkflowMergeAttemptNode } from "../workflows/workflow-merge-nodes.js"; import type { WorkflowPrimitiveContext } from "../execution/runtime-primitives.js"; @@ -31,6 +32,18 @@ describe("workflow merge nodes", () => { outcome: "failure", value: "file-scope-violation", }); + expect(classifyMergePrimitiveResult({ status: "failed", reason: "implementation-incomplete" }, undefined, "failure")).toEqual({ + outcome: "failure", + value: "implementation-incomplete", + }); + expect(classifyMergePrimitiveResult({ status: "failed", reason: " ImPlEmEnTaTiOn-InCoMpLeTe " }, undefined, "failure")).toEqual({ + outcome: "failure", + value: "implementation-incomplete", + }); + expect(classifyMergePrimitiveResult({ status: "failed", reason: "remote rejected" }, undefined, "failure")).toEqual({ + outcome: "failure", + value: "merge-failed", + }); expect(classifyMergePrimitiveResult({ status: "merged-requested" }, undefined, "failure")).toEqual({ outcome: "success", value: "merged-requested", @@ -47,6 +60,10 @@ describe("workflow merge nodes", () => { outcome: "success", value: "merged-requested", }); + expect(classifyMergePrimitiveResult(undefined, "implementation-incomplete", "failure")).toEqual({ + outcome: "failure", + value: "implementation-incomplete", + }); }); it("runs the existing merge primitive and emits a workflow capability audit event", async () => { @@ -71,6 +88,35 @@ describe("workflow merge nodes", () => { }); }); + it("preserves implementation-incomplete from a failed merge primitive in node context", async () => { + const audit = vi.fn(); + const requestMerge = vi.fn().mockResolvedValue({ + outcome: "failure", + value: "implementation-incomplete", + data: { status: "failed", reason: "implementation-incomplete" }, + }); + + await expect(runWorkflowMergeAttemptNode({ primitives: { requestMerge, audit } }, ctx, task)).resolves.toEqual({ + outcome: "failure", + value: "implementation-incomplete", + contextPatch: { "workflow:merge-status": "implementation-incomplete" }, + }); + }); + + it("preserves the legacy merge seam's implementation-incomplete value without primitives", async () => { + const merge = vi.fn().mockResolvedValue({ outcome: "failure", value: "implementation-incomplete" }); + const handler = createMergeAttemptHandler({ + seams: { merge }, + buildPrimitiveContext: vi.fn(), + }); + + await expect(handler({ id: "merge-attempt", kind: "merge-attempt" } as any, { + task, + settings: {}, + context: {}, + } as any)).resolves.toEqual({ outcome: "failure", value: "implementation-incomplete" }); + }); + it("does not retry the merge primitive when audit fails after classification", async () => { const audit = vi.fn().mockRejectedValue(new Error("audit unavailable")); const requestMerge = vi.fn().mockResolvedValue({ diff --git a/packages/engine/src/workflows/workflow-merge-nodes.ts b/packages/engine/src/workflows/workflow-merge-nodes.ts index 0e1ce6b465..d306459ef4 100644 --- a/packages/engine/src/workflows/workflow-merge-nodes.ts +++ b/packages/engine/src/workflows/workflow-merge-nodes.ts @@ -5,6 +5,8 @@ import type { WorkflowNodeResult } from "./workflow-graph-executor.js"; /** A terminal graph value: retrying cannot create missing merge-boundary proof. */ export const MERGE_BOUNDARY_UNPROVEN_VALUE = "merge-boundary-unproven"; +const PRESERVED_MERGE_FAILURE_REASONS = new Set(["implementation-incomplete"]); + export interface WorkflowMergeNodeDeps { primitives: Pick; } @@ -72,7 +74,16 @@ export function classifyMergePrimitiveResult( } function classifyMergeFailure(reason: string): WorkflowNodeResult { - const normalized = reason.toLowerCase(); + const normalized = reason.trim().toLowerCase(); + /* + FNXC:WorkflowMerge 2026-08-20-01:20: + implementation-incomplete must survive merge classification because handleGraphFailure, + routeGraphMergeFailureToRetry, and isRetryableBenignMergePauseAbort key on this literal. + Collapsing it to merge-failed reopens FN-1165's no-op-merge-proof hole through bounded retry. + */ + if (PRESERVED_MERGE_FAILURE_REASONS.has(normalized)) { + return { outcome: "failure", value: normalized }; + } if (normalized.includes("file scope") || normalized.includes("filescope")) { return { outcome: "failure", value: "file-scope-violation" }; }