Skip to content

fix: retarget a stacked child PR at the plan base before merging it - #160

Closed
vitali87 wants to merge 3 commits into
fix/merge-closed-while-pollingfrom
fix/stacked-merge-retargets-base
Closed

vitali87 wants to merge 3 commits into
fix/merge-closed-while-pollingfrom
fix/stacked-merge-retargets-base

Conversation

@vitali87

Copy link
Copy Markdown
Owner

Summary

With --stack, child PRs are opened against their parent's split branch. merge merged them as-is, relying on GitHub to retarget children once the parent's head branch is deleted — but gh pr merge --auto skips the head-branch deletion, and GitHub only deletes it itself when "Automatically delete head branches" is on (off by default). So split --stack + merge --auto merged every child into its parent's branch and reported success while main never received them (reproduced with a recording fake gh: pr-2 into pr-split/dev/pr-1, pr-3 into …/pr-2, Merged (6), exit 0).

merge_all now derives the stacked children from the plan's branch records (base_branch ≠ plan base) and, right before merge_pr — i.e. once iter_ready and the unmerged-parent guard guarantee the parent merged — calls a new retarget_pr (gh pr edit N --base <plan base>). GitHub refuses that edit for PRs in a native stack ("part of a stack"), which is tolerated with an info log because GitHub retargets those itself. Merge nodes already target the base and are untouched. README's stacking and merge paragraphs now describe this instead of the old (false) "GitHub retargets children automatically".

Stacked on #103 (stack #99 = #67→#98→#103→this), which owns the merge-loop changes.

Test plan

  • TestStackedMergeRetargetsBase: child retargeted at the plan base before merging; unstacked PRs untouched; --auto retargets before queueing — all fail on the base
  • TestRetargetPr: edits the base; native-stack refusal tolerated; other failures raise
  • Review re-ran the fake-gh scenario: every merge now records base=main
  • 460 tests pass, ruff clean
  • Local review: 5/5 (after two wording rounds)

@coderabbitai

coderabbitai Bot commented Aug 30, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 67972698-ca68-4597-9938-8df696f3c9be

📥 Commits

Reviewing files that changed from the base of the PR and between 9437e7b and e830066.

📒 Files selected for processing (6)
  • README.md
  • pr_split/cli.py
  • pr_split/git_ops/prs.py
  • pr_split/logs.py
  • tests/test_cli_new_features.py
  • tests/test_git_prs.py

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Resolved via the updated fix/merge-closed-while-polling (which already
contains origin/main). retarget_pr now reuses main's set_pr_base, and
the README describes the retarget against main's rewritten stacking
paragraph (children target their parent's branch with or without --stack).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015rnEEjXWDBdePRhKMmZr1w

Copy link
Copy Markdown
Owner Author

This change is already in main: it landed via the merge train #243, which includes this PR's head commit. GitHub can't mark a stacked PR merged when it lands that way, so I'm closing it as done.


Generated by Claude Code

@vitali87 vitali87 closed this Sep 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants