Skip to content

chore(seer-activity): Correct webhook url and web_url alert keys#118697

Open
leeandher wants to merge 2 commits into
masterfrom
leanderrodrigues/iswf-2955-add-web_url-and-url-to-match-the-issue-format-but-for
Open

chore(seer-activity): Correct webhook url and web_url alert keys#118697
leeandher wants to merge 2 commits into
masterfrom
leanderrodrigues/iswf-2955-add-web_url-and-url-to-match-the-issue-format-but-for

Conversation

@leeandher

Copy link
Copy Markdown
Member

Slight update to the webhook format I noticed while drafting getsentry/sentry-docs#18588. For issues, the url is by default for the API, and we specify user facing urls as web_url.

Added web_url and corrected that for the alert section of the webhook.

@leeandher leeandher requested a review from a team as a code owner June 29, 2026 21:55
@linear-code

linear-code Bot commented Jun 29, 2026

Copy link
Copy Markdown

ISWF-2955

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Jun 29, 2026
@leeandher leeandher force-pushed the leanderrodrigues/iswf-2955-add-web_url-and-url-to-match-the-issue-format-but-for branch from a7fc010 to 5ce235a Compare June 29, 2026 21:56

@cursor cursor Bot left a comment

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.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 5ce235a. Configure here.

url=organization.absolute_url(
url=organization.absolute_api_url(
f"organizations/{organization.slug}/workflows/{workflow.id}/"
),

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.

Duplicate org in API URL

High Severity

The alert url passed to absolute_api_url includes an organizations/{slug}/ prefix, but that helper already builds /api/0/organizations/{slug}/…. The webhook emits a duplicated segment and a non-existent API path, so consumers treating alert.url like issue url get broken links.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 5ce235a. Configure here.

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.

idk if this is valid or not, but we could add a test to prove/misprove this

Comment on lines +151 to +153
url=organization.absolute_api_url(
f"organizations/{organization.slug}/workflows/{workflow.id}/"
),

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: The call to organization.absolute_api_url redundantly includes the organization slug in the path, creating a malformed URL.
Severity: HIGH

Suggested Fix

Update the call to organization.absolute_api_url to only pass the path segment that follows the organization slug. Change the path argument from f"organizations/{organization.slug}/workflows/{workflow.id}/" to f"workflows/{workflow.id}/".

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/notifications/notification_action/activity_registry/sentry_app.py#L151-L153

Potential issue: The `organization.absolute_api_url` function is called with a path that
includes `organizations/{organization.slug}/`. However, the function itself is designed
to prepend `/api/0/organizations/{slug}/` to the provided path. This results in a
duplicated path segment, creating a malformed URL like
`/api/0/organizations/{slug}/organizations/{slug}/workflows/{workflow.id}//`. Any
webhook calls attempting to use this URL will fail with a 404 error, breaking the
notification functionality.

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

url=organization.absolute_url(
url=organization.absolute_api_url(
f"organizations/{organization.slug}/workflows/{workflow.id}/"
),

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.

idk if this is valid or not, but we could add a test to prove/misprove this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants