Skip to content

[orchestrator-v2] fix(desktop): Bundle staged runtime dependencies#3815

Closed
mwolson wants to merge 1 commit into
pingdotgg:t3code/codex-turn-mappingfrom
mwolson:fix/desktop-bundling-deps
Closed

[orchestrator-v2] fix(desktop): Bundle staged runtime dependencies#3815
mwolson wants to merge 1 commit into
pingdotgg:t3code/codex-turn-mappingfrom
mwolson:fix/desktop-bundling-deps

Conversation

@mwolson

@mwolson mwolson commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

What Changed

  • Promote the staged desktop dependency closure from installed package metadata before writing the production stage manifest.
  • Explicitly stage cross-spawn v7 runtime dependencies used by the packaged backend.
  • Explicitly stage required ffi-rs and fff native packages, including glibc-only Linux packages for the Windows WSL backend.
  • Realpath package manifests before walking child dependencies so pnpm symlink issuers resolve nested virtual-store dependencies correctly.
  • Add unit coverage for Effect runtime deps, cross-spawn, native package promotion, pnpm symlink fallback, stage workspace config, and ASAR unpack config.

Why

Problem and Why it Happened Fix
Packaged desktop backend could miss runtime packages such as fast-check and find-my-way-ts because pnpm keeps them under the virtual store rather than top-level app node_modules. Walk installed dependency metadata and promote the staged production dependency closure into the stage manifest.
Some runtime imports are not visible from the initial production stage manifest, including cross-spawn runtime deps and native optional packages. Add targeted staging for cross-spawn v7 deps and platform native packages needed by ffi-rs and fff.
pnpm package fallback paths can be app-facing symlinks, which breaks nested dependency resolution from virtual-store siblings. Store realpathSync(packageJsonPath) in the closure walk and cover the fallback symlink layout in a unit test.
Windows artifacts bundle a WSL Linux backend, but the staged install config only enables Linux glibc packages. Stage only glibc Linux native packages for the Windows WSL supplement while preserving both glibc and musl for Linux artifacts.

Validation

  • Real pnpm repro: with only effect as a direct dependency, real pnpm installs fast-check and find-my-way-ts under the virtual store, not as top-level app dependencies. This is not aubeshim-only.
  • vp test scripts/build-desktop-artifact.test.ts
  • vp check
  • vp run typecheck
  • Grok follow-up review: the realpath change and symlink-layout test resolve the previously blocking pnpm nested-closure issue.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • Screenshots are not applicable because this does not change UI
  • Video is not applicable because this does not change animation or interaction

Note

Bundle staged runtime dependencies for ffi-rs and cross-spawn v7 in desktop artifact build

  • Adds resolveStagedDependencyClosure to walk the installed dependency graph from a set of direct dependencies, using issuer-aware resolution to correctly handle pnpm symlink layouts.
  • Adds resolveDesktopStageDependencies to compute the final desktop stage dependency set, including ffi-rs platform/arch native packages, cross-spawn v7 runtime dependencies (isexe, which, etc.), and a re-walk after promotions.
  • Expands DESKTOP_ASAR_UNPACK to include node_modules/@yuuang/ffi-rs-*/**/* alongside existing @ff-labs/fff-bin-* globs.
  • Behavioral Change: On Windows, only the GNU variant of Linux fff binaries is now included instead of both GNU and MUSL.

Macroscope summarized 1c46b1a.


Note

Medium Risk
Changes core desktop packaging dependency resolution and native binary staging; mistakes could omit runtime deps or ship wrong platform natives, but scope is limited to the build script with strong unit test coverage.

Overview
Fixes packaged desktop builds missing runtime modules that pnpm keeps in the virtual store by walking installed package manifests and promoting the full transitive closure into the production stage package.json before pnpm install --prod.

Adds resolveDesktopStageDependencies, which layers on top of that closure: known cross-spawn v7 runtime packages (which, isexe, etc.) when they would not appear in the manifest, and platform @yuuang/ffi-rs-* natives (plus glibc-only Linux ffi-rs on Windows for the bundled WSL backend). Manifest resolution realpathSyncs package.json paths so nested pnpm symlink layouts resolve correctly.

DESKTOP_ASAR_UNPACK now includes node_modules/@yuuang/ffi-rs-*/**/*. Linux fff native selection is refactored to share helpers with ffi-rs; Windows WSL fff staging uses glibc-only Linux bins instead of full gnu+musl.

Unit tests cover closure walking, duplicate package names, symlink layouts, cross-spawn promotion, and ffi-rs staging per platform.

Reviewed by Cursor Bugbot for commit 1c46b1a. Bugbot is set up for automated code reviews on this repo. Configure here.

@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: f0913670-38b6-4271-98a6-da2c08b7a3f4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Jul 8, 2026
Comment thread scripts/build-desktop-artifact.ts Outdated
@mwolson
mwolson marked this pull request as ready for review July 8, 2026 20:09
@macroscopeapp

macroscopeapp Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs human review

1 blocking correctness issue found. This PR introduces new dependency resolution logic for desktop packaging that determines which packages are bundled for end users. The transitive dependency closure walking and ffi-rs native package handling are non-trivial changes to build infrastructure that warrant human verification.

You can customize Macroscope's approvability policy. Learn more.

@mwolson
mwolson force-pushed the fix/desktop-bundling-deps branch from a6b05bf to 1c46b1a Compare July 8, 2026 20:16
@mwolson

mwolson commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Closing in favor of #3816 (merged to main).

Local verification: rebuilt the orchestrator-v2 integration stack from current upstream/main (includes #3816) without this PR's cherry-pick. The Linux AppImage still contains:

  • @yuuang/ffi-rs-linux-x64-gnu (and musl)
  • ffi-rs
  • fast-check, find-my-way-ts
  • cross-spawn, which

#3816 covers the ffi-rs optional-native packaging gap that blocked nightlies; the rest of the staged-closure work is no longer needed for local packaging once main is in the integration base. Thanks Julius for the pointer.

@mwolson

mwolson commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Superseded for packaging by #3816 on main; verified local AppImage without this branch.

@mwolson mwolson closed this Jul 9, 2026
@mwolson
mwolson deleted the fix/desktop-bundling-deps branch July 9, 2026 04:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant