fix(copilot): renew GitHub auth instead of asking for repeated sign-ins - #102
Merged
Merged
Conversation
Copilot auth has two credentials: the stored GitHub OAuth token and the short-lived Copilot IDE token exchanged from it. Several bugs turned a routine token expiry into a fresh sign-in: - the device flow dropped refresh_token/expires_in, so an expiring GitHub login had no way to renew and could only be fixed by reconnecting - concurrent turns each ran their own exchange; a rotating refresh token could be spent twice - a late 401 for an old IDE token wiped the newer token another request had just installed, and the cache could outlive a disconnect or an account switch - expires_at was trusted against the local clock, so a skewed clock kept sending expired tokens - a 403 (no subscription) and a locked keychain both told the user to reconnect, which fixes neither Store the full credential (access + refresh + expiries), renew it before the exchange and on a 401 from it, share one renewal per credential, key the cache to the credential so disconnects/switches invalidate it, honour refresh_in and the server Date header, and keep OAuth error bodies out of user-facing messages. Legacy bare-token credentials still work untouched. Covered by test/copilot.ts in the app smoke run: persistence across restart, rotation, concurrency, clock skew, disconnect/switch races, delayed 401s, and failure classification, all against mocked HTTP.
Keep main's account-assigned API routing and tenant model policies alongside silent OAuth renewal. Give each login a stable id across token rotations so successful refreshes do not empty the model picker, and record discovery's rejected authorization so late 401s cannot discard a newer IDE token. Add regression coverage for discovery-time OAuth rotation, concurrent inference renewal, tenant endpoints, and delayed discovery 401s. Adapt main's auth fixtures and bump package/lockfile to 0.0.97. Verified build, 731 app smoke checks plus Copilot auth regressions, 1094 shared checks, store/i18n tests, catalog sync, and changed-file formatting. Co-authored-by: Roxy <299891354+roxy-commits@users.noreply.github.com>
Collaborator
Author
|
Conflicts resolved in a572d71. Both tenant-specific model discovery/routing from main and silent token renewal are preserved. Added regression coverage for OAuth rotation during discovery and delayed discovery 401s. All seven CI checks now pass, including builds and Electron tests on Windows, macOS, and Linux. Version: 0.0.97. |
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.
Goal
Routine Copilot token expiry should renew silently, not send users through GitHub sign-in again.
Compared With Main
Main's #94 adds account-specific model discovery, tenant-assigned API routing, and fail-closed model policies. It does not persist or renew expiring GitHub OAuth credentials. This PR keeps that behavior and adds silent refresh for both credential layers.
Merged main at 50c19c1 and resolved all three conflicts. Version is now 0.0.97 in package.json and package-lock.json, following main's 0.0.96.
Changes
Verification
Auth tests use mocked HTTP and a throwaway database, not a real GitHub account. Coverage includes persistence/restart, legacy tokens, rotation, 20-way concurrency, clock skew, exchange-401 recovery, disconnect/account-switch races, malformed/secret-bearing errors, tenant endpoints, rotation during discovery, concurrent inference renewal, and delayed discovery 401s. Main's tenant-policy regression tests also pass.
Limits
Normal expiry no longer requires another login when a usable refresh token is available. Revoked authorization, expired refresh credentials, or an already-expired legacy token whose refresh data was never saved can still require a reconnect. This is not a promise that sign-in can never be required. The reporter's original account-specific error was unavailable, so that exact production failure was not independently reproduced.