fix(github-checks): re-list repositories when the bindings change - #4601
Conversation
Completing an installation claim left the settings page rendering "No
repositories available. Connect a GitHub account above first." even though
the bind had succeeded and the backend was already answering with every
repository the App could reach. Only a reload fixed it.
The repository listing comes from an ACTION — a one-shot read that nothing
re-runs on its own — while the installations that decide its answer arrive on
a LIVE QUERY. The effect that fetched it depended on the organization,
availability and two memoized callbacks, and a bind changes none of those.
It now also depends on the bindings, through a stable key rather than the
array: a Convex subscription hands back a fresh array on every delivery, so
depending on the array would ask GitHub again on every poll. The key is each
binding's opaque installation reference and its status, sorted — exactly what
changes which repositories the App can reach, and nothing else.
Three things the surface guaranteed still hold:
- a slow answer for the previous organization cannot land on the new one.
The in-flight guard is now a generation rather than a per-run flag,
because adopting the bindings' first answer must leave a request in flight
alone while superseding one must guarantee it never lands;
- the picker still resets on an organization switch, and deliberately does
not reset on a binding change — the org has not changed, and a repository
missing from the refreshed listing cannot be submitted anyway;
- the bindings query answering for the FIRST time is not a change: it is not
subscribed until availability says `enabled`, so it always answers after
the first listing was requested.
The suite page's connect section had the same staleness reached from the other
side — no bind starts there, but another admin, a second tab or a webhook
suspension left it just as stale — and follows the same key now.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TvWWcFe2mWKLa3AV8u1X4e
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
GitHub Actions
Error
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_c4afa7f8-8ca5-4325-a3a1-032b0f846471) |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (6)
WalkthroughThe change adds a stable key for GitHub installation bindings. The settings route and suite GitHub Checks section use this key to refresh repository listings after binding or status changes. They preserve selections during binding-only refreshes, reset selections on organization changes, avoid duplicate initial requests, and ignore stale results. Tests cover these behaviors. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
The bug
A GitHub App installation claim completed in Settings → Integrations → GitHub. The bind succeeded — the backend immediately returned every connectable repository for that organization — and the page went on rendering:
A manual reload fixed it. Looking at that list is the first thing anyone does after connecting an account, so the surface was telling people their bind had failed at the exact moment it had worked.
Why
The repository listing comes from an action —
listInstallationRepos, a one-shot read that nothing re-runs on its own. What decides its answer, the set of installations bound to the organization, comes from a live query. The effect that fetched the listing depended on:Completing a bind changes none of those. Same organization, availability already
enabled, both callbacks memoized — so the effect never ran again and the component kept its pre-bind (empty) listing for the life of the page.The fix
The listing now also depends on the bindings, through a stable key rather than the array. A Convex subscription hands back a fresh array on every delivery — including one re-sending byte-identical rows — so depending on the array itself would ask GitHub again on every poll.
The key (
installationBindingsKey, inclient/src/lib/github-repo-picker.ts, shared by both surfaces) is each binding's opaqueinstallationRefpaired with itsstatus, sorted:installationRef— an installation appearing or disappearing changes what can be reached;status—suspended,removedandunboundeach stop an installation answering for its repositories, so a status change counts even when the set does not move;accountLogin,boundAtandstatusChangedAtare excluded: none of them changes what the App can reach.undefined(the query has not answered) maps tonull, never to the empty-set key. "Not told" is not "there are none".What was deliberately preserved
cancelledflag, because the two rules are genuinely different: a re-run that only adopts the bindings' first answer must leave the request already in flight alone, while a re-run that supersedes it — different org, real change in the bindings, availability going away, unmount — must guarantee its answer can never arrive.activeOrganizationIdstays in the deps, for the reason the old comment gave. The body reads it now as well, to tell an org switch apart from a bindings change; the comment says so rather than being left false.undefinedrenders nothing, only an explicitdisabledredirects.Two judgement calls
Resetting the picker on a binding change: no. The reset moved into its own effect keyed on
activeOrganizationId. Switching organizations must clear the selection — the connect sends the current org id, so a leftover pick would be submitted against an org the repository does not belong to. A binding changing is not that: the organization has not changed, and throwing away a half-made choice because a colleague connected an account is a punishment for someone else's action. It is safe as well as kinder —handleConnectre-resolves the picked value against the refreshed listing and refuses with a toast when it no longer resolves, so a repository that vanished cannot be submitted.The bindings query's FIRST answer is not a change. That query is not subscribed until availability says
enabled, so it always answers after the first listing was requested — treating it as news would ask GitHub twice on every cold page load and blank the picker in between, for installations that request was already made against. The page records what the listing on screen (or in flight) was fetched for and adopts that first answer as its baseline without refetching and without invalidating the request in flight.The suite page
suite-github-checks-section.tsxhad the same staleness, reached from the other side, and is fixed the same way. No bind starts there — that flow lives in Settings and navigates away — so the exact reported sequence is not reachable on it. But a binding still changes under an open suite: another admin connects an account, the same person does it in a second tab, or a GitHub webhook suspends or removes one. Its picker would go on offering, or go on failing to offer, whatever it read when the page opened.Verification
expected "spy" to be called 2 times, but got 1 times— a binding appearing, a binding's status changing, and the org-switch test at its "the bind re-lists" step. The suite-section test fails the same way. The two negative tests (identical re-poll, first answer) pass without the fix, as they must: they guard against the fix over-firing, not against the bug.client/src/components/settings/__tests__/github-checks-route.test.tsx— 62 passing (5 new).client/src/components/settings/__tests__/github-install-callback-route.test.tsx— 19 passing.client/src/components/evals/__tests__/suite-github-checks-section.test.tsx— 17 passing (2 new).tsc --noEmit -p client/tsconfig.typecheck.json— clean.Error copy for this surface stays in
client/src/lib/github-checks-errors.ts; nothing new was inlined.server/routes/v1/eval-checks.tsand the hook'sSUITES_QUERYcall are untouched.🤖 Generated with Claude Code
https://claude.ai/code/session_01TvWWcFe2mWKLa3AV8u1X4e
Note
Medium Risk
Changes async listing fetch and race handling on GitHub integration UI paths; mistakes could show wrong repos or drop user selections, but no auth or data-store changes.
Overview
Fixes the repository picker staying empty after a successful GitHub account connect until reload. Settings → Integrations → GitHub and the suite connect section now re-run
listInstallationReposwhen installation bindings actually change, not only on org or availability changes.Adds shared
installationBindingsKeyingithub-repo-picker.ts: a sortedinstallationRef:statusstring so Convex subscription array churn does not spam GitHub. Listing fetch logic uses a generation guard so stale org/bindings responses cannot land after superseding events, while the bindings query’s first answer after cold load does not trigger a duplicate fetch.Org switches still clear picker selections; binding-only updates refresh the list but keep in-progress picks (connect re-validates against the new listing). New tests cover re-list on connect/status change, no re-list on identical re-polls or first bindings answer, and org-switch + stale in-flight behavior.
Reviewed by Cursor Bugbot for commit e7c57cd. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes the GitHub checks repository picker going stale after connecting a GitHub account. The settings page and the suite section now re-list available repositories when bindings change, instead of showing "No repositories available" until a manual reload.
installationRefandstatus(sorted); depending on the array itself would refetch on every Convex delivery, since each one is a fresh array.Written for commit e7c57cd. Summary will update on new commits.