Skip to content

Fix native main token refresh and replay - #2497

Draft
MarcTCruz wants to merge 1 commit into
lidge-jun:devfrom
MarcTCruz:fix/native-main-refresh-v2
Draft

Fix native main token refresh and replay#2497
MarcTCruz wants to merge 1 commit into
lidge-jun:devfrom
MarcTCruz:fix/native-main-refresh-v2

Conversation

@MarcTCruz

@MarcTCruz MarcTCruz commented Aug 24, 2026

Copy link
Copy Markdown

Summary

  • Refreshes native Codex __main__ auth.json credentials before upstream use, including refresh-only auth files.
  • Adds exactly-one native-main 401 refresh/replay for Responses passthrough, generic adapter, and /v1/responses/compact paths.
  • Persists native auth.json first with snapshot checks, no-replace replacement, external-writer adoption, and then converges matching pool credentials.

Closes #2221.

Current head

  • PR head: 86a49e852 (86a49e852581835253a06e40ebb7012215820757).
  • Base: latest dev at faaa78dc0 (faaa78dc05489625e5c9bf450050a46a7fa91d1f).
  • Commit author and committer: MarcTCruz <58499846+MarcTCruz@users.noreply.github.com>.

Verification

Post-rewrite head 86a49e852:

  • git diff --check origin/dev...HEAD: pass.
  • bun run typecheck: pass.
  • bun run test tests/codex-main-account-refresh.test.ts tests/responses-native-main-refresh.test.ts tests/responses-compact-native-main-refresh.test.ts tests/codex-auth-context.test.ts tests/codex-account-store.test.ts tests/codex-model-entitlements.test.ts tests/chatgpt-oauth.test.ts tests/chatgpt-token-expiry.test.ts tests/config.test.ts tests/routing-analytics.test.ts: 313 pass, 0 fail.
  • bun run privacy:scan: pass.

Full bun run prepush / full-suite local CI is not claimed here.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed. No docs or release notes needed for this bugfix.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Security review

This changes authentication and credential persistence. Keep this PR Draft until maintainer security review is complete and a maintainer applies maintainer-sponsored.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • All CI tests are green on my local testing.
  • I pushed my PR to the latest dev commit.
  • I resolved all correct Codex and CodeRabbit findings.
  • My PR is ready for review.

@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the intake: hygiene-blocked Deterministic PR hygiene checks failed label Aug 24, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Deterministic hygiene checks failed.

  • unsponsored_surface — This changes an authentication, workflow, release-automation, or dependency surface. MAINTAINERS.md requires security review for these; ask a maintainer to apply maintainer-sponsored once they have reviewed it. Paths: src/codex/auth-context.ts, src/oauth/chatgpt.ts.

@github-actions github-actions Bot added the bug Something isn't working label Aug 24, 2026
@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • hygiene: unsponsored_surface.

What to do

  • Fix unsponsored_surface — This changes an authentication, workflow, release-automation, or dependency surface. MAINTAINERS.md requires security review for these; ask a maintainer to apply maintainer-sponsored once they have reviewed it. Paths: src/codex/auth-context.ts, src/oauth/chatgpt.ts.
  • Tick all four boxes in the PR description once you're done (currently 1/4).

Review readiness checklist

  • ⬜ All CI tests are green on my local testing.
  • ✅ I pushed my PR to the latest dev commit.
  • ⬜ I resolved all correct Codex and CodeRabbit findings.
  • ⬜ My PR is ready for review.

1/4 boxes ticked.

This pull request was already a draft. Its draft status will be preserved after every issue above is resolved.
@MarcTCruz Tick the boxes once your local CI is green, your branch is on the latest dev commit, and every correct Codex and CodeRabbit finding is resolved.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 63 / 80

설명: 이 풀 리퀘스트는 네이티브 메인 계정, 곧 Codex CLI 가 쓰는 ~/.codex/auth.json 의 만료된 접근 토큰을 프록시가 스스로 고치고, 업스트림이 401 을 주면 한 번만 다시 보내게 하려는 드래프트다. 이슈 번호는 2221 이다. 지금 CURRENT dev HEAD 는 02c302a54 이다. 이번 시간에 origin/dev 는 84ade0f 에서 02c302a 로 움직였다. 2477 이 이름 공간 별명을 tool_choice 에 맞추고, 2476 이 이어가기 스냅샷의 같은 내용 다시 쓰기를 막았다. 이 PR 의 헤드 20a4e72d2 는 그 HEAD 를 이미 머지했다. 뒤처지지 않았다. types.ts/config.ts 쪼개기 때문에 깨지지도 않는다. 고치는 잎은 src/config/atomic-write.ts 이다. 닫고 다시 짜라고 하지 않는다. package.json 은 2.32.0 이다. src/config.ts 는 3238줄이다. src/runtime 폴더는 지금 HEAD 에 없다.

