fix(ui): make ui:build work on Windows by xiwuqi · Pull Request #52291 · openclaw/openclaw

xiwuqi · 2026-03-22T14:36:27Z

Summary

Problem: pnpm ui:build can fail on Windows when the resolved pnpm.cmd path contains spaces, because scripts/ui.js launches shell-backed commands without quoting the executable path.
Additional issue found while reproducing on a clean checkout: the build auto-install path used pnpm install --prod, which skips vite and leaves the UI build unable to start.
What changed: quote Windows shell launcher paths before spawn/spawnSync, and make the build auto-install path install the full UI dependency set instead of production-only deps.
Scope boundary: no UI app code changed; this only touches the build wrapper script and its tests.

Change Type

Scope

Linked Issue/PR

Closes [Bug]: Windows 下执行 pnpm ui:build 失败，报错 'D:\Program' 不是内部或外部命令 #52282

User-visible / Behavior Changes

On Windows, pnpm ui:build now works when the package manager lives under a path with spaces (for example C:\Program Files\...). On clean checkouts, the command also installs the build-time UI dependencies it needs before invoking Vite.

Security Impact

New permissions/capabilities? No
Secrets/tokens handling changed? No
New/changed network calls? No
Command/tool execution surface changed? No
Data access scope changed? No

Repro + Verification

Environment

OS: Windows 11
Runtime/container: Node v20.19.0 / pnpm v10.32.1
Relevant command: pnpm ui:build

Expected

pnpm ui:build should launch the resolved package-manager command correctly on Windows.
The command should be able to install the UI dependencies required for the build and complete successfully.

Actual before fix

On affected Windows setups, shell-backed launchers under paths with spaces fail before running: 'D:\Program' is not recognized ...
On a clean checkout in my local reproduction, the same code path also installed with --prod, so vite build failed because vite was absent.

Evidence

Failing repro before + passing after
Screenshot/recording
Perf numbers

Commands run:

pnpm exec vitest run --config vitest.config.ts test/scripts/ui.test.ts
pnpm ui:build

Observed after fix:

