Skip to content
Draft
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
1 change: 0 additions & 1 deletion src/sentry/integrations/github/__init__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from sentry.rules import rules

from .actions.create_ticket import GitHubCreateTicketAction
from .handlers import GithubActionHandler # noqa: F401,F403
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action handler registration moved to unreliable import location

High Severity

The GithubActionHandler side-effect import was moved from sentry/integrations/github/__init__.py to sentry_plugins/github/client.py. This new location is only loaded on-demand, preventing the handler from reliably registering during startup. Consequently, GitHub workflow engine actions, like ticket creation, will not function.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 2f4ffa0. Configure here.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This statement is concerning, but importing .integrations.github.handlers in __init__.py is incompatible with importing the listeners statically in .../scm/stream.py. This all boils down to sentry/integrations/jira/client.py calling absolute_url at module level, which requires initialize_app to have been called.

I checked the original stacktrace when action_handler_registry.register was called for GithubActionHandler and made sure that it's still registered "during startup".

It might be worth defining "during startup": I'm talking about the startup of the backend web server. If GithubActionHandler should be registered to the action_handler_registry for other processes, their startup sequence must indeed be modified in a similar way.

The compromise I chose can be revisited by a human with a more effective brain than mine.


rules.add(GitHubCreateTicketAction)
17 changes: 4 additions & 13 deletions src/sentry/integrations/github/webhook.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,6 @@
from sentry.scm.private.stream_producer import produce_event_to_scm_stream
from sentry.seer.autofix.webhooks import handle_github_pr_webhook_for_autofix
from sentry.seer.code_review.contributor_seats import track_contributor_seat
from sentry.seer.code_review.webhooks.handlers import (
handle_webhook_event as code_review_handle_webhook_event,
)
from sentry.shared_integrations.exceptions import ApiError
from sentry.silo.base import SiloMode
from sentry.users.services.user.service import user_service
Expand Down Expand Up @@ -847,10 +844,7 @@ 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,
code_review_handle_webhook_event,
)
WEBHOOK_EVENT_PROCESSORS = (_handle_pr_webhook_for_autofix_processor,)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Seer code review functionality will silently fail upon deployment because the new async processing path is gated by a feature flag that is disabled by default.
Severity: CRITICAL

Suggested Fix

To prevent a silent outage, either remove the feature flag gating produce_event_to_scm_stream to enable the new path by default, or ensure the deployment process strictly requires enabling the sentry.scm.stream.rollout option beforehand. A safer approach would be to make the new path the default and remove the flag dependency.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/sentry/integrations/github/webhook.py#L847

Potential issue: The synchronous webhook processor `code_review_handle_webhook_event` is
removed and replaced with an asynchronous path gated by the `sentry.scm.stream.rollout`
feature flag. This flag is disabled by default. If the PR is deployed without explicitly
enabling this flag, all Seer code review functionality (PR analysis, check run
reactions, etc.) will silently stop working. The webhooks will continue to return
successful HTTP 204 responses, masking the fact that no processing is occurring.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This statement is true, and was anticipated in bold font in the PR description.


def _handle(
self,
Expand Down Expand Up @@ -976,10 +970,7 @@ class CheckRunEventWebhook(GitHubWebhook):
"""

EVENT_TYPE = IntegrationWebhookEventType.CI_CHECK
WEBHOOK_EVENT_PROCESSORS = (
code_review_handle_webhook_event,
handle_preprod_check_run_event,
)
WEBHOOK_EVENT_PROCESSORS = (handle_preprod_check_run_event,)


class IssueCommentEventWebhook(GitHubWebhook):
Expand All @@ -989,7 +980,7 @@ class IssueCommentEventWebhook(GitHubWebhook):
"""

EVENT_TYPE = IntegrationWebhookEventType.ISSUE_COMMENT
WEBHOOK_EVENT_PROCESSORS = (code_review_handle_webhook_event,)
WEBHOOK_EVENT_PROCESSORS = ()


@all_silo_endpoint
Expand Down Expand Up @@ -1150,7 +1141,7 @@ def handle(self, request: HttpRequest) -> HttpResponse:
{
"event_type_hint": request.headers.get(GITHUB_WEBHOOK_TYPE_HEADER_KEY),
"event": request.body.decode("utf-8"),
"extra": {},
"extra": {"github_delivery_id": github_delivery_id},
"received_at": int(time.time()),
"sentry_meta": None,
"type": IntegrationProviderSlug.GITHUB.value,
Expand Down
21 changes: 1 addition & 20 deletions src/sentry/scm/stream.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
# Example:
#
# from sentry.my_module import check_run_listener, pull_request_listener
import sentry.seer.code_review.webhooks.scm_listeners # noqa: F401 - registers listeners on import
from sentry.scm.private.event_stream import scm_event_stream
from sentry.scm.types import (
CheckRunEvent,
Expand All @@ -14,26 +15,6 @@
SubscriptionEvent,
)

# DEFAULT LISTENERS
#
# TODO: Remove after production testing.


@scm_event_stream.listen_for(event_type="check_run")
def listen_for_check_run(e):
return None


@scm_event_stream.listen_for(event_type="comment")
def listen_for_comment(e):
return None


@scm_event_stream.listen_for(event_type="pull_request")
def listen_for_pull_request(e):
return None


# Do not re-export your listener here.
__all__ = [
"scm_event_stream",
Expand Down
7 changes: 0 additions & 7 deletions src/sentry/seer/code_review/webhooks/__init__.py
Original file line number Diff line number Diff line change
@@ -1,7 +0,0 @@
"""
This module provides a single entry point for the webhook handler to handle webhook events.
"""

from .handlers import handle_webhook_event as code_review_webhook_processor

__all__ = ["code_review_webhook_processor"]
134 changes: 0 additions & 134 deletions src/sentry/seer/code_review/webhooks/handlers.py

This file was deleted.

Loading
Loading