From 663c20333cef24f513118d06af3f35d1f95ba569 Mon Sep 17 00:00:00 2001 From: codesensei-tushar Date: Tue, 14 Apr 2026 11:06:56 +0530 Subject: [PATCH 1/2] fix: re-fetch PR details in enricher to avoid stale requested_reviewers --- CHANGELOG.md | 9 +++++++++ src/event_processors/pull_request/enricher.py | 10 ++++++++++ 2 files changed, 19 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec83207..9013f23 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,15 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **Stale PR data in CODEOWNERS checks** -- `PullRequestEnricher` now + re-fetches PR details via `GET /repos/:owner/:repo/pulls/:num` before + building `event_data`, replacing the webhook payload's `requested_reviewers` + (and other point-in-time fields) with the current state. Fixes a race where + a `synchronize` webhook processed just before a `review_requested` webhook + would see a stale `requested_reviewers` list and incorrectly flag + `PathHasCodeOwnerCondition` / `RequireCodeOwnerReviewersCondition` + violations. Falls back to the webhook payload if the refresh fails. + - **`FilePatternCondition._get_changed_files` implementation** -- Replaced stub that always returned `[]` with a working implementation that extracts file paths from enriched PR data (`changed_files` list of dicts or plain diff --git a/src/event_processors/pull_request/enricher.py b/src/event_processors/pull_request/enricher.py index dce92fc..36c16d9 100644 --- a/src/event_processors/pull_request/enricher.py +++ b/src/event_processors/pull_request/enricher.py @@ -55,6 +55,16 @@ async def enrich_event_data(self, task: Any, github_token: str) -> dict[str, Any repo_full_name = getattr(task, "repo_full_name", "") installation_id = getattr(task, "installation_id", 0) + # the current state, not the stale webhook snapshot (webhooks for + # synchronize + review_requested can race). + if pr_number and repo_full_name: + try: + fresh_pr = await self.github_client.get_pull_request(repo_full_name, pr_number, installation_id) + if fresh_pr: + pr_data = fresh_pr + except Exception as e: + logger.warning(f"Could not refresh PR #{pr_number} details: {e}") + # Base event data event_data = { "pull_request_details": pr_data, From 3522656e2c31d0399d943ba6e074c157ae470936 Mon Sep 17 00:00:00 2001 From: codesensei-tushar Date: Tue, 14 Apr 2026 11:14:57 +0530 Subject: [PATCH 2/2] test: add enricher tests for PR details refresh --- .../pull_request/test_enricher.py | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/unit/event_processors/pull_request/test_enricher.py b/tests/unit/event_processors/pull_request/test_enricher.py index 3e0b340..3bb6d99 100644 --- a/tests/unit/event_processors/pull_request/test_enricher.py +++ b/tests/unit/event_processors/pull_request/test_enricher.py @@ -45,6 +45,7 @@ async def test_fetch_api_data_success(enricher, mock_github_client): @pytest.mark.asyncio async def test_enrich_event_data(enricher, mock_task, mock_github_client): + mock_github_client.get_pull_request.return_value = {"number": 1, "user": {"login": "author"}} mock_github_client.get_pull_request_reviews.return_value = [] mock_github_client.get_pull_request_files.return_value = [ {"filename": "test.py", "status": "added", "additions": 10, "deletions": 0, "patch": "+print('hello')"} @@ -61,6 +62,53 @@ async def test_enrich_event_data(enricher, mock_task, mock_github_client): assert "diff_summary" in event_data +@pytest.mark.asyncio +async def test_enrich_event_data_refreshes_pr_details(enricher, mock_task, mock_github_client): + """Stale webhook requested_reviewers is replaced by fresh PR details from the API. + + Simulates the synchronize+review_requested race: the webhook payload's + requested_reviewers is empty, but a fresh GET /pulls/:num shows alice was + requested. The enricher must surface the fresh state so CODEOWNERS rules + don't false-positive. + """ + mock_task.payload["pull_request"] = { + "number": 1, + "user": {"login": "author"}, + "requested_reviewers": [], + "requested_teams": [], + } + mock_github_client.get_pull_request.return_value = { + "number": 1, + "user": {"login": "author"}, + "requested_reviewers": [{"login": "alice"}], + "requested_teams": [], + } + mock_github_client.get_pull_request_reviews.return_value = [] + mock_github_client.get_pull_request_files.return_value = [] + + event_data = await enricher.enrich_event_data(mock_task, "fake_token") + + assert event_data["pull_request_details"]["requested_reviewers"] == [{"login": "alice"}] + mock_github_client.get_pull_request.assert_called_once_with("owner/repo", 1, 12345) + + +@pytest.mark.asyncio +async def test_enrich_event_data_falls_back_to_webhook_pr_when_refresh_fails(enricher, mock_task, mock_github_client): + """If the refresh API call fails or returns None, the webhook payload PR data is kept.""" + mock_task.payload["pull_request"] = { + "number": 1, + "user": {"login": "author"}, + "requested_reviewers": [{"login": "bob"}], + } + mock_github_client.get_pull_request.return_value = None + mock_github_client.get_pull_request_reviews.return_value = [] + mock_github_client.get_pull_request_files.return_value = [] + + event_data = await enricher.enrich_event_data(mock_task, "fake_token") + + assert event_data["pull_request_details"]["requested_reviewers"] == [{"login": "bob"}] + + @pytest.mark.asyncio async def test_fetch_acknowledgments(enricher, mock_github_client): mock_github_client.get_issue_comments.return_value = [