feat(staged): replace bundled ACP bridges with app-managed Node runtime and floating npm installs - #899
Merged
Conversation
Step 1 of the app-managed Node runtime port from Berd: land the managed Node install machinery with no call sites yet. - node-runtime.lock.json pins Node v24.11.0 for all 4 supported target triples with official SHASUMS256.txt checksums; regenerated by scripts/update-node-runtime-lock.mjs (`just bump-node-runtime`), and the committed pins were verified by regenerating them from Artifactory's nodejs mirror - src-tauri/src/managed_node.rs streams the pinned tarball download (90 MB cap, incremental SHA-256), validates archive entries against absolute and `..` paths before unpacking, extracts to a temp dir, and atomically swaps into ~/.staged/packages/node/<version>/<platform>/ with .old rollback; the readiness probe requires `bin/node --version` to equal the pin and bin/npm to be present - installs and prunes take a cross-process flock on ~/.staged/packages/.lock on top of the in-process tokio mutex, since ~/.staged is shared by concurrently running Staged instances - prune_superseded_node_runtimes is a standalone fn never called from install, so shims execing a superseded runtime keep working until a fully successful reconcile epilogue prunes - new `no-block-npm-registry` cargo feature switches the download base from Block's Artifactory nodejs mirror to nodejs.org - packages_dir() helper in paths.rs roots the ~/.staged/packages tree Gates: just check-all passes (cargo fmt, clippy -D warnings, svelte typecheck, 537 Rust tests including 15 new managed_node tests covering lock parsing, archive-entry validation, the readiness probe, the file lock, and the standalone prune, frontend tests); the managed_node suite also passes under --features no-block-npm-registry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
…managed runtime Step 2 of the app-managed Node runtime port from Berd: doctor fixes stop touching the host global npm prefix, npm traffic routes through Block's Artifactory square-npm registry, and the node-runtime doctor check reports the managed runtime instead of host Node. - new src-tauri/src/managed_acp_tools.rs (first half — the floating installer and startup reconciler land in step 3): packages-tree path helpers (npm-prefix, bin, tools/<id>, state.json), managed_npm_env() setting upper+lowercase NPM_CONFIG_PREFIX/NPM_CONFIG_CACHE plus COREPACK_HOME (sanitize_shell_env already strips these keys from captured shell snapshots, so the pairs are authoritative), managed_prepend_dirs() (dev override -> bridge shims -> npm-prefix/bin -> managed node bin), the managed_tools_enabled() predicate, the STAGED_ACP_TOOLS_DIR reader (single owner of the env var now: set -> management disabled, override dir prepended first), and the square-npm registry URL gated behind the no-block-npm-registry feature - doctor.rs overlays the managed npm env on the doctor env snapshot, passes the square-npm registry in both run and fix options, ensures the managed runtime before executing any npm-backed fix or update command, and routes the node-runtime check's Fix button natively to ensure_managed_node_runtime instead of a shell command - the node-runtime check now reports the managed runtime: Pass when the pinned install answers the readiness probe, Warn + native reinstall fix when it is damaged or missing while Staged-installed npm tools need it, silent otherwise; the bundled-manifest/host-Node probe is gone (acp_tools' manifest helper itself goes with step 4's bundle flip) - acp_tools.rs folds managed_prepend_dirs() into apply_bundled_tools_env so session spawns and doctor checks/fixes share one PATH shape — bundled bridges keep winning until the step-3 resolution flip, but the managed node bin dir on PATH already retires the bundled wrappers' host-Node dependency once the runtime is installed; GOOSE_SEARCH_PATHS carries the same dir list - managed_node.rs: explicit truncate(false) on the flock file open (clippy 1.96 suspicious_open_options), and the packages-lock test now retries the post-drop acquisition with a 5s deadline — a child forked by a concurrently-running test can hold a dup of the just-closed lock fd until its exec closes it, which made the single non-blocking attempt flake under the full suite Effect: copilot/amp doctor fixes land in ~/.staged/packages/npm-prefix on the managed runtime and resolve identically at check time and at session spawn time; the host npm global prefix is no longer written. Gates: just check-all passes (cargo fmt, clippy -D warnings, svelte typecheck, 543 Rust tests, 481 frontend tests); clippy and the full Rust suite also pass under --features no-block-npm-registry. Pending manual verification for a later session: run a copilot or amp doctor fix and confirm the install lands in the private prefix, resolves in a doctor re-check and in a session spawn, and leaves the host `npm root -g` untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
…nciler Step 3 of the app-managed Node runtime port from Berd: the claude/codex ACP bridges become floating npm installs under ~/.staged/packages, installed and upgraded on every launch by a startup reconciler, with the bundled resources demoted to a search-path fallback until step 4 flips the bundle off. - managed_acp_tools.rs (second half): the MANAGED_TOOLS table (claude-acp and codex-acp, both vendoring their agent's full CLI) and install_managed_tool — ensure the managed runtime, run a floating `npm install <pkg>@latest --prefix tools/<id>` on the managed npm (15-min timeout, --ignore-scripts, square-npm registry unless no-block-npm-registry), floor-check the dist/index.js entrypoint in place of the old lock's integrity pins, write a #!/bin/sh shim execing the managed node by absolute path (temp + chmod + atomic rename), and record version + node_version in state.json; installs run in place so a failed offline upgrade keeps the prior version working, and every mutation of the shared packages tree holds the cross-process flock (managed_node's lock, now pub(crate)) on top of the in-process tool-install mutex, taken only after the runtime ensure releases the same flock since nesting it deadlocks - acp_tools_reconciler.rs, spawned from lib.rs setup: installs/upgrades every managed tool, then finish_reconcile prunes stale ids/shims/tool dirs, records the outcome in state.json, and — only when every install succeeded — prunes superseded Node runtimes, since a failed bridge's un-rewritten shim may still exec the old runtime; the acp-tools-reconciled {ok, providerIds} event goes out via emit_to_all (WebSocket fanout included) on success and failure alike, and failures retry next launch - acp_tools.rs resolution precedence flips to STAGED_ACP_TOOLS_DIR -> managed shim dir -> bundled resource dir; the new AcpToolsDirs carries the primary (registered with acp_client's find_command, labeled bundled by doctor so no manual-update nag) plus the resource dir as the trailing spawned-PATH fallback through session spawns and doctor env shaping - doctor.rs routes fixes and updates for ai-agent-claude/ai-agent-codex through install_managed_tool instead of the crate's `npm install -g`, falling back to the regular commands whenever management is off; DoctorCheckRow's readout label becomes "Managed by Staged" - frontend: lib/listeners/acpToolsListener.ts (mounted in App.svelte) re-runs provider discovery (force, past the 30-minute SWR cache) and any loaded doctor report on the reconcile event, so a fresh profile's agent picker unsticks without a manual refresh or restart - new no-managed-acp-tools cargo feature compiles the managed set empty and hides the shim dir from PATH prepends, so a restricted build cannot pick up shims another build left in the shared ~/.staged tree Gates: just check-all passes (cargo fmt, clippy -D warnings, svelte typecheck, 557 Rust tests including 14 new managed_acp_tools tests covering the tool table, shim writing, state round-trips, the fake-npm install flow, and the reconcile epilogue's success-gated prune, 481 frontend tests); clippy and the full Rust suite also pass under --features no-block-npm-registry,no-managed-acp-tools. Pending manual verification for a later session (needs a scratch HOME and network control): first launch installs both bridges and the picker unsticks without reopening; kill-mid-download recovers on relaunch; offline launch keeps a prior install working; STAGED_ACP_TOOLS_DIR pointed at a local claude-agent-acp checkout still wins. One accepted transitional window until step 4: in-process resolution (find_command) consults the shim dir, so on a brand-new profile the bridges surface in the picker only once the first reconcile lands — the bundled resource fallback covers spawned-env PATH resolution, not the OnceLock dir. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
…g machinery Step 4 (folding in the step-5 cleanup sweep) of the app-managed Node runtime port from Berd: packaged and dev builds no longer ship or stage the pinned ACP bridges — the startup reconciler's floating managed installs (steps 1-3) are the only Staged-supplied source of the claude/codex bridges now. - delete the build-time bundling machinery: acp-tools.lock.json and its 50h-cooling-off updater (scripts/update-acp-tools-lock.mjs), the dev-cache installer (scripts/ensure-acp-tools.sh), the host-Node wrapper template (scripts/lib/acp-node-wrapper.sh), the resource stager with its codesign sweep (scripts/prepare-acp-tools-resource.sh — runtime-installed files carry no quarantine xattr, so Gatekeeper needs no ad-hoc signing), the resources/acp tree with its repo-root gitignore entries, and the daily pin-bump workflow (.github/workflows/staged-bump-acp-tools.yml) — floating @latest makes the reviewed bump-PR pipeline moot - tauri.conf.json stops bundling resources/acp; staged-release.yml drops its "Stage bundled ACP tools" step; justfile: install and build/release-build lose the ensure/prepare calls, bump-acp-tools is gone, and dev stops staging bridges and exporting STAGED_ACP_TOOLS_DIR — dev instances share the managed installs in ~/.staged/packages, and the env var remains a manual local-checkout override - acp_tools.rs sheds the resolution machinery: AcpToolsDirs and the bundled-resource fallback collapse into primary_tools_dir() (dev override -> managed shim dir), apply_bundled_tools_env becomes apply_managed_tools_env computing the prepend dirs itself, and node_runtime_manifest_path — the last bundled-manifest remnant — is deleted - the doctor commands and session spawn needed an AppHandle only to resolve the Tauri resource dir, so run_doctor, run_doctor_freshness, run_doctor_fix, and run_doctor_update drop the parameter and web_server dispatch follows - docs/comment sweep: README's agent-setup section describes the managed installs instead of a manual npm install of the renamed @zed-industries package, and bundle-era comments in managed_acp_tools.rs, doctor.rs, lib.rs, session_runner.rs, DoctorCheckRow.svelte, and commands.ts are reworded Release-notes pointer: the old dev cache at ~/Library/Caches/staged-dev/acp-tools is orphaned by this change and safe to delete; nothing reads or repopulates it anymore. Gates: just check-all passes (cargo fmt, clippy -D warnings, svelte typecheck, 551 Rust tests, 481 frontend tests); clippy and the full Rust suite also pass under --features no-block-npm-registry,no-managed-acp-tools. Local `just build` succeeds and the built Staged.app contains no acp resources (only resources/pikchr/grammar.md; 46 MB .app, 20 MB DMG). Still pending from the plan: the manual no-host-Node matrix (fresh first run, offline-after-install, upgrade from a bundled-era install whose resource dir disappears). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
The startup reconciler floated the claude/codex ACP bridges to `<pkg>@latest` only once, at launch, so a Staged instance left running for days never picked up a freshly-published bridge until the next restart. Turn the one-shot startup pass into a loop that reconciles at launch and then once every 24h for the process lifetime, following the interval-loop pattern already used by pr_poll_scheduler and background_sync. - acp_tools_reconciler.rs: rename spawn_startup_reconcile -> spawn_reconcile_loop; the loop runs an immediate launch pass (the first tokio interval tick resolves right away) then re-runs on a RECONCILE_INTERVAL (24h) cadence. MissedTickBehavior::Skip collapses the catch-up burst after a multi-day sleep into a single reconcile. The loop returns early when nothing is managed (dev override / no-managed-acp-tools / unsupported target) so an unmanaged build never spins a daily no-op timer; reconcile() re-checks the same predicate. - lib.rs: call the renamed spawn and note the launch + daily cadence. - acpToolsListener.ts: the reconciled event now also fires on the daily pass, so the mirrored comments describe launch + daily and the `ok` field doc drops "this launch". Listener logic is unchanged — it already re-probes providers/doctor on every event. Each daily pass still installs in place, holds the same in-process mutex + cross-process flock, records the outcome in state.json, and success-gates the superseded-Node prune, so an offline daily pass never removes a working bridge — identical semantics to the launch pass. Gates: just check-all passes (cargo fmt, clippy -D warnings, svelte typecheck, 551 Rust tests, 481 frontend tests); clippy also passes under --features no-block-npm-registry,no-managed-acp-tools. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68ba7648c4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+445
to
+448
| run_floating_npm_install( | ||
| packages_root, | ||
| node_install_dir, | ||
| &install_dir, |
There was a problem hiding this comment.
When a bridge is already installed, this floating npm install runs directly against the live prefix before the new entrypoint is verified. If npm partially applies an upgrade, or if upstream @latest changes/removes dist/index.js, the later check returns an error but the previous package tree may already have been replaced while the old shim/state remain, so subsequent sessions execute a broken bridge instead of preserving the last working version. Install into a temporary prefix and atomically swap only after verification succeeds.
Useful? React with 👍 / 👎.
…tating the live prefix in place The floating bridge install ran `npm install <pkg>@latest --prefix tools/<id>` directly against the live prefix, then floor-checked the entrypoint afterward. Because the shim points at the version-independent `tools/<id>/node_modules/<pkg>/dist/index.js` path, an npm run that aborts mid-reify — or an upstream `@latest` that drops the entrypoint — could leave the live tree half-migrated while the still-valid shim and state pointed at it, so a session spawned before the next successful reconcile executed a broken bridge. The in-code claim that "a failed floating upgrade leaves the previous tree untouched" did not actually hold. (Flagged by PR review and by the branch's own review of 68ba764.) Mirror the sibling managed_node runtime swap: install into a scratch `tools/<id>.staging` prefix, floor-check its entrypoint, then atomically swap the verified tree into the live prefix with a `.old` rollback. The live tree is now only ever replaced by a rename of an already-verified tree, so a failed or partial install genuinely leaves the previous bridge in place. npm re-unpacks from the shared download cache each reconcile (no network re-fetch), which is an acceptable cost on the background launch/daily path in exchange for never serving a broken bridge. - managed_acp_tools.rs: install_npm_tool stages into staging_install_dir and swaps via new swap_into_place (with `.old` rollback) + reset_dir helpers; failure paths clean up the staging dir. Docs on install_managed_tool and tool_install_dir updated to describe the swap and drop the now-inaccurate in-place claim. - new failed_upgrade_preserves_the_previous_install test: a fake npm that wipes its --prefix node_modules and exits non-zero leaves the pre-installed tree, shim, and recorded version intact and leaves no scratch dirs — a regression that fails against the old in-place code. Gates: cargo fmt --check, cargo clippy --lib -D warnings (default and --features no-block-npm-registry,no-managed-acp-tools), and the full 552-test Rust lib suite (managed_acp_tools now 19 tests) all pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.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
1 participant
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.
Ports the app-managed Node runtime from Berd into Staged: the Claude Code and Codex ACP bridges stop being build-time bundled resources and become floating npm installs that Staged installs, upgrades, and resolves itself — on a Staged-managed Node.js runtime, through Block's Artifactory
square-npmregistry, inside a Staged-private npm prefix. Packaged builds no longer ship the bridges at all, and no host Node install is required.What changed
Managed Node runtime (
managed_node.rs,node-runtime.lock.json)Pins Node v24.11.0 for all four supported target triples with official
SHASUMS256.txtchecksums (regenerated byjust bump-node-runtime). Installs stream the tarball with a 90 MB cap and incremental SHA-256, reject archive entries with absolute or..paths, extract to a temp dir, and atomically swap into~/.staged/packages/node/<version>/<platform>/with.oldrollback. Every mutation of the shared~/.staged/packagestree holds a cross-processflockon top of an in-process mutex, since concurrent Staged instances share the tree.Private npm prefix for doctor (
managed_acp_tools.rs,doctor.rs)Doctor fixes and updates no longer touch the host global npm prefix — npm-backed installs land in
~/.staged/packages/npm-prefixon the managed runtime and resolve identically at check time and at session-spawn time. Thenode-runtimedoctor check now reports the managed runtime (Pass when the pinned install answers the readiness probe, Warn + a native reinstall fix when it is damaged or missing) instead of probing host Node.Floating bridge installs + reconciler (
managed_acp_tools.rs,acp_tools_reconciler.rs)Each managed tool is installed via
npm install <pkg>@latestintotools/<id>, fronted by a#!/bin/shshim that execs the managed node by absolute path. Installs run in place, so a failed offline upgrade leaves the prior version working. A reconciler spawned at setup installs/upgrades every managed tool, prunes stale ids/shims/tool dirs, records the outcome instate.json, emits anacp-tools-reconciledevent, and — only when every install succeeded — prunes superseded Node runtimes. It runs at launch and then every 24h, so a long-running instance picks up newly published bridges without a restart. On the frontend,acpToolsListener.tsre-runs provider discovery and any loaded doctor report on that event, so a fresh profile's agent picker unsticks without a manual refresh.Bundling machinery deleted
Removes
acp-tools.lock.jsonand its 50h cooling-off updater, the dev-cache installer, the host-Node wrapper template, the resource stager and its codesign sweep, theresources/acptree, and the daily pin-bump workflow — floating@latestmakes the reviewed bump-PR pipeline moot.tauri.conf.jsonstops bundlingresources/acp; the release workflow drops its staging step;just devno longer stages bridges (dev instances share the managed installs, andSTAGED_ACP_TOOLS_DIRremains a manual local-checkout override).acp_tools.rsresolution collapses to dev override → managed shim dir, andrun_doctor*lose theAppHandlethey only needed for the resource dir.Feature flags
no-block-npm-registryswitches downloads/registry from Artifactory to nodejs.org and the public npm registry;no-managed-acp-toolscompiles the managed set empty and hides the shim dir from PATH prepends, so a restricted build cannot pick up shims another build left in the shared tree.Notes for reviewers
~/Library/Caches/staged-dev/acp-toolsis orphaned by this change and safe to delete — nothing reads or repopulates it.Testing
just check-allpasses on every commit (cargo fmt, clippy-D warnings, svelte typecheck, 551 Rust tests, 481 frontend tests), including 29 new tests acrossmanaged_nodeandmanaged_acp_toolscovering lock parsing, archive-entry validation, the readiness probe, the file lock, shim writing, state round-trips, the fake-npm install flow, and the reconcile epilogue's success-gated prune. Clippy and the full Rust suite also pass under--features no-block-npm-registry,no-managed-acp-tools. A localjust buildsucceeds and the builtStaged.appcontains no acp resources (46 MB .app, 20 MB DMG).Still pending manual verification: the no-host-Node matrix (fresh first run, offline-after-install, upgrade from a bundled-era install whose resource dir disappears), a copilot/amp doctor fix landing in the private prefix while leaving host
npm root -guntouched, kill-mid-download recovery, andSTAGED_ACP_TOOLS_DIRpointed at a localclaude-agent-acpcheckout.