build(desktop): stabilize Windows dev and distribution workflows#449
Merged
PerishCode merged 55 commits intomainfrom Apr 3, 2026
Merged
build(desktop): stabilize Windows dev and distribution workflows#449PerishCode merged 55 commits intomainfrom
PerishCode merged 55 commits intomainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMigrates desktop dev tooling from Bash to a Node.js CLI, makes runtime manifest creation asynchronous, adds archive-based sidecar extraction (tar.gz and zip) with cache fingerprinting and Windows symlink fallbacks, introduces Windows packaging and smoke-test tooling, and adds EOL normalization via .gitattributes. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as dev-cli.mjs
participant Lock as Lock Manager
participant Cache as Sidecar Cache
participant Extract as Archive Extractor
participant Build as Build Pipeline
participant Electron as Electron Process
User->>CLI: start
CLI->>Lock: acquire single-instance lock
Lock-->>CLI: lock obtained
CLI->>Cache: check sidecar cache fingerprint
alt cache hit
Cache-->>CLI: cached sidecar usable
else cache miss
CLI->>Extract: extract packaged sidecar (tar.gz or zip)
Extract-->>CLI: sidecar prepared
CLI->>Cache: write cache metadata
end
CLI->>Build: check/run pnpm build tasks (unless reuse)
Build-->>CLI: build artifacts ready
CLI->>Electron: spawn detached electron
Electron-->>CLI: PID recorded
CLI->>Lock: persist session state and release lock
CLI-->>User: started (PID/logs)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd6ed36760
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/scripts/prepare-openclaw-sidecar.mjs (1)
990-1016:⚠️ Potential issue | 🟠 MajorGive
openclaw.cmdthe same bundled-runner fallback as the POSIX launcher.The Windows wrapper always executes
node ..., while the shell launcher below falls back toOPENCLAW_ELECTRON_EXECUTABLE. If PATH does not expose the expected Node binary,dist:winhard-fails even though the packaged app already has a bundled runner available.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/scripts/prepare-openclaw-sidecar.mjs` around lines 990 - 1016, Update the Windows wrapper written by the writeFile call that creates "openclaw.cmd" so it mirrors the POSIX wrapper fallback: instead of unconditionally executing node with packagedOpenclawEntry, first check for node on PATH (e.g. using where/node check) and exec node if found; otherwise, if the OPENCLAW_ELECTRON_EXECUTABLE env var is set and points to an executable, run it with ELECTRON_RUN_AS_NODE=1 and the same packagedOpenclawEntry and arguments as the POSIX launcher. Keep the rest of the batch wrapper logic consistent with the POSIX script (use packagedOpenclawEntry and preserve %* argument passthrough).
🧹 Nitpick comments (5)
apps/desktop/scripts/lib/sidecar-paths.mjs (1)
67-84: Consider logging when falling back to copy on Windows.The Windows symlink failure fallback silently switches from symlink to copy semantics. While this works correctly for the current use cases, a log message would aid debugging when the two behaviors diverge (symlinks reflect source mutations; copies are snapshots).
🔧 Optional: Add fallback logging
} catch (error) { if (process.platform !== "win32") { throw error; } + console.log( + `[sidecar-paths] symlink failed, falling back to copy: ${sourcePath} -> ${targetPath}`, + ); await cp(sourcePath, targetPath, { recursive: true, dereference: true, filter: (source) => basename(source) !== ".bin", }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/scripts/lib/sidecar-paths.mjs` around lines 67 - 84, The catch block for symlink failure silently falls back to cp, so add a log entry before performing the copy to indicate the symlink failed and we're using a copy fallback; specifically, inside the catch(error) branch where process.platform === "win32" before calling cp(sourcePath, targetPath, ...), call the existing logger (or process.stdout/processLogger) to record a message containing the error and the sourcePath/targetPath and that a recursive copy will be used as fallback for symlink in the symlink => cp fallback code path.apps/desktop/scripts/prepare-runtime-sidecars.mjs (1)
14-23: Same argument quoting consideration asopenclaw-runtime/postinstall.mjs.The
join(" ")pattern doesn't quote arguments. Current usage (["run", script]) is safe, but the pattern should be consistent across both files if one is updated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/scripts/prepare-runtime-sidecars.mjs` around lines 14 - 23, The createCommandSpec function builds a Windows cmd.exe invocation using ["pnpm", ...args].join(" ") which doesn't quote individual args; update createCommandSpec to safely quote or escape each argument when composing the single string for cmd.exe (handle spaces and special chars), e.g. transform args into a properly quoted string before passing it as the single /c argument, keeping the same branch for command === "pnpm" || command === "pnpm.cmd" and leaving the non-Windows return { command, args } unchanged.openclaw-runtime/postinstall.mjs (1)
14-23: Arguments containing spaces or special characters would break the command.The
join(" ")approach on line 18 doesn't quote arguments. While current usage only passes simple flags (ci,--no-audit, etc.), this pattern is fragile if future callers pass arguments with spaces or shell metacharacters.🛡️ Optional: Quote arguments for robustness
function createCommandSpec(command, args) { if (process.platform === "win32" && (command === "npm" || command === "npm.cmd")) { + const quotedArgs = args.map((arg) => + arg.includes(" ") || arg.includes('"') ? `"${arg.replace(/"/g, '\\"')}"` : arg + ); return { command: "cmd.exe", - args: ["/d", "/s", "/c", ["npm", ...args].join(" ")], + args: ["/d", "/s", "/c", ["npm", ...quotedArgs].join(" ")], }; } return { command, args }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openclaw-runtime/postinstall.mjs` around lines 14 - 23, The Windows branch in createCommandSpec builds a single command string with ["npm", ...args].join(" ") which fails for arguments containing spaces or shell metacharacters; change this to safely escape and quote each argument before joining (e.g., replace the join with args.map(escapeAndQuote).join(" ") where escapeAndQuote properly escapes backslashes/quotes and wraps the arg in double-quotes when needed) so the returned args array contains a single, robust cmd.exe /c string that preserves argument boundaries.smoke/Dockerfile (1)
1-5: Drop root for the smoke image.This container only needs to run a Node script; keeping the default root user adds avoidable privilege and can leave root-owned files in the mounted workspace.
Possible fix
FROM node:22-bookworm WORKDIR /workspace/smoke +USER node CMD ["node", "./feishu-ws-smoke.mjs"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@smoke/Dockerfile` around lines 1 - 5, The Dockerfile currently runs as root (base image node:22-bookworm) which is unnecessary; modify the Dockerfile to create or switch to a non-root user and ensure WORKDIR /workspace/smoke is owned by that user before switching: either use the existing unprivileged 'node' user from the base image (add chown -R node:node /workspace/smoke and then USER node) or add a dedicated user (e.g., addgroup/adduser, chown the WORKDIR, and then USER smoke); keep the existing CMD ["node", "./feishu-ws-smoke.mjs"] unchanged but ensure the node binary and script are executable by the non-root user.apps/desktop/scripts/dist-win.mjs (1)
184-236: Consider adding error handling for the main orchestration.The
main()function runs multiple sequential build steps but has no top-level try/catch. If an early step fails, the error message fromrun()will propagate, but adding explicit error context could improve debugging for Windows-specific failures.💡 Optional: Add top-level error handling with context
async function main() { + const startTime = Date.now(); const env = { ...process.env, NEXU_WORKSPACE_ROOT: repoRoot, }; // ... rest of function ... - await runElectronBuilder( - [ - "--win", - // ... - ], - { - cwd: electronRoot, - env: { - ...env, - CSC_IDENTITY_AUTO_DISCOVERY: "false", - }, - }, - ); + await runElectronBuilder( + [ + "--win", + // ... + ], + { + cwd: electronRoot, + env: { + ...env, + CSC_IDENTITY_AUTO_DISCOVERY: "false", + }, + }, + ); + console.log(`[dist:win] completed in ${((Date.now() - startTime) / 1000).toFixed(1)}s`); } -await main(); +await main().catch((error) => { + console.error("[dist:win] build failed:", error.message); + process.exit(1); +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/scripts/dist-win.mjs` around lines 184 - 236, Wrap the orchestration in main() with top-level try/catch (or add a try/catch around the await calls when invoking main) to capture and log errors with Windows-specific context before exiting non-zero; catch errors thrown by run(), runElectronBuilder(), getElectronVersion(), execFileSync, etc., use a clear processLogger/error console message that includes the step or buildVersion/electronVersion context, and call process.exit(1) (or rethrow after logging) to ensure failures produce an explicit, contextualized error and non-zero exit code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/main/runtime/manifests.ts`:
- Around line 352-353: The branch that falls back to execFileSync("tar",
["-xzf", archivePath, "-C", tempExtractedSidecarRoot]) must be removed: when
archiveMetadata is missing or archiveMetadata.format === "tar.gz" replace the
PATH shell-out with an in-process extraction (e.g., require and call a JS tar
extractor such as the 'tar' module to extract archivePath into
tempExtractedSidecarRoot using streams and zlib) or, if we intentionally no
longer support legacy tar archives, throw a clear migration error referencing
archiveMetadata and archivePath; update the code paths around archiveMetadata,
execFileSync, and tempExtractedSidecarRoot accordingly so no PATH-dependent
external binary is invoked at startup.
In `@apps/desktop/scripts/dev-cli.mjs`:
- Around line 504-512: The cleanup currently kills whatever PID is in
state.electronPid and every PID returned by listListeningPids(defaultPorts);
change this to first validate each candidate PID before calling killPid by
checking its command line and/or working directory to ensure it belongs to this
workspace or is the expected electron binary. Specifically, when handling
state.electronPid (from readState) and each PID from
listListeningPids(defaultPorts), call a helper (e.g.,
getProcessCmdline/getProcessCwd) and only call killPid if the cmdline or cwd
contains the workspace identifier or matches the known electron executable name;
otherwise skip and log a warning so real port conflicts can surface (update the
logic around readState, state.electronPid, listListeningPids, and killPid to
perform this verification).
- Around line 679-689: Replace the manual file:// string assembly with
pathToFileURL to correctly encode Windows drive letters, spaces and UNC paths:
import pathToFileURL from the node 'url' module (e.g., add an ESM import for {
pathToFileURL }) and change the target in the control() function (the line that
currently sets target = `file://${join(appDir, "dist", "index.html")}`) to use
pathToFileURL(join(appDir, "dist", "index.html")).href (or .toString()) so the
spawn calls receive a correct, encoded file URL.
- Around line 26-35: createCommandSpec currently joins args into a single string
for cmd.exe which breaks when paths contain spaces; change it so on Windows you
either (A) quote each argument that contains spaces before joining (preserving
proper escaping) or preferably (B) avoid joining entirely and invoke the
resolved pnpm binary directly with an argument array to spawn/spawnSync.
Specifically, update createCommandSpec to resolve the pnpm binary
programmatically (use require.resolve or equivalent to locate the pnpm CLI
rather than relying on PATH) and return command set to that resolved binary and
args as an array (or, if sticking with cmd.exe, build a correctly quoted command
string by wrapping each arg with quotes when necessary) so that options like
"--dir" with rootDir/appDir containing spaces are passed intact. Ensure you
modify the branch that checks process.platform === "win32" and references "pnpm"
so it no longer constructs an unquoted "pnpm ...".
In `@apps/desktop/scripts/prepare-openclaw-sidecar.mjs`:
- Around line 472-495: The function currently waits for yazl's
ZipFile.outputStream to close but not for the destination write stream, which
can still be flushing; update createZipArchive (and the piping logic around
ZipFile.outputStream and createWriteStream) to capture the write stream (e.g.,
const writeStream = createWriteStream(archivePath)), pipe outputStream into
that, and wait for the write stream's 'finish' (and handle its 'error') before
resolving—i.e., replace or extend the existing completion promise that listens
on outputStream with listeners on the destination writeStream ('finish' and
'error') so the function only resolves once the file has been fully flushed to
disk.
In `@smoke/feishu-ws-smoke.mjs`:
- Around line 78-111: The argument parsing loop fails to check bounds and misses
a continue for the "--domain" flag; inside the function that iterates argv (the
for loop handling flags like "--reply", "--account", "--config", "--app-id",
"--app-secret", "--domain"), add a guard before reading argv[index + 1] (e.g.,
ensure index + 1 < argv.length) and handle the error case (return an error,
throw, or set a default) for each flag that consumes a value, and add the
missing continue after setting options.domain so control flow matches the other
flag cases.
---
Outside diff comments:
In `@apps/desktop/scripts/prepare-openclaw-sidecar.mjs`:
- Around line 990-1016: Update the Windows wrapper written by the writeFile call
that creates "openclaw.cmd" so it mirrors the POSIX wrapper fallback: instead of
unconditionally executing node with packagedOpenclawEntry, first check for node
on PATH (e.g. using where/node check) and exec node if found; otherwise, if the
OPENCLAW_ELECTRON_EXECUTABLE env var is set and points to an executable, run it
with ELECTRON_RUN_AS_NODE=1 and the same packagedOpenclawEntry and arguments as
the POSIX launcher. Keep the rest of the batch wrapper logic consistent with the
POSIX script (use packagedOpenclawEntry and preserve %* argument passthrough).
---
Nitpick comments:
In `@apps/desktop/scripts/dist-win.mjs`:
- Around line 184-236: Wrap the orchestration in main() with top-level try/catch
(or add a try/catch around the await calls when invoking main) to capture and
log errors with Windows-specific context before exiting non-zero; catch errors
thrown by run(), runElectronBuilder(), getElectronVersion(), execFileSync, etc.,
use a clear processLogger/error console message that includes the step or
buildVersion/electronVersion context, and call process.exit(1) (or rethrow after
logging) to ensure failures produce an explicit, contextualized error and
non-zero exit code.
In `@apps/desktop/scripts/lib/sidecar-paths.mjs`:
- Around line 67-84: The catch block for symlink failure silently falls back to
cp, so add a log entry before performing the copy to indicate the symlink failed
and we're using a copy fallback; specifically, inside the catch(error) branch
where process.platform === "win32" before calling cp(sourcePath, targetPath,
...), call the existing logger (or process.stdout/processLogger) to record a
message containing the error and the sourcePath/targetPath and that a recursive
copy will be used as fallback for symlink in the symlink => cp fallback code
path.
In `@apps/desktop/scripts/prepare-runtime-sidecars.mjs`:
- Around line 14-23: The createCommandSpec function builds a Windows cmd.exe
invocation using ["pnpm", ...args].join(" ") which doesn't quote individual
args; update createCommandSpec to safely quote or escape each argument when
composing the single string for cmd.exe (handle spaces and special chars), e.g.
transform args into a properly quoted string before passing it as the single /c
argument, keeping the same branch for command === "pnpm" || command ===
"pnpm.cmd" and leaving the non-Windows return { command, args } unchanged.
In `@openclaw-runtime/postinstall.mjs`:
- Around line 14-23: The Windows branch in createCommandSpec builds a single
command string with ["npm", ...args].join(" ") which fails for arguments
containing spaces or shell metacharacters; change this to safely escape and
quote each argument before joining (e.g., replace the join with
args.map(escapeAndQuote).join(" ") where escapeAndQuote properly escapes
backslashes/quotes and wraps the arg in double-quotes when needed) so the
returned args array contains a single, robust cmd.exe /c string that preserves
argument boundaries.
In `@smoke/Dockerfile`:
- Around line 1-5: The Dockerfile currently runs as root (base image
node:22-bookworm) which is unnecessary; modify the Dockerfile to create or
switch to a non-root user and ensure WORKDIR /workspace/smoke is owned by that
user before switching: either use the existing unprivileged 'node' user from the
base image (add chown -R node:node /workspace/smoke and then USER node) or add a
dedicated user (e.g., addgroup/adduser, chown the WORKDIR, and then USER smoke);
keep the existing CMD ["node", "./feishu-ws-smoke.mjs"] unchanged but ensure the
node binary and script are executable by the non-root user.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d323e986-21b4-41fb-a3f1-737e00964bee
⛔ Files ignored due to path filters (2)
openclaw-runtime/package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
.gitattributesAGENTS.mdapps/desktop/dev.shapps/desktop/main/index.tsapps/desktop/main/runtime/manifests.tsapps/desktop/package.jsonapps/desktop/scripts/dev-cli.mjsapps/desktop/scripts/dist-win.mjsapps/desktop/scripts/lib/sidecar-paths.mjsapps/desktop/scripts/prepare-controller-sidecar.mjsapps/desktop/scripts/prepare-openclaw-sidecar.mjsapps/desktop/scripts/prepare-runtime-sidecars.mjsapps/desktop/scripts/prepare-web-sidecar.mjsopenclaw-runtime/postinstall.mjspackage.jsonscripts/dev.mjsscripts/setup-git-hooks.mjssmoke/Dockerfilesmoke/README.mdsmoke/feishu-ws-smoke.mjssmoke/package.jsonspecs/guides/desktop-runtime-guide.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/main/bootstrap.ts`:
- Around line 93-101: Wrap the renameSync call in a try/catch to prevent an
unhandled exception from crashing startup on Windows: inside the existing
conditional that checks process.platform, overrideUserDataPath, userDataPath vs
legacyWindowsUserDataPath and path existence, change userDataPath from const to
let, attempt the renameSync(legacyWindowsUserDataPath, userDataPath) inside a
try block, and in the catch log the error (including error.code like
EBUSY/EPERM) and fall back to continuing startup without renaming or choose an
alternate recovery (e.g., leave legacy path untouched or set a different
userDataPath); ensure error handling references renameSync,
legacyWindowsUserDataPath, and userDataPath so the app no longer crashes on
rename failures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e8956844-9023-433e-b30f-93254a5b638b
📒 Files selected for processing (3)
apps/desktop/main/bootstrap.tsapps/desktop/package.jsonapps/desktop/scripts/dist-win.mjs
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/package.json
- apps/desktop/scripts/dist-win.mjs
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/scripts/dist-win.mjs (1)
14-23: Windows command arguments with spaces will break.When
repoRootor other arguments contain spaces, joining args with a single space on Line 18 produces an incorrectly tokenized command. For example,--dir C:/Program Files/projectbecomes three separate arguments.♻️ Proposed fix: Quote arguments containing spaces
function createCommandSpec(command, args) { if (process.platform === "win32" && (command === "pnpm" || command === "pnpm.cmd")) { + const quotedArgs = args.map((arg) => + /[\s&|<>^]/.test(arg) ? `"${arg.replace(/"/g, '\\"')}"` : arg + ); return { command: "cmd.exe", - args: ["/d", "/s", "/c", ["pnpm", ...args].join(" ")], + args: ["/d", "/s", "/c", ["pnpm", ...quotedArgs].join(" ")], }; } return { command, args }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/scripts/dist-win.mjs` around lines 14 - 23, In createCommandSpec the PNPM command string is built with args.join(" "), which breaks when any arg (like repoRoot) contains spaces; change the construction to safely quote/escape individual arguments before joining so cmd.exe receives a single command string with preserved arguments. Implement a helper (used in createCommandSpec) that for each arg escapes embedded double quotes/backslashes and wraps the arg in double quotes if it contains whitespace or special chars, then build args: ["/d","/s","/c", ["pnpm", ...args].map(quoteAndEscape).join(" ")]; this preserves tokenization on Windows when invoking "cmd.exe".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/desktop/scripts/dist-win.mjs`:
- Around line 14-23: In createCommandSpec the PNPM command string is built with
args.join(" "), which breaks when any arg (like repoRoot) contains spaces;
change the construction to safely quote/escape individual arguments before
joining so cmd.exe receives a single command string with preserved arguments.
Implement a helper (used in createCommandSpec) that for each arg escapes
embedded double quotes/backslashes and wraps the arg in double quotes if it
contains whitespace or special chars, then build args: ["/d","/s","/c", ["pnpm",
...args].map(quoteAndEscape).join(" ")]; this preserves tokenization on Windows
when invoking "cmd.exe".
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e9d3f990-ef5a-4c18-becc-9269def6d6b7
📒 Files selected for processing (2)
apps/desktop/package.jsonapps/desktop/scripts/dist-win.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/package.json
Switch the NSIS flow to an assisted installer so Windows install and uninstall are visible and data cleanup can be opted into without silent surprises. Restore executable resource editing and use tombstone-based app-data cleanup so shortcuts show the Nexu icon and uninstall stays fast without flashing a console window.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/build/installer.nsh`:
- Around line 53-56: The RunOnce registry value name currently uses only
GetTickCount() (System::Call 'kernel32::GetTickCount() i .r1') which can collide
when multiple tombstones are enqueued (written via WriteRegStr with
NEXU_RUNONCE_VALUE_PREFIX and NEXU_CLEANUP_SCRIPT), causing later writes to
overwrite earlier ones; change the value name generation in both the
single-write block (where GetTickCount() is used) and the multi-enqueue block
(the code handling multiple tombstones) to include a unique suffix per
tombstone—either append the tombstone basename (derived from the tombstone
path/argument like $1) or append an incrementing counter variable that you
increment for each WriteRegStr call—so each WriteRegStr uses
NEXU_RUNONCE_VALUE_PREFIX + uniqueSuffix to avoid collisions.
- Line 8: The installer currently writes and executes a predictable helper at
NEXU_CLEANUP_SCRIPT in %TEMP% and uses GetTickCount() for RunOnce names,
allowing TOCTOU and name-collision replay; fix by removing the mutable .vbs
helper and instead schedule the cleanup inline (for example create a one-shot
delete command in the RunOnce value itself or spawn cmd.exe /c "timeout & del
..." from an installer-owned, non-world-writable location), or if a helper file
is unavoidable write it to a unique, non-temporary, single‑use path (include a
GUID/PID/timestamp in the filename) and mark it non-replayable; also replace
GetTickCount() as the RunOnce key with a cryptographically-unique identifier
(GUID) or include PID+high-resolution timestamp to ensure uniqueness and
one-shot behavior.
In `@apps/desktop/scripts/dist-win.mjs`:
- Around line 169-170: The console.log that prints the entire env-derived config
object (the line using console.log("[dist:win] generated build-config.json from
env:", JSON.stringify(config))) may leak secrets; change it to avoid printing
verbatim config values—use the existing configPath and either log only the
resolved file path (configPath) or a redacted summary such as a list of config
keys or masked values for sensitive keys before or after the await writeFile
call; update the log message referenced here so it no longer includes
JSON.stringify(config) but instead uses a safe summary (e.g., configPath and/or
redactedKeys) to preserve intent without exposing secrets.
- Around line 37-63: The current try/catch blocks around preparing sharp and
`@img` swallow all errors (logging "skipping ...") which can hide failures and
leave stale native binaries; update the error handling in the sharp dereference
logic (around lstat(sharpPath), realpath(sharpPath), rm/sharp cp) and the `@img`
copy logic (pnpmImgPath/sharpImgPath, lstat, rm, cp) to fail fast: after logging
the error include rethrowing the caught error (or simply remove the outer
try/catch so the exception propagates). Ensure you reference the same symbols
(sharpPath, pnpmImgPath, imgPath, lstat, realpath, rm, cp, rmWithRetriesOptions)
so the error bubbles up to packaging instead of silently continuing.
- Around line 14-19: In createCommandSpec, when building the cmd.exe /c command
for pnpm, the current args.join(" ") loses argument boundaries and breaks paths
with spaces; fix this by quoting/escaping each element in the args array before
joining (e.g., wrap each arg in quotes and escape any embedded quotes) so
arguments like --dir "C:\Users\Jane Doe\..." remain one token, or alternatively
avoid cmd.exe entirely and return command: "pnpm" with args: [...args] on
Windows so the spawn call can pass the argument array directly; update the logic
in createCommandSpec to perform one of these two safe strategies.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0962f68b-d171-46ef-865b-41b1bba250c5
📒 Files selected for processing (3)
apps/desktop/build/installer.nshapps/desktop/package.jsonapps/desktop/scripts/dist-win.mjs
* build(desktop): skip exe editing for unsigned Windows dist * fix(openclaw-runtime): stabilize Windows postinstall Keep the full install path available for debugging while making pruned installs skip Windows-native optional dependencies that break postinstall flows. * fix(desktop): align dev health check with runtime state * fix(openclaw-runtime): refresh cache on installer changes * build(desktop): add windows CI coverage * fix(desktop): avoid sidecar copy symlink loops on windows * fix(desktop): clean runtime plugin self-links on windows * build(desktop): scope windows CI to build validation * fix(desktop): restore mac dist executable discovery --------- Co-authored-by: mRcfps <1402401442@qq.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/desktop-ci-check.mjs (1)
546-549:⚠️ Potential issue | 🟡 MinorOrphaned
tmuxSessionreference is dead code.
collectAppProcessResults()never setstmuxSessionon the returned object (it only returnsmainProcessandauxiliaryProcess). This reference will always evaluate totruevia optional chaining fallback.🧹 Suggested cleanup
function probesPassed(results, diagnostics) { return ( results.portResults.every((entry) => entry.listening) && results.apiReady.body.includes('"ready":true') && results.webReady.body.includes('"ready":true') && results.webSurface.body.includes('<div id="root"></div>') && results.openclawHealth.ok && results.browserControlListening && - results.appProcessResults.mainProcess.alive && - (results.appProcessResults.tmuxSession?.alive ?? true) && + results.appProcessResults.mainProcess.alive && diagnosticsChecksPassed(diagnostics) ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/desktop-ci-check.mjs` around lines 546 - 549, The condition references a non-existent tmuxSession property on results.appProcessResults (collectAppProcessResults only returns mainProcess and auxiliaryProcess), so remove the orphaned "(results.appProcessResults.tmuxSession?.alive ?? true)" check; if you intended to verify the auxiliary process instead, replace it with "results.appProcessResults.auxiliaryProcess?.alive ?? true" so the code uses the actual property returned by collectAppProcessResults and keeps the existing mainProcess.alive and diagnosticsChecksPassed(diagnostics) checks.
♻️ Duplicate comments (6)
apps/desktop/scripts/dist-win.mjs (3)
199-204:⚠️ Potential issue | 🟠 MajorAvoid logging build config verbatim—may leak DSN or credential-bearing URLs.
Line 202 logs the entire
configobject includingNEXU_DESKTOP_SENTRY_DSNandNEXU_UPDATE_FEED_URLif present. This undercuts the log-sanitizing goal of the PR. As per coding guidelines, credentials must never appear in logs.Suggested fix: log only the file path
await writeFile(configPath, JSON.stringify(config, null, 2)); - console.log( - "[dist:win] generated build-config.json from env:", - JSON.stringify(config), - ); + console.log("[dist:win] generated build-config.json:", configPath);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/scripts/dist-win.mjs` around lines 199 - 204, The current console.log prints the entire config object (variable config) which can expose secrets like NEXU_DESKTOP_SENTRY_DSN and NEXU_UPDATE_FEED_URL; update the logging in the block after writeFile(configPath, ...) so it does not emit the full JSON: either log only the file path (configPath) or a sanitized version of config that redacts DSNs/URLs (remove or mask keys NEXU_DESKTOP_SENTRY_DSN and NEXU_UPDATE_FEED_URL) before stringifying; locate the console.log call in the dist-win.mjs snippet and replace it accordingly.
41-76:⚠️ Potential issue | 🟠 MajorSilently skipping sharp/
@imgfailures may cause downstream packaging errors.Both try/catch blocks log "skipping" for all errors, but the electron-builder config unconditionally bundles these paths as
extraResources. A partial dereference leaves stale or missing binaries, causing cryptic failures later. Consider distinguishingENOENT(acceptable to skip) from other errors (should fail fast).Suggested fix: fail fast on non-ENOENT errors
+function isEnoent(error) { + return error && typeof error === "object" && "code" in error && error.code === "ENOENT"; +} + async function dereferencePnpmSymlinks() { // ... sharp handling ... } catch (error) { - console.log( - `[dist:win] skipping sharp: ${error instanceof Error ? error.message : String(error)}`, - ); + if (isEnoent(error)) { + console.log(`[dist:win] sharp not present; skipping dereference`); + } else { + throw new Error(`Failed to materialize sharp: ${error instanceof Error ? error.message : String(error)}`); + } } // ... similar for `@img` ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/scripts/dist-win.mjs` around lines 41 - 76, The current try/catch around dereferencing sharp (using lstat(sharpPath), realpath, rm, cp) and the subsequent block that copies `@img` (using sharpImgPath, lstat, rm, cp) swallow all errors; change both catch handlers to only silently ignore errors whose error.code === 'ENOENT' and for any other error rethrow or exit (e.g., throw error or process.exit(1)) after logging the full error (include error.stack or error.message) so failures other than "not found" fail fast and surface useful diagnostics.
15-27:⚠️ Potential issue | 🟠 MajorWindows argument quoting loses path boundaries.
The
args.join(" ")on line 22 breaks when--dirpaths contain spaces (e.g.,C:\Users\Jane Doe\...). cmd.exe parses the joined string and splits at spaces. This is a build-time script, so it may work in CI environments, but will fail on developer machines with spaces in usernames.Suggested fix: quote arguments containing spaces
function createCommandSpec(command, args) { if ( process.platform === "win32" && (command === "pnpm" || command === "pnpm.cmd") ) { + const quotedArgs = args.map((arg) => + arg.includes(" ") ? `"${arg}"` : arg + ); return { command: "cmd.exe", - args: ["/d", "/s", "/c", ["pnpm", ...args].join(" ")], + args: ["/d", "/s", "/c", ["pnpm", ...quotedArgs].join(" ")], }; } return { command, args }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/scripts/dist-win.mjs` around lines 15 - 27, In createCommandSpec, the current use of ["pnpm", ...args].join(" ") loses path boundaries because args.join(" ") doesn't quote arguments with spaces; update the logic that builds the cmd.exe invocation so each argument that contains spaces or special cmd characters is wrapped in quotes (and inner quotes escaped) before joining, i.e., map over args to quote them when necessary instead of a raw args.join(" "), so the returned object for the pnpm branch still uses command: "cmd.exe" and args: ["/d","/s","/c", quotedJoinedString].apps/desktop/scripts/dev-cli.mjs (3)
770-781:⚠️ Potential issue | 🟡 MinorUse
pathToFileURL()for correct Windows file URL construction.
file://${join(...)}produces invalid URLs on Windows—the drive letter is interpreted as a protocol and spaces are not encoded. UsepathToFileURL()fromnode:urlwhich correctly handles drive letters, path separators, and percent-encoding.Suggested fix
-import { fileURLToPath } from "node:url"; +import { fileURLToPath, pathToFileURL } from "node:url";async function control() { - const target = `file://${join(appDir, "dist", "index.html")}`; + const target = pathToFileURL(join(appDir, "dist", "index.html")).href;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/scripts/dev-cli.mjs` around lines 770 - 781, The file URL is being constructed manually in the control() function using `file://${join(appDir, "dist", "index.html")}`, which produces invalid URLs on Windows; import `pathToFileURL` from 'url' and replace that manual construction with `pathToFileURL(join(appDir, "dist", "index.html")).toString()` (or .href) to produce a correct, encoded file URL; update the top-level imports to include `pathToFileURL` and use this new `target` variable in the existing spawn calls inside control().
26-38:⚠️ Potential issue | 🟠 MajorWindows argument quoting loses path boundaries.
Same issue as other files:
args.join(" ")on line 33 breaks paths containing spaces. Forpnpm --dir rootDircommands whererootDirmay beC:\Users\Jane Doe\..., this causes command parsing failures.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/scripts/dev-cli.mjs` around lines 26 - 38, The Windows branch in createCommandSpec builds a single cmd.exe /c string with args.join(" "), which loses path boundaries for args containing spaces (e.g., C:\Users\Jane Doe). Update the pnpm branch in createCommandSpec to safely quote/escape each arg before joining (e.g., map each arg to a quoted/escaped form like wrap in double quotes and escape any internal quotes) so you produce ["/d","/s","/c", ["pnpm", ...args.map(escapeAndQuote)].join(" ")]. Ensure the escaping handles existing quotes in args and use the same createCommandSpec function to locate the change.
547-563:⚠️ Potential issue | 🟠 MajorResidual process cleanup may kill unrelated services.
killResidualProcessesterminates any process listening on ports 18789/50800/50810 and any PID stored instate.json, without verifying the process belongs to this workspace. A stale state file or an unrelated local service on these ports will be killed. Consider validating the process command line or working directory before terminating.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/scripts/dev-cli.mjs` around lines 547 - 563, killResidualProcesses currently kills any PID from state.json and any PID returned by listListeningPids(defaultPorts) without confirming ownership; update it to validate target processes before calling killPid by inspecting each PID's command line or working directory (use a helper like getProcessCommand/getProcessCwd or platform ps utilities) and only kill if the cmd/cwd matches this workspace or clearly belongs to our app (e.g., contains "electron" or the repo name). For the state.electronPid path (readState -> state.electronPid) also verify the PID's cmd/cwd matches before killing, and if a PID is stale/unrelated, avoid killing and consider cleaning state via removeState only when safe. Ensure you update any helpers and callers (killResidualProcesses, listListeningPids, killPid, readState, removeState, defaultPorts) to perform these checks so unrelated services are not terminated.
🧹 Nitpick comments (5)
apps/desktop/scripts/lib/sidecar-paths.mjs (1)
147-156: Consider parallelizing entry processing for large directories.The sequential
awaitloop processes entries one at a time. For directories with many entries, this could be slow. Parallelizing with bounded concurrency could improve performance.♻️ Optional: Parallel entry processing with Promise.all
- for (const entry of entries) { - await copyDirectoryTree( - resolve(sourcePath, entry), - resolve(targetPath, entry), - { - ...options, - baseSourcePath: options.baseSourcePath ?? sourcePath, - }, - ); - } + await Promise.all( + entries.map((entry) => + copyDirectoryTree( + resolve(sourcePath, entry), + resolve(targetPath, entry), + { + ...options, + baseSourcePath: options.baseSourcePath ?? sourcePath, + }, + ), + ), + );Note: Sequential processing is safer and may be intentional to avoid file system contention. Only apply if performance profiling shows this is a bottleneck.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/scripts/lib/sidecar-paths.mjs` around lines 147 - 156, The for-await loop in copyDirectoryTree that sequentially awaits copyDirectoryTree for each entry (the block iterating over entries) should be changed to run entries in parallel with bounded concurrency: create a concurrency limit (either add a concurrency option to copyDirectoryTree or use a lightweight limiter like p-limit/semaphore) and map entries to tasks calling copyDirectoryTree(resolve(sourcePath, entry), resolve(targetPath, entry), {...options, baseSourcePath: options.baseSourcePath ?? sourcePath}), then await Promise.all on the limited tasks; ensure errors propagate and respect existing behavior (preserve baseSourcePath handling) and avoid unbounded parallelism to prevent filesystem contention..github/workflows/desktop-ci-dev.yml (2)
98-100: Windows step validates build pipeline but not runtime health.The Windows step runs
pnpm --filter@nexu/desktopbuildwhich exercises the build tooling but not the full runtime unit health check that macOS performs viapnpm check:dev. This is reasonable for initial Windows smoke testing—consider expanding to runtime checks once the Windows dev-cli flow is stable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/desktop-ci-dev.yml around lines 98 - 100, The Windows workflow step titled "Verify Windows desktop build pipeline" currently only runs "pnpm --filter `@nexu/desktop` build"; update that step to also run the runtime health check used on macOS by invoking "pnpm check:dev" (either run it after the build or combine both commands) so the Windows job validates both the build and the dev runtime health for the desktop package.
84-90: Consider separating toolchain version checks by platform.The PowerShell block with embedded
matrix.oscomparison is functional but could be cleaner with separate platform-specific steps. However, this is a minor concern and the current approach works.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/desktop-ci-dev.yml around lines 84 - 90, The "Show desktop toolchain versions" step mixes platform logic inside a pwsh run block using "${{ matrix.os }}"; split this into two clearer steps instead: keep the existing "Show desktop toolchain versions" step to always run pnpm exec electron --version (shell: pwsh), and add a separate conditional step (e.g., name "Show tmux version on macOS") that runs tmux -V with an if: matrix.os == 'macos' condition and shell: bash/pwsh as appropriate; reference the step name "Show desktop toolchain versions" and the matrix variable "matrix.os" to locate where to extract the platform-specific command.scripts/desktop-check-dist.mjs (1)
34-46: Quoting pattern present but not exercised with path arguments.The
createCommandSpecfunction has the sameargs.join(" ")pattern, but in this file it's only used fortaskkilland simple commands without user-provided paths. Low risk in current usage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/desktop-check-dist.mjs` around lines 34 - 46, createCommandSpec currently builds cmd.exe args by joining args with " " which can mis-handle path arguments with spaces; update the branch that returns {command: "cmd.exe", args: [...] } so that instead of ["pnpm", ...args].join(" ") you either (a) preserve argument boundaries by passing each arg as a separate element to cmd.exe (e.g., args: ["/d","/s","/c","pnpm", ...args]) if the consumer supports it, or (b) safely quote/escape each element (map args through a quoting/escaping helper) before joining, ensuring functions like createCommandSpec, the pnpm branch, and the cmd.exe arg construction handle path arguments containing spaces correctly.openclaw-runtime/install-runtime.mjs (1)
9-21: Quoting pattern note forcreateCommandSpec.The
args.join(" ")pattern on line 16 can break if any argument contains spaces. For this script's usage (npm commands with flags like--omit=peer,--no-audit), this is safe since no user-provided paths are passed. However, this pattern is repeated across multiple files in this PR—consider extracting a shared utility if future use cases need path arguments.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openclaw-runtime/install-runtime.mjs` around lines 9 - 21, The current createCommandSpec function uses args.join(" ") which can break on arguments containing spaces; replace that ad-hoc join with a shared safe-quoting/joining utility (e.g., shellQuoteArgs or joinCommandArgs) and use it wherever args.join(" ") appears across the PR so arguments are correctly escaped when forming the cmd.exe command string; update createCommandSpec to call that utility instead of args.join(" "), and add the new utility in a common module to reuse it in other files.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@scripts/desktop-ci-check.mjs`:
- Around line 546-549: The condition references a non-existent tmuxSession
property on results.appProcessResults (collectAppProcessResults only returns
mainProcess and auxiliaryProcess), so remove the orphaned
"(results.appProcessResults.tmuxSession?.alive ?? true)" check; if you intended
to verify the auxiliary process instead, replace it with
"results.appProcessResults.auxiliaryProcess?.alive ?? true" so the code uses the
actual property returned by collectAppProcessResults and keeps the existing
mainProcess.alive and diagnosticsChecksPassed(diagnostics) checks.
---
Duplicate comments:
In `@apps/desktop/scripts/dev-cli.mjs`:
- Around line 770-781: The file URL is being constructed manually in the
control() function using `file://${join(appDir, "dist", "index.html")}`, which
produces invalid URLs on Windows; import `pathToFileURL` from 'url' and replace
that manual construction with `pathToFileURL(join(appDir, "dist",
"index.html")).toString()` (or .href) to produce a correct, encoded file URL;
update the top-level imports to include `pathToFileURL` and use this new
`target` variable in the existing spawn calls inside control().
- Around line 26-38: The Windows branch in createCommandSpec builds a single
cmd.exe /c string with args.join(" "), which loses path boundaries for args
containing spaces (e.g., C:\Users\Jane Doe). Update the pnpm branch in
createCommandSpec to safely quote/escape each arg before joining (e.g., map each
arg to a quoted/escaped form like wrap in double quotes and escape any internal
quotes) so you produce ["/d","/s","/c", ["pnpm",
...args.map(escapeAndQuote)].join(" ")]. Ensure the escaping handles existing
quotes in args and use the same createCommandSpec function to locate the change.
- Around line 547-563: killResidualProcesses currently kills any PID from
state.json and any PID returned by listListeningPids(defaultPorts) without
confirming ownership; update it to validate target processes before calling
killPid by inspecting each PID's command line or working directory (use a helper
like getProcessCommand/getProcessCwd or platform ps utilities) and only kill if
the cmd/cwd matches this workspace or clearly belongs to our app (e.g., contains
"electron" or the repo name). For the state.electronPid path (readState ->
state.electronPid) also verify the PID's cmd/cwd matches before killing, and if
a PID is stale/unrelated, avoid killing and consider cleaning state via
removeState only when safe. Ensure you update any helpers and callers
(killResidualProcesses, listListeningPids, killPid, readState, removeState,
defaultPorts) to perform these checks so unrelated services are not terminated.
In `@apps/desktop/scripts/dist-win.mjs`:
- Around line 199-204: The current console.log prints the entire config object
(variable config) which can expose secrets like NEXU_DESKTOP_SENTRY_DSN and
NEXU_UPDATE_FEED_URL; update the logging in the block after
writeFile(configPath, ...) so it does not emit the full JSON: either log only
the file path (configPath) or a sanitized version of config that redacts
DSNs/URLs (remove or mask keys NEXU_DESKTOP_SENTRY_DSN and NEXU_UPDATE_FEED_URL)
before stringifying; locate the console.log call in the dist-win.mjs snippet and
replace it accordingly.
- Around line 41-76: The current try/catch around dereferencing sharp (using
lstat(sharpPath), realpath, rm, cp) and the subsequent block that copies `@img`
(using sharpImgPath, lstat, rm, cp) swallow all errors; change both catch
handlers to only silently ignore errors whose error.code === 'ENOENT' and for
any other error rethrow or exit (e.g., throw error or process.exit(1)) after
logging the full error (include error.stack or error.message) so failures other
than "not found" fail fast and surface useful diagnostics.
- Around line 15-27: In createCommandSpec, the current use of ["pnpm",
...args].join(" ") loses path boundaries because args.join(" ") doesn't quote
arguments with spaces; update the logic that builds the cmd.exe invocation so
each argument that contains spaces or special cmd characters is wrapped in
quotes (and inner quotes escaped) before joining, i.e., map over args to quote
them when necessary instead of a raw args.join(" "), so the returned object for
the pnpm branch still uses command: "cmd.exe" and args: ["/d","/s","/c",
quotedJoinedString].
---
Nitpick comments:
In @.github/workflows/desktop-ci-dev.yml:
- Around line 98-100: The Windows workflow step titled "Verify Windows desktop
build pipeline" currently only runs "pnpm --filter `@nexu/desktop` build"; update
that step to also run the runtime health check used on macOS by invoking "pnpm
check:dev" (either run it after the build or combine both commands) so the
Windows job validates both the build and the dev runtime health for the desktop
package.
- Around line 84-90: The "Show desktop toolchain versions" step mixes platform
logic inside a pwsh run block using "${{ matrix.os }}"; split this into two
clearer steps instead: keep the existing "Show desktop toolchain versions" step
to always run pnpm exec electron --version (shell: pwsh), and add a separate
conditional step (e.g., name "Show tmux version on macOS") that runs tmux -V
with an if: matrix.os == 'macos' condition and shell: bash/pwsh as appropriate;
reference the step name "Show desktop toolchain versions" and the matrix
variable "matrix.os" to locate where to extract the platform-specific command.
In `@apps/desktop/scripts/lib/sidecar-paths.mjs`:
- Around line 147-156: The for-await loop in copyDirectoryTree that sequentially
awaits copyDirectoryTree for each entry (the block iterating over entries)
should be changed to run entries in parallel with bounded concurrency: create a
concurrency limit (either add a concurrency option to copyDirectoryTree or use a
lightweight limiter like p-limit/semaphore) and map entries to tasks calling
copyDirectoryTree(resolve(sourcePath, entry), resolve(targetPath, entry),
{...options, baseSourcePath: options.baseSourcePath ?? sourcePath}), then await
Promise.all on the limited tasks; ensure errors propagate and respect existing
behavior (preserve baseSourcePath handling) and avoid unbounded parallelism to
prevent filesystem contention.
In `@openclaw-runtime/install-runtime.mjs`:
- Around line 9-21: The current createCommandSpec function uses args.join(" ")
which can break on arguments containing spaces; replace that ad-hoc join with a
shared safe-quoting/joining utility (e.g., shellQuoteArgs or joinCommandArgs)
and use it wherever args.join(" ") appears across the PR so arguments are
correctly escaped when forming the cmd.exe command string; update
createCommandSpec to call that utility instead of args.join(" "), and add the
new utility in a common module to reuse it in other files.
In `@scripts/desktop-check-dist.mjs`:
- Around line 34-46: createCommandSpec currently builds cmd.exe args by joining
args with " " which can mis-handle path arguments with spaces; update the branch
that returns {command: "cmd.exe", args: [...] } so that instead of ["pnpm",
...args].join(" ") you either (a) preserve argument boundaries by passing each
arg as a separate element to cmd.exe (e.g., args: ["/d","/s","/c","pnpm",
...args]) if the consumer supports it, or (b) safely quote/escape each element
(map args through a quoting/escaping helper) before joining, ensuring functions
like createCommandSpec, the pnpm branch, and the cmd.exe arg construction handle
path arguments containing spaces correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d37a11bb-ba0f-4fae-9bb8-728e36541544
📒 Files selected for processing (19)
.github/workflows/desktop-ci-dev.yml.github/workflows/desktop-ci-dist.ymlapps/desktop/scripts/dev-cli.mjsapps/desktop/scripts/dist-win.mjsapps/desktop/scripts/lib/sidecar-paths.mjsapps/desktop/scripts/prepare-controller-sidecar.mjsapps/desktop/scripts/prepare-openclaw-sidecar.mjsapps/desktop/scripts/prepare-runtime-sidecars.mjsopenclaw-runtime/README.mdopenclaw-runtime/install-runtime.mjsopenclaw-runtime/package.jsonopenclaw-runtime/postinstall-cache.mjsopenclaw-runtime/postinstall.mjspackage.jsonscripts/clean-runtime-plugin-installs.mjsscripts/desktop-check-dev.mjsscripts/desktop-check-dist.mjsscripts/desktop-ci-check.mjssmoke/feishu-ws-smoke.mjs
✅ Files skipped from review due to trivial changes (2)
- openclaw-runtime/postinstall-cache.mjs
- openclaw-runtime/README.md
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/desktop/scripts/prepare-runtime-sidecars.mjs
- apps/desktop/scripts/prepare-controller-sidecar.mjs
- smoke/feishu-ws-smoke.mjs
- apps/desktop/scripts/prepare-openclaw-sidecar.mjs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c96eecd4c4
ℹ️ 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".
Deploying nexu-docs with
|
| Latest commit: |
54a2ca2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b32a1dd9.nexu-docs.pages.dev |
| Branch Preview URL: | https://feat-windows-distribution-sm.nexu-docs.pages.dev |
* feat: add local dev supervisor workflow * feat: refine local dev workflow and desktop runtime scaffolding * docs: add dev workflow faq * fix: remove nested controller tsx watcher * refactor: share ensure guards across dev process helpers * chore: remove stale task notes * refactor: centralize local dev path resolution * refactor: move dev orchestration into scripts/dev Keep @nexu/dev-utils focused on atomic helpers so service-level controller and web flows stay easier to reason about and recover. Add lightweight session tracing so leaked dev processes can be correlated and cleaned up without heavy self-healing. * refactor: clarify scripts dev module boundaries * feat: externalize dev runtime ownership * feat: split local dev into explicit service controls * refactor: remove legacy desktop dev launchers * fix: run desktop local dev through the Vite supervisor * fix: harden local dev stack flow * chore: sync workspace lockfiles * fix: restore desktop dev auth session
…#651) * chore: introduce shared desktop lifecycle contract * chore: move desktop platform lifecycle behind adapters * chore: centralize desktop platform compatibility * chore: stage patched OpenClaw runtime for local dev * chore: log staged OpenClaw runtime usage * chore: speed up windows desktop build iteration * chore: disable win executable editing for local builds * chore: ignore local build cache
mrcfps
approved these changes
Apr 2, 2026
anthhub
pushed a commit
that referenced
this pull request
Apr 3, 2026
* fix(desktop): make local startup work on Windows * fix(desktop): sanitize persisted startup logs * test(desktop): add Feishu websocket smoke harness * fix(smoke): improve Feishu websocket diagnostics * build(desktop): add Windows smoke distribution flow * build(desktop): align Windows sidecar archives with zip flow * build(desktop): align Windows packaging with NSIS defaults * build(desktop): refine Windows distribution commands * build(desktop): improve Windows installer UX Switch the NSIS flow to an assisted installer so Windows install and uninstall are visible and data cleanup can be opted into without silent surprises. Restore executable resource editing and use tombstone-based app-data cleanup so shortcuts show the Nexu icon and uninstall stays fast without flashing a console window. * fix(desktop): stabilize Windows dist and runtime install (#538) * build(desktop): skip exe editing for unsigned Windows dist * fix(openclaw-runtime): stabilize Windows postinstall Keep the full install path available for debugging while making pruned installs skip Windows-native optional dependencies that break postinstall flows. * fix(desktop): align dev health check with runtime state * fix(openclaw-runtime): refresh cache on installer changes * build(desktop): add windows CI coverage * fix(desktop): avoid sidecar copy symlink loops on windows * fix(desktop): clean runtime plugin self-links on windows * build(desktop): scope windows CI to build validation * fix(desktop): restore mac dist executable discovery --------- Co-authored-by: mRcfps <1402401442@qq.com> * build(desktop): stabilize Windows startup and shell polish * fix(web): refine Windows sidebar toggle layout * fix(desktop): harden Windows packaged runtime startup * feat: streamline desktop local dev workflow (#640) * feat: add local dev supervisor workflow * feat: refine local dev workflow and desktop runtime scaffolding * docs: add dev workflow faq * fix: remove nested controller tsx watcher * refactor: share ensure guards across dev process helpers * chore: remove stale task notes * refactor: centralize local dev path resolution * refactor: move dev orchestration into scripts/dev Keep @nexu/dev-utils focused on atomic helpers so service-level controller and web flows stay easier to reason about and recover. Add lightweight session tracing so leaked dev processes can be correlated and cleaned up without heavy self-healing. * refactor: clarify scripts dev module boundaries * feat: externalize dev runtime ownership * feat: split local dev into explicit service controls * refactor: remove legacy desktop dev launchers * fix: run desktop local dev through the Vite supervisor * fix: harden local dev stack flow * chore: sync workspace lockfiles * fix: restore desktop dev auth session * chore: unify scripts dev logging * chore: stabilize launchd encapsulation and windows desktop smoke flow (#651) * chore: introduce shared desktop lifecycle contract * chore: move desktop platform lifecycle behind adapters * chore: centralize desktop platform compatibility * chore: stage patched OpenClaw runtime for local dev * chore: log staged OpenClaw runtime usage * chore: speed up windows desktop build iteration * chore: disable win executable editing for local builds * chore: ignore local build cache * fix(desktop): support windows openclaw sidecar archives * chore: unify desktop dev launch under scripts/dev (#654) * chore(ci): add windows packaging to desktop dist full * chore(ci): opt desktop dist full into node 24 actions * Revert "chore(ci): opt desktop dist full into node 24 actions" This reverts commit b19336e. * fix(desktop): harden windows packaging and installer flow * chore: remove task handoff note * chore: align desktop tests with platform runtime changes * chore: fix linux desktop test platform mocks * chore: stabilize dev check service supervision * chore: fix packaged launchd path test platform setup * fix(desktop): preserve explicit dev runtime env overrides * fix(dev): preserve openclaw supervisor pid lock * chore(ci): track scripts dev changes in desktop dev checks * chore(ci): support manual desktop workflow dispatch * fix(dev): treat active openclaw port as running * fix(dev): align desktop runtime ports with CI contract * fix(dev): wait for openclaw gateway before controller start * chore(ci): capture dev logs and read desktop pid lock * fix(dev): wait for openclaw health before reporting ready * fix(controller): debounce sync on all skill dir events * fix(dev): derive openclaw status from runtime health * fix(dev): disable bonjour in desktop CI smoke * fix(desktop): harden Windows packaging and cleanup flows * fix(dev): stabilize Windows desktop dev lifecycle * fix(dev): allow overriding desktop dev ports * fix(dev): require openclaw readiness for running status * fix(dev): expose builtin openclaw extensions in local controller * fix(desktop): stabilize windows packaging workflows * fix: restore windows dev UI and openclaw startup compatibility * fix(desktop): clear stale startup errors after runtime recovery * fix(desktop): use packaged openclaw sidecars in windows builds * chore: refresh lockfile for desktop dependency changes * fix: make nexu runtime model plugin reload state by content --------- Co-authored-by: Marc Chan <mrc@powerformer.com> Co-authored-by: mRcfps <1402401442@qq.com>
lefarcen
added a commit
that referenced
this pull request
Apr 8, 2026
…esktop (#789) * feat: add desktop rewards center * feat(quota): add fallback and restore managed model endpoints - Introduced new API endpoints for triggering automatic fallback to BYOK provider and restoring the default model to a managed (cloud) model. - Updated OpenAPI schema to include new properties related to BYOK usage and fallback status. - Implemented QuotaFallbackService to handle fallback logic and model restoration. - Added BudgetWarningBanner component to notify users about credit status in the UI. - Enhanced existing services and routes to integrate new quota management features. * refactor(workspace-layout): enhance rewards link layout and styling - Updated the rewards link to use a flex layout for better alignment and spacing. - Increased the size of the Coins icon for improved visibility. - Adjusted the display of claimed and total credits to enhance readability. - Refined the progress bar styling for a more polished appearance. * feat(rewards): integrate cloud balance feature into rewards system - Updated OpenAPI schema to include cloud balance properties. - Implemented CloudRewardService for managing rewards status and claims. - Enhanced NexuConfigStore to fetch and convert cloud rewards status. - Added tests for CloudRewardService and updated existing tests for NexuConfigStore. - Updated UI components to display cloud balance information. - Localized new cloud balance strings in English and Chinese. * feat(budget-warning-banner): enhance budget warning banner functionality and localization - Introduced a status configuration for the BudgetWarningBanner to manage different states (warning and depleted) with corresponding styles and texts. - Updated the component to utilize the new configuration for rendering, improving code readability and maintainability. - Added new localization strings for budget warning and depleted states in English and Chinese. - Implemented a debug panel for development purposes to simulate budget banner states. - Added tests for the BudgetWarningBanner to ensure correct rendering based on status. * feat(desktop-rewards): implement auto-fallback logic and enhance rewards status handling - Added logic to automatically trigger a fallback to BYOK when the managed model's cloud balance is depleted. - Integrated logging for fallback failures to improve error tracking. - Created unit tests for the new fallback functionality to ensure reliability. - Updated the BudgetWarningBanner and related UI components to reflect changes in rewards status and actions. - Enhanced localization strings for clarity in user messaging regarding rewards and credits. * feat(desktop-rewards): implement auto-fallback logic and enhance rewards status handling - Added logic to automatically trigger a fallback to BYOK when the managed model's cloud balance is depleted. - Integrated logging for fallback failures to improve error tracking. - Created a new test suite for desktop rewards routes to validate fallback behavior and status retrieval. - Updated existing UI components and localization strings to reflect changes in rewards status messaging. * refactor(web): extract shared cloud connect hook and fix state bug - Remove 5 investigation markdown files accidentally committed to repo root - Fix cloudConnecting state not reset in "Connection already in progress" path - Extract handleCloudConnect + polling effects into shared useCloudConnect hook - Deduplicate formatRewardAmount by exporting from home-rewards-teaser * feat(rewards): add reward share assets and download functionality - Introduced six new PNG images for reward sharing. - Created a new module for managing reward share assets, including types and functions for selecting and downloading assets. - Implemented tests to ensure the integrity of the reward share assets and their download functionality. * chore: fix import sort order in home.tsx * feat(rewards): add loading skeleton, auto-fallback toast, and 60s polling - Show skeleton placeholders instead of fallback tasks during loading - Add autoFallbackTriggered flag to rewards status schema - Display warning toast when BYOK auto-fallback is triggered - Add refetchInterval: 60s to detect balance changes in background * feat(budget-warning-banner): update warning and depleted states with new headlines and actions - Refactored the BudgetWarningBanner to use new headline keys for warning and depleted states. - Updated the UI to include an upgrade action button and removed the previous earn credits button. - Enhanced localization strings for better clarity in user messaging regarding budget status. - Added a new RewardTaskIcon component for rendering task icons dynamically. * feat(rewards): enhance rewards display and localization - Updated the rewards status hook to disable retries for fetching data. - Added new localization strings for balance details in English and Chinese. - Improved the WorkspaceLayout to display rewards balance information with a detailed popup. - Refactored HomePage to manage session and channel data more effectively, ensuring proper rendering based on session history. - Added tests to verify the rendering of rewards components and their behavior during loading states. * feat(rewards): implement virtual reward check and enhance localization - Added a new module for handling virtual reward checks, including functions to manage task status and delay. - Updated localization strings in English and Chinese for task checking and claiming processes. - Refactored the RewardConfirmModal to utilize the new virtual check functionality, improving user feedback during reward processing. - Introduced tests to validate the behavior of the virtual reward check and description key retrieval. * fix(rewards): polish desktop credits flows * feat(controller): implement controller readiness management and budget banner enhancements - Introduced a new `ensureDesktopControllerReady` function to manage the readiness of the desktop controller, including polling and recovery logic. - Refactored the `DesktopShell` and `useDesktopRuntimeConfig` to utilize the new controller readiness management. - Enhanced the budget banner functionality in the `HomePage` to handle dismissal states more effectively, using session storage for tracking. - Added a new `BudgetDepletedDialog` component to provide user feedback when budget is exhausted. - Implemented tests for the controller readiness logic and budget banner dismissal behavior to ensure reliability. * feat(rewards): implement GitHub star verification and proof URL handling - Added a new `GithubStarVerificationService` to manage GitHub star session preparation and verification. - Enhanced the desktop rewards routes to include a new endpoint for preparing GitHub star verification sessions. - Updated the claim reward logic to validate proof URLs and GitHub session IDs before processing claims. - Introduced new localization strings for handling proof URL inputs and GitHub session errors. - Added tests to ensure the correct behavior of the new verification and proof handling features. * fix(rewards): harden desktop reward validation * chore: format skill watcher workspace test * fix: unify low-credit prompts and byok fallback * fix: move home budget banner below hero * fix: avoid zero balance while rewards sync * fix: route balance detail to environment billing * build(desktop): stabilize Windows dev and distribution workflows (#449) * fix(desktop): make local startup work on Windows * fix(desktop): sanitize persisted startup logs * test(desktop): add Feishu websocket smoke harness * fix(smoke): improve Feishu websocket diagnostics * build(desktop): add Windows smoke distribution flow * build(desktop): align Windows sidecar archives with zip flow * build(desktop): align Windows packaging with NSIS defaults * build(desktop): refine Windows distribution commands * build(desktop): improve Windows installer UX Switch the NSIS flow to an assisted installer so Windows install and uninstall are visible and data cleanup can be opted into without silent surprises. Restore executable resource editing and use tombstone-based app-data cleanup so shortcuts show the Nexu icon and uninstall stays fast without flashing a console window. * fix(desktop): stabilize Windows dist and runtime install (#538) * build(desktop): skip exe editing for unsigned Windows dist * fix(openclaw-runtime): stabilize Windows postinstall Keep the full install path available for debugging while making pruned installs skip Windows-native optional dependencies that break postinstall flows. * fix(desktop): align dev health check with runtime state * fix(openclaw-runtime): refresh cache on installer changes * build(desktop): add windows CI coverage * fix(desktop): avoid sidecar copy symlink loops on windows * fix(desktop): clean runtime plugin self-links on windows * build(desktop): scope windows CI to build validation * fix(desktop): restore mac dist executable discovery --------- Co-authored-by: mRcfps <1402401442@qq.com> * build(desktop): stabilize Windows startup and shell polish * fix(web): refine Windows sidebar toggle layout * fix(desktop): harden Windows packaged runtime startup * feat: streamline desktop local dev workflow (#640) * feat: add local dev supervisor workflow * feat: refine local dev workflow and desktop runtime scaffolding * docs: add dev workflow faq * fix: remove nested controller tsx watcher * refactor: share ensure guards across dev process helpers * chore: remove stale task notes * refactor: centralize local dev path resolution * refactor: move dev orchestration into scripts/dev Keep @nexu/dev-utils focused on atomic helpers so service-level controller and web flows stay easier to reason about and recover. Add lightweight session tracing so leaked dev processes can be correlated and cleaned up without heavy self-healing. * refactor: clarify scripts dev module boundaries * feat: externalize dev runtime ownership * feat: split local dev into explicit service controls * refactor: remove legacy desktop dev launchers * fix: run desktop local dev through the Vite supervisor * fix: harden local dev stack flow * chore: sync workspace lockfiles * fix: restore desktop dev auth session * chore: unify scripts dev logging * chore: stabilize launchd encapsulation and windows desktop smoke flow (#651) * chore: introduce shared desktop lifecycle contract * chore: move desktop platform lifecycle behind adapters * chore: centralize desktop platform compatibility * chore: stage patched OpenClaw runtime for local dev * chore: log staged OpenClaw runtime usage * chore: speed up windows desktop build iteration * chore: disable win executable editing for local builds * chore: ignore local build cache * fix(desktop): support windows openclaw sidecar archives * chore: unify desktop dev launch under scripts/dev (#654) * chore(ci): add windows packaging to desktop dist full * chore(ci): opt desktop dist full into node 24 actions * Revert "chore(ci): opt desktop dist full into node 24 actions" This reverts commit b19336e. * fix(desktop): harden windows packaging and installer flow * chore: remove task handoff note * chore: align desktop tests with platform runtime changes * chore: fix linux desktop test platform mocks * chore: stabilize dev check service supervision * chore: fix packaged launchd path test platform setup * fix(desktop): preserve explicit dev runtime env overrides * fix(dev): preserve openclaw supervisor pid lock * chore(ci): track scripts dev changes in desktop dev checks * chore(ci): support manual desktop workflow dispatch * fix(dev): treat active openclaw port as running * fix(dev): align desktop runtime ports with CI contract * fix(dev): wait for openclaw gateway before controller start * chore(ci): capture dev logs and read desktop pid lock * fix(dev): wait for openclaw health before reporting ready * fix(controller): debounce sync on all skill dir events * fix(dev): derive openclaw status from runtime health * fix(dev): disable bonjour in desktop CI smoke * fix(desktop): harden Windows packaging and cleanup flows * fix(dev): stabilize Windows desktop dev lifecycle * fix(dev): allow overriding desktop dev ports * fix(dev): require openclaw readiness for running status * fix(dev): expose builtin openclaw extensions in local controller * fix(desktop): stabilize windows packaging workflows * fix: restore windows dev UI and openclaw startup compatibility * fix(desktop): clear stale startup errors after runtime recovery * fix(desktop): use packaged openclaw sidecars in windows builds * chore: refresh lockfile for desktop dependency changes * fix: make nexu runtime model plugin reload state by content --------- Co-authored-by: Marc Chan <mrc@powerformer.com> Co-authored-by: mRcfps <1402401442@qq.com> * fix(i18n): update English and Chinese translations for rewards and balance tooltips; improve workspace layout with balance popup functionality * feat(rewards): enhance GitHub star session handling and add token support for rate limiting; update rewards page to manage session preparation and error handling * fix(rewards): simplify balance popup and enable github star auth token - Drop plan/consumed rows from sidebar balance popup so only the cumulative reward credits row remains, matching the latest design. - Update zh/en earned tooltip to spell out the consumption order (plan -> packs -> reward) and the funding sources. - GithubStarVerificationService now reads NEXU_GITHUB_TOKEN from env to authenticate against api.github.com (5000 req/h) and reports rate-limit headers plus auth state in error messages for easier diagnosis. - Remove the obsolete checkin total footer from the rewards page bottom strip. * refactor(workspace-layout): enhance balance popup tooltip functionality - Updated the balance popup to include a tooltip that appears on hover, providing additional context for earned rewards. - Improved the styling and accessibility of the tooltip for better user experience. * fix(i18n): update English and Chinese translations for balance popup earned credits * feat(api): add new endpoint for retrieving desktop-local auth session and update reward sharing tasks - Introduced the `/api/auth/get-session` endpoint to fetch the current desktop-local authentication session, including user and session details. - Updated reward sharing tasks to replace specific platforms with a generalized "mobile_share" task, enhancing flexibility in sharing options. - Revised English and Chinese translations for mobile sharing prompts and reward descriptions to reflect the new task structure. * refactor(rewards): remove unused desktop cloud status and simplify QR code sharing - Eliminated the use of desktop cloud status in the rewards page. - Updated QR code sharing functionality to use a fixed mobile share URL instead of resolving from cloud URL. * feat(rewards): add mobile QR code link hint and enhance mobile sharing UI - Introduced a new hint for the mobile QR code that directs users to the Nexu GitHub repository. - Improved the mobile sharing section's UI with enhanced styling and animations for better user experience. - Updated English and Chinese translations to reflect the new mobile QR code link hint. * fix(rewards): add mobile_share task id and surface silent cloud failures The rewards UI silently failed to render the cloud balance popup because the cloud API returned a `mobile_share` task that wasn't in `rewardTaskIdSchema`. Zod parse failed → cloud-reward-service returned parse_error → nexu-config-store fell through to the default branch returning cloudBalance: null and tasks: []. The error was swallowed by empty catches at every layer, so the only visible symptom was "clicking the credits button does nothing". - Add `mobile_share` to `rewardTaskIdSchema` enum and `rewardTasks` fallback array (group=social, icon=smartphone, image/weekly, requiresScreenshot, matching wechat/feishu/jike). - Regenerate openapi.json + web SDK. - Log cloud_rewards_status_{http,parse,network}_error in cloud-reward-service so future schema drift surfaces immediately. - Log desktop_rewards_status_cloud_fallback in nexu-config-store when the non-auth_failed branch falls through, so the dropped reason is visible in controller logs. * fix(rewards): wire credits popup detail button to cloud usage page The "view usage detail" button in the sidebar credits popup was navigating to the local /workspace/rewards page instead of the actual usage detail page on the connected cloud profile. Open the cloud profile's /workspace/usage URL in the system browser via openExternalUrl, mirroring how the cloud bill links work elsewhere. Also correct resolveCloudUsageUrl's fallback host from nexu.net to nexu.io (nexu.net is not the production domain). * feat(analytics): wire growth event tracking on PostHog base Add the analytics events the activation funnel needs from the desktop client side, on top of the PR #778 PostHog migration: - workspace_growth_rewards_click — fired when the sidebar "share nexu / earn extra credits" growth banner is clicked. - workspace_click_usage_detail — fired when the credits balance popup "view usage detail" button is clicked. - user_message_sent.state — controller analytics service now tags each user_message_sent with state="Success" or state="false". Failure is detected by openclaw:prompt-error transcript entries arriving before the assistant response; success is the default and gets confirmed when the assistant message lands. - AnalyticsAuthSource type now also accepts "home" so the home page can be the source of an auth click. The desktop side of the signup_success / login_success plumbing (source pass-through to cloud) is still pending — that side has to be done together with the cloud auth-init schema and is intentionally left out of this commit. The user_message_sent.credit_charged property the funnel spec also mentions cannot be implemented client-side: there is no per-message credit consumption pipeline yet. cloud's credit_usages table is empty of llm-call entries, link writes link.usage_events (USD cost) but nothing transforms USD into credit and writes credit_usages, and link does not record channel either. That work belongs to the cloud team under the existing credit consumption track and is documented in specs/current/credit.md. * develop menu: set balance * feat(auth): pass auth source from desktop to cloud through device-init The cloud auth page (apps/web/src/pages/auth.tsx in nexu-io/cloud) is already firing signup_success / login_success with the right method field, but it sources `source` from a `?source=...` query string and falls back to "welcome_page" otherwise. The desktop side never set that query string, so every auth event from the desktop client showed up as welcome_page regardless of where the user actually clicked. This wires the desktop side end-to-end so the cloud event gets the real source: - packages/shared: cloudConnectBodySchema (new) and cloudProfileConnectBodySchema both accept an optional `source` string. - controller (nexu-config-store / desktop-local-service / route): connect endpoints accept `source` in the request body and append it as `&source=…` to the cloud /auth browser URL. - apps/web hook (use-cloud-connect): handleCloudConnect accepts an optional AnalyticsAuthSource parameter and forwards it. - callers tagged with the page they fire from: - welcome.tsx → "welcome_page" (both initial connect and the retry-after-error path). - workspace-layout.tsx sidebar growth-card → derived from current route via existing isHomePage / isModelsPage flags (home / settings / fallback home). - rewards.tsx (task action and login CTA) → "home", since the rewards page is reached from home and the funnel spec only lists welcome_page / settings / home as valid sources. Cloud-side counterpart (still pending, separate PR in nexu-io/cloud): add "home" to the validSources allowlist in apps/web/src/pages/auth.tsx so the postApiV1MeAuthSource backfill accepts the new value. * fix(rewards): resolve CI failures and address Codex review feedback - rewards.tsx: guard i18n.language access with optional chaining to prevent SSR/test crash when i18n instance is not fully initialized - models.tsx: treat 'Connection attempt already in progress' as pending and keep polling instead of dropping user into error state (Codex P2) - controller-ready.ts: give final recovery attempt a bounded finalAttemptTimeoutMs (default 4x attemptTimeoutMs) so crash-looping controller eventually surfaces as false instead of leaking polling loops (Codex P2) - tests: align reward-share-assets / home locale / controller-ready tests with consolidated mobile_share task and new docs URL format * fix(desktop): stabilize packaged webview startup * fix(ci): assert web surface origin instead of /workspace pathname Verify packaged runtime unit health used to require that some embedded webview have lastUrl.includes("/workspace"), but that asserts product state ("the user has reached the workspace route") rather than runtime health. In fresh-state CI runs the renderer mounts /workspace, then the SPA's AuthLayout (apps/web/src/layouts/auth-layout.tsx:14-16) and WorkspaceLayout (apps/web/src/layouts/workspace-layout.tsx:346-349) immediately Navigate('/') because there is no auth session and SETUP_COMPLETE_KEY is unset. The desktop diagnostics reporter records contents.getURL() at did-finish-load time (apps/desktop/main/index.ts:1035-1056), so it captures the post-redirect URL — webview ends up with lastUrl="http://127.0.0.1:50810/" and the old check reported "workspace webview diagnostics are missing". Local runs accidentally passed because the developer machine had a persisted Local Storage with nexu_setup_complete=1 from earlier sessions, which suppressed the redirect. CI is ephemeral and never had it. The actual runtime health invariant is that an embedded webview successfully loaded a page from the local web sidecar's origin — the embed handshake worked, the renderer is alive, and there was no fail-load. The path under that origin (/, /workspace, /welcome, ...) is product state, not health. - Add webOrigin to readinessUrls (parsed from the web URL). - Replace getLatestWorkspaceWebview with getLatestWebSurfaceWebview that filters embedded entries by same-origin against webOrigin and ignores pathname entirely. - Thread webOrigin through diagnosticsChecksPassed, collectDiagnosticsFailures, probesPassed, verifyRuntime. - Failure message now reads "web surface webview diagnostics are missing (no embedded webview reported origin <origin>)". Verified locally with two fixtures: - {id:3, lastUrl:"http://127.0.0.1:50810/"} → matches (used to miss) - {id:3, lastUrl:"http://127.0.0.1:50810/workspace"} → still matches * fix(desktop): show NexuLoader instead of error card while controller is starting (#893) On relaunch, users saw a white card titled "Starting controller..." with "Retry controller" and "Open control plane" buttons during the short window where the local controller was booting but not yet ready. It looked like the app had landed on an error page instead of the home page, which is what issue #876 reported. The only difference between the "polling" and "failed" states in the old fallback UI was the heading text — both rendered the same card and the same Retry button, so users could not tell "still starting" from "actually broken". Split the fallback into three branches: - desktopWebUrl ready -> SurfaceFrame with the webview (unchanged) - controller failed / recovering -> the original error card with Retry - normal polling -> SurfaceFrame with src=null, which reuses the built-in NexuLoader overlay that SurfaceFrame already shows before webview did-finish-load The polling loader and the post-mount webview loader are now the same NexuLoader component with the same animation, so the transition from "controller starting" to "home page" is visually seamless and never exposes any text or buttons that could be mistaken for an error page. Closes #876 * refresh after set balance * perf(desktop): unblock event loop during packaged sidecar extraction The async openclaw sidecar materializer was named "Async" and wrapped in async/await, but it called execFileSync("/usr/bin/tar", …) and rmSync(…) internally. execFileSync blocks the Node.js event loop until the child process exits, so for the ~14s tar extraction on first install / post-update the main process could not respond to anything, including IPC handshake from the renderer's preload. As a result the renderer's loadFile took ~28s to fire did-finish-load (measured locally: 12:13:42.805 dispatched → 12:14:10.463 finished, while extraction ran 12:13:42 → 12:13:56), and the setup-animation video did not start playing until extraction was complete — defeating the entire point of showing the window with a white background "before loadFile, before React, before anything" (apps/desktop/main/index.ts:945-955). Switch the tar invocation to promisify(execFile), which still spawns a real /usr/bin/tar child process but waits for it via libuv asynchronously so the event loop stays responsive. Also replace the matching rmSync of the previous extracted sidecar root with the async fs/promises rm to keep the function uniformly non-blocking. Sync materializer (createSyncTarSidecarMaterializer) is left untouched because it is intentionally synchronous for early-bootstrap call sites. Effect: renderer mounts and the setup-animation video starts playing within seconds of window.show(), in parallel with extraction, instead of after extraction finishes. * fix(desktop): refresh packaged runtime on build changes Avoid reusing stale extracted runtime sidecars when a rebuilt app keeps the same app version but changes CFBundleVersion, and keep the desktop rewards balance popup reachable while cloud balance data is still loading. * fix(desktop): tear down stale runtime before replace When packaged runtime identity changes, boot out launchd services and clear surviving Nexu processes before replacing the extracted runner and controller sidecar so reinstall flows cannot keep stale runtime code alive. * fix(desktop): sign openclaw native sidecar binaries Sign all packaged OpenClaw native binary candidates directly during mac builds and invalidate stale archived sidecar payloads so notarization cannot fail on unsigned canvas/skia artifacts. * fix(rewards): address remaining PR review feedback Recover stale desktop cloud connect sessions, validate reward group IDs and profile renames more defensively, and route desktop test balance operations through the host bridge instead of renderer-origin fetches. * test(desktop): make webview preload URL test portable Build the expected file URL with Node path/url helpers so the desktop preload URL assertion passes on Windows as well as Unix runners. --------- Co-authored-by: PerishFire <39043006+PerishCode@users.noreply.github.com> Co-authored-by: Marc Chan <mrc@powerformer.com> Co-authored-by: mRcfps <1402401442@qq.com> Co-authored-by: lefarcen <935902669@qq.com> Co-authored-by: nettee <nettee.liu@gmail.com>
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Stabilize the Windows desktop path end to end across local development, packaged distribution, CI validation, and installer UX, while continuing the desktop platform/lifecycle cleanup that supports those flows.
Why
This branch grew into the main Windows stabilization line for the desktop product. Before these changes, the Windows path still had gaps in several places: local startup could be brittle, packaged runtime behavior still depended on platform-specific assumptions, Windows packaging was only partially covered in CI, and repeat installer runs did not clearly communicate upgrade, reinstall, or downgrade behavior. In parallel, parts of the desktop runtime lifecycle were still too spread across launchd- and platform-specific codepaths, which made the Windows work harder to extend safely.
How
dist:winpackaging flow and smoke-distribution supportdesktop-ci-dist-fullscripts/devflow and unify desktop dev launch underscripts/devscripts/devlogging while preserving the HMR-first control surfaceAffected areas
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpassespnpm generate-typesrun (if API routes/schemas changed)anytypes introduced (useunknownwith narrowing)Notes for reviewers
Desktop CI Dist Fullwas manually triggered and passed with the added Windows packaging job.pnpm --filter @nexu/desktop dist:winwas rerun locally after the latest icon, installer, and release-cleanup changes.