Skip to content

test(update): isolate the recovery child Codex home - #3655

Merged
lidge-jun merged 18 commits into
devfrom
codex/provider-ci-isolation-followup
Sep 5, 2026
Merged

test(update): isolate the recovery child Codex home#3655
lidge-jun merged 18 commits into
devfrom
codex/provider-ci-isolation-followup

Conversation

@lidge-jun

Copy link
Copy Markdown
Owner

Summary

  • Give the detached update-recovery fixture its own Codex home using the existing isolated-environment factory. Changing HOME alone previously preserved the runner's explicit CODEX_HOME.
  • Verify negative inheritance, real-home guard preservation, unchanged parent inputs, unmanaged service state and the actual child environment using the production home resolver.
  • Keep runtime selection, bundled dependency, timeouts, diagnostics and teardown unchanged. This fixes a fixture namespace contract; it does not claim the sole cause of historical intermittent startup failures.

Stack: fourth layer after #3582#3584#3598; base codex/provider-quota-parity. The user ended cross-task coordination and authorized independent delivery. Review this test-only layer's diff.

Verification

  • No local tests, typecheck, build, lint or scan commands; new exact-head remote CI is required before merge.
  • Kant independent plan and implementation review: PASS, read-only.
  • Commits and pushes use --no-verify. Original preparation commit b378414 was local-only; its later integrated PR head is the CI authority.

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.

Maintainer authorization covers conditional admin merge after successful exact-head CI. No service restart, release or user-account mutation is included.

t added 16 commits September 5, 2026 12:23
…uota-parity

# Conflicts:
#	gui/src/pages/Providers.tsx
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 5, 2026 11:36
@coderabbitai

coderabbitai Bot commented Sep 5, 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: Team

Run ID: 4e48e498-6a97-45ec-a3d0-4ef90b2686de

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 5, 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-05T11:39:44.861999Z d7e67e9 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

github-actions Bot commented Sep 5, 2026

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 5, 2026
@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 56 / 80

