Skip to content

Commit ffb4c8a

Browse files
authored
fix: handle status events to trigger can-be-merged re-evaluation (#1034) (#1036)
## Summary - Add `status` event handler in `github_api.py` so external services using GitHub's Status API (e.g., `pre-commit.ci`) trigger `can-be-merged` re-evaluation when their checks succeed - Add SHA-based PR lookup for `status` events in `get_pull_request()` - Skip processing for non-success states (pending, failure, error) ## Problem When `pre-commit.ci` completes, it reports via GitHub's Status API (not Check Runs API). The webhook server only handled `check_run` events, so `can-be-merged` was never re-evaluated after pre-commit.ci finished. This caused permanent "Some check runs not started: pre-commit.ci - pr" failures. ## Changes | File | Change | |---|---| | `webhook_server/libs/github_api.py` | Add `status` event handler in `process()`, skip clone for status events until needed, SHA-based PR lookup fallback | | `webhook_server/tests/test_github_api.py` | 4 tests: success triggers re-eval, pending/failure skipped, SHA PR lookup | ## Test plan - [x] All tests pass - [ ] Deploy and verify: push a commit to a PR with pre-commit.ci, confirm can-be-merged re-evaluates after pre-commit.ci succeeds Closes #1034 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Status events now associate with pull requests (by matching commit SHAs) and trigger can-be-merged re-evaluation for terminal states; pending statuses are skipped. * Check-run and status handling now defer repository cloning/fetching until necessary, aligning status flow with check-run behavior. * Improved logging and metrics around status processing and PR association. * **Tests** * Added tests covering status event handling (success, pending, failure, error), PR lookup, and conditional recheck logic. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 23d03c9 commit ffb4c8a

2 files changed

Lines changed: 187 additions & 15 deletions

File tree

webhook_server/libs/github_api.py

Lines changed: 66 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -508,8 +508,8 @@ async def process(self) -> Any:
508508
self.last_committer = getattr(self.last_commit.committer, "login", self.parent_committer)
509509

510510
# Clone repository for local file processing (OWNERS, changed files)
511-
# For check_run events, cloning happens later only when needed
512-
if self.github_event != "check_run":
511+
# For check_run and status events, cloning happens later only when needed
512+
if self.github_event not in ("check_run", "status"):
513513
await self._clone_repository(pull_request=pull_request)
514514

515515
if self.github_event == "issue_comment":
@@ -605,6 +605,37 @@ async def process(self) -> Any:
605605
await self._update_context_metrics()
606606
return None
607607

608+
elif self.github_event == "status":
609+
# Skip pending state — only terminal states (success, failure, error) trigger re-evaluation
610+
state = self.hook_data["state"]
611+
if state == "pending":
612+
token_metrics = await self._get_token_metrics()
613+
self.logger.info(
614+
f"{self.log_prefix} "
615+
f"Webhook processing completed successfully: status (state=pending, skipped) - "
616+
f"{token_metrics}",
617+
)
618+
await self._update_context_metrics()
619+
return None
620+
621+
context_name = self.hook_data["context"]
622+
self.logger.info(
623+
f"{self.log_prefix} Status check '{context_name}' reached terminal state ({state}), "
624+
f"re-evaluating can-be-merged"
625+
)
626+
627+
owners_file_handler = OwnersFileHandler(github_webhook=self)
628+
await PullRequestHandler(
629+
github_webhook=self, owners_file_handler=owners_file_handler
630+
).check_if_can_be_merged(pull_request=pull_request)
631+
632+
token_metrics = await self._get_token_metrics()
633+
self.logger.info(
634+
f"{self.log_prefix} Webhook processing completed successfully: status - {token_metrics}",
635+
)
636+
await self._update_context_metrics()
637+
return None
638+
608639
else:
609640
# Log warning when no PR found
610641
self.logger.warning(
@@ -856,6 +887,39 @@ async def get_pull_request(self, number: int | None = None) -> PullRequest | Non
856887
else:
857888
self.logger.debug(f"{self.log_prefix} No PR data in webhook payload")
858889

890+
def _get_pr_head_sha(pr: PullRequest) -> str:
891+
return pr.head.sha
892+
893+
if self.github_event == "check_run":
894+
head_sha = self.hook_data["check_run"]["head_sha"]
895+
self.logger.debug(f"{self.log_prefix} Searching open PRs for check_run head SHA: {head_sha}")
896+
open_pulls = await asyncio.to_thread(lambda: list(self.repository.get_pulls(state="open")))
897+
head_shas = await asyncio.gather(*(asyncio.to_thread(_get_pr_head_sha, pr) for pr in open_pulls))
898+
for _pull_request, pr_head_sha in zip(open_pulls, head_shas, strict=False):
899+
if pr_head_sha == head_sha:
900+
self.logger.debug(
901+
f"{self.log_prefix} Found pull request {_pull_request.title} "
902+
f"[{_pull_request.number}] for check run "
903+
f"{self.hook_data['check_run']['name']}"
904+
)
905+
return _pull_request
906+
self.logger.debug(f"{self.log_prefix} No open PR found matching check_run head SHA")
907+
908+
if self.github_event == "status":
909+
sha = self.hook_data["sha"]
910+
self.logger.debug(f"{self.log_prefix} Searching open PRs for status SHA: {sha}")
911+
open_pulls = await asyncio.to_thread(lambda: list(self.repository.get_pulls(state="open")))
912+
head_shas = await asyncio.gather(*(asyncio.to_thread(_get_pr_head_sha, pr) for pr in open_pulls))
913+
for _pull_request, pr_head_sha in zip(open_pulls, head_shas, strict=False):
914+
if pr_head_sha == sha:
915+
self.logger.debug(
916+
f"{self.log_prefix} Found pull request {_pull_request.title} "
917+
f"[{_pull_request.number}] for status context "
918+
f"{self.hook_data['context']}"
919+
)
920+
return _pull_request
921+
self.logger.debug(f"{self.log_prefix} No open PR found matching status SHA")
922+
859923
commit: dict[str, Any] = self.hook_data.get("commit", {})
860924
if commit:
861925
self.logger.debug(f"{self.log_prefix} Attempting to get PR from commit SHA: {commit.get('sha', 'unknown')}")
@@ -869,19 +933,6 @@ async def get_pull_request(self, number: int | None = None) -> PullRequest | Non
869933
else:
870934
self.logger.debug(f"{self.log_prefix} No commit data in webhook payload")
871935

872-
if self.github_event == "check_run":
873-
head_sha = self.hook_data["check_run"]["head_sha"]
874-
self.logger.debug(f"{self.log_prefix} Searching open PRs for check_run head SHA: {head_sha}")
875-
for _pull_request in await asyncio.to_thread(self.repository.get_pulls, state="open"):
876-
if _pull_request.head.sha == head_sha:
877-
self.logger.debug(
878-
f"{self.log_prefix} Found pull request {_pull_request.title} "
879-
f"[{_pull_request.number}] for check run "
880-
f"{self.hook_data['check_run']['name']}"
881-
)
882-
return _pull_request
883-
self.logger.debug(f"{self.log_prefix} No open PR found matching check_run head SHA")
884-
885936
self.logger.debug(f"{self.log_prefix} All PR lookup strategies exhausted, no PR found")
886937
return None
887938

webhook_server/tests/test_github_api.py

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -816,6 +816,127 @@ async def test_process_check_run_event(self) -> None:
816816
mock_check_handler.return_value.process_pull_request_check_run_webhook_data.assert_awaited_once()
817817
mock_pr_handler.return_value.check_if_can_be_merged.assert_awaited_once()
818818

819+
@pytest.mark.asyncio
820+
@pytest.mark.parametrize(
821+
("state", "should_recheck"),
822+
[
823+
("success", True),
824+
("pending", False),
825+
("failure", True),
826+
("error", True),
827+
],
828+
)
829+
async def test_process_status_event(self, state: str, should_recheck: bool) -> None:
830+
"""Test processing status event with various states."""
831+
logger = Mock()
832+
status_data = {
833+
"state": state,
834+
"context": "pre-commit.ci",
835+
"sha": "abc123",
836+
"repository": {"name": "test-repo", "full_name": "org/test-repo"},
837+
}
838+
headers = Headers({"X-GitHub-Event": "status", "X-GitHub-Delivery": "abc"})
839+
840+
with tempfile.TemporaryDirectory() as temp_dir:
841+
with patch("webhook_server.libs.github_api.Config") as mock_config:
842+
mock_config.return_value.repository = True
843+
mock_config.return_value.repository_local_data.return_value = {}
844+
mock_config.return_value.data_dir = temp_dir
845+
846+
with patch("webhook_server.libs.github_api.get_api_with_highest_rate_limit") as mock_get_api:
847+
mock_get_api.return_value = (Mock(), "token", "apiuser")
848+
849+
mock_repo = Mock()
850+
mock_repo.get_git_tree.return_value.tree = []
851+
mock_pr = Mock()
852+
mock_pr.head.sha = "abc123"
853+
mock_pr.title = "Test PR"
854+
mock_pr.number = 42
855+
mock_pr.draft = False
856+
mock_pr.user.login = "testuser"
857+
mock_pr.base.ref = "main"
858+
mock_pr.get_commits.return_value = [Mock()]
859+
mock_pr.get_files.return_value = []
860+
mock_repo.get_pulls.return_value = [mock_pr]
861+
mock_repo.get_pull.return_value = mock_pr
862+
with patch("webhook_server.libs.github_api.get_github_repo_api") as mock_get_repo_api:
863+
mock_get_repo_api.return_value = mock_repo
864+
865+
with patch("webhook_server.libs.github_api.get_repository_github_app_api") as mock_get_app_api:
866+
mock_get_app_api.return_value = Mock()
867+
868+
with patch(
869+
"webhook_server.libs.github_api.get_apis_and_tokes_from_config"
870+
) as mock_get_apis:
871+
mock_api1 = Mock()
872+
mock_api1.rate_limiting = [0, 5000]
873+
mock_api1.get_user.return_value.login = "user1"
874+
mock_get_apis.return_value = [(mock_api1, "token1")]
875+
876+
with patch("webhook_server.libs.github_api.PullRequestHandler") as mock_pr_handler:
877+
mock_pr_handler.return_value.check_if_can_be_merged = AsyncMock(return_value=None)
878+
879+
webhook = GithubWebhook(status_data, headers, logger)
880+
await webhook.process()
881+
882+
if should_recheck:
883+
mock_pr_handler.return_value.check_if_can_be_merged.assert_awaited_once()
884+
else:
885+
mock_pr_handler.return_value.check_if_can_be_merged.assert_not_awaited()
886+
887+
@pytest.mark.asyncio
888+
async def test_get_pull_request_from_status_sha(self) -> None:
889+
"""Test getting pull request from status event SHA.
890+
891+
Verifies that even when commit.get_pulls() returns a stale PR,
892+
the status SHA lookup finds the correct matching PR.
893+
"""
894+
logger = Mock()
895+
status_data = {
896+
"state": "success",
897+
"context": "pre-commit.ci",
898+
"sha": "status-sha-123",
899+
"commit": {"sha": "status-sha-123"}, # Real payloads include this
900+
"repository": {"name": "test-repo", "full_name": "org/test-repo"},
901+
}
902+
headers = Headers({"X-GitHub-Event": "status", "X-GitHub-Delivery": "abc"})
903+
904+
with patch("webhook_server.libs.github_api.Config") as mock_config:
905+
mock_config.return_value.repository = True
906+
mock_config.return_value.repository_local_data.return_value = {}
907+
908+
with patch("webhook_server.libs.github_api.get_api_with_highest_rate_limit") as mock_get_api:
909+
mock_get_api.return_value = (Mock(), "token", "apiuser")
910+
911+
with patch("webhook_server.libs.github_api.get_github_repo_api") as mock_get_repo_api:
912+
mock_repo = Mock()
913+
mock_pr = Mock()
914+
mock_pr.head.sha = "status-sha-123"
915+
mock_pr.title = "Test PR"
916+
mock_pr.number = 42
917+
mock_repo.get_pulls.return_value = [mock_pr]
918+
919+
# Stale PR from commit.get_pulls() fallback
920+
stale_pr = Mock()
921+
stale_pr.head.sha = "old-sha"
922+
mock_commit = Mock()
923+
mock_commit.get_pulls.return_value = [stale_pr]
924+
mock_repo.get_commit.return_value = mock_commit
925+
926+
mock_get_repo_api.return_value = mock_repo
927+
928+
with patch("webhook_server.libs.github_api.get_repository_github_app_api") as mock_get_app_api:
929+
mock_get_app_api.return_value = Mock()
930+
931+
with patch("webhook_server.utils.helpers.get_repository_color_for_log_prefix") as mock_color:
932+
mock_color.return_value = "test-repo"
933+
934+
gh = GithubWebhook(status_data, headers, logger)
935+
result = await gh.get_pull_request()
936+
assert result == mock_pr
937+
mock_repo.get_pulls.assert_called_once_with(state="open")
938+
mock_repo.get_commit.assert_not_called()
939+
819940
@pytest.mark.asyncio
820941
async def test_get_pull_request_by_number(
821942
self, minimal_hook_data: dict, minimal_headers: Headers, logger: Mock

0 commit comments

Comments
 (0)