[codex] Avoid shell for system executables#2950
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
🚀 Expo continuous deployment is ready!
|
ApprovabilityVerdict: Approved Mechanical refactor that removes shell intermediation on Windows by using explicit You can customize Macroscope's approvability policy. Learn more. |
Co-authored-by: Julius Marminge <julius@mac.lan> (cherry picked from commit 300f7fd)
Summary
Removes
shell: process.platform === "win32"from direct system executable launches where the command is not a package-manager or user-installed.cmdshim.Details
powershell.exeargv.ssh.exeandtailscale.exeon Windows so those executable launches can avoidcmd.exewhile still resolving the Windows binary name..cmdshims.Validation
vp checkpassed. It reported existingreact(no-unstable-nested-components)warnings outside this change.vp run typecheckpassed.Note
Avoid shell when spawning system executables for SSH, Tailscale, and git commands
shell: trueoption from allChildProcess.makeandProcessRunner.runcalls across the server and packages, so processes are spawned directly rather than via a shell.SSH_COMMANDinpackages/ssh/src/command.tsandTAILSCALE_COMMANDinpackages/tailscale/src/tailscale.ts, each selecting the.exevariant on Windows and the plain name elsewhere.cmd.exe; the executable must be onPATHdirectly.Macroscope summarized 1fbde6e.
Related issues
Refs #2537. This removes the SSH/Tailscale shell-wrapped executable launches called out there, but does not close the issue because OpenCode/provider process cleanup and shim-resolution work remains.
Note
Medium Risk
Changes Windows process launch semantics for SSH, Tailscale, git, and diagnostics; failures are possible if executables are not on PATH without cmd.exe resolution.
Overview
Stops wrapping known system executables in a Windows shell when spawning child processes.
shell: process.platform === "win32"is removed from diagnostics (ps/powershell.exe),ProcessRunnergit and host-label probes, terminal subprocess inspection, and SSH/TailscaleChildProcessspawns.SSH and Tailscale now use platform constants (
SSH_COMMAND,TAILSCALE_COMMAND) so Windows launchesssh.exe/tailscale.exedirectly while Unix keepsssh/tailscale. Package-manager or.cmdshim paths are intentionally unchanged per the PR description.On Windows, these calls no longer go through
cmd.exe; the binary must be resolvable onPATHwithout shell indirection.Reviewed by Cursor Bugbot for commit 1fbde6e. Bugbot is set up for automated code reviews on this repo. Configure here.