diff --git a/src/sentry/integrations/github/webhook.py b/src/sentry/integrations/github/webhook.py index 17bfa6b74ed8c5..41e8cb0b5fefa5 100644 --- a/src/sentry/integrations/github/webhook.py +++ b/src/sentry/integrations/github/webhook.py @@ -801,7 +801,10 @@ class PullRequestEventWebhook(GitHubWebhook): """https://developer.github.com/v3/activity/events/types/#pullrequestevent""" EVENT_TYPE = IntegrationWebhookEventType.MERGE_REQUEST - WEBHOOK_EVENT_PROCESSORS = (_handle_pr_webhook_for_autofix_processor,) + WEBHOOK_EVENT_PROCESSORS = ( + _handle_pr_webhook_for_autofix_processor, + code_review_handle_webhook_event, + ) def _handle( self, @@ -896,8 +899,6 @@ def _handle( "github.webhook.pull_request.created", sample_rate=1.0, tags={ - "organization_id": organization.id, - "repository_id": repo.id, "is_private": pr_repo_private, }, ) @@ -929,10 +930,6 @@ def _handle( metrics.incr( "github.webhook.organization_contributor.should_create", sample_rate=1.0, - tags={ - "organization_id": organization.id, - "repository_id": repo.id, - }, ) locked_contributor = None diff --git a/src/sentry/seer/code_review/utils.py b/src/sentry/seer/code_review/utils.py index 76b332e8450de3..e34bc7be59a6ad 100644 --- a/src/sentry/seer/code_review/utils.py +++ b/src/sentry/seer/code_review/utils.py @@ -1,6 +1,6 @@ from collections.abc import Mapping from enum import StrEnum -from typing import Any, Literal +from typing import Any import orjson from django.conf import settings @@ -16,6 +16,11 @@ from sentry.seer.signed_seer_api import make_signed_seer_api_request +class RequestType(StrEnum): + PR_REVIEW = "pr-review" + PR_CLOSED = "pr-closed" + + class ClientError(Exception): "Non-retryable client error from Seer" @@ -210,7 +215,7 @@ def transform_webhook_to_codegen_request( """ # Determine request_type based on event_type # For now, we only support pr-review for these webhook types - request_type: Literal["pr-review", "pr-closed"] = "pr-review" + request_type: RequestType = RequestType.PR_REVIEW # Extract pull request number # Different event types have PR info in different locations @@ -244,9 +249,10 @@ def transform_webhook_to_codegen_request( trigger_metadata = _get_trigger_metadata(github_event, event_payload) + # XXX: How can we share classes between Sentry and Seer? # Build CodecovTaskRequest return { - "request_type": request_type, + "request_type": request_type.value, "external_owner_id": repo.external_id, "data": { "repo": repo_definition, diff --git a/src/sentry/seer/code_review/webhooks/handlers.py b/src/sentry/seer/code_review/webhooks/handlers.py index 4a9aace2ecf659..215ae47871682f 100644 --- a/src/sentry/seer/code_review/webhooks/handlers.py +++ b/src/sentry/seer/code_review/webhooks/handlers.py @@ -16,6 +16,7 @@ from .check_run import handle_check_run_event from .issue_comment import handle_issue_comment_event +from .pull_request import handle_pull_request_event logger = logging.getLogger(__name__) @@ -25,6 +26,7 @@ EVENT_TYPE_TO_HANDLER: dict[GithubWebhookType, WebhookProcessor] = { GithubWebhookType.CHECK_RUN: handle_check_run_event, GithubWebhookType.ISSUE_COMMENT: handle_issue_comment_event, + GithubWebhookType.PULL_REQUEST: handle_pull_request_event, } diff --git a/src/sentry/seer/code_review/webhooks/pull_request.py b/src/sentry/seer/code_review/webhooks/pull_request.py new file mode 100644 index 00000000000000..8a21f79a2b7d7b --- /dev/null +++ b/src/sentry/seer/code_review/webhooks/pull_request.py @@ -0,0 +1,155 @@ +""" +Handler for GitHub pull_request webhook events. +https://docs.github.com/en/webhooks/webhook-events-and-payloads#pull_request +""" + +from __future__ import annotations + +import enum +import logging +from collections.abc import Mapping +from typing import Any + +from sentry import options +from sentry.integrations.github.webhook_types import GithubWebhookType +from sentry.integrations.services.integration import RpcIntegration +from sentry.models.organization import Organization +from sentry.models.repository import Repository +from sentry.models.repositorysettings import CodeReviewTrigger +from sentry.utils import metrics + +from ..utils import _get_target_commit_sha + +logger = logging.getLogger(__name__) + + +class ErrorStatus(enum.StrEnum): + MISSING_PULL_REQUEST = "missing_pull_request" + MISSING_ACTION = "missing_action" + UNSUPPORTED_ACTION = "unsupported_action" + DRAFT_PR = "draft_pr" + + +class Log(enum.StrEnum): + MISSING_PULL_REQUEST = "github.webhook.pull_request.missing-pull-request" + MISSING_ACTION = "github.webhook.pull_request.missing-action" + UNSUPPORTED_ACTION = "github.webhook.pull_request.unsupported-action" + DRAFT_PR = "github.webhook.pull_request.draft-pr" + + +class Metrics(enum.StrEnum): + ERROR = "seer.code_review.webhook.pull_request.error" + OUTCOME = "seer.code_review.webhook.pull_request.outcome" + + +class PullRequestAction(enum.StrEnum): + """ + GitHub pull_request webhook actions. + https://docs.github.com/en/webhooks/webhook-events-and-payloads#pull_request + """ + + ASSIGNED = "assigned" + AUTO_MERGE_DISABLED = "auto_merge_disabled" + AUTO_MERGE_ENABLED = "auto_merge_enabled" + CLOSED = "closed" + CONVERTED_TO_DRAFT = "converted_to_draft" + DEMILESTONED = "demilestoned" + DEQUEUED = "dequeued" + EDITED = "edited" + ENQUEUED = "enqueued" + LABELED = "labeled" + LOCKED = "locked" + MILESTONED = "milestoned" + OPENED = "opened" + READY_FOR_REVIEW = "ready_for_review" + REOPENED = "reopened" + REVIEW_REQUEST_REMOVED = "review_request_removed" + REVIEW_REQUESTED = "review_requested" + # New commits are pushed to the PR + SYNCHRONIZE = "synchronize" + UNASSIGNED = "unassigned" + UNLABELED = "unlabeled" + UNLOCKED = "unlocked" + + +WHITELISTED_ACTIONS = { + PullRequestAction.OPENED, + PullRequestAction.READY_FOR_REVIEW, + PullRequestAction.SYNCHRONIZE, +} + + +def _get_trigger_for_action(action: PullRequestAction) -> CodeReviewTrigger: + match action: + case PullRequestAction.OPENED | PullRequestAction.READY_FOR_REVIEW: + return CodeReviewTrigger.ON_READY_FOR_REVIEW + case PullRequestAction.SYNCHRONIZE: + return CodeReviewTrigger.ON_NEW_COMMIT + case _: + raise ValueError(f"Unsupported pull request action: {action}") + + +def _warn_and_increment_metric( + error_status: ErrorStatus, + extra: Mapping[str, Any], + action: PullRequestAction | str | None = None, +) -> None: + """ + Warn and increment metric for a given error status and action. + """ + logger.warning(Log[error_status.name].value, extra=extra) + tags = {"error_status": error_status.value} + if action: + tags["action"] = action + metrics.incr(Metrics.ERROR.value, tags=tags) + + +def handle_pull_request_event( + *, + github_event: GithubWebhookType, + event: Mapping[str, Any], + organization: Organization, + repo: Repository, + integration: RpcIntegration | None = None, + **kwargs: Any, +) -> None: + """ + Handle pull_request webhook events for code review. + + This handler processes PR events and sends them directly to Seer + """ + extra = {"organization_id": organization.id, "repo": repo.name} + pull_request = event.get("pull_request") + if not pull_request: + _warn_and_increment_metric(ErrorStatus.MISSING_PULL_REQUEST, extra=extra) + return + + action_value = event.get("action") + if not action_value or not isinstance(action_value, str): + _warn_and_increment_metric(ErrorStatus.MISSING_ACTION, extra=extra) + return + + try: + action = PullRequestAction(action_value) + except ValueError: + _warn_and_increment_metric(ErrorStatus.UNSUPPORTED_ACTION, action=action_value, extra=extra) + return + + if action not in WHITELISTED_ACTIONS: + return + + if pull_request.get("draft") is True: + _warn_and_increment_metric(ErrorStatus.DRAFT_PR, action=action_value, extra=extra) + return + + if not options.get("github.webhook.pr"): + from .task import schedule_task + + schedule_task( + github_event=github_event, + event=event, + organization=organization, + repo=repo, + target_commit_sha=_get_target_commit_sha(github_event, event, repo, integration), + trigger=_get_trigger_for_action(action), + ) diff --git a/src/sentry/testutils/helpers/github.py b/src/sentry/testutils/helpers/github.py index f5b8427739bd96..8f9943ae89f3a0 100644 --- a/src/sentry/testutils/helpers/github.py +++ b/src/sentry/testutils/helpers/github.py @@ -138,19 +138,21 @@ def send_github_webhook_event( class GitHubWebhookCodeReviewTestCase(GitHubWebhookTestCase): CODE_REVIEW_FEATURES = {"organizations:gen-ai-features", "organizations:code-review-beta"} + OPTIONS_TO_SET: dict[str, bool] = {} @contextmanager def code_review_setup( self, features: Collection[str] | Mapping[str, Any] | None = None, - forward_to_overwatch: bool = False, + options: dict[str, bool] | None = None, ) -> Generator[None]: """Helper to set up code review test context.""" self.organization.update_option("sentry:enable_pr_review_test_generation", True) features_to_enable = self.CODE_REVIEW_FEATURES if features is None else features + options_to_set = dict(self.OPTIONS_TO_SET) | (options or {}) with ( self.feature(features_to_enable), - self.options({"github.webhook.issue-comment": forward_to_overwatch}), + self.options(options_to_set), ): yield diff --git a/tests/sentry/seer/code_review/webhooks/test_issue_comment.py b/tests/sentry/seer/code_review/webhooks/test_issue_comment.py index f1c4f4db57c79d..5a9606f5f92f17 100644 --- a/tests/sentry/seer/code_review/webhooks/test_issue_comment.py +++ b/tests/sentry/seer/code_review/webhooks/test_issue_comment.py @@ -14,6 +14,8 @@ class IssueCommentEventWebhookTest(GitHubWebhookCodeReviewTestCase): """Integration tests for GitHub issue_comment webhook events.""" + OPTIONS_TO_SET = {"github.webhook.issue-comment": False} + @pytest.fixture(autouse=True) def mock_github_api_calls(self) -> Generator[None]: """ @@ -143,7 +145,7 @@ def test_skips_reaction_when_no_comment_id(self, mock_reaction: MagicMock) -> No @patch("sentry.seer.code_review.webhooks.issue_comment._add_eyes_reaction_to_comment") def test_skips_processing_when_option_is_true(self, mock_reaction: MagicMock) -> None: """Test that when github.webhook.issue-comment option is True (kill switch), no processing occurs.""" - with self.code_review_setup(forward_to_overwatch=True), self.tasks(): + with self.code_review_setup(options={"github.webhook.issue-comment": True}), self.tasks(): event = self._build_issue_comment_event(f"Please {SENTRY_REVIEW_COMMAND} this PR") response = self._send_issue_comment_event(event) assert response.status_code == 204 diff --git a/tests/sentry/seer/code_review/webhooks/test_pull_request.py b/tests/sentry/seer/code_review/webhooks/test_pull_request.py new file mode 100644 index 00000000000000..b6886b912b3f10 --- /dev/null +++ b/tests/sentry/seer/code_review/webhooks/test_pull_request.py @@ -0,0 +1,214 @@ +from collections.abc import Generator +from unittest.mock import MagicMock, patch + +import orjson +import pytest + +from fixtures.github import PULL_REQUEST_OPENED_EVENT_EXAMPLE +from sentry.integrations.github.webhook_types import GithubWebhookType +from sentry.seer.code_review.utils import RequestType +from sentry.testutils.helpers.github import GitHubWebhookCodeReviewTestCase + + +class PullRequestEventWebhookTest(GitHubWebhookCodeReviewTestCase): + """Integration tests for GitHub pull_request webhook events.""" + + OPTIONS_TO_SET = {"github.webhook.pr": False} + + @pytest.fixture(autouse=True) + def mock_github_api_calls(self) -> Generator[None]: + """ + Prevents real HTTP requests to GitHub API across all tests. + Uses autouse fixture to apply mocking automatically without @patch decorators on each test. + """ + mock_client_instance = MagicMock() + mock_client_instance.get_pull_request.return_value = {"head": {"sha": "abc123"}} + + with patch( + "sentry.seer.code_review.utils.GitHubApiClient", return_value=mock_client_instance + ) as mock_api_client: + self.mock_api_client = mock_api_client + yield + + @pytest.fixture(autouse=True) + def mock_seer_request(self) -> Generator[None]: + """ + Prevents real HTTP requests to Seer API across all tests. + Uses autouse fixture to apply mocking automatically without @patch decorators on each test. + """ + with patch("sentry.seer.code_review.webhooks.task.make_seer_request") as mock_seer: + self.mock_seer = mock_seer + yield + + def test_pull_request_opened(self) -> None: + """Test that opened action triggers Seer request.""" + with self.code_review_setup(), self.tasks(): + self._send_webhook_event( + GithubWebhookType.PULL_REQUEST, + PULL_REQUEST_OPENED_EVENT_EXAMPLE, + ) + event = orjson.loads(PULL_REQUEST_OPENED_EVENT_EXAMPLE) + assert event["action"] == "opened" + + self.mock_seer.assert_called_once() + call_kwargs = self.mock_seer.call_args[1] + assert call_kwargs["path"] == "/v1/automation/overwatch-request" + payload = call_kwargs["payload"] + assert payload["request_type"] == RequestType.PR_REVIEW.value + + def test_pull_request_skips_draft(self) -> None: + """Test that draft PRs are skipped.""" + with self.code_review_setup(), self.tasks(): + event_with_draft = orjson.loads(PULL_REQUEST_OPENED_EVENT_EXAMPLE) + event_with_draft["pull_request"]["draft"] = True + + response = self._send_webhook_event( + GithubWebhookType.PULL_REQUEST, + orjson.dumps(event_with_draft), + ) + + assert response.status_code == 204 + self.mock_seer.assert_not_called() + + def test_pull_request_skips_unsupported_action(self) -> None: + """Test that unsupported actions are skipped.""" + with self.code_review_setup(), self.tasks(): + event_with_unsupported_action = orjson.loads(PULL_REQUEST_OPENED_EVENT_EXAMPLE) + event_with_unsupported_action["action"] = "assigned" + + self._send_webhook_event( + GithubWebhookType.PULL_REQUEST, + orjson.dumps(event_with_unsupported_action), + ) + + self.mock_seer.assert_not_called() + + def test_pull_request_skips_when_option_enabled(self) -> None: + """Test that PR events are skipped when github.webhook.pr option is True (kill switch).""" + with self.code_review_setup(options={"github.webhook.pr": True}), self.tasks(): + self._send_webhook_event( + GithubWebhookType.PULL_REQUEST, + PULL_REQUEST_OPENED_EVENT_EXAMPLE, + ) + + self.mock_seer.assert_not_called() + + def test_pull_request_missing_action_field(self) -> None: + """Test that events without action field are skipped.""" + with self.code_review_setup(), self.tasks(): + event_without_action = orjson.loads(PULL_REQUEST_OPENED_EVENT_EXAMPLE) + del event_without_action["action"] + + self._send_webhook_event( + GithubWebhookType.PULL_REQUEST, + orjson.dumps(event_without_action), + ) + + self.mock_seer.assert_not_called() + + def test_pull_request_invalid_action_type(self) -> None: + """Test that events with non-string action are skipped.""" + with self.code_review_setup(), self.tasks(): + event_with_invalid_action = orjson.loads(PULL_REQUEST_OPENED_EVENT_EXAMPLE) + event_with_invalid_action["action"] = 123 + + self._send_webhook_event( + GithubWebhookType.PULL_REQUEST, + orjson.dumps(event_with_invalid_action), + ) + + self.mock_seer.assert_not_called() + + def test_pull_request_skips_when_code_review_disabled(self) -> None: + """Test that PR events are skipped when code review features are not enabled.""" + with self.tasks(): + event = orjson.loads(PULL_REQUEST_OPENED_EVENT_EXAMPLE) + + self._send_webhook_event( + GithubWebhookType.PULL_REQUEST, + orjson.dumps(event), + ) + + self.mock_seer.assert_not_called() + + def test_pull_request_ready_for_review_action(self) -> None: + """Test that ready_for_review action triggers Seer request.""" + with self.code_review_setup(), self.tasks(): + event = orjson.loads(PULL_REQUEST_OPENED_EVENT_EXAMPLE) + event["action"] = "ready_for_review" + + self._send_webhook_event( + GithubWebhookType.PULL_REQUEST, + orjson.dumps(event), + ) + + self.mock_seer.assert_called_once() + + def test_pull_request_reopened_action(self) -> None: + """Test that reopened action is skipped (not in whitelist).""" + with self.code_review_setup(), self.tasks(): + event = orjson.loads(PULL_REQUEST_OPENED_EVENT_EXAMPLE) + event["action"] = "reopened" + + self._send_webhook_event( + GithubWebhookType.PULL_REQUEST, + orjson.dumps(event), + ) + + self.mock_seer.assert_not_called() + + def test_pull_request_synchronize_action(self) -> None: + """Test that synchronize action triggers Seer request.""" + with self.code_review_setup(), self.tasks(): + event = orjson.loads(PULL_REQUEST_OPENED_EVENT_EXAMPLE) + event["action"] = "synchronize" + + self._send_webhook_event( + GithubWebhookType.PULL_REQUEST, + orjson.dumps(event), + ) + + self.mock_seer.assert_called_once() + + def test_pull_request_invalid_enum_action(self) -> None: + """Test that actions not in PullRequestAction enum are handled gracefully.""" + with self.code_review_setup(), self.tasks(): + event = orjson.loads(PULL_REQUEST_OPENED_EVENT_EXAMPLE) + event["action"] = "future_action_not_in_enum" + + self._send_webhook_event( + GithubWebhookType.PULL_REQUEST, + orjson.dumps(event), + ) + + self.mock_seer.assert_not_called() + + def test_pull_request_blocks_draft_for_ready_for_review_action(self) -> None: + """Test that draft PRs are blocked for ready_for_review action.""" + with self.code_review_setup(), self.tasks(): + event = orjson.loads(PULL_REQUEST_OPENED_EVENT_EXAMPLE) + event["action"] = "ready_for_review" + event["pull_request"]["draft"] = True + + response = self._send_webhook_event( + GithubWebhookType.PULL_REQUEST, + orjson.dumps(event), + ) + + assert response.status_code == 204 + self.mock_seer.assert_not_called() + + def test_pull_request_blocks_draft_for_synchronize_action(self) -> None: + """Test that draft PRs are blocked for synchronize action.""" + with self.code_review_setup(), self.tasks(): + event = orjson.loads(PULL_REQUEST_OPENED_EVENT_EXAMPLE) + event["action"] = "synchronize" + event["pull_request"]["draft"] = True + + response = self._send_webhook_event( + GithubWebhookType.PULL_REQUEST, + orjson.dumps(event), + ) + + assert response.status_code == 204 + self.mock_seer.assert_not_called()