diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index 734d1953..7c5cc18e 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -508,8 +508,8 @@ async def process(self) -> Any: self.last_committer = getattr(self.last_commit.committer, "login", self.parent_committer) # Clone repository for local file processing (OWNERS, changed files) - # For check_run events, cloning happens later only when needed - if self.github_event != "check_run": + # For check_run and status events, cloning happens later only when needed + if self.github_event not in ("check_run", "status"): await self._clone_repository(pull_request=pull_request) if self.github_event == "issue_comment": @@ -605,6 +605,37 @@ async def process(self) -> Any: await self._update_context_metrics() return None + elif self.github_event == "status": + # Skip pending state — only terminal states (success, failure, error) trigger re-evaluation + state = self.hook_data["state"] + if state == "pending": + token_metrics = await self._get_token_metrics() + self.logger.info( + f"{self.log_prefix} " + f"Webhook processing completed successfully: status (state=pending, skipped) - " + f"{token_metrics}", + ) + await self._update_context_metrics() + return None + + context_name = self.hook_data["context"] + self.logger.info( + f"{self.log_prefix} Status check '{context_name}' reached terminal state ({state}), " + f"re-evaluating can-be-merged" + ) + + owners_file_handler = OwnersFileHandler(github_webhook=self) + await PullRequestHandler( + github_webhook=self, owners_file_handler=owners_file_handler + ).check_if_can_be_merged(pull_request=pull_request) + + token_metrics = await self._get_token_metrics() + self.logger.info( + f"{self.log_prefix} Webhook processing completed successfully: status - {token_metrics}", + ) + await self._update_context_metrics() + return None + else: # Log warning when no PR found self.logger.warning( @@ -856,6 +887,39 @@ async def get_pull_request(self, number: int | None = None) -> PullRequest | Non else: self.logger.debug(f"{self.log_prefix} No PR data in webhook payload") + def _get_pr_head_sha(pr: PullRequest) -> str: + return pr.head.sha + + if self.github_event == "check_run": + head_sha = self.hook_data["check_run"]["head_sha"] + self.logger.debug(f"{self.log_prefix} Searching open PRs for check_run head SHA: {head_sha}") + open_pulls = await asyncio.to_thread(lambda: list(self.repository.get_pulls(state="open"))) + head_shas = await asyncio.gather(*(asyncio.to_thread(_get_pr_head_sha, pr) for pr in open_pulls)) + for _pull_request, pr_head_sha in zip(open_pulls, head_shas, strict=False): + if pr_head_sha == head_sha: + self.logger.debug( + f"{self.log_prefix} Found pull request {_pull_request.title} " + f"[{_pull_request.number}] for check run " + f"{self.hook_data['check_run']['name']}" + ) + return _pull_request + self.logger.debug(f"{self.log_prefix} No open PR found matching check_run head SHA") + + if self.github_event == "status": + sha = self.hook_data["sha"] + self.logger.debug(f"{self.log_prefix} Searching open PRs for status SHA: {sha}") + open_pulls = await asyncio.to_thread(lambda: list(self.repository.get_pulls(state="open"))) + head_shas = await asyncio.gather(*(asyncio.to_thread(_get_pr_head_sha, pr) for pr in open_pulls)) + for _pull_request, pr_head_sha in zip(open_pulls, head_shas, strict=False): + if pr_head_sha == sha: + self.logger.debug( + f"{self.log_prefix} Found pull request {_pull_request.title} " + f"[{_pull_request.number}] for status context " + f"{self.hook_data['context']}" + ) + return _pull_request + self.logger.debug(f"{self.log_prefix} No open PR found matching status SHA") + commit: dict[str, Any] = self.hook_data.get("commit", {}) if commit: 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 else: self.logger.debug(f"{self.log_prefix} No commit data in webhook payload") - if self.github_event == "check_run": - head_sha = self.hook_data["check_run"]["head_sha"] - self.logger.debug(f"{self.log_prefix} Searching open PRs for check_run head SHA: {head_sha}") - for _pull_request in await asyncio.to_thread(self.repository.get_pulls, state="open"): - if _pull_request.head.sha == head_sha: - self.logger.debug( - f"{self.log_prefix} Found pull request {_pull_request.title} " - f"[{_pull_request.number}] for check run " - f"{self.hook_data['check_run']['name']}" - ) - return _pull_request - self.logger.debug(f"{self.log_prefix} No open PR found matching check_run head SHA") - self.logger.debug(f"{self.log_prefix} All PR lookup strategies exhausted, no PR found") return None diff --git a/webhook_server/tests/test_github_api.py b/webhook_server/tests/test_github_api.py index 3f8655a2..ccfe97f8 100644 --- a/webhook_server/tests/test_github_api.py +++ b/webhook_server/tests/test_github_api.py @@ -816,6 +816,127 @@ async def test_process_check_run_event(self) -> None: mock_check_handler.return_value.process_pull_request_check_run_webhook_data.assert_awaited_once() mock_pr_handler.return_value.check_if_can_be_merged.assert_awaited_once() + @pytest.mark.asyncio + @pytest.mark.parametrize( + ("state", "should_recheck"), + [ + ("success", True), + ("pending", False), + ("failure", True), + ("error", True), + ], + ) + async def test_process_status_event(self, state: str, should_recheck: bool) -> None: + """Test processing status event with various states.""" + logger = Mock() + status_data = { + "state": state, + "context": "pre-commit.ci", + "sha": "abc123", + "repository": {"name": "test-repo", "full_name": "org/test-repo"}, + } + headers = Headers({"X-GitHub-Event": "status", "X-GitHub-Delivery": "abc"}) + + with tempfile.TemporaryDirectory() as temp_dir: + with patch("webhook_server.libs.github_api.Config") as mock_config: + mock_config.return_value.repository = True + mock_config.return_value.repository_local_data.return_value = {} + mock_config.return_value.data_dir = temp_dir + + with patch("webhook_server.libs.github_api.get_api_with_highest_rate_limit") as mock_get_api: + mock_get_api.return_value = (Mock(), "token", "apiuser") + + mock_repo = Mock() + mock_repo.get_git_tree.return_value.tree = [] + mock_pr = Mock() + mock_pr.head.sha = "abc123" + mock_pr.title = "Test PR" + mock_pr.number = 42 + mock_pr.draft = False + mock_pr.user.login = "testuser" + mock_pr.base.ref = "main" + mock_pr.get_commits.return_value = [Mock()] + mock_pr.get_files.return_value = [] + mock_repo.get_pulls.return_value = [mock_pr] + mock_repo.get_pull.return_value = mock_pr + with patch("webhook_server.libs.github_api.get_github_repo_api") as mock_get_repo_api: + mock_get_repo_api.return_value = mock_repo + + with patch("webhook_server.libs.github_api.get_repository_github_app_api") as mock_get_app_api: + mock_get_app_api.return_value = Mock() + + with patch( + "webhook_server.libs.github_api.get_apis_and_tokes_from_config" + ) as mock_get_apis: + mock_api1 = Mock() + mock_api1.rate_limiting = [0, 5000] + mock_api1.get_user.return_value.login = "user1" + mock_get_apis.return_value = [(mock_api1, "token1")] + + with patch("webhook_server.libs.github_api.PullRequestHandler") as mock_pr_handler: + mock_pr_handler.return_value.check_if_can_be_merged = AsyncMock(return_value=None) + + webhook = GithubWebhook(status_data, headers, logger) + await webhook.process() + + if should_recheck: + mock_pr_handler.return_value.check_if_can_be_merged.assert_awaited_once() + else: + mock_pr_handler.return_value.check_if_can_be_merged.assert_not_awaited() + + @pytest.mark.asyncio + async def test_get_pull_request_from_status_sha(self) -> None: + """Test getting pull request from status event SHA. + + Verifies that even when commit.get_pulls() returns a stale PR, + the status SHA lookup finds the correct matching PR. + """ + logger = Mock() + status_data = { + "state": "success", + "context": "pre-commit.ci", + "sha": "status-sha-123", + "commit": {"sha": "status-sha-123"}, # Real payloads include this + "repository": {"name": "test-repo", "full_name": "org/test-repo"}, + } + headers = Headers({"X-GitHub-Event": "status", "X-GitHub-Delivery": "abc"}) + + with patch("webhook_server.libs.github_api.Config") as mock_config: + mock_config.return_value.repository = True + mock_config.return_value.repository_local_data.return_value = {} + + with patch("webhook_server.libs.github_api.get_api_with_highest_rate_limit") as mock_get_api: + mock_get_api.return_value = (Mock(), "token", "apiuser") + + with patch("webhook_server.libs.github_api.get_github_repo_api") as mock_get_repo_api: + mock_repo = Mock() + mock_pr = Mock() + mock_pr.head.sha = "status-sha-123" + mock_pr.title = "Test PR" + mock_pr.number = 42 + mock_repo.get_pulls.return_value = [mock_pr] + + # Stale PR from commit.get_pulls() fallback + stale_pr = Mock() + stale_pr.head.sha = "old-sha" + mock_commit = Mock() + mock_commit.get_pulls.return_value = [stale_pr] + mock_repo.get_commit.return_value = mock_commit + + mock_get_repo_api.return_value = mock_repo + + with patch("webhook_server.libs.github_api.get_repository_github_app_api") as mock_get_app_api: + mock_get_app_api.return_value = Mock() + + with patch("webhook_server.utils.helpers.get_repository_color_for_log_prefix") as mock_color: + mock_color.return_value = "test-repo" + + gh = GithubWebhook(status_data, headers, logger) + result = await gh.get_pull_request() + assert result == mock_pr + mock_repo.get_pulls.assert_called_once_with(state="open") + mock_repo.get_commit.assert_not_called() + @pytest.mark.asyncio async def test_get_pull_request_by_number( self, minimal_hook_data: dict, minimal_headers: Headers, logger: Mock