Add Copilot multi-account support#637
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67f244ab7e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4dadf7cb32
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@ratulsarna - do you intend to add multiple accounts for other providers beyond Codex in the near future. Your previous #613 seems to suggest you might. If you are, I'll close this PR and wait for the update. I'll use my forked version for multiple GitHub Copilot account. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 339d4add1a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Addressed the latest Codex feedback in 7b64703:\n\n- Copilot re-auth now fails closed if GitHub identity lookup fails while Copilot accounts already exist, avoiding anonymous duplicate accounts and stale credentials on the original account.\n- Copilot stacked account cards are only forced when all accounts fit within the menu snapshot fetch limit; larger account sets keep the switcher available so hidden accounts remain selectable.\n\nValidation run locally:\n- ./Scripts/compile_and_run.sh ✅\n- pnpm check ✅\n- swift test was rerun; it is currently hitting existing flaky/unrelated failures (UsageStoreCoverage provider detection / CostUsageJsonlPerformance benchmark), not changes in this patch. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b64703247
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| func tokenAccountErrorMessage(_ error: any Error) -> String? { | ||
| guard !(error is CancellationError) else { return nil } |
There was a problem hiding this comment.
Keep cancellation as an explicit per-account outcome
Returning nil for CancellationError removes both snapshot and error text for that account, which downstream menu rendering treats as “no override” and can fall back to the provider’s live snapshot for multiple cards. When account refreshes are canceled (for example during rapid refresh/switch sequences), this can display duplicated or misleading per-account cards instead of a canceled/error state. Preserve a distinct cancellation marker (or at least a non-empty error override) so canceled account rows do not render as normal data.
Useful? React with 👍 / 👎.
|
Looking forward to going through this PR once the codex review is clean! |
Register .copilot in TokenAccountSupportCatalog with environment-based injection (COPILOT_API_TOKEN), matching the existing env key used by ProviderConfigEnvironment and ProviderTokenResolver. Uses requiresManualCookieSource: false since Copilot authenticates via OAuth, not cookies. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add fetchGitHubUsername(token:) static method that calls GET /user with the OAuth token to retrieve the GitHub login. Used for auto-labeling token accounts during the multi-account add flow and migration. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace single-token storage (settings.copilotAPIToken) with multi-account storage via addTokenAccount(). After OAuth, fetches GitHub username via /user and plan name via copilot_internal/user to build a "username (Plan)" label. Detects duplicate accounts by username prefix and refreshes the token instead of creating duplicates. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace single-token secure field and login buttons with an "Add Account" button that triggers OAuth device flow. The generic token accounts section (account list, picker, remove, paste fallback) appears automatically now that Copilot is in the TokenAccountSupportCatalog. Migration from config apiKey is triggered via observeSettings(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When copilotAPIToken exists in config but no token accounts exist, migrate the token to a ProviderTokenAccount with a fallback label and clear the config key. An async Task enriches the label with the GitHub username via /user. Migration is idempotent — guarded by checking that token accounts are empty. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…e tests Cover the token account catalog entry (injection type, env override), config-to-account migration (happy path, idempotent, no-op cases), and environment precedence (token account overrides config API key). Uses Swift Testing framework with InMemory stores matching existing patterns. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ount Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the tab-style account switcher with stacked usage cards so each account's quota is visible at once. Fix a race where switching accounts rebuilt the menu before the async refresh completed, showing stale data. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
3f2b245 to
eab3b40
Compare
Address Codex review feedback on PR steipete#637: - Add ProviderTokenAccount.externalIdentifier (optional, backwards- compatible Codable). Persists the GitHub login alongside Copilot token accounts. - CopilotLoginFlow now matches existing accounts by externalIdentifier first, only falling back to label-prefix matching for legacy accounts that pre-date the field. Update writes the identifier back so future re-auths use the stable identity path. This prevents duplicate accounts when usernames previously stored as 'Account N' or hand- edited labels. - Preserve CancellationError as a non-empty per-account error marker via tokenAccountSnapshotErrorMessage so the menu does not silently fall back to the live selected-account snapshot when an individual account refresh is cancelled. Global error path keeps suppressing cancellations as before. - Tests: 7 new tests covering identifier persistence, in-place update preservation, legacy backfill, decoding compatibility, and snapshot vs global error message behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Addressed the latest Codex review findings in be6446d: Re-auth dedupe now uses stable GitHub identity. Added an optional Cancellation marker preserved for per-account snapshots. Introduced Tests: 7 new tests covering identifier persistence, in-place update preservation across re-auth, legacy backfill on update, JSON decode compatibility for pre-feature accounts, and the divergence between snapshot vs global error messages for cancellation/non-empty failures. Validation:
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be6446d1f5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
When the user switches menu tabs mid-flight, the in-flight per-account refresh task is cancelled. Two issues then surfaced: 1. menuCardModel fell back to the provider-level live snapshot whenever snapshotOverride was nil — even on override-context cards. That caused every cancelled per-account card to render with the *selected* account's data, producing N identical 'cancelled' cards. 2. refreshTokenAccounts overwrote accountSnapshots wholesale, so a single cancellation wiped the previous good state for every account. Fixes: - menuCardModel: when surface == .overrideCard, never fall back to the live snapshot. Override cards belong to a specific account context; borrowing live data leaks the wrong identity into the card. - refreshTokenAccounts: capture the prior accountSnapshots and pass each account's prior snapshot into resolveAccountOutcome. On CancellationError with a valid prior snapshot, preserve the prior state instead of replacing it with an error chip. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The previous fix preserved prior per-account snapshots when a refresh was cancelled, but only helped when there was prior data to preserve. On a fresh refresh that gets cancelled (e.g. the user closed/reopened the menu quickly), every row would still render as a 'Refresh cancelled' chip with no usable data — three identical 'Copilot / cancelled' cards. Cancellation is not a user-facing error. Treat it as 'no result yet': - resolveAccountOutcome now returns a nil snapshot for CancellationError with no usable prior state. The row is dropped instead of becoming a placeholder. - refreshTokenAccounts skips overwriting accountSnapshots wholesale when every fetch was cancelled and produced no usable data, leaving any prior state intact. - When all accounts are dropped, the existing menu fallback already shows a single live card for the selected account instead of a row per account — so the menu stays usable while a real refresh re-runs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Codex Review: Didn't find any major issues. Swish! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@ratulsarna Codex came back with no additional reviews, and I'm happy with the current state of the pull request. Regarding the individual AI views, I prefer the visual layout of showing multiple accounts stacked on top of each other, even though that differs from how multiple Codex accounts are being displayed i.e. as a segment controller. If you'd like, I can rewrite the copilot view to use segmented controllers like Codex, or we can just keep the stacked view. Additionally we could add a setting that lets users choose between stacking multiple accounts or using a tab segment, but I'd prefer to handle that in a separate PR. |
Summary
Refine Copilot multi-account support and UI. Similar to #613
This keeps legacy single-account Copilot configs working via
apiKey, fixes re-auth so existing accounts are updated in place, scopes stacked multi-account cards to Copilot only and improves the settings/menu-bar account actions.Latest upstream
mainhas been merged into this branch; the merge fixes preserve the Copilot multi-account behavior while keeping the new main-repo token-account provider additions intact.UI
Changes
apiKeyfallback for existing single-account usersmainCompatibility / risk
apiKeyfallback.Validation
./Scripts/compile_and_run.shswift test— 1838 tests passedpnpm check— SwiftFormat + SwiftLint passed with 0 violations