Skip to content

refactor(oauth): move the pool rotation kernel out of the Codex namespace - #4279

Merged
lidge-jun merged 3 commits into
devfrom
codex/pool-kernel
Sep 11, 2026
Merged

refactor(oauth): move the pool rotation kernel out of the Codex namespace#4279
lidge-jun merged 3 commits into
devfrom
codex/pool-kernel

Conversation

@lidge-jun

Copy link
Copy Markdown
Owner

Summary

Moves the account-pool rotation primitives out of the Codex namespace so every credential kind can share them, and closes a sweep hole the move exposed.

`src/codex/pool-rotation.ts` never contained anything Codex-specific: it imports two types and nothing else. But living under `src/codex/` made it look like Codex property, which is part of why the generic OAuth kind grew a parallel implementation instead of calling it. This PR moves the whole module, state map included, to `src/oauth/pool-kernel.ts`.

`src/codex/pool-rotation.ts` stays as a re-export shim. That is deliberate: the move is behaviour-preserving, and the shim means `routing.ts`, `auth-api.ts`, `account-priority.ts` and `state-store-registrations.ts` keep their existing import path and are not touched at all.

The move surfaced a real defect. `reconcilePoolRotationState` recognised only `codex`, `codex:` and `anthropic` pool keys and skipped everything else as unknown, so any other pool key would keep its rotation cursor forever after the account behind it was removed. This PR adds `genericPoolKey(provider)` and teaches the sweep to resolve `generic:` from the `provider\0id` roster the Anthropic pass already walks, so no new context field is needed.

No behaviour changes for Codex or Anthropic. This is the extraction step; the generic kind starts consuming its persisted `strategy` in the layer above, behind the `pool.kernel` flag.

Design and audit record: `devlog/_plan/260911_account_pool_unification/020_phase2_shared_kernel.md` (PR #4275). Two audit rounds before any code: the first rejected lifting the Codex fill-first copy, because that would have edited a file another work stream owns, and rejected dropping the generic `inert` flag without a kill switch. Both are folded into the design this PR implements the first half of.

Verification

  • `bun x tsc --noEmit` - pass, which is also the proof the re-export shim covers every existing importer
  • `bun test tests/codex-integration/codex-pool-rotation.test.ts tests/oauth/generic-oauth-failover.test.ts` - 105 pass, 0 fail (unchanged behaviour through the shim)
  • `bun test tests/oauth/pool-kernel-generic-sweep.test.ts` - 4 pass, 0 fail (new: a live generic entry survives, a stale one is dropped, and one provider's roster does not sweep another's)
  • `bun test tests/test-layout.test.ts tests/test-layout-tooling.test.ts` - 17 pass, 0 fail; the new test is registered in both `scripts/test-layout/layout.json` and `tests/fixtures/test-layout-expected.json`
  • `bun run privacy:scan` - passed

Checklist

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

@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 11, 2026 12:50
@coderabbitai

coderabbitai Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (2)
  • ^dev$
  • ^preview$

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: ce33d914-35f1-48c3-8643-00283327a59c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-11T12:53:58.220430Z e96f0eb PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature). label Sep 11, 2026
@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 68 / 80

