fix(cli): harden PR merge path against stale-base dead-ends (empty-diff no-op + conflict escape)#1943
Draft
flexi767 wants to merge 1 commit into
Draft
Conversation
Two failure modes wedged the PR-based merge flow (dormant under the
current direct-merge config, but the rollback target):
1. Empty-diff branches ("No commits between ...") were marked `failed`,
pinning the serial merge slot + file leases on a task that is a
legitimate no-op. Now finalized as a terminal DONE via
`finalizeNoOpMergeTask`, mirroring the engine's canonical
`noOpResult` decision (merger-ai.ts).
2. A stale-base PR that GitHub reports CONFLICTING never becomes
mergeable on its own (nothing in the PR path rebases the head), so
`awaiting-pr-checks` waited unbounded — no escape, because the PR
path never incremented `mergeRetries`. Now each conflicting poll
counts against `mergeRetries`, so the existing
`getInReviewStallReason` "merge-retries-exhausted" escape disposes
the task after `maxAutoMergeRetries` cycles. Pending/behind PRs
still wait (checks legitimately run; "behind" is fixed by rebase).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Two stale-base failure modes wedge the PR-based merge flow (
processPullRequestMergeTaskinpackages/cli/src/commands/task-lifecycle.ts). Both pin the serial merge slot + file-scope leases on a dead-end task, starving ready work behind it.Empty-diff branch —
createPrthrowsNo commits between <base> and <head>. The task was markedfailed, even though a zero-commit branch is a legitimate no-op, not a failure. This is the PR-path analog of the engine-path fix in fix(engine): short-circuit zero-commits-ahead branch before AI-merge clean-room churn #1920 (noOpResult→ done).Conflicting stale-base PR — a PR GitHub reports as
CONFLICTINGnever becomes mergeable on its own (nothing in the PR path rebases the head branch). The task sat inawaiting-pr-checksunbounded:awaiting-pr-checksis not inTRANSIENT_MERGE_STATUSES, so the age-based stall escape never catches it, and the only other escape —getInReviewStallReason'smerge-retries-exhausted— never fired because the PR path never incrementedmergeRetries.Fix
finalizeNoOpMergeTaskfinalizes empty-diff branches as a terminal DONE, mirroring the engine's canonicalnoOpResultdecision (merged:false, noOp:true). Both the group and per-task empty-diff sites route through it.Each
CONFLICTINGpoll now counts againstmergeRetries, so the existingmerge-retries-exhaustedescape disposes the task aftermaxAutoMergeRetriescycles. Detection is gated onprInfo.mergeable === "conflicting"(the genuine unresolvable signal), notblockingReasons(status/review/checks only). Pending /behindPRs still wait indefinitely — checks legitimately run, andbehindis resolved by the pre-merge rebase.No new machinery — fix #2 reuses the existing stall-escape counter rather than a hand-rolled fail branch.
Tests
packages/cli/src/commands/__tests__/task-lifecycle.test.ts:moveTask(id,"done"),updateTask({status:null,mergeRetries:0}), neverfailed)mergeRetriesbumpedmergeRetriesnot bumpedpnpm --filter @runfusion/fusion test(task-lifecycle suite 39/39),typecheck, andbuildall green.🤖 Generated with Claude Code