From df01ab71ddc1509e124f7a0abafbe7cd713ed42f Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sat, 13 Jun 2026 17:46:11 -0700 Subject: [PATCH] FN-6402: detect PR preflight conflicts by exit code Use merge-tree exit status and empty-commit handling so PR conflict remediation reports clean states correctly. - Treat git merge-tree exit code 1, not stdout, as the PR preflight conflict signal. - Skip conflict-resolution commits when the branch already contains the selected base or the AI pass leaves no staged changes. - Cover the preflight and resolver no-op paths with dashboard route and conflict resolver tests. - Document the no-empty-commit conflict remediation behavior and add a published package changeset. Files changed: .changeset/fn-6402-pr-conflict-detection.md | 5 + docs/dashboard-guide.md | 2 +- .../src/__tests__/pr-conflict-resolver.test.ts | 130 +++++++++++++++++++++ ...it-github.pr-options-preflight-metadata.test.ts | 22 +++- .../register-git-github.pr-push-branch.test.ts | 2 +- ...egister-git-github.pr-resolve-conflicts.test.ts | 2 +- packages/dashboard/src/pr-conflict-resolver.ts | 57 +++++++-- .../dashboard/src/routes/register-git-github.ts | 8 +- 8 files changed, 212 insertions(+), 16 deletions(-) Fusion-Task-Id: FN-6402 Fusion-Task-Lineage: 9e709fb8-204a-4d88-8351-260a356c7520 --- .changeset/fn-6402-pr-conflict-detection.md | 5 + docs/dashboard-guide.md | 2 +- .../__tests__/pr-conflict-resolver.test.ts | 130 ++++++++++++++++++ ...thub.pr-options-preflight-metadata.test.ts | 22 ++- ...register-git-github.pr-push-branch.test.ts | 2 +- ...er-git-github.pr-resolve-conflicts.test.ts | 2 +- .../dashboard/src/pr-conflict-resolver.ts | 57 ++++++-- .../src/routes/register-git-github.ts | 8 +- 8 files changed, 212 insertions(+), 16 deletions(-) create mode 100644 .changeset/fn-6402-pr-conflict-detection.md create mode 100644 packages/dashboard/src/__tests__/pr-conflict-resolver.test.ts diff --git a/.changeset/fn-6402-pr-conflict-detection.md b/.changeset/fn-6402-pr-conflict-detection.md new file mode 100644 index 0000000000..038bddc909 --- /dev/null +++ b/.changeset/fn-6402-pr-conflict-detection.md @@ -0,0 +1,5 @@ +--- +"@runfusion/fusion": patch +--- + +Fix Create Pull Request conflict preflight to derive `conflictsWithBase` from `git merge-tree --write-tree` exit codes instead of non-empty output, and treat no-op PR conflict resolution merges as successful without attempting an empty commit. diff --git a/docs/dashboard-guide.md b/docs/dashboard-guide.md index b8aea81326..6c0a5a9325 100644 --- a/docs/dashboard-guide.md +++ b/docs/dashboard-guide.md @@ -708,7 +708,7 @@ Inspect task definition, logs, review feedback, comments, documents, workflow ou - From this section you can explicitly enable/disable tracking and manage a per-task repo override (`owner/repo`). Clearing the override saves `null` and falls back to project/global defaults. - In `in-review`, pull-request controls/status (including stall badges) are in a dedicated **Pull Request** tab instead of the Definition tab. - Task Detail and list split-pane PR affordances follow the live project auto-merge setting: when auto-merge is off, manual **Create PR** / merge actions are shown; when it is on, the tab shows the automatic auto-merge hint unless a per-task override changes the effective behavior. -- The **Create Pull Request** modal now offers in-app remediation for every blocking preflight check. If `branchOnRemote` is false, use **Push branch to remote** and Fusion will publish `fusion/` to `origin` and refresh preflight. If `conflictsWithBase` is true, use **Resolve conflicts with AI** and Fusion will use an AI coding agent to resolve merge markers on the task branch, commit the result, push the branch, and refresh preflight so normal PR creation can continue once all checks pass. +- The **Create Pull Request** modal now offers in-app remediation for every blocking preflight check. If `branchOnRemote` is false, use **Push branch to remote** and Fusion will publish `fusion/` to `origin` and refresh preflight. If `conflictsWithBase` is true, use **Resolve conflicts with AI** and Fusion will use an AI coding agent to resolve merge markers on the task branch, commit and push real merge changes, or report success without an empty commit when the selected base is already merged; preflight then refreshes so normal PR creation can continue once all checks pass. - The modal shell renders immediately: preflight checks and PR options load independently of AI-generated title/body metadata, so slow AI suggestions no longer block base-branch selection, diagnostics, or manual PR authoring. - AI title/body generation is bounded to 60 seconds and is canceled if the dialog request disconnects; on timeout/cancel, Fusion falls back to deterministic task-based PR title/body content instead of leaving the spinner stuck forever. - The **Review** tab is separate from **Comments**: Review shows actionable PR/reviewer feedback and same-task revision controls, while Comments remains the general collaboration thread. diff --git a/packages/dashboard/src/__tests__/pr-conflict-resolver.test.ts b/packages/dashboard/src/__tests__/pr-conflict-resolver.test.ts new file mode 100644 index 0000000000..68a9effb6e --- /dev/null +++ b/packages/dashboard/src/__tests__/pr-conflict-resolver.test.ts @@ -0,0 +1,130 @@ +// @vitest-environment node + +import { mkdtemp, rm } from "node:fs/promises"; +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { Settings, Task, TaskStore } from "@fusion/core"; + +const { mockRunGitCommand, mockCreateResolvedAgentSession } = vi.hoisted(() => ({ + mockRunGitCommand: vi.fn(), + mockCreateResolvedAgentSession: vi.fn(), +})); + +vi.mock("../routes/resolve-diff-base.js", () => ({ + runGitCommand: mockRunGitCommand, +})); + +vi.mock("@fusion/engine", () => ({ + createResolvedAgentSession: mockCreateResolvedAgentSession, +})); + +import { resolvePrConflicts } from "../pr-conflict-resolver.js"; + +function createTask(overrides: Partial = {}): Task { + return { + id: "FN-001", + title: "Task", + description: "desc", + column: "in-review", + status: "in-review", + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + comments: [], + ...overrides, + } as Task; +} + +function createStore(task: Task): TaskStore { + return { + getTask: vi.fn().mockResolvedValue(task), + logEntry: vi.fn().mockResolvedValue(undefined), + } as unknown as TaskStore; +} + +const settings = { + defaultProvider: "mock", + defaultModelId: "scripted", +} as Settings; + +async function createRootDir(): Promise { + return mkdtemp(join(tmpdir(), "fusion-pr-conflict-resolver-")); +} + +describe("resolvePrConflicts", () => { + const rootDirs: string[] = []; + + afterEach(async () => { + vi.clearAllMocks(); + await Promise.all(rootDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true }))); + }); + + it("treats an already-merged base as resolved without making an empty commit", async () => { + const rootDir = await createRootDir(); + rootDirs.push(rootDir); + const store = createStore(createTask()); + mockRunGitCommand + .mockResolvedValueOnce("") // worktree add + .mockResolvedValueOnce("") // checkout task branch + .mockResolvedValueOnce("Already up to date.\n") // merge --no-commit --no-ff base + .mockResolvedValueOnce("") // add -A + .mockResolvedValueOnce("") // diff --cached --quiet => empty index + .mockResolvedValueOnce(""); // worktree remove + + const result = await resolvePrConflicts({ + taskId: "FN-001", + baseRef: "main", + rootDir, + store, + settings, + }); + + expect(result).toMatchObject({ + resolved: true, + pushed: false, + conflictedFiles: [], + }); + expect(result.message).toContain("already merged"); + expect(mockRunGitCommand).not.toHaveBeenCalledWith(expect.arrayContaining(["commit"]), expect.anything(), expect.anything()); + expect(mockRunGitCommand).not.toHaveBeenCalledWith(expect.arrayContaining(["push"]), expect.anything(), expect.anything()); + expect(store.logEntry).toHaveBeenCalledWith("FN-001", "Skipped PR conflict-free merge commit", "main already merged into fusion/fn-001"); + }); + + it("commits and pushes a conflict-free merge when staged changes exist", async () => { + const rootDir = await createRootDir(); + rootDirs.push(rootDir); + const store = createStore(createTask()); + mockRunGitCommand + .mockResolvedValueOnce("") // worktree add + .mockResolvedValueOnce("") // checkout task branch + .mockResolvedValueOnce("") // merge --no-commit --no-ff base + .mockResolvedValueOnce("") // add -A + .mockRejectedValueOnce(Object.assign(new Error("diff has changes"), { code: 1 })) // diff --cached --quiet => staged changes + .mockResolvedValueOnce("") // commit + .mockResolvedValueOnce("") // push + .mockResolvedValueOnce(""); // worktree remove + + const result = await resolvePrConflicts({ + taskId: "FN-001", + baseRef: "main", + rootDir, + store, + settings, + }); + + expect(result).toMatchObject({ + resolved: true, + pushed: true, + conflictedFiles: [], + }); + expect(mockRunGitCommand).toHaveBeenCalledWith([ + "commit", + "-m", + "fix(FN-5949): merge main into FN-001", + "-m", + "Fusion-Task-Id: FN-001", + ], expect.stringContaining("conflict-fn-001"), 60000); + expect(mockRunGitCommand).toHaveBeenCalledWith(["push", "-u", "origin", "fusion/fn-001"], expect.stringContaining("conflict-fn-001"), 60000); + expect(store.logEntry).toHaveBeenCalledWith("FN-001", "Pushed PR branch after conflict-free merge", "fusion/fn-001"); + }); +}); diff --git a/packages/dashboard/src/__tests__/register-git-github.pr-options-preflight-metadata.test.ts b/packages/dashboard/src/__tests__/register-git-github.pr-options-preflight-metadata.test.ts index 6de406ed01..c41fe2e77e 100644 --- a/packages/dashboard/src/__tests__/register-git-github.pr-options-preflight-metadata.test.ts +++ b/packages/dashboard/src/__tests__/register-git-github.pr-options-preflight-metadata.test.ts @@ -169,7 +169,7 @@ describe("PR metadata/preflight/options routes", () => { queueTryRunSuccess("deadbeef\n"); queueTryRunSuccess("refs/heads/fusion/fn-001\n"); queueRunSuccess("2\n"); - queueRunSuccess(""); + queueTryRunSuccess("tree-oid\n"); queueRunSuccess("abc123\tAdd feature\tDev\ndef456\tFix tests\tDev\n"); queueRunSuccess("5\t1\tsrc/a.ts\n1\t1\told.ts => new.ts\n"); queueRunSuccess("M\tsrc/a.ts\nR100\told.ts\tnew.ts\n"); @@ -201,7 +201,7 @@ describe("PR metadata/preflight/options routes", () => { queueTryRunSuccess("deadbeef\n"); queueTryRunFailure(2, "missing remote branch"); queueRunSuccess("0\n"); - queueRunSuccess("conflicted-file.ts\n"); + queueTryRunFailure(1, "conflicted-file.ts\n"); queueRunSuccess(""); queueRunSuccess("not-a-numstat-line\n"); queueRunSuccess("M\n\n"); @@ -220,6 +220,24 @@ describe("PR metadata/preflight/options routes", () => { }); }); + it("GET /pr/preflight treats non-conflict merge-tree errors as no conflict", async () => { + queueTryRunSuccess("deadbeef\n"); + queueTryRunSuccess("refs/heads/fusion/fn-001\n"); + queueRunSuccess("2\n"); + queueTryRunFailure(128, "fatal: bad revision"); + queueRunSuccess("abc123\tAdd feature\tDev\n"); + queueRunSuccess("1\t0\tsrc/a.ts\n"); + queueRunSuccess("M\tsrc/a.ts\n"); + + const app = createServer(createStore(createTask())); + const response = await performGet(app, "/api/tasks/FN-001/pr/preflight"); + + expect(response.status).toBe(200); + expect(response.body.conflictsWithBase).toBe(false); + expect(tryRunQueue).toHaveLength(0); + expect(runQueue).toHaveLength(0); + }); + it("GET /pr/preflight returns 404 for missing task", async () => { const missing = Object.assign(new Error("missing"), { code: "ENOENT" }); const app = createServer(createStore(missing)); diff --git a/packages/dashboard/src/__tests__/register-git-github.pr-push-branch.test.ts b/packages/dashboard/src/__tests__/register-git-github.pr-push-branch.test.ts index ee65179a11..5ca4898016 100644 --- a/packages/dashboard/src/__tests__/register-git-github.pr-push-branch.test.ts +++ b/packages/dashboard/src/__tests__/register-git-github.pr-push-branch.test.ts @@ -128,7 +128,7 @@ describe("POST /pr/push-branch", () => { queueTryRunSuccess("main"); // computePrPreflight -> resolvePrBaseRef local base check queueTryRunSuccess("fusion/fn-001\n"); // computePrPreflight -> ls-remote (branchOnRemote) queueRunSuccess("2\n"); // computePrPreflight -> rev-list --count (commitsPresent) - queueRunSuccess(""); // computePrPreflight -> merge-tree (no conflicts) + queueTryRunSuccess("tree-oid\n"); // computePrPreflight -> merge-tree (clean exit 0) queueRunSuccess("abc123\tAdd feature\tDev\n"); // computePrPreflight -> git log queueRunSuccess("3\t1\tsrc/a.ts\n"); // computePrPreflight -> git diff --numstat queueRunSuccess("M\tsrc/a.ts\n"); // computePrPreflight -> git diff --name-status diff --git a/packages/dashboard/src/__tests__/register-git-github.pr-resolve-conflicts.test.ts b/packages/dashboard/src/__tests__/register-git-github.pr-resolve-conflicts.test.ts index c4ca3028cb..92c7cc8347 100644 --- a/packages/dashboard/src/__tests__/register-git-github.pr-resolve-conflicts.test.ts +++ b/packages/dashboard/src/__tests__/register-git-github.pr-resolve-conflicts.test.ts @@ -130,7 +130,7 @@ describe("POST /pr/resolve-conflicts", () => { queueTryRunSuccess("main"); // computePrPreflight base check queueTryRunSuccess("refs/heads/fusion/fn-001\n"); // remote branch exists queueRunSuccess("2\n"); // git rev-list --count - queueRunSuccess(""); // git merge-tree --write-tree --name-only + queueTryRunSuccess("tree-oid\n"); // git merge-tree --write-tree --name-only (clean exit 0) queueRunSuccess("abc123\tResolve conflicts\tDev\n"); // git log queueRunSuccess("3\t1\tsrc/a.ts\n"); // git diff --numstat queueRunSuccess("M\tsrc/a.ts\n"); // git diff --name-status diff --git a/packages/dashboard/src/pr-conflict-resolver.ts b/packages/dashboard/src/pr-conflict-resolver.ts index 230b80d0b2..8cf248c858 100644 --- a/packages/dashboard/src/pr-conflict-resolver.ts +++ b/packages/dashboard/src/pr-conflict-resolver.ts @@ -97,6 +97,32 @@ async function findFilesWithConflictMarkers(rootDir: string, files: string[]): P return conflicted; } +function getGitExitCode(error: unknown): number | undefined { + const code = (error as { code?: unknown } | undefined)?.code; + return typeof code === "number" ? code : undefined; +} + +async function hasStagedChanges(cwd: string): Promise { + try { + await runGitCommand(["diff", "--cached", "--quiet"], cwd, GIT_TIMEOUT_MS); + return false; + } catch (error) { + if (getGitExitCode(error) === 1) { + return true; + } + throw error; + } +} + +async function stageAndCommitIfNeeded(cwd: string, commitArgs: string[]): Promise { + await runGitCommand(["add", "-A"], cwd, GIT_TIMEOUT_MS); + if (!await hasStagedChanges(cwd)) { + return false; + } + await runGitCommand(["commit", ...commitArgs], cwd, GIT_TIMEOUT_MS); + return true; +} + async function abortMerge(cwd: string): Promise { try { await runGitCommand(["merge", "--abort"], cwd, GIT_TIMEOUT_MS); @@ -206,14 +232,22 @@ export async function resolvePrConflicts(input: ResolvePrConflictsInput): Promis } await store.logEntry(taskId, "AI PR conflict resolution completed", `${conflictedFiles.length} conflicted file(s) resolved`); - await runGitCommand(["add", "-A"], cwd, GIT_TIMEOUT_MS); - await runGitCommand([ - "commit", + const committed = await stageAndCommitIfNeeded(cwd, [ "-m", `fix(FN-5949): resolve PR conflicts for ${taskId}`, "-m", `Fusion-Task-Id: ${taskId}`, - ], cwd, GIT_TIMEOUT_MS); + ]); + if (!committed) { + await abortMerge(cwd); + await store.logEntry(taskId, "Skipped PR conflict resolution commit", "No staged changes after AI conflict resolution"); + return { + resolved: true, + pushed: false, + conflictedFiles, + message: `Resolved conflicts with ${baseRef}, but no merge commit was needed because there were no staged changes.`, + }; + } await runGitCommand(["push", "-u", "origin", branchName], cwd, GIT_TIMEOUT_MS); await store.logEntry(taskId, "Pushed PR branch after AI conflict resolution", branchName); @@ -229,14 +263,21 @@ export async function resolvePrConflicts(input: ResolvePrConflictsInput): Promis } } - await runGitCommand(["add", "-A"], cwd, GIT_TIMEOUT_MS); - await runGitCommand([ - "commit", + const committed = await stageAndCommitIfNeeded(cwd, [ "-m", `fix(FN-5949): merge ${baseRef} into ${taskId}`, "-m", `Fusion-Task-Id: ${taskId}`, - ], cwd, GIT_TIMEOUT_MS); + ]); + if (!committed) { + await store.logEntry(taskId, "Skipped PR conflict-free merge commit", `${baseRef} already merged into ${branchName}`); + return { + resolved: true, + pushed: false, + conflictedFiles: [], + message: `${baseRef} already merged into ${branchName}; no merge commit needed.`, + }; + } await runGitCommand(["push", "-u", "origin", branchName], cwd, GIT_TIMEOUT_MS); await store.logEntry(taskId, "Pushed PR branch after conflict-free merge", branchName); diff --git a/packages/dashboard/src/routes/register-git-github.ts b/packages/dashboard/src/routes/register-git-github.ts index a42f74fec8..123f1b1d0e 100644 --- a/packages/dashboard/src/routes/register-git-github.ts +++ b/packages/dashboard/src/routes/register-git-github.ts @@ -388,12 +388,14 @@ async function computePrPreflight(task: Task, repoRoot: string, requestedBase?: ).catch(() => "0"); response.commitsPresent = Number.parseInt(commitCountOutput, 10) > 0; - const mergeTreeOutput = await prRouteCommandRunner.run( + const mergeTreeResult = await prRouteCommandRunner.tryRun( `git merge-tree --write-tree --name-only ${shellQuote(baseRef)} ${shellQuote(safeHead)}`, repoRoot, PR_PREFLIGHT_TIMEOUT_MS, - ).catch(() => ""); - response.conflictsWithBase = mergeTreeOutput.trim().length > 0; + ); + // `git merge-tree --write-tree` exits 0 for clean merges and 1 for real conflicts; + // stdout can be non-empty in both cases, so conflict state must come from the exit code. + response.conflictsWithBase = !mergeTreeResult.ok && mergeTreeResult.code === 1; const [commitLogOutput, numstatOutput, nameStatusOutput] = await Promise.all([ prRouteCommandRunner.run(