FN-9170: Preserve structured merge failure sentinels
Keep merge-unavailable intact across workflow merge dispatch so graph routing receives the engine's structured result. - preserve merge-unavailable alongside implementation-incomplete during primitive classification - cover direct and legacy merge paths, audit context, normalization, and non-terminal retry behavior - document the structured sentinel invariant and add a patch changeset Files changed: .changeset/fn-9170-merge-unavailable.md | 7 ++ docs/architecture.md | 2 + .../merge-unavailable-classification.test.ts | 113 +++++++++++++++++++++ .../src/__tests__/workflow-merge-nodes.test.ts | 47 ++++++++- .../engine/src/workflows/workflow-merge-nodes.ts | 13 ++- 5 files changed, 176 insertions(+), 6 deletions(-) Fusion-Task-Id: FN-9170 Fusion-Task-Lineage: 99cc501f-8a25-49f5-b81a-74dd2e6e872b Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
7
.changeset/fn-9170-merge-unavailable.md
Normal file
7
.changeset/fn-9170-merge-unavailable.md
Normal file
@@ -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.
|
||||
@@ -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.
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
@@ -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({
|
||||
|
||||
@@ -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<WorkflowRuntimePrimitives, "requestMerge" | "audit">;
|
||||
@@ -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 };
|
||||
|
||||
Reference in New Issue
Block a user