fix(codex): fence refresh failures across bulk routing resets

[Ingwannu]

Summary

• invalidate every in-flight Codex credential refresh when the routing layer bulk-clears account state
• keep per-account reauthentication fencing unchanged while adding a global routing-state generation
• clear obsolete per-account fence entries during the bulk reset
• add the missing pre-reset-flight regression and document the two-level fence contract

Why this follow-up is required

PR #4690 correctly fenced a late refresh failure after an individual credential replacement, but clearAllCodexPoolRefreshFailures() only cleared the backoff map. A refresh flight that started before a whole routing-roster reset could therefore fail afterward with the still-current fence and re-open cooldown for the replacement roster.

This is especially easy to miss for a first failure: that in-flight account has no backoff entry to iterate. The fix uses a global generation in addition to the existing per-account generation, so the bulk reset invalidates unknown in-flight work in O(1).

Decision Log

• Purpose: prevent obsolete refresh failures from re-quarantining accounts after a whole routing-state reset.
• Existing constraint: account-local reauthentication must invalidate only that account, while bulk reset must invalidate flights for accounts not yet present in either state map.
• Alternatives considered: increment known account fences, retain and iterate every seen account, or add a global generation.
• Choice: capture global:account as the opaque flight fence and advance the global component on bulk reset.
• Why: map iteration cannot see a first in-flight failure; retaining every seen account would add unbounded state; a generation is constant-space and constant-time.
• Impact: stale pre-reset failures are ignored, while post-reset failures still count toward the existing bounded cooldown.

Validation

Exact head: a20a4aa46

• bun test tests/codex-integration/codex-pool-refresh-backoff.test.ts --timeout 30000 — 13 pass, 0 fail
• bun run typecheck — pass
• bun run structure:check — pass
• bun run privacy:scan — pass
• full isolated suite is running; hosted exact-head CI is still required

All local validation used temporary HOME, OPENCODEX_HOME, CODEX_HOME, and PASEO_HOME; credential environment variables were removed. The five protected runtime files remained byte-identical.

@lidge-jun This is an urgent narrow follow-up to the blocker identified on the already-merged #4690. Please review the auth-state fence and exact-head CI before merge. I will not self-approve or self-merge it.

Summary by CodeRabbit

• Bug Fixes
  • Improved credential refresh failure handling during routing resets.
  • Obsolete failures are now ignored, preventing stale requests from triggering cooldowns for replacement credentials.
  • Failures recorded after a reset continue to apply the expected bounded cooldown behavior.
  • Improved reliability when credentials are refreshed while routing state changes are in progress.
• Tests
  • Added coverage for bulk failure resets and stale versus current refresh attempts.
  • Verified that current failures continue to trigger cooldowns as expected.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 23547d2e-be09-41cc-90d7-7a2b8c67a427

📥 Commits

Reviewing files that changed from the base of the PR and between 26201d4 and 2853fdc.

📒 Files selected for processing (1)

• structure/transports/responses.md

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

The refresh-failure fencing model now combines per-account and global routing generations. Bulk resets invalidate stale in-flight refreshes. Tests verify that stale failures are ignored and current failures still apply cooldown.

Changes

Refresh failure fencing

Layer / File(s)	Summary
Generation fence implementation src/codex/pool-refresh-backoff.ts	Adds globalFence, returns combined string fences, resets the global generation in test setup, and advances it during clearAllCodexPoolRefreshFailures() (lines 46–51, 61–67, 82–83, 127).
Reset behavior validation structure/transports/responses.md, tests/codex-integration/codex-pool-refresh-backoff.test.ts	Documents account and routing generations. The integration test verifies that pre-reset fences do not open cooldown and post-reset fences still count (lines 237–240 and 224–251).

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to 2853f

The change invalidates stale refresh failures after bulk resets while preserving cooldown handling for current failures, with no unresolved merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: fencing Codex refresh failures across bulk routing resets.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ingw/fix-bulk-refresh-fence

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

✅ Deterministic PR hygiene checks passed.

[lidge-jun]

리뷰 · 우선순위 76 / 80

이 PR은 이미 dev에 들어간 #4690의 계정 단위 refresh 펜스를, 라우팅이 로스터를 통째로 비울 때까지 덮는 follow-up이다. 지금 dev HEAD는 5dfea7235 (#4686으로 패키지 2.57.0 open)이고, 그 직전 tip 386303af1 (#4690)에 계정 펜스와 clearCodexPoolRefreshFailure가 들어갔다. 그런데 clearAllCodexPoolRefreshFailures()는 backoff 맵만 비웠다. src/codex/routing/thread-affinity.ts의 clearThreadAccountMap()이 그 함수를 부른다. 로스터를 갈아엎기 전에 이미 날아간 refresh가, 아직 첫 실패라서 맵에 계정 칸이 없는 채로 나중에 실패하면, 옛 펜스 값과 새 펜스 값이 둘 다 0으로 같아져 쿨다운이 다시 열린다. 첫 실패라서 맵을 돌며 펜스를 올릴 계정도 없다. 그래서 이 PR은 전역 세대(globalFence)를 하나 더 두고, 비행 중인 작업이 잡는 값을 "global:account" 형태의 불투명 문자열로 바꾼다.