test/scripts/ui.test.ts: 1 passed, 6 passed tests
pnpm ui:build: completed successfully and emitted dist/control-ui/*

Human Verification

What I personally verified:

The Windows shell launcher path is now quoted when needed.
pnpm ui:build succeeds locally on a fresh worktree after the script changes.
Non-shell launchers remain unquoted.

Review Conversations

I replied to or resolved every bot review conversation I addressed in this PR.
I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

Backward compatible? Yes
Config/env changes? No
Migration needed? No

Failure Recovery

How to disable/revert this change quickly: revert this commit
Known bad symptoms reviewers should watch for: none beyond the previous ui:build failures returning

Risks and Mitigations

Risk: quoting shell launchers on Windows could affect commands that were already quoted.
- Mitigation: the helper preserves already-quoted commands and only touches shell-backed launchers with whitespace in the path.
Risk: installing full deps for build may do more work than the previous production-only install.
- Mitigation: this is only used when UI deps are missing, and the build already requires vite/other dev-time tooling to succeed.

greptile-apps · 2026-03-22T14:38:42Z

Greptile Summary

This PR fixes two related issues that caused pnpm ui:build to fail on Windows: shell-backed launcher paths containing spaces were passed unquoted to spawn/spawnSync, causing cmd.exe to misparse the executable token; and the auto-install code path used pnpm install --prod, which excluded devDependencies such as vite, causing the build to fail on a clean checkout.

Changes:

Adds quoteWindowsShellCommand(cmd, platform): wraps .cmd/.bat/.com paths in double-quotes when whitespace is present; is a no-op for already-quoted commands, non-shell executables, and non-Windows platforms.
run and runSync now apply quoteWindowsShellCommand to the command before passing it to spawn/spawnSync, while still using the original path for createSpawnOptions so shell-mode detection (based on file extension) works correctly.
Removes the --prod flag and NODE_ENV=production override from the dependency auto-install path, ensuring devDependencies like vite are available for build.
New unit test covers the quoting helper for the three expected cases: spaced shell path → quoted; non-spaced shell path → unchanged; non-shell .exe with spaces → unchanged.

The implementation is clean and well-scoped. The logic in quoteWindowsShellCommand correctly guards against double-quoting through the startsWith('"') && endsWith('"') check. The only minor omission is a test for the already-quoted guard branch, but that path is never reached in normal operation since which() never returns a pre-quoted result.

Confidence Score: 5/5

Safe to merge — targeted Windows-only fix with no changes to non-Windows behaviour, UI app code, or security surface.
Both fixes are narrowly scoped and correct: quoting shell-backed paths only when whitespace is present and the path is not already quoted is the standard cmd.exe workaround, and removing --prod from the auto-install path is a clear correctness fix. createSpawnOptions continues to receive the original unquoted path for extension-based shell-mode detection. Tests cover the key cases. No production-path behaviour changes on Linux/macOS.
No files require special attention.

_{Reviews (1): Last reviewed commit: "fix(ui): make ui build work on Windows" | Re-trigger Greptile}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 05a293af24

ℹ️ 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".

chatgpt-codex-connector · 2026-03-22T14:40:04Z

-      action === "build" ? { ...process.env, NODE_ENV: "production" } : process.env;
-    const installArgs = action === "build" ? ["install", "--prod"] : ["install"];
-    runSync(runner.cmd, installArgs, installEnv);
+    runSync(runner.cmd, ["install"]);


Limit the fallback install to the UI package

At this line, the missing-deps fallback switched from pnpm install --prod to a bare pnpm install while createSpawnOptions() still runs it from ui/. I checked pnpm install --help, and inside a workspace that command installs dependencies for all workspace projects, not just the current package. Because depsInstalled("build") only requires vite and dompurify, pnpm ui:build now turns a clean checkout into a full workspace dev install, pulling unrelated dev/test tooling just to build the UI. That is a regression in scope and can make ui:build unexpectedly slow or fail on hosts that intentionally do not have the full repo toolchain available.

Useful? React with 👍 / 👎.

LouisGameDev · 2026-04-27T22:18:37Z

Related follow-up: #73059.

This PR appears adjacent to that report but not sufficient to close it.

This PR is about Windows pnpm.cmd quoting and UI dependency install behavior.
[Bug]: Windows linked source install can skip Control UI auto-build through junction-launched scripts/ui.js #73059 still reproduces when invoking node %APPDATA%\npm\node_modules\openclaw\scripts\ui.js build directly, before pnpm.cmd resolution matters.
In that path, the failure is the direct-execution guard in scripts/ui.js: it compares path.resolve(process.argv[1]) with fileURLToPath(import.meta.url), which diverge across a Windows directory junction, so main() never runs.
The user-visible result is the gateway logging that the Control UI build completed while dist/control-ui/index.html is still missing.

So the junction-path bug should remain tracked separately from the quoting/install fix here.

clawsweeper · 2026-04-28T01:29:18Z

Closing this as implemented after Codex automated review.

The Windows ui:build launcher-path fix from this PR is already present on current main and in the latest verified release tag. Main quotes Windows pnpm .cmd/.bat/.com launchers before spawn, keeps the full dependency install fallback for missing UI build deps, and has regression coverage in the Windows script tests.

Best possible solution:

Close this PR as already implemented on main and keep the shipped Windows UI launcher fix. Continue tracking the separate junction-launched scripts/ui.js bug in #73059, and handle any desired narrowing of the UI auto-install fallback as a new focused issue or PR against current main.

What I checked:

Current implementation quotes Windows shell launchers: Current main uses prepareSpawnCommand() to quote Windows shell-backed command launchers before both spawn() and spawnSync(), while createSpawnOptions() still derives shell mode from the original command and rejects unsafe Windows shell metacharacters. (scripts/ui.js:79, 06a80fa81357)
Clean-checkout fallback installs build-time UI deps: When build/test dependencies such as vite or dompurify are missing, main now runs pnpm install from the UI directory without the old --prod/NODE_ENV=production path, matching the PR's second requested fix. (scripts/ui.js:134, 06a80fa81357)
Regression coverage exists: The UI script test covers a C:\Program Files\nodejs\pnpm.cmd launcher, confirms non-shell .exe launchers are not quoted, and checks unsafe shell arguments are rejected; package.json includes this file in test:windows:ci. (test/scripts/ui.test.ts:21, 06a80fa81357)
Release evidence: The changelog records the UI/Windows fix for the same C:\Program failure mode, and the verified v2026.4.26 tag contains the same scripts/ui.js and test/scripts/ui.test.ts implementation. (CHANGELOG.md:533, be8c24633aaa)
Related discussion checked: The PR body links the original Windows pnpm.cmd quoting report [Bug]: Windows 下执行 pnpm ui:build 失败，报错 'D:\Program' 不是内部或外部命令 #52282, which is closed. The later [Bug]: Windows linked source install can skip Control UI auto-build through junction-launched scripts/ui.js #73059 discussion explicitly separates the junction/direct-execution bug from this PR, so that separate open issue is not a reason to keep this already-implemented PR open.
Security review pass: The PR only touched scripts/ui.js and test/scripts/ui.test.ts. Current main adds no workflow, lockfile, dependency-source, package-publishing, or secrets-handling changes for this surface; the shell execution path includes an explicit Windows metacharacter guard before shell:true spawns. (scripts/ui.js:64, 06a80fa81357)

So I’m closing this as already implemented rather than keeping a duplicate issue open.

Codex Review notes: model gpt-5.5, reasoning high; reviewed against 06a80fa81357; fix evidence: release v2026.4.26, commit be8c24633aaa.

fix(ui): make ui build work on Windows

05a293a

openclaw-barnacle Bot added scripts Repository scripts size: XS labels Mar 22, 2026

chatgpt-codex-connector Bot reviewed Mar 22, 2026

View reviewed changes

xiwuqi mentioned this pull request Mar 22, 2026

[Bug]: Windows 下执行 pnpm ui:build 失败，报错 'D:\Program' 不是内部或外部命令 #52282

Closed

BradGroux mentioned this pull request Mar 26, 2026

WORKING: All Microsoft Issues and PRs #49126

Closed

Copilot AI mentioned this pull request Mar 27, 2026

List Azure and Windows open PRs available for review johnsonshi/openclaw#1

Closed

17 tasks

clawsweeper Bot mentioned this pull request Apr 26, 2026

[Bug] ui:build fails on Windows when pnpm.cmd is installed under Program Files #55101

Closed

BingqingLyu mentioned this pull request Apr 27, 2026

fix(ui): make ui:build work on Windows BingqingLyu/openclaw#1234

Open

19 tasks

LouisGameDev mentioned this pull request Apr 27, 2026

[Bug]: Windows linked source install can skip Control UI auto-build through junction-launched scripts/ui.js #73059

Open

clawsweeper Bot closed this Apr 28, 2026

github-actions Bot mentioned this pull request Apr 28, 2026

🦞 OpenClaw Ecosystem Digest 2026-04-28 borq168/big_model_radar#60

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(ui): make ui:build work on Windows#52291

fix(ui): make ui:build work on Windows#52291
xiwuqi wants to merge 1 commit intoopenclaw:mainfrom
xiwuqi:wuxi/fix-ui-build-windows-shell

xiwuqi commented Mar 22, 2026

Uh oh!

greptile-apps Bot commented Mar 22, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Mar 22, 2026

Uh oh!

LouisGameDev commented Apr 27, 2026

Uh oh!

clawsweeper Bot commented Apr 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

xiwuqi commented Mar 22, 2026

Summary

Change Type

Scope

Linked Issue/PR

User-visible / Behavior Changes

Security Impact

Repro + Verification

Environment

Expected

Actual before fix

Evidence

Human Verification

Review Conversations

Compatibility / Migration

Failure Recovery

Risks and Mitigations

Uh oh!

greptile-apps Bot commented Mar 22, 2026

Greptile Summary

Confidence Score: 5/5

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

LouisGameDev commented Apr 27, 2026

Uh oh!

clawsweeper Bot commented Apr 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants