feat(FN-4966): complete Step 4 — wire diagnostics into PR refresh routes
Fusion-Task-Id: FN-4966 Fusion-Task-Lineage: 8f5ea300-feaa-469c-aeed-f7304a5a1b6d
This commit is contained in:
@@ -65,6 +65,7 @@ import type {
|
||||
ResearchRunStatus,
|
||||
TaskPriority,
|
||||
TaskSourceIssue,
|
||||
PrConflictDiagnostics,
|
||||
PrInfo,
|
||||
ManagedDockerNodeInput,
|
||||
DockerNodeConfig,
|
||||
@@ -2199,6 +2200,7 @@ export interface PrStatusResponse {
|
||||
|
||||
export interface PrRefreshResponse {
|
||||
prInfo: PrInfo;
|
||||
conflictDiagnostics?: PrConflictDiagnostics;
|
||||
mergeReady: boolean;
|
||||
mergeable?: PrInfo["mergeable"];
|
||||
blockingReasons: string[];
|
||||
|
||||
@@ -2426,6 +2426,7 @@ describe("PR conflict refresh + reclaim routes", () => {
|
||||
beforeEach(() => {
|
||||
store = createMockStore({
|
||||
getTask: vi.fn(),
|
||||
getSettings: vi.fn().mockResolvedValue({ directMergeCommitStrategy: "auto" }),
|
||||
updatePrInfo: vi.fn().mockResolvedValue(undefined),
|
||||
applyPrMergedTransition: vi.fn().mockResolvedValue(undefined),
|
||||
});
|
||||
@@ -2469,8 +2470,13 @@ describe("PR conflict refresh + reclaim routes", () => {
|
||||
mergeReady: false,
|
||||
blockingReasons: ["conflict"],
|
||||
checks: [],
|
||||
prInfo: { status: "open", merged: false, mergeable: "conflicting" },
|
||||
prInfo: { status: "open", merged: false, mergeable: "conflicting", headBranch: "fusion/fn-900", baseBranch: "main" },
|
||||
} as any);
|
||||
vi.spyOn(GitHubClient.prototype, "getPrConflictDiagnostics").mockResolvedValue({
|
||||
conflictingFiles: ["packages/dashboard/src/github.ts"],
|
||||
suggestedCommands: ["git fetch origin"],
|
||||
capturedAt: "2026-05-18T00:00:00.000Z",
|
||||
});
|
||||
|
||||
const reclaimSpy = vi.fn().mockResolvedValue({ outcome: "reclaimed" });
|
||||
const engine = {
|
||||
@@ -2481,6 +2487,14 @@ describe("PR conflict refresh + reclaim routes", () => {
|
||||
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(store.updatePrInfo).toHaveBeenCalledWith(
|
||||
task.id,
|
||||
expect.objectContaining({
|
||||
conflictDiagnostics: expect.objectContaining({
|
||||
conflictingFiles: ["packages/dashboard/src/github.ts"],
|
||||
}),
|
||||
}),
|
||||
);
|
||||
expect(reclaimSpy).toHaveBeenCalledWith(task.id);
|
||||
});
|
||||
|
||||
@@ -2525,6 +2539,97 @@ describe("PR conflict refresh + reclaim routes", () => {
|
||||
expect(reclaimSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("clears persisted conflict diagnostics when PR becomes clean", async () => {
|
||||
const task = {
|
||||
...FAKE_TASK_DETAIL,
|
||||
id: "FN-905",
|
||||
branch: "fusion/fn-905",
|
||||
worktree: "/tmp/test/.worktrees/fn-905",
|
||||
prInfo: {
|
||||
url: "https://github.com/owner/repo/pull/905",
|
||||
number: 905,
|
||||
status: "open",
|
||||
title: "PR",
|
||||
headBranch: "fusion/fn-905",
|
||||
baseBranch: "main",
|
||||
commentCount: 0,
|
||||
mergeable: "conflicting",
|
||||
conflictDiagnostics: {
|
||||
conflictingFiles: ["stale.txt"],
|
||||
suggestedCommands: ["git fetch origin"],
|
||||
capturedAt: "2026-01-01T00:00:00.000Z",
|
||||
},
|
||||
},
|
||||
};
|
||||
(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", headBranch: "fusion/fn-905", baseBranch: "main" },
|
||||
} as any);
|
||||
|
||||
const res = await REQUEST(buildApp(), "POST", `/api/tasks/${task.id}/pr/refresh`, JSON.stringify({}), { "content-type": "application/json" });
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(store.updatePrInfo).toHaveBeenCalledWith(
|
||||
task.id,
|
||||
expect.objectContaining({
|
||||
mergeable: "clean",
|
||||
conflictDiagnostics: undefined,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("returns fallback diagnostics when repoRoot is unavailable", async () => {
|
||||
const task = {
|
||||
...FAKE_TASK_DETAIL,
|
||||
id: "FN-906",
|
||||
branch: "fusion/fn-906",
|
||||
worktree: "/tmp/test/.worktrees/fn-906",
|
||||
prInfo: {
|
||||
url: "https://github.com/owner/repo/pull/906",
|
||||
number: 906,
|
||||
status: "open",
|
||||
title: "PR",
|
||||
headBranch: "fusion/fn-906",
|
||||
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", headBranch: "fusion/fn-906", baseBranch: "main" },
|
||||
} as any);
|
||||
vi.spyOn(GitHubClient.prototype, "getPrConflictDiagnostics").mockResolvedValue({
|
||||
conflictingFiles: ["a.ts"],
|
||||
suggestedCommands: ["git fetch origin", "# Note: file list reflects PR changes; resolve conflicts as reported by git status during rebase."],
|
||||
capturedAt: "2026-05-18T00:00:00.000Z",
|
||||
});
|
||||
|
||||
const res = await REQUEST(buildApp(), "POST", `/api/tasks/${task.id}/pr/refresh`, JSON.stringify({}), { "content-type": "application/json" });
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(store.updatePrInfo).toHaveBeenCalledWith(
|
||||
task.id,
|
||||
expect.objectContaining({
|
||||
conflictDiagnostics: expect.objectContaining({
|
||||
conflictingFiles: ["a.ts"],
|
||||
suggestedCommands: expect.any(Array),
|
||||
}),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("returns queued true for manual reclaim when conflict reclaim is available", async () => {
|
||||
const task = {
|
||||
...FAKE_TASK_DETAIL,
|
||||
|
||||
@@ -5,6 +5,7 @@ import type {
|
||||
BatchStatusEntry,
|
||||
BatchStatusResponse,
|
||||
BatchStatusResult,
|
||||
DirectMergeCommitStrategy,
|
||||
IssueInfo,
|
||||
PrInfo,
|
||||
RunAuditEventInput,
|
||||
@@ -1205,7 +1206,11 @@ export async function refreshPrInBackground(
|
||||
taskId: string,
|
||||
currentPrInfo: PrInfo,
|
||||
token?: string,
|
||||
options?: { onConflictDetected?: (taskId: string) => Promise<void> },
|
||||
options?: {
|
||||
onConflictDetected?: (taskId: string) => Promise<void>;
|
||||
repoRoot?: string;
|
||||
directMergeCommitStrategy?: DirectMergeCommitStrategy;
|
||||
},
|
||||
): Promise<void> {
|
||||
try {
|
||||
let owner: string;
|
||||
@@ -1240,10 +1245,27 @@ export async function refreshPrInBackground(
|
||||
const reviewSnapshot = await client.getPrReviewSnapshot(owner, repo, currentPrInfo.number);
|
||||
const mergeStatus = await client.getPrMergeStatus(owner, repo, currentPrInfo.number);
|
||||
const prior = task.prInfo;
|
||||
let conflictDiagnostics = mergeStatus.prInfo.conflictDiagnostics;
|
||||
if (mergeStatus.prInfo.mergeable === "conflicting" && mergeStatus.prInfo.headBranch && mergeStatus.prInfo.baseBranch) {
|
||||
try {
|
||||
conflictDiagnostics = await client.getPrConflictDiagnostics(owner, repo, currentPrInfo.number, {
|
||||
baseBranch: mergeStatus.prInfo.baseBranch,
|
||||
headBranch: mergeStatus.prInfo.headBranch,
|
||||
repoRoot: options?.repoRoot,
|
||||
directMergeCommitStrategy: options?.directMergeCommitStrategy,
|
||||
});
|
||||
} catch (err) {
|
||||
console.error("[pr-conflict-diagnostics]", err);
|
||||
}
|
||||
} else {
|
||||
conflictDiagnostics = undefined;
|
||||
}
|
||||
|
||||
const prInfo = {
|
||||
...prior,
|
||||
...mergeStatus.prInfo,
|
||||
mergeable: mergeStatus.prInfo.mergeable,
|
||||
conflictDiagnostics,
|
||||
autoMergeOnGreen: prior?.autoMergeOnGreen,
|
||||
autoMergeStrategy: prior?.autoMergeStrategy,
|
||||
lastMergeError: prior?.lastMergeError,
|
||||
@@ -3360,7 +3382,11 @@ 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 settings = await scopedStore.getSettings();
|
||||
refreshPrInBackground(scopedStore, task.id, task.prInfo, githubToken, {
|
||||
repoRoot: scopedStore.getRootDir(),
|
||||
directMergeCommitStrategy: settings.directMergeCommitStrategy,
|
||||
});
|
||||
}
|
||||
} catch (err: unknown) {
|
||||
if (err instanceof ApiError) {
|
||||
@@ -3429,11 +3455,29 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void {
|
||||
const client = new GitHubClient();
|
||||
const reviewSnapshot = await client.getPrReviewSnapshot(owner, repo, task.prInfo.number);
|
||||
const mergeStatus = await client.getPrMergeStatus(owner, repo, task.prInfo.number);
|
||||
const settings = await scopedStore.getSettings();
|
||||
|
||||
let conflictDiagnostics = mergeStatus.prInfo.conflictDiagnostics;
|
||||
if (mergeStatus.prInfo.mergeable === "conflicting" && mergeStatus.prInfo.headBranch && mergeStatus.prInfo.baseBranch) {
|
||||
try {
|
||||
conflictDiagnostics = await client.getPrConflictDiagnostics(owner, repo, task.prInfo.number, {
|
||||
baseBranch: mergeStatus.prInfo.baseBranch,
|
||||
headBranch: mergeStatus.prInfo.headBranch,
|
||||
repoRoot: scopedStore.getRootDir(),
|
||||
directMergeCommitStrategy: settings.directMergeCommitStrategy,
|
||||
});
|
||||
} catch (err) {
|
||||
console.error("[pr-conflict-diagnostics]", err);
|
||||
}
|
||||
} else {
|
||||
conflictDiagnostics = undefined;
|
||||
}
|
||||
|
||||
const prInfo: PrInfo = {
|
||||
...task.prInfo,
|
||||
...mergeStatus.prInfo,
|
||||
mergeable: mergeStatus.prInfo.mergeable,
|
||||
conflictDiagnostics,
|
||||
autoMergeOnGreen: task.prInfo.autoMergeOnGreen,
|
||||
autoMergeStrategy: task.prInfo.autoMergeStrategy,
|
||||
lastMergeError: task.prInfo.lastMergeError,
|
||||
@@ -3476,6 +3520,7 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void {
|
||||
const refreshedTask = await scopedStore.getTask(task.id);
|
||||
res.json({
|
||||
prInfo: refreshedTask.prInfo ?? prInfo,
|
||||
conflictDiagnostics: (refreshedTask.prInfo ?? prInfo).conflictDiagnostics,
|
||||
mergeReady: mergeStatus.mergeReady,
|
||||
mergeable: prInfo.mergeable,
|
||||
blockingReasons: mergeStatus.blockingReasons,
|
||||
|
||||
Reference in New Issue
Block a user