fix(oauth): harden Codex and add Grok PKCE - #1946
Merged
Merged
Conversation
Astro-Han
added a commit
that referenced
this pull request
Aug 3, 2026
Round-2 codex review of PR #1950 found 3 P2 + 1 P3 (verdict FAIL) plus a deletion/simplification list. All addressed: P2 fixes: - Desktop: the poll had already consumed the one-time device code when a cancellation arrived; the exchange reused the aborted signal and burned the code. Exchange now uses an independent signal so exchange + persistence complete even after cancel (deferred-poll test added). - xAI device poll (runtime-host path) could sleep past the window and still issue a request, and mapped local expiry to provider_rejected. Symmetric to the Codex poll: sleep min(interval, remaining), re-check after sleep, and throw OAuthDeviceAuthorizationExpiredError on both local expiry and server-side expired_token. - Codex enrollment now accepts the official CLI's alias in addition to (serde alias parity, test added). P3 fix: - After rebasing onto #1946 (which moved desktop xAI to a PKCE loopback), the renderer mislabeled xAI's random state prefix as a sign-in code. Codex shows the device code; xAI shows no code and its copy describes automatic browser write-back. Deletion/simplification (per review): - Removed the entire Codex loopback public path from oauth-login.ts (OAuthLoginProvider no longer includes openai-codex; buildCodex- AuthorizationUrl/CodexAuthorizationConfig deleted in favor of a private generic loopback builder; provider-config and contracts pruned of codex loopback fields; codex loopback tests deleted). Codex initial-token decoding stays via OAuthInitialTokenProvider. - Deleted apps/desktop/.../openai-codex-helpers.ts: JWT claim extraction merged into runtime subscription-auth.ts (extractCodexAccountClaims), with extractCodexAccountId kept sub-free for header routing per #1946; experimental gate now delegates to runtime isOAuthEnrollmentProvider- Enabled('openai-codex'); claims tests migrated to runtime. - Desktop service now persists runtime OAuthSubscriptionTokens directly (PersistedTokens removed). - Desktop tests tightened: protocol assertions (poll body, exchange form) and the mid-sleep poll-count assertion are owned by the runtime enrollment tests; desktop keeps lifecycle/UI-reason assertions. - Renamed stale "browser-loopback" wording in renderer + e2e fixture. Desktop 1403, runtime 2731, runtime-host 570; lint, format, typecheck, knip clean.
Astro-Han
added a commit
that referenced
this pull request
Aug 3, 2026
Round-2 codex review of PR #1950 found 3 P2 + 1 P3 (verdict FAIL) plus a deletion/simplification list. All addressed: P2 fixes: - Desktop: the poll had already consumed the one-time device code when a cancellation arrived; the exchange reused the aborted signal and burned the code. Exchange now uses an independent signal so exchange + persistence complete even after cancel (deferred-poll test added). - xAI device poll (runtime-host path) could sleep past the window and still issue a request, and mapped local expiry to provider_rejected. Symmetric to the Codex poll: sleep min(interval, remaining), re-check after sleep, and throw OAuthDeviceAuthorizationExpiredError on both local expiry and server-side expired_token. - Codex enrollment now accepts the official CLI's usercode alias in addition to user_code (serde alias parity, test added). P3 fix: - After rebasing onto #1946 (which moved desktop xAI to a PKCE loopback), the renderer mislabeled xAI's random state prefix as a sign-in code. Codex shows the device code; xAI shows no code and its copy describes automatic browser write-back. Deletion/simplification (per review): - Removed the entire Codex loopback public path from oauth-login.ts (OAuthLoginProvider no longer includes openai-codex; the codex-specific authorization builder and config types were deleted in favor of a private generic loopback builder; provider config and contracts pruned of codex loopback fields; codex loopback tests deleted). Codex initial-token decoding stays via OAuthInitialTokenProvider. - Deleted apps/desktop/src/main/oauth/openai-codex-helpers.ts: JWT claim extraction merged into runtime subscription-auth.ts (extractCodexAccountClaims), with extractCodexAccountId kept sub-free for header routing per #1946; the experimental gate now delegates to runtime isOAuthEnrollmentProviderEnabled('openai-codex'); claims tests migrated to runtime. - Desktop service now persists runtime OAuthSubscriptionTokens directly (PersistedTokens removed). - Desktop tests tightened: protocol assertions (poll body, exchange form) and the mid-sleep poll-count assertion are owned by the runtime enrollment tests; desktop keeps lifecycle/UI-reason assertions. - Renamed stale "browser-loopback" wording in renderer + e2e fixture. Desktop 1403, runtime 2731, runtime-host 570; lint, format, typecheck, knip clean.
Astro-Han
added a commit
that referenced
this pull request
Aug 3, 2026
… runtime-host) (#1950) * fix(desktop): switch Codex OAuth to ChatGPT device-code flow Replace the localhost:1455 loopback authorization with the official codex CLI device-auth flow (auth.openai.com/api/accounts/deviceauth/*): - getAuthorizationUrl POSTs {client_id} to /deviceauth/usercode and surfaces the one-time user_code as stateHint. - openAuthorizationUrl opens auth.openai.com/codex/device and starts polling /deviceauth/token (403/404 = pending, 15 min TTL). - completeAuthorization exchanges the returned authorization code at /oauth/token with redirect_uri=auth.openai.com/deviceauth/callback. Removes the loopback listener, PKCE authorize URL builder, and fixed port 1455, eliminating the IPv4/IPv6 loopback mismatch, EADDRINUSE on re-entry, wrong-state callback hangs, and port leaks. Refresh, persistence, and JWT claim extraction are unchanged; requests still go through proxiedFetch so Maka's proxy settings apply (verified the deviceauth endpoints accept undici traffic). Verified against the live endpoints (usercode returns 200, token poll returns deviceauth_authorization_pending), 1397 desktop main tests, renderer/main typecheck, and global lint. * fix(runtime-host): switch Codex OAuth to ChatGPT device-code flow Replace the localhost:1455 loopback listener in the Host OAuth coordinator with the official codex CLI device-auth flow, closing the same IPv4/IPv6 mismatch, port collision, and state-hang failures the desktop service had. - runtime: add codex-oauth-enrollment (startCodexDeviceAuthorization / pollCodexDeviceAuthorization / exchangeCodexDeviceAuthorizationCode) mirroring the xAI enrollment seam, and extend the openai-codex provider contract with deviceAuthBaseUrl / deviceVerifyUrl / deviceRedirectUri. - coordinator: route openai-codex through #runCodexDeviceLogin (present verification URL + user code as stateHint, poll until approval or expiry, exchange the grant); delete the loopback server, callback constants, and deadline helpers. #runAuthorizationCodeLogin is now claude-subscription only. 567 runtime-host tests, 2709 runtime tests, 1397 desktop tests, lint and typecheck pass. * fix(oauth): address codex review — single protocol authority + edge fixes Addresses all 4 P2 findings from the codex review of PR #1950: 1. Desktop duplicated the device-auth protocol with unvalidated token exchange (a malformed 200 with missing refresh_token/expires_in was reported as success and persisted an unparseable credential). The desktop service now reuses @maka/runtime's codex-oauth-enrollment (start/poll/exchange) as the single protocol authority and keeps only product lifecycle: authRequestId/pending state, browser opening, account snapshots, credential persistence. openai-codex-helpers.ts keeps only JWT claims + experimental gate (endpoints live once in runtime contracts). 2. Both polls could issue a request after the device window elapsed and accept a late grant. codex-oauth-enrollment polls first, sleeps at most until expires_at, and re-checks on every loop; a 200 issued in-window is honored even if it arrives just past the boundary. 3. Codex poll passed the caller's AbortSignal into the admitted request, so a cancellation could discard a poll that consumed the one-time code. The admitted request now uses an independent signal (matching xAI), bounded by its own deadline. 4. A local window expiry was encoded as invalid_grant and surfaced as provider_rejected (coordinator), mislabeling a user timeout as a provider rejection; my own coordinator test had pinned the wrong mapping. New OAuthDeviceAuthorizationExpiredError is thrown by both codex and xAI polls on local expiry; the coordinator maps it to authorization_failed. oauth-login.ts codex loopback branches stay as a legacy public-library API, now marked LEGACY. Tests added: desktop malformed-200 no-write, expiry-stops-polling, and credential-write-failure; runtime expiry classification, no-fetch-after- expiry, and admission-cancel races (codex + xAI); coordinator codex cancellation-deferral. Desktop 1399, runtime 2712, runtime-host 568. * fix(oauth): round-2 codex review — edge fixes and deletion pass Round-2 codex review of PR #1950 found 3 P2 + 1 P3 (verdict FAIL) plus a deletion/simplification list. All addressed: P2 fixes: - Desktop: the poll had already consumed the one-time device code when a cancellation arrived; the exchange reused the aborted signal and burned the code. Exchange now uses an independent signal so exchange + persistence complete even after cancel (deferred-poll test added). - xAI device poll (runtime-host path) could sleep past the window and still issue a request, and mapped local expiry to provider_rejected. Symmetric to the Codex poll: sleep min(interval, remaining), re-check after sleep, and throw OAuthDeviceAuthorizationExpiredError on both local expiry and server-side expired_token. - Codex enrollment now accepts the official CLI's usercode alias in addition to user_code (serde alias parity, test added). P3 fix: - After rebasing onto #1946 (which moved desktop xAI to a PKCE loopback), the renderer mislabeled xAI's random state prefix as a sign-in code. Codex shows the device code; xAI shows no code and its copy describes automatic browser write-back. Deletion/simplification (per review): - Removed the entire Codex loopback public path from oauth-login.ts (OAuthLoginProvider no longer includes openai-codex; the codex-specific authorization builder and config types were deleted in favor of a private generic loopback builder; provider config and contracts pruned of codex loopback fields; codex loopback tests deleted). Codex initial-token decoding stays via OAuthInitialTokenProvider. - Deleted apps/desktop/src/main/oauth/openai-codex-helpers.ts: JWT claim extraction merged into runtime subscription-auth.ts (extractCodexAccountClaims), with extractCodexAccountId kept sub-free for header routing per #1946; the experimental gate now delegates to runtime isOAuthEnrollmentProviderEnabled('openai-codex'); claims tests migrated to runtime. - Desktop service now persists runtime OAuthSubscriptionTokens directly (PersistedTokens removed). - Desktop tests tightened: protocol assertions (poll body, exchange form) and the mid-sleep poll-count assertion are owned by the runtime enrollment tests; desktop keeps lifecycle/UI-reason assertions. - Renamed stale "browser-loopback" wording in renderer + e2e fixture. Desktop 1403, runtime 2731, runtime-host 570; lint, format, typecheck, knip clean.
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.
Summary
Review fixes
Verification
No full test suite was run for this PR.