지금 HEAD 의 src/codex/main-account.ts 는 68줄이다. getMainAccountTokenreadCodexTokens() 로 auth.json 을 읽기만 한다. 리프레시 토큰이 있어도 쓰지 않는다. isMainAccountTokenLive 는 JWT exp 만 본다. 풀 계정은 이미 src/codex/account-store.ts 의 getValidCodexToken 이 리프레시한다. 메인만 뒤처져 있다. 그래서 네이티브 메인을 회전 후보로 고른 뒤 죽은 접근 토큰을 업스트림으로 보낸다. 401 이 난다. 다시 고치지 않는다. 사용자가 Codex CLI 로 로그인해 두고 프록시만 오래 켜 두면 이 구멍이 보인다. 이슈 2221 제목이 바로 그것이다. 이 PR 이 랜딩되기 전에는 2221 을 닫지 말 것.

PR 이 넣는 길은 세 덩어리다. 첫째, src/codex/main-account.ts 에 getValidMainAccountTokenforceRefreshMainAccountToken 이 생긴다. auth.json 을 디바이스·아이노드·시각·크기·해시 스냅샷으로 읽고, 같은 리프레시 그랜트 락 안에서 고치고, 바깥 작가가 파일을 바꿨으면 그 내용을 채택한다. 스냅샷이 같으면 하드 링크로 원자 교체한다. 그 다음 같은 그랜트의 풀 자격 증명을 publishFreshCredentialForGrant 로 맞춘다. 둘째, src/codex/auth-context.ts 의 resolveCodexAuthContext 가 메인 풀을 고를 때 읽기 전용 토큰 대신 이 비동기 길을 탄다. Direct 대체의 materializeCodexUpstreamAuthAsync 도 같다. 셋째, src/server/responses/core.ts 패스스루와 일반 어댑터 루프, src/server/responses/compact.ts 가 업스트림 401 을 보면 codexMain401ReplayAttempted 가드로 한 번만 리프레시하고 다시 보낸다. 복구 종류는 src/usage/log.ts 의 codex-main-401 이다. 실패 매핑은 src/server/responses/codex-auth-error.ts 의 nativeMainRefreshFailureResponse 다. 만료·철회는 401, 락 경쟁과 알 수 없는 리프레시는 503 Retry-After 1 이다.

테스트는 크다. tests/codex-main-account-refresh.test.ts 가 784줄이다. 리프레시만 있는 auth.json, 바깥 작가 채택, 최종 게시 경쟁을 본다. tests/responses-native-main-refresh.test.ts 와 tests/responses-compact-native-main-refresh.test.ts 가 401 재전송을 본다. 작성자는 관련 묶음 313 통과와 typecheck 통과를 적었다. 전체 스위트는 통과라고 주장하지 않는다. Bun 1.4.0 isolate 자식이 십 분 넘게 CPU 만 썼다고 적었다. 리뷰 준비 체크리스트는 로컬 CI 전체, 코더래빗 정리, 리뷰 준비가 비어 있다. 보안 민감 변경이라 드래프트로 두고 maintainer-sponsored 가 필요하다고 본문이 말한다. 그 말은 맞다. 자격 증명을 디스크에 다시 쓰는 변경이다.

이 구멍이 사용자에게 보이는 이유는 메인이 풀과 같이 돌아가기 때문이다. 풀은 고치고 메인은 고치지 않으면, 메인이 고를 때마다 401 이 난다. 우선순위 63 은 그 때문이다. 요청이 바로 502 로 죽는 전송 구멍은 아니다. 2473 이 막아 둔 웹소켓 큰 프레임 구멍과도 다르다. 지금은 자격 증명이 낡은 채로 업스트림을 치는 문제다. 그래도 머지하지 말 것. 드래프트다. 윈도우에서 하드 링크 교체가 실패할 수 있고, 같은 거절 토큰으로 리프레시 비행에 다시 들어가는 재귀가 있다. Direct kind main 과 채팅 컴플리션 길은 401 재전송이 없다. 본문이 닫겠다고 한 2221 은 랜딩 전에는 닫지 말 것. 카탈로그 팁은 Ox Alpha x-preview-f-free + deepseek-v4-flash-vision-exp. Cursor 정적 카탈로그는 opus-4-8-fast / opus-5-fast. 2334 CursorCredentialRouter 는 여전히 src/providers/cursor-pool.ts 모듈+테스트만 있다. 2332 H2 는 discovery 전용. src/lib/bounded-body.ts 128줄은 여전히 미리 죽은 시그널에서 취소 없이 던진다. shouldUseCodexWsUpstream 101줄은 여전히 나가는 크기 가드가 없다.

src/codex/main-account.ts getMainAccountToken - 지금 HEAD 는 auth.json 을 읽기만 한다. 리프레시가 없다
src/codex/main-account.ts forceRefreshMainAccountToken - PR 은 합류한 비행이 거절된 접근 토큰을 그대로 돌려주면 같은 함수를 다시 부른다. 깊이 한도가 없다
src/codex/main-account.ts replaceMainAuthJsonWithoutClobbering - 교체가 linkSync 하드 링크다. 지금 HEAD 의 renameAtomicFile 윈도우 원자 교체 길을 타지 않는다
src/server/responses/core.ts usesCodexForwardPoolAuth + kind main-pool - 401 재전송은 메인 풀 포워드 openai-responses 만 탄다. Direct kind main 과 다른 어댑터는 다시 보내지 않는다
src/config/atomic-write.ts recordOwnedConfigPath(getConfigDir(), path) - 네이티브 ~/.codex/auth.json 쓰기도 설정 디렉터리 소유 경로로 기록된다
src/codex/account-usability.ts isMainAccountCredentialUsable - 리프레시 토큰만 있으면 라우팅 후보가 된다. 이미 철회된 그랜트도 고를 수 있다
이슈 2221 - 이 PR 본문이 Closes 를 걸었다. 랜딩 전에는 닫지 말 것