이 PR은 제품 코드를 바꾸는 게 아니라, 업데이트 복구(fixture) 테스트가 부모 테스트 러너의 Codex 집을 그대로 물려받던 구멍을 막는 작업이다. 지금 dev HEAD(45f3bed84, #3645 pending selection write fence) 기준으로 src/codex/home.tsresolveCodexHomeDir은 환경변수 CODEX_HOME이 있으면 그걸 최우선으로 쓴다. 예전 복구 fixture는 HOME / USERPROFILE / OPENCODEX_HOME만 바꾸고 ...process.env를 그대로 펼쳤기 때문에, CI나 로컬에서 이미 CODEX_HOME이 잡혀 있으면 분리된 임시 루트가 있어도 자식이 부모 Codex 네임스페이스를 공유할 수 있었다. 이번 패치는 이미 있는 scripts/test.tscreateIsolatedTestEnvironment를 재사용해서 케이스 루트 아래에 .codex / .opencodex를 만들고 CODEX_HOME까지 명시적으로 덮어쓴다. 베이스는 dev가 아니라 스택 브랜치 codex/provider-quota-parity(본문 기준 #3582#3584#3598 위 네 번째 레이어)다. 프로덕션 업데이트 경로·포트 프로브·타임아웃·진단·teardown 순서는 건드리지 않았고, 간헐 시작 실패의 원인 단정도 하지 않는다. 그래서 지금 dev가 밀고 있는 모델 선택/쿼터 제품 축과는 직접 겹치지 않지만, 업데이트 복구 CI가 러너 환경에 오염되지 않게 하는 계약 수정이라 스택 안에서 가치는 분명하다.

변경 파일은 두 개뿐이다. tests/update/update-stop-first.test.ts에 (1) parentEnv에 가짜 CODEX_HOME을 넣고 격리 팩토리 결과가 join(root, ".codex")인지, OCX_REAL_HOME 보존, 부모 입력 불변, service-state.json 부재를 보는 단위 테스트가 추가됐고, (2) Windows에서 스킵되던 npm launcher 복구 통합 테스트가 mkdtempSync+수동 env 대신 createIsolatedTestEnvironment()isolated.env를 쓰며, 스폰 전에 생산용 resolveCodexHomeDir({ env })로 실제 자식 환경을 한 번 더 검증한다. devlog/_plan/260905_provider_usage_quota_parity/014_recovery_fixture_isolation.md는 결함 범위(픽스처 격리)와 비범위(과거 flaky 원인 단정 금지, 로컬 검증 미실행, exact-head CI 필요)를 적어 둔다. 이 방향은 resolveCodexHomeDir의 명시 CODEX_HOME 우선 규칙과 맞고, 격리 팩토리가 이미 CODEX_HOME: join(root, ".codex")를 세팅하는 기존 계약과도 일치한다.

라인 498 근처 신규 단위 테스트 - 격리 계약(resolveCodexHomeDir / OCX_REAL_HOME / 부모 불변 / service-state 부재)은 잘 잠그지만, 실제 Bun.spawnSync 복구 경로를 돌리지는 않는다. 통합 테스트의 전제 조건을 분리해 둔 것이고, “복구 자식이 살아 동작한다”까지 증명한 건 아니다.
라인 609–632 (복구 통합 테스트 env 교체) - ...process.env + HOME만 덮던 패턴을 ...isolated.env로 바꾼 핵심 수정이다. OPENCODEX_HOME도 팩토리가 만든 .opencodex를 쓰므로 예전 opencodex-home 수동 mkdir과 경로가 달라졌는데, 설정 파일을 그 경로에 쓰는 후속 코드와 맞는지 CI에서 한 번 확인이 필요하다.
014_recovery_fixture_isolation.md - 본문 “in040”처럼 잘린 참조 표기가 보인다. 계획 문서 가독성만의 문제라 머지를 막을 정도는 아니다.
베이스 codex/provider-quota-parity - 타깃이 dev/preview가 아니라 스택 중간이다. CodeRabbit도 그 이유로 자동 리뷰를 건너뛰었다. exact-head CI가 이 헤드에서 초록이어도, 하위 레이어(#3582/#3584/#3598 등) 머지 순서 없이 dev에 단독 랜딩하면 안 된다.
Windows - 복구 통합 테스트는 여전히 test.skipIf(process.platform === "win32")다. 이번 PR 범위 밖이지만, 픽스처 격리가 Windows AppData 샌드박스 경로까지 검증되는지는 이 변경만으로는 증명되지 않는다.

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

  • 이 레이어를 스택 순서대로 codex/provider-quota-parity 위에 올린 뒤 dev로 올릴지, 테스트 전용 커밋만 체리픽/dev 리타깃할지
  • exact-head CI(특히 update/복구 관련 shard) 통과만으로 admin merge 할지, 로컬에서 해당 파일 스모크를 한 번 더 볼지
  • 간헐 시작 실패 조사(포트 프로브 등)와 이 픽스처 PR을 계속 분리할지, 같은 트레인에 묶을지

너의 추천
exact-head Cross-platform CI가 이 헤드(d7e67e9 기준)에서 초록이면, 스택 베이스가 준비된 뒤 테스트 전용으로 머지해도 된다. 제품 회귀 위험은 낮고 픽스처 계약은 resolveCodexHomeDir과 맞다. CI 끝나기 전 머지는 하지 말고, dev 직행 리타깃은 하위 레이어 없이 하지 말 것.

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

@lidge-jun
lidge-jun changed the base branch from codex/provider-quota-parity to dev September 5, 2026 12:11
@lidge-jun
lidge-jun merged commit ba9a455 into dev Sep 5, 2026
24 of 42 checks passed
@lidge-jun
lidge-jun deleted the codex/provider-ci-isolation-followup branch September 5, 2026 12: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.

1 participant