Skip to content

Commit 9b32c79

Browse files
authored
feat: check a pull request's commits without fetch-depth: 0 (#295)
1 parent 82a93ea commit 9b32c79

3 files changed

Lines changed: 186 additions & 21 deletions

File tree

‎README.md‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,8 @@ jobs:
4646
steps:
4747
- uses: actions/checkout@v7
4848
with:
49-
# Required. With the default fetch-depth: 1 the clone holds only GitHub's
50-
# merge commit: the PR's own commits cannot be listed, author checks are
51-
# skipped, and the action warns and falls back to checking HEAD alone.
49+
# Recommended: the action then reads the PR's commits from the clone.
50+
# With the default fetch-depth: 1 it asks the GitHub API instead.
5251
fetch-depth: 0
5352
- uses: commit-check/commit-check-action@v2
5453
with:
@@ -60,11 +59,13 @@ jobs:
6059
pr-comments: true
6160
```
6261
63-
> [!IMPORTANT]
64-
> Keep `fetch-depth: 0`. A shallow clone holds only GitHub's merge commit,
65-
> whose `Merge <sha> into <sha>` subject passes the default rules, so every
66-
> pull request would look green; the action warns when it happens. On
67-
> `pull_request_target`, also check out `refs/pull/<number>/merge`.
62+
> [!NOTE]
63+
> `fetch-depth: 0` is recommended, not required. A shallow clone holds only
64+
> GitHub's merge commit, so the action lists the pull request's commits
65+
> through the API — up to 250, with `pull-requests: read` — and fetches the
66+
> head commit for the author checks. If the commits cannot be listed, it warns
67+
> and checks HEAD alone; if the head commit cannot be fetched, it warns and
68+
> skips the author checks.
6869

6970
Runs on `ubuntu-latest`, `macos-latest` and `windows-latest`. Self-hosted
7071
runners need a few tools — see [Good to know](#good-to-know).
@@ -263,7 +264,7 @@ The action is a composite step and uses what the runner already has:
263264
dependencies are pinned by `requirements.txt` but not verified.
264265
- **Network access to PyPI and `api.github.com`** — the pinned wheels are
265266
downloaded once per run and the attestation is fetched from GitHub.
266-
- **`git`** on `PATH`, and a checkout with `fetch-depth: 0` (see above).
267+
- **`git`** on `PATH`.
267268

268269
There is currently no input to skip attestation verification.
269270

‎main.py‎

Lines changed: 69 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,21 @@ def _rev_resolves(rev: str) -> bool:
299299
return result.returncode == 0
300300

301301

302+
def _fetch_commit(sha: str) -> bool:
303+
"""Fetch one commit, without its history, into a clone that lacks it."""
304+
try:
305+
result = subprocess.run(
306+
["git", "fetch", "--no-tags", "--quiet", "--depth=1", "origin", sha],
307+
stdout=subprocess.PIPE,
308+
stderr=subprocess.PIPE,
309+
encoding="utf-8",
310+
check=False,
311+
)
312+
except OSError:
313+
return False
314+
return result.returncode == 0 and _rev_resolves(sha)
315+
316+
302317
def pr_head_rev() -> str | None:
303318
"""The commit whose recorded author the PR's author checks read.
304319
@@ -308,12 +323,13 @@ def pr_head_rev() -> str | None:
308323
that, ``HEAD^2`` on a ``pull_request`` checkout, where HEAD is the
309324
merge ref and its second parent is that same tip. Never ``HEAD^2`` on
310325
``pull_request_target``: there HEAD is the base branch, so ``HEAD^2``
311-
is nothing, or the parent of some unrelated merge on it.
326+
is nothing, or the parent of some unrelated merge on it. A shallow clone
327+
that lacks the tip gets it fetched, without its history.
312328
313-
``None`` when the clone is too shallow to hold either.
329+
``None`` when neither can be had.
314330
"""
315331
sha = get_pr_head_sha()
316-
if sha and _rev_resolves(sha):
332+
if sha and (_rev_resolves(sha) or _fetch_commit(sha)):
317333
return sha
318334
if os.getenv("GITHUB_EVENT_NAME") == "pull_request" and _rev_resolves(PR_HEAD_REV):
319335
return PR_HEAD_REV
@@ -460,6 +476,37 @@ def get_messages_from_head_ref(base_ref: str) -> list[Commit]:
460476
return _messages_in_range(f"origin/{base_ref}..HEAD")
461477

462478

479+
#: The most commits GitHub's pull request commits endpoint returns.
480+
API_COMMIT_LIMIT = 250
481+
482+
483+
def get_messages_from_api() -> list[Commit]:
484+
"""The pull request's commits from the REST API, for a clone without them.
485+
486+
The default ``actions/checkout`` clone holds only GitHub's merge commit.
487+
Deepening it cannot list a pull request that merged its base branch in;
488+
the API lists any pull request exactly. It stops at 250 commits, so a
489+
longer one is left unlisted rather than checked in part.
490+
"""
491+
pr = get_pr_event()
492+
number, total = pr.get("number"), pr.get("commits", 0)
493+
token, repo = os.getenv("GITHUB_TOKEN"), os.getenv("GITHUB_REPOSITORY")
494+
if not (number and token and repo) or total > API_COMMIT_LIMIT:
495+
return []
496+
from github import Auth, Github # type: ignore
497+
498+
api_url = os.getenv("GITHUB_API_URL", "https://api.github.com")
499+
pull = (
500+
Github(auth=Auth.Token(token), base_url=api_url).get_repo(repo).get_pull(number)
501+
)
502+
commits = [(c.sha, c.commit.message) for c in pull.get_commits()]
503+
# A force push after the event can leave the same count but other commits.
504+
head = get_pr_head_sha()
505+
if len(commits) != total or (head and commits[-1][0] != head):
506+
return []
507+
return commits
508+
509+
463510
def get_pr_commit_messages() -> list[Commit]:
464511
"""Get all commits, as ``(sha, message)``, for the current PR workflow.
465512
@@ -468,22 +515,32 @@ def get_pr_commit_messages() -> list[Commit]:
468515
checkout HEAD is the synthetic merge commit, so ``HEAD^1..HEAD^2`` is
469516
the same range. If the workflow checks out the PR head SHA instead,
470517
diff against ``origin/<base-ref>`` when that ref is available locally.
518+
A clone that holds none of these, or only part of the pull request (a
519+
shallow boundary ends a range early), asks the API.
471520
"""
472521
if not is_pr_event():
473522
return []
474523

475524
try:
476-
messages = get_messages_from_event_range()
477-
if messages:
525+
base_ref = os.getenv("GITHUB_BASE_REF", "")
526+
messages = (
527+
get_messages_from_event_range()
528+
or get_messages_from_merge_ref()
529+
or (get_messages_from_head_ref(base_ref) if base_ref else [])
530+
)
531+
total = get_pr_event().get("commits")
532+
if messages and (not total or len(messages) == total):
478533
return messages
479-
480-
messages = get_messages_from_merge_ref()
534+
from_api = get_messages_from_api()
535+
if from_api:
536+
return from_api
481537
if messages:
482-
return messages
483-
484-
base_ref = os.getenv("GITHUB_BASE_REF", "")
485-
if base_ref:
486-
return get_messages_from_head_ref(base_ref)
538+
warn_shallow_checkout(
539+
f"Only {len(messages)} of the pull request's {total} commits "
540+
"could be listed",
541+
"the others were not checked",
542+
)
543+
return messages
487544
except Exception as e:
488545
print(
489546
f"::warning::Failed to retrieve PR commit messages: {e}",

‎main_test.py‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,12 @@ def test_unknown_flag_is_skipped(self):
458458

459459

460460
class TestGetPrCommitMessages(unittest.TestCase):
461+
def setUp(self):
462+
# Never the event of whatever CI run the tests happen to be in.
463+
patcher = patch("main.get_pr_event", return_value={})
464+
patcher.start()
465+
self.addCleanup(patcher.stop)
466+
461467
def test_non_pr_event_returns_empty(self):
462468
with patch.dict(os.environ, {"GITHUB_EVENT_NAME": "push"}):
463469
result = main.get_pr_commit_messages()
@@ -502,6 +508,32 @@ def test_pull_request_target_is_supported(self):
502508
result = main.get_pr_commit_messages()
503509
self.assertEqual(result, ["fix: first"])
504510

511+
def test_a_range_cut_short_by_a_shallow_clone_asks_the_api(self):
512+
with (
513+
patch.dict(os.environ, {"GITHUB_EVENT_NAME": "pull_request"}),
514+
patch("main.get_messages_from_event_range", return_value=[("b2", "fix")]),
515+
patch("main.get_pr_event", return_value={"commits": 2}),
516+
patch("main.get_messages_from_api", return_value=[]) as api,
517+
patch("main.warn_shallow_checkout") as warn,
518+
):
519+
# The API cannot help either: check what the clone had, and say so.
520+
self.assertEqual(main.get_pr_commit_messages(), [("b2", "fix")])
521+
api.assert_called_once()
522+
self.assertIn("Only 1 of the pull request's 2 commits", warn.call_args[0][0])
523+
524+
def test_asks_the_api_when_the_clone_holds_none_of_them(self):
525+
with (
526+
patch.dict(
527+
os.environ,
528+
{"GITHUB_EVENT_NAME": "pull_request", "GITHUB_BASE_REF": "main"},
529+
),
530+
patch("main.get_messages_from_event_range", return_value=[]),
531+
patch("main.get_messages_from_merge_ref", return_value=[]),
532+
patch("main.get_messages_from_head_ref", return_value=[]),
533+
patch("main.get_messages_from_api", return_value=[("a1", "feat: one")]),
534+
):
535+
self.assertEqual(main.get_pr_commit_messages(), [("a1", "feat: one")])
536+
505537
def test_falls_back_to_base_ref_when_merge_ref_is_unavailable(self):
506538
with (
507539
patch.dict(
@@ -1027,6 +1059,81 @@ def test_missing_git_returns_none(self):
10271059
self.assertIsNone(main.pr_head_rev())
10281060

10291061

1062+
class TestFetchPrHead(unittest.TestCase):
1063+
def test_a_head_the_clone_lacks_is_fetched_without_history(self):
1064+
# rev-parse fails, the fetch succeeds, rev-parse now resolves.
1065+
run = MagicMock(
1066+
side_effect=[
1067+
MagicMock(returncode=1),
1068+
MagicMock(returncode=0),
1069+
MagicMock(returncode=0),
1070+
]
1071+
)
1072+
with (
1073+
patch("main.get_pr_head_sha", return_value="abc123"),
1074+
patch("main.subprocess.run", run),
1075+
):
1076+
self.assertEqual(main.pr_head_rev(), "abc123")
1077+
self.assertEqual(
1078+
run.call_args_list[1][0][0],
1079+
["git", "fetch", "--no-tags", "--quiet", "--depth=1", "origin", "abc123"],
1080+
)
1081+
1082+
def test_a_fetch_without_git_gives_up(self):
1083+
with patch("main.subprocess.run", side_effect=OSError("no git")):
1084+
self.assertFalse(main._fetch_commit("abc123"))
1085+
1086+
1087+
class TestGetMessagesFromApi(unittest.TestCase):
1088+
"""A clone without the pull request's commits lists them through the API."""
1089+
1090+
def _run(self, total, api_commits, head="b2"):
1091+
pull = MagicMock()
1092+
pull.get_commits.return_value = [
1093+
MagicMock(sha=sha, commit=MagicMock(message=msg))
1094+
for sha, msg in api_commits
1095+
]
1096+
github_module = MagicMock()
1097+
github_module.Github.return_value.get_repo.return_value.get_pull.return_value = (
1098+
pull
1099+
)
1100+
env = {
1101+
"GITHUB_TOKEN": "token",
1102+
"GITHUB_REPOSITORY": "owner/repo",
1103+
"GITHUB_API_URL": "https://ghe.example.com/api/v3",
1104+
}
1105+
with (
1106+
patch.dict(os.environ, env),
1107+
patch("main.get_pr_event", return_value={"number": 7, "commits": total}),
1108+
patch("main.get_pr_head_sha", return_value=head),
1109+
patch.dict(sys.modules, {"github": github_module}),
1110+
):
1111+
return main.get_messages_from_api(), github_module
1112+
1113+
def test_lists_the_commits_oldest_first(self):
1114+
commits = [("a1", "feat: one"), ("b2", "fix: two")]
1115+
result, github_module = self._run(2, commits)
1116+
self.assertEqual(result, commits)
1117+
self.assertEqual(
1118+
github_module.Github.call_args[1]["base_url"],
1119+
"https://ghe.example.com/api/v3",
1120+
)
1121+
1122+
def test_a_list_that_ends_elsewhere_is_not_used(self):
1123+
# A force push after the event: same count, other commits.
1124+
result, _ = self._run(2, [("a1", "feat: one"), ("c3", "fix: other")])
1125+
self.assertEqual(result, [])
1126+
1127+
def test_a_partial_list_is_not_used(self):
1128+
result, _ = self._run(3, [("a1", "feat: one"), ("b2", "fix: two")])
1129+
self.assertEqual(result, [])
1130+
1131+
def test_more_than_the_api_returns_is_left_unlisted(self):
1132+
result, github_module = self._run(main.API_COMMIT_LIMIT + 1, [])
1133+
self.assertEqual(result, [])
1134+
github_module.Github.assert_not_called()
1135+
1136+
10301137
class TestCheckoutHint(unittest.TestCase):
10311138
def test_pull_request_names_fetch_depth(self):
10321139
with patch.dict(os.environ, {"GITHUB_EVENT_NAME": "pull_request"}):

0 commit comments

Comments
 (0)