diff --git a/packages/cli/src/commands/__tests__/task-lifecycle.test.ts b/packages/cli/src/commands/__tests__/task-lifecycle.test.ts index a3b1fa233..49cdd75e8 100644 --- a/packages/cli/src/commands/__tests__/task-lifecycle.test.ts +++ b/packages/cli/src/commands/__tests__/task-lifecycle.test.ts @@ -668,6 +668,73 @@ describe("processPullRequestMergeTask", () => { expect(github.getPrMergeStatus).toHaveBeenCalledWith("owner", "repo", 7); }); + it("counts a conflicting stale-base PR against mergeRetries so the stall escape fires", async () => { + const task: MockTask = { + id: "FN-9020", + title: "test", + description: "desc", + column: "in-review", + mergeRetries: 1, + }; + const branch = getTaskBranchName(task.id); + const store = makeStore(task); + execMock.mockImplementation(() => ""); + + const existingPr = { number: 9, url: "https://github.com/x/y/pull/9", status: "open" as const, headBranch: branch, baseBranch: "main" }; + const github = { + findPrForBranch: vi.fn(async () => existingPr), + createPr: vi.fn(), + getPrMergeStatus: vi.fn(async () => ({ + prInfo: { ...existingPr, mergeable: "conflicting" as const }, + 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(store.updateTask).toHaveBeenCalledWith(task.id, { + status: "awaiting-pr-checks", + mergeRetries: 2, + }); + }); + + it("does not bump mergeRetries for a non-conflicting not-ready PR", async () => { + const task: MockTask = { + id: "FN-9021", + title: "test", + description: "desc", + column: "in-review", + mergeRetries: 1, + }; + const branch = getTaskBranchName(task.id); + const store = makeStore(task); + execMock.mockImplementation(() => ""); + + const existingPr = { number: 10, url: "https://github.com/x/y/pull/10", status: "open" as const, headBranch: branch, baseBranch: "main" }; + const github = { + findPrForBranch: vi.fn(async () => existingPr), + createPr: vi.fn(), + getPrMergeStatus: vi.fn(async () => ({ + prInfo: { ...existingPr, mergeable: "unknown" as const }, + 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(store.updateTask).toHaveBeenCalledWith(task.id, { status: "awaiting-pr-checks" }); + }); + it("skips the push when an existing PR already covers the branch", async () => { const task: MockTask = { id: "FN-9002", @@ -883,7 +950,7 @@ describe("processPullRequestMergeTask", () => { expect(github.createPr).toHaveBeenCalledWith(expect.objectContaining({ head: branch })); }); - it("parks no-delta branches instead of retrying into branch push failures", async () => { + it("finalizes no-delta branches as a no-op done instead of failing", async () => { const task: MockTask = { id: "FN-9012", title: "test", @@ -913,13 +980,13 @@ describe("processPullRequestMergeTask", () => { expect(result).toBe("skipped"); expect(store.updateTask).toHaveBeenCalledWith(task.id, { - status: "failed", - error: `No pull request created for ${branch}: the branch has no commits relative to the base branch.`, + status: null, + mergeRetries: 0, }); - expect(store.logEntry).toHaveBeenCalledWith( + expect(store.moveTask).toHaveBeenCalledWith(task.id, "done"); + expect(store.updateTask).not.toHaveBeenCalledWith( task.id, - `No pull request created for ${branch}: the branch has no commits relative to the base branch.`, - expect.stringContaining("No commits between"), + expect.objectContaining({ status: "failed" }), ); }); diff --git a/packages/cli/src/commands/task-lifecycle.ts b/packages/cli/src/commands/task-lifecycle.ts index 1f90e5907..fb6b869b7 100644 --- a/packages/cli/src/commands/task-lifecycle.ts +++ b/packages/cli/src/commands/task-lifecycle.ts @@ -640,6 +640,40 @@ async function finalizePullRequestMerge( } as MergeResult); } +/** + * Finalize a task whose branch has no commits relative to the base branch + * ("No commits between ...") as a terminal no-op DONE, mirroring the engine's + * canonical zero-commits decision (merger-ai.ts `noOpResult` → done, PR #1920). + * Marking it `failed` here left an empty-diff follow-up task pinning the merge + * slot + file leases indefinitely; a no-op branch is not a failure — there was + * simply nothing to merge. + */ +async function finalizeNoOpMergeTask( + store: TaskStore, + cwd: string, + task: TaskDetail, + reason: string, + pool?: WorktreePool, +): Promise { + const branch = task.branch ?? getTaskBranchName(task.id); + await cleanupMergedTaskArtifacts(cwd, task, { pool }); + await store.updateTask(task.id, { status: null, mergeRetries: 0 }); + const movedTask = await store.moveTask(task.id, "done"); + const mergedTask = movedTask ?? (await store.getTask(task.id)); + await store.logEntry(task.id, reason, `Branch ${branch} has no commits relative to the base branch; nothing to merge.`); + store.emit("task:merged", { + task: mergedTask, + branch: mergedTask.branch ?? branch, + merged: false, + noOp: true, + ok: true, + reason, + mergeConfirmed: true, + worktreeRemoved: false, + branchDeleted: false, + } as MergeResult); +} + /** * Result of processing a PR merge task. * - "waiting": PR exists but not ready to merge (checks pending, reviews needed) @@ -774,8 +808,7 @@ export async function processPullRequestMergeTask( const message = err instanceof Error ? err.message : String(err); if (message.includes("No commits between")) { await store.updateBranchGroup(branchGroup.id, { prState: "none", prNumber: null, prUrl: null }); - await store.updateTask(task.id, { status: "failed", error: `No pull request created for ${branchGroup.branchName}: no commits relative to ${projectDefaultBranch}.` }); - await store.logEntry(task.id, "No group pull request created", message); + await finalizeNoOpMergeTask(store, cwd, task, "No group pull request created (no commits vs base) — finalizing as no-op", pool); return "skipped"; } throw err; @@ -878,9 +911,7 @@ export async function processPullRequestMergeTask( } catch (err: unknown) { const message = err instanceof Error ? err.message : String(err); if (message.includes("No commits between")) { - const error = `No pull request created for ${branch}: the branch has no commits relative to the base branch.`; - await store.updateTask(task.id, { status: "failed", error }); - await store.logEntry(task.id, error, message); + await finalizeNoOpMergeTask(store, cwd, task, "No pull request created (no commits vs base) — finalizing as no-op", pool); return "skipped"; } throw err; @@ -924,7 +955,23 @@ export async function processPullRequestMergeTask( if (!mergeStatus.mergeReady) { if (mergeStatus.prInfo.status === "open") { - await store.updateTask(task.id, { status: "awaiting-pr-checks" }); + // A stale-base PR that GitHub reports as CONFLICTING never becomes + // mergeable on its own — nothing in the PR path rebases the head branch — + // so an unbounded `awaiting-pr-checks` wait pins the merge slot + file + // leases forever (FN-485). Count each conflicting poll against + // `mergeRetries` so the existing `getInReviewStallReason` + // "merge-retries-exhausted" escape disposes the task after + // `maxAutoMergeRetries` cycles. Pending/behind PRs still wait indefinitely + // (checks legitimately run; "behind" is resolved by the pre-merge rebase). + // ponytail: bounded escape via the existing stall counter, not an in-path + // rebase. Upgrade path: rebase the head branch onto base + force-push here + // before giving up, then reset mergeRetries. + await store.updateTask(task.id, { + status: "awaiting-pr-checks", + ...(mergeStatus.prInfo.mergeable === "conflicting" + ? { mergeRetries: (task.mergeRetries ?? 0) + 1 } + : {}), + }); } else { await store.updateTask(task.id, { status: null }); }