fix(dev-lead): force-with-lease retry when a rebase rewrites branch history (#647) - #649
Conversation
…istory push_with_merge_guard did a plain `git push`, so when an engine resolved a rebase under a non-rebase intent (on-mention, review-changes) the rewritten branch could never be published — the push was rejected non-fast-forward and the run failed red, even though the rebase + conflict resolution succeeded (e.g. PR #624 stuck dirty for a day). Detect that case — a non-fast-forward rejection while HEAD has diverged from its upstream (history rewritten) — and retry once with --force-with-lease. The lease (our remote-tracking ref) makes the retry abort if the remote advanced under us, so a concurrent push is never clobbered. Normal non-fast-forwards (remote legitimately advanced, branch not diverged) still fail as before. This mirrors the dedicated rebase intent's own force-push (prompts/dev-lead/rebase.md). Closes #647 https://claude.ai/code/session_01BTcE5pAvrmNqkatFUTAetF
|
Warning Review limit reached
More reviews will be available in 39 minutes and 9 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ 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 |
|
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
There was a problem hiding this comment.
Code Review
This pull request introduces a retry mechanism using --force-with-lease in scripts/lib/auto-merge.sh when a push is rejected due to a non-fast-forward error and the local branch has diverged from its upstream (indicating a history rewrite). It also adds corresponding unit tests in tests/dev-lead/unit/test_auto_merge.bats. The review feedback suggests improving readability in the test stub helper by extracting the outer function's positional parameter into a named local variable to avoid confusion with the inner script's escaped positional parameters inside the heredoc.
| _install_git_stub_force() { | ||
| cat > "$STUB_BIN_DIR/git" <<EOF | ||
| #!/usr/bin/env bash | ||
| case "\$1" in | ||
| push) | ||
| shift | ||
| for a in "\$@"; do | ||
| [ "\$a" = "--force-with-lease" ] && exit 0 | ||
| done | ||
| echo " ! [rejected] feature -> feature (non-fast-forward)" >&2 | ||
| exit 1 | ||
| ;; | ||
| rev-parse) | ||
| case "\$*" in | ||
| *"@{u}"*) echo "origin/feature" ;; | ||
| *) echo "deadbeefcafe" ;; | ||
| esac | ||
| exit 0 | ||
| ;; | ||
| merge-base) | ||
| # --is-ancestor <upstream> HEAD: 0 = upstream reachable (not diverged), | ||
| # 1 = diverged (history rewritten by a rebase). | ||
| [ "${1:-}" = "diverged" ] && exit 1 || exit 0 | ||
| ;; |
There was a problem hiding this comment.
Mixing escaped and unescaped positional parameters (like \$1 and ${1:-}) inside a double-quoted heredoc (<<EOF) can be confusing and hard to maintain. It is not immediately obvious that ${1:-} refers to the argument of the outer function _install_git_stub_force rather than the generated script's own $1 (which is matched as \$1 on line 197).
Using a clearly named local variable (e.g., local mode="${1:-}") and referencing it as "$mode" makes the distinction between generation-time expansion and runtime evaluation much clearer.
_install_git_stub_force() {
local mode="${1:-}"
cat > "$STUB_BIN_DIR/git" <<EOF
#!/usr/bin/env bash
case "\$1" in
push)
shift
for a in "\$@"; do
[ "\$a" = "--force-with-lease" ] && exit 0
done
echo " ! [rejected] feature -> feature (non-fast-forward)" >&2
exit 1
;;
rev-parse)
case "\$*" in
*"@{u}"*) echo "origin/feature" ;;
*) echo "deadbeefcafe" ;;
esac
exit 0
;;
merge-base)
# --is-ancestor <upstream> HEAD: 0 = upstream reachable (not diverged),
# 1 = diverged (history rewritten by a rebase).
[ "$mode" = "diverged" ] && exit 1 || exit 0
;;
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…istory (#649) push_with_merge_guard did a plain `git push`, so when an engine resolved a rebase under a non-rebase intent (on-mention, review-changes) the rewritten branch could never be published — the push was rejected non-fast-forward and the run failed red, even though the rebase + conflict resolution succeeded (e.g. PR #624 stuck dirty for a day). Detect that case — a non-fast-forward rejection while HEAD has diverged from its upstream (history rewritten) — and retry once with --force-with-lease. The lease (our remote-tracking ref) makes the retry abort if the remote advanced under us, so a concurrent push is never clobbered. Normal non-fast-forwards (remote legitimately advanced, branch not diverged) still fail as before. This mirrors the dedicated rebase intent's own force-push (prompts/dev-lead/rebase.md). Closes #647 https://claude.ai/code/session_01BTcE5pAvrmNqkatFUTAetF Co-authored-by: Claude <noreply@anthropic.com>
…istory (#649) push_with_merge_guard did a plain `git push`, so when an engine resolved a rebase under a non-rebase intent (on-mention, review-changes) the rewritten branch could never be published — the push was rejected non-fast-forward and the run failed red, even though the rebase + conflict resolution succeeded (e.g. PR #624 stuck dirty for a day). Detect that case — a non-fast-forward rejection while HEAD has diverged from its upstream (history rewritten) — and retry once with --force-with-lease. The lease (our remote-tracking ref) makes the retry abort if the remote advanced under us, so a concurrent push is never clobbered. Normal non-fast-forwards (remote legitimately advanced, branch not diverged) still fail as before. This mirrors the dedicated rebase intent's own force-push (prompts/dev-lead/rebase.md). Closes #647 https://claude.ai/code/session_01BTcE5pAvrmNqkatFUTAetF Co-authored-by: Claude <noreply@anthropic.com>
…istory (#649) push_with_merge_guard did a plain `git push`, so when an engine resolved a rebase under a non-rebase intent (on-mention, review-changes) the rewritten branch could never be published — the push was rejected non-fast-forward and the run failed red, even though the rebase + conflict resolution succeeded (e.g. PR #624 stuck dirty for a day). Detect that case — a non-fast-forward rejection while HEAD has diverged from its upstream (history rewritten) — and retry once with --force-with-lease. The lease (our remote-tracking ref) makes the retry abort if the remote advanced under us, so a concurrent push is never clobbered. Normal non-fast-forwards (remote legitimately advanced, branch not diverged) still fail as before. This mirrors the dedicated rebase intent's own force-push (prompts/dev-lead/rebase.md). Closes #647 https://claude.ai/code/session_01BTcE5pAvrmNqkatFUTAetF Co-authored-by: Claude <noreply@anthropic.com>
…istory (#649) push_with_merge_guard did a plain `git push`, so when an engine resolved a rebase under a non-rebase intent (on-mention, review-changes) the rewritten branch could never be published — the push was rejected non-fast-forward and the run failed red, even though the rebase + conflict resolution succeeded (e.g. PR #624 stuck dirty for a day). Detect that case — a non-fast-forward rejection while HEAD has diverged from its upstream (history rewritten) — and retry once with --force-with-lease. The lease (our remote-tracking ref) makes the retry abort if the remote advanced under us, so a concurrent push is never clobbered. Normal non-fast-forwards (remote legitimately advanced, branch not diverged) still fail as before. This mirrors the dedicated rebase intent's own force-push (prompts/dev-lead/rebase.md). Closes #647 https://claude.ai/code/session_01BTcE5pAvrmNqkatFUTAetF Co-authored-by: Claude <noreply@anthropic.com>



Closes #647
Problem
push_with_merge_guard(scripts/lib/auto-merge.sh) ran a plaingit push. When an engine resolved a rebase under a non-rebase intent (on-mention,review-changes), the rewritten branch history could never be published — the push was rejected non-fast-forward and the whole run failed red, even though the rebase and conflict resolution succeeded.Observed in run #27471503333 (
@dev-lead - resolve the rebase main conflicton PR #624), which left PR #624 stuckdirtyfor a day. Only the dedicatedrebaseintent worked, becauseprompts/dev-lead/rebase.mdhas the engine force-push itself.Fix
When the plain push is rejected non-fast-forward and
HEADhas diverged from its upstream (i.e. history was rewritten), retry once with--force-with-lease:push_with_merge_guardwith no args, so the behavior change is scoped to the rejected-push path only.This mirrors the established force-push in the dedicated rebase intent.
Tests
tests/dev-lead/unit/test_auto_merge.bats— added 2 cases (force-with-lease retry succeeds on rewritten history; no force-push when branch isn't diverged). Full suite: 25/25 pass,shellcheck --severity=warning -xclean.Note: the original symptom (PR #624 stuck) was already resolved manually by rebasing + force-pushing it; #624 has since merged and closed #573. This PR fixes the underlying push path so it can't recur.
Generated by Claude Code