FN-5697: retry transient auto-merge failures and fix migration versioning
Treat transient auto-merge/provider abort errors as bounded retries while preserving correct schema migration ordering. - Add transient merge retry handling with capped exponential backoff, queue re-enqueue, and exhaustion logging before failing tasks. - Extend task/core types and evaluator evidence plumbing for merge transient retry tracking and MergeTransientRetryExhausted visibility. - Add regression coverage for transient auto-merge retries and exhaustion behavior in merge error recovery tests. - Resolve migration collision by promoting workflow_steps.gateMode migration to version 77, shifting subsequent migrations, and bumping schema version to 95. - Add a changeset and architecture note documenting transient retry behavior. Files changed: .changeset/fn-5697-auto-merge-transient-retry.md | 5 ++ docs/architecture.md | 1 + packages/core/src/db.ts | 85 +++++++++++---------- packages/core/src/eval-types.ts | 1 + packages/core/src/store.ts | 19 +++-- packages/core/src/types.ts | 7 ++ packages/engine/src/__tests__/evaluator-evidence.test.ts | 1 + packages/engine/src/__tests__/merge-error-recovery.test.ts | 87 ++++++++++++++++++++++ packages/engine/src/evaluator-evidence.ts | 1 + packages/engine/src/project-engine.ts | 68 +++++++++++++++++ 10 files changed, 231 insertions(+), 44 deletions(-) Fusion-Task-Id: FN-5697 Fusion-Task-Lineage: c8be7374-7cb6-444e-9920-9227e05a43dc
This commit is contained in:
@@ -151,6 +151,7 @@ describe("collectTaskEvaluationEvidence", () => {
|
||||
verificationFailureCount: 7,
|
||||
mergeConflictBounceCount: 8,
|
||||
mergeAuditBounceCount: 0,
|
||||
mergeTransientRetryCount: 0,
|
||||
});
|
||||
|
||||
const summary = evidence.taskMetadata[0]?.summary ?? "";
|
||||
|
||||
@@ -54,6 +54,7 @@ type MockTask = {
|
||||
mergeDetails?: { mergeConfirmed?: boolean; commitSha?: string; mergedAt?: string } | null;
|
||||
verificationFailureCount?: number;
|
||||
mergeConflictBounceCount?: number;
|
||||
mergeTransientRetryCount?: number;
|
||||
branch?: string;
|
||||
worktree?: string;
|
||||
sourceType?: string;
|
||||
@@ -591,7 +592,56 @@ describe("ProjectEngine merge error recovery", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("re-enqueues direct merge on transient non-conflict errors", async () => {
|
||||
vi.useFakeTimers();
|
||||
const setTimeoutSpy = vi.spyOn(globalThis, "setTimeout");
|
||||
const store = makeStore();
|
||||
vi.mocked(aiMergeTask).mockRejectedValueOnce(new Error("This operation was aborted"));
|
||||
|
||||
const engine = createEngine(store);
|
||||
const privateEngine = engine as unknown as { internalEnqueueMerge: (taskId: string) => void };
|
||||
const enqueueSpy = vi.spyOn(privateEngine, "internalEnqueueMerge");
|
||||
await runMergeCycle(engine);
|
||||
|
||||
expect(store.updateTask).toHaveBeenCalledWith(TASK_ID, {
|
||||
mergeTransientRetryCount: 1,
|
||||
status: null,
|
||||
});
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith(
|
||||
TASK_ID,
|
||||
expect.objectContaining({ status: "failed" }),
|
||||
);
|
||||
expect(setTimeoutSpy).toHaveBeenCalledWith(expect.any(Function), 5000);
|
||||
|
||||
await vi.advanceTimersByTimeAsync(5000);
|
||||
expect(enqueueSpy).toHaveBeenCalledWith(TASK_ID);
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("parks direct merge when transient retry cap is exhausted", async () => {
|
||||
const store = makeStore({
|
||||
tasks: [makeTask({ mergeTransientRetryCount: 3 }), makeTask({ mergeTransientRetryCount: 3 })],
|
||||
});
|
||||
vi.mocked(aiMergeTask).mockRejectedValueOnce(new Error("socket hang up"));
|
||||
|
||||
const engine = createEngine(store);
|
||||
await runMergeCycle(engine);
|
||||
|
||||
expect(store.updateTask).toHaveBeenCalledWith(TASK_ID, {
|
||||
status: "failed",
|
||||
mergeRetries: 3,
|
||||
error: "socket hang up",
|
||||
});
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
TASK_ID,
|
||||
expect.stringContaining("transient retries exhausted"),
|
||||
"MergeTransientRetryExhausted",
|
||||
);
|
||||
});
|
||||
|
||||
it("stores terminal merge metadata for non-conflict direct merge errors", async () => {
|
||||
vi.useFakeTimers();
|
||||
const setTimeoutSpy = vi.spyOn(globalThis, "setTimeout");
|
||||
const store = makeStore();
|
||||
vi.mocked(aiMergeTask).mockRejectedValueOnce(new Error("remote branch missing"));
|
||||
|
||||
@@ -603,7 +653,13 @@ describe("ProjectEngine merge error recovery", () => {
|
||||
mergeRetries: 3,
|
||||
error: "remote branch missing",
|
||||
});
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith(
|
||||
TASK_ID,
|
||||
expect.objectContaining({ mergeTransientRetryCount: expect.any(Number) }),
|
||||
);
|
||||
expect(setTimeoutSpy).not.toHaveBeenCalledWith(expect.any(Function), 5000);
|
||||
expect(hasErrorLog(errorSpy, "after non-conflict error")).toBe(false);
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("parks merge-confirmed tasks in stable failed state when finalization is blocked by incomplete steps", async () => {
|
||||
@@ -698,6 +754,37 @@ describe("ProjectEngine merge error recovery", () => {
|
||||
expect(hasErrorLog(errorSpy, "sqlite locked")).toBe(true);
|
||||
});
|
||||
|
||||
it("re-enqueues pull-request merge on transient strategy errors", async () => {
|
||||
vi.useFakeTimers();
|
||||
const setTimeoutSpy = vi.spyOn(globalThis, "setTimeout");
|
||||
const processPullRequestMerge = vi.fn(async () => {
|
||||
throw new Error("socket hang up");
|
||||
});
|
||||
const store = makeStore();
|
||||
|
||||
const engine = createEngine(store, {
|
||||
getMergeStrategy: () => "pull-request",
|
||||
processPullRequestMerge,
|
||||
});
|
||||
const privateEngine = engine as unknown as { internalEnqueueMerge: (taskId: string) => void };
|
||||
const enqueueSpy = vi.spyOn(privateEngine, "internalEnqueueMerge");
|
||||
|
||||
await runMergeCycle(engine);
|
||||
|
||||
expect(store.updateTask).toHaveBeenCalledWith(TASK_ID, {
|
||||
mergeTransientRetryCount: 1,
|
||||
status: null,
|
||||
});
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith(
|
||||
TASK_ID,
|
||||
expect.objectContaining({ status: "failed" }),
|
||||
);
|
||||
expect(setTimeoutSpy).toHaveBeenCalledWith(expect.any(Function), 5000);
|
||||
await vi.advanceTimersByTimeAsync(5000);
|
||||
expect(enqueueSpy).toHaveBeenCalledWith(TASK_ID);
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("logs when non-direct merge strategy recovery update fails", async () => {
|
||||
const store = makeStore({
|
||||
updateTask: vi.fn(async () => {
|
||||
|
||||
@@ -238,6 +238,7 @@ export async function collectTaskEvaluationEvidence(params: {
|
||||
verificationFailureCount: task.verificationFailureCount ?? 0,
|
||||
mergeConflictBounceCount: task.mergeConflictBounceCount ?? 0,
|
||||
mergeAuditBounceCount: task.mergeAuditBounceCount ?? 0,
|
||||
mergeTransientRetryCount: task.mergeTransientRetryCount ?? 0,
|
||||
},
|
||||
}],
|
||||
commits: commitEvidence,
|
||||
|
||||
@@ -39,6 +39,7 @@ import {
|
||||
createAutomatedFollowup,
|
||||
extractFailingTestFiles,
|
||||
} from "./verification-followup-dedup.js";
|
||||
import { isTransientError } from "./transient-error-detector.js";
|
||||
import { TunnelProcessManager } from "./remote-access/tunnel-process-manager.js";
|
||||
import type {
|
||||
ExternalTunnelInfo,
|
||||
@@ -284,6 +285,10 @@ export class ProjectEngine {
|
||||
private shuttingDown = false;
|
||||
|
||||
private static readonly MAX_AUTO_MERGE_RETRIES = 3;
|
||||
/** FN-5697/FN-5674: cap transient provider/network abort retries in auto-merge.
|
||||
* Examples: "This operation was aborted", "socket hang up", `server_error`.
|
||||
* After this cap, the task is parked failed for human visibility. */
|
||||
private static readonly MAX_AUTO_MERGE_TRANSIENT_RETRIES = 3;
|
||||
/** Cap on outer in-review→in-progress bounces caused by deterministic
|
||||
* verification failures during auto-merge. After this many failed merges
|
||||
* for the same task, we stop bouncing it back, mark it failed, and create
|
||||
@@ -2254,6 +2259,16 @@ export class ProjectEngine {
|
||||
// re-attempt; the catch-block-top logEntry already recorded the
|
||||
// failure on the task log.
|
||||
try {
|
||||
if (await this.maybeRetryTransientMerge(store, taskId, taskOnErr, errorMsg)) {
|
||||
continue;
|
||||
}
|
||||
if (this.isTransientMergeRetryExhausted(taskOnErr, errorMsg)) {
|
||||
await store.logEntry(
|
||||
taskId,
|
||||
`Auto-merge transient retries exhausted (${ProjectEngine.MAX_AUTO_MERGE_TRANSIENT_RETRIES}/${ProjectEngine.MAX_AUTO_MERGE_TRANSIENT_RETRIES}); parking task as failed: ${errorMsg}`,
|
||||
"MergeTransientRetryExhausted",
|
||||
);
|
||||
}
|
||||
await store.updateTask(taskId, {
|
||||
status: "failed",
|
||||
mergeRetries: ProjectEngine.MAX_AUTO_MERGE_RETRIES,
|
||||
@@ -2274,6 +2289,16 @@ export class ProjectEngine {
|
||||
// Non-direct merge strategy (e.g. pull-request) errored — park as
|
||||
// failed so the cooldown sweep stops re-attempting silently.
|
||||
try {
|
||||
if (await this.maybeRetryTransientMerge(store, taskId, taskOnErr, errorMsg)) {
|
||||
continue;
|
||||
}
|
||||
if (this.isTransientMergeRetryExhausted(taskOnErr, errorMsg)) {
|
||||
await store.logEntry(
|
||||
taskId,
|
||||
`Auto-merge transient retries exhausted (${ProjectEngine.MAX_AUTO_MERGE_TRANSIENT_RETRIES}/${ProjectEngine.MAX_AUTO_MERGE_TRANSIENT_RETRIES}); parking task as failed: ${errorMsg}`,
|
||||
"MergeTransientRetryExhausted",
|
||||
);
|
||||
}
|
||||
await store.updateTask(taskId, {
|
||||
status: "failed",
|
||||
mergeRetries: ProjectEngine.MAX_AUTO_MERGE_RETRIES,
|
||||
@@ -2312,6 +2337,49 @@ export class ProjectEngine {
|
||||
}
|
||||
}
|
||||
|
||||
private isTransientMergeRetryExhausted(task: Task | null, errorMsg: string): boolean {
|
||||
if (!task || !isTransientError(errorMsg)) {
|
||||
return false;
|
||||
}
|
||||
const current = task.mergeTransientRetryCount ?? 0;
|
||||
return current >= ProjectEngine.MAX_AUTO_MERGE_TRANSIENT_RETRIES;
|
||||
}
|
||||
|
||||
private async maybeRetryTransientMerge(
|
||||
store: TaskStore,
|
||||
taskId: string,
|
||||
taskOnErr: Task | null,
|
||||
errorMsg: string,
|
||||
): Promise<boolean> {
|
||||
if (!taskOnErr || !isTransientError(errorMsg)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const currentRetries = taskOnErr.mergeTransientRetryCount ?? 0;
|
||||
if (currentRetries >= ProjectEngine.MAX_AUTO_MERGE_TRANSIENT_RETRIES) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const nextRetryCount = currentRetries + 1;
|
||||
const delayMs = 5000 * Math.pow(2, currentRetries);
|
||||
await store.updateTask(taskId, {
|
||||
mergeTransientRetryCount: nextRetryCount,
|
||||
status: null,
|
||||
});
|
||||
await store.logEntry(
|
||||
taskId,
|
||||
`Auto-merge transient retry ${nextRetryCount}/${ProjectEngine.MAX_AUTO_MERGE_TRANSIENT_RETRIES} scheduled in ${delayMs / 1000}s: ${errorMsg}`,
|
||||
"MergeTransientRetry",
|
||||
);
|
||||
runtimeLog.log(
|
||||
`Auto-merge transient retry ${nextRetryCount}/${ProjectEngine.MAX_AUTO_MERGE_TRANSIENT_RETRIES} for ${taskId} in ${delayMs / 1000}s`,
|
||||
);
|
||||
setTimeout(() => {
|
||||
if (!this.shuttingDown) this.internalEnqueueMerge(taskId);
|
||||
}, delayMs);
|
||||
return true;
|
||||
}
|
||||
|
||||
private wireAutoMerge(store: TaskStore, _cwd: string): void {
|
||||
this.taskMovedHandler = async ({ task, to }: { task: Task; to: string }) => {
|
||||
if (to !== "in-review") return;
|
||||
|
||||
Reference in New Issue
Block a user