이 PR은 계정 풀 통합 유닛의 2단계 중 커널 추출만 먼저 떼어 온 작업이다. 지금 dev(HEAD bd1864905, #4276 구조 SOT 게이트 위, package 2.52.0)에는 회전 상태와 pick/peek/seed/reconcile이 전부 src/codex/pool-rotation.ts 안에 있다. 파일은 Codex·Anthropic 두 종류만 알고 있고, reconcilePoolRotationStatecodex / codex:* / anthropic만 쓸어 간다. 그래서 제네릭 OAuth 쪽이 나중에 같은 커서를 쓰려면, 지금 구조로는 “Codex 폴더 안의 공용 코드”처럼 보이거나, 쓸어 주지 않은 키가 영원히 남는 구멍이 생긴다.

이 변경은 그 공용 코드를 src/oauth/pool-kernel.ts로 옮기고, src/codex/pool-rotation.tsexport * from "../oauth/pool-kernel" 호환 셸로 남긴다. 그래서 routing.ts, auth-api.ts, account-priority.ts, state-store-registrations.ts, anthropic-routing.ts 같은 이미 다른 레인/작업이 건드리는 import 경로를 이번 PR에서 다시 쓰지 않는다. 실제로 dev에서 그 파일들은 여전히 ../codex/pool-rotation을 가리키고, 이 PR도 그걸 유지한다. Codex/Anthropic 선택 결과는 그대로 두고, 옮기면서 드러난 구멍만 막는다: genericPoolKey(provider)generic:이름 키를 만들고, reconcile이 oauthAccountKeysprovider\0id 명단으로 그 키도 쓸어 가게 한다. 새 컨텍스트 필드는 없다.

왜 지금 dev 기준으로 점수가 높은가. 메인테이너가 열어 둔 260911_account_pool_unification / L3 계정 풀 레인의 실제 첫 코드 조각이고, 제네릭 쪽이 이미 strategy를 저장만 하고 inert: true인 상태(pool-settings-capability.ts)에서, 다음 레이어가 커널을 붙일 자리를 만든다. 검증도 좁고 명확하다. 기존 codex-pool-rotation + generic-oauth-failover 회귀, 새 pool-kernel-generic-sweep 4케이스, test-layout 등록, tsc. CI도 하이진/도커/키링 쪽은 이미 통과 중이고 테스트 샤드만 아직 돌고 있다.

다만 이 PR만으로는 dev에 바로 못 얹는다. base가 dev가 아니라 codex/key-pool-strategy다. 스택은 대략 #4275(codex/pool-unify-roadmapdev, 문서) → #4277(codex/key-pool-strategy → 그 위, 워 API 키/설정) → 이번 #4279(codex/pool-kernel). 그리고 phase2 문서(020_phase2_shared_kernel.md)가 원래 같이 말했던 제네릭 strategy 소비, Anthropic import 스왑, 공유 pickFillFirst 헬퍼는 이 diff에 없다. PR 본문이 말한 대로 “추출 절반”이다. 그래서 우선순위는 유닛 자체로는 높지만, 단독 머지 가능 점수는 아니다.

라인 12 - genericPoolKey는 제공자 이름을 그대로 붙인다. 빈 문자열·대소문자·별칭이 섞이면 generic: 키와 provider\0id 명단이 어긋날 수 있다. 다음 소비 PR에서 호출부가 쓰는 이름과 store 키를 한곳으로 고정해야 한다.
라인 282-284 - reconcile 루프에서 anthropic 계정은 anthropicIds로만 넣고 genericIds에는 안 넣는다. 그래서 실수로 genericPoolKey("anthropic")를 쓰면 명단이 비어 보여 상태가 통째로 지워지거나, 반대로 기대와 다른 쓸기가 된다. 테스트가 네임스페이스 분리는 증명하지만, 호출 규약은 아직 문서/타입으로 잠그지 않았다.
라인 299-302 - generic: 인데 해당 provider 계정이 0개면 new Set()으로 보고 엔트리 전체를 지운다. 의도는 맞다. 다만 아직 어떤 런타임 경로도 generic: 키를 seed하지 않으므로, 이 쓸기 분기는 다음 소비 PR이 오기 전까지는 사실상 죽은 방어 코드다. 추출과 소비를 나눈 대가이다.
020_phase2_shared_kernel.md / 이 PR 범위 - 문서 phase2는 커널 + 제네릭 소비(+ Anthropic 쪽 헬퍼 스왑)를 한 사이클로 적었는데, 실제 PR은 커널·쓸기·shim만 담았다. 계획과 diff 범위가 어긋난 것은 버그는 아니지만, 다음 PR 제목/베이스를 미리 고정하지 않으면 스택이 더 길어질 수 있다.
베이스 codex/key-pool-strategy - dev 직접 타겟이 아니다. #4275·#4277이 먼저 안정적으로 쌓여야 하고, 지금 mergeable_state는 unstable(테스트 샤드 pending)이다. 하이진만 보고 머지하면 안 된다.

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

너의 추천
스택 아래(#4275 문서, #4277)와 CI 테스트 샤드가 초록이 된 뒤, 이 PR은 커널 추출 단독으로 머지해도 된다. 제네릭 소비·pickFillFirst·Anthropic 헬퍼 스왑은 바로 이어서 pool.kernel 플래그 뒤의 다음 PR로 고정하고, 이 댓글의 “죽은 쓸기 분기”를 그 PR의 첫 통합 테스트로 연결하자. types/config 대형 분할 캠페인과는 겹치지 않으니 close-don't-rebase 대상은 아니다. 라벨 교체는 하지 말 것.

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

@lidge-jun
lidge-jun changed the base branch from codex/key-pool-strategy to dev September 11, 2026 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants