feat(FN-4763): complete Step 4 — add conflict reclaim route test coverage
Fusion-Task-Id: FN-4763 Fusion-Task-Lineage: b3628e63-682b-4a39-9a7d-2c96278c5366
This commit is contained in:
committed by
gsxdsm
parent
f9bcf2acff
commit
76b1ba8178
@@ -2419,3 +2419,166 @@ describe("POST /tasks/:id/review/refresh", () => {
|
||||
|
||||
// --- Git Management route tests ---
|
||||
// These are integration tests that run against the actual git repository
|
||||
|
||||
describe("PR conflict refresh + reclaim routes", () => {
|
||||
let store: TaskStore;
|
||||
|
||||
beforeEach(() => {
|
||||
store = createMockStore({
|
||||
getTask: vi.fn(),
|
||||
updatePrInfo: vi.fn().mockResolvedValue(undefined),
|
||||
applyPrMergedTransition: vi.fn().mockResolvedValue(undefined),
|
||||
});
|
||||
mockIsGhAuthenticated.mockReturnValue(true);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
function buildApp(options?: Parameters<typeof createApiRoutes>[1]) {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use("/api", createApiRoutes(store, options));
|
||||
return app;
|
||||
}
|
||||
|
||||
it("queues conflict reclaim during refresh when mergeable is conflicting", async () => {
|
||||
const task = {
|
||||
...FAKE_TASK_DETAIL,
|
||||
id: "FN-900",
|
||||
branch: "fusion/fn-900",
|
||||
worktree: "/tmp/test/.worktrees/fn-900",
|
||||
prInfo: {
|
||||
url: "https://github.com/owner/repo/pull/900",
|
||||
number: 900,
|
||||
status: "open",
|
||||
title: "PR",
|
||||
headBranch: "fusion/fn-900",
|
||||
baseBranch: "main",
|
||||
commentCount: 0,
|
||||
},
|
||||
};
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(task);
|
||||
vi.spyOn(GitHubClient.prototype, "getPrReviewSnapshot").mockResolvedValue({
|
||||
decision: "approved",
|
||||
items: [],
|
||||
summary: { approved: 0, changesRequested: 0, commented: 0 },
|
||||
} as any);
|
||||
vi.spyOn(GitHubClient.prototype, "getPrMergeStatus").mockResolvedValue({
|
||||
mergeReady: false,
|
||||
blockingReasons: ["conflict"],
|
||||
checks: [],
|
||||
prInfo: { status: "open", merged: false, mergeable: "conflicting" },
|
||||
} as any);
|
||||
|
||||
const reclaimSpy = vi.fn().mockResolvedValue({ outcome: "reclaimed" });
|
||||
const engine = {
|
||||
getTaskStore: () => store,
|
||||
getSelfHealingManager: () => ({ reclaimPrConflictForTask: reclaimSpy }),
|
||||
};
|
||||
|
||||
const res = await REQUEST(buildApp({ engine } as any), "POST", `/api/tasks/${task.id}/pr/refresh`, JSON.stringify({}), { "content-type": "application/json" });
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.conflictReclaimQueued).toBe(true);
|
||||
expect(reclaimSpy).toHaveBeenCalledWith(task.id);
|
||||
});
|
||||
|
||||
it("does not queue reclaim during refresh for non-conflicting PR", async () => {
|
||||
const task = {
|
||||
...FAKE_TASK_DETAIL,
|
||||
id: "FN-901",
|
||||
branch: "fusion/fn-901",
|
||||
worktree: "/tmp/test/.worktrees/fn-901",
|
||||
prInfo: {
|
||||
url: "https://github.com/owner/repo/pull/901",
|
||||
number: 901,
|
||||
status: "open",
|
||||
title: "PR",
|
||||
headBranch: "fusion/fn-901",
|
||||
baseBranch: "main",
|
||||
commentCount: 0,
|
||||
},
|
||||
};
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(task);
|
||||
vi.spyOn(GitHubClient.prototype, "getPrReviewSnapshot").mockResolvedValue({
|
||||
decision: "approved",
|
||||
items: [],
|
||||
summary: { approved: 0, changesRequested: 0, commented: 0 },
|
||||
} as any);
|
||||
vi.spyOn(GitHubClient.prototype, "getPrMergeStatus").mockResolvedValue({
|
||||
mergeReady: true,
|
||||
blockingReasons: [],
|
||||
checks: [],
|
||||
prInfo: { status: "open", merged: false, mergeable: "clean" },
|
||||
} as any);
|
||||
|
||||
const reclaimSpy = vi.fn().mockResolvedValue({ outcome: "skipped" });
|
||||
const engine = {
|
||||
getTaskStore: () => store,
|
||||
getSelfHealingManager: () => ({ reclaimPrConflictForTask: reclaimSpy }),
|
||||
};
|
||||
|
||||
const res = await REQUEST(buildApp({ engine } as any), "POST", `/api/tasks/${task.id}/pr/refresh`, JSON.stringify({}), { "content-type": "application/json" });
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.conflictReclaimQueued).toBe(false);
|
||||
expect(reclaimSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("returns queued true for manual reclaim when conflict reclaim is available", async () => {
|
||||
const task = {
|
||||
...FAKE_TASK_DETAIL,
|
||||
id: "FN-904",
|
||||
branch: "fusion/fn-904",
|
||||
worktree: "/tmp/test/.worktrees/fn-904",
|
||||
prInfo: {
|
||||
url: "https://github.com/owner/repo/pull/904",
|
||||
number: 904,
|
||||
status: "open",
|
||||
title: "PR",
|
||||
headBranch: "fusion/fn-904",
|
||||
baseBranch: "main",
|
||||
commentCount: 0,
|
||||
mergeable: "conflicting",
|
||||
},
|
||||
};
|
||||
const reclaimSpy = vi.fn().mockResolvedValue({ outcome: "reclaimed" });
|
||||
const engine = {
|
||||
getTaskStore: () => store,
|
||||
getSelfHealingManager: () => ({ reclaimPrConflictForTask: reclaimSpy }),
|
||||
};
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(task);
|
||||
const res = await REQUEST(buildApp({ engine } as any), "POST", `/api/tasks/${task.id}/pr/reclaim-conflict`, JSON.stringify({}), { "content-type": "application/json" });
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toEqual({ queued: true });
|
||||
});
|
||||
|
||||
it("returns 404 for manual reclaim when task has no PR", async () => {
|
||||
const task = { ...FAKE_TASK_DETAIL, id: "FN-902", prInfo: undefined };
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(task);
|
||||
const res = await REQUEST(buildApp(), "POST", `/api/tasks/${task.id}/pr/reclaim-conflict`, JSON.stringify({}), { "content-type": "application/json" });
|
||||
expect(res.status).toBe(404);
|
||||
});
|
||||
|
||||
it("returns 409 for manual reclaim when task has no branch/worktree", async () => {
|
||||
const task = {
|
||||
...FAKE_TASK_DETAIL,
|
||||
id: "FN-903",
|
||||
branch: null,
|
||||
worktree: null,
|
||||
prInfo: {
|
||||
url: "https://github.com/owner/repo/pull/903",
|
||||
number: 903,
|
||||
status: "open",
|
||||
title: "PR",
|
||||
headBranch: "fusion/fn-903",
|
||||
baseBranch: "main",
|
||||
commentCount: 0,
|
||||
mergeable: "conflicting",
|
||||
},
|
||||
};
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(task);
|
||||
const res = await REQUEST(buildApp(), "POST", `/api/tasks/${task.id}/pr/reclaim-conflict`, JSON.stringify({}), { "content-type": "application/json" });
|
||||
expect(res.status).toBe(409);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1173,7 +1173,13 @@ async function mergeTaskPr(
|
||||
}
|
||||
}
|
||||
|
||||
export async function refreshPrInBackground(store: TaskStore, taskId: string, currentPrInfo: PrInfo, token?: string): Promise<void> {
|
||||
export async function refreshPrInBackground(
|
||||
store: TaskStore,
|
||||
taskId: string,
|
||||
currentPrInfo: PrInfo,
|
||||
token?: string,
|
||||
options?: { onConflictDetected?: (taskId: string) => Promise<void> },
|
||||
): Promise<void> {
|
||||
try {
|
||||
let owner: string;
|
||||
let repo: string;
|
||||
@@ -1210,6 +1216,7 @@ export async function refreshPrInBackground(store: TaskStore, taskId: string, cu
|
||||
const prInfo = {
|
||||
...prior,
|
||||
...mergeStatus.prInfo,
|
||||
mergeable: mergeStatus.prInfo.mergeable,
|
||||
autoMergeOnGreen: prior?.autoMergeOnGreen,
|
||||
autoMergeStrategy: prior?.autoMergeStrategy,
|
||||
lastMergeError: prior?.lastMergeError,
|
||||
@@ -1223,6 +1230,10 @@ export async function refreshPrInBackground(store: TaskStore, taskId: string, cu
|
||||
await syncPrReviewsToTask(store, task, reviewSnapshot);
|
||||
await applyChangesRequestedTransition(store, task, reviewSnapshot, prInfo);
|
||||
|
||||
if (prInfo.mergeable === "conflicting" && task?.branch && task?.worktree && options?.onConflictDetected) {
|
||||
await options.onConflictDetected(taskId);
|
||||
}
|
||||
|
||||
if (prInfo.status === "merged") {
|
||||
await store.applyPrMergedTransition(taskId, {
|
||||
agentId: "dashboard",
|
||||
@@ -3322,7 +3333,12 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void {
|
||||
|
||||
// Trigger background refresh if stale (don't await, let it run)
|
||||
if (isStale) {
|
||||
refreshPrInBackground(scopedStore, task.id, task.prInfo, githubToken);
|
||||
const selfHealingManager = engine?.getSelfHealingManager?.();
|
||||
refreshPrInBackground(scopedStore, task.id, task.prInfo, githubToken, {
|
||||
onConflictDetected: async (taskId) => {
|
||||
await selfHealingManager?.reclaimPrConflictForTask(taskId);
|
||||
},
|
||||
});
|
||||
}
|
||||
} catch (err: unknown) {
|
||||
if (err instanceof ApiError) {
|
||||
@@ -3343,7 +3359,7 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void {
|
||||
*/
|
||||
router.post("/tasks/:id/pr/refresh", async (req, res) => {
|
||||
try {
|
||||
const { store: scopedStore } = await getProjectContext(req);
|
||||
const { store: scopedStore, engine } = await getProjectContext(req);
|
||||
const task = await scopedStore.getTask(req.params.id);
|
||||
|
||||
if (!task.prInfo) {
|
||||
@@ -3395,6 +3411,7 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void {
|
||||
const prInfo: PrInfo = {
|
||||
...task.prInfo,
|
||||
...mergeStatus.prInfo,
|
||||
mergeable: mergeStatus.prInfo.mergeable,
|
||||
autoMergeOnGreen: task.prInfo.autoMergeOnGreen,
|
||||
autoMergeStrategy: task.prInfo.autoMergeStrategy,
|
||||
lastMergeError: task.prInfo.lastMergeError,
|
||||
@@ -3408,6 +3425,15 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void {
|
||||
await syncPrReviewsToTask(scopedStore, task, reviewSnapshot);
|
||||
await applyChangesRequestedTransition(scopedStore, task, reviewSnapshot, prInfo);
|
||||
|
||||
let conflictReclaimQueued = false;
|
||||
if (prInfo.mergeable === "conflicting" && task.branch && task.worktree) {
|
||||
const selfHealingManager = engine?.getSelfHealingManager?.();
|
||||
if (selfHealingManager) {
|
||||
await selfHealingManager.reclaimPrConflictForTask(task.id);
|
||||
conflictReclaimQueued = true;
|
||||
}
|
||||
}
|
||||
|
||||
if (prInfo.status === "merged") {
|
||||
await scopedStore.applyPrMergedTransition(task.id, {
|
||||
agentId: "dashboard",
|
||||
@@ -3425,10 +3451,12 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void {
|
||||
res.json({
|
||||
prInfo: refreshedTask.prInfo ?? prInfo,
|
||||
mergeReady: mergeStatus.mergeReady,
|
||||
mergeable: prInfo.mergeable,
|
||||
blockingReasons: mergeStatus.blockingReasons,
|
||||
reviewDecision: reviewSnapshot.decision,
|
||||
checks: mergeStatus.checks,
|
||||
automationStatus: refreshedTask.status ?? task.status ?? null,
|
||||
conflictReclaimQueued,
|
||||
});
|
||||
} catch (err: unknown) {
|
||||
if (err instanceof ApiError) {
|
||||
@@ -3444,6 +3472,30 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void {
|
||||
}
|
||||
});
|
||||
|
||||
router.post("/tasks/:id/pr/reclaim-conflict", async (req, res) => {
|
||||
try {
|
||||
const { store: scopedStore, engine } = await getProjectContext(req);
|
||||
const task = await scopedStore.getTask(req.params.id);
|
||||
if (!task.prInfo) {
|
||||
throw notFound("Task has no associated PR");
|
||||
}
|
||||
if (!task.branch || !task.worktree) {
|
||||
throw conflict("Task has no branch/worktree to reclaim");
|
||||
}
|
||||
|
||||
const selfHealingManager = engine?.getSelfHealingManager?.();
|
||||
if (!selfHealingManager) {
|
||||
return res.json({ queued: false, reason: "engine-unavailable" });
|
||||
}
|
||||
|
||||
await selfHealingManager.reclaimPrConflictForTask(task.id);
|
||||
res.json({ queued: true });
|
||||
} catch (err: unknown) {
|
||||
if (err instanceof ApiError) throw err;
|
||||
rethrowAsApiError(err, "Failed to queue PR conflict reclaim");
|
||||
}
|
||||
});
|
||||
|
||||
router.post("/tasks/:id/pr/merge", async (req, res) => {
|
||||
try {
|
||||
const method = req.body?.method;
|
||||
|
||||
@@ -28,9 +28,19 @@ function makeTask(overrides: Partial<Task> = {}): Task {
|
||||
} as Task;
|
||||
}
|
||||
|
||||
function makeStore(task: Task | null, paused = false, enginePaused = false): TaskStore & EventEmitter {
|
||||
function makeStore(
|
||||
task: Task | null,
|
||||
paused = false,
|
||||
enginePaused = false,
|
||||
settingsOverrides: Partial<Settings> = {},
|
||||
): TaskStore & EventEmitter {
|
||||
const emitter = new EventEmitter();
|
||||
const settings = { globalPause: paused, enginePaused, autoRecovery: { mode: "deterministic-only", maxRetries: 3 } } as Settings;
|
||||
const settings = {
|
||||
globalPause: paused,
|
||||
enginePaused,
|
||||
autoRecovery: { mode: "deterministic-only", maxRetries: 3 },
|
||||
...settingsOverrides,
|
||||
} as Settings;
|
||||
return Object.assign(emitter, {
|
||||
getSettings: vi.fn(async () => settings),
|
||||
getTask: vi.fn((id: string) => (task && id === task.id ? task : null)),
|
||||
@@ -158,6 +168,24 @@ describe("SelfHealingManager.reclaimPrConflictForTask", () => {
|
||||
expect(result).toEqual({ outcome: "skipped", reason: "missing-branch-or-worktree" });
|
||||
});
|
||||
|
||||
it("skips checked out tasks", async () => {
|
||||
const task = makeTask({ checkedOutBy: "agent-1" as any });
|
||||
const store = makeStore(task);
|
||||
const manager = new SelfHealingManager(store as any, { rootDir: "/tmp/test" } as any);
|
||||
const result = await manager.reclaimPrConflictForTask(task.id);
|
||||
expect(result).toEqual({ outcome: "skipped", reason: "checked-out" });
|
||||
});
|
||||
|
||||
it("only sweeps tasks marked as conflicting", async () => {
|
||||
const task = makeTask({ prInfo: { ...makeTask().prInfo!, mergeable: "clean" } as any });
|
||||
const store = makeStore(task, false, false, { worktrunk: { enabled: true } as any });
|
||||
const manager = new SelfHealingManager(store as any, { rootDir: "/tmp/test" } as any);
|
||||
const reclaimSpy = vi.spyOn(manager, "reclaimPrConflictForTask");
|
||||
const reclaimed = await manager.reclaimPrConflicts();
|
||||
expect(reclaimed).toBe(0);
|
||||
expect(reclaimSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("skips when worktree has an active session", async () => {
|
||||
const task = makeTask();
|
||||
const store = makeStore(task);
|
||||
|
||||
Reference in New Issue
Block a user