diff --git a/README.md b/README.md index 000465b..b513b92 100644 --- a/README.md +++ b/README.md @@ -66,6 +66,14 @@ Works for same-repo and fork PRs: the PR's head (`refs/pull//head`) is fetche pr-split split '#42' --base main ``` +A bare number (with or without `#`) is always treated as a PR number, even if a +local branch or abbreviated commit hash with the same digits exists. To split a +local branch literally named like a number, pass its full ref: + +```bash +pr-split split refs/heads/1006 --base main +``` + ### Split a fork PR by user:branch ```bash diff --git a/pr_split/cli.py b/pr_split/cli.py index e2ea3e3..6670978 100644 --- a/pr_split/cli.py +++ b/pr_split/cli.py @@ -1220,7 +1220,14 @@ def _resolve_fork_ref(dev_branch: str) -> ForkPRInfo | None: @app.command(help="Split a large PR into smaller dependency-ordered PRs.") def split( - dev_branch: Annotated[str, typer.Argument(help="Branch name, PR number, or user:branch")], + dev_branch: Annotated[ + str, + typer.Argument( + help="Branch name, PR number, or user:branch. A bare number is always " + "treated as a PR number; for a local branch literally named like a " + "number, use its full ref, e.g. refs/heads/1006." + ), + ], base: Annotated[str, typer.Option(help="Base branch")] = "main", min_loc: Annotated[ int | None, @@ -1315,7 +1322,11 @@ def split( if not _is_fork_ref(name): adopt_remote_branch(name) - if not branch_exists(dev_branch): + # Decide the argument form before consulting git: `git rev-parse` accepts + # abbreviated SHAs, so a bare PR number such as 1006 would otherwise be + # taken for a commit whose hash starts with those digits and the wrong + # diff would be split without any warning. + if _is_fork_ref(dev_branch) or not branch_exists(dev_branch): # Only a PR number or user:branch can be fetched from GitHub; a plain # branch name that does not exist is just a typo, so say so before # touching gh (which --dry-run must not require). diff --git a/tests/test_cli_coverage.py b/tests/test_cli_coverage.py index 6c3e81f..464d514 100644 --- a/tests/test_cli_coverage.py +++ b/tests/test_cli_coverage.py @@ -715,6 +715,52 @@ def test_split_missing_plain_branch_reports_branch_not_found( mock_auth.assert_not_called() mock_resolve.assert_not_called() + @patch("pr_split.cli._resolve_fork_ref", return_value=None) + @patch("pr_split.cli.is_worktree_clean", return_value=True) + @patch("pr_split.cli.check_gh_auth", return_value=True) + @patch("pr_split.cli.branch_exists", return_value=True) + def test_pr_number_is_never_taken_for_an_abbreviated_sha( + self, + mock_be: MagicMock, + mock_auth: MagicMock, + mock_clean: MagicMock, + mock_resolve: MagicMock, + ) -> None: + # branch_exists says True because `git rev-parse 1006` resolves a + # commit whose hash starts with 1006; the argument is still PR #1006. + result = runner.invoke(app, ["split", "1006", "--dry-run"]) + assert result.exit_code == 1 + mock_resolve.assert_called_once_with("1006") + assert "Branch '1006' does not exist" in result.output + + def test_full_ref_is_not_a_fork_ref(self) -> None: + from pr_split.cli import _is_fork_ref + + # Documented escape hatch: a local branch literally named like a + # number is reachable through its full ref. + assert _is_fork_ref("refs/heads/1006") is False + assert _is_fork_ref("1006") is True + assert _is_fork_ref("#1006") is True + + @patch.dict("os.environ", {"ANTHROPIC_API_KEY": "test-key"}) + @patch("pr_split.cli.extract_diff", side_effect=RuntimeError("stop here")) + @patch("pr_split.cli._resolve_fork_ref", return_value=None) + @patch("pr_split.cli.is_worktree_clean", return_value=True) + @patch("pr_split.cli.check_gh_auth", return_value=True) + @patch("pr_split.cli.branch_exists", return_value=True) + def test_full_ref_escape_hatch_takes_the_branch_path( + self, + mock_be: MagicMock, + mock_auth: MagicMock, + mock_clean: MagicMock, + mock_resolve: MagicMock, + mock_extract: MagicMock, + ) -> None: + result = runner.invoke(app, ["split", "refs/heads/1006", "--dry-run"]) + mock_resolve.assert_not_called() + mock_extract.assert_called_once() + assert result.exit_code != 0 # stopped by the stubbed extract_diff + @patch("pr_split.cli.check_gh_auth", return_value=False) @patch("pr_split.cli.branch_exists", return_value=False) def test_split_fork_shaped_ref_still_checks_gh_auth(