Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions src/event_processors/pull_request/enricher.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
48 changes: 48 additions & 0 deletions tests/unit/event_processors/pull_request/test_enricher.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')"}
Expand All @@ -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 = [
Expand Down
Loading