MUL-5547: fix(daemon): detect Volta-managed CLIs - #6295
Conversation
…red shim Volta installs one volta-shim trampoline in ~/.volta/bin and symlinks every managed command to it, picking the tool to run from the name it was invoked as. resolveAgentExecutablePath canonicalized bare command names through filepath.EvalSymlinks, which collapsed claude/codex/pi onto that one shim path and discarded the only input that selects the tool. The shim then exits 126, so version detection failed and none of the runtimes registered: detect version for ~/.volta/bin/volta-shim: exit status 126 The pinned path also feeds agent.Config.ExecutablePath, so this broke task launches too, not just registration — the fix therefore belongs at path resolution rather than in the version probe. canonicalExecutablePath now keeps the caller's alias path when the symlink target is volta-shim, and resolves every other symlink as before. Placing the exception in that shared helper covers both call sites, including the ~/.multica/hooks unshadowing branch which canonicalizes independently. The alias is also the upgrade-stable path: it survives `volta install` replacing the versioned binary underneath it. Deliberately narrow — disabling symlink resolution generally would regress the PATH-drift pinning, the hooks recursion guard, and the MUL-4486 self-heal. Fixes #6183 Co-authored-by: multica-agent <github@multica.ai>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Record where the argv[0] dispatch behavior is defined upstream so the exception can be re-verified without re-deriving it: - get_tool_name() takes file_name() of argv[0] (volta-core/src/run/mod.rs) - get_executor() rejects the shim called by its own name (RunShimDirectly) - volta-shim's main exits 126 on any Volta error, which is the reported symptom - the binary is named volta-shim[.exe] (volta-layout/src/v1.rs) - Unix shims are symlinks to it; Windows shims are .cmd scripts instead - volta-cli/volta#579 documents the same 'resolving the symlink loses the tool name' hazard from Volta's own side Comment-only; no behavior change. Co-authored-by: multica-agent <github@multica.ai>
|
Validated the approach against Volta's upstream source rather than inference. Summary for reviewers: The dispatch key really is /// Determine the name of the command to run by inspecting the first argument to the active process
fn get_tool_name(args: &mut ArgsOs) -> Fallible<OsString> {
args.next()
.and_then(|arg0| Path::new(&arg0).file_name().map(tool_name_from_file_name))
.ok_or_else(|| ErrorKind::CouldNotDetermineTool.into())
}Go sets Calling the shim by its own name is an explicit upstream error, not an accident: Correction to the exit-code story. Err(Error::Volta(err)) => { report_error(...); session.exit(ExitCode::ExecutionFailure); } // = 126So the reporter's Volta documents this hazard from its own side. volta-cli/volta#579, filed by a Volta maintainer: "Git Bash on Windows will fully resolve symlinks before calling the executable, so shims don't work because the information about what tool was actually called is lost." Same failure class as ours, independently described — which is good evidence that "don't resolve the symlink" is the intended contract and not a workaround. Details that check out against the implementation:
Two honest scope notes surfaced by the research (neither changes the patch):
Pushed |
…alias
Addresses review feedback on the first pass, which kept the Volta alias path
and let the shim dispatch on argv[0].
That broke the {path, version} invariant the daemon depends on. The alias
resolves per working directory (`volta which` prefers a project-local bin), so
the version verified at registration was not necessarily the version a task
would execute, and the minimum-version gate could be bypassed. The claim that
every discovery round re-probes the version was also wrong: the refresh loop
skips convergeRuntimeRegistrations entirely when no provider is missing
(agents_refresh.go), and resolveAgentEntry returns the cached version whenever
the pinned path still exists — which an alias always does.
Now we ask Volta for the concrete binary (`volta which <cmd>`, invoked by
absolute path next to volta-shim since the daemon may not have Volta's bin dir
on PATH) and pin that. This puts Volta installs on the same footing as every
other install method and keeps the gated path identical to the launched path.
It is also exactly what the reporter's verified MULTICA_*_PATH workaround does.
Resolution failure is deliberately fail-closed: we keep the shim path, version
detection fails, and the provider stays unregistered rather than being launched
through an ungated path.
Also tightened per review:
- isVoltaShimPath now accepts only "volta-shim" / "volta-shim.exe" instead of
trimming any extension, which had matched volta-shim.bak and .wrapper.
- No path returns unresolved parent directories any more; the concrete answer
replaces the previous `return abs`, so directory canonicalization and the
PATH-drift guarantee are fully intact.
- `volta which` answers are cached process-wide and revalidated by existence,
so a steady-state daemon does not fork a subprocess on every discovery tick,
and a replaced binary is re-resolved.
Tests: fixture versions raised above agent.MinVersions (claude 2.0.0, codex
0.100.0) so registration is actually reachable, plus a registration-level
regression test through detectBuiltinRuntimes with the real version probe and
real min-version gate asserting claude/codex/pi all reach the payload. Added
fail-closed, cache-revalidation, symlinked-parent-dir, and near-miss coverage.
Co-authored-by: multica-agent <github@multica.ai>
multica-eve
changed the title
|
Thanks — all three blocking items addressed in 1. Version gate vs actual launch target (blocking). Accepted, and the diagnosis was correct on both counts. I verified the two claims:
Rather than mark the shim dynamic and re-gate at launch, I took the other option you named: resolve through
Two consequences I want to be explicit about rather than bury: this gives up Volta's project-local selection in favour of the default toolchain, which is the deterministic behavior the gate requires; and the concrete path can be stable across package upgrades, so version staleness there is the same pre-existing property every stable path has (Homebrew, npm global) rather than a Volta-specific invariant break. 2. Exception too broad (blocking). Both sub-points fixed:
3. Registration not covered (blocking). Fixed, and this caught a real hole — you were right that the old fixture's Also added cache-revalidation coverage ( Process nit. Title is now Verification: |
The fixture tests model Volta's shim behavior; this proves the model matches a
real installation, which is the one gap a fixture cannot close by construction.
Verified on macOS arm64 with Volta 2.0.2 (the reporter's version), claude-code
2.1.220 and codex 0.146.0:
- ~/.volta/bin/{claude,codex} are symlinks to a shared volta-shim, and
`volta-shim --version` exits 126 with Volta's "should not be called directly"
error — the reported symptom, reproduced.
- With the fix disabled, detectBuiltinRuntimes registers nothing and reports
both providers as "version detection failed: detect version for
.../volta-shim: exit status 126", matching the reporter's daemon log.
- With the fix, both resolve to exactly what `volta which` reports, clear the
real min-version gate, and reach the registration payload.
Gated on MULTICA_VERIFY_VOLTA_HOME so CI and ordinary `go test` runs skip it;
the file header documents the full setup for re-running it.
Co-authored-by: multica-agent <github@multica.ai>
|
The standing caveat is now closed: verified against a real Volta install, not just the fixture. macOS arm64, Volta 2.0.2 (the reporter's exact version), 1. The reported setup reproduces exactly. Both commands are symlinks to one shared shim, and the shim refuses to run under its own name: That confirms on real hardware what I had only derived from upstream source last turn — including that the 126 carries the 2. With the fix disabled, the daemon fails exactly as reported. Running the real Nothing registers, and the skip reasons match the reporter's daemon log line for line. 3. With the fix, both register. Same install, fix enabled: The pinned paths are byte-identical to what Two design assumptions also confirmed by the real layout rather than by reading:
Captured as
|
…nvironment
Addresses the four blocking review items on the previous pass. All were real;
each has a regression test verified to fail without its fix.
1. The GUI/login-shell fallback bypassed the new resolution. probeAgentCLIs and
reresolveAgentCommand wrote resolveAgentsViaLoginShell results straight into
AgentEntry.Path, and that script only canonicalizes the directory (it prints
$c/$f), so a Volta alias survived intact. Terminal-launched daemons pinned the
concrete binary while Desktop ones kept the dynamic alias — exactly the
inconsistency this fix exists to remove. Both legs now run canonicalExecutable.
2. `volta which` inherited the daemon's working directory. Volta resolves a
project-local bin before the user default (upstream src/command/which.rs), so
a daemon started inside a JS project could pin that project's dependency as
the machine-wide runtime. It now runs in the filesystem root, the only
directory with no ancestor that can hold a package.json.
3. A path alone is not runnable. Volta package bins are frequently
`#!/usr/bin/env node` scripts, and Volta supplies the Node platform bound to
the package when it launches them (volta-core/src/run/binary.rs). Resolution
now returns a resolvedExecutable{Path, PathDirs}; AgentEntry, the self-heal
pair, the registration version probe and the task launch all carry it, so the
gated version is produced in the same environment the task runs in. Confirmed
necessary on a real install: codex 0.146.0 is a node script and exits 127
without it, while claude 2.1.220 is a native binary and does not care.
4. Failure cost was additive. The old code held a global mutex across each 5s
timeout and never cached failures, so one wedged Volta install cost a timeout
per command. Resolution now runs outside the lock, failures start a cooldown,
and a per-install budget caps total spend. Measured: three hung aliases went
from 3.23s to 0.31s.
Also updates the opt-in real-Volta test, which previously masked item 3 by
putting Volta's bin dir (and therefore its node shim) on PATH.
Co-authored-by: multica-agent <github@multica.ai>
|
All four addressed in 1. GUI/login-shell fallback bypassed resolution. Correct, and the sharpest of the four: 2. 3. The path did not carry Volta's execution environment. This was the substantive one, and the real install settled it. The two CLIs behave differently: Resolution now returns You were also right that my earlier real-Volta test masked this by putting Volta's bin dir — and therefore its 4. Failure timeouts were additive. Confirmed: the old code held a global mutex across each 5s wait and never cached failures. Resolution now runs outside the lock, a failure starts a per-install cooldown, and a cumulative budget caps total spend. Verification
Note on scope. Item 3 required widening the diff beyond the daemon package: One residual I want to state rather than leave implied: |
CI failure was my own test, not the fix under test:
--- FAIL: TestVoltaResolve_BoundsTotalCostWhenVoltaHangs
resolving 3 hung aliases took 1.002604382s (> 600ms)
Two problems, both in the test:
1. The budget ignored cmd.WaitDelay. A hung shell leaves `sleep` grandchildren
holding our stdout, and cmd.Output() blocks in Wait() until WaitDelay forces
the pipes shut, so the real worst case for one call is timeout + WaitDelay,
not timeout. WaitDelay was hardcoded at 1s while the test shrank the timeout
to 300ms, so one legitimate call already blew a 2x-timeout budget on Linux.
macOS reaped the child sooner, which is why it passed locally.
2. Counting invocations by having the hung script append to a file raced with
the kill: the write became visible one call later (call 1 => 0 lines,
call 2 => 1 line), so the counter could never be trusted.
The invariant being tested is "ask a broken install once per cooldown, not once
per command", so runVolta is now an injectable var and the test counts calls in
Go and simulates the stall with a sleep. No real subprocess, nothing to race
with, and the wall-clock check is kept as a secondary guard with the corrected
timeout + WaitDelay arithmetic. WaitDelay also becomes a var so the test can
shrink both halves of that budget.
Added TestVoltaResolve_FailureCooldownExpires for the other half of the circuit
breaker: a transient failure must be retried after the cooldown, not written off
for the daemon's lifetime.
Verified with CI's own flags (scripts/test-go.sh uses -race): the two timing
tests pass 8/8 under -race at 0.16s against a 450ms bound, the full
./internal/daemon/... and ./pkg/agent/... suites pass under -race, and removing
the cooldown still reproduces "invoked 3 times for 3 commands, want 1".
Co-authored-by: multica-agent <github@multica.ai>
Addresses the five blocking review items. All five were real; each fix has a
regression test verified to fail without it.
1. The rebuilt environment was not Volta's. It used `volta which node`, i.e. the
*current default* Node, while Volta binds the Node a global package was
installed against and uses exactly that (upstream binary.rs
DefaultBinary::from_config reads bin_config.platform.node). It also omitted the
shared NODE_PATH that upstream sets via
`command.env("NODE_PATH", shared_module_path())`. Resolution now reads the
bound platform from $VOLTA_HOME/tools/user/bins/<cmd>.json and carries both the
bound Node bin dir and $VOLTA_HOME/tools/shared, so switching the default Node
no longer changes which Node an installed CLI runs under.
2. The context did not reach every consumer that launches a CLI. Model discovery
and the service-tier / thinking validators still passed only a path, so a Volta
package script would fail there (Codex silently degrading to its static
catalog). Introduced agent.ExecEnv and threaded it through ListModelsWithEnv,
ValidateServiceTierWithEnv and ValidateThinkingLevelWithEnv down to every exec
site, and folded it into the discovery cache key so one binary under two
toolchains cannot share a cached catalog. The existing zero-env entry points
are kept, so no other caller changes.
3. A transient Node lookup failure cached a permanently incomplete result: the
bool from the node query was ignored, "done" was recorded anyway, and validity
only checked the tool path. Resolution now fails closed unless BOTH halves
succeed, and cache validity covers the environment's directories, so a removed
Node image invalidates the entry.
4. The breaker latched open forever. `spent` accumulated for the process lifetime
and was never reset, so two near-timeout failures exhausted the 8s budget and
no later attempt was made even after every cooldown expired. The budget now
bounds one breaker window and resets with the cooldown. Concurrent callers are
merged through a singleflight group, so releasing the lock for the subprocess
can no longer let several requests past the budget check at once.
5. A GUI daemon with a custom VOLTA_HOME resolved against the wrong install.
`volta` inherited the daemon environment, and Volta reads package data relative
to VOLTA_HOME (defaulting to ~/.volta), so an uninherited or wrong value
described a different installation than the alias found. VOLTA_HOME is now
derived from the alias path itself — shims live in $VOLTA_HOME/bin — by walking
the symlink chain to the hop that points at the shim, and passed explicitly to
every `volta` invocation. The opt-in real-Volta test no longer exports
VOLTA_HOME, so it exercises that scenario instead of bypassing it.
Verified with CI's flags (scripts/test-go.sh --race) plus the real Volta install:
claude 2.1.220 and codex 0.146.0 both resolve with the bound Node and register
with VOLTA_HOME absent from the environment.
Co-authored-by: multica-agent <github@multica.ai>
|
All five addressed in 1. The rebuilt environment was not Volta's. Confirmed from Resolution now reads the bound platform from 2. The context did not reach every consumer. Added 3. A partial answer was cached permanently. Correct on all three counts — the node query's bool was ignored, "done" was set anyway, and validity checked only the tool path. Resolution now fails closed unless both halves succeed, and 4. The breaker latched open forever. Also correct, and this was the worst of the five since it was unrecoverable. 5. Custom VOLTA_HOME resolved against the wrong install. VOLTA_HOME is now derived from the alias path rather than the environment: shims live in You were right that the real-Volta test bypassed this by exporting VOLTA_HOME. It no longer does, and I found a second bug while fixing it: Verification
Scope note. Item 2 widened the diff into One residual, stated rather than hidden: the bound platform comes from Volta's on-disk bin config, which is internal layout rather than a public CLI contract. There is no public accessor for it ( |
Addresses the six blocking review items. All were real; each fix has a regression
test verified to fail without it.
1. The WithEnv chain was broken in three places and discovery was under-keyed.
thinking.go's validators still called the env-less ListModels; ACP discovery
built an env-aware command and then assigned os.Environ()+extraEnv over
cmd.Env, discarding it; and most dynamic discovery cached on the provider name
alone, so two CLI paths or two Volta homes reused each other's catalog inside
the TTL. The validators now use ListModelsWithEnv, ACP layers extraEnv on top
of the resolved environment (extracted as acpChildEnv so it is testable), and
all 15 cache sites key on provider + executable path + ExecEnv.
2. Volta state was not isolated per home. It keyed on the volta binary, but a
Homebrew install has several VOLTA_HOMEs sharing one volta/volta-shim pair, so
home B served home A's tool paths and environment. State is now keyed by
canonical home + binary.
3. A cached entry never self-healed its environment. resolveAgentEntry returned an
entry as long as the CLI file existed, so a Volta upgrade that prunes the old
Node image left every task launching with a PATH pointing nowhere. Validity now
covers the whole resolution (agentEntryLaunchable), so a stale entry is
re-resolved and atomically replaced by the existing self-heal.
4. The breaker could still latch permanently — this time via successes. Budget
accounting charged successful query time too, while success cleared failedAt,
so two slow-but-working resolutions exhausted the budget and no cooldown could
ever reset it. The budget now counts only failures and a success clears it. The
gate is also installation-level: concurrent resolutions of different commands
serialize behind one per-install mutex instead of each passing the check.
5. Only Node was restored. Volta's Image::bins() prefixes npm, pnpm and yarn image
bins before Node ("so that any custom version of npm will be earlier in the
PATH"), so an agent bound to a custom package manager could not find it. The
bound platform is now reconstructed in full, in Volta's order.
6. A custom NODE_PATH wiped Volta's required prefix. The resolved environment was
applied before custom_env, and NODE_PATH is not on the blocked-key list, so a
user value replaced the shared-modules prefix outright. Layering is now
custom_env first, resolved environment last (extracted as layerTaskEnvironment);
because ExecEnv prefixes, the user's value is preserved after Volta's entries.
Verified with CI's own command (scripts/test-go.sh --race) and against the real
Volta install: claude 2.1.220 and codex 0.146.0 still resolve with the bound
platform and register with VOLTA_HOME absent from the environment.
Co-authored-by: multica-agent <github@multica.ai>
|
All six addressed in 1. Broken
Validators now use 2. State not isolated per home. Right, and the Homebrew shape is the exact trigger: several homes, one 3. Cached environment never self-healed. Confirmed — 4. Breaker still latchable — via successes. This was the sharpest catch: successful query time was charged to the budget while success cleared 5. Only Node restored. Confirmed against 6. Custom Verification
On your |
1 participant
Fixes #6183. Multica issue: MUL-5547.
Background
Volta installs a single
volta-shimtrampoline and symlinks every managed command to it, choosing which tool to run from the name it was invoked as. Upstreamget_tool_name()takesfile_name()ofargv[0](volta-core/src/run/mod.rs), andget_executor()refuses the shim under its own name (Some("volta-shim") => Err(ErrorKind::RunShimDirectly));volta-shim'smainthen exits 126 for any Volta error.resolveAgentExecutablePathcanonicalized bare command names throughfilepath.EvalSymlinks, collapsingclaude/codex/pionto that one shim path and discarding the tool selector. Result: version detection fails and none of them register.The pinned path also feeds
agent.Config.ExecutablePath, so this broke task launches too, not just registration — the fix belongs at path resolution rather than in the version probe.Core logic
When a symlink resolves to
volta-shim, ask Volta for the concrete binary (volta which <command>) and pin that.voltais invoked by absolute path from the same directory asvolta-shim(both areVoltaInstallentries) rather than via PATH, since the daemon may not have Volta's bin dir at all. Answers are cached process-wide and revalidated by existence, so a steady-state daemon does not fork a subprocess on every discovery tick and a replaced binary is re-resolved.Why pin the concrete path instead of keeping the alias. Keeping
~/.volta/bin/claudeand letting the shim dispatch looks more faithful to Volta, but it breaks the{path, version}invariant the rest of the daemon relies on. The shim resolves per working directory —volta whichitself prefers a project-local bin (src/command/which.rs) — so the version verified at registration would not necessarily be the version a task executes, and the minimum-version gate could be bypassed. Nor does the daemon re-probe often enough to paper over it: the refresh loop skipsconvergeRuntimeRegistrationsentirely when no provider is missing, andresolveAgentEntryreturns the cached version whenever the pinned path still exists — which an alias always does. Pinning the concrete path puts Volta installs on exactly the same footing as Homebrew or npm-global installs, and is what the reporter's own verifiedMULTICA_*_PATH="$(volta which claude)"workaround does.Fail-closed. If Volta cannot be asked, we keep the shim path: version detection fails and the provider stays unregistered rather than being launched through an ungated path.
MULTICA_*_PATHremains the manual override. There is a test for this.Scope.
isVoltaShimPathaccepts onlyvolta-shimandvolta-shim.exe— exact names, so neighbours likevolta-shim.bakorvolta-shim.wrappercannot opt out of symlink resolution. Nothing returns unresolved parent directories, so directory canonicalization and the PATH-drift guarantee stay intact. The exception lives in the sharedcanonicalExecutablePathhelper, which also covers the~/.multica/hooksunshadowing branch (it canonicalizes independently) and thereresolveAgentCommand/ MUL-4486 self-heal path.Testing
server/internal/daemon/agents_probe_volta_test.gouses a faithful fixture: anargv[0]-dispatching shim that exits 126 under its own name, plus avoltathat answerswhich. Fixture versions clearagent.MinVersions(claude ≥ 2.0.0, codex ≥ 0.100.0) so registration is actually reachable.TestDetectBuiltinRuntimes_RegistersVoltaManagedCLIs— the end-result test, through the real version probe and real min-version gate, asserting claude/codex/pi all reach the registration payload.TestResolveAgentExecutablePath_PinsVoltaConcreteBinary— pins the concrete binary; explicitly not the shim and not the alias.TestProbeAgentCLIs_DiscoversVoltaManagedCLIs— three distinct concrete paths at the discovery entry point.TestResolveAgentExecutablePath_FailsClosedWithoutVoltaResolution— no fallback to the alias.TestVoltaConcreteExecutable_ReresolvesAfterBinaryReplaced— cache revalidation after Volta swaps the binary.TestResolveAgentExecutablePath_VoltaAliasShadowedByHooks— hooks wrapper skipped and concrete binary still pinned.TestCanonicalExecutablePath_NonVoltaSymlinkStillCanonicalized/_SymlinkedParentDirIsCanonicalized— blast-radius guards for files and parent dirs.TestCanonicalExecutablePath_ExplicitShimPathStaysCanonical,TestIsVoltaShimPath— edge cases and near-miss names.Confirmed the regression tests genuinely fail without the fix (behavior branch neutralized), with the reporter's exact error — including at registration level:
Full runs, all green:
go build ./...,go test ./internal/daemon/... -count=1(daemon, execenv, repocache),go test ./pkg/agent/ -count=1,go vet ./internal/daemon/,gofmtclean on touched files,git diff --checkclean.Remaining risk
volta-shim --versionexits 126 with Volta's "should not be called directly" error; with the fix disableddetectBuiltinRuntimesregisters nothing and reports both providers asversion detection failed: detect version for .../volta-shim: exit status 126, matching the reporter's log; with the fix both resolve to exactly whatvolta whichreports and reach the registration payload. Captured as an opt-in test (TestRealVolta_*, gated onMULTICA_VERIFY_VOLTA_HOME, skipped in CI).volta whichis a subprocess on the discovery path. It is bounded by a 5s timeout withWaitDelay, only runs when a shim is actually detected, and is cached; failure is fail-closed.VOLTA_HOMEthat the daemon does not inherit,volta whichresolves against the default location; that is the same environment-inheritance class the login-shell PATH fallback already handles, and it fails closed.