Skip to content

Commit 60ae336

Browse files
authored
Merge pull request #74 from codesensei-tushar/fix/code-owner-reviewers-stale-check
fix: re-fetch PR details in enricher to avoid stale requested_reviewers
2 parents 5658ed3 + 3522656 commit 60ae336

3 files changed

Lines changed: 67 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,15 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
88

99
### Fixed
1010

11+
- **Stale PR data in CODEOWNERS checks** -- `PullRequestEnricher` now
12+
re-fetches PR details via `GET /repos/:owner/:repo/pulls/:num` before
13+
building `event_data`, replacing the webhook payload's `requested_reviewers`
14+
(and other point-in-time fields) with the current state. Fixes a race where
15+
a `synchronize` webhook processed just before a `review_requested` webhook
16+
would see a stale `requested_reviewers` list and incorrectly flag
17+
`PathHasCodeOwnerCondition` / `RequireCodeOwnerReviewersCondition`
18+
violations. Falls back to the webhook payload if the refresh fails.
19+
1120
- **`FilePatternCondition._get_changed_files` implementation** -- Replaced
1221
stub that always returned `[]` with a working implementation that extracts
1322
file paths from enriched PR data (`changed_files` list of dicts or plain

src/event_processors/pull_request/enricher.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,16 @@ async def enrich_event_data(self, task: Any, github_token: str) -> dict[str, Any
5555
repo_full_name = getattr(task, "repo_full_name", "")
5656
installation_id = getattr(task, "installation_id", 0)
5757

58+
# the current state, not the stale webhook snapshot (webhooks for
59+
# synchronize + review_requested can race).
60+
if pr_number and repo_full_name:
61+
try:
62+
fresh_pr = await self.github_client.get_pull_request(repo_full_name, pr_number, installation_id)
63+
if fresh_pr:
64+
pr_data = fresh_pr
65+
except Exception as e:
66+
logger.warning(f"Could not refresh PR #{pr_number} details: {e}")
67+
5868
# Base event data
5969
event_data = {
6070
"pull_request_details": pr_data,

tests/unit/event_processors/pull_request/test_enricher.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ async def test_fetch_api_data_success(enricher, mock_github_client):
4545

4646
@pytest.mark.asyncio
4747
async def test_enrich_event_data(enricher, mock_task, mock_github_client):
48+
mock_github_client.get_pull_request.return_value = {"number": 1, "user": {"login": "author"}}
4849
mock_github_client.get_pull_request_reviews.return_value = []
4950
mock_github_client.get_pull_request_files.return_value = [
5051
{"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):
6162
assert "diff_summary" in event_data
6263

6364

65+
@pytest.mark.asyncio
66+
async def test_enrich_event_data_refreshes_pr_details(enricher, mock_task, mock_github_client):
67+
"""Stale webhook requested_reviewers is replaced by fresh PR details from the API.
68+
69+
Simulates the synchronize+review_requested race: the webhook payload's
70+
requested_reviewers is empty, but a fresh GET /pulls/:num shows alice was
71+
requested. The enricher must surface the fresh state so CODEOWNERS rules
72+
don't false-positive.
73+
"""
74+
mock_task.payload["pull_request"] = {
75+
"number": 1,
76+
"user": {"login": "author"},
77+
"requested_reviewers": [],
78+
"requested_teams": [],
79+
}
80+
mock_github_client.get_pull_request.return_value = {
81+
"number": 1,
82+
"user": {"login": "author"},
83+
"requested_reviewers": [{"login": "alice"}],
84+
"requested_teams": [],
85+
}
86+
mock_github_client.get_pull_request_reviews.return_value = []
87+
mock_github_client.get_pull_request_files.return_value = []
88+
89+
event_data = await enricher.enrich_event_data(mock_task, "fake_token")
90+
91+
assert event_data["pull_request_details"]["requested_reviewers"] == [{"login": "alice"}]
92+
mock_github_client.get_pull_request.assert_called_once_with("owner/repo", 1, 12345)
93+
94+
95+
@pytest.mark.asyncio
96+
async def test_enrich_event_data_falls_back_to_webhook_pr_when_refresh_fails(enricher, mock_task, mock_github_client):
97+
"""If the refresh API call fails or returns None, the webhook payload PR data is kept."""
98+
mock_task.payload["pull_request"] = {
99+
"number": 1,
100+
"user": {"login": "author"},
101+
"requested_reviewers": [{"login": "bob"}],
102+
}
103+
mock_github_client.get_pull_request.return_value = None
104+
mock_github_client.get_pull_request_reviews.return_value = []
105+
mock_github_client.get_pull_request_files.return_value = []
106+
107+
event_data = await enricher.enrich_event_data(mock_task, "fake_token")
108+
109+
assert event_data["pull_request_details"]["requested_reviewers"] == [{"login": "bob"}]
110+
111+
64112
@pytest.mark.asyncio
65113
async def test_fetch_acknowledgments(enricher, mock_github_client):
66114
mock_github_client.get_issue_comments.return_value = [

0 commit comments

Comments
 (0)