diff --git a/.changeset/fn-9170-merge-unavailable.md b/.changeset/fn-9170-merge-unavailable.md new file mode 100644 index 0000000000..f69ad3f42b --- /dev/null +++ b/.changeset/fn-9170-merge-unavailable.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Preserve unavailable merge diagnostics across workflow merge dispatch paths. +category: fix +dev: Adds merge-unavailable to PRESERVED_MERGE_FAILURE_REASONS while deliberately keeping it non-terminal. diff --git a/docs/architecture.md b/docs/architecture.md index 29a665eb26..d5432f3f8b 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -2428,3 +2428,5 @@ 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. `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`. + +Structured merge sentinels emitted with `data.status:"failed"` are preserved verbatim by `PRESERVED_MERGE_FAILURE_REASONS`; its substring heuristics classify only free-text merge-requester reasons. The primitive `merge-attempt` runner and legacy `merge` seam must report the same literal. `merge-unavailable` is intentionally preserved but non-terminal: it is emitted only when no `mergeRequester` is wired, and the bounded retry route short-circuits on that same absence. Add a new structured sentinel to the preserved-reason set rather than relying on reason text. diff --git a/packages/engine/src/__tests__/merge-unavailable-classification.test.ts b/packages/engine/src/__tests__/merge-unavailable-classification.test.ts new file mode 100644 index 0000000000..be27ccc12b --- /dev/null +++ b/packages/engine/src/__tests__/merge-unavailable-classification.test.ts @@ -0,0 +1,113 @@ +import { describe, expect, it, vi } from "vitest"; +import type { TaskDetail } from "@fusion/core"; +import { createAuthoritativeWorkflowSeams } from "../executor/create-authoritative-workflow-seams.js"; +import { graphFailureValue, isMergeGraphFailure } from "../executor/graph-failure-pure.js"; +import { routeGraphMergeFailureToRetry } from "../executor/route-graph-merge-failure-to-retry.js"; +import { isTerminalMergeGraphFailureValue } from "../executor/task-predicates.js"; +import { createMergeAttemptHandler } from "../workflow-node-runners/merge-runner.js"; + +/* +FNXC:WorkflowMerge 2026-08-20-02:36: +FN-9170 reproduces the original merge-unavailable symptom through the real legacy seam and +runner before changing the classifier. The legacy branch is the executable baseline: no wired +merge requester preserves the literal, while the pre-fix primitive branch renames its failed-data +sentinel to merge-failed. +*/ + +const task = { id: "FN-9170" } as TaskDetail; +const node = { id: "merge-attempt", kind: "merge-attempt" } as any; +const context = { source: "merge-unavailable-regression" }; +const signal = new AbortController().signal; + +function createHandlerContext() { + return { + task, + settings: {}, + context, + signal, + } as any; +} + +describe("FN-9170 merge-unavailable dispatch classification", () => { + it("keeps the real legacy seam as the pre-fix dispatch baseline", async () => { + // The unavailable guard is the seam's first statement, so this narrow deps bag proves no later dependency is touched. + const ensureWorkflowMergeBoundaryTask = vi.fn(); + const realSeams = createAuthoritativeWorkflowSeams({ + mergeRequester: undefined, + ensureWorkflowMergeBoundaryTask, + } as any, {} as any); + const merge = vi.fn(realSeams.merge); + const buildPrimitiveContext = vi.fn(); + const handler = createMergeAttemptHandler({ + seams: { merge }, + buildPrimitiveContext, + }); + + await expect(handler(node, createHandlerContext())).resolves.toEqual({ + outcome: "failure", + value: "merge-unavailable", + }); + expect(merge).toHaveBeenCalledWith(task, context, signal); + expect(buildPrimitiveContext).not.toHaveBeenCalled(); + expect(ensureWorkflowMergeBoundaryTask).not.toHaveBeenCalled(); + }); + + it("keeps both real dispatch paths on the merge-unavailable literal", async () => { + const requestMerge = vi.fn().mockResolvedValue({ + outcome: "failure", + value: "merge-unavailable", + data: { status: "failed", reason: "merge-unavailable" }, + }); + const buildPrimitiveContext = vi.fn().mockReturnValue({ + run: { runId: "run-9170", taskId: task.id, workflowId: "builtin:coding" }, + node: { node }, + }); + const handler = createMergeAttemptHandler({ + primitives: { requestMerge, audit: vi.fn() } as any, + seams: { merge: vi.fn() }, + buildPrimitiveContext, + }); + + await expect(handler(node, createHandlerContext())).resolves.toEqual({ + outcome: "failure", + value: "merge-unavailable", + contextPatch: { "workflow:merge-status": "merge-unavailable" }, + }); + expect(requestMerge).toHaveBeenCalledTimes(1); + }); + + it("keeps merge-unavailable readable but non-terminal across merge graph node ids", () => { + for (const nodeId of ["merge-attempt", "merge"]) { + expect(graphFailureValue({ + visitedNodeIds: [nodeId], + context: { [`node:${nodeId}:value`]: "merge-unavailable" }, + } as any)).toBe("merge-unavailable"); + } + for (const nodeId of ["merge-attempt", "merge", "requestMerge", "merge-gate", "merge-retry", "merge-manual-hold"]) { + expect(isMergeGraphFailure(nodeId)).toBe(true); + } + expect(isTerminalMergeGraphFailureValue("merge-unavailable")).toBe(false); + }); + + it("does not route unavailable merge infrastructure to a retry", async () => { + const ensureWorkflowMergeBoundaryTask = vi.fn(); + const updateTask = vi.fn(); + const logEntry = vi.fn(); + + // This non-terminal literal is deliberate: the absent requester is the first retry guard. + await expect(routeGraphMergeFailureToRetry({ + store: { updateTask, logEntry } as any, + getRunContextFor: () => undefined, + mergeRequester: undefined, + ensureWorkflowMergeBoundaryTask, + persistTokenUsage: vi.fn(), + }, task, { + visitedNodeIds: ["merge-attempt"], + context: { "node:merge-attempt:value": "merge-unavailable" }, + } as any, undefined)).resolves.toBe(false); + + expect(ensureWorkflowMergeBoundaryTask).not.toHaveBeenCalled(); + expect(updateTask).not.toHaveBeenCalled(); + expect(logEntry).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/engine/src/__tests__/workflow-merge-nodes.test.ts b/packages/engine/src/__tests__/workflow-merge-nodes.test.ts index e5c43caf3f..717807eef9 100644 --- a/packages/engine/src/__tests__/workflow-merge-nodes.test.ts +++ b/packages/engine/src/__tests__/workflow-merge-nodes.test.ts @@ -32,6 +32,26 @@ describe("workflow merge nodes", () => { outcome: "failure", value: "file-scope-violation", }); + expect(classifyMergePrimitiveResult({ status: "failed", reason: "already on main" }, undefined, "failure")).toEqual({ + outcome: "success", + value: "already-landed", + }); + expect(classifyMergePrimitiveResult({ status: "failed", reason: "socket timeout" }, undefined, "failure")).toEqual({ + outcome: "success", + value: "transient-failure", + }); + expect(classifyMergePrimitiveResult({ status: "failed", reason: "merge conflict" }, undefined, "failure")).toEqual({ + outcome: "success", + value: "manual-required", + }); + expect(classifyMergePrimitiveResult({ status: "failed", reason: "merge-unavailable" }, "merge-unavailable", "failure")).toEqual({ + outcome: "failure", + value: "merge-unavailable", + }); + expect(classifyMergePrimitiveResult({ status: "failed", reason: " MeRgE-UnAvAiLaBlE " }, "merge-unavailable", "failure")).toEqual({ + outcome: "failure", + value: "merge-unavailable", + }); expect(classifyMergePrimitiveResult({ status: "failed", reason: "implementation-incomplete" }, undefined, "failure")).toEqual({ outcome: "failure", value: "implementation-incomplete", @@ -40,7 +60,7 @@ describe("workflow merge nodes", () => { outcome: "failure", value: "implementation-incomplete", }); - expect(classifyMergePrimitiveResult({ status: "failed", reason: "remote rejected" }, undefined, "failure")).toEqual({ + expect(classifyMergePrimitiveResult({ status: "failed", reason: "remote rejected: non-fast-forward" }, undefined, "failure")).toEqual({ outcome: "failure", value: "merge-failed", }); @@ -64,6 +84,10 @@ describe("workflow merge nodes", () => { outcome: "failure", value: "implementation-incomplete", }); + expect(classifyMergePrimitiveResult(undefined, "merge-unavailable", "failure")).toEqual({ + outcome: "failure", + value: "merge-unavailable", + }); }); it("runs the existing merge primitive and emits a workflow capability audit event", async () => { @@ -88,6 +112,27 @@ describe("workflow merge nodes", () => { }); }); + it("preserves merge-unavailable from a failed merge primitive in node context and audit", async () => { + const audit = vi.fn(); + const primitiveData = { status: "failed" as const, reason: "merge-unavailable" }; + const requestMerge = vi.fn().mockResolvedValue({ + outcome: "failure", + value: "merge-unavailable", + data: primitiveData, + }); + + await expect(runWorkflowMergeAttemptNode({ primitives: { requestMerge, audit } }, ctx, task)).resolves.toEqual({ + outcome: "failure", + value: "merge-unavailable", + contextPatch: { "workflow:merge-status": "merge-unavailable" }, + }); + expect(requestMerge).toHaveBeenCalledTimes(1); + expect(audit).toHaveBeenCalledWith(ctx, expect.objectContaining({ + type: "workflow-merge-node", + metadata: expect.objectContaining({ primitiveValue: "merge-unavailable", primitiveData }), + })); + }); + it("preserves implementation-incomplete from a failed merge primitive in node context", async () => { const audit = vi.fn(); 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 d306459ef4..7c9baa9dd3 100644 --- a/packages/engine/src/workflows/workflow-merge-nodes.ts +++ b/packages/engine/src/workflows/workflow-merge-nodes.ts @@ -5,7 +5,7 @@ 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 const PRESERVED_MERGE_FAILURE_REASONS = new Set(["implementation-incomplete", "merge-unavailable"]); export interface WorkflowMergeNodeDeps { primitives: Pick; @@ -76,10 +76,13 @@ export function classifyMergePrimitiveResult( function classifyMergeFailure(reason: string): WorkflowNodeResult { 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. + FNXC:WorkflowMerge 2026-08-20-02:36: + Structured engine sentinels must survive classification: these heuristics are only for free-text + merge-requester reasons, and renaming exact literals made primitive merge-attempt dispatch disagree + with the legacy merge seam for the same engine state. implementation-incomplete protects its no-op + merge-proof route; merge-unavailable deliberately remains non-terminal because it is emitted only + when mergeRequester is absent and routeGraphMergeFailureToRetry returns false on that same absence. + Marking it terminal would instead park both paths as operator-action-required failures. */ if (PRESERVED_MERGE_FAILURE_REASONS.has(normalized)) { return { outcome: "failure", value: normalized };