Conversation
- Check $LASTEXITCODE after bun install and bun build in install.ps1 - Use -Encoding OEM for .cmd shims to preserve non-ASCII paths - Use -Encoding UTF8 for built script with shebang - Distinguish "not found on PATH" from "health check failed" - Add fail-fast: false to CI matrix for independent OS results - Fix stale comments: close() docstring, HOME references, CRLF note - Add Windows sections to Chinese README and CONTRIBUTING.md - Fix ambiguous "Both" wording in README prerequisites
- Merge prerequisites inline with installation heading - Share git clone step, split into Linux/macOS and Windows subheadings - Apply same structure to Chinese README
- Add extensionless bash shim alongside .cmd for Git Bash/MSYS2 compat - Fall back to copy when symlinks fail (no Developer Mode) - Use system default encoding for .cmd shims (locale-safe) - Add spawnSync timeout to CLI tests (prevents hang when codex installed) - Fix protocol test race: use local clients instead of shared afterEach - Show only first result from 'where' in health check output - Add installer smoke tests to CI for both platforms
On Windows, bun:test's expect(promise).rejects.toThrow() prevents the event loop from reaching the I/O phase needed to flush Bun's FileSink stdin buffer, causing request timeouts in tests 4 and 5. Replace the four affected assertions with a captureErrorMessage() helper that uses plain await (matching how production callers use the client), which flushes correctly. Also fix install.ps1 to write bash shims without a UTF-8 BOM using [System.IO.File]::WriteAllText with UTF8Encoding($false), replacing Set-Content -Encoding UTF8 which emits a BOM in PowerShell 5.1 and breaks shebang recognition.
The bash wrapper script in install.ps1 used #!/usr/bin/env bun but contained bash syntax (exec bun ...), causing Git Bash to pass it to Bun which failed to parse it as JavaScript. Fixed both dev-mode and build-mode shims to use #!/usr/bin/env bash.
- Log warnings on proc.kill() and taskkill failures instead of empty catches - Check taskkill exit status via stdio: pipe - Harden captureErrorMessage test helper to throw on unexpected resolve - Wrap bun build in try/catch consistent with bun install - Exit 1 when codex-collab health fails in installer
taskkill returns status null when the process is already dead (killed by proc.kill()). Add null to the skip condition so this expected case doesn't produce a noisy warning during health checks.
P1: Swap kill order in close() — run taskkill /T /F first to kill the process tree, then proc.kill() as fallback. Previous order killed the direct child first, removing the PID that taskkill needs to traverse the tree when codex is invoked via a .cmd wrapper. P2: Use UTF-8 encoding for .cmd shims instead of ASCII. ASCII encoding corrupts non-ASCII path segments (e.g. Chinese directory names), breaking dev installs in those locations.
![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: 23d73999ac
ℹ️ 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".
- Add npm install -g @openai/codex step before installer smoke tests - Run install.ps1 in-session with Set-ExecutionPolicy Bypass so PATH propagates - Simulate terminal reload in CMD step by reading PATH from registry - Verify codex-collab --help and health from both pwsh and CMD shells - Remove dead code in installer health check, add "open new terminal" hint
Kevin7Qi
force-pushed
the
feat/windows-support
branch
from
9c53f90 to
e6f3500
Compare
- Extract shim-creation block from install.ps1 if/else branches - Hoist spawnSync import to top-level instead of dynamic await import - Add inline comment for taskkill exit codes (128, null) - Assert specific error message for run-without-prompt test
Avoids spurious "rollout" warnings from the Codex app-server when killing a thread that was already killed or archived. Instead of suppressing the errors, check local status in threads.json first and early-exit if the thread is already marked as killed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2
Summary
path.join(),os.homedir(),os.tmpdir()) across all source and test filesprotocol.tsclose()usingtaskkill /T /Ffor process tree cleanupinstall.ps1) with .cmd and bash shims, symlink support, and PATH management.gitattributesfor consistent line endingsprocess.stdinevents for Windows compatibilitycaptureErrorMessagetest helper and installer error handlingTest plan
install.sh --devworks on Linuxinstall.ps1andinstall.ps1 -Devwork on Windowshealth,run,run --resume,review,models,jobs,output,progress,kill,delete,cleanall functional