Add multi-account support for Codex via OAuth token accounts#509
Add multi-account support for Codex via OAuth token accounts#509niklassaers wants to merge 2 commits intosteipete:mainfrom
Conversation
- Add Codex to TokenAccountSupportCatalog with environment-based injection - Create CodexSettingsReader for CODEX_OAUTH_ACCESS_TOKEN env var - Add codexOAuthToken/codexOAuthResolution to ProviderTokenResolver - Modify CodexOAuthFetchStrategy to check env override before auth.json - Add "Add Account..." menu action opening Settings → Providers - Add settingsProvider menu action and routing - Add "Import from auth.json" button in token accounts UI - Force Settings window key status for proper keyboard focus
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6acc3d0f1
ℹ️ 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".
|
|
||
| @MainActor | ||
| private static func forceSettingsWindowKey() { | ||
| NSApp.setActivationPolicy(.regular) |
There was a problem hiding this comment.
Calling NSApp.setActivationPolicy(.regular) here permanently promotes this LSUIElement app to a regular app, and this commit never switches it back to .accessory; after opening Settings once, users can be left with a persistent Dock icon/app-switcher presence instead of menu-bar-only behavior. This side effect is triggered on the new settings-focus path and is likely unintended for a “no Dock icon” app.
Useful? React with 👍 / 👎.
| idToken: nil, | ||
| accountId: nil, |
There was a problem hiding this comment.
When a token-account override is active, this path synthesizes CodexOAuthCredentials with idToken/accountId set to nil, so mapUsage cannot derive account email/plan from credentials. In the default single-account refresh flow (showAllTokenAccountsInMenu is false), we also skip the label-injection path, so switching Codex token accounts can show fallback/stale account identity in the menu even though usage is fetched with a different token.
Useful? React with 👍 / 👎.
…dentity - Revert activation policy to .accessory when Settings window closes so the app does not permanently show in the Dock / Cmd-Tab switcher. - Pass access token as idToken in env-based OAuth credentials so resolveAccountEmail can extract identity from the JWT claims. - Improve keyboard focus: use multi-retry activation with modern NSApp.activate() API on macOS 14+, and set activation policy before posting the open-settings notification.
ratulsarna
left a comment
ratulsarna
left a comment
There was a problem hiding this comment.
Sharing a few questions I had while reading these changes.
| -> (label: String, action: MenuDescriptor.MenuAction)? | ||
| { | ||
| guard TokenAccountSupportCatalog.support(for: .codex) != nil else { return nil } | ||
| return ("Add Account...", .settingsProvider) |
There was a problem hiding this comment.
Could we walk through the first-time-user path here? If someone has no ~/.codex/auth.json yet, where in this flow do they actually run codex login?
| title: "OAuth tokens", | ||
| subtitle: "Store Codex/OpenAI OAuth access tokens from auth.json.", | ||
| placeholder: "Paste access_token…", | ||
| injection: .environment(key: CodexSettingsReader.oauthAccessTokenKey), |
There was a problem hiding this comment.
In Usage source = Auto, how do we ensure the selected Codex token account cannot end up showing usage fetched from a fallback CLI account when this OAuth token is stale or invalid?
| guard let credentials = try? CodexOAuthCredentialsStore.load() else { return } | ||
| let email = Self.codexEmailFromCredentials(credentials) | ||
| let label = email ?? "Codex (\(DateFormatter.localizedString(from: Date(), dateStyle: .short, timeStyle: .none)))" | ||
| self.settings.addTokenAccount(provider: .codex, label: label, token: credentials.accessToken) |
There was a problem hiding this comment.
Can we double-check what auth behavior we want after import if we only store accessToken here? Where do refresh and account/workspace identity come from in that case?
2 participants
Summary
auth.json, following the existing Zai environment-variable injection patternCodexSettingsReaderreadsCODEX_OAUTH_ACCESS_TOKENfrom the environment (mirrorsZaiSettingsReader)CodexOAuthFetchStrategyresolves credentials from env override first, falling back to~/.codex/auth.jsonNSApp.activate()API on macOS 14+)Approach
This uses
.environment(key:)injection and routes through the OAuth fetch strategy (API-based), complementing #461 which uses.cookieHeaderinjection through the web dashboard strategy. The two approaches handle different auth flows and could coexist.Files changed
TokenAccountSupportCatalog+Data.swift.codexcatalog entry with env injectionCodexSettingsReader.swiftCODEX_OAUTH_ACCESS_TOKENProviderTokenResolver.swiftcodexOAuthToken()/codexOAuthResolution()CodexProviderDescriptor.swiftCodexOAuthFetchStrategyfor env-first resolutionCodexProviderImplementation.swiftloginMenuActionoverride ("Add Account...")MenuDescriptor.swift/MenuContent.swift.settingsProvideractionStatusItemController+Actions.swift/+Menu.swiftshowSettingsProviders()PreferencesProvidersPane.swiftPreferencesProviderSettingsRows.swiftProviderSettingsDescriptors.swiftimportFromFileto token accounts descriptorHiddenWindowView.swiftTest plan
swift buildcompiles without errorsswift testpasses