feat(KB-010): add autoResolveConflicts setting with intelligent merge conflict resolution
- Add autoResolveConflicts setting to types and store (default: true) - Implement smart conflict detection for lock files and generated files - Add 3-attempt retry logic with escalating strategies to merger - Auto-resolve lock files using 'ours', trivial whitespace conflicts - Track mergeRetries per task for retry loop management - Update AGENTS.md with conflict resolution documentation - Add comprehensive merger tests for retry scenarios
This commit is contained in:
gsxdsm
@@ -20,6 +20,8 @@ function makeMockStore() {
|
||||
pollIntervalMs: 60_000,
|
||||
}),
|
||||
listTasks: vi.fn().mockResolvedValue([]),
|
||||
getTask: vi.fn().mockResolvedValue({ column: "in-review", paused: false }),
|
||||
updateTask: vi.fn().mockResolvedValue({}),
|
||||
on: vi.fn((event: string, handler: (...args: unknown[]) => void) => {
|
||||
emitter.on(event, handler);
|
||||
}),
|
||||
@@ -726,3 +728,201 @@ describe("runDashboard — --paused flag", () => {
|
||||
expect(pausedMessageCalls).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
// ── Merge conflict retry logic tests ────────────────────────────────────
|
||||
|
||||
describe("runDashboard — merge conflict retry logic", () => {
|
||||
let mockStore: ReturnType<typeof makeMockStore>;
|
||||
let consoleSpy: ReturnType<typeof vi.spyOn>;
|
||||
|
||||
beforeEach(async () => {
|
||||
capturedExecutorOpts = undefined;
|
||||
vi.clearAllMocks();
|
||||
mockStore = makeMockStore();
|
||||
const { TaskStore } = await import("@kb/core");
|
||||
(TaskStore as ReturnType<typeof vi.fn>).mockImplementation(() => mockStore);
|
||||
|
||||
// Default mock store.getTask implementation
|
||||
mockStore.getTask = vi.fn().mockImplementation(async (id: string) => ({
|
||||
id,
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
mergeRetries: 0,
|
||||
}));
|
||||
|
||||
const engine = await import("@kb/engine");
|
||||
(engine.aiMergeTask as ReturnType<typeof vi.fn>).mockImplementation(() =>
|
||||
Promise.resolve({ merged: true }),
|
||||
);
|
||||
(engine.TaskExecutor as unknown as ReturnType<typeof vi.fn>).mockImplementation(
|
||||
(_store: unknown, _cwd: unknown, opts: unknown) => {
|
||||
capturedExecutorOpts = opts as Record<string, unknown>;
|
||||
return { resumeOrphaned: vi.fn().mockResolvedValue(undefined) };
|
||||
},
|
||||
);
|
||||
consoleSpy = vi.spyOn(console, "log").mockImplementation(() => {});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
consoleSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("increments mergeRetries and re-enqueues on conflict error", async () => {
|
||||
const { aiMergeTask } = await import("@kb/engine");
|
||||
|
||||
// Simulate merge failure with conflict
|
||||
(aiMergeTask as ReturnType<typeof vi.fn>).mockRejectedValue(
|
||||
new Error("Merge conflict detected in package-lock.json"),
|
||||
);
|
||||
|
||||
mockStore.getSettings.mockResolvedValue({
|
||||
maxConcurrent: 1,
|
||||
maxWorktrees: 2,
|
||||
autoMerge: true,
|
||||
autoResolveConflicts: true,
|
||||
pollIntervalMs: 60_000,
|
||||
enginePaused: false,
|
||||
globalPause: false,
|
||||
});
|
||||
|
||||
mockStore.listTasks.mockResolvedValue([
|
||||
{ id: "KB-RETRY", column: "in-review", paused: false },
|
||||
]);
|
||||
|
||||
await runDashboard(0, { open: false });
|
||||
|
||||
// Wait for retry scheduling
|
||||
await new Promise((r) => setTimeout(r, 100));
|
||||
|
||||
// Should have incremented mergeRetries
|
||||
expect(mockStore.updateTask).toHaveBeenCalledWith(
|
||||
"KB-RETRY",
|
||||
expect.objectContaining({ mergeRetries: 1 }),
|
||||
);
|
||||
|
||||
// Should log retry attempt
|
||||
const retryLog = consoleSpy.mock.calls.find(
|
||||
(call) => typeof call[0] === "string" && call[0].includes("retry 1/3"),
|
||||
);
|
||||
expect(retryLog).toBeDefined();
|
||||
});
|
||||
|
||||
it("gives up after max retries (3) exceeded", async () => {
|
||||
const { aiMergeTask } = await import("@kb/engine");
|
||||
|
||||
(aiMergeTask as ReturnType<typeof vi.fn>).mockRejectedValue(
|
||||
new Error("Merge conflict detected"),
|
||||
);
|
||||
|
||||
mockStore.getSettings.mockResolvedValue({
|
||||
maxConcurrent: 1,
|
||||
maxWorktrees: 2,
|
||||
autoMerge: true,
|
||||
autoResolveConflicts: true,
|
||||
pollIntervalMs: 60_000,
|
||||
enginePaused: false,
|
||||
globalPause: false,
|
||||
});
|
||||
|
||||
// Task already has 3 retries
|
||||
mockStore.getTask = vi.fn().mockImplementation(async (id: string) => ({
|
||||
id,
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
mergeRetries: 3,
|
||||
}));
|
||||
|
||||
mockStore.listTasks.mockResolvedValue([
|
||||
{ id: "KB-MAX", column: "in-review", paused: false, mergeRetries: 3 },
|
||||
]);
|
||||
|
||||
await runDashboard(0, { open: false });
|
||||
|
||||
await new Promise((r) => setTimeout(r, 50));
|
||||
|
||||
// Should log max retries exceeded
|
||||
const maxRetryLog = consoleSpy.mock.calls.find(
|
||||
(call) =>
|
||||
typeof call[0] === "string" && call[0].includes("max retries (3) exceeded"),
|
||||
);
|
||||
expect(maxRetryLog).toBeDefined();
|
||||
|
||||
// Should reset mergeRetries on the task
|
||||
expect(mockStore.updateTask).toHaveBeenCalledWith(
|
||||
"KB-MAX",
|
||||
expect.objectContaining({ status: null }),
|
||||
);
|
||||
});
|
||||
|
||||
it("skips retry when autoResolveConflicts is disabled", async () => {
|
||||
const { aiMergeTask } = await import("@kb/engine");
|
||||
|
||||
(aiMergeTask as ReturnType<typeof vi.fn>).mockRejectedValue(
|
||||
new Error("Merge conflict detected"),
|
||||
);
|
||||
|
||||
mockStore.getSettings.mockResolvedValue({
|
||||
maxConcurrent: 1,
|
||||
maxWorktrees: 2,
|
||||
autoMerge: true,
|
||||
autoResolveConflicts: false, // Disabled
|
||||
pollIntervalMs: 60_000,
|
||||
enginePaused: false,
|
||||
globalPause: false,
|
||||
});
|
||||
|
||||
mockStore.listTasks.mockResolvedValue([
|
||||
{ id: "KB-NO-AUTO", column: "in-review", paused: false },
|
||||
]);
|
||||
|
||||
await runDashboard(0, { open: false });
|
||||
|
||||
await new Promise((r) => setTimeout(r, 50));
|
||||
|
||||
// Should log that auto-resolve is disabled
|
||||
const disabledLog = consoleSpy.mock.calls.find(
|
||||
(call) =>
|
||||
typeof call[0] === "string" &&
|
||||
call[0].includes("autoResolveConflicts disabled"),
|
||||
);
|
||||
expect(disabledLog).toBeDefined();
|
||||
});
|
||||
|
||||
it("clears mergeRetries on successful merge after retries", async () => {
|
||||
const { aiMergeTask } = await import("@kb/engine");
|
||||
|
||||
(aiMergeTask as ReturnType<typeof vi.fn>).mockResolvedValue({ merged: true });
|
||||
|
||||
mockStore.getSettings.mockResolvedValue({
|
||||
maxConcurrent: 1,
|
||||
maxWorktrees: 2,
|
||||
autoMerge: true,
|
||||
autoResolveConflicts: true,
|
||||
pollIntervalMs: 60_000,
|
||||
enginePaused: false,
|
||||
globalPause: false,
|
||||
});
|
||||
|
||||
// Task had previous retries
|
||||
mockStore.getTask = vi.fn().mockImplementation(async (id: string) => ({
|
||||
id,
|
||||
column: "in-review",
|
||||
paused: false,
|
||||
mergeRetries: 2,
|
||||
}));
|
||||
|
||||
mockStore.listTasks.mockResolvedValue([
|
||||
{ id: "KB-SUCCESS", column: "in-review", paused: false, mergeRetries: 2 },
|
||||
]);
|
||||
|
||||
await runDashboard(0, { open: false });
|
||||
|
||||
await new Promise((r) => setTimeout(r, 100));
|
||||
|
||||
// Should clear mergeRetries on success
|
||||
expect(mockStore.updateTask).toHaveBeenCalledWith(
|
||||
"KB-SUCCESS",
|
||||
expect.objectContaining({ mergeRetries: 0 }),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -164,12 +164,53 @@ export async function runDashboard(port: number, opts: { open?: boolean; paused?
|
||||
console.log(`[auto-merge] Merging ${taskId}...`);
|
||||
await onMerge(taskId);
|
||||
console.log(`[auto-merge] ✓ ${taskId} merged`);
|
||||
// Clear mergeRetries on success
|
||||
if (task.mergeRetries && task.mergeRetries > 0) {
|
||||
await store.updateTask(taskId, { mergeRetries: 0 });
|
||||
}
|
||||
} catch (err: any) {
|
||||
console.log(`[auto-merge] ✗ ${taskId}: ${err.message ?? err}`);
|
||||
// Reset task status so it doesn't appear stuck as "merging" in the UI
|
||||
try {
|
||||
await store.updateTask(taskId, { status: null });
|
||||
} catch { /* best-effort */ }
|
||||
const errorMsg = err.message ?? String(err);
|
||||
console.log(`[auto-merge] ✗ ${taskId}: ${errorMsg}`);
|
||||
|
||||
// Check if this is a conflict error and if we should retry
|
||||
const isConflictError = errorMsg.includes("conflict") || errorMsg.includes("Conflict");
|
||||
const task = await store.getTask(taskId).catch(() => null);
|
||||
|
||||
if (task && isConflictError) {
|
||||
const settings = await store.getSettings().catch(() => ({ autoResolveConflicts: true }));
|
||||
const currentRetries = task.mergeRetries ?? 0;
|
||||
const maxRetries = 3;
|
||||
|
||||
if (settings.autoResolveConflicts !== false && currentRetries < maxRetries) {
|
||||
// Increment retry counter and re-enqueue with delay
|
||||
const newRetryCount = currentRetries + 1;
|
||||
await store.updateTask(taskId, { mergeRetries: newRetryCount, status: null });
|
||||
|
||||
// Calculate exponential backoff delay: 5s, 10s, 20s
|
||||
const delayMs = 5000 * Math.pow(2, currentRetries);
|
||||
console.log(`[auto-merge] ↻ ${taskId}: retry ${newRetryCount}/${maxRetries} in ${delayMs / 1000}s`);
|
||||
|
||||
setTimeout(() => {
|
||||
enqueueMerge(taskId);
|
||||
}, delayMs);
|
||||
} else {
|
||||
// Max retries exceeded or auto-resolve disabled - keep in in-review
|
||||
if (currentRetries >= maxRetries) {
|
||||
console.log(`[auto-merge] ⊘ ${taskId}: max retries (${maxRetries}) exceeded — manual resolution required`);
|
||||
} else {
|
||||
console.log(`[auto-merge] ⊘ ${taskId}: autoResolveConflicts disabled — manual resolution required`);
|
||||
}
|
||||
// Reset task status so it doesn't appear stuck as "merging" in the UI
|
||||
try {
|
||||
await store.updateTask(taskId, { status: null });
|
||||
} catch { /* best-effort */ }
|
||||
}
|
||||
} else {
|
||||
// Non-conflict error - reset task status
|
||||
try {
|
||||
await store.updateTask(taskId, { status: null });
|
||||
} catch { /* best-effort */ }
|
||||
}
|
||||
} finally {
|
||||
mergeActive.delete(taskId);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user