Repository navigation
fix(dsh): package the bridge profile on Windows - #2296
Merged
Merged
Conversation
The 0.2.18 Desktop Package run failed in `Package (windows-x64)` with `spawnSync npm ENOENT` out of `build-profile.mjs`, taking the whole `frontend:build-all` down with it. Three separate Windows assumptions: - `npm` is a `.cmd` shim there, and Node has refused to spawn one without a shell since CVE-2024-27980. A shell then re-splits every argument, so passing an absolute `--pack-destination` would break on its first space. Both `npm pack` and `tar` now run *in* the staging directory, which leaves their arguments as bare package names and one filename — no quoting to get wrong, and no drive letter reaching `tar -f`, which GNU tar would read as a remote host. - `copyTree`'s filter derived a basename by slicing on '/', which on a '\'-separated path yields the whole path and therefore matched nothing. A vendored tree would have dragged `node_modules` along. - `hashTree` recorded native separators, so the same sources produced a different content stamp per build host. Digests are unchanged on Unix (verified byte-for-byte against the previous script). `prepare:dsh-profile` runs only inside `frontend:build-all`, which no CI job invokes, so its first Windows execution ever was a release build. Add a small `windows-latest` job that runs the packaging and asserts the profile is complete, stamped, and carries nothing it must not ship. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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 broke
The 0.2.18
Desktop Packagerun failed inPackage (windows-x64)— run 31824204681.packages/dsh-acp/scripts/build-profile.mjsdied withspawnSync npm ENOENT,prepare:dsh-profilereportedprofile packaging failed, and that tookfrontend:build-all— and so the whole Tauri build — down with it. The other four platforms were unaffected.Three Windows assumptions in one script
npmis a.cmdshim. Node has refused to spawn.bat/.cmdwithout a shell since CVE-2024-27980, which is the ENOENT. The parent script (scripts/prepare-dsh-profile.mjs) already passesshell: process.platform === 'win32'; this one never did. Adding a shell is only half the fix, because a shell then re-splits every argument and the old call passed an absolute--pack-destinationthat would break on its first space. Sonpm packandtarnow both run in the staging directory: their arguments are bare package names and one tarball filename, there is nothing to quote, and no drive letter reachestar -f— which GNU tar (the one Git for Windows puts on PATH) reads as a remote hostname.copyTreesliced a basename on'/'. On a'\'-separated pathlastIndexOf('/')is-1, sobasebecame the entire absolute path and thenode_modules/.gitfilter matched nothing. Nowbasename().hashTreerecorded native separators, so identical sources produced a different content stamp per build host. Paths are now normalized to/. Digests are unchanged on Unix — verified byte-for-byte by running the previous script side by side (same56833147…,diff -rof the two trees clean).The reason this reached a release build
prepare:dsh-profileruns only insidefrontend:build-all, and no CI job invokes that —Rust Build Checkjustmkdirs an emptydist-profileto satisfy Tauri, andFrontend Buildcallsbuild:web/build:mobile-webdirectly. So the packaging script's first execution on Windows, ever, was a release build.This adds a
DSH Profile Packaging (windows-latest)job: it runs the real packaging and then asserts the profile is complete, stamped, and carries nothing it must not ship (nonode_modulesor source maps underlib/orpresets/, which is exactly what the separator bug would have produced). ~2 minutes, no Rust, no pnpm.Verification
npm ci→tsc→ profile packaging, both pinned packages vendored at the right versions (@agentclientprotocol/sdk@0.25.1,@deepseek-ai/dsh-agent-spine-demo@0.1.0-rc.6), stamp written.diff -rclean).packages/dsh-acp: 30 vitest tests,tsc --noEmitclean.node --test scripts/check-github-config.test.mjs(14) andpnpm run check:repo-hygienepass with the new job in place.🤖 Generated with Claude Code