Repository navigation
Conversation
Codex Review Summary: ✅ CompletedThis 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. |
|
Closing this PR because it was opened against the wrong repository for the intended contribution. Sorry for the noise. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d558e56d5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| shell: options.shell ?? (process.platform === "win32" ? (process.env.SHELL || true) : false), | ||
| // Windows npm shims are .cmd files. Let Node use the Windows command shell | ||
| // rather than SHELL, which may point to PowerShell and select codex.ps1. | ||
| shell: options.shell ?? (process.platform === "win32"), |
There was a problem hiding this comment.
Resolve Windows shims outside the checkout
When the plugin is run from an untrusted repository containing node.cmd or codex.cmd, shell: true delegates these bare command names to cmd.exe, whose normal lookup checks cwd before PATH; /codex:setup can therefore execute the repository's batch file as the user instead of the installed tool, and the changed app-server spawn does the same for codex. This newly exposes the SHELL=PowerShell users targeted by this change because PowerShell does not search the current directory by default, so resolve the intended shim from PATH to an absolute path before invoking it rather than relying on cmd lookup.
Useful? React with 👍 / 👎.
Summary
Fixes #832. On Windows, use Node's default command shell for Codex CLI availability checks and direct app-server launches, even when
SHELLpoints to PowerShell. This selects npm'scodex.cmdshim instead of acodex.ps1shim that may be blocked by PowerShell execution policy.Document the Windows launch behavior and add a Windows CI regression test that installs both fake shims, sets
SHELL=powershell.exe, then verifies setup and a task run.Verification
node --test tests/process.test.mjs tests/runtime.test.mjs— 63 passed, 1 Windows-only test skipped on macOSnpm run build— passedgit diff --check— passedThe Windows regression test will run in the new
windows-shimCI job.