Skip to content

feat(seer): Add failing check run URLs to autofix overview API - #122486

Merged
NicoHinderling merged 3 commits into
masterfrom
nico/feat/seer-overview-failing-check-run-urls
Aug 24, 2026
Merged

feat(seer): Add failing check run URLs to autofix overview API#122486
NicoHinderling merged 3 commits into
masterfrom
nico/feat/seer-overview-failing-check-run-urls

Conversation

@NicoHinderling

@NicoHinderling NicoHinderling commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Summary

The Autofix Overview page shows a "N Checks Failing" pill whose tooltip lists each failing PR check by name. Those names are dead text — there's no way to jump from a failing check to its CI run.

This backend change captures each failing check's run URL from GitHub and exposes it on the autofix-overview API so a follow-up frontend PR can link each check to its run.

What changed

  • GitHub GraphQL (pull_request_status.py): fetch detailsUrl on CheckRun nodes (the external run page) and targetUrl on legacy StatusContext nodes.
  • PullRequestStatusResult.failed_checks: now a tuple of a new FailedCheck(name, url) dataclass instead of bare strings. URL is None when the provider doesn't give one.
  • API payload: adds a new failedCheckDetails: [{name, url}] field on PullRequestPayload.

Deploy safety

Frontend and backend don't deploy atomically. Rather than change the shape of the existing failedChecks: string[] field (which would make the currently-deployed frontend try to render objects as text and crash the tooltip during the deploy window), this adds failedCheckDetails and leaves failedChecks untouched. The follow-up frontend PR reads the new field; a later cleanup PR can drop failedChecks once the frontend is out.

Tests

  • Extraction unit tests cover check-run detailsUrl, status-context targetUrl, and null URLs.
  • Serializer tests assert both failedChecks (names, unchanged) and the new failedCheckDetails.

All affected backend suites pass; prek + mypy clean.

Security

The check url is provider/CI-controlled (StatusContext.targetUrl is set by third-party CI apps), and Sentry's frontend link primitives don't sanitize the URL scheme. To prevent a javascript:/data: link from becoming a DOM-XSS sink once the frontend renders it as an anchor, FailedCheck enforces an http(s)-only invariant at construction — any other scheme (or a non-string) becomes None.

The autofix overview surfaces failing PR checks by name only, so the
frontend cannot link a failing check to its run. Capture each check's
run URL from GitHub (detailsUrl for check runs, targetUrl for legacy
statuses) and expose it on the API.

Add a new failedCheckDetails field ({name, url}) alongside the existing
failedChecks name list rather than changing failedChecks in place, so
the currently-deployed frontend keeps working across the deploy window.
A follow-up frontend PR will read failedCheckDetails to link each check.
@NicoHinderling
NicoHinderling requested review from a team as code owners August 24, 2026 17:10
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Aug 24, 2026

@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.

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ae2991b. Configure here.

Comment thread src/sentry/seer/endpoints/organization_seer_autofix_overview.py
A failing check's run URL comes verbatim from GitHub's detailsUrl /
targetUrl. StatusContext.targetUrl is set by third-party CI apps, so it is
attacker-influenceable and could be a javascript:/data: URI. Sentry's
frontend link primitives do not sanitize the scheme, so once the follow-up
frontend renders these as anchor hrefs an unsafe scheme becomes DOM XSS in
the sentry.io origin.

Make http(s)-only a construction invariant of FailedCheck: any other scheme
(or non-string) is coerced to None, protecting every consumer of the field.
Comment thread src/sentry/seer/endpoints/organization_seer_autofix_overview.py
NicoHinderling added a commit that referenced this pull request Aug 24, 2026
get_pull_request_statuses caches PullRequestStatusResult for 60s. This
branch changed failed_checks from tuple[str, ...] to tuple[FailedCheck, ...],
but the isinstance(cached, PullRequestStatusResult) guard still accepts an
entry pickled by the old code, whose failed_checks are bare strings. The
overview serializer would then call .name/.url on a str and 500 for up to the
cache TTL after deploy.

Bump the cache-key namespace to /v2 so the new code never reads old-shape
entries; they expire on their own.

Flagged by Cursor Bugbot and the Sentry PR bot on #122486.
NicoHinderling added a commit that referenced this pull request Aug 24, 2026
get_pull_request_statuses caches PullRequestStatusResult for 60s. This
branch changed failed_checks from tuple[str, ...] to tuple[FailedCheck, ...],
but the isinstance(cached, PullRequestStatusResult) guard still accepts an
entry pickled by the old code, whose failed_checks are bare strings. The
overview serializer would then call .name/.url on a str and 500 for up to the
cache TTL after deploy.

Bump the cache-key namespace to /v2 so the new code never reads old-shape
entries; they expire on their own.

Flagged by Cursor Bugbot and the Sentry PR bot on #122486.
@NicoHinderling
NicoHinderling force-pushed the nico/feat/seer-overview-failing-check-run-urls branch from f985830 to 643ff03 Compare August 24, 2026 17:53
get_pull_request_statuses caches PullRequestStatusResult for 60s. This
branch changed failed_checks from tuple[str, ...] to tuple[FailedCheck, ...],
but the isinstance(cached, PullRequestStatusResult) guard still accepts an
entry pickled by the old code, whose failed_checks are bare strings. The
overview serializer would then call .name/.url on a str and 500 for up to the
cache TTL after deploy.

Bump the cache-key namespace to /v2 so the new code never reads old-shape
entries; they expire on their own.

Flagged by Cursor Bugbot and the Sentry PR bot on #122486.
@NicoHinderling
NicoHinderling force-pushed the nico/feat/seer-overview-failing-check-run-urls branch from 643ff03 to c3a6043 Compare August 24, 2026 17:59
@NicoHinderling
NicoHinderling merged commit 219af01 into master Aug 24, 2026
70 checks passed
@NicoHinderling
NicoHinderling deleted the nico/feat/seer-overview-failing-check-run-urls branch August 24, 2026 19:20
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