fix(codex): repair desktop restart membership and POSIX-only cases on Windows

[luvs01]

Current ordinary-gate verification

All ordinary jobs in CI 34812947370 passed on published head f79c14730986a9e63f2e51cdeeea099d13ec4ea3: four Linux shards, two normal macOS shards, static/storage/API gates, three-platform keyring, Docker and packaging. The supplemental macOS control suite and aggregate have also completed successfully. The full Windows diagnostic suite was intentionally skipped, which is not Windows test proof. Author readiness uses this complete ordinary graph together with 27 passing focused local tests (10 platform skips, 69 assertions); it does not claim a successful full local command. This current status supersedes pending-CI prose below; earlier command failures and coverage limitations remain recorded. Required merge checks and new review findings remain independently enforced.

Current author follow-up

Published head f79c14730986a9e63f2e51cdeeea099d13ec4ea3 is based on the fixed dev snapshot 43f4450a538d729f353144c029aa97de1c4f2483. This supersedes older head/behind/CI status statements below; older verification remains historical evidence. Required conflicts were resolved without dropping upstream contracts. No repeated tip chasing was performed.

• bun run typecheck: passed.
• bun run structure:check: passed.
• bun run privacy:scan: passed.
• git diff --check: passed.
• bun test tests/clients/desktop-app-restart-posix.test.ts tests/clients/desktop-app-restart.test.ts: passed (27 pass; 10 skip; 0 fail; 69 expect() calls).

Exact-head CI 34812947370 is pending. The existing macos-control manual lane retains ordinary Linux/macOS/static/storage/API/keyring/packaging checks and the macOS control suite; it intentionally skips the supplemental full Windows matrix. A skipped job is not a Windows pass. Current full-suite readiness remains pending until its result is reviewed.

Maintainer sponsorship, where required, remains a separate hold. Existing resolved review findings were not reopened.

Summary

Every Windows CI shard that picks up tests/clients/desktop-app-restart-posix.test.ts currently fails, so contributor PRs based on recent dev cannot go green. Nine cases fail for two separate reasons, and only one of them is a test-only concern.

isUnderRoot builds its boundary from path.sep alone, so on Windows a member path written with forward slashes is read as outside the install root. Windows accepts / wherever it accepts \, and a probe can return either, so the restart the user asked for could silently become a no-op while still looking fail-closed. Accept / as a separator on every host and keep \ a separator only where the host says so, because \ is an ordinary filename character on POSIX and admitting it there would reopen the sibling-prefix hole the function exists to close.

The remaining cases drive the macOS and Linux adapters, which scope enumeration to the current user through process.getuid(). A Windows host does not provide it, so the probe correctly reports that it could not run and the assertions cannot hold there at all. Those two describes now skip on Windows with that reason recorded; the Ubuntu and macOS shards, where the adapters actually run, still cover them.

Verification

• Before the change, the file failed 9 of 17 cases on a Windows host, matching the Windows 5/6 shard failure observed on CI. After it, tests/clients/desktop-app-restart-posix.test.ts and tests/clients/desktop-app-restart.test.ts together report 27 pass, 10 skip, 0 fail, 69 assertions.
• A new membership case pins the separator rule in both directions, including that a backslash stays a filename character on POSIX.
• bun run typecheck, bun run structure:check, bun run privacy:scan, and git diff --check pass.
• POSIX behavior is unchanged by construction: with sep of /, the new boundary admits exactly the strings the previous prefix comparison admitted. The Windows-only skips were verified as skipped rather than silently passing.

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.

Review readiness evidence

Rebased onto dev at 15fbd49f3 and pushed; the branch was 0 commits behind at push time. Published head is e696a223e.

Local verification on that head, on Windows, which is the platform this fixes: bun test tests/clients/desktop-app-restart-posix.test.ts reports 8 pass / 10 skip / 0 fail, where the skips are the two uid-dependent describes this PR marks POSIX-only. bun run typecheck, bun run structure:check, bun run privacy:scan and git diff --check all pass.

Hosted cross-platform CI on the previous head e4696273a, run 34792398505: 24 jobs green with no failures, including windows 1/6 through windows 5/6. The remaining windows 6/6 job was cancelled by the rebase push that produced the current head, not failed. A fresh full run on e696a223e, run 34793832912, is queued; its result will be posted here.

Review readiness checklist

• All CI tests are green on my local testing.
• I pushed my PR to the latest dev commit.
• I resolved all correct Codex and CodeRabbit findings.
• My PR is ready for review.

Summary by CodeRabbit

• Bug Fixes

  • Improved path membership checks to prevent similarly named paths from being treated as children of a root path.
  • Preserved correct separator handling across POSIX and Windows systems.

