fix(desktop): replace cpSync with manual recursive copy for Windows non-ASCII path compatibility

[liuhao1024]

What does this PR do?

Stages node-pty's native payloads without recursive fs.cpSync, which crashes in Node's native copy path when the source path contains non-ASCII characters (e.g. C:\Users\Pınar\... on Windows). Two symptoms of the same root cause are reported: Node 22 throws a catchable EIO/errno 5, and Node 24 fail-fasts with 0xC0000409 inside fsBinding.cpSyncCopyDir before any handler runs. Per-file copyFileSync never enters that native path, so both the conpty prebuild directory and build/Release subdirectories are now staged through copyDirByFile, a small recursive walk over copyFileSync.

This is a re-scope of the original patch onto the current build path: the .cjs script it patched was deleted on main; the active call sites are in apps/desktop/scripts/stage-native-deps.mjs. The earlier PowerShell-based retry loop is dropped entirely (single-file copies don't hit the bug, so no retry is needed — and the unguarded shell-out was a liability on non-Windows hosts).

Related Issue

Fixes #60447
Refs #70779 (bundles this EIO family with the 8.3 short-path family; also carries a second reproduction on a Chinese-username system where the same per-file workaround was verified end-to-end, and the Node 24 fail-fast variant)

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

• apps/desktop/scripts/stage-native-deps.mjs
  • add copyDirByFile(srcDir, destDir): recursive directory copy built on per-file copyFileSync, with a comment documenting the Windows non-ASCII source-path crash (EIO on Node 22, 0xC0000409 on Node 24)
  • copyBuildRelease: wholesale directory entries now go through copyDirByFile instead of cpSync(..., { recursive: true })
  • stageNodePtyInto: the conpty prebuild directory (the reported crash site) now goes through copyDirByFile
  • all single-file cpSync calls are unchanged (they never enter the native recursive path)
• apps/desktop/scripts/stage-native-deps.test.mjs
  • vi.mock('node:fs') simulates the Node 22 EIO/errno 5 for any recursive: true cpSync (delegating everything else to the real module), so every platform exercises the failure mode
  • new regression test stages a fake node-pty tree under a non-ASCII user directory (.../Pınar/hermes-agent/...) with a conpty/ payload, nested build/Release subdirectory, and filter-bait files (.pdb, README.md); staging must complete with byte-identical payloads and filtering preserved — it fails on main and passes with this fix

How to Test

1. cd apps/desktop && npx vitest run --project electron scripts/stage-native-deps.test.mjs
2. Observed result: 31 passed (31) with the fix; reverting only stage-native-deps.mjs to main makes the new non-ASCII source path test fail with the simulated EIO: Access is denied from the conpty directory copy, matching the original report's stack (stageNodePtyInto → cpSync of node-pty/prebuilds/win32-x64/conpty).
3. On a real Windows host with a non-ASCII user profile (C:\Users\Pınar), npm run build in apps/desktop previously failed in stage-native-deps; bug: Installation/Build fails on Windows when user path contains non-ASCII/Unicode characters (EIO Access Denied & path distortion) #70779 reports the same per-file workaround verified end-to-end on a Chinese-username system.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run pytest tests/ -q and all tests pass (JS-only change; full vitest suite for the touched file passes — 31/31)
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: macOS 15 (Apple Silicon) + verified against the Windows reproductions in stage-native-deps.cjs cpSync fails on Windows when user path contains non-ASCII characters #60447/bug: Installation/Build fails on Windows when user path contains non-ASCII/Unicode characters (EIO Access Denied & path distortion) #70779

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — or N/A (helper docstring documents the failure mode)
• I've updated cli-config.yaml.example if I added/changed config keys — or N/A
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A (the change exists specifically for Windows non-ASCII paths; per-file copies are behavior-identical elsewhere)
• I've updated tool descriptions/schemas if I changed tool behavior — or N/A

[teknium1]

Thanks for investigating the Windows packaging failure. This needs a re-scope onto current main before it can address the active build path.

Problems

• apps/desktop/scripts/stage-native-deps.cjs was deleted by 39d09453f95e8aefc0c97e5d9b30ff341cae9ed8; the current build invokes scripts/stage-native-deps.mjs in apps/desktop/package.json:22. The submitted patch therefore has no active call site on main.
• The active script still has recursive cpSync calls at apps/desktop/scripts/stage-native-deps.mjs:74 and :234, neither of which this patch reaches.
• The proposed retry path launches PowerShell after any failed copy attempt. As written in the PR diff, that is not platform-guarded and would obscure a copy failure on non-Windows hosts.

Suggested changes

• Reproduce against the current MJS script and apply the workaround only to the active call site(s) that exhibit the issue.
• Add focused regression coverage in apps/desktop/scripts/stage-native-deps.test.mjs for the updated copy path.

Automated hermes-sweeper review.

[liuhao1024]

Re-scoped onto the active .mjs build path as requested — thanks for the pointers, they made this straightforward to land correctly. Point by point:

• Active call sites: both remaining recursive cpSync sites in stage-native-deps.mjs are now replaced — the conpty prebuild directory in stageNodePtyInto (the reported crash site) and the wholesale directory branch of copyBuildRelease. Single-file cpSync calls are untouched; they never enter the native recursive copy path.
• PowerShell retry dropped entirely: the per-file copyDirByFile helper is a plain copyFileSync walk with no retry and no shell-out, so there's nothing platform-guarded to get wrong on non-Windows hosts.
• Regression coverage added in stage-native-deps.test.mjs: vi.mock('node:fs') makes any recursive: true cpSync throw the exact Node 22 EIO/errno 5 from the report (delegating everything else to the real module), then the test stages a fake node-pty tree under a non-ASCII user directory (.../Pınar/hermes-agent/...). It fails on main and passes with the fix, so the invariant "staging never depends on recursive cpSync" is now checked on every platform, not just Windows runners.

For context, #70779 (which bundles this EIO family) has since added a second reproduction on a Chinese-username Windows 11 system with two useful data points: on Node 24 recursive cpSync doesn't even produce a catchable error (fail-fast 0xC0000409 inside fsBinding.cpSyncCopyDir), and the per-file copyFileSync workaround was verified end-to-end there — which is exactly the approach this PR takes, so no catch-and-fallback layer is needed.

[ehz0ah]

Approval conclusion (formal approval unavailable without explicit repository access)

Reviewed exact head 97d9828bcfc87f19bece21f808768f871331ed3a. I found no blocking issue.

Motivation

The desktop native dependency staging path uses recursive cpSync for node-pty directories. On Windows, that operation fails when the source path contains non-ASCII characters. This blocks desktop build or packaging for affected users. The change keeps the staging contract and replaces only the failing directory-copy mechanism.

Approach

copyDirByFile walks known node-pty payload directories, creates matching destination directories, and copies each regular file with copyFileSync. stageNodePtyInto uses it for nested build/Release directories and prebuilds/<platform>-<arch>/conpty. The existing top-level native-file filters and destination cleanup remain unchanged.

The positive path now stages the same regular-file payload without recursive cpSync. Filesystem errors still propagate to the existing build or packaging caller. There is no new retry or failure-suppression path.

Changes

• apps/desktop/scripts/stage-native-deps.mjs adds the focused directory walker and replaces the two recursive cpSync calls that can reach the affected Windows paths.
• apps/desktop/scripts/stage-native-deps.test.mjs expands the filesystem mock and adds a non-ASCII-path regression case that verifies nested directory and file copying.

Both active production entry points are covered: the desktop build script and target-specific packaging through before-pack.mjs. The pinned node-pty@1.1.0 payload contains regular files only in these directories. Its own installation code also copies the relevant Windows payload files individually.

Risk to main

The main residual risk is that the helper intentionally ignores symlinks and special directory entries. Recursive cpSync would copy symlinks, but the pinned payload has none, so this does not change the current staging result. Regular-file content and modes matched the prior implementation in direct probes. Error propagation, overwrite behavior, and possible partial output after a copy failure remain compatible with the previous path.

Validation covered the focused suite at the PR head with 31 passing tests, the same change composed onto current main with 31 passing tests, and a counterfactual run against the base implementation where the new regression test failed as expected. Exact-head required CI is green, including JS and TS checks. OS-specific CI was skipped, so I am not treating Windows behavior as runner-verified here. The Windows failure and per-file success are supported by the linked reproduction and direct code-path analysis.

Overall assessment

This is a proportionate fix for the reported P1 packaging failure. It changes the two affected copy sites, preserves current payload semantics, and adds a regression test that fails without the production change. No P0-P2 defect was verified in the exact-head diff.

English verdict: APPROVE at 97d9828bcfc87f19bece21f808768f871331ed3a. The focused tests pass, the regression test fails against the base implementation, current-main composition is clean, and no blocking finding was identified.

[teknium1]

Fixed on main by #113919 (4146fcbd7a7), which lands @kosazoltan's #66687 — the same approach as here (libuv-backed copyFileSync walks instead of fs.cpSync/fs.rmSync in apps/desktop/scripts/stage-native-deps.mjs), widened to rmSync, before-pack.mjs and the get-windows staging. Thanks for the report and fix; closing as superseded.

[liuhao1024]

Thanks for the confirmation and the landing link! Glad to see this reach main via #113919 — the widened scope (the rmSync walk, before-pack.mjs cleanup, and the get-windows/rollback staging paths) goes beyond what this PR covered, so superseding is the right call. Appreciate the earlier re-scope pointers that pointed this investigation at the active .mjs call sites.