Repository navigation
feat(codex): refresh and persist tokens for independent Codex homes on account cards - #1322
Conversation
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Two failure paths need attention before merging: incomplete xswap metadata can grant write access to managed credentials, and failed persistence can cause a rotated refresh token to be retried from its stale on-disk copy.
Reviewed changes across account assembly, credential scoping, shared refresh/persistence, regression tests, and documentation.
- Write permissions: Independent Codex homes gain canonical-path write permissions while account-card xswap, pi, and Keychain sources remain read-only.
- Shared refresh: Account cards use the common probe with refresh-and-retry, source-change detection, post-rotation identity validation, and off-main-actor Keychain reloads.
- Coverage: Added tests exercise persistence, fallback, identity changes, and symlink exclusions. Validation here was source-based; the macOS test suite cannot run in this Linux environment.
GPT Astra ｜ 𝕏
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The failed-save fix restores stale credentials that the remainder of the probe still uses.
Reviewed changes since da1df58d, focusing on persistence-failure handling and its regression test.
- Changed failed-save handling: Restored the account's disk-state credential after unsuccessful persistence while returning the refreshed access token.
- Added regression coverage: Introduced a throwing file-access fake and checked successful completion, one OAuth request, and unchanged disk contents.
Validation was source-based; the macOS test suite could not run in this Linux environment.
GPT Astra ｜ 𝕏
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Devin resolved all 2 findings on 5f00aa9
Fixed by Devin (2)
- Failed saves lose refreshed credential state
- Concurrent Codex rotation can be overwritten
View all findings in Devin Review
5f00aa9 to
38b4a26
Compare
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes since 5f00aa9f, focusing on credential persistence, xswap write protection, and plain-card routing.
- Separated working credentials: Preserved rotated tokens for usage, reset-credit requests, and retries while checking source changes against a separate on-disk baseline.
- Strengthened write protection: Rechecked credentials before file writes and derived managed homes from the entire xswap registry, including slots without usable identities.
- Enabled sibling-home renewal: Passed independent sibling-home write permissions through the plain-card assembly and catalog paths.
- Expanded regression coverage: Added rotated-token retry, stale-write rejection, incomplete-registry, and lone-sibling-home cases, with corresponding documentation updates.
Validation was source-based; the macOS test suite could not run in this Linux environment.
GPT Astra ｜ 𝕏
38b4a26 to
edd2635
Compare
…n account cards Co-Authored-By: Robin <rob@sunstory.com>
Co-Authored-By: Robin <rob@sunstory.com>
…st newer logins, and derive managed homes from the whole xswap registry - probe now tracks the working credential and the on-disk baseline separately. A rotated token whose save failed keeps serving the usage fetch, reset-credit request, and any 401 retry (which now sends the rotated refresh token, not the consumed one); the card's conflict check compares the source against the baseline instead of the working copy. - CodexAuthStore.save(_:replacing:) re-reads the file immediately before writing and refuses the write when it no longer holds the credential the refresh started from. - xswap's managed homes now come from the registry itself, including slots with a missing or unusable identity, so CODEX_HOME pointing at such a slot stays read-only. - The plain card's lone sibling home is writable when xswap does not manage it. Co-Authored-By: Robin <rob@sunstory.com>
edd2635 to
d20f114
Compare
# Conflicts: # Sources/OpenUsage/Services/ProviderAccountAssembly+Codex.swift # Tests/OpenUsageTests/CodexSwapMaintainerReviewTests.swift
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes in the merge of main since the previously reviewed writable-home implementation, focusing on the integration with Codex history routing.
- Preserved write permissions: Retained
writableAuthHomeswhile adopting the new account-cardclaimsPiUsagefield in assembly and the test fixture. - Integrated history routing: Kept plain-card sibling-home scanning and account-scoped history configuration alongside the existing refresh and persistence guards.
Validation was source-based, including inspection of the merge-conflict resolutions. The macOS test suite could not run in this Linux environment.
gpt-6.1-sol ｜ 𝕏
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3089b96. Configure here.
| } else if !isDiscovered(candidate.source), CodexSwapAccount.discover(environment: environment, files: files, | ||
| if isWritable(candidate.source) { return candidate } | ||
| } else if !isDiscovered(candidate.source) || isWritable(candidate.source), | ||
| CodexSwapAccount.discover(environment: environment, files: files, |
There was a problem hiding this comment.
Xswap home writable on plain card
Medium Severity
The plain-card branch in scoped still treats a non-discovered file as writable whenever CodexSwapAccount.discover is empty. discover ignores slots with no usable identity, so an xswap registry that only has identity-less slots leaves CODEX_HOME / the configured home writable. managedHomes already classifies that home as managed and plainWritableAuthHomes excludes it, but scoped never consults isWritable on this path, so OpenUsage can rotate a token xswap owns.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 3089b96. Configure here.
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
Adopted from upstream: - Pricing: Claude Sonnet 5.5, Cursor Grok 4.7/grok-bot-cua, Codex Ultrafast (6x GPT-6 Astra), GPT-6 Sol/Luna models; gpt-5.6-sol rate correction (robinebers#1327/robinebers#1331/robinebers#1329/robinebers#1286) - Claude: usage-limit reset grants row (robinebers#1290), credential-rotation preservation (robinebers#1316), launch-cached limits on first 429 (robinebers#1325) - Cursor: structured team usage pools (robinebers#1337), CSV export 20s deadline (robinebers#1324) - Grok: team-principal 412 keeps local spend (robinebers#1272) - Ollama: monthly cloud limit (robinebers#1270), unreadable-plan warning (robinebers#1300) - OpenCode: sub-1% session reset countdown (#062fb5ec) - UI: panel top-edge steady during height animation (robinebers#1345), per-screen header/footer (robinebers#1346), Report an Issue menu item (robinebers#1343), settings copy (robinebers#1328/robinebers#1330) - Deps: PostHog 3.85.3, Sparkle, KeyboardShortcuts, actions/checkout v7 Kept fork-side (divergent designs, upstream versions dropped with their files): - Codex multi-account: fork's scopedAuthPath/accountLabel cards instead of expectedIdentity/writableAuthHomes (robinebers#1321/robinebers#1322/robinebers#1349, CodexSwapAccount, PiCodexLoginScanner, CodexHistoryRefresh/Scope, CodexHomeScanner) - OpenCode credentials: fork's channel-database scanning instead of Go-key stores (robinebers#1323/robinebers#1284) - Claude session attribution: fork's organizationsClaimedByOtherCards (robinebers#1299) - iCloud sync identity schema, Settings/Customize layout, Agent Usage screen Fusion notes: CodexUsagePricing keeps fork's 4-tuple rates (cacheWrite) with upstream's corrected gpt-5.6-sol rates + ultrafast tier + gpt-6-luna. Claude swap cards mirror the new rateLimitResets row. pricing_supplement deduped (WIP duplicates), gpt-6.1-sol alias restored to dot-or-dash matching. WIP manual port (411c175) reconciled: ollama monthly + claude reset grants replaced by upstream originals; its grok-4.7 SKU separation kept and completed.
2 participants
Approved issue
Part of #1265 — part 3 of 3, after #1320 and #1321 (both merged). Replaces the writable-refresh half of #1319.
TL;DR
Codex cards can now renew the token in a Codex home the account owns outright and write it back, through the same
probepath the plain Codex card already uses. xswap-managed homes, pi, and the Keychain stay read-only — including aCODEX_HOMEthat is a symlink to an xswap home or points at an xswap slot with no usable identity.What was happening
~/.codex-workshows "login expired" once its access token lapses until the user runs Codex there again.CodexProvider+Accounts.swiftthat re-implementedprobe/refreshAccessToken/fetchUsageWithRetry, and guarded xswap homes by comparing paths lexically — a symlinkedCODEX_HOMEpointing at xswap's main home would have had its refresh token rotated by OpenUsage.What this changes
CodexSwapAccount.managedHomes(environment:files:home:)(new) returns the main home plus every registry slot's home, whether or not aCodexAccountIdentitycan be built for it;discoverstill returns only identity-bearing slots for card creation.ProviderAccountAssembly.makeCodexCardscomputeswritableAuthHomesper card — the account's own homes minusmanagedHomes, compared afterresolvingSymlinksInPath(CodexHomeScanner.canonicalHome) — andplainWritableAuthHomesfor the single-account path, so a lone sibling home on the plain card renews too.CodexAuthStore.scopedkeeps the refresh token and leavesreadOnly = falseonly for a.filecandidate whose canonical home is writable;.pi,.keychain, and every other file stay read-only exactly as before.refreshAccount()shrinks to a loop over candidates that calls the sharedprobe. The duplicated usage/reset/mapping code andCodexSwapLoginErrorare deleted.allowsAuthFallbackerrors move to the next matching login; anything else is the card's error.probetracks two credentials:authStateis the working copy;onDiskis what the source held when last read or written.refreshAccessTokenre-reads the source after the network round-trip and throwstokenConflictif it differs fromonDisk; on a successful saveonDiskadvances; on a failed save the rotated token stays inauthState(serving the usage fetch, the reset-credit request, and a 401 retry with the new refresh token) whileonDiskstays put, sorefreshAccount'sisCurrent(onDisk)check keeps the result instead of retrying with a consumed refresh token.CodexAuthStore.save(_:replacing:)re-reads the file immediately before writing and throwstokenConflictwhen it no longer holdsonDisk, closing the check-then-write gap against a concurrentcodexrotation.tokenConflict(the rotation is kept on disk; the card falls through to a matching login).reloadLiveAuthis nowasyncand reads the Keychain off the main actor.docs/providers/codex.mddescribes which logins renew and which never do.Heads-up
CodexHomeScanner.standardizedHome(lexical) is still used for discovery and log homes; only the write guard resolves symlinks, so display paths keep the user's spelling.savecovers files only; Keychain writes (plain card) keep the pre-savereloadLiveAuthcheck.Tests
Tests/OpenUsageTests/CodexWritableHomeRefreshTests.swift(14 tests):CODEX_HOMEstays read-only; an xswap main home stays read-only when no slot has a usable identitysave(_:replacing:)refuses a file that changed since it was readCODEX_HOME -> <xswap mainHome>yieldswritableAuthHomes == []and zero network requests on refreshswift build,swift test(1514 tests, 0 failures),./script/build_and_run.sh verify.Screenshots
Not applicable
Link to Devin session: https://app.devin.ai/sessions/a6161a7f3fd44b62b23c5de7ca15d4ab
Open in Devin Desktop: https://app.devin.ai/desktop/session/a6161a7f3fd44b62b23c5de7ca15d4ab?variant=devin
Requested by: @robinebers
Note
High Risk
Changes OAuth refresh, persistence, and concurrency guards for Codex credentials across multi-source account cards; mistakes could corrupt tokens or overwrite xswap/pi-managed logins.
Overview
Codex account cards can refresh and persist OAuth tokens only in independent Codex homes, while xswap-managed homes, pi, and Keychain stay read-only (including
CODEX_HOMEsymlinks into xswap or identity-less swap slots).Account assembly now computes per-card
writableAuthHomesas the account’s homes minusCodexSwapAccount.managedHomes, compared via symlink-resolvingCodexHomeScanner.canonicalHome.CodexAuthStoreuses that set inisWritable/scopedso only those file sources keep a refresh token and may be written.refreshAccount()no longer duplicates usage/refresh logic; it loops candidates through the sharedprobe, which tracks working vs on-disk credentials. Token rotation re-reads the source before save, usessave(_:replacing:)to avoid overwriting a login Codex rotated concurrently, and still serves the refresh in memory when persistence fails. Docs and broad tests cover writable vs read-only behavior, fallbacks, and assembly edge cases.Reviewed by Cursor Bugbot for commit 3089b96. Bugbot is set up for automated code reviews on this repo. Configure here.