Repository navigation
fix(codex): serve the in-flight token when a native main save is refused - #7035
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 21 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: 45abf1e904
ℹ️ 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".
| // Another writer owns auth.json now (#2999). The exchange already succeeded, so | ||
| // failing the request would waste it. Serve the new access token to this request | ||
| // only: leave the file as the other writer left it, keep nothing as the main | ||
| // credential, and do not retire the grant. | ||
| noteNativeMainRefresh("publish-refused-served-in-flight"); | ||
| if (dependencies.signal?.aborted) throw dependencies.signal.reason; | ||
| return inFlightMainAccountToken(locked, refreshed); |
There was a problem hiding this comment.
Update the owned structure documentation for this new request-only success path. structure/providers/openai-accounts.md:283-297 still states that every successful refresh publishes the rotated grant, advances the credential mutation epoch, and only refuses an external writer; this branch instead returns the refreshed bearer without publishing or advancing the epoch. Leaving the authentication contract unchanged makes the documented safety and cancellation guarantees contradict the runtime behavior.
AGENTS.md reference: src/AGENTS.md:L11-L11
Useful? React with 👍 / 👎.
|
✅ Deterministic PR hygiene checks passed. |
…inary exists (#7048) Fixes #2999. Add nativeMainRefresh: "codex" | "opencodex". When a Codex binary at or above 0.130.0 is found (runtime resolver: CODEX_CLI_PATH, pinned runtime, shim backups, PATH, ChatGPT.app bundle), Codex owns the native main refresh: OpenCodex calls app-server getAuthStatus with includeToken/refreshToken, uses the returned authToken, writes nothing and does not reread auth.json. A chatgpt method with a null token maps to reauth. A missing or older binary, or an explicit "opencodex", keeps the guarded writer plus the in-flight token from #7035. ocx doctor reports the active mode. Integrated by the repository owner's delegated agent under the dev maintainer-integration policy (MAINTAINERS.md); exact-head CI on ffed9fe was all green (35 pass, 7 skipped).
…nd a reliable watcher (lidge-jun#2999) All OpenCodex code reads the native-main ChatGPT credentials from one in-memory store instead of CODEX_HOME/auth.json. One module writes the file (refresh, device reauth, profile switch, rollback). Only the store observes the file: a directory watcher, a short stat timer and a forced check before every refresh and write. - Adopt only complete credentials; empty, partial or malformed content is a hiccup that keeps memory and blocks writes. - A missing file is a logout only after a grace period; no refresh or write during it. - Writes use temp file + rename and re-observe before the rename; an external change wins over the pending write. - A rotated refresh token is written right away; refresh and write stay under the existing CODEX_HOME claim. - Consumers that rejected a symlinked auth.json on base still do. Accepted risk: a Codex write that lands within the same split second as OpenCodex's rename can still be lost (needs a lock Codex also honors). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> This builds on dev 4ae4936, which already contains lidge-jun#7048 and lidge-jun#7035. "codex" mode is unchanged and starts from the in-memory copy. In "opencodex" mode, a save that the store refuses because another writer changed auth.json still serves the in-flight token to that request, while memory follows the file. That keeps the behavior of lidge-jun#7035. A confirmed deletion still gives no token. The desktop-compatibility identity reader (lidge-jun#6079) now reads through the store observe step.
1 participant
Summary
Refs #2999.
When the native main refresh exchange succeeds but saving
auth.jsonis refused because another writer (usually Codex itself) changed the file, the request used to fail withMainAuthJsonChangedDuringRefreshError. The exchange had already happened, so the request was wasted and the user saw a retry error.Now the new access token is returned to that one request. Nothing else changes on that path:
auth.jsonstays exactly as the other writer left it, the rotated grant is not stored anywhere as the main credential, the old grant is not marked rejected, and the credential mutation epoch does not advance. The next request reads whatever Codex left on disk.This does not close the race (Codex writes
auth.jsonin place, so the guard can still miss a write that lands after its last read). It only stops a refused save from also failing the request that paid for the exchange. The ownership fix (delegating refresh to Codex) follows in a separate PR.The comment claiming the dev/ino identity check protects a Codex login is replaced: Codex rewrites the file with truncate+write, which keeps the inode, so only the content hash sees those writes. The identity check stays because it still catches rename-based replacements with identical bytes.
Verification
bun test tests/codex-integration/codex-main-account-refresh.test.ts tests/codex-integration/codex-main-grant-refusal.test.ts tests/codex-integration/main-device-reauth.test.ts tests/codex-integration/codex-model-entitlement-admission.test.ts: 80 pass, 0 fail.src/change reverted, 6 cases incodex-main-account-refresh.test.tsfail (the four refused-save cases now expect the in-flight token, plus the new torn-write case and the cancelled-exchange case).bun run typecheck: clean.bun run test:changed(Bun 1.4.3, 1690 files): 37082 pass, 22 fail. The run overlapped another worktree's suite on a nearly full disk (one earlier attempt hit ENOSPC). Rerunning the eight failing files alone leaves 4 failures intests/codex-integration/codex-runtime.test.ts(persisted runtime selection) andtests/cli/cli-access-audio-live.test.ts(5s timeouts). Neither reaches the changed catch branch; exact-head CI is the authority for those.bun run structure:check: passed (the structure doc for native-main refresh now records the in-flight token).Checklist