코드 경로는 좁다. src/codex/pool-refresh-backoff.ts에서 codexPoolRefreshFence 반환 타입이 number → string이고, noteCodexPoolRefreshFailure의 fence 인자도 같다. 호출부는 src/codex/account-store.ts 한 곳뿐이다. 비행 시작 전에 펜스를 잡고, 실패 then에서 그대로 넘긴다. 타입만 바뀌고 호출 모양은 그대로라 추가 배선은 없다. 계정 재인증 쪽 clearCodexPoolRefreshFailure(accountId)는 예전처럼 그 계정 펜스만 +1한다. 통째 리셋만 fenceByAccount.clear() + globalFence += 1이다. 문서 structure/transports/responses.md에도 계정 세대와 전역 라우팅 세대가 같이 쓴다고 적어 두었다.

테스트 tests/codex-integration/codex-pool-refresh-backoff.test.ts에 "a bulk routing reset fences every in-flight refresh" 케이스가 추가된다. 리셋 전에 잡은 staleFence로 실패를 여러 번 넣어도 쿨링이 안 켜지고, 리셋 뒤 freshFence로 같은 횟수를 넣으면 쿨링이 켜진다. #4690이 계정 교체 레이스를 막았다면, 이번 건은 로스터 통째 교체 + 첫 실패(맵에 칸 없음) 레이스를 막는다. 작성자가 말한 대로 맵 순회로는 첫 비행을 볼 수 없고, 본 적 있는 계정을 전부 남기면 상태가 한없이 커지므로 전역 세대가 맞는 선택이다.

릴리스 맥락도 같이 봐야 한다. 2.56.0 제품 트리는 여전히 386303af1이고 #4694가 그걸 main으로 promote 중이다. 이 PR은 #4686 이후 dev(2.57.0 open) 위에 있다. 그래서 지금 그대로 머지하면 2.56.0 출시 묶음에는 안 들어가고, 2.57.0 트랙 수정이 된다. 구멍 자체는 #4690이 들어간 freeze 트리에도 있다.

pool-refresh-backoff.ts clearAllCodexPoolRefreshFailures - HEAD에서는 backoff만 비워서, 맵에 없는 첫 비행 실패가 리셋 뒤에도 쿨다운을 다시 연다. 이번 전역 세대 + 맵 clear가 그 구멍을 막는다.
codexPoolRefreshFence 반환 타입 number→string - 불투명 토큰으로 바꾼 선택은 맞다. 호출부는 account-store.ts 한 곳이라 파급은 작다.
clearCodexPoolRefreshFailure - 계정 단위 재인증 펜스는 그대로 두고, 통째 리셋만 전역을 올린다. 두 층 계약이 코드와 문서에 맞게 갈라져 있다.
테스트 pre-reset / post-reset - 리셋 전 실패는 무시, 리셋 후 실패는 카운트. 회귀 오라클이 필요한 경계를 직접 고정한다.
#4694 / 2.56.0 promote와의 관계 - 이 수정은 freeze tip 386303af1에는 없다. dev 머지만으로는 이번 출시 묶음에 안 실려 간다.

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

• 2.56.0에 이 구멍을 넣을지: [WRONG BRANCH] release: promote the verified 2.56.0 product tree to main #4694 cherry-pick / 재컷, 아니면 2.56.0은 알려진 위험으로 두고 2.57.0 dev에만 둘지.
• 호스팅 CI가 아직 pending이다. 로컬 suite 정책상 exact-head CI 초록을 기다릴지, CI 도중에 머지할지.
• fix(responses,codex): stop charging a send that never happened, and let a reauthenticated account back in #4690 리뷰에서 남겨 둔 “세대 펜싱은 follow-up”을 이 PR로 닫은 걸로 볼지, 계정 맵 순회 방식의 다른 대안을 더 볼지(작성자 Decision Log는 전역 세대로 이미 닫음).

너의 추천
호스팅 CI가 이 PR head(a20a4aa46)에서 초록이면 dev에 머지. 갓파일/types 분해 PR이 아니라 #4690 펜스 구멍의 좁은 닫힘이다. 2.56.0 promote(#4694)에는 기본으로 안 들어가니, 출시 묶음에 꼭 넣으려면 cherry-pick 여부를 따로 정하고, 아니면 릴리스 노트에 “bulk routing reset 중 in-flight refresh 실패가 쿨다운을 다시 열 수 있음 → 2.57.0에서 수정” 한 줄을 남기면 된다. 코드 자체는 머지해도 된다.

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

[Ingwannu]

Full isolated-suite result at a20a4aa46: 25,195 pass, 22 skip, 2 fail. Both failures are in tests/clients/remote-workspace-command-runner.test.ts and are unchanged on a clean detached origin/dev (5dfea7235): this host rejects process.execPath because one executable parent directory is group/world-writable, so the expected later fixture assertion is never reached. The PR does not touch Remote Workspace files. Focused refresh-backoff regression remains 13/13 green; typecheck, structure, and privacy scans are green. Hosted exact-head CI is still the merge gate.

[Ingwannu]

Rebased onto current dev (cc182a405, including #4568/#4660/#4603). New exact head: 26201d44b. Post-rebase isolated validation remains 13/13 focused plus typecheck, structure, privacy, and diff checks. The protected runtime files remain unchanged. @lidge-jun please review this exact head; it remains an auth-state fence change and I will not self-approve or self-merge it.