refactor(responses,codex): split state and shim behind facades - #4642
Conversation
Pure move. state.ts 2432 -> 1355 with five leaves under src/responses/state/, shim.ts 2466 -> 1246 with six shim-* leaves. Public export surfaces are byte-identical in name; consumers keep their import paths.
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. |
|
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 |
github-actions
Bot
added
chore
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 913e0d071f
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| import { isWslRuntime, wslAutomountRoot } from "./home"; | ||
| import { truncateRetainedUtf8 } from "../lib/admission"; | ||
| import { | ||
| buildUnixCodexShim, | ||
| buildWindowsCodexShim, |
There was a problem hiding this comment.
This moves substantial shim responsibilities into six new modules, but the commit changes none of the documents mapped to src/codex/ in structure/INDEX.md; for example, structure/runtime.md still attributes the Codex autostart shim solely to src/codex/shim.ts. Update the mapped structure documents so maintainers can locate the new owners for templates, fingerprints, state, probing, locking, and inspection.
AGENTS.md reference: src/AGENTS.md:L10-L11
Useful? React with 👍 / 👎.
| export type { ResponseStateTempRecoveryResult, ResponseStateTempRecoveryOptions } from "./state/temp-recovery"; | ||
| export { recoverStaleResponseStateTemps, reclaimAbandonedResponseStateTemps, inspectAbandonedResponseStateTemps, sweepAbandonedResponseStateTemps } from "./state/temp-recovery"; | ||
| import { recoverStaleResponseStateTemps } from "./state/temp-recovery"; | ||
| export type { ResponseSpillWriteFailureCode, ResponseSpillWriteStatus, ResponseSpillWriteFailureOrigin } from "./state/spill-failure"; |
There was a problem hiding this comment.
The response-state implementation is redistributed across five new leaf modules without updating any structure document mapped to src/responses/, leaving the repository's architecture and ownership references unaware of the new queue, recovery, codec, failure-state, and replay-fingerprint owners. Update the mapped structure documents as part of this source-area change.
AGENTS.md reference: src/AGENTS.md:L10-L11
Useful? React with 👍 / 👎.
✅ READY
Hygiene✅ Deterministic PR hygiene checks passed. |
리뷰 · 우선순위 42 / 80이 PR은 제품 동작을 바꾸려는 기능이 아니라, 너무 커진 두 godfile을 파사드 + 리프 모듈로 나누는 정리입니다. 지금 우선순위 숫자는 「기능 가치」보다 「스택·게이트 상태」를 반영했습니다. 분해 방향 자체는 m2k 로드맵과 맞고, SSOT/래칫을 로컬에서 통과했다고 적혀 있습니다. 다만 CI는
모듈 스코프 가변 상태 소유권 - ESM live binding 때문에 파사드가
메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
Pure move. inject.ts 2342 -> 987 with five leaves, catalog/sync.ts 2698 -> 52 with seven leaves. INV-TOML-01 moves to inject/config-toml.ts and INV-AGENT-01 to catalog/subagent-roster.ts. Three source oracles that read these files as text are repointed in the same commit.
lidge-jun
added
the
test-exception-approved
github-actions
Bot
removed
the
intake: hygiene-blocked
Pure move. routing.ts 3507 -> 1475 with six leaves, quota.ts 3313 -> 558 with five leaves. Dispatchers that would close a cycle stay on the facade. Retry budget scope is unchanged; no new attempt counter exists in any leaf.
Three identifiers lost their binding when the leaves were cut: the spill write-status types were re-exported from state.ts but never imported for local use, snapshot-codec lost OcxProviderContinuationState, and spill-queue lost existsSync. Caught by the translator-budget typecheck fixture on CI.
github-actions
Bot
added
intake: hygiene-blocked
The inject and catalog leaves imported six symbols from modules that never exported them. Each one is re-pointed at where it is actually defined: parsing, account-models, subagent-roster, paths, desired-state. Import paths only; no declaration moved.
…ta split Fifteen symbols lost their binding: leaves that defined a symbol never exported it, quota type imports pointed at src/types instead of providers/quota-types, and isModelDetourAffinityScope lost its definition entirely while its call site survived. Imports and exports only; no declaration was moved or rewritten.
lidge-jun
added
the
test-exception-approved
github-actions
Bot
removed
the
intake: hygiene-blocked
The leaf sits one directory deeper than routing.ts, so ../config resolved to src/codex/config, which does not exist. Every test shard that loaded the routing graph failed at import time.
refactor(codex,providers): split routing and quota behind facades
refactor(codex): split inject and catalog sync behind facades
github-actions
Bot
removed
the
test-exception-approved
1 participant
Summary
src/responses/state.ts와src/codex/shim.ts를 파사드로 남기고 리프 모듈로 나눈다. 순수 이동이며 로직·식별자·문자열을 바꾸지 않았다. 두 파일의 공개 export 표면은 이동 전과 동일하므로 소비자의 import 경로는 하나도 바뀌지 않는다.src/responses/state.tsstate/replay-fingerprint.ts80,state/snapshot-codec.ts103,state/spill-failure.ts118,state/temp-recovery.ts257,state/spill-queue.ts664src/codex/shim.tsshim-templates.ts265,shim-fingerprint.ts222,shim-state-file.ts151,shim-probe.ts367,shim-restore-lock.ts169,shim-inspect.ts175모듈 스코프 가변 상태는 소유 모듈 하나에만 둔다.
states, 바이트 회계,stateRevision,loaded,persistTimer,replayOverlapSkips는 파사드가 계속 소유하고 큐는 store 핸들로 접근한다. ESM live binding은 밖에서 쓸 수 없어서 소유권을 옮기면 조용히 갈라지기 때문이다. 스필 카운터는spill-failure가 소유하고responseStateMetrics가 getter로 읽는다.shim.ts에서writeShim과findWindowsCodexTargets는 남겼다.tests/codex-integration/codex-shim.test.ts가 이 파일 원문을 텍스트로 읽어 BOM+PowerShell 빌더 호출,const gitBashLauncher = join(dir, "codex");,for (const path of [cmd, ps1, gitBashLauncher])를toContain으로 검사한다. 빌더 정의만shim-templates.ts로 옮겼고 호출부는 그대로다.lastShimDiscoveryError를 탐색부와 설치부가 양쪽에서 쓰기 때문에findCodexOnPath도 남겼다.Verification
bun scripts/structure-ssot.ts→structure/ SSOT checks passedbun scripts/file-size-ratchet.ts→file-size ratchet passedBun.Transpiler구문 검사 15개 파일 통과origin/dev기준):shim.ts23 → 23,state.ts38 → 38, 누락 0node_modules가 없고 사용자 환경에서 스위트를 돌리지 않는 제약이 있어, 타입 해석과 런타임 회귀는 이 PR head의 hosted CI가 증거다.알려진 동작 차이 하나를 남긴다.
clearResponseStateMemoryForTests가 이제resetSpillQueueForTests()를 거치면서 메모리 리셋 시에도 큐 바이트 회계가 0으로 내려간다. 원본은 full clear에서만 그랬다. 테스트 전용 헬퍼라 런타임 경로에는 영향이 없지만 CI에서 확인이 필요하다.Checklist