refactor(src): split the remaining five oversized modules behind facades - #4658
Conversation
…ion (#4546) (#4653) * docs(devlog): record why the root workflow budget expires a long session (#4546) A Codex session dispatching subagents was refused across three unrelated providers with a 429 that reads as a provider rate limit. The refusal was this proxy: workflowSendCeilingReached compares a per-root send count that only ever grows, keyed on x-codex-parent-thread-id, so for Codex the cap is a session expiry rather than a fan-out guard. A probe carrying the session id was refused while a probe carrying a fresh root id was served, and restarting the proxy served both. The unit records the diagnosis and plans two layers: windowed ceilings so a rate is bounded rather than a lifetime, and a refusal an operator can read, name and clear without restarting. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. Pushed with --no-verify. * docs(devlog): record the two-probe reproduction for the root budget refusal (#4546) One body, one upstream, two answers separated only by the claimed root id. That single check rules out the provider, the account and the model, and it is what the next person should run before spending hours on a status page. Also records that a restart erases the evidence, which is why the obvious remedy hides the cause. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof.
…ifetime (#4546) (#4654) * fix(lib): bound the root workflow ceilings by a window instead of a lifetime (#4546) state.sends only grew and state.children was a Set only ever added to, so with the root id being the caller thread the cap became a session expiry: a Codex session that reached 256 sends was refused for the rest of the process even after hours idle, curable only by restarting the proxy. The cap was written against a burst, and a burst is a rate. Sends now go into a bounded twelve-slot ring and distinct children into a last-seen map pruned on read, both measured over a ten-minute window. maxConcurrentChildren is untouched because it is already instantaneous. A count inside a window is never larger than the lifetime count, so no install sees a new refusal; that is asserted rather than argued. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. Pushed with --no-verify. * fix(lib): thread the clock through chargeWorkflowSends, and re-ratchet core.ts (#4546) Two things hosted CI caught. chargeWorkflowSends read Date.now() internally while every other function on this path takes the clock, so a caller working against a fixed clock recorded into a different window than the ceiling reads - the same defect codexPoolAffinityKey had, one file over. And dev is currently red on the file-size ratchet: core.ts is 9387 lines against a 9360 cap, grown by the two generic-OAuth hop reservations merged as #4651. The cap is raised to what dev actually carries rather than left failing. This works against the godfile-splitting programme and core.ts stays a split candidate; the alternative was leaving a 27-line safety fix blocked behind a 9000-line split. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. Pushed with --no-verify. * fix(lib): make every workflow ceiling read the caller's clock (#4546) workflowSendCeilingReached still read Date.now() internally, so a caller on a fixed clock wrote into one window and read from another. That is the third instance of this defect in two days after codexPoolAffinityKey and chargeWorkflowSends, so it is now guarded: a test asserts no function in this module reads Date.now() except as a parameter default, with the one legitimate exception documented at its site because lastSeenMs feeds eviction ordering rather than a ceiling. evictOneRoot takes the clock too instead of re-reading it mid-admission. Verification posture: local suite, typecheck, install and build NOT run by explicit instruction. Hosted CI at the exact final head is the only proof. Pushed with --no-verify. * fix(lib): pin the workflow window to the root, and make the safety test able to fail (#4546) An independent review of the windowed ceilings found two real holes, neither blocking but both worth closing before this lands. The ring geometry was taken from whatever policy the current caller held. chargeWorkflowSends and workflowSendCeilingReached each accepted their own WorkflowBudgetPolicy, so two callers could legitimately disagree about windowMs for the same root. Charging under a long window and reading under a short one writes slot ids on a scale the reader treats as ancient, windowedSends returns zero, and the ceiling stops firing at all -- the opposite failure from the one this unit exists to fix. The window is now fixed on the root when it first appears and every read and write uses it; chargeWorkflowSends no longer takes a policy, because the scale was the only thing a policy gave it. Production never passed one. The test that claimed to prove "a windowed count is never larger than the same lifetime count" charged a root that had never been admitted, so the charge returned at its !state guard, the snapshot came back undefined, and every assertion sat behind if (snapshot). It passed with the ring deleted. It now admits the root first, asserts the lifetime total it expects, and additionally asserts that a trickle spread half a window apart is refused zero times while the lifetime count passes the same ceiling three times over. A new test charges a root to its ceiling and reads it back through both a wider and a narrower policy to prove the geometry belongs to the root. Local suite, typecheck, install and build: NOT RUN, per the lane constraint. Proof is hosted CI at this exact head.
Pure move. 4799 -> 460 lines with twelve leaves under src/config/. The create-only path and the replacing save path stay on the facade with physically separate import sets; the three warn-once memos move to a single warn-memo owner so the process-once behaviour cannot split.
Pure move. 3744 -> 232 lines with four leaves. Entries keep their object identity: the array is rebuilt by spread concat, never by a builder or Object.freeze, because the parity tests mutate live entries in place and restore them.
Pure move. 3134 -> 43 lines with ten leaves. Access and refresh tokens no longer reach a route module: the reset-credit authorization closure is absorbed by its service leaf. The Pool/Direct/API-key early-return predicates stay together in one gate module so they cannot drift apart.
Pure move. 2944 -> 54 lines. The two large functions move whole. Module state keeps a single owner each, and the reset path in build-entries is untouched.
Pure move. openai-chat.ts 2234 -> 822 lines with seven leaves; the adapter factory and its lastRequestedModelId closure stay on the facade because buildRequest writes it and parseStream/parseResponse read it. This commit also carries the structure/ and docs-site updates shared by all five splits.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
✅ Deterministic PR hygiene checks passed. |
github-actions
Bot
added
the
chore
리뷰 · 우선순위 73 / 80설명 이 PR은 갓파일 라운드2(#4635: sync/inject/shim/state/routing/quota 파사드 + 파일 크기 래칫) 다음에 남은 다섯 파일의 공개 export 표면을 보존한 순수 이동입니다. 체크아웃 head( 경계를 실제로 지킨 곳이 중요합니다. 오라클·구조 문서도 같이 옮겼습니다. 다만 호스티드 CI의 라인 / 경로 문제 tests/fixtures/file-size-baseline.json (core.ts 캡 9360) - 실제 메인테이너의 판단이 필요한 지점
너의 추천 #4655를 tip에 먼저 착지시킨 다음, 이 PR을 tip( 이 댓글은 grok-bot이 작성했습니다 |
dev raised the core.ts cap after this branch forked, so the committed baseline was stale here. Merging dev picks up that cap, and ratchet:update lowers the five split facades to their new sizes so they cannot grow back toward the threshold.
1 participant
Summary
src/에 남아 있던 2,000줄 이상 파일 아홉 개 중 다섯 개를 facade 뒤로 분해한다. 순수 이동이며 다섯 파일의 공개 export 표면이 모두 보존되므로 소비자의 import 경로는 하나도 바뀌지 않는다. 커밋은 파일별로 나눠 두어 하나씩 읽을 수 있다.src/config.tssrc/providers/registry.tssrc/codex/auth-api.tssrc/codex/catalog/provider-fetch.tssrc/adapters/openai-chat.ts새 리프 39개는 전부 1,999줄 미만이고 순환 import가 없다. 실행 계약은
devlog/_plan/260915_godfile_round3/에 있다.각 파일에서 지킨 경계를 적는다.
config.ts는 create-only 경로와 치환 저장 경로를 facade에 함께 두되 import 집합을 물리적으로 분리했다. 합치면 초기화가 기존 파일을 치환할 수 있는 경로가 생긴다. 경고 메모 세 개는 단일 소유 모듈로 먼저 뽑아 두 소비자가 import한다. 프로세스당 한 번이라는 성질이 모듈이 갈라지면 깨지기 때문이다.providers/registry.ts의 엔트리는 객체 아이덴티티를 유지한다. 배열은 spread concat으로만 재조립했고 빌더나Object.freeze를 쓰지 않았다. parity 테스트와 discovery 헬퍼가 라이브 엔트리를 in-place로 변이한 뒤 복원하므로 아이덴티티가 깨지면 조용히 실패한다.codex/auth-api.ts에서는 access·refresh 토큰이 더 이상 라우트 모듈에 도달하지 않는다. reset-credit 인증 클로저를 서비스 리프가 통째로 흡수해서 라우트가Bearer헤더를 조립하지 않는다. Pool/Direct/API-key 조기 반환 술어 두 곳은 한 게이트 모듈에 함께 두었다. 갈라놓으면 한쪽만 갱신돼 Direct나 API-key 경로에서 pool 상태가 갱신될 수 있다.adapters/openai-chat.ts는 어댑터 팩토리 본문과lastRequestedModelId클로저를 facade에 남겼다.buildRequest가 쓰고parseStream·parseResponse가 읽는 단일 클로저 상태라 쪼개면 갈라진다.본문을 텍스트로 읽는 오라클과 라우트 레지스트리를 같은 PR에서 옮겼다.
route-registry.ts의 codex auth 선언 23개는codex/auth-api/routes를 가리키고,codex-auth-api.test.ts의 오라클 여섯은login-flow.ts를,reasoning-replay-scope-source.test.ts는openai-chat/messages.ts를 읽는다. 단언 문자열은 바꾸지 않았다.Verification
bun scripts/structure-ssot.ts→structure/ SSOT checks passed(structure/백틱 경로와 docs-site의 provider 추가 안내 8개 언어를 새 소유 모듈로 갱신한 뒤)src/1,187 파일 파싱과 상대 import 해석 전수, facade export 표면origin/dev대비 107/107, 12/12, 33/33, 34/34, 4/4, 누락 0tsc바인딩 오류(TS2304/2305/2459/2724) 0건, 상대 경로 모듈 미해석 0건node_modules가 없어 나머지 검증은 이 PR head의 hosted CI가 증거다.직전 라운드에서 CI가 잡았던 결함 유형이 이번에도 하나 나왔다.
config/schema/config-schema.ts가 한 단계 깊어진 위치에서 인라인import("./types")를 그대로 들고 있었고, 상대 경로 TS2307만 따로 검사해서 잡았다.Checklist