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

feat(FN-5627): auto-recover from fast-path foreign-commit refusal

Browse Source 
Instead of immediately parking the task as failed when the auto-merge
fast-path detects a non-ancestor commitSha (the symptom of a pre-FN-5627
TOCTOU poisoning), clear the poisoned mergeDetails fields and re-enqueue
for a fresh aiMergeTask attempt. The branch typically still has the work
intact; the merger just needs to redo the squash + ref-advance with the
now-fixed flow.

Recovery semantics:
- mergeRetries < MAX_AUTO_MERGE_RETRIES (3): clear poisoned fields
  (commitSha, mergedAt, landedFiles, filesChanged, insertions, deletions,
  noOpVerifiedShortCircuit, landedFilesAttributionRestricted, mergeConfirmed),
  increment mergeRetries, clear status/error, re-enqueue via
  internalEnqueueMerge. Emit new merger:fast-path-auto-recovered audit event.
- mergeRetries >= MAX_AUTO_MERGE_RETRIES: terminal park as failed (existing
  behavior), with merger:fast-path-blocked-foreign-commit audit event
  carrying budgetExhausted=true.

The recoverable path keeps the task in in-review with status=null, so
downstream consumers (dashboard banner, ntfy notifications) don't surface
a transient failure for what should be a self-healing event. The terminal
path remains FN-4538/FN-5488 compatible: status=failed at retry ceiling
on in-review is recognized by clearStaleBlockedBy fast paths so
downstream todos don't deadlock.

Tests updated and added:
- FN-5627 auto-recover test: asserts mergeRetries=1, status=null, all
  poisoned fields cleared, no moveTask('done'), no task:merged emit.
- FN-5627 budget-exhausted test: asserts status=failed with descriptive
  error, mergeRetries=3 input, no moveTask('done').

Fusion-Task-Id: FN-5627
This commit is contained in:
gsxdsm
2026-05-28 13:19:26 -07:00
parent b2d547eae5
commit 230f6f45b3
3 changed files with 216 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

119
packages/engine/src/__tests__/project-engine.test.ts
View File

