ref(scm): Centralize PR -> Autofix run resolution - #121904
Draft
joseph-sentry wants to merge 7 commits into
Draft
Conversation
PR iteration has never been exercised against a GitHub Enterprise instance, and there is no longer one to test against. GHE sends the same issue_comment and pull_request_review events as github.com, so a GHE repo reaches the trigger tasks and — being GitHub-shaped — passes every other check as if it were github.com. Name the supported provider explicitly and turn GHE away at both entry points, rather than leaving it supported by accident.
Both entry points already turn GitHub Enterprise away before a task is ever scheduled: the SCM event stream review listener drops it on the subscription provider, and the issue_comment mention processor drops it on ``Integration.provider``. The provider checks inside the tasks were a second line of defense on a path nothing reaches, and they read as if the task were an entry point in its own right. Restore the plain ``repo.provider is None`` guards, which exist only to narrow the type for the Seer lookups below.
The re-enabling note pointed at ``sentry.integrations.source_code_management.pr_id_cache`` as the thing to audit first, but that module does not exist on master — it arrives in a separate, later change, so on this branch the reference resolves to nothing. State the hazard directly instead: repo external ids are unique per GitHub instance rather than globally, so anything keyed on one alone collides across a GHE install and github.com. That holds whether or not any particular cache exists yet.
The issue_comment event behind an `@sentry` mention carries only a repo-scoped PR number, while Seer's run lookup is keyed on the provider's numeric PR id, so every mention spent a REST round-trip recovering it. The mapping is permanently immutable — PR numbers are never reused within a repo and a PR's id never changes — so it is cached with no invalidation path, warmed from the pull_request and check_suite webhooks that carry both halves.
trigger_pr_iteration_from_review still recovered the PR's numeric id with a get_pull_request REST call, the same round-trip the comment path just dropped — even though the pull_request and check_suite webhooks have almost certainly already warmed the PR-number -> id cache for the PR under review. Route it through get_or_fetch_pr_id, mirroring the comment path, so a submitted review reuses the cached id and only falls back to GitHub when no webhook has warmed the PR yet.
The trigger tasks re-read `repo.provider` off the reloaded repo purely to feed `get_or_fetch_pr_id` and `get_agent_state_from_pr_id`, which both treat github.com as the only supported provider. The provider is already pinned before either task runs: the mention and review entry points reject everything but `PR_ITERATION_PROVIDER_SLUG` before dispatching. Read `PR_ITERATION_PROVIDER` directly instead, dropping the `repo.provider is None` guards that existed only to narrow the type for those two calls, and point a comment at the entry-point gate that guarantees the provider.
The mention and review tasks each recovered a PR's global id with the same copy-pasted block, and check_suite called Seer directly once per candidate org per PR per suite. Move both steps into seer/autofix/pr_iteration/run_resolution.py: - resolve_pr_id: number -> id via pr_id_cache. The SCM client is built inside, so it's only constructed on a miss. ApiError still propagates so callers keep their existing handling. - get_run_state_for_pr_id: the Seer lookup, caching only the absence of a run for a day, keyed by (provider, pr_id, organization_id). Run state itself is never cached since it's live and downstream staleness checks compare head shas against it. The negatives are what repeat (pushes to non-Autofix PRs, webhooks fanned out to regions that don't own the session) and can't go stale, because Autofix creates the run before the PR. Non-404 errors aren't cached. Both metrics are tagged by calling listener so the paths can be split apart. Also gate the check_suite listener on github.com at its entry point, as the mention and review listeners already do. That's what lets callers here pass PR_ITERATION_PROVIDER directly instead of threading a runtime provider through.
github-actions
Bot
added
the
Scope: Backend
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The mention and review tasks each recovered a PR's global id with the
same copy-pasted block, and check_suite called Seer directly once per
candidate org per PR per suite. Move both steps into
seer/autofix/pr_iteration/run_resolution.py:
inside, so it's only constructed on a miss. ApiError still propagates
so callers keep their existing handling.
a run for a day, keyed by (provider, pr_id, organization_id).
Run state itself is never cached since it's live and downstream
staleness checks compare head shas against it. The negatives are what
repeat (pushes to non-Autofix PRs, webhooks fanned out to regions that
don't own the session) and can't go stale, because Autofix creates the
run before the PR. Non-404 errors aren't cached.
Both metrics are tagged by calling listener so the paths can be split
apart.
Also gate the check_suite listener on github.com at its entry point, as
the mention and review listeners already do. That's what lets callers
here pass PR_ITERATION_PROVIDER directly instead of threading a runtime
provider through.