• Tests

  • Added coverage for platform-specific path separators.
  • Updated platform-dependent test execution to skip unsupported Windows scenarios.

[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: 8c2b28e7-246f-4ff4-9685-0e245021cdac

📥 Commits

Reviewing files that changed from the base of the PR and between 2e6a031 and f79c147.

📒 Files selected for processing (2)

• src/codex/desktop-app/types.ts
• tests/clients/desktop-app-restart-posix.test.ts

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

---

📝 Walkthrough

Walkthrough

The change makes isUnderRoot use platform-aware separator checks. It prevents POSIX sibling paths from matching a root prefix. Tests cover separator behavior and skip POSIX-only suites on Windows.

Changes

Root membership validation

Layer / File(s)	Summary
Platform-aware root membership logic src/codex/desktop-app/types.ts	isUnderRoot checks the character after the root. It treats / as a separator on all hosts and \ as a separator only on Windows.
Membership and platform test coverage tests/clients/desktop-app-restart-posix.test.ts	Tests cover forward- and backslash separators, including Windows-style paths. POSIX-only suites now skip on Windows. The file also removes an empty trailing region.

Priority: ⬇️ Low

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to f79c1

The cross-platform membership fix and test updates are reported passing, with no unresolved merge-blocking issue identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main changes: it fixes desktop restart membership handling and prevents POSIX-only test cases from running on Windows.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

[github-actions]

✅ READY

• all PR quality gates passed; the review readiness checklist is complete.

Review readiness checklist

• ✅ All CI tests are green on my local testing.
• ✅ I pushed my PR to the latest dev commit.
• ✅ I resolved all correct Codex and CodeRabbit findings.
• ✅ My PR is ready for review.

✅ 4/4 boxes ticked.

This pull request is already Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers: @lidge-jun @Ingwannu

[lidge-jun]

리뷰 · 우선순위 67 / 80

이 PR은 Windows CI에서 tests/clients/desktop-app-restart-posix.test.ts가 깨지는 문제를 고칩니다. 지금 dev(HEAD 6e08402d8, 패키지 2.55.0)의 src/codex/desktop-app/types.ts isUnderRoot는 멤버십 경계를 path.sep 하나만으로 만듭니다. Windows는 /도 경로 구분자로 받는데, 프로브가 슬래시 경로를 돌려주면 설치 루트 안에 있는 실행 파일이 “루트 밖”으로 잘못 판정됩니다. 그러면 사용자가 요청한 데스크톱 재시작이 실패처럼 보이지 않은 채 아무 일도 안 하는 no-op이 될 수 있습니다. 형제 접두사 구멍(chatgpt vs chatgpt-evil)을 막으려고 만든 함수라, 닫히는 방향이 너무 세면 실제 멤버까지 잘라 버립니다.

고치는 방식은 두 갈래입니다. 첫째, isMembershipSeparator를 두고 /는 모든 호스트에서 구분자로 인정하고, \\는 호스트 sep이 백슬래시일 때만 구분자로 둡니다. POSIX에서 백슬래시는 파일 이름 문자라서 거기서까지 구분자로 인정하면 형제 구멍이 다시 열립니다. 둘째, macOS/Linux 어댑터 테스트는 process.getuid()에 의존하는데 Windows 호스트에는 없어서 프로브가 “실행 불가”로 끝나는 것이 맞습니다. 그래서 해당 describe를 describe.skipIf(process.platform === "win32")로 건너뛰고, Ubuntu/macOS 샤드에서만 돌립니다. 멤버십·락 케이스는 Windows에서도 계속 돕니다. 본문 기준 로컬에서 해당 스위트가 27 pass / 10 skip / 0 fail이라고 합니다.

현재 dev 방향(#4536 Google structured output + Anthropic parallel=false)과는 다른 축이지만, 최근 데스크톱 재시작 사다리(src/codex/desktop-app/)가 Windows 샤드에서 빨간 채로 있으면 기여 PR이 초록으로 못 가는 비용이 큽니다. 범위는 작습니다(+31/−5). Draft이고 Cross-platform CI 전체는 아직 안 돌린 상태입니다. 베이스는 dev보다 #4536 한 커밋 뒤입니다.

라인 119-125 - isMembershipSeparator - /를 전 호스트에서 인정하고 \\는 win32에서만 인정하는 규칙은 형제 구멍과 Windows 혼용 구분자를 동시에 다루는 올바른 최소 수정입니다.
라인 127-132 - isUnderRoot - 루트 끝/다음 문자가 구분자인지만 보므로, 예전 root + sep 접두사와 POSIX에서는 같은 집합을 유지합니다. Windows에서만 슬래시 멤버가 새로 통과합니다.
경로 src/codex/desktop-app/windows.ts 주석 - 아직 “isUnderRoot prefixes with the host path.sep”이라고 적혀 있어 이 PR 이후에는 어긋납니다. toHostMembershipPath는 소문자화·유닉스 CI용 \\→/ 접기만 하고, 실제 Windows에서 /와 \\가 섞인 케이스는 이제 isUnderRoot가 받습니다. 주석을 같이 고치는 편이 좋습니다.
경로 테스트 describe 제목 - “measured on Windows”인데 이제 Windows에서 skip됩니다. 이름만 읽으면 혼란스럽습니다.
경로 베이스 - HEAD 6e08402d8(#4536)이 빠져 있습니다. 충돌 가능성은 낮지만 머지 전에 fast-forward 리베이스가 필요합니다.
경로 Draft / CI - hygiene·target은 통과, Cross-platform 전체는 미실행. Windows 샤드에서 이 파일이 정말 초록인지가 이 PR의 존재 이유입니다.

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

• Draft를 ready로 올리기 전에 Windows 포함 Cross-platform CI를 필수 게이트로 둘지
• windows.ts 주석·skip된 describe 이름을 이 PR에서 같이 정리할지, 후속으로 미룰지
• [agent] fix: send Google structured output on the generateContent wire and map Anthropic parallel=false #4536 한 커밋만 뒤인 상태를 리베이스 후 바로 머지할지

너의 추천
dev에 리베이스한 뒤 Windows 샤드에서 해당 테스트가 초록인지 확인하고 ready로 올린 다음 머지하세요. windows.ts 주석과 skip describe 제목은 같은 PR에서 짧게 맞추는 편이 낫습니다. 기능 범위는 유지할 가치가 큽니다.

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

[luvs01]

Hosted cross-platform CI result for the published head e696a223e, run 34793832912.

The change does what it claims. Five of the six Windows shards are green: windows 1/6, 2/6, 3/6, 4/6 and 6/6 all pass, and tests/clients/desktop-app-restart-posix.test.ts no longer fails anywhere. At dev that file failed 9 of 17 on Windows and took the shard down for every contributor pull request. Ubuntu and macOS shards are green as well.

One Windows shard still fails, windows 5/6, for a reason that is not this change. The failure is injectCodexConfig integration (Design B) > a paginated home still receives the model catalog path the picker reads in tests/codex-integration/codex-inject-integration.test.ts. That test asserts the raw text of config.toml contains the catalog path, but model_catalog_json is written as a TOML basic string, so on Windows the file holds C:\\Users\\... while the assertion looks for C:\Users\.... I reproduced it on a Windows host at dev with no local changes: 74 pass / 1 fail, same failure text. It is a second independent dev regression on the same shard, and I opened #4568 for it as a test-only fix rather than mixing it into this pull request.

So the Windows shard needs both changes to go green. This one clears the desktop-app-restart-posix failures; #4568 clears the remaining codex-inject-integration failure. Neither depends on the other.

For context on why this matters beyond CI: the isUnderRoot change is a real product fix, not a test accommodation. On Windows the boundary was built from path.sep alone, so a member path written with forward slashes read as outside the install root, and windows.ts folds case but keeps backslashes. The POSIX semantics are provably unchanged, since / was already accepted there and \ is only accepted where sep === "\\".

[chatgpt-codex-connector]

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-14T07:39:12.868417Z	f79c147	Draft marked ready

ℹ️ 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.

[Ingwannu]

Requesting changes on exact head f79c14730986a9e63f2e51cdeeea099d13ec4ea3.

The new boundary-character check fixes C:/root/file against C:/root, but it does not fix mixed Windows spellings. isUnderRoot() still runs executable.startsWith(root) before looking at the separator. Therefore C:/Program Files/OpenAI.Codex/chatgpt.exe versus C:\Program Files\OpenAI.Codex (and the reverse) still returns false even though Windows names the same tree. The rationale explicitly says a probe can return either separator, but the regression uses / on both operands.

Please normalize both slash forms in the existing Windows-only toHostMembershipPath() seam before calling the shared boundary helper, and add both mixed-direction cases plus the OpenAI.Codex-evil control. Keeping that normalization Windows-local preserves the correct POSIX rule that backslash is an ordinary filename character; the shared helper can then remain a lexical boundary check over one normalized spelling.

The exact-head ordinary CI and the tested same-spelling cases are green, but they do not exercise this remaining Windows failure. The POSIX-only skips themselves are reasonable and are not the blocker.