feat(toolchains): managed installs for node, uv, ripgrep - #98
Conversation
The daemon resolved external runtimes through identity no-op ports, so the headless deployment could only run npx/uvx agents and uvx MCP servers when Node/uv happened to be on PATH, with no verification and no toolchain UX. Modeled on ThinkInAIXYZ/deepchat#2193, re-designed for Argos' daemon-first architecture. - add a daemon-owned ToolchainService: explicit persisted sources (custom / unconfigured) over derived ones (managed / bundled / system), with precedence, a warm sync cache for sync host seams, and timestamped quarantine of corrupt state - managed installs download pinned Node (v24.18.0) and uv (0.9.18) archives, verify SHA-256, extract to staging, and activate atomically via rename; the previous tree rotates to .prev and a failed or cancelled install leaves it active - wire daemon consumers through the service: ACP launch resolves npx/npm/node/uvx (npx becomes node npx-cli.js via a new optional resolveCommandWithArgs host seam) and prepends resolved bin dirs to the spawn PATH; MCP stdio commands rewrite through the warm cache - probe bundled seeds for the headless daemon (execDir/../runtime, execDir/runtime, cwd/runtime, dataDir/runtime) - add a Toolchains settings page: per-tool source, path, version, install/repair, cancel, revert, custom path - fix bundled-runtime doc drift (no bundled Bun/rtk; seeds are uv + ripgrep) SDD: docs/features/managed-toolchains
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
React Doctor found 2 new issues in 2 files · 2 warnings · score 92 / 100 (Great) · 1 fixed · vs 2 warnings
Reviewed by React Doctor for commit |
Confidence Score: 1/5This PR is not safe to merge until managed runtimes are reliably available to MCP consumers, uvx fallback preserves valid invocation semantics, and managed installs can be reverted. Four blocking failures remain: MCP startup can race toolchain warmup, successful installs leave the synchronous resolver stale, uvx can be rewritten into an invalid uv invocation, and managed installations have no functional revert path. Files Needing Attention: apps/daemon/src/index.ts, apps/daemon/src/host/toolchains/service.ts, apps/daemon/src/host/toolchains/state.ts, packages/ui/settings/components/ToolchainsSettings.tsx
|
| Filename | Overview |
|---|---|
| apps/daemon/src/host/toolchains/service.ts | Central toolchain resolution and install orchestration has stale synchronous-cache behavior, an invalid uvx fallback, and no way to remove a managed tree. |
| apps/daemon/src/index.ts | Toolchain warmup is not ordered before MCP startup, allowing consumers to observe an empty cache. |
| apps/daemon/src/host/toolchains/state.ts | Explicit source persistence is structured appropriately, but corrupt recovery leaves the live file as invalid JSON. |
| apps/daemon/src/host/toolchains/install.ts | Implements verified staged activation and rollback, though several exceptional filesystem transitions remain weakly protected. |
| apps/daemon/src/host/acpPorts.ts | Adds asynchronous ACP command rewriting and resolved tool directories to spawned process environments. |
| apps/daemon/src/host/daemonMcpPorts.ts | Connects MCP runtime seams to synchronous toolchain resolution, making correct startup and cache refresh ordering essential. |
| packages/acp-runtime/src/process/acpProcessManager.ts | Correctly consumes the optional command-and-arguments rewrite before spawning an ACP agent. |
| packages/shared-contracts/src/routes/toolchains.routes.ts | Defines typed list, source, install, removal, and cancellation contracts for the new feature. |
| packages/ui/settings/components/ToolchainsSettings.tsx | Adds toolchain status and actions, but managed sources never qualify for the advertised Revert control. |
Prompt To Fix All With AI
### Issue 1
apps/daemon/src/index.ts:338-340
**Toolchain Warmup Race**
Toolchain warmup and enabled MCP server startup both run without being awaited or ordered. An `npx` or `uvx` MCP server can therefore use the synchronous resolver before its cache is populated, fall back to the daemon's `PATH`, and fail even when a bundled or managed runtime is available. Complete warmup before starting MCP servers that depend on this cache.
### Issue 2
apps/daemon/src/host/toolchains/service.ts:229-231
**Install Leaves Cache Stale**
A successful managed installation clears only the version cache and never refreshes `syncCache`. The install route resolves status immediately after starting the background job, before the new executable exists, so a later MCP launch can keep using the old unconfigured or system resolution and fail despite the completed install. Refresh the resolved tool entry after activation succeeds.
### Issue 3
apps/daemon/src/host/toolchains/service.ts:283-286
**Invalid uvx Fallback**
When an adjacent `uvx` executable is absent, this substitutes `uv` but leaves the arguments unchanged. Custom sources accept any existing uv path, while ACP launches pass arguments such as `some-package`; the resulting `uv some-package` invocation is invalid and prevents the agent or MCP server from starting. Invoke uv's equivalent subcommand or leave `uvx` unchanged when no sibling exists. The synchronous implementation at lines 319–322 has the same behavior.
### Issue 4
apps/daemon/src/host/toolchains/service.ts:183-188
**Managed Installs Cannot Revert**
`removeSource` deletes only a persisted custom or unconfigured choice, while a managed source is derived from the installed tree and has `explicit: false`. As a result, the UI does not show Revert for managed installs, and the managed tree continues to override bundled or system tools. Users cannot perform the advertised managed-to-bundled/system revert without manually deleting daemon data.
### Issue 5
apps/daemon/src/host/toolchains/state.ts:58
**Reset State Remains Corrupt**
Corrupt-state recovery writes an empty string back to `state.json`, which is still invalid JSON. If no source-changing action saves state afterward, every fresh daemon process treats the file as corrupt again and creates another quarantine copy. Persist a valid serialized empty state after preserving the corrupt content.
```suggestion
await Bun.write(filePath, JSON.stringify(emptyState(), null, 2)); // reset so the next load succeeds
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "feat(toolchains): managed installs for n..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| void toolchainService.warmup().catch((error) => { | ||
| logger.warn("[daemon] toolchain warmup failed:", error); | ||
| }); |
There was a problem hiding this comment.
Toolchain warmup and enabled MCP server startup both run without being awaited or ordered. An npx or uvx MCP server can therefore use the synchronous resolver before its cache is populated, fall back to the daemon's PATH, and fail even when a bundled or managed runtime is available. Complete warmup before starting MCP servers that depend on this cache.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/daemon/src/index.ts
Line: 338-340
Comment:
**Toolchain Warmup Race**
Toolchain warmup and enabled MCP server startup both run without being awaited or ordered. An `npx` or `uvx` MCP server can therefore use the synchronous resolver before its cache is populated, fall back to the daemon's `PATH`, and fail even when a bundled or managed runtime is available. Complete warmup before starting MCP servers that depend on this cache.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| // Inject an activating tick so clients observe the final phase. | ||
| advance("activating"); | ||
| this.versionCache.clear(); |
There was a problem hiding this comment.
A successful managed installation clears only the version cache and never refreshes syncCache. The install route resolves status immediately after starting the background job, before the new executable exists, so a later MCP launch can keep using the old unconfigured or system resolution and fail despite the completed install. Refresh the resolved tool entry after activation succeeds.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/daemon/src/host/toolchains/service.ts
Line: 229-231
Comment:
**Install Leaves Cache Stale**
A successful managed installation clears only the version cache and never refreshes `syncCache`. The install route resolves status immediately after starting the background job, before the new executable exists, so a later MCP launch can keep using the old unconfigured or system resolution and fail despite the completed install. Refresh the resolved tool entry after activation succeeds.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| } | ||
| const uvxName = process.platform === "win32" ? "uvx.exe" : "uvx"; | ||
| const uvx = path.join(binDirFor(uv.path), uvxName); | ||
| return { command: existsSync(uvx) ? uvx : uv.path, args }; |
There was a problem hiding this comment.
When an adjacent uvx executable is absent, this substitutes uv but leaves the arguments unchanged. Custom sources accept any existing uv path, while ACP launches pass arguments such as some-package; the resulting uv some-package invocation is invalid and prevents the agent or MCP server from starting. Invoke uv's equivalent subcommand or leave uvx unchanged when no sibling exists. The synchronous implementation at lines 319–322 has the same behavior.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/daemon/src/host/toolchains/service.ts
Line: 283-286
Comment:
**Invalid uvx Fallback**
When an adjacent `uvx` executable is absent, this substitutes `uv` but leaves the arguments unchanged. Custom sources accept any existing uv path, while ACP launches pass arguments such as `some-package`; the resulting `uv some-package` invocation is invalid and prevents the agent or MCP server from starting. Invoke uv's equivalent subcommand or leave `uvx` unchanged when no sibling exists. The synchronous implementation at lines 319–322 has the same behavior.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| async removeSource(tool: ToolchainName): Promise<ToolchainStatus> { | ||
| const state = await this.withState(); | ||
| delete state.sources[tool]; | ||
| this.versionCache.clear(); | ||
| await this.persist(state); | ||
| return this.status(tool); |
There was a problem hiding this comment.
removeSource deletes only a persisted custom or unconfigured choice, while a managed source is derived from the installed tree and has explicit: false. As a result, the UI does not show Revert for managed installs, and the managed tree continues to override bundled or system tools. Users cannot perform the advertised managed-to-bundled/system revert without manually deleting daemon data.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/daemon/src/host/toolchains/service.ts
Line: 183-188
Comment:
**Managed Installs Cannot Revert**
`removeSource` deletes only a persisted custom or unconfigured choice, while a managed source is derived from the installed tree and has `explicit: false`. As a result, the UI does not show Revert for managed installs, and the managed tree continues to override bundled or system tools. Users cannot perform the advertised managed-to-bundled/system revert without manually deleting daemon data.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| join(toolchainsDir(dataDir), `state.corrupt-${Date.now()}.json`), | ||
| await Bun.file(filePath).arrayBuffer(), | ||
| ); | ||
| await Bun.write(filePath, ""); // reset so the next load succeeds |
There was a problem hiding this comment.
Corrupt-state recovery writes an empty string back to state.json, which is still invalid JSON. If no source-changing action saves state afterward, every fresh daemon process treats the file as corrupt again and creates another quarantine copy. Persist a valid serialized empty state after preserving the corrupt content.
| await Bun.write(filePath, ""); // reset so the next load succeeds | |
| await Bun.write(filePath, JSON.stringify(emptyState(), null, 2)); // reset so the next load succeeds |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/daemon/src/host/toolchains/state.ts
Line: 58
Comment:
**Reset State Remains Corrupt**
Corrupt-state recovery writes an empty string back to `state.json`, which is still invalid JSON. If no source-changing action saves state afterward, every fresh daemon process treats the file as corrupt again and creates another quarantine copy. Persist a valid serialized empty state after preserving the corrupt content.
```suggestion
await Bun.write(filePath, JSON.stringify(emptyState(), null, 2)); // reset so the next load succeeds
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
🟡 Changes recommended
Startup races, platform gaps, recovery defects, and incomplete revert behavior can prevent reliable managed-toolchain operation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds daemon-managed Node, uv, and ripgrep resolution with verified installs, runtime integration, and settings UX.
Changes:
- Introduces toolchain discovery, persistence, installation, and route contracts.
- Integrates resolution into ACP and MCP process launching.
- Adds settings UI, tests, and architecture documentation.
File summaries
| File | Description |
|---|---|
packages/ui/settings/main.tsx |
Registers the Toolchains page. |
packages/ui/settings/components/ToolchainsSettings.tsx |
Adds toolchain status and controls. |
packages/ui/api/ToolchainClient.ts |
Wraps toolchain routes. |
packages/shared/src/settingsNavigation.ts |
Adds Toolchains navigation. |
packages/shared-contracts/src/routes/toolchains.routes.ts |
Defines toolchain contracts. |
packages/shared-contracts/src/routes/system.routes.ts |
Adds the settings route name. |
packages/shared-contracts/src/routes.ts |
Registers toolchain routes. |
packages/acp-runtime/src/process/acpProcessManager.ts |
Supports command-and-argument rewriting. |
packages/acp-runtime/src/host/ports.ts |
Extends the runtime host port. |
docs/features/managed-toolchains/tasks.md |
Records implementation tasks. |
docs/features/managed-toolchains/spec.md |
Documents requirements and decisions. |
docs/features/managed-toolchains/plan.md |
Describes the implementation plan. |
CONTRIBUTING.md |
Documents managed runtimes. |
apps/daemon/test/toolchainsState.test.ts |
Tests persisted state handling. |
apps/daemon/test/toolchainsService.test.ts |
Tests resolution and installation. |
apps/daemon/test/toolchainsRoutes.test.ts |
Tests route dispatching. |
apps/daemon/src/index.ts |
Constructs and injects the service. |
apps/daemon/src/host/toolchains/types.ts |
Defines internal toolchain types. |
apps/daemon/src/host/toolchains/state.ts |
Persists explicit source choices. |
apps/daemon/src/host/toolchains/service.ts |
Implements the service facade. |
apps/daemon/src/host/toolchains/locate.ts |
Discovers bundled and system tools. |
apps/daemon/src/host/toolchains/install.ts |
Implements verified activation. |
apps/daemon/src/host/toolchains/catalog.ts |
Defines pins, assets, and hashes. |
apps/daemon/src/host/daemonMcpPorts.ts |
Connects MCP runtime resolution. |
apps/daemon/src/host/acpPorts.ts |
Connects ACP runtime resolution. |
apps/daemon/src/host/acp-provider-execution.ts |
Injects toolchains into ACP execution. |
apps/daemon/src/dispatch/daemonDispatcher.ts |
Handles toolchain routes. |
AGENTS.md |
Updates runtime architecture guidance. |
Review details
Suppressed comments (2)
apps/daemon/src/host/toolchains/install.ts:143
- Cancellation after extraction rethrows without removing
stagingTarget, which now contains the complete extracted toolchain. The next attempt merely renames it to another.old-*directory, so repeated cancellations leak tens of megabytes. Remove the incoming tree before rethrowing the cancellation.
} catch (error) {
if (error instanceof ToolchainInstallError && error.code === "cancelled") {
throw error;
}
packages/ui/settings/components/ToolchainsSettings.tsx:183
- If the initial list request fails,
toolsremainsnull, so the page displays skeletons forever after the error toast and provides no retry path. Transition to an explicit error state with a retry action (or otherwise clear the loading state) in this catch.
} catch (error) {
toast({
title: "Could not load toolchains",
description: error instanceof Error ? error.message : String(error),
variant: "destructive",
- Files reviewed: 28/28 changed files
- Comments generated: 10
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| deps.toolchains ? deps.toolchains.resolveCommandSync(command, args) : { command, args }, | ||
| normalizePathEnv: (paths) => ({ key: "PATH", value: paths.join(":") }), | ||
| getDefaultPaths: () => [], | ||
| getDefaultPaths: () => deps.toolchains?.binDirsSync() ?? [], |
| const bytes = new Uint8Array(await response.arrayBuffer()); | ||
| ensureCancel(ctx, "download"); | ||
| const digest = createHash("sha256").update(bytes).digest("hex"); | ||
| if (digest !== archive.sha256) { | ||
| throw new ToolchainInstallError( |
| path.join(home, ".cargo", "bin"), | ||
| ); | ||
| if (home) { | ||
| dirs.push(path.join(home, ".nvm", "versions"), path.join(home, ".volta", "bin"), path.join(home, ".bun", "bin")); |
| // Inject an activating tick so clients observe the final phase. | ||
| advance("activating"); |
| await Bun.write( | ||
| join(toolchainsDir(dataDir), `state.corrupt-${Date.now()}.json`), | ||
| await Bun.file(filePath).arrayBuffer(), | ||
| ); | ||
| await Bun.write(filePath, ""); // reset so the next load succeeds |
| void toolchainService.warmup().catch((error) => { | ||
| logger.warn("[daemon] toolchain warmup failed:", error); | ||
| }); |
| {status.explicit ? ( | ||
| <Button size="sm" variant="ghost" disabled={busy} onClick={() => onRevert(status.tool)}> | ||
| Revert | ||
| </Button> | ||
| ) : null} |
| onClick={() => { | ||
| onSetCustom(status.tool, customPath.trim()); | ||
| setShowCustomInput(false); | ||
| setCustomPath(""); | ||
| }} |
| * uv archive filenames do not embed the version, so a pin bump without fresh | ||
| * hashes would pass compile-time checks and fail at first install — the | ||
| * catalog tests therefore assert every pin has complete non-empty hashes. |
Address reviewer findings on the managed toolchains PR: - await toolchain warmup before MCP servers start so npx/uvx rewriting never falls back to PATH mid-startup - refresh the warm sync cache after a successful install; emit the activating phase before the atomic rename instead of after completion - revert now removes the managed tree so managed installs can fall back to bundled/system; the UI shows Revert for managed sources - uvx fallback without a sibling binary becomes "uv tool run" instead of passing uvx arguments to bare uv (both sync and async rewrites) - corrupt toolchain state resets to a valid serialized empty state instead of an empty string that re-corrupted on every load - add darwin-x64 to the uv catalog (sha256 captured from the release) - stream downloads to disk while hashing instead of buffering the whole archive; clean the staging tree when extraction is cancelled - expand nvm version directories for system Node detection - Toolchains settings: explicit load-error state with retry; drop manual memoization flagged by react-compiler lint SDD: docs/features/managed-toolchains (review-hardening section)
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| ); | ||
| } | ||
|
|
||
| function ToolchainCard({ |
There was a problem hiding this comment.
React Doctor · react-doctor/no-high-complexity-react-function (warning)
ToolchainCard has cyclomatic complexity 18, cognitive complexity 24, and maximum nesting depth 3, so its React logic is hard to understand and change. Extract independent branches into components or hooks.
Fix → Extract independent render branches and state logic into focused components or hooks until the control flow is easy to follow.
Summary
The daemon resolved external runtimes through identity no-op ports, so the headless deployment (the first-class one per
distro/) could only runnpx-distribution ACP agents oruvxMCP servers when Node/uv happened to be on PATH — with no verification anywhere and no toolchain UX. Modeled on ThinkInAIXYZ/deepchat#2193 ("move Node and uv to managed installs"), re-designed for Argos' daemon-first architecture. SDD docs:docs/features/managed-toolchains/.Stacked on #97 (shares the dispatcher/routes-catalog files) — GitHub will retarget this to
masterwhen #97 merges.What it does
ToolchainService(apps/daemon/src/host/toolchains/) resolvesnode,uv, andripgrepthrough an explicit persisted source:custom | unconfigured(user choices, persisted) overmanaged | bundled | system(derived on demand, never persisted — a PATH refresh or removed seed can't leave a stale pointer). Corruptstate.jsonis quarantined under a timestamped name (the fixed-name collision flagged in upstream review is fixed from the start).v24.18.0, nodejs.org SHASUMS256) and uv (0.9.18, GitHub release assets — hashes captured from the actual artifacts) with SHA-256 verification, extract to staging, and activate atomically via rename. The previous tree rotates to.prev; a failed or cancelled install leaves the previous tree active. Same accepted deviation as upstream: the bundled uv seed stays, with the managed install as an override.npx/npm/node/uv/uvx(npx -y pkg→node npx-cli.js -y pkg— no.cmd/shell spawning) via a new optionalresolveCommandWithArgshost seam, and prepends resolved bin dirs to the spawn PATH. MCP stdio commands rewrite through the warm sync cache. Desktop hosts are unchanged.execDir/../runtime,execDir/runtime,cwd/runtime, anddataDir/runtime.settings-toolchains, tools group): per-tool source, resolved path, version, pin; install/repair, cancel, revert, and custom-path actions, with live progress phases while an install is in flight.Deliberately out of scope (documented in spec)
Windows login-shell PATH refresh, download resume, migrating desktop
RuntimeHelper/skills execution onto the service, per-skill python/node policy migration, and an aggregated missing-toolchain banner outside settings. Installer-size work (stop shipping seeds) is a follow-up build change.UI (settings → Toolchains)
Testing
npx/uvxrewrites, warm-cache bin dirs, install pipeline (checksum mismatch, cancel, atomic activation, Windows-gated rollback), dispatcher route surface.test:main1737 passed / 6 skipped.bun run typecheck(desktop + UI), daemon typecheck,bun run lint(route-catalog drift guard: 416 routes),bun run format— all clean.