From 3f2237d7fbcf0f43c651091d931d8f6e639e7f8c Mon Sep 17 00:00:00 2001 From: MarcTCruz <58499846+MarcTCruz@users.noreply.github.com> Date: Fri, 9 Oct 2026 22:33:37 +0000 Subject: [PATCH 1/2] fix(codex): never discard a rotated native-main refresh token on cancel A successful refresh_token exchange retires the token it sent. Two paths in resolveMainAccountToken still lost the replacement when the caller went away: - the caller's signal was passed into the exchange, so a disconnect after the POST was sent aborted the request or its body read; - after the exchange resolved, a caller-abort check threw before persistRefreshedMainAuthJson ran (added in #5268). Either way auth.json kept the retired token and the account needed a manual re-login. Caller cancellation now stops only the waits before the exchange. The exchange observes the refresh's own 30-second timeout, the result is published through the unchanged snapshot/identity guards, and the caller's abort reason is thrown after the commit. An external writer that replaced auth.json during the exchange still wins. The claim and refresh-lock helpers await the callback through completion, so a cancelled wait cannot release an acquired lock mid-exchange. Co-Authored-By: Claude Opus 5.5 --- .../src/content/docs/guides/providers.md | 6 ++ src/codex/main-account.ts | 13 ++-- structure/codex-home.md | 6 +- structure/providers/openai-accounts.md | 16 +++++ .../codex-main-account-refresh.test.ts | 72 ++++++++++++++++--- ...ponses-alternate-main-cancellation.test.ts | 21 ++++-- 6 files changed, 109 insertions(+), 25 deletions(-) diff --git a/docs-site/src/content/docs/guides/providers.md b/docs-site/src/content/docs/guides/providers.md index b6621a8c2ee..114292e8266 100644 --- a/docs-site/src/content/docs/guides/providers.md +++ b/docs-site/src/content/docs/guides/providers.md @@ -381,6 +381,12 @@ from provider enforcement, rate limits, or account actions. Terminal refresh failures mark the account as needing reauthentication instead of retrying forever. +For the native main Codex credential in `CODEX_HOME/auth.json`, cancelling a request stops a +waiting refresh before the token exchange starts. An exchange already started finishes within +its own 30-second timeout and saves successful access and refresh tokens before reporting the +request's cancellation. This preserves the rotated refresh token for later requests; a concurrent +external credential replacement still takes precedence. + **Cooldowns (Codex pool).** Upstream `429` / quota responses set a hard cooldown from `Retry-After`, quota `reset` headers (capped), or a short default backoff. Accounts on an explicit `Retry-After` cooldown are not probed early; reset-derived cooldowns may receive a paced probe lease diff --git a/src/codex/main-account.ts b/src/codex/main-account.ts index 107b486522d..2fa128737ae 100644 --- a/src/codex/main-account.ts +++ b/src/codex/main-account.ts @@ -510,8 +510,9 @@ async function resolveMainAccountToken( } const refresh = dependencies.refreshToken ?? refreshNativeMainGrant; let refreshed: OAuthCredentials; + if (signal.aborted) throw signal.reason; try { - refreshed = await refresh(locked.refreshToken, { signal }); + refreshed = await refresh(locked.refreshToken, { signal: refreshTimeout }); } catch (cause) { // A refusal already classified at the endpoint keeps that verdict. An injected refresh // threw an ordinary Error and has only its message, so prose still decides there -- @@ -535,16 +536,14 @@ async function resolveMainAccountToken( } throw new MainAccountTokenRefreshError(reason, { cause }); } - // The refresh may resolve after the caller went away (an implementation that does - // not observe the signal, or an abort landing in the window between resolution and - // commit). A cancelled request's late refresh must not rewrite auth.json on behalf - // of a request that no longer exists -- the same fence the reauth twin applies - // before its own commit above. - if (dependencies.signal?.aborted) throw dependencies.signal.reason; + // Once the exchange starts, only its own timeout may abort it: a successful + // rotating grant retires the old refresh token. Save the only live grant before + // honoring caller cancellation, while retaining the external-writer guards. const result = persistRefreshedMainAuthJson(locked, refreshed); // A permitted in-flight success speaks only for its own profile and grant. clearMainRefreshGrantRejection(mainRefreshGrantKey(locked)!); if (dependencies.preserveReauth !== true) clearAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID); + if (dependencies.signal?.aborted) throw dependencies.signal.reason; return result; }), { waitMs: 30_000, signal }, diff --git a/structure/codex-home.md b/structure/codex-home.md index 19d95ea24da..66c7c5cebbc 100644 --- a/structure/codex-home.md +++ b/structure/codex-home.md @@ -227,9 +227,9 @@ reopened. `ocx status` reports the same drift, also while the loop is gated off, It says nothing in a sibling's home, whose routing names the owner beside it, and it promises only `ocx sync`, because it cannot see whether a healer runs, is gated, or is paused. -The native main slot also accepts one same-identity device reauth (#3898): -`/api/codex-auth/main/reauth-device` (start/status/cancel) plus -`ocx account main reauth`. The grant is the OpenAI deviceauth grant already +Native-main refresh follows the [rotation contract](providers/openai-accounts.md#native-main-refresh-cancellation). +Same-identity device reauth (#3898) instead obtains a new login grant through +`/api/codex-auth/main/reauth-device` (start/status/cancel) plus `ocx account main reauth`. It uses the OpenAI deviceauth grant already used for pool accounts, but nothing routes through the pool login surface — `/api/codex-auth/login` keeps rejecting `__main__` — and the commit is a sibling of the refresh write: same-identity check against the snapshot diff --git a/structure/providers/openai-accounts.md b/structure/providers/openai-accounts.md index c3888bc4706..6b55ddc5edf 100644 --- a/structure/providers/openai-accounts.md +++ b/structure/providers/openai-accounts.md @@ -280,6 +280,22 @@ API-key and custom forward destinations preserve their metadata. See [Responses Listener startup diagnostics follow [the runtime lifecycle contract](../runtime.md#lifecycle); malformed optional listener blocks follow [config loading](../config.md#config-surface). +## Native-main refresh cancellation + +`src/codex/main-account.ts` takes the CODEX_HOME exclusive claim, then the +per-grant refresh file lock. Caller cancellation can stop either wait and is +checked before the token exchange starts. Once started, the exchange observes +only the refresh's own 30-second timeout: success rotates and retires the old +refresh token, so cancellation cannot discard the only live grant. Both locks +await the callback through completion, including publication; aborting their +wait signal does not release an acquired lock while the exchange runs. +Successful refresh publishes access and refresh tokens and advances the +credential mutation epoch before clearing grant rejection and applicable +reauth quarantine, then throws the caller's abort reason if cancelled. +Snapshot and identity checks still refuse an external auth writer, including +when the caller also cancelled. Same-identity device reauth obtains a new login +grant rather than rotating the existing grant and keeps its pre-publication fence. + ## Manual account pause and resume Manual pause/resume in `src/codex/auth-api/account-pause-group.ts` resolves existing native-main diff --git a/tests/codex-integration/codex-main-account-refresh.test.ts b/tests/codex-integration/codex-main-account-refresh.test.ts index 9541891ba58..b71a594301c 100644 --- a/tests/codex-integration/codex-main-account-refresh.test.ts +++ b/tests/codex-integration/codex-main-account-refresh.test.ts @@ -329,27 +329,79 @@ describe("publication never overwrites an external Codex writer (#2999)", () => expect(readFileSync(authPath, "utf8")).toContain("ocx-staged-access"); }); - test("a caller cancelled during the refresh does not commit the late result", async () => { - // The refresh seam resolves after the caller aborted (a client that disconnected - // mid-/v1/models). The late result must never reach auth.json on behalf of a - // request that no longer exists: the publication is fenced before the rename, - // the file keeps its prior bytes, and the credential mutation epoch does not - // advance for a commit that never happened. + test("a caller cancelled during the refresh commits the rotated grant before rejecting", async () => { + // A successful exchange retires the old refresh token. Even after a client + // disconnect, publication must save the only live grant before reporting cancellation. const authPath = join(home, "auth.json"); seedExpired(authPath); - const before = readFileSync(authPath, "utf8"); const controller = new AbortController(); + const abortReason = new Error("client disconnected"); const epochBefore = codexCredentialMutationEpoch(); await expect(getValidMainAccountToken({ signal: controller.signal, refreshToken: async () => { - controller.abort(new Error("client disconnected")); + controller.abort(abortReason); return refreshOk(); }, - })).rejects.toThrow("client disconnected"); + })).rejects.toBe(abortReason); - expect(readFileSync(authPath, "utf8")).toBe(before); + expect(JSON.parse(readFileSync(authPath, "utf8")).tokens).toMatchObject({ + access_token: "ocx-staged-access", + refresh_token: "ocx-staged-refresh", + }); + expect(codexCredentialMutationEpoch()).toBe(epochBefore + 1); + }); + + test("caller cancellation does not abort the token exchange signal", async () => { + const authPath = join(home, "auth.json"); + seedExpired(authPath); + const controller = new AbortController(); + const abortReason = new Error("client disconnected during exchange"); + const epochBefore = codexCredentialMutationEpoch(); + let exchangeAborted: boolean | undefined; + + await expect(getValidMainAccountToken({ + signal: controller.signal, + refreshToken: async (_refreshToken, options) => { + controller.abort(abortReason); + // Recorded rather than asserted here: a throw inside the seam would be masked by the + // caller's abort reason on the way out. + exchangeAborted = options.signal.aborted; + return refreshOk(); + }, + })).rejects.toBe(abortReason); + + expect(exchangeAborted).toBe(false); + expect(JSON.parse(readFileSync(authPath, "utf8")).tokens).toMatchObject({ + access_token: "ocx-staged-access", + refresh_token: "ocx-staged-refresh", + }); + expect(codexCredentialMutationEpoch()).toBe(epochBefore + 1); + }); + + test("an external writer during a cancelled exchange still refuses publication", async () => { + const authPath = join(home, "auth.json"); + seedExpired(authPath); + const external = JSON.stringify({ + tokens: { access_token: "external-access", refresh_token: "external-refresh", account_id: "account-external" }, + }); + const controller = new AbortController(); + const epochBefore = codexCredentialMutationEpoch(); + + await expect(getValidMainAccountToken({ + signal: controller.signal, + refreshToken: async () => { + const swapPath = join(home, "external-auth.json"); + writeFileSync(swapPath, external); + renameSync(swapPath, authPath); + controller.abort(new Error("client disconnected after external replacement")); + return refreshOk(); + }, + })).rejects.toThrow("changed while its token was refreshing"); + + expect(readFileSync(authPath, "utf8")).toBe(external); + expect(readdirSync(home).filter(name => name.includes(".tmp"))).toEqual([]); expect(codexCredentialMutationEpoch()).toBe(epochBefore); }); }); diff --git a/tests/responses/responses-alternate-main-cancellation.test.ts b/tests/responses/responses-alternate-main-cancellation.test.ts index c59a6952787..0fb94bc6b9a 100644 --- a/tests/responses/responses-alternate-main-cancellation.test.ts +++ b/tests/responses/responses-alternate-main-cancellation.test.ts @@ -173,8 +173,8 @@ for (const entry of ["retry helper", "messages ingress"] as const) { if (url.href === "https://auth.openai.com/oauth/token") { refreshes.push(new URLSearchParams(String(init?.body)).get("refresh_token") ?? ""); entered.resolve(); - // Deliberately ignore init.signal: a late endpoint result must be fenced by - // the credential owner even if the network operation does not stop on abort. + // Deliberately ignore init.signal: a late rotating grant must be saved even + // though caller cancellation still prevents subsequent inference. return release.promise; } if (url.hostname === "chatgpt.com" && url.pathname.endsWith("/responses")) { @@ -213,9 +213,20 @@ for (const entry of ["retry helper", "messages ingress"] as const) { expect(after.epoch).toBe(before.epoch + 1); expect(after.refused).toBe(false); } else { - // One aggregate assertion reports every forbidden side effect, including - // terminal refusal that leaves bytes intact but poisons later requests. - expect({ ...credentialState(), inference }).toEqual({ ...before, inference: initialInference }); + expect(inference).toEqual(initialInference); + if (scenario === "cancel-success") { + const after = credentialState(); + expect(JSON.parse(after.bytes).tokens).toMatchObject({ + access_token: "fixture-refreshed-main", refresh_token: "fixture-rotated-main", + }); + expect(after.epoch).toBe(before.epoch + 1); + expect(after.refused).toBe(false); + expect(after.genericReauth).toBe(false); + } + if (scenario === "cancel-terminal") { + // A cancelled terminal refusal must not poison later requests. + expect(credentialState()).toEqual(before); + } } } finally { release.resolve(refreshReply(false)); From 2077b8a6757a352e1b595cc876d95bb34df3b5b7 Mon Sep 17 00:00:00 2001 From: JUN Date: Sat, 10 Oct 2026 18:05:32 +0900 Subject: [PATCH 2/2] test(codex): assert the persisted account identity after cancelled refresh publication --- tests/codex-integration/codex-main-account-refresh.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/codex-integration/codex-main-account-refresh.test.ts b/tests/codex-integration/codex-main-account-refresh.test.ts index b71a594301c..4f0f35332c6 100644 --- a/tests/codex-integration/codex-main-account-refresh.test.ts +++ b/tests/codex-integration/codex-main-account-refresh.test.ts @@ -349,6 +349,7 @@ describe("publication never overwrites an external Codex writer (#2999)", () => expect(JSON.parse(readFileSync(authPath, "utf8")).tokens).toMatchObject({ access_token: "ocx-staged-access", refresh_token: "ocx-staged-refresh", + account_id: "account-main", }); expect(codexCredentialMutationEpoch()).toBe(epochBefore + 1); }); @@ -376,6 +377,7 @@ describe("publication never overwrites an external Codex writer (#2999)", () => expect(JSON.parse(readFileSync(authPath, "utf8")).tokens).toMatchObject({ access_token: "ocx-staged-access", refresh_token: "ocx-staged-refresh", + account_id: "account-main", }); expect(codexCredentialMutationEpoch()).toBe(epochBefore + 1); });