Fix Windows provider CLI spawn + cross-platform scripts#638
Fix Windows provider CLI spawn + cross-platform scripts#638JKamsker wants to merge 10 commits intogeneralaction:mainfrom
Conversation
|
@JKamsker is attempting to deploy a commit to the General Action Team on Vercel. A member of the Team first needs to authorize it. |
|
@codex review |
|
@JKamsker - Your PR looks great! I have one suggestion that would make the PTY spawning even more Currently on Unix, when spawning provider terminals, we rely on the shell's PATH to find the CLI: To make this more reliable (especially for users with non-standard shell configs), we could resolve Add this helper function near the top of ptyManager.ts (around line 15): } Then modify the provider command building (around line 182-189): // To: This would make the provider spawning use absolute paths (e.g., What do you think? Happy to test this addition if you'd like to include it! The rest of your PR |
|
lmk what you think @JKamsker :) |
|
Hey @arnestrickmann, thanks for the quick reply! |
- Prefer executable extensions from where on win32 (e.g. .cmd) - Spawn the resolved executable path (and use shell for cmd/bat/ps1) - Replace unix-only npm scripts (rm/cp/mkdir) with node scripts - Make postinstall rebuild native deps with optional PTY rebuild
- Show real PTY start errors instead of 'isn’t installed' - Allow suppressing install command for start failures - Resolve and spawn provider CLIs robustly on Windows
Include PATH/PATHEXT and key Windows env vars in the PTY environment so codex.cmd can locate ode.
0af9715 to
2596e48
Compare
Summary
This PR fixes provider CLI detection/spawning on Windows (e.g.
codex), and makes the dev/build scripts cross-platform so Windows can run the project without relying on Unix shell utilities.What changed
src/main/services/ConnectionsService.tsnow prefers executable extensions fromwhere(e.g..cmd) and spawns the resolved executable path..cmd/.batexecution: on win32, when the resolved executable is.cmd/.bat/.ps1, spawning usesshell: truesospawn()doesn’t fail (e.g.EINVAL).rm -rf,mkdir -p, andcpusage with small Node-based scripts.postinstallnow runs a script that rebuilds required native deps and optionally rebuilds PTY support (skipped when PTY is disabled).Why
where codexcan return an extensionless shim first (e.g.%APPDATA%\npm\codex). Node’sspawn()cannot execute that shim directly, causingspawn ... ENOENTeven though the tool is installed..cmddirectly without a shell can also produceEINVAL.Testing
where codexresolves both shim and.cmd, and the code now selects.cmd.npm run type-check.npm run devon Windows and confirmed providers likecodexshow as connected.Notes
EMDASH_DISABLE_PTY=1for environments lacking the required Visual Studio Spectre-mitigated libs.Note
Improves Windows CLI detection/execution and makes tooling cross‑platform, plus better UI surfacing of terminal start failures.
whereand spawn withshellfor.cmd/.bat/.ps1inConnectionsServiceandPrGenerationService(alsowindowsHide)ptyManager: cleaner environment initialization; add Windows PATH/PATHEXT/SystemRoot vars and better shell resolution/errorsInstallBannergainsstart_failedmode with error details (incl. PTY-disabled hint);ChatInterfacecaptures and displays CLI start errorscopy-app-config.cjs,clean.cjs); newpostinstall.cjsselectively runselectron-rebuildfor native deps (e.g.,sqlite3,node-pty,keytar); updatepackage.jsonscripts accordinglyWritten by Cursor Bugbot for commit 83f319a. This will update automatically on new commits. Configure here.