Repository navigation
fix(chatgpt): build the bundled app-server path with posix.join on every host - #6413
Conversation
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. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe launcher now uses POSIX path construction for candidate macOS bundle paths. A test checks that the probed paths use ChangesBundle path resolution
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The change addresses host-dependent separators for macOS bundle candidates, and the supplied context reports hosted CI coverage. No actionable merge-blocking regression is established. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 2 systems. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e906c019fa
ℹ️ 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".
| ): string | null { | ||
| for (const layout of BUNDLED_CODEX_LAYOUTS) { | ||
| const candidate = join(bundleRoot, ...layout); | ||
| const candidate = posix.join(bundleRoot, ...layout); // macOS bundle paths are POSIX on every host |
There was a problem hiding this comment.
Update the owned ChatGPT architecture document
This changes the path-construction invariant in the owned src/chatgpt/ area, but the commit does not update its mapped document, structure/clients/chatgpt-desktop.md. Document that bundle candidates must retain POSIX semantics even on non-macOS test hosts so the Windows-safe behavior remains part of the subsystem contract; repository ownership rules require an owned source change and its document update to land together.
AGENTS.md reference: AGENTS.md:L42-L43
Useful? React with 👍 / 👎.
|
✅ Deterministic PR hygiene checks passed. |
|
Maintainer integration record (MAINTAINERS.md, dev-only)
|
Summary
The full Cross-platform CI run on
dev(36891940246) failswindows 3/9ontests/clients/desktop-app-server-shim-launcher.test.ts:60, a test #6361 added: "the bundled binary is derived from the discovered bundle root".resolveChatgptCodexBinarybuilds the candidate withnode:pathjoin, which uses backslashes on Windows. The test then receivesnullwhere it expects the POSIX bundle path.A ChatGPT.app bundle path is a macOS path, so this PR builds the candidate with
posix.joinon every host. On macOS nothing changes. On Windows, the resolver now returns the same path the test, and the rest of the shim, expect.Verification
Checklist
Summary by CodeRabbit