fix(setup): make codex install opt-in on the auto target - #156
Merged
Conversation
Setup crashed on fresh installs where a codex proxy shim lives on PATH (e.g. cmux CLI shims at $TMPDIR/cmux-cli-shims/.../codex). Bun.which resolved the shim, so codexCliAvailable() returned true; then readCodexPluginState invoked `codex plugin list --json`, the shim exited non-zero with "Error: codex not found in PATH", and the raw error surfaced as a stack trace instead of falling through to the friendly "Codex CLI is required for a full Codex install" message. - codexCliAvailable now probes `codex --version` (3s timeout) after the hasCommand check, memoized per process to avoid re-probing across the ~15 call sites. Non-zero exit or spawn failure -> unavailable. - resetCodexCliAvailabilityMemoForTests exported so tests that mutate PATH can force a re-probe. - Two new tests: POSIX shim that exits non-zero (mimics cmux) and empty PATH fast path. Windows-gated via describe.skipIf since the shim uses #!/bin/sh. bash setup.sh on the reproducing machine now prints the friendly "Codex CLI is required for a full Codex install" and exits 1 cleanly, with no stack trace.
The `auto` target used to install cc-settings for Codex whenever a `codex`
binary was on PATH. Two problems:
1. Not everyone uses codex. Surprise-installing it during `./setup.sh` is
the wrong default when the user only asked for Claude support.
2. On machines with a cmux CLI shim under $TMPDIR/cmux-cli-shims/*/codex,
the existing check tripped positive and the install then crashed
downstream trying to actually run the fake `codex`.
Change: `resolveInstallTarget("auto")` now prompts (Y/n) when codex is
detected and (y/N) when it is not. Non-interactive callers (CI, piped
input) fall through to the default silently. Explicit `--target=...`
still bypasses the prompt.
Also tighten `codexCliAvailable()` so a probe that exits 0 with a bogus
stdout (real cmux behavior: prints "Error: codex not found in PATH" and
exits 0) is treated as unavailable. Exit code alone is not enough; the
output has to look like an actual codex version banner. The banner-shape
check is factored out as an exported pure function so it can be
unit-tested — the surrounding spawn+Bun.which path reads PATH from a
boot-time snapshot the tests cannot override.
Errors that go through `restoreCombinedAfterClaudeFailure` now unwrap
`AggregateError.errors` in the main catch, so the real cause surfaces
instead of the "compensation was incomplete" wrapper.
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.
What this does
Codex is optional.
./setup.shused to silently install cc-settings for the Codex CLI whenever anything namedcodexwas on PATH — surprising users who only wanted Claude, and hard-crashing on machines with a cmux proxy shim that fakes the binary.Now the auto-detect path asks. If Codex is detected, the prompt defaults to yes. If not, it defaults to no. Non-interactive callers (CI, piped input) get the default silently. Explicit
--target=claude|codex|bothbypasses the prompt.Follow-up to #155 — that PR fixed the "codex looks installed but fails on invoke" case for the plugin marketplace probe. This one extends the same reasoning to the
--target=autodecision and adds the prompt users actually want.Summary
resolveInstallTarget("auto")now prompts and is async; only caller updated withawait.codexCliAvailable()probe now requires the--versionstdout to look like a real codex banner. The cmux shim printsError: codex not found in PATHand exits 0 — exit code alone tripped positive. New pure helperlooksLikeCodexVersionis exported so it can be unit-tested (the surrounding spawn+Bun.which path reads PATH from a boot-time snapshot the tests cannot override).main()error catch now unwrapsAggregateError.errorssorestoreCombinedAfterClaudeFailureno longer swallows the real cause.Test Plan
bun run typecheckbun test tests/codex-install.test.ts -t "shim detection"(4/4 pass, including the two new cases)./setup.shon a machine with the cmux shim: probe now reports codex unavailable; non-interactive install picksclaude, no combined-install pathywhen codex is genuinely installed,nwhen not