fix(cli): push per-task branch to origin before creating PR; drop dead autoCreatePr
In PR-mode, processPullRequestMergeTask called gh pr create --head fusion/<task-id> without ever pushing the branch to origin. PR creation failed and the task stalled in in-review — and combined with the recover-mergeable-review sweep bug, the stalled task got force-merged locally instead. Push the branch via git push -u origin <branch> immediately before createPr (skipped when an existing PR already covers the branch). Also remove the dead autoCreatePr setting: defined as a default in the schema and Settings type but never read anywhere. Related to https://github.com/Runfusion/Fusion/issues/21 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
16
.changeset/pr-mode-push-branch-and-cleanup.md
Normal file
16
.changeset/pr-mode-push-branch-and-cleanup.md
Normal file
@@ -0,0 +1,16 @@
|
|||||||
|
---
|
||||||
|
"@runfusion/fusion": patch
|
||||||
|
"runfusion.ai": patch
|
||||||
|
"@fusion/core": patch
|
||||||
|
"@fusion/dashboard": patch
|
||||||
|
"@fusion/desktop": patch
|
||||||
|
"@fusion/engine": patch
|
||||||
|
"@fusion/mobile": patch
|
||||||
|
"@fusion/pi-claude-cli": patch
|
||||||
|
"@fusion/plugin-sdk": patch
|
||||||
|
---
|
||||||
|
|
||||||
|
Fix PR-mode merge flow (related to [#21](https://github.com/Runfusion/Fusion/issues/21)):
|
||||||
|
|
||||||
|
- **PR-mode now pushes the per-task branch to origin before creating the PR.** `processPullRequestMergeTask` previously called `gh pr create --head fusion/<task-id>` without ever publishing the branch, so the PR creation failed and the task stalled in `in-review`. The branch is now pushed via `git push -u origin <branch>` immediately before `createPr` (skipped when an existing PR already covers the branch).
|
||||||
|
- **Removed dead `autoCreatePr` setting** from the schema and `Settings` type. It was defined as a default but never read anywhere.
|
||||||
190
packages/cli/src/commands/__tests__/task-lifecycle.test.ts
Normal file
190
packages/cli/src/commands/__tests__/task-lifecycle.test.ts
Normal file
@@ -0,0 +1,190 @@
|
|||||||
|
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||||
|
import { EventEmitter } from "node:events";
|
||||||
|
|
||||||
|
// Mock child_process so we can intercept the `git push -u origin <branch>`
|
||||||
|
// call that processPullRequestMergeTask issues before createPr.
|
||||||
|
const execMock = vi.hoisted(() => vi.fn());
|
||||||
|
vi.mock("node:child_process", () => ({
|
||||||
|
exec: (cmd: string, opts: unknown, cb: (err: Error | null, stdout: string, stderr: string) => void) => {
|
||||||
|
try {
|
||||||
|
const result = execMock(cmd, opts);
|
||||||
|
cb(null, typeof result === "string" ? result : "", "");
|
||||||
|
} catch (err) {
|
||||||
|
cb(err as Error, "", (err as Error).message);
|
||||||
|
}
|
||||||
|
},
|
||||||
|
}));
|
||||||
|
|
||||||
|
import { processPullRequestMergeTask, getTaskBranchName } from "../task-lifecycle.js";
|
||||||
|
|
||||||
|
interface MockTask {
|
||||||
|
id: string;
|
||||||
|
title: string;
|
||||||
|
description: string;
|
||||||
|
worktree?: string;
|
||||||
|
prInfo?: unknown;
|
||||||
|
column: string;
|
||||||
|
}
|
||||||
|
|
||||||
|
function makeStore(task: MockTask) {
|
||||||
|
const emitter = new EventEmitter();
|
||||||
|
const updates: Array<{ id: string; patch: Record<string, unknown> }> = [];
|
||||||
|
return Object.assign(emitter, {
|
||||||
|
getTask: vi.fn().mockResolvedValue(task),
|
||||||
|
updateTask: vi.fn(async (id: string, patch: Record<string, unknown>) => {
|
||||||
|
updates.push({ id, patch });
|
||||||
|
}),
|
||||||
|
updatePrInfo: vi.fn().mockResolvedValue(undefined),
|
||||||
|
moveTask: vi.fn().mockResolvedValue(undefined),
|
||||||
|
logEntry: vi.fn().mockResolvedValue(undefined),
|
||||||
|
getActiveMergingTask: vi.fn().mockReturnValue(null),
|
||||||
|
_updates: updates,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("processPullRequestMergeTask", () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
execMock.mockReset();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("pushes the per-task branch to origin before creating a new PR", async () => {
|
||||||
|
const task: MockTask = {
|
||||||
|
id: "FN-9001",
|
||||||
|
title: "test",
|
||||||
|
description: "desc",
|
||||||
|
column: "in-review",
|
||||||
|
};
|
||||||
|
const branch = getTaskBranchName(task.id); // "fusion/fn-9001"
|
||||||
|
const store = makeStore(task);
|
||||||
|
|
||||||
|
const callOrder: string[] = [];
|
||||||
|
execMock.mockImplementation((cmd: string) => {
|
||||||
|
callOrder.push(`exec:${cmd}`);
|
||||||
|
return "";
|
||||||
|
});
|
||||||
|
|
||||||
|
const github = {
|
||||||
|
findPrForBranch: vi.fn(async () => {
|
||||||
|
callOrder.push("findPrForBranch");
|
||||||
|
return null;
|
||||||
|
}),
|
||||||
|
createPr: vi.fn(async () => {
|
||||||
|
callOrder.push("createPr");
|
||||||
|
return {
|
||||||
|
number: 42,
|
||||||
|
url: "https://github.com/x/y/pull/42",
|
||||||
|
status: "open" as const,
|
||||||
|
headBranch: branch,
|
||||||
|
baseBranch: "main",
|
||||||
|
};
|
||||||
|
}),
|
||||||
|
getPrMergeStatus: vi.fn(async () => ({
|
||||||
|
prInfo: { number: 42, status: "open" as const, url: "https://github.com/x/y/pull/42" },
|
||||||
|
reviewDecision: null,
|
||||||
|
checks: [],
|
||||||
|
mergeReady: false,
|
||||||
|
blockingReasons: [],
|
||||||
|
})),
|
||||||
|
mergePr: vi.fn(),
|
||||||
|
};
|
||||||
|
|
||||||
|
const result = await processPullRequestMergeTask(
|
||||||
|
store as never,
|
||||||
|
"/repo",
|
||||||
|
task.id,
|
||||||
|
github as never,
|
||||||
|
() => undefined,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(result).toBe("waiting");
|
||||||
|
expect(github.findPrForBranch).toHaveBeenCalled();
|
||||||
|
|
||||||
|
// The git push must happen after findPrForBranch and before createPr.
|
||||||
|
const pushIdx = callOrder.findIndex((c) => c === `exec:git push -u origin "${branch}"`);
|
||||||
|
const findIdx = callOrder.indexOf("findPrForBranch");
|
||||||
|
const createIdx = callOrder.indexOf("createPr");
|
||||||
|
expect(pushIdx).toBeGreaterThan(-1);
|
||||||
|
expect(pushIdx).toBeGreaterThan(findIdx);
|
||||||
|
expect(pushIdx).toBeLessThan(createIdx);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("skips the push when an existing PR already covers the branch", async () => {
|
||||||
|
const task: MockTask = {
|
||||||
|
id: "FN-9002",
|
||||||
|
title: "test",
|
||||||
|
description: "desc",
|
||||||
|
column: "in-review",
|
||||||
|
};
|
||||||
|
const branch = getTaskBranchName(task.id);
|
||||||
|
const store = makeStore(task);
|
||||||
|
|
||||||
|
const pushed: string[] = [];
|
||||||
|
execMock.mockImplementation((cmd: string) => {
|
||||||
|
if (cmd.startsWith("git push")) pushed.push(cmd);
|
||||||
|
return "";
|
||||||
|
});
|
||||||
|
|
||||||
|
const existingPr = {
|
||||||
|
number: 7,
|
||||||
|
url: "https://github.com/x/y/pull/7",
|
||||||
|
status: "open" as const,
|
||||||
|
headBranch: branch,
|
||||||
|
baseBranch: "main",
|
||||||
|
};
|
||||||
|
|
||||||
|
const github = {
|
||||||
|
findPrForBranch: vi.fn(async () => existingPr),
|
||||||
|
createPr: vi.fn(),
|
||||||
|
getPrMergeStatus: vi.fn(async () => ({
|
||||||
|
prInfo: existingPr,
|
||||||
|
reviewDecision: null,
|
||||||
|
checks: [],
|
||||||
|
mergeReady: false,
|
||||||
|
blockingReasons: [],
|
||||||
|
})),
|
||||||
|
mergePr: vi.fn(),
|
||||||
|
};
|
||||||
|
|
||||||
|
await processPullRequestMergeTask(
|
||||||
|
store as never,
|
||||||
|
"/repo",
|
||||||
|
task.id,
|
||||||
|
github as never,
|
||||||
|
() => undefined,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(github.createPr).not.toHaveBeenCalled();
|
||||||
|
expect(pushed).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("surfaces a clear error when the pre-create push fails", async () => {
|
||||||
|
const task: MockTask = {
|
||||||
|
id: "FN-9003",
|
||||||
|
title: "test",
|
||||||
|
description: "desc",
|
||||||
|
column: "in-review",
|
||||||
|
};
|
||||||
|
const branch = getTaskBranchName(task.id);
|
||||||
|
const store = makeStore(task);
|
||||||
|
|
||||||
|
execMock.mockImplementation((cmd: string) => {
|
||||||
|
if (cmd.startsWith("git push")) {
|
||||||
|
throw new Error("remote rejected: permission denied");
|
||||||
|
}
|
||||||
|
return "";
|
||||||
|
});
|
||||||
|
|
||||||
|
const github = {
|
||||||
|
findPrForBranch: vi.fn(async () => null),
|
||||||
|
createPr: vi.fn(),
|
||||||
|
getPrMergeStatus: vi.fn(),
|
||||||
|
mergePr: vi.fn(),
|
||||||
|
};
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
processPullRequestMergeTask(store as never, "/repo", task.id, github as never, () => undefined),
|
||||||
|
).rejects.toThrow(new RegExp(`Failed to push branch "${branch}" to origin`));
|
||||||
|
|
||||||
|
expect(github.createPr).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -52,6 +52,26 @@ export function getTaskBranchName(taskId: string): string {
|
|||||||
return `fusion/${taskId.toLowerCase()}`;
|
return `fusion/${taskId.toLowerCase()}`;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Push the per-task branch to origin so `gh pr create --head <branch>`
|
||||||
|
* can find it. Idempotent: creates the remote branch on first push and
|
||||||
|
* fast-forwards thereafter. Required because the GitHub PR-create flow
|
||||||
|
* does not implicitly publish the local branch.
|
||||||
|
*/
|
||||||
|
async function pushTaskBranchToOrigin(cwd: string, branch: string): Promise<void> {
|
||||||
|
try {
|
||||||
|
await execAsync(`git push -u origin "${branch}"`, {
|
||||||
|
cwd,
|
||||||
|
timeout: 60_000,
|
||||||
|
});
|
||||||
|
} catch (err: unknown) {
|
||||||
|
const message = err instanceof Error ? err.message : String(err);
|
||||||
|
throw new Error(
|
||||||
|
`Failed to push branch "${branch}" to origin before PR creation: ${message}`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Build the PR title for a task.
|
* Build the PR title for a task.
|
||||||
* Format: "{taskId}: {title}" or just "{taskId}" if no title.
|
* Format: "{taskId}: {title}" or just "{taskId}" if no title.
|
||||||
@@ -173,6 +193,12 @@ export async function processPullRequestMergeTask(
|
|||||||
await store.updateTask(task.id, { status: "creating-pr" });
|
await store.updateTask(task.id, { status: "creating-pr" });
|
||||||
|
|
||||||
const existingPr = await github.findPrForBranch({ head: branch, state: "all" });
|
const existingPr = await github.findPrForBranch({ head: branch, state: "all" });
|
||||||
|
if (!existingPr) {
|
||||||
|
// gh pr create / GitHub REST require the head branch to exist on
|
||||||
|
// origin. Nothing else in the merge path publishes the per-task
|
||||||
|
// branch, so we push it here right before creating the PR.
|
||||||
|
await pushTaskBranchToOrigin(cwd, branch);
|
||||||
|
}
|
||||||
prInfo = existingPr ?? await github.createPr({
|
prInfo = existingPr ?? await github.createPr({
|
||||||
title: buildPullRequestTitle(task),
|
title: buildPullRequestTitle(task),
|
||||||
body: buildPullRequestBody(task),
|
body: buildPullRequestBody(task),
|
||||||
|
|||||||
@@ -139,7 +139,6 @@ export const DEFAULT_PROJECT_SETTINGS = {
|
|||||||
autoArchiveDoneAfterMs: 48 * 60 * 60 * 1000,
|
autoArchiveDoneAfterMs: 48 * 60 * 60 * 1000,
|
||||||
archiveAgentLogMode: "compact",
|
archiveAgentLogMode: "compact",
|
||||||
autoUpdatePrStatus: false,
|
autoUpdatePrStatus: false,
|
||||||
autoCreatePr: false,
|
|
||||||
githubCommentOnDone: false,
|
githubCommentOnDone: false,
|
||||||
githubCommentTemplate: undefined,
|
githubCommentTemplate: undefined,
|
||||||
autoBackupEnabled: false,
|
autoBackupEnabled: false,
|
||||||
|
|||||||
@@ -1694,9 +1694,6 @@ export interface ProjectSettings {
|
|||||||
/** When true, automatically poll and update PR status badges for tasks linked to GitHub PRs.
|
/** When true, automatically poll and update PR status badges for tasks linked to GitHub PRs.
|
||||||
* Default: false. */
|
* Default: false. */
|
||||||
autoUpdatePrStatus?: boolean;
|
autoUpdatePrStatus?: boolean;
|
||||||
/** When true, automatically create GitHub PRs for completed tasks.
|
|
||||||
* Default: false. */
|
|
||||||
autoCreatePr?: boolean;
|
|
||||||
/** When true, automatically post a comment to the originating GitHub issue
|
/** When true, automatically post a comment to the originating GitHub issue
|
||||||
* when an imported task is moved to done. Default: false. */
|
* when an imported task is moved to done. Default: false. */
|
||||||
githubCommentOnDone?: boolean;
|
githubCommentOnDone?: boolean;
|
||||||
|
|||||||
Reference in New Issue
Block a user