fix(engine): prevent auto-merge cooldown loop on unresolvable conflicts
Tasks were getting stuck in `in-review` forever when auto-merge could not resolve conflicts within MAX_AUTO_MERGE_RETRIES. The conflict-exhaustion branch silently cleared `status` (no error, no log entry, no comment), and the 30-min cooldown sweep would reset retries and re-attempt the same impossible merge — looping silently with no user-facing surface. Why: - FN-2918 and FN-2903 both spent hours in this loop with no error/comment visible on the task. The only log evidence was repeated "Auto-merge retry cooldown elapsed (30m idle)" entries with no follow-up outcome. How to apply: - Every merge failure now writes a `<Manual|Auto>-merge failed: <msg>` entry to the task log so the dashboard surfaces the reason. - Conflict-retry exhaustion now bounces the task back to `in-progress` with a comment + log entry so the executor re-rebases against main and retries — mirroring the verification-failure-bounce pattern. - New `mergeConflictBounceCount` task field caps outer bounces (`MAX_MERGE_CONFLICT_BOUNCES = 2`); past the cap, the task is parked in `in-review` with `status="failed"` and a follow-up triage task is created so a human can resolve the conflict manually. - Non-conflict and non-direct-strategy errors now also set `status="failed"` so the cooldown sweep can't re-pick them up. - `canMergeTask` skips tasks with `status="failed"` so terminal failures (verification cap, bounce cap, non-conflict error) are no longer eligible for cooldown re-attempts. Schema migration v52 adds the `mergeConflictBounceCount` column. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -131,7 +131,7 @@ describe("Database", () => {
|
||||
});
|
||||
|
||||
it("seeds schema version", () => {
|
||||
expect(db.getSchemaVersion()).toBe(51);
|
||||
expect(db.getSchemaVersion()).toBe(52);
|
||||
});
|
||||
|
||||
it("seeds lastModified", () => {
|
||||
@@ -154,7 +154,7 @@ describe("Database", () => {
|
||||
|
||||
it("is idempotent - calling init() twice does not fail", () => {
|
||||
expect(() => db.init()).not.toThrow();
|
||||
expect(db.getSchemaVersion()).toBe(51);
|
||||
expect(db.getSchemaVersion()).toBe(52);
|
||||
});
|
||||
|
||||
it("does not overwrite existing config on re-init", () => {
|
||||
@@ -761,7 +761,7 @@ describe("schema migrations", () => {
|
||||
db.init();
|
||||
|
||||
// Verify version bumped to 29 (includes v1→v2 through v26→v29)
|
||||
expect(db.getSchemaVersion()).toBe(51);
|
||||
expect(db.getSchemaVersion()).toBe(52);
|
||||
|
||||
// Verify new columns exist and existing data is intact
|
||||
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
@@ -786,11 +786,11 @@ describe("schema migrations", () => {
|
||||
const db = new Database(fusionDir);
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(51);
|
||||
expect(db.getSchemaVersion()).toBe(52);
|
||||
|
||||
// Re-init should not fail
|
||||
db.init();
|
||||
expect(db.getSchemaVersion()).toBe(51);
|
||||
expect(db.getSchemaVersion()).toBe(52);
|
||||
|
||||
db.close();
|
||||
});
|
||||
@@ -825,7 +825,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(51);
|
||||
expect(db.getSchemaVersion()).toBe(52);
|
||||
|
||||
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
expect(cols.map((col) => col.name)).toContain("priority");
|
||||
@@ -866,7 +866,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(51);
|
||||
expect(db.getSchemaVersion()).toBe(52);
|
||||
|
||||
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
const colNames = cols.map((col) => col.name);
|
||||
@@ -935,7 +935,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(51);
|
||||
expect(db.getSchemaVersion()).toBe(52);
|
||||
|
||||
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
const colNames = cols.map((col) => col.name);
|
||||
@@ -994,7 +994,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(51);
|
||||
expect(db.getSchemaVersion()).toBe(52);
|
||||
|
||||
const cols = db.prepare("PRAGMA table_info(chat_messages)").all() as Array<{ name: string }>;
|
||||
expect(cols.map((col) => col.name)).toContain("attachments");
|
||||
@@ -1016,7 +1016,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(51);
|
||||
expect(db.getSchemaVersion()).toBe(52);
|
||||
|
||||
const tables = db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'agentRatings'").all() as Array<{ name: string }>;
|
||||
expect(tables).toEqual([{ name: "agentRatings" }]);
|
||||
@@ -1040,7 +1040,7 @@ describe("schema migrations", () => {
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(51);
|
||||
expect(db.getSchemaVersion()).toBe(52);
|
||||
|
||||
const tables = db.prepare("SELECT name FROM sqlite_master WHERE type='table' AND name = 'mission_events'").all() as Array<{ name: string }>;
|
||||
expect(tables).toEqual([{ name: "mission_events" }]);
|
||||
@@ -1144,7 +1144,7 @@ describe("schema migrations", () => {
|
||||
db.init();
|
||||
|
||||
// Verify version bumped to 29
|
||||
expect(db.getSchemaVersion()).toBe(51);
|
||||
expect(db.getSchemaVersion()).toBe(52);
|
||||
|
||||
// Verify new columns exist and existing data is intact
|
||||
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
@@ -1595,7 +1595,7 @@ describe("createDatabase factory", () => {
|
||||
const db = createDatabase(fusionDir);
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(51);
|
||||
expect(db.getSchemaVersion()).toBe(52);
|
||||
expect(db.getLastModified()).toBeGreaterThan(0);
|
||||
|
||||
db.close();
|
||||
|
||||
@@ -779,7 +779,7 @@ describe("Migration: pre-33 DB upgrade", () => {
|
||||
// Step 1: Create a fresh database at v33 (runs all migrations up to 33)
|
||||
const db1 = createDatabase(legacyDir);
|
||||
db1.init();
|
||||
expect(db1.getSchemaVersion()).toBe(51);
|
||||
expect(db1.getSchemaVersion()).toBe(52);
|
||||
db1.close();
|
||||
|
||||
// Step 2: Manually downgrade to version 32 and drop insight tables
|
||||
@@ -814,7 +814,7 @@ describe("Migration: pre-33 DB upgrade", () => {
|
||||
expect(tableNamesBefore).not.toContain("project_insight_runs");
|
||||
// Now run init — this triggers the v32→v33 migration
|
||||
db3.init();
|
||||
expect(db3.getSchemaVersion()).toBe(51);
|
||||
expect(db3.getSchemaVersion()).toBe(52);
|
||||
|
||||
// Step 4: Verify insight tables exist after migration
|
||||
const tablesAfter = db3.prepare(
|
||||
@@ -845,12 +845,12 @@ describe("Migration: pre-33 DB upgrade", () => {
|
||||
try {
|
||||
const db1 = createDatabase(testDir);
|
||||
db1.init();
|
||||
expect(db1.getSchemaVersion()).toBe(51);
|
||||
expect(db1.getSchemaVersion()).toBe(52);
|
||||
db1.close();
|
||||
|
||||
const db2 = createDatabase(testDir);
|
||||
expect(() => db2.init()).not.toThrow();
|
||||
expect(db2.getSchemaVersion()).toBe(51);
|
||||
expect(db2.getSchemaVersion()).toBe(52);
|
||||
db2.close();
|
||||
} finally {
|
||||
rmSync(testDir, { recursive: true, force: true });
|
||||
|
||||
@@ -2629,7 +2629,7 @@ describe("MissionStore", () => {
|
||||
|
||||
describe("Loop State & Validator Run Schema (v31)", () => {
|
||||
it("schema version is 40 after migration", () => {
|
||||
expect(db.getSchemaVersion()).toBe(51);
|
||||
expect(db.getSchemaVersion()).toBe(52);
|
||||
});
|
||||
|
||||
it("mission_features table has loop state columns", () => {
|
||||
|
||||
@@ -742,7 +742,7 @@ describe("RoadmapStore", () => {
|
||||
|
||||
describe("schema version", () => {
|
||||
it("schema version is 40 after init", () => {
|
||||
expect(db.getSchemaVersion()).toBe(51);
|
||||
expect(db.getSchemaVersion()).toBe(52);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -465,7 +465,7 @@ describe("Run Audit", () => {
|
||||
});
|
||||
|
||||
it("schema version is bumped to 40", () => {
|
||||
expect(db.getSchemaVersion()).toBe(51);
|
||||
expect(db.getSchemaVersion()).toBe(52);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -51,7 +51,7 @@ describe("TaskStore task documents", () => {
|
||||
|
||||
expect(tableNames.has("task_documents")).toBe(true);
|
||||
expect(tableNames.has("task_document_revisions")).toBe(true);
|
||||
expect(db.getSchemaVersion()).toBe(51);
|
||||
expect(db.getSchemaVersion()).toBe(52);
|
||||
|
||||
const index = db
|
||||
.prepare(
|
||||
|
||||
@@ -86,7 +86,7 @@ export function probeFts5(db: DatabaseSync): boolean {
|
||||
|
||||
// ── Schema Definition ────────────────────────────────────────────────
|
||||
|
||||
const SCHEMA_VERSION = 51;
|
||||
const SCHEMA_VERSION = 52;
|
||||
|
||||
function normalizeTaskComments(
|
||||
steeringComments: SteeringComment[] | undefined,
|
||||
@@ -1949,6 +1949,16 @@ export class Database {
|
||||
});
|
||||
}
|
||||
|
||||
// Outer auto-merge bounce counter so the cooldown sweep can't loop forever
|
||||
// on a task whose conflicts can't be auto-resolved. Capped by
|
||||
// MAX_MERGE_CONFLICT_BOUNCES in project-engine.ts; once reached, the task
|
||||
// is parked in in-review with status="failed" and a follow-up is created.
|
||||
if (version < 52) {
|
||||
this.applyMigration(52, () => {
|
||||
this.addColumnIfMissing("tasks", "mergeConflictBounceCount", "INTEGER DEFAULT 0");
|
||||
});
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -54,6 +54,7 @@ interface TaskRow {
|
||||
recoveryRetryCount: number | null;
|
||||
taskDoneRetryCount: number | null;
|
||||
verificationFailureCount: number | null;
|
||||
mergeConflictBounceCount: number | null;
|
||||
nextRecoveryAt: string | null;
|
||||
error: string | null;
|
||||
summary: string | null;
|
||||
@@ -537,6 +538,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
recoveryRetryCount: row.recoveryRetryCount ?? undefined,
|
||||
taskDoneRetryCount: row.taskDoneRetryCount ?? undefined,
|
||||
verificationFailureCount: row.verificationFailureCount ?? undefined,
|
||||
mergeConflictBounceCount: row.mergeConflictBounceCount ?? undefined,
|
||||
nextRecoveryAt: row.nextRecoveryAt || undefined,
|
||||
error: row.error || undefined,
|
||||
summary: row.summary || undefined,
|
||||
@@ -834,7 +836,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
"modelPresetId", "modelProvider", "modelId",
|
||||
"validatorModelProvider", "validatorModelId",
|
||||
"planningModelProvider", "planningModelId",
|
||||
"mergeRetries", "workflowStepRetries", "stuckKillCount", "postReviewFixCount", "recoveryRetryCount", "taskDoneRetryCount", "verificationFailureCount", "nextRecoveryAt",
|
||||
"mergeRetries", "workflowStepRetries", "stuckKillCount", "postReviewFixCount", "recoveryRetryCount", "taskDoneRetryCount", "verificationFailureCount", "mergeConflictBounceCount", "nextRecoveryAt",
|
||||
"error", "summary", "thinkingLevel", "executionMode",
|
||||
"tokenUsageInputTokens", "tokenUsageOutputTokens", "tokenUsageCachedTokens", "tokenUsageTotalTokens", "tokenUsageFirstUsedAt", "tokenUsageLastUsedAt",
|
||||
"createdAt", "updatedAt", "columnMovedAt",
|
||||
@@ -882,7 +884,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
"modelPresetId", "modelProvider", "modelId",
|
||||
"validatorModelProvider", "validatorModelId",
|
||||
"planningModelProvider", "planningModelId",
|
||||
"mergeRetries", "workflowStepRetries", "stuckKillCount", "postReviewFixCount", "recoveryRetryCount", "taskDoneRetryCount", "verificationFailureCount", "nextRecoveryAt",
|
||||
"mergeRetries", "workflowStepRetries", "stuckKillCount", "postReviewFixCount", "recoveryRetryCount", "taskDoneRetryCount", "verificationFailureCount", "mergeConflictBounceCount", "nextRecoveryAt",
|
||||
"error", "summary", "thinkingLevel", "executionMode",
|
||||
"tokenUsageInputTokens", "tokenUsageOutputTokens", "tokenUsageCachedTokens", "tokenUsageTotalTokens", "tokenUsageFirstUsedAt", "tokenUsageLastUsedAt",
|
||||
"createdAt", "updatedAt", "columnMovedAt",
|
||||
@@ -924,7 +926,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
id, title, description, priority, "column", status, size, reviewLevel, currentStep,
|
||||
worktree, blockedBy, paused, baseBranch, branch, baseCommitSha, modelPresetId, modelProvider,
|
||||
modelId, validatorModelProvider, validatorModelId, planningModelProvider, planningModelId, mergeRetries,
|
||||
workflowStepRetries, stuckKillCount, postReviewFixCount, recoveryRetryCount, taskDoneRetryCount, verificationFailureCount, nextRecoveryAt, error,
|
||||
workflowStepRetries, stuckKillCount, postReviewFixCount, recoveryRetryCount, taskDoneRetryCount, verificationFailureCount, mergeConflictBounceCount, nextRecoveryAt, error,
|
||||
summary, thinkingLevel, executionMode, tokenUsageInputTokens, tokenUsageOutputTokens, tokenUsageCachedTokens,
|
||||
tokenUsageTotalTokens, tokenUsageFirstUsedAt, tokenUsageLastUsedAt, createdAt, updatedAt, columnMovedAt,
|
||||
dependencies, steps, log, attachments, steeringComments,
|
||||
@@ -932,7 +934,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
sourceIssueProvider, sourceIssueRepository, sourceIssueExternalIssueId, sourceIssueNumber, sourceIssueUrl,
|
||||
mergeDetails, breakIntoSubtasks, enabledWorkflowSteps, modifiedFiles, missionId, sliceId, assignedAgentId, assigneeUserId, nodeId, effectiveNodeId, effectiveNodeSource, checkedOutBy, checkedOutAt
|
||||
) VALUES (
|
||||
?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?
|
||||
?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?
|
||||
)
|
||||
ON CONFLICT(id) DO UPDATE SET
|
||||
title = excluded.title,
|
||||
@@ -963,6 +965,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
recoveryRetryCount = excluded.recoveryRetryCount,
|
||||
taskDoneRetryCount = excluded.taskDoneRetryCount,
|
||||
verificationFailureCount = excluded.verificationFailureCount,
|
||||
mergeConflictBounceCount = excluded.mergeConflictBounceCount,
|
||||
nextRecoveryAt = excluded.nextRecoveryAt,
|
||||
error = excluded.error,
|
||||
summary = excluded.summary,
|
||||
@@ -1034,6 +1037,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
task.recoveryRetryCount ?? null,
|
||||
task.taskDoneRetryCount ?? 0,
|
||||
task.verificationFailureCount ?? 0,
|
||||
task.mergeConflictBounceCount ?? 0,
|
||||
task.nextRecoveryAt ?? null,
|
||||
task.error ?? null,
|
||||
task.summary ?? null,
|
||||
@@ -2665,7 +2669,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
|
||||
async updateTask(
|
||||
id: string,
|
||||
updates: { title?: string; description?: string; priority?: TaskPriority | null; prompt?: string; worktree?: string | null; status?: string | null; dependencies?: string[]; steps?: import("./types.js").TaskStep[]; currentStep?: number; blockedBy?: string | null; assignedAgentId?: string | null; assigneeUserId?: string | null; nodeId?: string | null; effectiveNodeId?: string | null; effectiveNodeSource?: string | null; checkedOutBy?: string | null; checkedOutAt?: string | null; paused?: boolean; baseBranch?: string | null; branch?: string | null; baseCommitSha?: string | null; size?: "S" | "M" | "L"; reviewLevel?: number; executionMode?: import("./types.js").ExecutionMode | null; mergeRetries?: number; workflowStepRetries?: number; stuckKillCount?: number | null; postReviewFixCount?: number | null; recoveryRetryCount?: number | null; taskDoneRetryCount?: number | null; verificationFailureCount?: number | null; nextRecoveryAt?: string | null; enabledWorkflowSteps?: string[]; modelProvider?: string | null; modelId?: string | null; validatorModelProvider?: string | null; validatorModelId?: string | null; planningModelProvider?: string | null; planningModelId?: string | null; thinkingLevel?: string | null; error?: string | null; summary?: string | null; sessionFile?: string | null; workflowStepResults?: import("./types.js").WorkflowStepResult[] | null; mergeDetails?: import("./types.js").MergeDetails | null; sourceIssue?: import("./types.js").TaskSourceIssue | null; tokenUsage?: import("./types.js").TaskTokenUsage | null; modifiedFiles?: string[] | null; missionId?: string | null; sliceId?: string | null },
|
||||
updates: { title?: string; description?: string; priority?: TaskPriority | null; prompt?: string; worktree?: string | null; status?: string | null; dependencies?: string[]; steps?: import("./types.js").TaskStep[]; currentStep?: number; blockedBy?: string | null; assignedAgentId?: string | null; assigneeUserId?: string | null; nodeId?: string | null; effectiveNodeId?: string | null; effectiveNodeSource?: string | null; checkedOutBy?: string | null; checkedOutAt?: string | null; paused?: boolean; baseBranch?: string | null; branch?: string | null; baseCommitSha?: string | null; size?: "S" | "M" | "L"; reviewLevel?: number; executionMode?: import("./types.js").ExecutionMode | null; mergeRetries?: number; workflowStepRetries?: number; stuckKillCount?: number | null; postReviewFixCount?: number | null; recoveryRetryCount?: number | null; taskDoneRetryCount?: number | null; verificationFailureCount?: number | null; mergeConflictBounceCount?: number | null; nextRecoveryAt?: string | null; enabledWorkflowSteps?: string[]; modelProvider?: string | null; modelId?: string | null; validatorModelProvider?: string | null; validatorModelId?: string | null; planningModelProvider?: string | null; planningModelId?: string | null; thinkingLevel?: string | null; error?: string | null; summary?: string | null; sessionFile?: string | null; workflowStepResults?: import("./types.js").WorkflowStepResult[] | null; mergeDetails?: import("./types.js").MergeDetails | null; sourceIssue?: import("./types.js").TaskSourceIssue | null; tokenUsage?: import("./types.js").TaskTokenUsage | null; modifiedFiles?: string[] | null; missionId?: string | null; sliceId?: string | null },
|
||||
runContext?: RunMutationContext,
|
||||
): Promise<Task> {
|
||||
return this.withTaskLock(id, async () => {
|
||||
@@ -2813,6 +2817,11 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
} else if (updates.verificationFailureCount !== undefined) {
|
||||
task.verificationFailureCount = updates.verificationFailureCount;
|
||||
}
|
||||
if (updates.mergeConflictBounceCount === null) {
|
||||
task.mergeConflictBounceCount = undefined;
|
||||
} else if (updates.mergeConflictBounceCount !== undefined) {
|
||||
task.mergeConflictBounceCount = updates.mergeConflictBounceCount;
|
||||
}
|
||||
if (updates.nextRecoveryAt === null) {
|
||||
task.nextRecoveryAt = undefined;
|
||||
} else if (updates.nextRecoveryAt !== undefined) {
|
||||
|
||||
@@ -914,6 +914,14 @@ export interface Task {
|
||||
* follow-up triage task is created so a human / fresh agent can investigate
|
||||
* rather than endlessly re-attempting the same fix. */
|
||||
verificationFailureCount?: number;
|
||||
/** Number of times this task has bounced from `in-review` back to `in-progress`
|
||||
* due to auto-merge conflict-retry exhaustion. Incremented by the auto-merge
|
||||
* error handler (project-engine.ts) when conflicts can't be auto-resolved
|
||||
* within `MAX_AUTO_MERGE_RETRIES`. When this reaches
|
||||
* `MAX_MERGE_CONFLICT_BOUNCES`, the task is parked in `in-review` with
|
||||
* `status="failed"` and a follow-up triage task is created — preventing the
|
||||
* cooldown sweep from re-attempting the same impossible merge forever. */
|
||||
mergeConflictBounceCount?: number;
|
||||
/** ISO-8601 timestamp indicating when the task becomes eligible for the next
|
||||
* recovery retry. Scheduler and triage processor skip tasks whose
|
||||
* `nextRecoveryAt` is still in the future. Cleared alongside `recoveryRetryCount`. */
|
||||
|
||||
@@ -36,6 +36,9 @@ type MockTask = {
|
||||
status: string | null;
|
||||
error: string | null;
|
||||
verificationFailureCount?: number;
|
||||
mergeConflictBounceCount?: number;
|
||||
branch?: string;
|
||||
worktree?: string;
|
||||
updatedAt: string;
|
||||
log: Array<{ action?: string }>;
|
||||
};
|
||||
@@ -158,20 +161,36 @@ describe("ProjectEngine merge error recovery", () => {
|
||||
logSpy = vi.spyOn(runtimeLog, "log").mockImplementation(() => undefined);
|
||||
});
|
||||
|
||||
it("clears status when conflict retries are exhausted and recovery update succeeds", async () => {
|
||||
it("bounces task to in-progress when conflict retries are exhausted (under bounce cap)", async () => {
|
||||
const store = makeStore({
|
||||
tasks: [makeTask({ mergeRetries: 2 }), makeTask({ mergeRetries: 3 })],
|
||||
tasks: [makeTask({ mergeRetries: 2 }), makeTask({ mergeRetries: 3, branch: "fusion/fn-2084" })],
|
||||
});
|
||||
vi.mocked(aiMergeTask).mockRejectedValueOnce(new Error("merge conflict detected"));
|
||||
|
||||
const engine = createEngine(store);
|
||||
await runMergeCycle(engine);
|
||||
|
||||
expect(store.updateTask).toHaveBeenCalledWith(TASK_ID, { status: null });
|
||||
expect(hasErrorLog(errorSpy, "failed to clear status on")).toBe(false);
|
||||
expect(store.updateTask).toHaveBeenCalledWith(TASK_ID, {
|
||||
status: null,
|
||||
mergeRetries: 0,
|
||||
error: null,
|
||||
mergeConflictBounceCount: 1,
|
||||
});
|
||||
expect(store.moveTask).toHaveBeenCalledWith(TASK_ID, "in-progress");
|
||||
expect(store.addTaskComment).toHaveBeenCalledWith(
|
||||
TASK_ID,
|
||||
expect.stringContaining("Bouncing back to in-progress"),
|
||||
"agent",
|
||||
);
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
TASK_ID,
|
||||
expect.stringContaining("bounced to in-progress"),
|
||||
"MergeConflictBounce",
|
||||
);
|
||||
expect(hasErrorLog(errorSpy, "failed to bounce")).toBe(false);
|
||||
});
|
||||
|
||||
it("logs when clearing status fails after conflict retries are exhausted", async () => {
|
||||
it("logs when bouncing fails after conflict retries are exhausted", async () => {
|
||||
const store = makeStore({
|
||||
tasks: [makeTask({ mergeRetries: 2 }), makeTask({ mergeRetries: 3 })],
|
||||
updateTask: vi.fn(async () => {
|
||||
@@ -183,10 +202,36 @@ describe("ProjectEngine merge error recovery", () => {
|
||||
const engine = createEngine(store);
|
||||
await expect(runMergeCycle(engine)).resolves.toBeUndefined();
|
||||
|
||||
expect(hasErrorLog(errorSpy, `failed to clear status on ${TASK_ID}`)).toBe(true);
|
||||
expect(hasErrorLog(errorSpy, `failed to bounce ${TASK_ID}`)).toBe(true);
|
||||
expect(hasErrorLog(errorSpy, "db write failed")).toBe(true);
|
||||
});
|
||||
|
||||
it("parks task and creates follow-up when conflict bounce cap is exceeded", async () => {
|
||||
// Already bounced twice (cap is 2) — next bounce would be 3, exceeding cap
|
||||
const store = makeStore({
|
||||
tasks: [
|
||||
makeTask({ mergeRetries: 2, mergeConflictBounceCount: 2, branch: "fusion/fn-2084" }),
|
||||
makeTask({ mergeRetries: 3, mergeConflictBounceCount: 2, branch: "fusion/fn-2084" }),
|
||||
],
|
||||
});
|
||||
vi.mocked(aiMergeTask).mockRejectedValueOnce(new Error("merge conflict detected"));
|
||||
|
||||
const engine = createEngine(store);
|
||||
await runMergeCycle(engine);
|
||||
|
||||
expect(store.moveTask).not.toHaveBeenCalledWith(TASK_ID, "in-progress");
|
||||
expect(store.updateTask).toHaveBeenCalledWith(
|
||||
TASK_ID,
|
||||
expect.objectContaining({
|
||||
status: "failed",
|
||||
mergeRetries: 3,
|
||||
}),
|
||||
);
|
||||
expect(store.createTask).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ column: "triage", priority: "high" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("stores terminal merge metadata for non-conflict direct merge errors", async () => {
|
||||
const store = makeStore();
|
||||
vi.mocked(aiMergeTask).mockRejectedValueOnce(new Error("remote branch missing"));
|
||||
@@ -195,7 +240,7 @@ describe("ProjectEngine merge error recovery", () => {
|
||||
await runMergeCycle(engine);
|
||||
|
||||
expect(store.updateTask).toHaveBeenCalledWith(TASK_ID, {
|
||||
status: null,
|
||||
status: "failed",
|
||||
mergeRetries: 3,
|
||||
error: "remote branch missing",
|
||||
});
|
||||
@@ -238,7 +283,7 @@ describe("ProjectEngine merge error recovery", () => {
|
||||
|
||||
expect(processPullRequestMerge).toHaveBeenCalledTimes(1);
|
||||
expect(store.updateTask).toHaveBeenCalledWith(TASK_ID, {
|
||||
status: null,
|
||||
status: "failed",
|
||||
mergeRetries: 3,
|
||||
error: "PR API timeout",
|
||||
});
|
||||
|
||||
@@ -174,6 +174,12 @@ export class ProjectEngine {
|
||||
* a follow-up triage task so a fresh agent (or human) can investigate
|
||||
* the underlying flake/regression instead of looping forever. */
|
||||
private static readonly MAX_VERIFICATION_FAILURE_BOUNCES = 3;
|
||||
/** Cap on outer in-review→in-progress bounces caused by auto-merge conflict
|
||||
* retries being exhausted. After this many bounces the task is parked in
|
||||
* in-review with status=failed and a follow-up task is created, so the
|
||||
* 30-minute cooldown sweep cannot loop forever on a merge that requires
|
||||
* human intervention. */
|
||||
private static readonly MAX_MERGE_CONFLICT_BOUNCES = 2;
|
||||
/** 30-minute cooldown before a retry-exhausted task gets another sweep attempt */
|
||||
private static readonly AUTO_MERGE_COOLDOWN_MS = 30 * 60 * 1000;
|
||||
|
||||
@@ -938,6 +944,10 @@ export class ProjectEngine {
|
||||
// Already-confirmed merges always eligible — just need to move to done
|
||||
if (task.mergeDetails?.mergeConfirmed) return true;
|
||||
if (this.options.getTaskMergeBlocker?.(task as Task)) return false;
|
||||
// Terminal failure: don't let the cooldown sweep re-attempt a merge that
|
||||
// already gave up (verification cap, conflict-bounce cap, or non-conflict
|
||||
// error). The task is parked for human/follow-up intervention.
|
||||
if (task.status === "failed") return false;
|
||||
return (
|
||||
(task.mergeRetries ?? 0) < ProjectEngine.MAX_AUTO_MERGE_RETRIES ||
|
||||
this.hasAutoHealableVerificationBufferFailure(task) ||
|
||||
@@ -1153,6 +1163,20 @@ export class ProjectEngine {
|
||||
|
||||
runtimeLog.error(`${manualResolver ? "Manual" : "Auto"}-merge failed for ${taskId}: ${errorMsg}`);
|
||||
|
||||
// Surface every merge failure on the task log so the dashboard shows
|
||||
// *why* a merge didn't complete instead of silently looping.
|
||||
await store
|
||||
.logEntry(
|
||||
taskId,
|
||||
`${manualResolver ? "Manual" : "Auto"}-merge failed: ${errorMsg}`,
|
||||
err instanceof Error ? err.name : undefined,
|
||||
)
|
||||
.catch((logErr: unknown) => {
|
||||
runtimeLog.warn(
|
||||
`Auto-merge: failed to log merge-failure entry on ${taskId}: ${logErr instanceof Error ? logErr.message : String(logErr)}`,
|
||||
);
|
||||
});
|
||||
|
||||
// If this was a manual merge, reject the promise and skip auto-retry logic
|
||||
if (manualResolver) {
|
||||
this.manualMergeResolvers.delete(taskId);
|
||||
@@ -1276,23 +1300,122 @@ export class ProjectEngine {
|
||||
if (!this.shuttingDown) this.internalEnqueueMerge(taskId);
|
||||
}, delayMs);
|
||||
} else {
|
||||
// Max retries exceeded or auto-resolve disabled
|
||||
try {
|
||||
await store.updateTask(taskId, { status: null });
|
||||
} catch (recoveryErr) {
|
||||
runtimeLog.error(
|
||||
`Auto-merge: failed to clear status on ${taskId} after max retries exceeded: ${recoveryErr instanceof Error ? recoveryErr.message : String(recoveryErr)}`,
|
||||
);
|
||||
// Conflict retries exhausted (or auto-resolve disabled).
|
||||
// Previous behavior: silently clear status, leaving the task in
|
||||
// in-review with mergeRetries=MAX. The 30-min cooldown sweep
|
||||
// would then reset retries and re-attempt the same impossible
|
||||
// merge forever, with no error surface for the user.
|
||||
//
|
||||
// New behavior: bounce the task back to in-progress so the
|
||||
// executor can rebase against the latest main and retry. Cap
|
||||
// bounces at MAX_MERGE_CONFLICT_BOUNCES — past that, park in
|
||||
// in-review with status=failed and create a follow-up task so
|
||||
// a human can resolve the conflict manually.
|
||||
const previousBounces = taskOnErr.mergeConflictBounceCount ?? 0;
|
||||
const nextBounces = previousBounces + 1;
|
||||
const bounceCap = ProjectEngine.MAX_MERGE_CONFLICT_BOUNCES;
|
||||
const autoResolveDisabled =
|
||||
(settingsOnErr as Settings).autoResolveConflicts === false;
|
||||
|
||||
if (autoResolveDisabled || nextBounces > bounceCap) {
|
||||
// Park for human intervention.
|
||||
const reason = autoResolveDisabled
|
||||
? "autoResolveConflicts is disabled"
|
||||
: `merge-conflict bounce cap reached (${nextBounces - 1}/${bounceCap})`;
|
||||
try {
|
||||
await store.updateTask(taskId, {
|
||||
status: "failed",
|
||||
mergeRetries: ProjectEngine.MAX_AUTO_MERGE_RETRIES,
|
||||
error: `Auto-merge gave up: ${reason}. ${errorMsg}`,
|
||||
});
|
||||
await store.addTaskComment(
|
||||
taskId,
|
||||
`Auto-merge gave up after ${ProjectEngine.MAX_AUTO_MERGE_RETRIES} conflict-resolution retries (${reason}). ` +
|
||||
`Resolve the conflict on branch \`${taskOnErr.branch ?? "?"}\` manually, then unpause/retry.`,
|
||||
"agent",
|
||||
);
|
||||
await store.logEntry(
|
||||
taskId,
|
||||
`Auto-merge gave up after conflict retries exhausted (${reason}); task parked for human intervention`,
|
||||
"MergeConflictGiveUp",
|
||||
);
|
||||
if (!autoResolveDisabled) {
|
||||
// Create a follow-up only when we capped on bounces; if
|
||||
// auto-resolve is just disabled, the user is presumed to
|
||||
// be handling merges manually and a follow-up is noise.
|
||||
try {
|
||||
const followUp = await store.createTask({
|
||||
description:
|
||||
`Resolve auto-merge conflict on ${taskId} (${taskOnErr.title || "untitled"}). ` +
|
||||
`Auto-merge attempted to rebase + resolve ${nextBounces - 1} times against main and exhausted retries each pass. ` +
|
||||
`Branch: \`${taskOnErr.branch ?? "?"}\`. Worktree: \`${taskOnErr.worktree ?? "?"}\`. ` +
|
||||
`Last merge error: ${errorMsg}`,
|
||||
column: "triage",
|
||||
priority: "high",
|
||||
});
|
||||
await store.addTaskComment(
|
||||
taskId,
|
||||
`Created follow-up ${followUp.id} to track manual conflict resolution.`,
|
||||
"agent",
|
||||
);
|
||||
} catch (followUpErr) {
|
||||
runtimeLog.warn(
|
||||
`Auto-merge: failed to create follow-up for ${taskId}: ${followUpErr instanceof Error ? followUpErr.message : String(followUpErr)}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
} catch (recoveryErr) {
|
||||
runtimeLog.error(
|
||||
`Auto-merge: failed to park ${taskId} after conflict-bounce cap: ${recoveryErr instanceof Error ? recoveryErr.message : String(recoveryErr)}`,
|
||||
);
|
||||
}
|
||||
} else {
|
||||
// Bounce to in-progress for a fresh rebase + retry pass.
|
||||
try {
|
||||
await store.addTaskComment(
|
||||
taskId,
|
||||
`Auto-merge could not resolve conflicts within ${ProjectEngine.MAX_AUTO_MERGE_RETRIES} retries (bounce ${nextBounces}/${bounceCap}). ` +
|
||||
`Bouncing back to in-progress for a fresh rebase against main; the executor will re-run quality gates and re-attempt the merge.`,
|
||||
"agent",
|
||||
);
|
||||
await store.updateTask(taskId, {
|
||||
status: null,
|
||||
mergeRetries: 0,
|
||||
error: null,
|
||||
mergeConflictBounceCount: nextBounces,
|
||||
});
|
||||
await store.moveTask(taskId, "in-progress");
|
||||
await store.logEntry(
|
||||
taskId,
|
||||
`Auto-merge conflicts unresolved (${ProjectEngine.MAX_AUTO_MERGE_RETRIES}/${ProjectEngine.MAX_AUTO_MERGE_RETRIES}) — bounced to in-progress for re-rebase (bounce ${nextBounces}/${bounceCap})`,
|
||||
"MergeConflictBounce",
|
||||
);
|
||||
runtimeLog.log(
|
||||
`Auto-merge: ${taskId} conflict retries exhausted — bounced to in-progress (${nextBounces}/${bounceCap})`,
|
||||
);
|
||||
} catch (recoveryErr) {
|
||||
runtimeLog.error(
|
||||
`Auto-merge: failed to bounce ${taskId} after conflict exhaustion: ${recoveryErr instanceof Error ? recoveryErr.message : String(recoveryErr)}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// Non-conflict error — stop retrying until user intervenes
|
||||
// Non-conflict error — stop retrying until user intervenes.
|
||||
// Mark status=failed so the cooldown sweep won't silently
|
||||
// re-attempt; the catch-block-top logEntry already recorded the
|
||||
// failure on the task log.
|
||||
try {
|
||||
await store.updateTask(taskId, {
|
||||
status: null,
|
||||
status: "failed",
|
||||
mergeRetries: ProjectEngine.MAX_AUTO_MERGE_RETRIES,
|
||||
error: errorMsg,
|
||||
});
|
||||
await store.addTaskComment(
|
||||
taskId,
|
||||
`Auto-merge failed with a non-conflict error and stopped retrying: ${errorMsg}`,
|
||||
"agent",
|
||||
);
|
||||
} catch (recoveryErr) {
|
||||
runtimeLog.error(
|
||||
`Auto-merge: failed to update ${taskId} after non-conflict error: ${recoveryErr instanceof Error ? recoveryErr.message : String(recoveryErr)}`,
|
||||
@@ -1300,9 +1423,11 @@ export class ProjectEngine {
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// Non-direct merge strategy (e.g. pull-request) errored — park as
|
||||
// failed so the cooldown sweep stops re-attempting silently.
|
||||
try {
|
||||
await store.updateTask(taskId, {
|
||||
status: null,
|
||||
status: "failed",
|
||||
mergeRetries: ProjectEngine.MAX_AUTO_MERGE_RETRIES,
|
||||
error: errorMsg,
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user