[codex] Restore Windows CLI runtime detection#3
Conversation
The runtime assumed Unix command lookup and Unix executable semantics, so Windows npm shims for Codex and Claude were invisible to the Studio even though they were installed on PATH. The CLI package also used Unix chmod during build, which blocked a clean Windows install. This teaches detection to use where.exe on Windows, resolves the actual command path before spawning agents, wraps Windows .cmd/.bat shims through cmd.exe, and keeps package build/test scripts portable for fresh Windows checkouts. Constraint: Windows npm global CLIs are exposed as .cmd/.ps1 shims rather than executable Unix-style binaries. Rejected: Require users to launch Studio from a special shell | the app should use the same PATH-visible CLIs users already have. Confidence: high Scope-risk: moderate Directive: Do not reintroduce Unix-only which/chmod/test assumptions into CLI install or runtime detection. Tested: pnpm -r build; pnpm -r typecheck; pnpm -r test; pnpm smoke; /api/agents?force=1 shows claude and codex available; /api/agents/claude/test and /api/agents/codex/test returned hello
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19e038bc72
ℹ️ 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".
| if (process.platform === 'win32' && ['.cmd', '.bat'].includes(ext)) { | ||
| return { | ||
| bin: 'cmd.exe', | ||
| args: ['/d', '/s', '/c', [bin, ...args].map(quoteCmdArg).join(' ')], |
There was a problem hiding this comment.
Escape cmd.exe arguments safely
On Windows .cmd/.bat shims this builds a single cmd.exe /c command line, but quoteCmdArg escapes embedded quotes with backslashes, which cmd.exe does not treat as quote escaping. For agents that pass user/source text in argv, such as Hermes (packages/runtime/src/defs/hermes.ts:25), a prompt containing a quote followed by & ... can break out of the quoted argument and execute another shell command instead of being delivered as data.
Useful? React with 👍 / 👎.
|
@tackcrypto1031 I'm holding off on generating review comments for #3 because this pull request has merge conflicts right now. Please resolve the conflicts with main and push the updated branch. Once that's done, request or wait for the review to run again and I'll take another look. 🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos. |
|
Heads-up: PR #29 is also open against the same Windows CLI runtime path. Both PRs touch Sharing this so the approaches can be compared before maintainers decide what lands. |
Summary
Fixes Windows CLI install and agent detection paths for the local Studio.
Root cause
The runtime assumed Unix command lookup and executable semantics:
which, so Windows npm shims such ascodex.cmdandclaude.cmdwere not foundchmod, which fails on Windowsnode --test test/but shipped notest/directoryChanges
where.exeon Windows and prefer.exe,.cmd, or.batmatches during CLI detection.cmd/.batshims throughcmd.exechmodwith a cross-platform Node script/favicon.icono-content response to avoid Studio browser 404 noisedoctorso Windows ffmpeg and Chrome/Edge installs are detected correctlyValidation
pnpm -r buildpnpm -r typecheckpnpm -r testpnpm smoke/api/agents?force=1returnsclaude.available=trueandcodex.available=true/api/agents/claude/testreturnedhello/api/agents/codex/testreturnedhelloNotes
The test command now skips packages with no
test/directory. That preserves the current repo state while allowing future package tests to run automatically when added.