fix(codex-auth): replace lexicographic timestamp compare with refresh-token identity check - #29
Closed
dean0x wants to merge 2 commits into
Closed
fix(codex-auth): replace lexicographic timestamp compare with refresh-token identity check#29dean0x wants to merge 2 commits into
dean0x wants to merge 2 commits into
Conversation
…en identity check The concurrent-refresh guard in `persistTokens` used lexicographic `>` on `last_refresh` strings, which is wrong: ISO-8601 format divergence (e.g., fractional-second precision) or a same-second write makes the comparison unreliable. If the guard missed, subswitch would clobber the Codex CLI's rotated refresh_token with its own already-consumed token, causing an `invalid_grant` failure on the next cycle with no downstream rescue. Fix 1: replace the timestamp comparison with a refresh-token identity check (`fileNow.tokens.refresh_token !== refreshToken`). This is format-independent and is the established idiom in the `invalid_grant` retry path at line 291. A secondary `Date.parse` numeric comparison is kept for the same-second case. Fix 2: add an early return when `fileIsNewer` is true but `materialFrom` fails on the newer file. Previously, control fell through into the merge and wrote anyway, defeating the guard even when it had correctly fired. Fixes #26 Co-Authored-By: Claude <noreply@anthropic.com>
…terialFrom fallback
- CHANGELOG: version header from [Unreleased] to [0.2.1] - 2026-08-19 so the
release-notes extractor can locate the entry (RELEASE-FLOW.md:137)
- Comments: reword 'same-second case' to 'cross-format ordering' in both the
CHANGELOG entry and the codex-auth.ts inline comment; the Date.parse secondary
signal fires on format mismatches (e.g. '.500Z' vs 'Z'), not same-second writes
where identical strings produce equality. Add NaN note (Change 5).
- Regression test: fix the format-diverged fixture so lexicographic order is
genuinely wrong while chronological order is right. Baseline '2027-01-15T08:00:00.000Z',
file writes epoch-millis '1800000005000' ('1' < '2' lexicographically → string guard
misses; identity check fires). Empirically confirmed: FAILS on main, PASSES on branch.
Add Falsifier: comment per convention.
- materialFrom fallback: when fileIsNewer is true but materialFrom fails on the newer
file, serve the just-refreshed tokens from memory rather than returning an error.
No write occurs (clobber protection intact); the request now succeeds. Update the
corresponding test to assert success + no write.
Co-Authored-By: Claude <noreply@anthropic.com>
Owner
Author
|
Superseded by #33. All commits from this branch are contained in |
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
last_refreshstring>comparison inpersistTokenswith a refresh-token identity check (fileNow.tokens.refresh_token !== refreshToken), making the concurrent-refresh guard format-independent and collision-safe.fileIsNeweris true butmaterialFromfails on the newer file, preventing the guard from being silently bypassed on a parse error.Changes
src/codex-auth.tspersistTokensgains arefreshToken: stringparameter (the token we sent to the OAuth endpoint).doRefreshupdated to passrefreshToken.fileIsNewernow checksfileNow.tokens.refresh_token !== refreshTokenas primary signal;Date.parsenumeric comparison retained as secondary||signal.materialFromfails on the newer file so control cannot fall through into the merge/write path.test/unit/codex-auth.test.tslast_refreshformats diverge butrefresh_tokendiffers (format-divergence case).fileIsNeweris true butmaterialFromfails (Fix 2 path).CHANGELOG.md— Unreleased section added.Breaking Changes
None.
persistTokensis private.Reviewer Focus Areas
fileIsNewerlogic inpersistTokens: verify the primary (identity) and secondary (timestamp) signals are ordered correctly and that fallback behavior under edge cases (both tokens equal, timestamps unparseable viaDate.parse → NaN) is safe.NaN > NaNisfalse, so an unparseable timestamp does not produce a false-positive newer signal — control reaches the merge and writes, which is the safe fallback when the tokens are equal (no rotation detected).if (fileIsNewer)and cannot affect the normal (non-newer) code path.Fixes #26