Skip to content

fix(codex): use access_token.exp instead of id_token.exp for import expiresAt - #6084

Merged
diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.44from
anki1kr:fix/codex-auth-import-expiry-6075
Jul 3, 2026
Merged

diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.44from
anki1kr:fix/codex-auth-import-expiry-6075

Conversation

@anki1kr

@anki1kr anki1kr commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Problem

When a Codex auth.json has an expired id_token but a still-valid access_token (common — the CLI keeps using tokens after export), the importer reads expiry from id_token.exp. This causes:

  1. The connection appears expired immediately after import
  2. OmniRoute triggers a proactive refresh
  3. If the refresh_token was already rotated by the Codex CLI, the refresh returns invalid_grant → refresh_token becomes NULL
  4. The connection is permanently broken

Root Cause

extractExpiresAt(idToken) only looked at the id token:

expiresAt: extractExpiresAt(idToken),

The id_token's exp claim carries its own TTL (often shorter), while the access_token.exp is the operationally relevant expiry.

Fix

extractExpiresAt now takes both tokens and selects in order:

  1. access_token.exp — preferred (what actually gates API calls)
  2. id_token.exp — fallback when access_token carries no exp claim
  3. null — if neither has a decodeable exp

refresh_token is preserved exactly as-is; the "do NOT refresh-on-import" invariant from the existing code comment remains intact.

Tests

tests/unit/codex-auth-import-expiry.test.ts — 5 tests:

  • uses access_token.exp when id_token.exp is expired
  • falls back to id_token.exp when access_token has no exp claim
  • returns null expiresAt when neither has exp
  • uses access_token.exp when both have future expiries (prefers access)
  • preserves refresh_token in all cases

Fixes #6075

Copilot AI review requested due to automatic review settings July 3, 2026 10:27
@anki1kr
anki1kr requested a review from diegosouzapw as a code owner July 3, 2026 10:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@diegosouzapw
diegosouzapw changed the base branch from main to release/v3.8.44 July 3, 2026 12:39
anki1kr and others added 2 commits July 3, 2026 18:51
…xpiresAt (diegosouzapw#6075)

extractExpiresAt now prefers the access_token exp claim, falling back to
id_token exp only when the access token has none. An expired id_token with a
still-valid access_token no longer marks the connection expired on import and
no longer triggers a premature refresh that could invalidate the token family.

Reconstructed on the current release tip to drop unrelated stacked/base-drift
changes; test rewritten from vitest to node:test so it runs under test-unit.

Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
@diegosouzapw
diegosouzapw force-pushed the fix/codex-auth-import-expiry-6075 branch from a2c949e to 8841808 Compare July 3, 2026 21:52
@diegosouzapw

Copy link
Copy Markdown
Owner

Merged — thank you, @anki1kr! 🙏 The core fix is exactly right: preferring access_token.exp over id_token.exp (with fallback) stops a valid connection from being marked expired on import and prevents the premature refresh that could invalidate the token family.

I reconstructed the branch on the current release tip to keep the PR scoped to just this fix — the original branch had picked up unrelated stacked/base-drift changes (the Zed OAuth provider from #6078, plus older serve.mjs/models/route.ts versions that predated the release's #5172/#5242/#5899 fixes). The Zed work stays with #6078. I also rewrote your regression test from vitest to node:test so it actually runs under the test-unit CI job (tests/unit/*.test.ts here is the Node native runner, not vitest) — it's a genuine guard: 2/5 cases fail without the fix, all 5 pass with it. Ships in the next release.

@diegosouzapw
diegosouzapw merged commit 00d97ca into diegosouzapw:release/v3.8.44 Jul 3, 2026
1 of 3 checks passed
diegosouzapw added a commit to anki1kr/OmniRoute that referenced this pull request Jul 3, 2026
…er (diegosouzapw#6087)

No-auth account providers pack multiple accounts as UUID fingerprints inside
providerSpecificData.fingerprints of a single provider_connections row. The
combo builder mapped each row to one option, so only 'Account 1' appeared.
expandConnectionOptions() now inflates each fingerprint into a selectable
'Account N' option whose id encodes the fingerprint for per-account pinning.

Reconstructed on the current release tip to drop unrelated stacked/base-drift
changes (this branch had carried diegosouzapw#6078/diegosouzapw#6084/diegosouzapw#6086 content). Complementary to
the routing-time expansion in diegosouzapw#6082.

Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
tkgo11 pushed a commit to tkgo11/OmniRoute that referenced this pull request Sep 23, 2026
…xpiresAt (diegosouzapw#6075) (diegosouzapw#6084)

Prefer access_token.exp over id_token.exp for Codex auth import (diegosouzapw#6075). Integrated into release/v3.8.44.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(auth): importing valid Codex auth.json results in broken provider connection

3 participants