메인테이너의 판단이 필요한 지점

  • 드래프트를 유지할지. 유지하는 편이 맞다. 보안 리뷰와 maintainer-sponsored 가 본문 조건이다
  • 윈도우 Codex 홈에서 하드 링크 교체를 허용할지. 실패하면 리프레시가 503 으로 죽는다. renameAtomicFile 을 쓰는 편이 안전할 수 있다
  • 합류 비행이 같은 거절 토큰을 줄 때 재귀를 한 번으로 막을지. 막는 편이 맞다
  • Direct kind main 과 채팅 컴플리션 401 재전송을 이번 범위에 넣을지. 지금은 말 것. Responses 메인 풀만 닫혀도 2221 의 핵심은 줄어든다
  • 2221 을 지금 닫을지. 닫지 말 것. 2475 는 여전히 드래프트라 2407 도 닫지 말 것. 2492 는 아직 안 합쳐져 2489 도 닫지 말 것

너의 추천
드래프트로 둔다. 머지하지 않는다. 2221 은 열어 둔다. 윈도우 하드 링크와 재귀 한도를 고친 뒤 보안 리뷰가 끝나면 레디로 올린다. 라벨은 그대로 둔다. 프리뷰 배포가 아니다. types/config 쪼개기 때문에 닫지 말 것.

이 댓글은 grok-bot이 작성했습니다

@lidge-jun

Copy link
Copy Markdown
Owner

Ran the credential-boundary security review AGENTS.md requires for this surface — an independent adversarial reviewer, then my own verification of each finding against the tree. Rebased cleanly onto current dev (1/1) first.

Not merging yet. Three blockers, two of which are ownership decisions rather than defects I should resolve inside your PR.

1. Publication is not atomic (main-account.ts:319). replaceMainAuthJsonWithoutClobbering renames auth.json to a backup, then linkSyncs the temp into place. A crash between the two leaves no auth.json, and nothing recovers the .ocx-main-auth.*.bak at startup. Credit where due: the code does restore the backup on both failure paths it can observe and re-verifies the published snapshot — it is more careful than "non-atomic" suggests. What it cannot cover is process death in that window, or an external writer holding the old inode.

2. The same-account fallback adopts a different grant (main-account.ts:403). freshStoredCredentialForMain correctly looks for the same refresh grant first, then falls back to any fresh pool credential sharing the ChatGPT account id — and writes that pool refresh token into native-main. This is the hazard anthropic-routing.ts fails closed on (canRefreshAnthropicPoolAccount): a background slot's credential must not be adopted into an active one just because the identity matches. Account-id equivalence is not grant ownership. Your test at codex-main-account-refresh.test.ts:544 blesses the substitution, so this looks intentional — which is exactly why it needs the maintainer's call rather than mine.

3. "Exactly one replay" is one logical replay, not one physical send (core.ts:3532). The guard is correctly set before refresh/rebuild/relay on all three paths, and compact explicitly uses single-send mode (compact.ts:647). But passthrough's post-401 send goes through fetchWithTransientRetry, whose ladder is 3 transient × 3 reset. One 401 recovery can be up to nine physical sends. Nothing reaches the client twice, but upstream work can be committed more than once.

4. Your new suites are red at this head1 pass / 9 fail. Not your fault: the fixtures request account-gated gpt-5.6-sol without an authenticated /models roster, and dev's entitlement gate (#2550) landed after you wrote them, so it fails closed before the replay path is reached. Rebasing plus a roster mock in the fixtures should clear it — but it does mean the regressions this PR relies on currently prove nothing.

Non-blocking: persistence preserves the old id_token while reads prioritize its account id over a refreshed access_token/account_id, which can reintroduce a stale account header (main-account.ts:173).

The review is recorded in devlog/_plan/260825_owner_backlog_and_bugpr_closeout/120 (merged as #2602). Happy to help with any of these — I did not want to rewrite a credential PR under someone else's name without your agreement on #2.

lidge-jun added a commit that referenced this pull request Aug 25, 2026
Audited against the goal objective's exact lists rather than a running tally:
all 15 named lidge-jun issues CLOSED, 15 of 16 named bug PRs terminal.

#2497 is the single open item and it is a decision, not a task - the credential
boundary AGENTS.md places under security review, with three hand-verified
blockers, one of which is an ownership question about cross-grant adoption.

Records the four remaining items and the specific input each is blocked on, so
the next person does not re-derive why they are open.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working intake: hygiene-blocked Deterministic PR hygiene checks failed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants