CLIs installed via nvm/Homebrew are not found when Codeman runs as a service - #329
Merged
Merged
Conversation
… as a service A CLI installed by nvm, Homebrew or a user-level npm prefix lives on a PATH that only a login shell sets up. Codeman running under systemd or launchd does not get that PATH — launchd hands a job `/usr/bin:/bin:/usr/sbin:/sbin` — so every resolver reported the CLI as unavailable on installs where it is plainly there and works from a terminal. Each of the six resolvers had its own hand-rolled copy of the same PATH walk, so the fix is factored into one shared `createCliExecutableResolver()` with an explicit lookup order: the server process PATH, then common install directories in order, then an interactive login shell as the last resort. Only the last step spawns anything, and only when the cheap lookups have already missed. Also adds `formatCliNotFoundMessage()`, so a failure explains where it looked instead of just asserting the CLI is missing. Its diagnostics are bounded and control characters are flattened, so a not-found message cannot dump arbitrary environment data. Success is cached and failure is retried, so installing a CLI while the server is running is picked up without a restart. Net -103 lines across the six resolvers. Behaviour is unchanged wherever the CLI was already on the process PATH: that remains the first thing checked. Tests: 20 cases in test/cli-executable-resolver.test.ts covering the precedence order, login-shell-only resolution, the caching rule, unsafe-name rejection, and the bounded diagnostics.
Uh oh!
There was an error while loading. Please reload this page.
Ark0N
commented
Aug 21, 2026
Owner
Merged, thank you! The login-shell fallback is exactly what service installs on nvm/Homebrew setups needed. Follow-ups landing on master before release: negative-result caching so a missing CLI doesn't re-run the login shell on every request, SIGKILL on the exec timeout (interactive bash shrugs off SIGTERM), and the VITEST guards in the pi resolver restored so tests never execute a real pi binary from PATH. Ships in 1.20.0. |
Ark0N pushed a commit
that referenced
this pull request
Aug 21, 2026
…red VITEST hermeticity, wired not-found diagnostics Post-merge follow-ups for PR #329 (shared CLI executable resolution): - Negative-cache resolution misses with a doubling backoff (1min -> 5min cap, cliResolveRetryDelayMs, mirroring claudeVersionRetryDelayMs): the shared resolver cached success only, so a missing CLI re-ran the whole chain - ending in a synchronous interactive login-shell spawn bounded by the 5s EXEC_TIMEOUT_MS - on every /api/<cli>/status request and Run attempt, stalling the event loop each time, forever. Success still caches for the process lifetime, so an installed CLI is picked up within minutes without a restart. Tests drive the backoff via an injectable clock (createCliExecutableResolver `now` option, threaded through the createPiResolverForTest / createAntigravityResolverForTest wrappers). - Pass killSignal: 'SIGKILL' on the resolver's login-shell spawn and on the pi/claude --version probes: execFileSync's timeout only SENDS the kill signal and then keeps waiting for the child to exit, and interactive bash ignores SIGTERM, so a login shell stuck in a blocking .bash_profile survived the timeout and blocked the server permanently. - Restore test hermeticity (PR #329 deleted pi's VITEST guards, and one test pinned the deletion): under vitest the production resolver host now replaces un-injected IO primitives with inert stubs - no real PATH scanning, no login-shell spawns - and probePiVersion never executes a `pi` candidate again (`pi` is a generic binary name, so route tests hitting /api/pi/status executed whatever binary the machine carried). Tests opt in through the runCommand/isExecutableFile injection hooks or allowRealIoUnderVitest for real-filesystem fixtures. The deletion-pinning test is replaced by behavioral pins, including a real-executable fixture in the new test/pi-cli-resolver.test.ts that fails loudly if the pi gate is ever removed again. - Wire the six get*NotFoundMessage() exports (previously dead) into their intended call sites: the createSession throws in tmux-manager and the availability gates on POST /api/sessions and POST /api/quick-start in session-routes, replacing a third hardcoded copy of the text. A not-found error now names where resolution looked (server PATH, login shell, checked directories). npm run knip no longer reports any unused export from the resolver modules. Co-Authored-By: Claude Fable 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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
The bug
A CLI installed by nvm, Homebrew, or a user-level npm prefix lives on a PATH that only a login shell sets up.
Codeman running under systemd or launchd never gets that PATH — launchd hands a job
/usr/bin:/bin:/usr/sbin:/sbin. So every resolver reports the CLI as unavailable on installs where it is plainly present and works fine from a terminal.This is the same PATH problem
service installalready works around by baking the installing shell's PATH into the unit; the resolvers had no equivalent.The fix
Each of the six resolvers carried its own hand-rolled copy of the same PATH walk, so rather than patch the same bug six times this factors them onto one shared
createCliExecutableResolver()with an explicit lookup order:Only step 3 spawns anything, and only once the cheap lookups have missed. Behaviour is unchanged wherever the CLI was already on the process PATH, since that stays the first thing checked.
Success is cached and failure is retried, so installing a CLI while the server is running is picked up without a restart.
Also included
formatCliNotFoundMessage()— a failure now explains where it looked instead of just asserting the CLI is missing. The diagnostics are bounded in length and control characters are flattened, so a not-found message cannot dump arbitrary environment data into a response.Size
Net −103 lines across the six resolvers, plus the shared module.
shell-resolver.tsis used as-is and unchanged.Tests
test/cli-executable-resolver.test.ts, 20 cases: the precedence order, login-shell-only resolution, the cache-success/retry-failure rule, unsafe-name rejection, and the bounded diagnostics.Verified non-vacuous by disabling the login-shell probe — 2 tests fail, including the one that pins resolution through the injected login-shell runner.
Full gate green:
tsc --noEmit, eslint, prettier, andvitest run --config config/vitest.ci.config.ts(5394 passed / 12 skipped).