@@ -1499,14 +1499,13 @@ describe("ProjectEngine paused in-review auto-merge behavior", () => {
await engine.stop();
});
it("FN-5627: refuses fast-path and parks task when mergeConfirmed commitSha is not ancestor of integration branch", async () => {
it("FN-5627: auto-recovers fast-path refusal by clearing poisoned mergeDetails + re-enqueueing (mergeRetries < budget)", async () => {
// Repro for the FN-5625/FN-5623 false-positive done class: the merger
// has a TOCTOU between writing `mergeConfirmed: true` and `git update-ref`
// succeeding. When the ref-advance fails after the optimistic write, the
// task row is poisoned. Without this gate, the auto-merge fast-path
// would silently promote the poisoned row to `done`. With the gate, the
// fast-path verifies reachability and refuses, parking the task in
// in-review with status=failed for manual review.
// task row is poisoned. The gate detects this, clears the lies, and
// re-enqueues for a fresh aiMergeTask attempt — no human intervention
// required as long as the retry budget isn't exhausted.
const mockStore = createMockStore({ ...baseSettings, autoMerge: true });
mockStore.store.getTask.mockResolvedValueOnce({
id: "FN-poisoned",
@@ -1521,6 +1520,10 @@ describe("ProjectEngine paused in-review auto-merge behavior", () => {
commitSha: "abc123abc123abc123abc123abc123abc1234567",
mergeTargetBranch: "main",
mergedAt: "2026-05-28T19:34:17.022Z",
landedFiles: ["packages/foo/bar.ts"],
filesChanged: 1,
insertions: 10,
deletions: 2,
},
});
mocks.currentStore = mockStore.store;
@@ -1557,22 +1560,39 @@ describe("ProjectEngine paused in-review auto-merge behavior", () => {
await engine.start();
engine.enqueueMerge("FN-poisoned");
// Gate refuses the fast-path: the task row is updated with status=failed,
// mergeConfirmed is cleared, and the task is parked in in-review.
// Auto-recovery path: the task row is updated with cleared poisoned
// fields, mergeRetries incremented, status null (NOT failed).
await vi.waitFor(() => {
const calls = (mockStore.store.updateTask as ReturnType<typeof vi.fn>).mock.calls as unknown as Array<[string, Record<string, unknown>]>;
const refusalCall = calls.find((call) =>
const recoveryCall = calls.find((call) =>
call[0] === "FN-poisoned"
&& call[1]?.status === "failed"
&& typeof call[1]?.error === "string"
&& /not reachable/.test(call[1].error as string),
&& call[1]?.mergeRetries === 1
&& call[1]?.status === null,
);
expect(refusalCall).toBeDefined();
const updates = refusalCall![1] as { mergeDetails?: { mergeConfirmed?: boolean } };
expect(recoveryCall).toBeDefined();
const updates = recoveryCall![1] as {
mergeDetails?: {
mergeConfirmed?: boolean;
commitSha?: string;
mergedAt?: string;
landedFiles?: string[];
filesChanged?: number;
};
};
// Poisoned fields cleared.
expect(updates.mergeDetails?.mergeConfirmed).toBe(false);
expect(updates.mergeDetails?.commitSha).toBeUndefined();
expect(updates.mergeDetails?.mergedAt).toBeUndefined();
expect(updates.mergeDetails?.landedFiles).toBeUndefined();
expect(updates.mergeDetails?.filesChanged).toBeUndefined();
});
// moveTask("done") was NOT called.
// Critical invariant: status is NOT failed (this is auto-recoverable).
const failedCall = (mockStore.store.updateTask as ReturnType<typeof vi.fn>).mock.calls
.find((call: unknown[]) => call[0] === "FN-poisoned" && (call[1] as { status?: string })?.status === "failed");
expect(failedCall).toBeUndefined();
// moveTask("done") was NOT called (no false-positive completion).
expect(mockStore.store.moveTask).not.toHaveBeenCalledWith("FN-poisoned", "done");
// task:merged was NOT emitted for the poisoned task.
const emitCalls = (mockStore.store.emit as ReturnType<typeof vi.fn>).mock.calls as unknown as Array<[string, { task?: { id: string } }]>;
@@ -1583,6 +1603,77 @@ describe("ProjectEngine paused in-review auto-merge behavior", () => {
await engine.stop();
});
it("FN-5627: fast-path refusal parks task as failed when mergeRetries budget is exhausted", async () => {
// When auto-recovery has already cycled through 3 attempts without
// landing, the next refusal is terminal. The task is parked with
// status=failed for manual review, with no further re-enqueue. The
// downstream FN-5488 fast-path on `clearStaleBlockedBy` recognizes this
// as a permanent blocker so dependents aren't held forever.
const mockStore = createMockStore({ ...baseSettings, autoMerge: true });
mockStore.store.getTask.mockResolvedValueOnce({
id: "FN-exhausted",
column: "in-review",
paused: false,
mergeRetries: 3, // already at budget
status: null,
branch: "fusion/fn-exhausted",
baseBranch: "main",
mergeDetails: {
mergeConfirmed: true,
commitSha: "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef",
mergeTargetBranch: "main",
mergedAt: "2026-05-28T19:34:17.022Z",
},
});
mocks.currentStore = mockStore.store;
mocks.execFile.mockImplementation((
_file: string,
args: string[],
_options: unknown,
callback?: (error: (Error & { code?: number }) | null, result: { stdout: string; stderr: string }) => void,
) => {
const cb = (typeof _options === "function" ? _options : callback) as (
error: (Error & { code?: number }) | null,
result: { stdout: string; stderr: string },
) => void;
if (args[0] === "cat-file") {
cb(null, { stdout: "", stderr: "" });
return {} as never;
}
if (args[0] === "merge-base" && args[1] === "--is-ancestor") {
const err = new Error("Command failed: git merge-base --is-ancestor") as Error & { code?: number };
err.code = 1;
cb(err, { stdout: "", stderr: "" });
return {} as never;
}
cb(null, { stdout: "/usr/bin/mock\n", stderr: "" });
return {} as never;
});
const engine = createEngine();
await engine.start();
engine.enqueueMerge("FN-exhausted");
await vi.waitFor(() => {
const calls = (mockStore.store.updateTask as ReturnType<typeof vi.fn>).mock.calls as unknown as Array<[string, Record<string, unknown>]>;
const terminalCall = calls.find((call) =>
call[0] === "FN-exhausted"
&& call[1]?.status === "failed"
&& typeof call[1]?.error === "string"
&& /retry budget exhausted|after 3 attempts/.test(call[1].error as string),
);
expect(terminalCall).toBeDefined();
const updates = terminalCall![1] as { mergeDetails?: { mergeConfirmed?: boolean; commitSha?: string } };
expect(updates.mergeDetails?.mergeConfirmed).toBe(false);
expect(updates.mergeDetails?.commitSha).toBeUndefined();
});
expect(mockStore.store.moveTask).not.toHaveBeenCalledWith("FN-exhausted", "done");
await engine.stop();
});
it("FN-5627: fast-path still works when mergeConfirmed has no commitSha (verified-no-op path)", async () => {
// Legitimate no-op merges have mergeConfirmed=true with no commitSha
// (verified-short-circuit / proven-no-op / already-on-main paths). The

110
packages/engine/src/project-engine.ts
View File

@@ -1460,34 +1460,109 @@ export class ProjectEngine {
if (!reachability.reachable) {
const sha = task.mergeDetails.commitSha || "";
const shortSha = sha ? sha.slice(0, 8) : "<no-sha>";
const errorMsg =
`Merge confirmed flag set but commit ${shortSha} is not reachable from ` +
`${integrationBranchForGate} (${reachability.reason}). ` +
`Task parked in in-review pending manual review.`;
const currentRetries = task.mergeRetries ?? 0;
const budgetExhausted =
currentRetries >= ProjectEngine.MAX_AUTO_MERGE_RETRIES;
// Clear poisoned mergeDetails fields. These persisted before
// the integration ref-advance actually succeeded (pre-FN-5627
// optimistic-write TOCTOU). Drop the lies but keep diagnostic
// context (mergeTargetBranch, attemptsMade, etc.).
const cleanedMergeDetails = {
...task.mergeDetails,
mergeConfirmed: false,
commitSha: undefined,
mergedAt: undefined,
landedFiles: undefined,
filesChanged: undefined,
insertions: undefined,
deletions: undefined,
noOpVerifiedShortCircuit: undefined,
landedFilesAttributionRestricted: undefined,
};
if (budgetExhausted) {
// Retry budget exhausted — terminal park for manual review.
// FN-4538-class invariant: failed `in-review` blockers at the
// retry ceiling are recognized by downstream `clearStaleBlockedBy`
// fast paths (FN-5488), so dependents won't deadlock.
const errorMsg =
`Auto-merge fast-path refused after ${currentRetries} attempts: commit ${shortSha} is not reachable from ` +
`${integrationBranchForGate} (${reachability.reason}). Manual review required.`;
runtimeLog.warn(
`Auto-merge: ${taskId} fast-path REFUSED + budget exhausted — ${reachability.reason}: ${reachability.diagnostic}`,
);
await store.logEntry(
taskId,
`[FN-5627] Auto-merge fast-path refused (retry budget exhausted) — ${errorMsg}`,
);
await store.updateTask(taskId, {
mergeDetails: cleanedMergeDetails,
status: "failed",
error: errorMsg,
});
try {
const auditor = createRunAuditor(store, {
runId: generateSyntheticRunId("merger-fast-path-refused", taskId),
agentId: "merger",
taskId,
phase: "auto-merge-fast-path-gate",
});
await auditor.database({
type: "merger:fast-path-blocked-foreign-commit",
target: taskId,
metadata: {
taskId,
commitSha: sha,
integrationBranch: integrationBranchForGate,
reason: reachability.reason,
diagnostic: reachability.diagnostic,
mergeRetries: currentRetries,
budgetExhausted: true,
},
});
} catch (auditErr) {
runtimeLog.warn(
`Auto-merge: ${taskId} fast-path audit emit failed: ${
auditErr instanceof Error ? auditErr.message : String(auditErr)
}`,
);
}
continue;
}
// FN-5627 auto-recovery: clear the poisoned mergeDetails,
// increment the merge retry counter, and re-enqueue. The next
// dequeue runs a fresh `aiMergeTask` against the task branch —
// because the merger's TOCTOU is now fixed, the redo either
// lands cleanly or fails with a real merger error that surfaces
// through normal lifecycle. We don't need an executor to be
// re-engaged for this kind of recovery; the branch already
// has the work, it just needs to be re-applied to the
// integration tip.
const nextRetries = currentRetries + 1;
runtimeLog.warn(
`Auto-merge: ${taskId} fast-path REFUSED — ${reachability.reason}: ${reachability.diagnostic}`,
`Auto-merge: ${taskId} fast-path REFUSED — auto-recovering (attempt ${nextRetries}/${ProjectEngine.MAX_AUTO_MERGE_RETRIES}): ${reachability.reason}: ${reachability.diagnostic}`,
);
await store.logEntry(
taskId,
`[FN-5627] Auto-merge fast-path refused — ${errorMsg}`,
`[FN-5627] Auto-merge fast-path refused — cleared poisoned mergeDetails (commit ${shortSha} not reachable from ${integrationBranchForGate}, ${reachability.reason}). Re-enqueueing for fresh merge attempt ${nextRetries}/${ProjectEngine.MAX_AUTO_MERGE_RETRIES}.`,
);
await store.updateTask(taskId, {
mergeDetails: {
...task.mergeDetails,
mergeConfirmed: false,
},
status: "failed",
error: errorMsg,
mergeDetails: cleanedMergeDetails,
mergeRetries: nextRetries,
status: null,
error: null,
});
try {
const auditor = createRunAuditor(store, {
runId: generateSyntheticRunId("merger-fast-path-refused", taskId),
runId: generateSyntheticRunId("merger-fast-path-auto-recovered", taskId),
agentId: "merger",
taskId,
phase: "auto-merge-fast-path-gate",
});
await auditor.database({
type: "merger:fast-path-blocked-foreign-commit",
type: "merger:fast-path-auto-recovered",
target: taskId,
metadata: {
taskId,
@@ -1495,6 +1570,8 @@ export class ProjectEngine {
integrationBranch: integrationBranchForGate,
reason: reachability.reason,
diagnostic: reachability.diagnostic,
mergeRetries: nextRetries,
maxRetries: ProjectEngine.MAX_AUTO_MERGE_RETRIES,
},
});
} catch (auditErr) {
@@ -1504,6 +1581,11 @@ export class ProjectEngine {
}`,
);
}
// Re-enqueue this task for the next cycle. We continue past
// the current iteration because `task` is a stale snapshot;
// the re-enqueued tick reads fresh state with mergeConfirmed=false
// and falls through to the normal `aiMergeTask` path.
this.internalEnqueueMerge(taskId);
continue;
}
const blockerReason = getTaskHardMergeBlocker(task as Task);

18
packages/engine/src/run-audit.ts
View File

@@ -550,10 +550,22 @@ export type DatabaseMutationType =
* `task.mergeDetails.commitSha` is not reachable from the integration branch
* tip, indicating the merger persisted `mergeConfirmed: true` before the
* ref-advance actually landed (TOCTOU window in merger.ts ~9762 vs ~9845).
* Task is parked in in-review with `status: "failed"` for manual review.
* Metadata: { taskId, commitSha, integrationBranch, reason, diagnostic }
* Emitted on TERMINAL refusal only — when `mergeRetries` has reached
* `MAX_AUTO_MERGE_RETRIES` and the task is parked in in-review with
* `status: "failed"` for manual review.
* Metadata: { taskId, commitSha, integrationBranch, reason, diagnostic, mergeRetries, budgetExhausted }
*/
| "merger:fast-path-blocked-foreign-commit";
| "merger:fast-path-blocked-foreign-commit"
/**
* FN-5627: Auto-recoverable variant of the fast-path refusal. The gate cleared
* the poisoned mergeDetails fields (commitSha/mergedAt/landedFiles/etc.) and
* re-enqueued the task for a fresh `aiMergeTask` attempt. Emitted on each
* recoverable refusal until `mergeRetries` reaches
* `MAX_AUTO_MERGE_RETRIES`, at which point the next refusal switches to
* `merger:fast-path-blocked-foreign-commit` and parks as failed.
* Metadata: { taskId, commitSha, integrationBranch, reason, diagnostic, mergeRetries, maxRetries }
*/
| "merger:fast-path-auto-recovered";
// ── Filesystem mutation types ─────────────────────────────────────────────────