commit-check · shenxianpeng · Aug 7, 2026 · Aug 6, 2026 · Aug 6, 2026 · Aug 6, 2026
@@ -21,6 +21,7 @@
     get_upstream_remote_sha,
     has_commits,
     git_merge_base,
+    git_rev_parse_verify,
 )
 from commit_check.imperatives import IMPERATIVES
 
@@ -481,6 +482,24 @@ def validate(self, context: ValidationContext) -> ValidationResult:
             return ValidationResult.PASS
 
         result = git_merge_base(target_branch, current_branch)
+        if result == 128:
+            # 128 is git failing to resolve a name, not an answer about
+            # ancestry. A CI checkout of a pull request leaves a detached HEAD
+            # with no local branch created, while get_branch_name() still
+            # reports a name from GITHUB_HEAD_REF — so the name here refers to
+            # nothing on disk. The remote-tracking ref is the real branch.
+            result = git_merge_base(target_branch, f"origin/{current_branch}")
+        if result == 128:
+            # Last resort, when the branch is unresolvable under either name.
+            # On a pull_request event HEAD is GitHub's synthetic merge commit,
+            # whose first parent IS the target tip, so asking about HEAD would
+            # pass every branch, rebased or not. Its *second* parent is the
+            # pull request head — the commit actually under review — so ask
+            # about that instead whenever HEAD is a merge. Where HEAD has a
+            # single parent it is the branch commit itself (a push event, or a
+            # branch that was never pushed) and answers for itself.
+            source = "HEAD^2" if git_rev_parse_verify("HEAD^2") else "HEAD"
+            result = git_merge_base(target_branch, source)
         if result == 0:
             return ValidationResult.PASS
 
@@ -527,7 +546,14 @@ def _find_target_branch(self, pattern: str) -> str | None:
                 stderr=subprocess.DEVNULL,
                 check=True,
             )
-            return branch_name
+            # Qualified with the remote, because that is the ref that was just
+            # verified. Returning the bare name here made the caller run
+            # ``git merge-base --is-ancestor main HEAD`` in a checkout that has
+            # only ``origin/main``; git exits 128 on the unresolvable name and
+            # the branch was reported as "not rebased onto target branch" when
+            # it was correctly based all along. A CI checkout of a pull request
+            # is exactly that shape.
+            return f"origin/{branch_name}"
         except subprocess.CalledProcessError:
             pass
 

@@ -195,6 +195,24 @@ def has_commits() -> bool:
         return False
 
 
