fix(onboarding): Handle repo selection race with background link_all_repos#111716
Merged
fix(onboarding): Handle repo selection race with background link_all_repos#111716
Conversation
github-actions
bot
added
the
Scope: Frontend
| }, | ||
| }, | ||
| ]; | ||
| const [matches] = await queryClient.fetchQuery({ |
Member
There was a problem hiding this comment.
Is this different from our useApiQuery?
Member
Author
There was a problem hiding this comment.
Somewhat, useApiQuery uses fetchDataQuery under the hood (passed to useQuery as a queryFn), this is the imperative equivalent. We need the GET inside a callback, so the hook doesn't make sense here.
- user selects a repo
- check if it exists (imperative GET)
- conditionally POST
jaydgoss
force-pushed
the
jaygoss/fix-repo-selection-race-with-link-all-repos
branch
from
67c18fd to
41763b5
Compare
jaydgoss
force-pushed
the
jaygoss/vdy-24-scm-platform-features-step-ui-polish-and-analytics-pass
branch
from
491f83c to
c2bb8d3
Compare
jaydgoss
force-pushed
the
jaygoss/fix-repo-selection-race-with-link-all-repos
branch
from
41763b5 to
58a0241
Compare
evanpurkhiser
approved these changes
Mar 30, 2026
Base automatically changed from
jaygoss/vdy-24-scm-platform-features-step-ui-polish-and-analytics-pass
to
master
March 30, 2026 20:22
…repos After a GitHub integration is installed, link_all_repos runs async and may still be populating repos when the user reaches the repo selector. The previous approach cached existing repos on mount and looked them up before deciding whether to POST, but the cache was often stale or paginated, causing selection to fail silently. Now always POST first. If the repo already exists (background task or previous session created it), the POST fails and we fall back to a targeted GET filtered by repo name to find the existing record. This avoids both the stale cache and pagination issues.
Flip the order: do a targeted GET filtered by repo name first, and only POST if the repo doesn't exist yet. This avoids expected 400s showing up in devtools and Sentry when the background link_all_repos task has already created the repo.
Replace direct Client.requestPromise usage with fetchMutation which wraps the same call but is the recommended API.
fetchMutation only accepts PUT/POST/DELETE methods. Use the shared QUERY_API_CLIENT instance directly for the imperative GET call.
Replace deprecated QUERY_API_CLIENT.requestPromise with the sanctioned queryClient.fetchQuery + fetchDataQuery pattern for imperative GET requests outside of React Query hooks.
Use ApiQueryKey type with getApiUrl for proper URL typing, satisfying the queryKey constraint on queryClient.fetchQuery.
With link_all_repos running in the background after integration install, all repos are automatically registered in Sentry. The hide-on-deselect and cleanup-on-switch logic was designed for manual repo registration and is now counterproductive — it removes repo records that the background task created, causing them to disappear from settings. Selection is now just a context pointer.
Rewrite tests to match the GET-first-then-POST approach and removal of repo hide/cleanup logic. Remove tests for cleanup-on-switch and hide-on-remove since those behaviors no longer exist.
Match on r.name === repo.identifier instead of externalSlug because externalSlug varies by provider (GitLab uses a numeric project ID, VSTS uses external_id). Repository.name is consistently the full repo path across all providers.
…rage Remove unnecessary async from handleRemove test to fix require-await lint error. Rename misleading test name to accurately reflect that only POST fails (GET succeeds with empty results). Add dedicated test for GET failure scenario. Simplify busy state test by removing waitFor dance that couldn't observe the intermediate state.
jaydgoss
force-pushed
the
jaygoss/fix-repo-selection-race-with-link-all-repos
branch
from
58a0241 to
b289d7b
Compare
jaydgoss
deleted the
jaygoss/fix-repo-selection-race-with-link-all-repos
branch
dashed
pushed a commit
that referenced
this pull request
Apr 1, 2026
…repos (#111716) Fix repo selection in the SCM onboarding connect step to handle the race with the background `link_all_repos` task. After a GitHub integration is installed, `link_all_repos` runs async and registers all repos. The old approach cached existing repos on mount and checked that cache before deciding to POST. This broke because the cache was often stale (task still running) and the repos endpoint is paginated (specific repo might not be on page 1). New approach: - GET first with a targeted query filtered by repo name (sidesteps pagination) - POST only if the repo doesn't exist yet The old code also hid (soft-deleted) repo records when the user deselected or switched repos. This made sense when repos were manually registered one at a time, but with `link_all_repos` auto-registering everything, hiding repos on deselect was counterproductive -- it removed records the background task created, causing them to disappear from the integration settings page. Repo selection is now just a context pointer with no side effects on the repo records themselves. Also switches to `queryClient.fetchQuery` + `fetchDataQuery` for the imperative GET instead of deprecated `Client.requestPromise`. Replaces #111699 (closed due to CODEOWNERS auto-review noise from base change).
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
Fix repo selection in the SCM onboarding connect step to handle the race with the background
link_all_repostask.After a GitHub integration is installed,
link_all_reposruns async and registers all repos. The old approach cached existing repos on mount and checked that cache before deciding to POST. This broke because the cache was often stale (task still running) and the repos endpoint is paginated (specific repo might not be on page 1).New approach:
The old code also hid (soft-deleted) repo records when the user deselected or switched repos. This made sense when repos were manually registered one at a time, but with
link_all_reposauto-registering everything, hiding repos on deselect was counterproductive -- it removed records the background task created, causing them to disappear from the integration settings page. Repo selection is now just a context pointer with no side effects on the repo records themselves.Also switches to
queryClient.fetchQuery+fetchDataQueryfor the imperative GET instead of deprecatedClient.requestPromise.Replaces #111699 (closed due to CODEOWNERS auto-review noise from base change).