fix: buildNpmScriptCommand always uses cmd.exe on Windows#734
fix: buildNpmScriptCommand always uses cmd.exe on Windows#734jamiechicago312 wants to merge 6 commits into
Conversation
On Windows, npm sets npm_execpath to a path like C:\Program Files\nodejs\node_modules\npm\bin\npm-cli.js which contains spaces. buildNpmScriptCommand was returning that path as a spawn argument with the full node.exe path as the command. spawnService uses shell:true on Windows, so Node.js passes the unquoted command to cmd.exe: cmd.exe /d /s /c C:\Program Files\nodejs\node.exe ... cmd.exe splits on the space and fails with 'C:\Program' is not recognized as an internal or external command causing Vite to exit with code 1 immediately after npm run dev. Fix: check platform === 'win32' BEFORE checking npm_execpath so Windows always uses the safe cmd.exe /d /s /c npm run <script> form. Co-authored-by: openhands <openhands@all-hands.dev>
On Windows, npm sets npm_execpath to a path like C:\Program Files\nodejs\node_modules\npm\bin\npm-cli.js which contains spaces. buildNpmScriptCommand was returning that path as a spawn argument with the full node.exe path as the command. spawnService uses shell:true on Windows, so Node.js passes the unquoted command to cmd.exe: cmd.exe /d /s /c C:\Program Files\nodejs\node.exe ... cmd.exe splits on the space and fails with 'C:\Program' is not recognized as an internal or external command causing Vite to exit with code 1 immediately after npm run dev. Fix: check platform === 'win32' BEFORE checking npm_execpath so Windows always uses the safe cmd.exe /d /s /c npm run <script> form. Co-authored-by: openhands <openhands@all-hands.dev>
|
@jamiechicago312 is attempting to deploy a commit to the openhands Team on Vercel. A member of the Team first needs to authorize it. |
ChatStatusIndicator had two separately-keyed motion.span children inside
<AnimatePresence mode="wait">. framer-motion's wait mode expects exactly ONE
child to exit before the next enters; two children trigger the repeated warning:
"attempting to animate multiple children within AnimatePresence,
but its mode is set to 'wait'"
Fix: wrap both elements in a single motion.span with unified key={status}
and className="contents" (CSS display:contents preserves flex layout).
Co-authored-by: openhands <openhands@all-hands.dev>
Python on Windows defaults to the system ANSI codepage (cp1252). The agent-server writes metadata JSON containing emoji (e.g. U+2705 ✅) that cp1252 cannot encode, producing UnicodeEncodeError → POST /api/conversations 500. Old UTF-8 conversation files also fail to load at startup (UnicodeDecodeError). PYTHONUTF8=1 enables Python's UTF-8 mode (PEP 540) for the process, matching Linux/macOS behaviour. Co-authored-by: openhands <openhands@all-hands.dev>
Python on Windows defaults to the system ANSI codepage (cp1252). The agent-server writes metadata JSON containing emoji (e.g. U+2705 ✅) that cp1252 cannot encode, producing UnicodeEncodeError → POST /api/conversations 500. Old UTF-8 conversation files also fail to load at startup (UnicodeDecodeError). PYTHONUTF8=1 enables Python's UTF-8 mode (PEP 540) for the process, matching Linux/macOS behaviour. Co-authored-by: openhands <openhands@all-hands.dev>
|
Logs to proof this fix is successful and a screenshot 20260522-pr734-succesful-logs.txt 
|
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
Windows Path Fix: Excellent Solution ✅
The core fix is elegant and well-tested. Moving the win32 check before npm_execpath correctly avoids the cmd.exe space-splitting issue. The regression test specifically covers the failing scenario (paths with spaces). Bonus: the PYTHONUTF8=1 addition prevents UTF-8 encoding errors on Windows.
[RISK ASSESSMENT]
🟢 LOW - Bug fix that makes Windows dev environment work correctly. No breaking changes, good test coverage, and the cmd.exe approach is the standard Windows pattern for npm script execution.
Was this automated review useful? React with 👍 or 👎 to this review to help us measure review quality.
Workflow run: https://github.com/OpenHands/agent-canvas/actions/runs/26301906834
| <motion.span | ||
| key={`dot-${status}`} | ||
| className="flex-shrink-0 animate-[pulse_1.2s_ease-in-out_infinite]" | ||
| key={status} |
There was a problem hiding this comment.
🟡 Suggestion: This framer-motion fix is correct but unrelated to the Windows path issue. Consider either:
- Mentioning it in the PR description ("Also fixes a framer-motion AnimatePresence warning in chat status indicator")
- Moving it to a separate PR for cleaner change tracking
Not a blocker - the fix itself is sound (wrapping both elements in a single keyed motion.div with className="contents" is the right solution for mode="wait").
2 participants
Problem
npm run devfails immediately on Windows with:Vite never starts, and the ingress proxy returns Bad Gateway for every request to
localhost:8000. This happens on a clean install — no.env, no special config needed.Root cause
When
npm run devis executed on Windows, npm setsnpm_execpathto something like:In
buildNpmScriptCommandthenpm_execpathbranch fires first, returning that full path (with spaces) as a spawn argument, and usingprocess.execPath(C:\Program Files\nodejs\node.exe— also with spaces) as the command.spawnServiceusesshell: process.platform === "win32", so Node.js builds the shell command:cmd.exesplits on the space, seesC:\Programas the command, and fails. Thewin32branch inbuildNpmScriptCommandalready has the correct fix (cmd.exe /d /s /c npm run <script>), but it came after thenpm_execpathbranch, so it was never reached on Windows when running vianpm run dev.Fix
Move the
win32check before thenpm_execpathcheck. On Windows, we always use:This is safe on all Windows Node installs regardless of where npm was installed or whether
npm_execpathcontains spaces.No changes needed on macOS or Linux — those platforms continue to use the
npm_execpath-aware path.Testing
npm_execpathwith spaces (the exact scenario from the bug report).npm_execpathtest to POSIX since the npm_execpath shortcut only matters on non-Windows platforms.Closes / supersedes #716 (the docs-only workaround).
This PR was created by an AI agent (OpenHands) on behalf of the user.
@jamiechicago312 can click here to continue refining the PR