+def git_rev_parse_verify(rev: str) -> bool:
+    """Check whether a revision resolves in the current repository.
+    :param rev: any revision expression, e.g. ``HEAD^2``
+
+    :returns: `True` if the revision resolves, `False` otherwise.
+    """
+    try:
+        subprocess.run(
+            ["git", "rev-parse", "--verify", "--quiet", f"{rev}^{{commit}}"],
+            stdout=subprocess.DEVNULL,
+            stderr=subprocess.DEVNULL,
+            check=True,
+        )
+        return True
+    except subprocess.CalledProcessError:
+        return False
+
+
 def get_commit_info(format_string: str, sha: str = "HEAD") -> str:
     """Get latest commits information
     :param format_string: could be

@@ -35,6 +35,84 @@
 USE_CONVENTIONAL_FORMAT = "Use conventional format"
 
 
+def _pull_request_shaped_clone(tmp_path):
+    """Build a clone shaped like a CI checkout of a pull request.
+
+    One commit on origin/main, one commit of work on top, then every local
+    branch removed and HEAD detached — so the target exists only as a
+    remote-tracking ref and the branch name resolves to nothing.
+
+    Returns the clone path.
+    """
+    import subprocess as sp
+
+    def git(*args, cwd):
+        return sp.run(
+            ["git", *args],
+            cwd=cwd,
+            check=True,
+            stdout=sp.PIPE,
+            stderr=sp.PIPE,
+            text=True,
+        )
+
+    origin = tmp_path / "origin"
+    origin.mkdir()
+    git("init", "-q", "-b", "main", ".", cwd=origin)
+    git("config", "user.name", "Dev", cwd=origin)
+    git("config", "user.email", "dev@example.com", cwd=origin)
+    git("commit", "-q", "--allow-empty", "-m", "feat: base", cwd=origin)
+
+    clone = tmp_path / "clone"
+    git("clone", "-q", str(origin), str(clone), cwd=tmp_path)
+    git("config", "user.name", "Dev", cwd=clone)
+    git("config", "user.email", "dev@example.com", cwd=clone)
+    git("checkout", "-q", "-b", "feat/work", cwd=clone)
+    git("commit", "-q", "--allow-empty", "-m", "feat: work", cwd=clone)
+    return clone, git
+
+
+def _diverged_merge_ref_clone(tmp_path):
+    """Extend the pull-request shape into the one that hides false passes.
+
+    Publishes feat/work, moves main past it so the branch is genuinely behind,
+    then detaches at a merge of the two — the shape of GitHub's synthetic merge
+    commit, whose first parent is the target tip. Every local branch is removed,
+    so the branch resolves only through refs/remotes/origin/feat/work.
+    """
+    clone, git = _pull_request_shaped_clone(tmp_path)
+    git("push", "-q", "origin", "feat/work", cwd=clone)
+    origin = tmp_path / "origin"
+    git("commit", "-q", "--allow-empty", "-m", "feat: main moved on", cwd=origin)
+    git("fetch", "-q", "origin", cwd=clone)
+    git("checkout", "-q", "--detach", "origin/main", cwd=clone)
+    git(
+        "merge",
+        "-q",
+        "--no-ff",
+        "-m",
+        "Merge feat/work into main",
+        "feat/work",
+        cwd=clone,
+    )
+    git("branch", "-q", "-D", "main", "feat/work", cwd=clone)
+    return clone, git
+
+
+def _validate_merge_base(clone, branch="feat/work", target="main"):
+    """Run MergeBaseValidator inside ``clone`` as a CI checkout would."""
+    cwd = os.getcwd()
+    try:
+        os.chdir(clone)
+        with patch.dict(os.environ, {"GITHUB_HEAD_REF": branch}):
+            validator = MergeBaseValidator(
+                ValidationRule(check="merge_base", regex=target)
+            )
+            return validator.validate(ValidationContext(no_banner=True))
+    finally:
+        os.chdir(cwd)
+
+
 class TestValidationResult:
     @pytest.mark.benchmark
     def test_validation_result_enum(self):
@@ -1045,22 +1123,38 @@ def test_validate_with_whitespace_only_message(self):
 
 
 class TestMergeBaseValidator:
-    @patch("commit_check.util.git_merge_base")
+    @patch("commit_check.engine.git_merge_base")
     @pytest.mark.benchmark
     def test_merge_base_validator_valid(self, mock_git_merge_base):
-        """Test MergeBaseValidator with valid merge base."""
+        """Test MergeBaseValidator with valid merge base.
+
+        Patched on commit_check.engine, not commit_check.util: the engine
+        imported the name directly, so patching the util module rebound
+        nothing and these tests were running real git against the checkout
+        they happened to be executed in.
+        """
         mock_git_merge_base.return_value = 0
 
-        rule = ValidationRule(check="merge_base")
+        # With no regex, validate() returns PASS at the "no target configured"
+        # exit without ever calling git_merge_base — the assertion below would
+        # hold no matter what the mock returned. Give it a target so the
+        # merge-base path actually runs.
+        rule = ValidationRule(check="merge_base", regex=r"^main$")
         validator = MergeBaseValidator(rule)
         context = ValidationContext()
 
-        result = validator.validate(context)
+        with (
+            patch.object(validator, "_find_target_branch", return_value="origin/main"),
+            patch("commit_check.engine.get_branch_name", return_value="feature/test"),
+            patch("commit_check.engine.has_commits", return_value=True),
+        ):
+            result = validator.validate(context)
         assert result == ValidationResult.PASS
+        mock_git_merge_base.assert_called_once_with("origin/main", "feature/test")
 
     @patch("commit_check.engine.has_commits")
     @patch("commit_check.engine.get_branch_name")
-    @patch("commit_check.util.git_merge_base")
+    @patch("commit_check.engine.git_merge_base")
     @pytest.mark.benchmark
     def test_merge_base_validator_invalid(
         self, mock_git_merge_base, mock_get_branch_name, mock_has_commits
@@ -1122,14 +1216,21 @@ def test_find_target_branch_local_found(self, mock_run):
 
     @patch("subprocess.run")
     def test_find_target_branch_local_missing_remote_found(self, mock_run):
-        """Local missing, remote tracking exists: returns the stripped branch name."""
+        """Local missing, remote tracking exists: returns the remote-qualified name.
+
+        Qualified, not bare: the caller feeds this straight to ``git merge-base
+        --is-ancestor``, and a checkout holding only ``origin/develop`` cannot
+        resolve ``develop``. Git exits 128 there, which the validator reports as
+        "not rebased onto target branch" — a clean branch failing for a name it
+        could not look up.
+        """
         mock_run.side_effect = [
             subprocess.CalledProcessError(1, []),  # local fails
             subprocess.CompletedProcess(args=[], returncode=0),  # remote succeeds
         ]
         validator = MergeBaseValidator(ValidationRule(check="merge_base"))
         result = validator._find_target_branch("develop")
-        assert result == "develop"
+        assert result == "origin/develop"
         assert mock_run.call_args_list[1][0][0][:5] == [
             "git",
             "rev-parse",
@@ -1148,6 +1249,80 @@ def test_find_target_branch_not_found(self, mock_run):
         result = validator._find_target_branch("nonexistent-branch")
         assert result is None
 
+    def test_merge_base_against_a_real_pull_request_shaped_checkout(self, tmp_path):
+        """A branch based on origin/main passes when no local main exists.
+
+        Mocked subprocess is what let this through: every call was asserted
+        against the arguments the code happened to pass, so a target name git
+        could not resolve looked correct. This drives real git instead.
+        """
+        from commit_check.util import git_merge_base
+
+        clone, git = _pull_request_shaped_clone(tmp_path)
+        git("branch", "-q", "-D", "main", cwd=clone)
+
+        cwd = os.getcwd()
+        try:
+            os.chdir(clone)
+            validator = MergeBaseValidator(ValidationRule(check="merge_base"))
+            target = validator._find_target_branch("main")
+            assert target == "origin/main"
+            # The point of the qualification: this is what the caller runs.
+            assert git_merge_base(target, "feat/work") == 0
+        finally:
+            os.chdir(cwd)
+
+    def test_merge_base_on_a_detached_checkout_with_no_local_branch(self, tmp_path):
+        """A detached checkout passes when the branch name resolves to nothing.
+
+        The other half of the same mistake: get_branch_name() falls back to
+        GITHUB_HEAD_REF, so on a CI checkout of a pull request it reports a
+        branch that was never created locally. git exits 128 on the name, and
+        128 was read as "not an ancestor" rather than "could not look that up".
+        """
+        clone, git = _pull_request_shaped_clone(tmp_path)
+        git("checkout", "-q", "--detach", "HEAD", cwd=clone)
+        git("branch", "-q", "-D", "main", cwd=clone)
+        git("branch", "-q", "-D", "feat/work", cwd=clone)
+
+        cwd = os.getcwd()
+        try:
+            os.chdir(clone)
+            with patch.dict(os.environ, {"GITHUB_HEAD_REF": "feat/never-created"}):
+                validator = MergeBaseValidator(
+                    ValidationRule(check="merge_base", regex="main")
+                )
+                result = validator.validate(ValidationContext())
+        finally:
+            os.chdir(cwd)
+        assert result == ValidationResult.PASS
+
+    def test_merge_base_fails_a_diverged_branch_on_a_merge_ref_checkout(self, tmp_path):
+        """A branch that is NOT rebased must fail, even on a merge-ref checkout.
+
+        On a pull_request event the runner checks out GitHub's synthetic merge
+        commit, whose first parent IS the target tip — so answering the
+        ancestry question from HEAD passes every branch, rebased or not. The
+        branch must be resolved through its remote-tracking ref instead; this
+        test pins that, and fails if the HEAD fallback is consulted first.
+        """
+        clone, _ = _diverged_merge_ref_clone(tmp_path)
+        assert _validate_merge_base(clone) == ValidationResult.FAIL
+
+    def test_merge_base_fails_a_diverged_branch_with_no_remote_ref(self, tmp_path):
+        """The same diverged branch must still fail when even the remote ref is
+        gone, which is where the fallback chain runs out of names.
+
+        Measured in this shape: origin/main vs feat/work is 128, vs
+        origin/feat/work is 128, and vs HEAD is 0 -- so falling through to HEAD
+        would pass a branch that is genuinely behind. HEAD's *second* parent is
+        the pull request head and answers 1, the truth.
+        """
+        clone, git = _diverged_merge_ref_clone(tmp_path)
+        # Leave the branch unresolvable under every name.
+        git("update-ref", "-d", "refs/remotes/origin/feat/work", cwd=clone)
+        assert _validate_merge_base(clone) == ValidationResult.FAIL
+
     @patch("subprocess.run")
     def test_find_target_branch_empty_pattern(self, mock_run):
         """Empty or anchor-only pattern: returns None without calling subprocess."""