feat: more accurate head detection (#843)

We currently uses HEAD to the tip of the PR branch.
When it's a PR, the HEAD is a merge_commit, this merge commits may
contains file changes from main, but not from the PR.

This change adjusts the code to use head.sha instead.

This change the changed_files algo more accurate.

Author: sileht

mergify_cli/ci/scopes/changed_files.py

@@ -48,7 +48,7 @@ def ensure_git_history(base: str, head: str) -> None:
     last_commits_count = get_commits_count()
     while not has_merge_base(base, head):
         fetch_depth = min(fetch_depth * 2, sys.maxsize)
-        _run(["git", "fetch", f"--deepen={fetch_depth}", "origin", base, "HEAD"])
+        _run(["git", "fetch", f"--deepen={fetch_depth}", "origin", base, head])
         commits_count = get_commits_count()
         if commits_count == last_commits_count:
             if not has_merge_base(base, head):
@@ -59,8 +59,7 @@ def ensure_git_history(base: str, head: str) -> None:
         last_commits_count = commits_count
 
 
-def git_changed_files(base: str) -> list[str]:
-    head = "HEAD"
+def git_changed_files(base: str, head: str) -> list[str]:
     ensure_git_history(base, head)
     # Committed changes only between base_sha and HEAD.
     # Includes: Added (A), Copied (C), Modified (M), Renamed (R), Type-changed (T), Deleted (D)

mergify_cli/ci/scopes/cli.py

@@ -10,17 +10,18 @@
 import pydantic
 
 from mergify_cli import utils
-from mergify_cli.ci.scopes import base_detector
 from mergify_cli.ci.scopes import changed_files
 from mergify_cli.ci.scopes import config
 from mergify_cli.ci.scopes import exceptions
+from mergify_cli.ci.scopes import git_refs_detector
 
 
 if typing.TYPE_CHECKING:
     from collections import abc
 
 GITHUB_ACTIONS_SCOPES_OUTPUT_NAME = "scopes"
 GITHUB_ACTIONS_BASE_OUTPUT_NAME = "base"
+GITHUB_ACTIONS_HEAD_OUTPUT_NAME = "head"
 
 
 def match_scopes(
@@ -56,6 +57,7 @@ def match_scopes(
 
 def maybe_write_github_outputs(
     base: str,
+    head: str,
     all_scopes: abc.Iterable[str],
     scopes_hit: set[str],
 ) -> None:
@@ -73,21 +75,23 @@ def maybe_write_github_outputs(
             key: "true" if key in scopes_hit else "false" for key in sorted(all_scopes)
         }
         fh.write(f"{GITHUB_ACTIONS_BASE_OUTPUT_NAME}={base}\n")
+        fh.write(f"{GITHUB_ACTIONS_HEAD_OUTPUT_NAME}={head}\n")
         fh.write(
             f"{GITHUB_ACTIONS_SCOPES_OUTPUT_NAME}<<{delimiter}\n{json.dumps(data)}\n{delimiter}\n",
         )
 
 
 def maybe_write_github_step_summary(
     base: str,
+    head: str,
     all_scopes: abc.Iterable[str],
     scopes_hit: set[str],
 ) -> None:
     gha = os.environ.get("GITHUB_STEP_SUMMARY")
     if not gha:
         return
     # Build a pretty Markdown table with emojis
-    markdown = f"## Mergify CI Scope Matching Results for `{base}...HEAD`\n\n"
+    markdown = f"## Mergify CI Scope Matching Results for `{base}...{head}`\n\n"
     markdown += "| 🎯 Scope | ✅ Match |\n|:--|:--|\n"
     for scope in sorted(all_scopes):
         emoji = "✅" if scope in scopes_hit else "❌"
@@ -103,6 +107,7 @@ class InvalidDetectedScopeError(exceptions.ScopesError):
 
 class DetectedScope(pydantic.BaseModel):
     base_ref: str
+    head_ref: str
     scopes: set[str]
 
     def save_to_file(self, file: str) -> None:
@@ -120,9 +125,10 @@ def load_from_file(cls, filename: str) -> DetectedScope:
 
 def detect(config_path: str) -> DetectedScope:
     cfg = config.Config.from_yaml(config_path)
-    base = base_detector.detect()
+    git_refs = git_refs_detector.detect()
 
-    click.echo(f"Base: {base.ref}")
+    click.echo(f"Base: {git_refs.base}")
+    click.echo(f"Head: {git_refs.head}")
 
     scopes_hit: set[str]
     per_scope: dict[str, list[str]]
@@ -133,7 +139,7 @@ def detect(config_path: str) -> DetectedScope:
         scopes_hit = set()
         per_scope = {}
     elif isinstance(source, config.SourceFiles):
-        changed = changed_files.git_changed_files(base.ref)
+        changed = changed_files.git_changed_files(git_refs.base, git_refs.head)
         click.echo("Changed files detected:")
         for file in changed:
             click.echo(f"- {file}")
@@ -148,7 +154,7 @@ def detect(config_path: str) -> DetectedScope:
 
     if cfg.scopes.merge_queue_scope is not None:
         all_scopes.add(cfg.scopes.merge_queue_scope)
-        if base.is_merge_queue:
+        if git_refs.is_merge_queue:
             scopes_hit.add(cfg.scopes.merge_queue_scope)
 
     if scopes_hit:
@@ -161,9 +167,18 @@ def detect(config_path: str) -> DetectedScope:
     else:
         click.echo("No scopes matched.")
 
-    maybe_write_github_outputs(base.ref, all_scopes, scopes_hit)
-    maybe_write_github_step_summary(base.ref, all_scopes, scopes_hit)
-    return DetectedScope(base_ref=base.ref, scopes=scopes_hit)
+    maybe_write_github_outputs(git_refs.base, git_refs.head, all_scopes, scopes_hit)
+    maybe_write_github_step_summary(
+        git_refs.base,
+        git_refs.head,
+        all_scopes,
+        scopes_hit,
+    )
+    return DetectedScope(
+        base_ref=git_refs.base,
+        head_ref=git_refs.head,
+        scopes=scopes_hit,
+    )
 
 
 async def send_scopes(

mergify_cli/ci/scopes/base_detector.py …rgify_cli/ci/scopes/git_refs_detector.pymergify_cli/ci/scopes/base_detector.py renamed to mergify_cli/ci/scopes/git_refs_detector.py mergify_cli/ci/scopes/base_detector.py renamed to mergify_cli/ci/scopes/git_refs_detector.py

@@ -59,6 +59,16 @@ def _detect_base_from_merge_queue_payload(ev: dict[str, typing.Any]) -> str | No
     return None
 
 
+def _detect_head_from_event(ev: dict[str, typing.Any]) -> str | None:
+    pr = ev.get("pull_request")
+    if isinstance(pr, dict):
+        sha = pr.get("head", {}).get("sha")
+        if isinstance(sha, str) and sha:
+            return sha
+
+    return None
+
+
 def _detect_base_from_event(ev: dict[str, typing.Any]) -> str | None:
     pr = ev.get("pull_request")
     if isinstance(pr, dict):
@@ -77,6 +87,13 @@ def _detect_default_branch_from_event(ev: dict[str, typing.Any]) -> str | None:
     return None
 
 
+def _detect_head_from_push_event(ev: dict[str, typing.Any]) -> str | None:
+    sha = ev.get("after")
+    if isinstance(sha, str) and sha:
+        return sha
+    return None
+
+
 def _detect_base_from_push_event(ev: dict[str, typing.Any]) -> str | None:
     sha = ev.get("before")
     if isinstance(sha, str) and sha:
@@ -85,8 +102,9 @@ def _detect_base_from_push_event(ev: dict[str, typing.Any]) -> str | None:
 
 
 @dataclasses.dataclass
-class Base:
-    ref: str
+class References:
+    base: str
+    head: str
     is_merge_queue: bool
 
 
@@ -98,37 +116,39 @@ class Base:
 }
 
 
-def detect() -> Base:
+def detect() -> References:
     try:
         event_name, event = utils.get_github_event()
     except utils.GitHubEventNotFoundError:
         # fallback to last commit
-        return Base("HEAD^", is_merge_queue=False)
+        return References("HEAD^", "HEAD", is_merge_queue=False)
     else:
         if event_name in PULL_REQUEST_EVENTS:
+            head = _detect_head_from_event(event) or "HEAD"
             # 0) merge-queue PR override
             mq_sha = _detect_base_from_merge_queue_payload(event)
             if mq_sha:
-                return Base(mq_sha, is_merge_queue=True)
+                return References(mq_sha, head, is_merge_queue=True)
 
             # 1) standard event payload
             event_sha = _detect_base_from_event(event)
             if event_sha:
-                return Base(event_sha, is_merge_queue=False)
+                return References(event_sha, head, is_merge_queue=False)
 
             # 2) standard event payload
             event_sha = _detect_default_branch_from_event(event)
             if event_sha:
-                return Base(event_sha, is_merge_queue=False)
+                return References(event_sha, head, is_merge_queue=False)
 
         elif event_name == "push":
-            event_sha = _detect_base_from_push_event(event)
-            if event_sha:
-                return Base(event_sha, is_merge_queue=False)
+            head_sha = _detect_head_from_push_event(event) or "HEAD"
+            base_sha = _detect_base_from_push_event(event)
+            if base_sha:
+                return References(base_sha, head_sha, is_merge_queue=False)
 
             event_sha = _detect_default_branch_from_event(event)
             if event_sha:
-                return Base(event_sha, is_merge_queue=False)
+                return References(event_sha, "HEAD", is_merge_queue=False)
         else:
             msg = "Unhandled GITHUB_EVENT_NAME"
             raise BaseNotFoundError(msg)

mergify_cli/tests/ci/scopes/test_changed_files.py

@@ -20,47 +20,48 @@ def test_git_changed_files(mock_subprocess: test_utils.SubprocessMocks) -> None:
         "file1.py\nfile2.js\n",
     )
 
-    result = changed_files.git_changed_files("main")
+    result = changed_files.git_changed_files("main", "HEAD")
 
     assert result == ["file1.py", "file2.js"]
 
 
 def test_git_changed_files_fetch_alot_of_history(
     mock_subprocess: test_utils.SubprocessMocks,
 ) -> None:
-    sha = "b3deb84c4befe1918995b18eb06fa05f9074636d"
+    base = "b3deb84c4befe1918995b18eb06fa05f9074636d"
+    head = "9b6d25af10e6285862eb2476106f266d2aa303cf"
 
     mock_subprocess.register(
-        ["git", "merge-base", sha, "HEAD"],
+        ["git", "merge-base", base, head],
         "No such git object",
         1,
     )
     mock_subprocess.register(
-        ["git", "fetch", "--no-tags", "--depth=100", "origin", sha, "HEAD"],
+        ["git", "fetch", "--no-tags", "--depth=100", "origin", base, head],
     )
     mock_subprocess.register(["git", "rev-list", "--count", "--all"], "100")
 
     # Loop until we find it
     for count in (200, 400, 800, 1600):
         mock_subprocess.register(
-            ["git", "merge-base", sha, "HEAD"],
+            ["git", "merge-base", base, head],
             "No such git object",
             1,
         )
         mock_subprocess.register(
-            ["git", "fetch", f"--deepen={count}", "origin", sha, "HEAD"],
+            ["git", "fetch", f"--deepen={count}", "origin", base, head],
         )
         mock_subprocess.register(["git", "rev-list", "--count", "--all"], f"{count}")
 
     # We found it!
-    mock_subprocess.register(["git", "merge-base", sha, "HEAD"])
+    mock_subprocess.register(["git", "merge-base", base, head])
 
     mock_subprocess.register(
-        ["git", "diff", "--name-only", "--diff-filter=ACMRTD", f"{sha}...HEAD"],
+        ["git", "diff", "--name-only", "--diff-filter=ACMRTD", f"{base}...{head}"],
         "file1.py\nfile2.js\n",
     )
 
-    result = changed_files.git_changed_files(sha)
+    result = changed_files.git_changed_files(base, head)
 
     assert result == ["file1.py", "file2.js"]
 
@@ -74,7 +75,7 @@ def test_git_changed_files_empty(mock_subprocess: test_utils.SubprocessMocks) ->
         "",
     )
 
-    result = changed_files.git_changed_files("main")
+    result = changed_files.git_changed_files("main", "HEAD")
 
     assert result == []
 
@@ -90,4 +91,4 @@ def test_run_command_failure(mock_subprocess: test_utils.SubprocessMocks) -> Non
     )
 
     with pytest.raises(changed_files.ChangedFilesError, match="Command failed"):
-        changed_files.git_changed_files("main")
+        changed_files.git_changed_files("main", "HEAD")