From 91e6202b22804c0742ceb723ec6d0467618f6918 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Tue, 1 Sep 2026 04:12:30 +0900 Subject: [PATCH 1/4] fix(codex): serialize native-main refresh on the CODEX_HOME claim Reimplements #3000 (author @MarcTCruz) for the #2999 lock-scope half. The refresh lock is keyed on the grant fingerprint and lives under OPENCODEX_HOME (src/codex/account-store.ts:420-422, via getConfigDir). The file it protects is auth.json under CODEX_HOME, which every OpenCodex install on the machine shares regardless of its own home. Two proxies with distinct OPENCODEX_HOMEs therefore took two unrelated locks and refreshed the one credential concurrently; the loser published its rotated grant over the winner's and the provider then rejected it. The outer lock is now withNativeMainExclusiveClaim on resolveNativeProfileContext(), which is the CODEX_HOME coordination the other native-main paths already use (.opencodex-native-main.claim.sqlite). No new primitive, no FFI. Why not #3000's approach: it introduces src/lib/atomic-file-preserving-replace.ts, which dlopens libc.so.6 / libSystem.B.dylib / kernel32.dll for renameat2 / renamex_np / ReplaceFileW and throws "No rename fallback is safe" on anything else. musl names its libc libc.so, not libc.so.6, so publication would crash on Alpine. It also throws MainAccountTokenRefreshError("transient") on an aborted signal BEFORE persistRefreshedMainAuthJson, so a late cancel discards a grant the provider has already rotated -- the only live refresh token, dropped. The existing check-then-rename guard is left as it is: atomicWriteFile with assertMainAuthJsonSnapshotUnchanged in both beforeRename and validateBeforeRename refuses rather than overwrites, and the covering test (refuses to overwrite an external auth writer after refresh) already passes. Lock order is claim (machine-wide) then fingerprint lock (per-grant), never the reverse: two processes holding different fingerprint locks and then reaching for the same claim would deadlock. Mutation-checked: dropping the claim wrapper fails exactly the new test (3 pass / 1 fail), restored to 4/0. Closes #2999. --- src/codex/main-account.ts | 21 +++++++- tests/codex-main-account-refresh.test.ts | 69 ++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 2 deletions(-) diff --git a/src/codex/main-account.ts b/src/codex/main-account.ts index f1307b1aac..f7ddb34f3e 100644 --- a/src/codex/main-account.ts +++ b/src/codex/main-account.ts @@ -19,6 +19,8 @@ import { resolveCodexHomeDir } from "./home"; import { assertNotRealCodexHomeUnderTest } from "../lib/test-home-guard"; import { clearAccountNeedsReauth } from "./account-runtime-state"; import { advanceCodexCredentialMutationEpoch } from "./credential-mutation-epoch"; +import { withNativeMainExclusiveClaim } from "./native-main-claim"; +import { resolveNativeProfileContext } from "./native-profile-store"; export { MAIN_CODEX_ACCOUNT_ID } from "./account-id"; @@ -191,7 +193,21 @@ async function resolveMainAccountToken( ? AbortSignal.any([dependencies.signal, AbortSignal.timeout(30_000)]) : AbortSignal.timeout(30_000); const lockKey = refreshGrantFingerprintForToken(initial.refreshToken); - return withCodexRefreshFileLock(lockKey, signal, async () => { + // Two locks, because they guard two different things that live in two different + // homes. `withCodexRefreshFileLock` is keyed on the grant fingerprint and lives + // under OPENCODEX_HOME; it serializes refreshes of the SAME grant within one + // install. The file being rewritten is `auth.json` under CODEX_HOME, which every + // OpenCodex install on the machine shares no matter what its own home is -- so two + // proxies with distinct OPENCODEX_HOMEs took two unrelated fingerprint locks and + // refreshed the one credential concurrently (#2999). + // + // The outer claim is the CODEX_HOME coordination the other native-main paths + // already use (`.opencodex-native-main.claim.sqlite`), so this needs no new + // primitive and no FFI. Order is claim (machine-wide) then fingerprint lock + // (per-grant), never the reverse: two processes holding different fingerprint + // locks and then reaching for the same claim would deadlock. + return withNativeMainExclusiveClaim(resolveNativeProfileContext(), () => + withCodexRefreshFileLock(lockKey, signal, async () => { const locked = readMainAuthJsonCredential(); if (!locked) throw new MainAuthJsonChangedDuringRefreshError(); if (!locked.refreshToken @@ -221,7 +237,8 @@ async function resolveMainAccountToken( const result = persistRefreshedMainAuthJson(locked, refreshed); clearAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID); return result; - }); + }), + { waitMs: 30_000 }); } /** Refresh the CLI-owned native credential before upstream I/O and publish it atomically. */ diff --git a/tests/codex-main-account-refresh.test.ts b/tests/codex-main-account-refresh.test.ts index b0910c823b..d6c3b48d26 100644 --- a/tests/codex-main-account-refresh.test.ts +++ b/tests/codex-main-account-refresh.test.ts @@ -122,4 +122,73 @@ describe("native main token refresh", () => { expect(readFileSync(authPath)).toEqual(original); expect(readdirSync(home).filter(name => name.includes(".tmp"))).toEqual([]); }); + + /** + * #2999: the refresh lock is keyed on the grant fingerprint and lives under + * OPENCODEX_HOME, but the file it protects is `auth.json` under CODEX_HOME, which + * every install on the machine shares. Two proxies with different OPENCODEX_HOMEs + * therefore took two unrelated locks and refreshed the one credential at once, so + * the loser's rotated grant was published over the winner's and then rejected by + * the provider. + * + * The claim this now takes lives in CODEX_HOME, so it is the same lock for both. + * Driven through the real `getValidMainAccountToken` with OPENCODEX_HOME actually + * swapped between the two calls: asserting on the claim primitive directly would + * pass even if `main-account.ts` never took it. + */ + test("two OPENCODEX_HOMEs serialize on the one CODEX_HOME credential", async () => { + const authPath = join(home, "auth.json"); + writeFileSync(authPath, JSON.stringify({ + tokens: { access_token: expiredJwt(), refresh_token: "old-refresh", account_id: "account-main" }, + })); + + const homeA = mkdtempSync(join(tmpdir(), "ocx-home-a-")); + const homeB = mkdtempSync(join(tmpdir(), "ocx-home-b-")); + const previousOcxHome = process.env.OPENCODEX_HOME; + // Overlap is observed, not assumed: each refresh records when it entered and + // left, so a serialized pair reads enter/leave/enter and a concurrent one + // reads enter/enter. + const order: string[] = []; + let release: (() => void) | undefined; + const firstEntered = Promise.withResolvers(); + + const refreshFor = (label: string, gate: boolean) => async () => { + order.push(`enter:${label}`); + if (gate) { + firstEntered.resolve(); + await new Promise(resolve => { release = resolve; }); + } + order.push(`leave:${label}`); + return { + accessToken: `fresh-${label}`, + refreshToken: `rotated-${label}`, + expires: Date.now() + 3_600_000, + accountId: "account-main", + }; + }; + + try { + process.env.OPENCODEX_HOME = homeA; + const first = getValidMainAccountToken({ refreshToken: refreshFor("a", true) }); + await firstEntered.promise; + + // Second install, different OPENCODEX_HOME, same CODEX_HOME. Before the fix + // this entered immediately; now it waits on the shared claim. + process.env.OPENCODEX_HOME = homeB; + const second = getValidMainAccountToken({ refreshToken: refreshFor("b", false) }); + await Bun.sleep(50); + expect(order).toEqual(["enter:a"]); + + release?.(); + await first; + await second.catch(() => null); + expect(order.slice(0, 3)).toEqual(["enter:a", "leave:a", "enter:b"]); + } finally { + release?.(); + if (previousOcxHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousOcxHome; + rmSync(homeA, { recursive: true, force: true }); + rmSync(homeB, { recursive: true, force: true }); + } + }); }); From 85cccdfc72d5864a044981b15a03e5cc0d80cbe2 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Tue, 1 Sep 2026 09:16:18 +0900 Subject: [PATCH 2/4] fix(codex): abort contended native-main refresh claims --- src/codex/main-account.ts | 60 ++++++++++---------- src/codex/native-main-claim.ts | 23 +++++++- tests/codex-main-account-refresh.test.ts | 71 +++++++++++++++++++++--- 3 files changed, 116 insertions(+), 38 deletions(-) diff --git a/src/codex/main-account.ts b/src/codex/main-account.ts index f7ddb34f3e..f218479a5a 100644 --- a/src/codex/main-account.ts +++ b/src/codex/main-account.ts @@ -206,39 +206,41 @@ async function resolveMainAccountToken( // primitive and no FFI. Order is claim (machine-wide) then fingerprint lock // (per-grant), never the reverse: two processes holding different fingerprint // locks and then reaching for the same claim would deadlock. - return withNativeMainExclusiveClaim(resolveNativeProfileContext(), () => - withCodexRefreshFileLock(lockKey, signal, async () => { - const locked = readMainAuthJsonCredential(); - if (!locked) throw new MainAuthJsonChangedDuringRefreshError(); - if (!locked.refreshToken - || refreshGrantFingerprintForToken(locked.refreshToken) !== lockKey) { + return withNativeMainExclusiveClaim( + resolveNativeProfileContext(), + () => withCodexRefreshFileLock(lockKey, signal, async () => { + const locked = readMainAuthJsonCredential(); + if (!locked) throw new MainAuthJsonChangedDuringRefreshError(); + if (!locked.refreshToken + || refreshGrantFingerprintForToken(locked.refreshToken) !== lockKey) { + if (locked.accessToken !== rejectedAccessToken + && mainAccessTokenFresh(locked.accessToken, Date.now(), 0)) { + return { accessToken: locked.accessToken!, chatgptAccountId: locked.chatgptAccountId }; + } + throw new MainAuthJsonChangedDuringRefreshError(); + } if (locked.accessToken !== rejectedAccessToken - && mainAccessTokenFresh(locked.accessToken, Date.now(), 0)) { + && mainAccessTokenFresh(locked.accessToken, Date.now(), MAIN_TOKEN_REFRESH_SKEW_MS)) { return { accessToken: locked.accessToken!, chatgptAccountId: locked.chatgptAccountId }; } - throw new MainAuthJsonChangedDuringRefreshError(); - } - if (locked.accessToken !== rejectedAccessToken - && mainAccessTokenFresh(locked.accessToken, Date.now(), MAIN_TOKEN_REFRESH_SKEW_MS)) { - return { accessToken: locked.accessToken!, chatgptAccountId: locked.chatgptAccountId }; - } - const refresh = dependencies.refreshToken - ?? ((refreshToken: string, options: { signal: AbortSignal }) => refreshChatGPTToken(refreshToken, options)); - let refreshed: OAuthCredentials; - try { - refreshed = await refresh(locked.refreshToken, { signal }); - } catch (cause) { - const message = cause instanceof Error ? cause.message.toLowerCase() : ""; - const reason = /invalid_grant|invalidated|revoked|expired/.test(message) - ? "reauth" as const - : "transient" as const; - throw new MainAccountTokenRefreshError(reason, { cause }); - } - const result = persistRefreshedMainAuthJson(locked, refreshed); - clearAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID); - return result; + const refresh = dependencies.refreshToken + ?? ((refreshToken: string, options: { signal: AbortSignal }) => refreshChatGPTToken(refreshToken, options)); + let refreshed: OAuthCredentials; + try { + refreshed = await refresh(locked.refreshToken, { signal }); + } catch (cause) { + const message = cause instanceof Error ? cause.message.toLowerCase() : ""; + const reason = /invalid_grant|invalidated|revoked|expired/.test(message) + ? "reauth" as const + : "transient" as const; + throw new MainAccountTokenRefreshError(reason, { cause }); + } + const result = persistRefreshedMainAuthJson(locked, refreshed); + clearAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID); + return result; }), - { waitMs: 30_000 }); + { waitMs: 30_000, signal }, + ); } /** Refresh the CLI-owned native credential before upstream I/O and publish it atomically. */ diff --git a/src/codex/native-main-claim.ts b/src/codex/native-main-claim.ts index 2cbbe45ae1..58f7af518e 100644 --- a/src/codex/native-main-claim.ts +++ b/src/codex/native-main-claim.ts @@ -17,6 +17,7 @@ export const NATIVE_MAIN_CLAIM_DB = ".opencodex-native-main.claim.sqlite"; export interface NativeMainClaimOptions { waitMs?: number; pollMs?: number; + signal?: AbortSignal; hardenPath?: (path: string) => Promise; platform?: NodeJS.Platform; env?: NodeJS.ProcessEnv; @@ -121,6 +122,22 @@ function releaseClaim(database: Database | undefined, file: StableLockFile | und try { file?.close(); } catch { /* operation already completed */ } } +function waitForClaimRetry(ms: number, signal?: AbortSignal): Promise { + if (!signal) return Bun.sleep(ms); + if (signal.aborted) return Promise.reject(signal.reason); + return new Promise((resolve, reject) => { + const timer = setTimeout(() => { + signal.removeEventListener("abort", onAbort); + resolve(); + }, ms); + const onAbort = () => { + clearTimeout(timer); + reject(signal.reason); + }; + signal.addEventListener("abort", onAbort, { once: true }); + }); +} + export async function withNativeMainSharedClaim( context: NativeProfileContext, operation: () => Promise, @@ -151,9 +168,11 @@ export async function withNativeMainExclusiveClaim( operation: () => Promise, options: NativeMainClaimOptions = {}, ): Promise { + const signal = options.signal; const deadline = Date.now() + Math.max(0, options.waitMs ?? 0); const pollMs = Math.max(1, options.pollMs ?? 50); for (;;) { + if (signal?.aborted) throw signal.reason; let database: Database | undefined; let file: StableLockFile | undefined; try { @@ -162,14 +181,16 @@ export async function withNativeMainExclusiveClaim( assertStableLockFile(nativeMainClaimPath(context), file); } catch (error) { releaseClaim(database, file); + if (signal?.aborted) throw signal.reason; const mapped = mapClaimSetupError(error, "Native-main credentials are in use."); if (mapped.code === "NATIVE_MAIN_CLAIM_BUSY" && Date.now() < deadline) { - await Bun.sleep(Math.min(pollMs, Math.max(1, deadline - Date.now()))); + await waitForClaimRetry(Math.min(pollMs, Math.max(1, deadline - Date.now())), signal); continue; } throw mapped; } try { + if (signal?.aborted) throw signal.reason; return await operation(); } finally { releaseClaim(database, file); diff --git a/tests/codex-main-account-refresh.test.ts b/tests/codex-main-account-refresh.test.ts index d6c3b48d26..08ff5c05bc 100644 --- a/tests/codex-main-account-refresh.test.ts +++ b/tests/codex-main-account-refresh.test.ts @@ -145,9 +145,9 @@ describe("native main token refresh", () => { const homeA = mkdtempSync(join(tmpdir(), "ocx-home-a-")); const homeB = mkdtempSync(join(tmpdir(), "ocx-home-b-")); const previousOcxHome = process.env.OPENCODEX_HOME; - // Overlap is observed, not assumed: each refresh records when it entered and - // left, so a serialized pair reads enter/leave/enter and a concurrent one - // reads enter/enter. + // The first refresh records its entry and exit. A concurrent second refresh would + // add enter:b before the first release; the serialized follower instead rereads + // fresh credentials and does not refresh the now-rotated grant itself. const order: string[] = []; let release: (() => void) | undefined; const firstEntered = Promise.withResolvers(); @@ -160,8 +160,8 @@ describe("native main token refresh", () => { } order.push(`leave:${label}`); return { - accessToken: `fresh-${label}`, - refreshToken: `rotated-${label}`, + access: `fresh-${label}`, + refresh: `rotated-${label}`, expires: Date.now() + 3_600_000, accountId: "account-main", }; @@ -176,13 +176,12 @@ describe("native main token refresh", () => { // this entered immediately; now it waits on the shared claim. process.env.OPENCODEX_HOME = homeB; const second = getValidMainAccountToken({ refreshToken: refreshFor("b", false) }); - await Bun.sleep(50); expect(order).toEqual(["enter:a"]); release?.(); await first; - await second.catch(() => null); - expect(order.slice(0, 3)).toEqual(["enter:a", "leave:a", "enter:b"]); + await second; + expect(order).toEqual(["enter:a", "leave:a"]); } finally { release?.(); if (previousOcxHome === undefined) delete process.env.OPENCODEX_HOME; @@ -191,4 +190,60 @@ describe("native main token refresh", () => { rmSync(homeB, { recursive: true, force: true }); } }); + + test("aborts a contended native-main refresh before it retries", async () => { + const authPath = join(home, "auth.json"); + writeFileSync(authPath, JSON.stringify({ + tokens: { access_token: expiredJwt(), refresh_token: "old-refresh", account_id: "account-main" }, + })); + + const homeA = mkdtempSync(join(tmpdir(), "ocx-home-a-")); + const homeB = mkdtempSync(join(tmpdir(), "ocx-home-b-")); + const previousOcxHome = process.env.OPENCODEX_HOME; + const firstEntered = Promise.withResolvers(); + const releaseFirst = Promise.withResolvers(); + const abort = new AbortController(); + const abortReason = new Error("refresh cancelled while native-main claim was busy"); + let secondRefreshStarted = false; + + try { + process.env.OPENCODEX_HOME = homeA; + const first = getValidMainAccountToken({ + refreshToken: async () => { + firstEntered.resolve(); + await releaseFirst.promise; + return { + access: "fresh-a", + refresh: "rotated-a", + expires: Date.now() + 3_600_000, + accountId: "account-main", + }; + }, + }); + await firstEntered.promise; + + // The first refresh holds the CODEX_HOME claim. Cancellation must release the + // second caller from that wait instead of letting it refresh after the holder exits. + process.env.OPENCODEX_HOME = homeB; + const second = getValidMainAccountToken({ + signal: abort.signal, + refreshToken: async () => { + secondRefreshStarted = true; + throw new Error("must not refresh after cancellation"); + }, + }); + abort.abort(abortReason); + releaseFirst.resolve(); + + await expect(second).rejects.toBe(abortReason); + await expect(first).resolves.toEqual({ accessToken: "fresh-a", chatgptAccountId: "account-main" }); + expect(secondRefreshStarted).toBe(false); + } finally { + releaseFirst.resolve(); + if (previousOcxHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousOcxHome; + rmSync(homeA, { recursive: true, force: true }); + rmSync(homeB, { recursive: true, force: true }); + } + }); }); From 118f9a857201d415de15526a7fdadbf0e8cbc804 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Tue, 1 Sep 2026 09:41:32 +0900 Subject: [PATCH 3/4] fix(codex): preserve native-main claim transient contracts --- src/codex/auth-context.ts | 3 + src/codex/main-account.ts | 76 ++++++++------- src/codex/native-main-claim.ts | 1 + src/server/responses/codex-auth-error.ts | 4 +- src/server/responses/compact.ts | 6 ++ src/server/responses/core.ts | 6 ++ tests/native-main-claim.test.ts | 53 +++++++++- tests/responses-compaction-routing.test.ts | 28 +++++- tests/responses-native-main-refresh.test.ts | 101 +++++++++++++++++++- 9 files changed, 236 insertions(+), 42 deletions(-) diff --git a/src/codex/auth-context.ts b/src/codex/auth-context.ts index 812cadd22f..febf9415ff 100644 --- a/src/codex/auth-context.ts +++ b/src/codex/auth-context.ts @@ -10,6 +10,7 @@ import { import { isAccountNeedsReauth, markAccountNeedsReauth } from "./account-runtime-state"; import { isCodexAccountPaused } from "./account-pause"; import { ConfigMutationLockError } from "../config"; +import { NativeProfileError } from "./native-profile-types"; import { isCodexAccountUsable } from "./account-usability"; import { reconcileMainCodexAccountRuntimeState } from "./account-lifecycle"; import { @@ -328,6 +329,8 @@ export function shouldMarkAccountNeedsReauthForCodexAuthFailure(cause: unknown): && !(cause instanceof CodexCredentialRefreshStaleError) && !(cause instanceof MainAuthJsonChangedDuringRefreshError) && !(cause instanceof MainAccountTokenRefreshError && cause.reason === "transient") + && !(cause instanceof NativeProfileError && cause.retryable) + && !(cause instanceof DOMException && cause.name === "AbortError") && !(cause instanceof ConfigMutationLockError); } diff --git a/src/codex/main-account.ts b/src/codex/main-account.ts index f218479a5a..cd5cfb50ba 100644 --- a/src/codex/main-account.ts +++ b/src/codex/main-account.ts @@ -189,9 +189,10 @@ async function resolveMainAccountToken( : null; } + const refreshTimeout = AbortSignal.timeout(30_000); const signal = dependencies.signal - ? AbortSignal.any([dependencies.signal, AbortSignal.timeout(30_000)]) - : AbortSignal.timeout(30_000); + ? AbortSignal.any([dependencies.signal, refreshTimeout]) + : refreshTimeout; const lockKey = refreshGrantFingerprintForToken(initial.refreshToken); // Two locks, because they guard two different things that live in two different // homes. `withCodexRefreshFileLock` is keyed on the grant fingerprint and lives @@ -206,41 +207,48 @@ async function resolveMainAccountToken( // primitive and no FFI. Order is claim (machine-wide) then fingerprint lock // (per-grant), never the reverse: two processes holding different fingerprint // locks and then reaching for the same claim would deadlock. - return withNativeMainExclusiveClaim( - resolveNativeProfileContext(), - () => withCodexRefreshFileLock(lockKey, signal, async () => { - const locked = readMainAuthJsonCredential(); - if (!locked) throw new MainAuthJsonChangedDuringRefreshError(); - if (!locked.refreshToken - || refreshGrantFingerprintForToken(locked.refreshToken) !== lockKey) { + try { + return await withNativeMainExclusiveClaim( + resolveNativeProfileContext(), + () => withCodexRefreshFileLock(lockKey, signal, async () => { + const locked = readMainAuthJsonCredential(); + if (!locked) throw new MainAuthJsonChangedDuringRefreshError(); + if (!locked.refreshToken + || refreshGrantFingerprintForToken(locked.refreshToken) !== lockKey) { + if (locked.accessToken !== rejectedAccessToken + && mainAccessTokenFresh(locked.accessToken, Date.now(), 0)) { + return { accessToken: locked.accessToken!, chatgptAccountId: locked.chatgptAccountId }; + } + throw new MainAuthJsonChangedDuringRefreshError(); + } if (locked.accessToken !== rejectedAccessToken - && mainAccessTokenFresh(locked.accessToken, Date.now(), 0)) { + && mainAccessTokenFresh(locked.accessToken, Date.now(), MAIN_TOKEN_REFRESH_SKEW_MS)) { return { accessToken: locked.accessToken!, chatgptAccountId: locked.chatgptAccountId }; } - throw new MainAuthJsonChangedDuringRefreshError(); - } - if (locked.accessToken !== rejectedAccessToken - && mainAccessTokenFresh(locked.accessToken, Date.now(), MAIN_TOKEN_REFRESH_SKEW_MS)) { - return { accessToken: locked.accessToken!, chatgptAccountId: locked.chatgptAccountId }; - } - const refresh = dependencies.refreshToken - ?? ((refreshToken: string, options: { signal: AbortSignal }) => refreshChatGPTToken(refreshToken, options)); - let refreshed: OAuthCredentials; - try { - refreshed = await refresh(locked.refreshToken, { signal }); - } catch (cause) { - const message = cause instanceof Error ? cause.message.toLowerCase() : ""; - const reason = /invalid_grant|invalidated|revoked|expired/.test(message) - ? "reauth" as const - : "transient" as const; - throw new MainAccountTokenRefreshError(reason, { cause }); - } - const result = persistRefreshedMainAuthJson(locked, refreshed); - clearAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID); - return result; - }), - { waitMs: 30_000, signal }, - ); + const refresh = dependencies.refreshToken + ?? ((refreshToken: string, options: { signal: AbortSignal }) => refreshChatGPTToken(refreshToken, options)); + let refreshed: OAuthCredentials; + try { + refreshed = await refresh(locked.refreshToken, { signal }); + } catch (cause) { + const message = cause instanceof Error ? cause.message.toLowerCase() : ""; + const reason = /invalid_grant|invalidated|revoked|expired/.test(message) + ? "reauth" as const + : "transient" as const; + throw new MainAccountTokenRefreshError(reason, { cause }); + } + const result = persistRefreshedMainAuthJson(locked, refreshed); + clearAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID); + return result; + }), + { waitMs: 30_000, signal }, + ); + } catch (cause) { + if (refreshTimeout.aborted && !dependencies.signal?.aborted) { + throw new MainAccountTokenRefreshError("transient", { cause }); + } + throw cause; + } } /** Refresh the CLI-owned native credential before upstream I/O and publish it atomically. */ diff --git a/src/codex/native-main-claim.ts b/src/codex/native-main-claim.ts index 58f7af518e..9b523f1038 100644 --- a/src/codex/native-main-claim.ts +++ b/src/codex/native-main-claim.ts @@ -132,6 +132,7 @@ function waitForClaimRetry(ms: number, signal?: AbortSignal): Promise { }, ms); const onAbort = () => { clearTimeout(timer); + signal.removeEventListener("abort", onAbort); reject(signal.reason); }; signal.addEventListener("abort", onAbort, { once: true }); diff --git a/src/server/responses/codex-auth-error.ts b/src/server/responses/codex-auth-error.ts index 63da982922..8d84c54392 100644 --- a/src/server/responses/codex-auth-error.ts +++ b/src/server/responses/codex-auth-error.ts @@ -15,6 +15,7 @@ import { MainAccountTokenRefreshError, MainAuthJsonChangedDuringRefreshError, } from "../../codex/main-account"; +import { NativeProfileError } from "../../codex/native-profile-types"; export interface CodexAuthContextErrorResponseOptions { accountSelector?: string; @@ -26,7 +27,8 @@ export function nativeMainRefreshFailureResponse(error: unknown): Response { return formatErrorResponse(401, "authentication_error", "Codex main account needs reauthentication"); } if (error instanceof MainAccountTokenRefreshError - || error instanceof MainAuthJsonChangedDuringRefreshError) { + || error instanceof MainAuthJsonChangedDuringRefreshError + || (error instanceof NativeProfileError && error.retryable)) { const response = formatErrorResponse( 503, "server_busy", diff --git a/src/server/responses/compact.ts b/src/server/responses/compact.ts index b14800fc7b..eac14d3d50 100644 --- a/src/server/responses/compact.ts +++ b/src/server/responses/compact.ts @@ -267,6 +267,9 @@ async function refreshNativeMainCompactContext(args: { } return { ok: true, authCtx: refreshedAuthCtx, provider: refreshedProvider, headers }; } catch (error) { + if (req.signal.aborted) { + return { ok: false, response: formatErrorResponse(499, "client_cancelled", "Client cancelled compact request") }; + } return { ok: false, response: nativeMainRefreshFailureResponse(error) }; } } @@ -614,6 +617,9 @@ export async function handleResponsesCompact( } } } catch (err) { + if (req.signal.aborted) { + return formatErrorResponse(499, "client_cancelled", "Client cancelled compact request"); + } const response = mapCodexAuthContextErrorToResponse(err, { accountSelector: route.codexAccountNamespace, now: Date.now(), diff --git a/src/server/responses/core.ts b/src/server/responses/core.ts index 1f56ed979a..10026c57ce 100644 --- a/src/server/responses/core.ts +++ b/src/server/responses/core.ts @@ -1757,6 +1757,9 @@ async function resolveResponsesCodexAuth( substituteMainCredential, }; } catch (err) { + if (options.abortSignal?.aborted || req.signal.aborted) { + return { ok: false, response: clientCancelledResponse() }; + } if (err instanceof CodexAuthContextError) { const safeAccountLabel = route.codexAccountNamespace ? `${route.providerName}-${route.codexAccountNamespace}` @@ -1902,6 +1905,9 @@ async function refreshNativeMainForwardAuth(args: { }); return { ok: true, authCtx: refreshedAuthCtx, provider, headers }; } catch (error) { + if (options.abortSignal?.aborted || req.signal.aborted) { + return { ok: false, response: clientCancelledResponse() }; + } return { ok: false, response: nativeMainRefreshFailureResponse(error) }; } } diff --git a/tests/native-main-claim.test.ts b/tests/native-main-claim.test.ts index 2cd01c3872..9b54924f26 100644 --- a/tests/native-main-claim.test.ts +++ b/tests/native-main-claim.test.ts @@ -1,4 +1,4 @@ -import { afterEach, describe, expect, test } from "bun:test"; +import { afterEach, describe, expect, spyOn, test } from "bun:test"; import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -35,11 +35,12 @@ describe("native-main shared and exclusive claims", () => { const context = fixture(); const entered = deferred(); const release = deferred(); + let firstSettled = false; const first = withNativeMainSharedClaim(context, async () => { entered.resolve(); await release.promise; return "first"; - }, { hardenPath: noHardening }); + }, { hardenPath: noHardening }).finally(() => { firstSettled = true; }); await entered.promise; await expect(withNativeMainSharedClaim( @@ -47,14 +48,60 @@ describe("native-main shared and exclusive claims", () => { async () => "second", { hardenPath: noHardening }, )).resolves.toBe("second"); + let contenderRuns = 0; await expect(withNativeMainExclusiveClaim( context, - async () => "must-not-run", + async () => { + contenderRuns += 1; + return "must-not-run"; + }, { waitMs: 0, hardenPath: noHardening }, )).rejects.toMatchObject({ code: "NATIVE_MAIN_CLAIM_BUSY", retryable: true }); + expect(contenderRuns).toBe(0); + expect(firstSettled).toBe(false); release.resolve(); await expect(first).resolves.toBe("first"); + await expect(withNativeMainExclusiveClaim( + context, + async () => "exclusive-after-release", + { waitMs: 0, hardenPath: noHardening }, + )).resolves.toBe("exclusive-after-release"); + }); + + test("a cancelled exclusive claimant removes its retry listener without releasing the holder", async () => { + const context = fixture(); + const entered = deferred(); + const release = deferred(); + const holder = withNativeMainSharedClaim(context, async () => { + entered.resolve(); + await release.promise; + }, { hardenPath: noHardening }); + await entered.promise; + + const controller = new AbortController(); + const addListener = spyOn(controller.signal, "addEventListener"); + const removeListener = spyOn(controller.signal, "removeEventListener"); + const cancellation = new Error("client cancelled claim wait"); + const contender = withNativeMainExclusiveClaim( + context, + async () => "must-not-run", + { waitMs: 2_000, pollMs: 10, signal: controller.signal, hardenPath: noHardening }, + ); + while (!addListener.mock.calls.some(([type]) => type === "abort")) await Promise.resolve(); + expect(addListener).toHaveBeenCalledWith("abort", expect.any(Function), { once: true }); + + controller.abort(cancellation); + await expect(contender).rejects.toBe(cancellation); + expect(removeListener).toHaveBeenCalledWith("abort", expect.any(Function)); + + await expect(withNativeMainExclusiveClaim( + context, + async () => "still-locked", + { waitMs: 0, hardenPath: noHardening }, + )).rejects.toMatchObject({ code: "NATIVE_MAIN_CLAIM_BUSY" }); + release.resolve(); + await holder; }); test("closing a sibling reader cannot let another process bypass a retained shared claim", async () => { diff --git a/tests/responses-compaction-routing.test.ts b/tests/responses-compaction-routing.test.ts index d97154c661..47fd315aa4 100644 --- a/tests/responses-compaction-routing.test.ts +++ b/tests/responses-compaction-routing.test.ts @@ -19,7 +19,8 @@ import { resolveCodexAccountForThread, } from "../src/codex/routing"; import { clearAccountQuota, updateAccountQuota } from "../src/codex/auth-api"; -import { MAIN_CODEX_ACCOUNT_ID } from "../src/codex/main-account"; +import { MAIN_CODEX_ACCOUNT_ID, MainAccountTokenRefreshError } from "../src/codex/main-account"; +import { NativeProfileError } from "../src/codex/native-profile-types"; import { fallbackCodexAccountLogLabel } from "../src/codex/account-label"; import * as authContextModule from "../src/codex/auth-context"; import { @@ -211,6 +212,31 @@ describe("Codex auth-context error parity (#2392)", () => { status: 401, regularLog: true, }, + { + label: "native-main claim contention", + createError: () => new authContextModule.CodexAuthContextError( + MAIN_CODEX_ACCOUNT_ID, + new NativeProfileError( + "NATIVE_MAIN_CLAIM_BUSY", + "Native-main credentials are in use.", + 503, + true, + ), + ), + status: 503, + retryAfter: "1", + regularLog: true, + }, + { + label: "native-main claim timeout", + createError: () => new authContextModule.CodexAuthContextError( + MAIN_CODEX_ACCOUNT_ID, + new MainAccountTokenRefreshError("transient"), + ), + status: 503, + retryAfter: "1", + regularLog: true, + }, { label: "pool authentication failure", createError: () => new authContextModule.CodexPoolAuthenticationError("Pool credential is unavailable"), diff --git a/tests/responses-native-main-refresh.test.ts b/tests/responses-native-main-refresh.test.ts index ab59bebe1f..685effc53c 100644 --- a/tests/responses-native-main-refresh.test.ts +++ b/tests/responses-native-main-refresh.test.ts @@ -1,10 +1,13 @@ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { clearAccountNeedsReauth } from "../src/codex/auth-api"; import { saveCodexAccountCredential } from "../src/codex/account-store"; -import { MAIN_CODEX_ACCOUNT_ID } from "../src/codex/main-account"; +import { isAccountNeedsReauth } from "../src/codex/account-runtime-state"; +import { getValidMainAccountToken, MAIN_CODEX_ACCOUNT_ID } from "../src/codex/main-account"; +import { withNativeMainSharedClaim } from "../src/codex/native-main-claim"; +import type { NativeProfileContext } from "../src/codex/native-profile-store"; import { clearCodexUpstreamHealth, clearThreadAccountMap } from "../src/codex/routing"; import { handleResponses, handleResponsesCompact } from "../src/server/responses"; import type { RequestLogContext } from "../src/server/request-log"; @@ -34,13 +37,14 @@ function config(options: { secondAccount?: boolean } = {}): OcxConfig { } as OcxConfig; } -function request(path: "/v1/responses" | "/v1/responses/compact"): Request { +function request(path: "/v1/responses" | "/v1/responses/compact", signal?: AbortSignal): Request { return new Request(`http://localhost${path}`, { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify(path.endsWith("compact") ? { model: "gpt-5.5", input: [] } : { model: "gpt-5.5", input: "hello", stream: false }), + signal, }); } @@ -137,6 +141,42 @@ describe("native main 401 refresh and replay", () => { expect(sends).toEqual(["Bearer refreshed-access"]); }); + test("converts an outer native-main claim timeout into a transient refresh failure", async () => { + writeFileSync(join(home, "auth.json"), JSON.stringify({ + tokens: { refresh_token: "refresh-grant", account_id: "account-main" }, + })); + let releaseHolder!: () => void; + const holderRelease = new Promise(resolve => { releaseHolder = resolve; }); + let holderEntered!: () => void; + const holderReady = new Promise(resolve => { holderEntered = resolve; }); + const holder = withNativeMainSharedClaim( + { codexHome: home } as NativeProfileContext, + async () => { + holderEntered(); + await holderRelease; + }, + { hardenPath: async () => {} }, + ); + await holderReady; + + const timeout = new AbortController(); + const addListener = spyOn(timeout.signal, "addEventListener"); + const timeoutSpy = spyOn(AbortSignal, "timeout").mockReturnValue(timeout.signal); + try { + const pending = getValidMainAccountToken(); + while (!addListener.mock.calls.some(([type]) => type === "abort")) await Promise.resolve(); + timeout.abort(new DOMException("claim timed out", "TimeoutError")); + await expect(pending).rejects.toMatchObject({ + name: "MainAccountTokenRefreshError", + reason: "transient", + }); + } finally { + timeoutSpy.mockRestore(); + releaseHolder(); + await holder; + } + }); + test("Responses refreshes and performs exactly one physical replay", async () => { const harness = install401ThenRefreshHarness(); const response = await handleResponses( @@ -212,4 +252,59 @@ describe("native main 401 refresh and replay", () => { expect(refreshes).toEqual(["refresh-grant"]); }); } + + test.each(["/v1/responses", "/v1/responses/compact"] as const)( + "%s keeps an outer native-main claim cancellation as 499 without quarantining main", + async path => { + writeFileSync(join(home, "auth.json"), JSON.stringify({ + tokens: { refresh_token: "refresh-grant", account_id: "account-main" }, + })); + let releaseHolder!: () => void; + const holderRelease = new Promise(resolve => { releaseHolder = resolve; }); + let holderEntered!: () => void; + const holderReady = new Promise(resolve => { holderEntered = resolve; }); + const holder = withNativeMainSharedClaim( + { codexHome: home } as NativeProfileContext, + async () => { + holderEntered(); + await holderRelease; + }, + { hardenPath: async () => {} }, + ); + await holderReady; + + const controller = new AbortController(); + const originalAny = AbortSignal.any; + let claimWaitListener: ReturnType | undefined; + const anySpy = spyOn(AbortSignal, "any").mockImplementation(signals => { + const combined = originalAny.call(AbortSignal, signals); + claimWaitListener = spyOn(combined, "addEventListener"); + return combined; + }); + try { + const pending = path === "/v1/responses" + ? handleResponses( + request(path, controller.signal), + config(), + { model: "", provider: "" } as RequestLogContext, + { abortSignal: controller.signal }, + ) + : handleResponsesCompact( + request(path, controller.signal), + config(), + { model: "", provider: "" } as RequestLogContext, + ); + while (!claimWaitListener?.mock.calls.some(([type]) => type === "abort")) await Promise.resolve(); + controller.abort(); + + const response = await pending; + expect(response.status).toBe(499); + expect(isAccountNeedsReauth(MAIN_CODEX_ACCOUNT_ID)).toBe(false); + } finally { + anySpy.mockRestore(); + releaseHolder(); + await holder; + } + }, + ); }); From f3c4e9f75b17fa616279fdb8e90c59049cf6a1c1 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Tue, 1 Sep 2026 09:50:39 +0900 Subject: [PATCH 4/4] fix(codex): honor websocket abort state for reauth --- src/codex/auth-context.ts | 4 ++-- tests/responses-native-main-refresh.test.ts | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/codex/auth-context.ts b/src/codex/auth-context.ts index febf9415ff..dc0161a98c 100644 --- a/src/codex/auth-context.ts +++ b/src/codex/auth-context.ts @@ -618,7 +618,7 @@ export async function resolveCodexAuthContext( } catch (cause) { if (probeLeaseId && probeQuotaScope) releaseCodexQuotaScopeProbeLease(accountId, probeQuotaScope, probeLeaseId); else if (probeLeaseId) releaseCodexQuotaProbeLease(accountId, probeLeaseId); - if (shouldMarkAccountNeedsReauthForCodexAuthFailure(cause)) { + if (!options.signal?.aborted && shouldMarkAccountNeedsReauthForCodexAuthFailure(cause)) { markAccountNeedsReauth(accountId, writerGeneration); } throw new CodexAuthContextError(accountId, cause); @@ -663,7 +663,7 @@ export async function resolveCodexAuthContext( } catch (cause) { if (probeLeaseId && probeQuotaScope) releaseCodexQuotaScopeProbeLease(accountId, probeQuotaScope, probeLeaseId); else if (probeLeaseId) releaseCodexQuotaProbeLease(accountId, probeLeaseId); - if (shouldMarkAccountNeedsReauthForCodexAuthFailure(cause)) { + if (!options.signal?.aborted && shouldMarkAccountNeedsReauthForCodexAuthFailure(cause)) { markAccountNeedsReauth(accountId, writerGeneration); } throw new CodexAuthContextError(accountId, cause); diff --git a/tests/responses-native-main-refresh.test.ts b/tests/responses-native-main-refresh.test.ts index 685effc53c..94b01c72fd 100644 --- a/tests/responses-native-main-refresh.test.ts +++ b/tests/responses-native-main-refresh.test.ts @@ -254,7 +254,7 @@ describe("native main 401 refresh and replay", () => { } test.each(["/v1/responses", "/v1/responses/compact"] as const)( - "%s keeps an outer native-main claim cancellation as 499 without quarantining main", + "%s keeps the WebSocket string-abort claim cancellation as 499 without quarantining main", async path => { writeFileSync(join(home, "auth.json"), JSON.stringify({ tokens: { refresh_token: "refresh-grant", account_id: "account-main" }, @@ -287,7 +287,7 @@ describe("native main 401 refresh and replay", () => { request(path, controller.signal), config(), { model: "", provider: "" } as RequestLogContext, - { abortSignal: controller.signal }, + { abortSignal: controller.signal, inboundTransport: "websocket" }, ) : handleResponsesCompact( request(path, controller.signal), @@ -295,7 +295,7 @@ describe("native main 401 refresh and replay", () => { { model: "", provider: "" } as RequestLogContext, ); while (!claimWaitListener?.mock.calls.some(([type]) => type === "abort")) await Promise.resolve(); - controller.abort(); + controller.abort("websocket turn superseded or closed"); const response = await pending; expect(response.status).toBe(499);