From 9e71e250df94229db437059f9ef38bbfa1039132 Mon Sep 17 00:00:00 2001 From: Julien Danjou Date: Fri, 10 Apr 2026 11:25:12 +0200 Subject: [PATCH] feat(stack): add reorder command Add `mergify stack reorder` to non-interactively reorder all commits in a stack. Takes commit prefixes (SHA or Change-Id) in desired order and uses a temporary Python script as GIT_SEQUENCE_EDITOR to rewrite the rebase todo list, avoiding the sed quoting issues that make inline GIT_SEQUENCE_EDITOR unreliable for automation. Supports --dry-run to preview the new order without rebasing. Co-Authored-By: Claude Opus 4.6 (1M context) Change-Id: I46dc35fd28e3535e9f64f85581db87a50b6d4c21 Claude-Session-Id: cdc0d010-0b6d-4808-a13c-d8e205472a4b --- mergify_cli/stack/changes.py | 11 + mergify_cli/stack/cli.py | 15 + mergify_cli/stack/reorder.py | 228 +++++++++++++++ mergify_cli/stack/skill.md | 2 + mergify_cli/tests/stack/test_reorder.py | 360 ++++++++++++++++++++++++ 5 files changed, 616 insertions(+) create mode 100644 mergify_cli/stack/reorder.py create mode 100644 mergify_cli/tests/stack/test_reorder.py diff --git a/mergify_cli/stack/changes.py b/mergify_cli/stack/changes.py index 7979f485..884d3219 100644 --- a/mergify_cli/stack/changes.py +++ b/mergify_cli/stack/changes.py @@ -30,6 +30,17 @@ CHANGEID_RE = re.compile(r"Change-Id: (I[0-9a-z]{40})") + + +def is_change_id_prefix(prefix: str) -> bool: + """Return True if *prefix* looks like a Change-Id prefix.""" + return ( + len(prefix) >= 2 + and prefix[0] == "I" + and all(c in "0123456789abcdef" for c in prefix[1:]) + ) + + ChangeId = typing.NewType("ChangeId", str) RemoteChanges = typing.NewType( "RemoteChanges", diff --git a/mergify_cli/stack/cli.py b/mergify_cli/stack/cli.py index 07c9170e..f7eb06fb 100644 --- a/mergify_cli/stack/cli.py +++ b/mergify_cli/stack/cli.py @@ -19,6 +19,7 @@ from mergify_cli.stack import new as stack_new_mod from mergify_cli.stack import open as stack_open_mod from mergify_cli.stack import push as stack_push_mod +from mergify_cli.stack import reorder as stack_reorder_mod from mergify_cli.stack import setup as stack_setup_mod from mergify_cli.stack import skill as stack_skill_mod @@ -201,6 +202,20 @@ async def edit() -> None: await stack_edit_mod.stack_edit() +@stack.command(help="Reorder the stack's commits") +@click.argument("commits", nargs=-1, required=True) +@click.option( + "--dry-run", + "-n", + is_flag=True, + default=False, + help="Show the plan without reordering", +) +@utils.run_with_asyncio +async def reorder(*, commits: tuple[str, ...], dry_run: bool) -> None: + await stack_reorder_mod.stack_reorder(list(commits), dry_run=dry_run) + + @stack.command(help="Create a new stack branch") @click.argument("name") @click.option( diff --git a/mergify_cli/stack/reorder.py b/mergify_cli/stack/reorder.py new file mode 100644 index 00000000..718946f4 --- /dev/null +++ b/mergify_cli/stack/reorder.py @@ -0,0 +1,228 @@ +# +# Copyright © 2021-2026 Mergify SAS +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from __future__ import annotations + +import os +import pathlib +import shlex +import subprocess +import sys +import tempfile + +from mergify_cli import console +from mergify_cli import utils +from mergify_cli.stack.changes import CHANGEID_RE +from mergify_cli.stack.changes import is_change_id_prefix + + +def get_stack_commits(base: str) -> list[tuple[str, str, str]]: + """Return (full_sha, subject, change_id) tuples from base to HEAD. + + Uses ``git log --reverse`` so the list is in commit order + (oldest first). + """ + raw = subprocess.check_output( # noqa: S603 + ["git", "log", "--reverse", "--format=%H%x00%s%x00%b%x1e", f"{base}..HEAD"], + text=True, + ) + commits: list[tuple[str, str, str]] = [] + for record in raw.split("\x1e"): + stripped = record.strip() + if not stripped: + continue + parts = stripped.split("\x00", 2) + if len(parts) != 3: + continue + sha = parts[0].strip() + subject = parts[1].strip() + body = parts[2].strip() + match = CHANGEID_RE.search(body) + change_id = match.group(1) if match else "" + commits.append((sha, subject, change_id)) + return commits + + +def match_commit( + prefix: str, + commits: list[tuple[str, str, str]], +) -> tuple[str, str, str]: + """Match a SHA or Change-Id prefix to exactly one commit. + + Auto-detect: if prefix starts with ``I`` and the rest is hex, match + against the change_id field; otherwise match against the sha field. + + Calls ``sys.exit(1)`` with an error message on no match or ambiguous + match. + """ + if is_change_id_prefix(prefix): + matches = [c for c in commits if c[2].startswith(prefix)] + field_name = "Change-Id" + else: + matches = [c for c in commits if c[0].startswith(prefix)] + field_name = "SHA" + + if len(matches) == 0: + console.print( + f"error: no commit found matching {field_name} prefix '{prefix}'", + style="red", + ) + sys.exit(1) + if len(matches) > 1: + console.print( + f"error: ambiguous {field_name} prefix '{prefix}' matches {len(matches)} commits:", + style="red", + ) + for sha, subject, change_id in matches: + console.print(f" {sha[:12]} {subject} ({change_id[:12]})", style="red") + sys.exit(1) + + return matches[0] + + +def run_rebase(base: str, ordered_shas: list[str]) -> None: + """Run ``git rebase -i`` with a generated sequence editor script. + + The temporary Python script rewrites the rebase todo list so that + the pick lines appear in the order given by *ordered_shas*. + """ + script_content = ( + "#!/usr/bin/env python3\n" + "import sys\n" + "order = " + repr(ordered_shas) + "\n" + "todo_path = sys.argv[1]\n" + "with open(todo_path) as f:\n" + " lines = f.readlines()\n" + "pick_lines = {}\n" + "other_lines = []\n" + "for line in lines:\n" + " stripped = line.strip()\n" + " if stripped and not stripped.startswith('#'):\n" + " parts = stripped.split(None, 2)\n" + " if len(parts) >= 2:\n" + " pick_lines[parts[1]] = line\n" + " else:\n" + " other_lines.append(line)\n" + " else:\n" + " other_lines.append(line)\n" + "reordered = []\n" + "for sha in order:\n" + " for key in pick_lines:\n" + " if sha.startswith(key) or key.startswith(sha):\n" + " reordered.append(pick_lines[key])\n" + " break\n" + "with open(todo_path, 'w') as f:\n" + " f.writelines(reordered + other_lines)\n" + ) + + tmp_fd, tmp_path = tempfile.mkstemp(suffix=".py", prefix="mergify_reorder_") + try: + with os.fdopen(tmp_fd, "w") as f: + f.write(script_content) + pathlib.Path(tmp_path).chmod(0o755) + + env = os.environ.copy() + python = shlex.quote(sys.executable) + script = shlex.quote(tmp_path) + env["GIT_SEQUENCE_EDITOR"] = f"{python} {script}" + + result = subprocess.run( # noqa: S603 + ["git", "rebase", "-i", base], + env=env, + ) + + if result.returncode != 0: + console.print( + "error: rebase failed — there may be conflicts", + style="red", + ) + console.print( + "Resolve conflicts then run: git rebase --continue", + ) + console.print( + "Or abort the rebase with: git rebase --abort", + ) + sys.exit(1) + finally: + tmp_file = pathlib.Path(tmp_path) + if tmp_file.exists(): + tmp_file.unlink() + + +def display_plan( + title: str, + commits: list[tuple[str, str, str]], +) -> None: + """Print the planned commit order.""" + console.log(title) + for idx, (sha, subject, change_id) in enumerate(commits, 1): + cid_display = f" ({change_id[:12]})" if change_id else "" + console.log(f" {idx}. {sha[:12]} {subject}{cid_display}") + + +async def stack_reorder( + commit_prefixes: list[str], + *, + dry_run: bool, +) -> None: + os.chdir(await utils.git("rev-parse", "--show-toplevel")) + trunk = await utils.get_trunk() + base = await utils.git("merge-base", trunk, "HEAD") + commits = get_stack_commits(base) + + if not commits: + console.print("No commits in the stack", style="green") + return + + if len(commit_prefixes) != len(commits): + console.print( + f"error: expected {len(commits)} commits but got {len(commit_prefixes)} prefixes", + style="red", + ) + sys.exit(1) + + # Match each prefix to a commit + matched = [match_commit(p, commits) for p in commit_prefixes] + + # Check for duplicates + matched_shas = [c[0] for c in matched] + if len(set(matched_shas)) != len(matched_shas): + seen: set[str] = set() + for prefix, sha in zip(commit_prefixes, matched_shas, strict=True): + if sha in seen: + console.print( + f"error: duplicate — prefix '{prefix}' resolves to the same commit as another prefix", + style="red", + ) + sys.exit(1) + seen.add(sha) + + # Check if already in order + current_shas = [c[0] for c in commits] + if matched_shas == current_shas: + console.print( + "Stack is already in the requested order", + style="green", + ) + return + + display_plan("Reorder plan:", matched) + + if dry_run: + console.print("Dry run — no changes made", style="green") + return + + run_rebase(base, matched_shas) + console.print("Stack reordered successfully", style="green") diff --git a/mergify_cli/stack/skill.md b/mergify_cli/stack/skill.md index f263429b..b6da7167 100644 --- a/mergify_cli/stack/skill.md +++ b/mergify_cli/stack/skill.md @@ -21,6 +21,7 @@ A branch is a stack. Keep stacks short and focused: - **Push**: Use `mergify stack push` (never `git push`) - **Fixes**: Use `git commit --amend` (never create new commits to fix issues) - **Mid-stack fixes**: Use `git rebase -i` to edit the specific commit, amend it, continue rebase, then `mergify stack push` +- **Reordering**: Use `mergify stack reorder` (list all commits in desired order) instead of manual `git rebase -i` — non-interactive and avoids `GIT_SEQUENCE_EDITOR` quoting issues - **Commit titles**: Follow [Conventional Commits](https://www.conventionalcommits.org/) (e.g., `feat:`, `fix:`, `docs:`) - **PR title & body**: `mergify stack` copies the commit message title to the PR title and the commit message body to the PR body — so write commit messages as if they were PR descriptions. **Everything that should appear in the PR (ticket references, context, test plans) MUST go in the commit message.** - **Ticket references**: Include ticket/issue references (e.g., `MRGFY-1234`, `Fixes #123`) in the commit message body, not added separately to the PR. @@ -46,6 +47,7 @@ mergify stack new NAME # Create a new stack/branch for new work mergify stack push # Push and create/update PRs mergify stack list # Show commit <-> PR mapping for current stack mergify stack list --json # Same, but machine-readable JSON output +mergify stack reorder C A B # Reorder all commits (pass SHA or Change-Id prefixes) ``` Use `mergify stack list` to see which commits have been pushed, which PRs they map to, and whether the stack is up to date with the remote. This is the go-to command to understand the current state of a stack. Use `--json` when you need to parse the output programmatically. diff --git a/mergify_cli/tests/stack/test_reorder.py b/mergify_cli/tests/stack/test_reorder.py new file mode 100644 index 00000000..dbb23f14 --- /dev/null +++ b/mergify_cli/tests/stack/test_reorder.py @@ -0,0 +1,360 @@ +# +# Copyright © 2021-2026 Mergify SAS +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from __future__ import annotations + +import os +import re +import subprocess +from typing import TYPE_CHECKING + +import pytest + +from mergify_cli.stack.reorder import get_stack_commits +from mergify_cli.stack.reorder import match_commit +from mergify_cli.stack.reorder import stack_reorder + + +if TYPE_CHECKING: + import pathlib + + +def _run_git(*args: str, cwd: pathlib.Path | None = None) -> str: + return subprocess.check_output( + ["git", *args], + text=True, + cwd=cwd, + ).strip() + + +def _create_commit( + repo: pathlib.Path, + filename: str, + content: str, + message: str, +) -> tuple[str, str | None]: + """Create a commit and return (sha, change_id).""" + (repo / filename).write_text(content) + _run_git("add", filename, cwd=repo) + _run_git("commit", "-m", message, cwd=repo) + sha = _run_git("rev-parse", "HEAD", cwd=repo) + body = _run_git("log", "-1", "--format=%b", "HEAD", cwd=repo) + change_id_match = re.search(r"Change-Id: (I[0-9a-z]{40})", body) + return sha, change_id_match.group(1) if change_id_match else None + + +def _get_commit_subjects(repo: pathlib.Path, n: int = 10) -> list[str]: + """Return the last n commit subjects, oldest first.""" + raw = _run_git( + "log", + "--reverse", + f"-{n}", + "--format=%s", + cwd=repo, + ) + return [line for line in raw.splitlines() if line.strip()] + + +def _setup_tracking(repo: pathlib.Path) -> None: + """Create a bare origin and set up tracking for the current branch.""" + origin_path = repo.parent / f"{repo.name}_origin.git" + _run_git("init", "--bare", str(origin_path)) + _run_git("remote", "add", "origin", str(origin_path), cwd=repo) + _run_git("push", "origin", "main", cwd=repo) + _run_git("branch", "--set-upstream-to=origin/main", cwd=repo) + + +@pytest.fixture +def stack_repo( + git_repo_with_hooks: pathlib.Path, +) -> tuple[pathlib.Path, list[tuple[str, str | None]]]: + """Create a repo with 3 commits (A, B, C) on a feature branch.""" + repo = git_repo_with_hooks + + # Create an initial commit on main + (repo / "init.txt").write_text("init") + _run_git("add", "init.txt", cwd=repo) + _run_git("commit", "-m", "Initial commit", cwd=repo) + + _setup_tracking(repo) + + # Create a feature branch + _run_git("checkout", "-b", "feature", "main", cwd=repo) + _run_git("branch", "--set-upstream-to=origin/main", cwd=repo) + + # Create 3 commits + commits = [] + for label, filename in [("A", "a.txt"), ("B", "b.txt"), ("C", "c.txt")]: + sha, cid = _create_commit(repo, filename, f"content {label}", f"Commit {label}") + commits.append((sha, cid)) + + return repo, commits + + +class TestGetStackCommits: + def test_returns_commits_in_order( + self, + stack_repo: tuple[pathlib.Path, list[tuple[str, str | None]]], + ) -> None: + repo, expected_commits = stack_repo + os.chdir(repo) + base = _run_git("merge-base", "origin/main", "HEAD", cwd=repo) + result = get_stack_commits(base) + assert len(result) == 3 + for (sha, _subject, change_id), (expected_sha, expected_cid) in zip( + result, + expected_commits, + strict=True, + ): + assert sha == expected_sha + assert expected_cid is not None + assert change_id == expected_cid + + +class TestMatchCommit: + def test_match_by_sha_prefix(self) -> None: + commits = [ + ("abc123def456", "Commit A", "I0000000000000000000000000000000000000001"), + ("def456abc123", "Commit B", "I0000000000000000000000000000000000000002"), + ] + result = match_commit("abc", commits) + assert result == commits[0] + + def test_match_by_change_id_prefix(self) -> None: + commits = [ + ("abc123def456", "Commit A", "I1111111111111111111111111111111111111111"), + ("def456abc123", "Commit B", "I2222222222222222222222222222222222222222"), + ] + result = match_commit("I111", commits) + assert result == commits[0] + + def test_no_match_exits(self) -> None: + commits = [ + ("abc123def456", "Commit A", "I1111111111111111111111111111111111111111"), + ] + with pytest.raises(SystemExit) as exc_info: + match_commit("zzz", commits) + assert exc_info.value.code == 1 + + def test_ambiguous_match_exits(self) -> None: + commits = [ + ("abc123000000", "Commit A", "I1111111111111111111111111111111111111111"), + ("abc123999999", "Commit B", "I2222222222222222222222222222222222222222"), + ] + with pytest.raises(SystemExit) as exc_info: + match_commit("abc123", commits) + assert exc_info.value.code == 1 + + +class TestStackReorder: + async def test_reorder_success( + self, + stack_repo: tuple[pathlib.Path, list[tuple[str, str | None]]], + ) -> None: + """Reorder A,B,C -> C,A,B and verify new order.""" + repo, commits = stack_repo + os.chdir(repo) + + sha_a = commits[0][0][:12] + sha_b = commits[1][0][:12] + sha_c = commits[2][0][:12] + + await stack_reorder([sha_c, sha_a, sha_b], dry_run=False) + + subjects = _get_commit_subjects(repo) + # Filter to only our feature commits + feature_subjects = [s for s in subjects if s.startswith("Commit")] + assert feature_subjects == ["Commit C", "Commit A", "Commit B"] + + async def test_reorder_with_change_id( + self, + stack_repo: tuple[pathlib.Path, list[tuple[str, str | None]]], + ) -> None: + """Reorder using Change-Id prefixes.""" + repo, commits = stack_repo + os.chdir(repo) + + cid_a = commits[0][1] + cid_b = commits[1][1] + cid_c = commits[2][1] + assert cid_a is not None + assert cid_b is not None + assert cid_c is not None + + # Use Change-Id prefixes (first 8 chars) + await stack_reorder( + [cid_c[:8], cid_a[:8], cid_b[:8]], + dry_run=False, + ) + + subjects = _get_commit_subjects(repo) + feature_subjects = [s for s in subjects if s.startswith("Commit")] + assert feature_subjects == ["Commit C", "Commit A", "Commit B"] + + async def test_reorder_dry_run( + self, + stack_repo: tuple[pathlib.Path, list[tuple[str, str | None]]], + ) -> None: + """Verify dry-run doesn't change anything.""" + repo, commits = stack_repo + os.chdir(repo) + + head_before = _run_git("rev-parse", "HEAD", cwd=repo) + + sha_c = commits[2][0][:12] + sha_a = commits[0][0][:12] + sha_b = commits[1][0][:12] + + await stack_reorder([sha_c, sha_a, sha_b], dry_run=True) + + head_after = _run_git("rev-parse", "HEAD", cwd=repo) + assert head_before == head_after + + async def test_reorder_already_in_order( + self, + stack_repo: tuple[pathlib.Path, list[tuple[str, str | None]]], + ) -> None: + """Pass commits in current order: no rebase should happen.""" + repo, commits = stack_repo + os.chdir(repo) + + head_before = _run_git("rev-parse", "HEAD", cwd=repo) + + sha_a = commits[0][0][:12] + sha_b = commits[1][0][:12] + sha_c = commits[2][0][:12] + + await stack_reorder([sha_a, sha_b, sha_c], dry_run=False) + + head_after = _run_git("rev-parse", "HEAD", cwd=repo) + assert head_before == head_after + + async def test_reorder_wrong_count_too_few( + self, + stack_repo: tuple[pathlib.Path, list[tuple[str, str | None]]], + ) -> None: + """Pass 2 prefixes for a 3-commit stack.""" + repo, commits = stack_repo + os.chdir(repo) + + sha_a = commits[0][0][:12] + sha_b = commits[1][0][:12] + + with pytest.raises(SystemExit) as exc_info: + await stack_reorder([sha_a, sha_b], dry_run=False) + assert exc_info.value.code == 1 + + async def test_reorder_wrong_count_too_many( + self, + stack_repo: tuple[pathlib.Path, list[tuple[str, str | None]]], + ) -> None: + """Pass 4 prefixes for a 3-commit stack.""" + repo, commits = stack_repo + os.chdir(repo) + + sha_a = commits[0][0][:12] + sha_b = commits[1][0][:12] + sha_c = commits[2][0][:12] + + with pytest.raises(SystemExit) as exc_info: + await stack_reorder( + [sha_a, sha_b, sha_c, sha_a], + dry_run=False, + ) + assert exc_info.value.code == 1 + + async def test_reorder_unknown_prefix( + self, + stack_repo: tuple[pathlib.Path, list[tuple[str, str | None]]], + ) -> None: + """Pass a prefix that doesn't match any commit.""" + repo, commits = stack_repo + os.chdir(repo) + + sha_a = commits[0][0][:12] + sha_b = commits[1][0][:12] + + with pytest.raises(SystemExit) as exc_info: + await stack_reorder( + [sha_a, sha_b, "deadbeef1234"], + dry_run=False, + ) + assert exc_info.value.code == 1 + + async def test_reorder_ambiguous_prefix(self) -> None: + """Test match_commit logic with ambiguous prefix.""" + commits = [ + ( + "abc1230000000000000000000000000000000000", + "A", + "Ia000000000000000000000000000000000000000", + ), + ( + "abc1239999999999999999999999999999999999", + "B", + "Ib000000000000000000000000000000000000000", + ), + ( + "def4560000000000000000000000000000000000", + "C", + "Ic000000000000000000000000000000000000000", + ), + ] + with pytest.raises(SystemExit) as exc_info: + match_commit("abc123", commits) + assert exc_info.value.code == 1 + + async def test_reorder_duplicate_prefix( + self, + stack_repo: tuple[pathlib.Path, list[tuple[str, str | None]]], + ) -> None: + """Pass same prefix twice.""" + repo, commits = stack_repo + os.chdir(repo) + + sha_a = commits[0][0][:12] + sha_b = commits[1][0][:12] + + with pytest.raises(SystemExit) as exc_info: + await stack_reorder([sha_a, sha_a, sha_b], dry_run=False) + assert exc_info.value.code == 1 + + async def test_reorder_empty_stack( + self, + git_repo_with_hooks: pathlib.Path, + ) -> None: + """No commits between base and HEAD.""" + repo = git_repo_with_hooks + + # Create just an initial commit on main + (repo / "init.txt").write_text("init") + _run_git("add", "init.txt", cwd=repo) + _run_git("commit", "-m", "Initial commit", cwd=repo) + + _setup_tracking(repo) + + # Create feature branch with no new commits + _run_git("checkout", "-b", "feature", "main", cwd=repo) + _run_git("branch", "--set-upstream-to=origin/main", cwd=repo) + + os.chdir(repo) + + # Should just print no-op and return without error + # We need to pass at least 1 prefix to click, but stack_reorder + # checks the stack first - empty stack returns early + # Actually the function validates count vs stack size, and since + # there are no commits it would say "no commits in the stack" + # Let's just call with an empty list to test the empty branch + await stack_reorder([], dry_run=False)