From b5eeb6473ed5c4c3cd7a098ff88ebbe7fbd67e27 Mon Sep 17 00:00:00 2001 From: Remy DUTHU Date: Thu, 26 Feb 2026 10:22:46 +0100 Subject: [PATCH] fix(stack): Rebase branch in dry-run mode for accurate plan output The dry-run plan was showing stale pre-rebase commit hashes, making its output inconsistent with actual push. Rebase is now always performed unless --skip-rebase is explicitly passed. Co-Authored-By: Claude Opus 4.6 Change-Id: If1e56b2bd38dc668e57ffb814b06436225e6837d --- mergify_cli/stack/changes.py | 5 + mergify_cli/stack/push.py | 20 ++++ mergify_cli/tests/stack/test_push.py | 160 +++++++++++++++++++++++++++ 3 files changed, 185 insertions(+) diff --git a/mergify_cli/stack/changes.py b/mergify_cli/stack/changes.py index 00abb7a2..e6a5a27d 100644 --- a/mergify_cli/stack/changes.py +++ b/mergify_cli/stack/changes.py @@ -203,6 +203,11 @@ class Changes: locals: list[LocalChange] = dataclasses.field(default_factory=list) orphans: list[OrphanChange] = dataclasses.field(default_factory=list) + def replace_local_action(self, old: ActionT, new: ActionT) -> None: + for change in self.locals: + if change.action == old: + change.action = new + def display_plan( changes: Changes, diff --git a/mergify_cli/stack/push.py b/mergify_cli/stack/push.py index e707db27..3e0d20da 100644 --- a/mergify_cli/stack/push.py +++ b/mergify_cli/stack/push.py @@ -215,6 +215,20 @@ async def stack_push( await utils.git("pull", "--rebase", remote, base_branch) console.log(f"branch `{dest_branch}` rebased on `{remote}/{base_branch}`") + rebase_required = False + if dry_run and not skip_rebase: + commits_behind = int( + await utils.git("rev-list", "--count", f"HEAD..{remote}/{base_branch}"), + ) + rebase_required = commits_behind > 0 + + if rebase_required: + console.log( + f"[orange]branch `{dest_branch}` is behind `{remote}/{base_branch}` " + f"by {commits_behind} {'commit' if commits_behind == 1 else 'commits'}, " + f"commit SHAs will differ after rebase[/]", + ) + base_commit_sha = await utils.git( "merge-base", "--fork-point", @@ -249,6 +263,12 @@ async def stack_push( next_only=next_only, ) + if rebase_required: + # If the branch is behind, we know for sure that all the existing + # pull requests will need to be updated, so we can directly plan + # them as "update" instead of "skip-up-to-date". + planned_changes.replace_local_action(old="skip-up-to-date", new="update") + changes.display_plan( planned_changes, create_as_draft=create_as_draft, diff --git a/mergify_cli/tests/stack/test_push.py b/mergify_cli/tests/stack/test_push.py index 753b844d..9ac20b51 100644 --- a/mergify_cli/tests/stack/test_push.py +++ b/mergify_cli/tests/stack/test_push.py @@ -19,6 +19,7 @@ import pytest +from mergify_cli.stack import changes from mergify_cli.stack import push from mergify_cli.tests import utils as test_utils @@ -491,6 +492,142 @@ async def test_stack_update_keep_title_and_body( } +@pytest.mark.respx(base_url="https://api.github.com/") +async def test_stack_dry_run_does_not_rebase( + git_mock: test_utils.GitMock, + respx_mock: respx.MockRouter, +) -> None: + git_mock.commit( + test_utils.Commit( + sha="commit1_sha", + title="Title commit 1", + message="Message commit 1", + change_id="I29617d37762fd69809c255d7e7073cb11f8fbf50", + ), + ) + git_mock.finalize() + git_mock.mock("rev-list", "--count", "HEAD..origin/main", output="0") + + respx_mock.get("/user").respond(200, json={"login": "author"}) + respx_mock.get("/search/issues").respond(200, json={"items": []}) + + with pytest.raises(SystemExit, match="0"): + await push.stack_push( + github_server="https://api.github.com/", + token="", + skip_rebase=False, + next_only=False, + branch_prefix="", + dry_run=True, + trunk=("origin", "main"), + ) + + # Dry-run never rebases. + assert not git_mock.has_been_called_with("pull", "--rebase", "origin", "main") + + # No branches are pushed. + assert not git_mock.has_been_called_with( + "push", + "-f", + "origin", + "commit1_sha:refs/heads/current-branch/I29617d37762fd69809c255d7e7073cb11f8fbf50", + ) + + +@pytest.mark.respx(base_url="https://api.github.com/") +async def test_stack_dry_run_behind_flips_up_to_date_to_update( + git_mock: test_utils.GitMock, + respx_mock: respx.MockRouter, +) -> None: + # PR exists with matching SHA — normally "skip-up-to-date". + # But branch is behind base, so rebase would change SHAs → "update". + git_mock.commit( + test_utils.Commit( + sha="commit1_sha", + title="Title commit 1", + message="Message commit 1", + change_id="I29617d37762fd69809c255d7e7073cb11f8fbf50", + ), + ) + git_mock.finalize() + git_mock.mock("rev-list", "--count", "HEAD..origin/main", output="3") + + respx_mock.get("/user").respond(200, json={"login": "author"}) + respx_mock.get("/search/issues").respond( + 200, + json={ + "items": [ + { + "pull_request": { + "url": "https://api.github.com/repos/user/repo/pulls/42", + }, + }, + ], + }, + ) + respx_mock.get("/repos/user/repo/pulls/42").respond( + 200, + json={ + "html_url": "", + "head": { + "sha": "commit1_sha", + "ref": "current-branch/I29617d37762fd69809c255d7e7073cb11f8fbf50", + }, + "state": "open", + "merged_at": None, + "draft": False, + }, + ) + + with pytest.raises(SystemExit, match="0"): + await push.stack_push( + github_server="https://api.github.com/", + token="", + skip_rebase=False, + next_only=False, + branch_prefix="", + dry_run=True, + trunk=("origin", "main"), + ) + + # Dry-run never rebases. + assert not git_mock.has_been_called_with("pull", "--rebase", "origin", "main") + + +@pytest.mark.respx(base_url="https://api.github.com/") +async def test_stack_dry_run_skip_rebase( + git_mock: test_utils.GitMock, + respx_mock: respx.MockRouter, +) -> None: + git_mock.commit( + test_utils.Commit( + sha="commit1_sha", + title="Title commit 1", + message="Message commit 1", + change_id="I29617d37762fd69809c255d7e7073cb11f8fbf50", + ), + ) + git_mock.finalize() + + respx_mock.get("/user").respond(200, json={"login": "author"}) + respx_mock.get("/search/issues").respond(200, json={"items": []}) + + with pytest.raises(SystemExit, match="0"): + await push.stack_push( + github_server="https://api.github.com/", + token="", + skip_rebase=True, + next_only=False, + branch_prefix="", + dry_run=True, + trunk=("origin", "main"), + ) + + # Rebase check is skipped when --skip-rebase is passed. + assert not git_mock.has_been_called_with("rev-list", "--count", "HEAD..origin/main") + assert not git_mock.has_been_called_with("pull", "--rebase", "origin", "main") + + @pytest.mark.respx(base_url="https://api.github.com/") async def test_stack_on_destination_branch_raises_an_error( git_mock: test_utils.GitMock, @@ -535,3 +672,26 @@ async def test_stack_without_common_commit_raises_an_error( dry_run=False, trunk=("origin", "main"), ) + + +def test_replace_local_action_flips_up_to_date() -> None: + def _make_local_change(action: changes.ActionT) -> changes.LocalChange: + return changes.LocalChange( + id=changes.ChangeId(""), + pull=None, + commit_sha="", + title="", + message="", + base_branch="", + dest_branch="", + action=action, + ) + + planned = changes.Changes(stack_prefix="") + planned.locals.append(_make_local_change("skip-up-to-date")) + planned.locals.append(_make_local_change("create")) + + planned.replace_local_action(old="skip-up-to-date", new="update") + + assert planned.locals[0].action == "update" + assert planned.locals[1].action == "create"