From 3f5a3257aac3cff9cc76de61d527c3b894b354e3 Mon Sep 17 00:00:00 2001 From: vanvasten Date: Tue, 25 Aug 2026 21:07:12 -0300 Subject: [PATCH 1/2] fix(codex): detect proxy shims that pass which() but fail on invoke 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. --- src/lib/codex-install.ts | 36 ++++++++++++++++++++++++- tests/codex-install.test.ts | 52 +++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/src/lib/codex-install.ts b/src/lib/codex-install.ts index 867b98b..2ae09a8 100644 --- a/src/lib/codex-install.ts +++ b/src/lib/codex-install.ts @@ -697,8 +697,42 @@ function testCodexCommand(): string[] | null { return parsed as string[]; } +// Memoized probe result. `null` = not yet probed; `boolean` = final answer for +// this process. Cleared via `resetCodexCliAvailabilityMemoForTests()` between +// test scenarios that install/uninstall codex under a synthetic PATH. +let codexCliAvailabilityMemo: boolean | null = null; + +/** Test-only: reset the memoized probe so PATH changes take effect. */ +export function resetCodexCliAvailabilityMemoForTests(): void { + codexCliAvailabilityMemo = null; +} + export function codexCliAvailable(): boolean { - return testCodexCommand() !== null || hasCommand("codex"); + // Test escape hatch: an explicit fixture command bypasses PATH resolution + // entirely and is always considered available. + if (testCodexCommand() !== null) return true; + if (codexCliAvailabilityMemo !== null) return codexCliAvailabilityMemo; + if (!hasCommand("codex")) { + codexCliAvailabilityMemo = false; + return false; + } + // A binary named `codex` is on PATH, but proxy shims (e.g. cmux CLI shims + // at $TMPDIR/cmux-cli-shims/.../codex) can pass the existence check while + // failing on invocation. Probe with `codex --version` before trusting the + // CLI for real plugin/marketplace queries. A 3s cap keeps a hung shim + // from stalling setup. + try { + const probe = Bun.spawnSync({ + cmd: ["codex", "--version"], + stdout: "ignore", + stderr: "ignore", + timeout: 3000, + }); + codexCliAvailabilityMemo = probe.exitCode === 0; + } catch { + codexCliAvailabilityMemo = false; + } + return codexCliAvailabilityMemo; } const REQUIRED_SOURCE_ARTIFACTS = [ diff --git a/tests/codex-install.test.ts b/tests/codex-install.test.ts index 1908ab2..89fff39 100644 --- a/tests/codex-install.test.ts +++ b/tests/codex-install.test.ts @@ -3830,3 +3830,55 @@ exit 0 240_000, ); }); + +describe.skipIf(process.platform === "win32")("codexCliAvailable — shim detection", () => { + test("PATH entry named 'codex' that exits non-zero is treated as unavailable", async () => { + // Simulates the cmux CLI shim case: a binary named `codex` is on PATH, + // but invocation fails. Bun.which passes; the version probe must not. + const { codexCliAvailable, resetCodexCliAvailabilityMemoForTests } = await import( + "../src/lib/codex-install.ts" + ); + const shimDir = await mkdtemp(join(tmpdir(), "cc-codex-shim-")); + try { + const shim = join(shimDir, "codex"); + // POSIX shim that mimics the cmux behavior: exits non-zero with a + // "codex not found" stderr message on every invocation. + await writeFile(shim, "#!/bin/sh\necho 'Error: codex not found in PATH' >&2\nexit 1\n"); + await chmod(shim, 0o755); + + const originalPath = process.env.PATH; + process.env.PATH = prependTestPath(shimDir); + resetCodexCliAvailabilityMemoForTests(); + try { + expect(codexCliAvailable()).toBe(false); + } finally { + if (originalPath === undefined) delete process.env.PATH; + else process.env.PATH = originalPath; + resetCodexCliAvailabilityMemoForTests(); + } + } finally { + await rm(shimDir, { recursive: true, force: true }); + } + }); + + test("no codex on PATH → unavailable (fast path, no spawn)", async () => { + const { codexCliAvailable, resetCodexCliAvailabilityMemoForTests } = await import( + "../src/lib/codex-install.ts" + ); + const emptyDir = await mkdtemp(join(tmpdir(), "cc-codex-empty-")); + try { + const originalPath = process.env.PATH; + process.env.PATH = emptyDir; + resetCodexCliAvailabilityMemoForTests(); + try { + expect(codexCliAvailable()).toBe(false); + } finally { + if (originalPath === undefined) delete process.env.PATH; + else process.env.PATH = originalPath; + resetCodexCliAvailabilityMemoForTests(); + } + } finally { + await rm(emptyDir, { recursive: true, force: true }); + } + }); +}); From 8b9ea2603d681918694e09896dd291919a334364 Mon Sep 17 00:00:00 2001 From: vanvasten Date: Wed, 26 Aug 2026 18:11:19 -0300 Subject: [PATCH 2/2] fix(setup): make codex install opt-in on the auto target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/lib/codex-install.ts | 26 ++++++++++++++++----- src/setup.ts | 38 ++++++++++++++++++++++++++++--- tests/codex-install.test.ts | 45 +++++++++++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 9 deletions(-) diff --git a/src/lib/codex-install.ts b/src/lib/codex-install.ts index 2ae09a8..416f628 100644 --- a/src/lib/codex-install.ts +++ b/src/lib/codex-install.ts @@ -718,23 +718,37 @@ export function codexCliAvailable(): boolean { } // A binary named `codex` is on PATH, but proxy shims (e.g. cmux CLI shims // at $TMPDIR/cmux-cli-shims/.../codex) can pass the existence check while - // failing on invocation. Probe with `codex --version` before trusting the - // CLI for real plugin/marketplace queries. A 3s cap keeps a hung shim - // from stalling setup. + // failing on invocation. Probe with `codex --version` and require the + // output to actually look like a codex version banner, because at least + // one shim (cmux) prints "Error: codex not found in PATH" and still exits + // 0 — exit code alone is not enough. A 3s cap keeps a hung shim from + // stalling setup. try { const probe = Bun.spawnSync({ cmd: ["codex", "--version"], - stdout: "ignore", - stderr: "ignore", + stdout: "pipe", + stderr: "pipe", timeout: 3000, }); - codexCliAvailabilityMemo = probe.exitCode === 0; + const stdout = probe.stdout ? new TextDecoder().decode(probe.stdout) : ""; + codexCliAvailabilityMemo = probe.exitCode === 0 && looksLikeCodexVersion(stdout); } catch { codexCliAvailabilityMemo = false; } return codexCliAvailabilityMemo; } +/** Real `codex --version` prints a line like `codex-cli 0.15.2` or + * `codex 0.15.2` — a leading `codex` token followed by a semver-ish + * number. Shims that swallow the invocation with an error message do + * not match. Kept as a fragment match so a future banner prefix (e.g. + * `codex 0.16.0 (release build)`) still passes. Exported for direct + * unit tests because the surrounding probe (spawn + Bun.which) reads + * PATH via a boot-time snapshot that tests cannot override. */ +export function looksLikeCodexVersion(output: string): boolean { + return /\bcodex[\w-]*\s+\d+\.\d+/i.test(output); +} + const REQUIRED_SOURCE_ARTIFACTS = [ "AGENTS.md", "codex/AGENTS.append.md", diff --git a/src/setup.ts b/src/setup.ts index b658cd4..575794d 100644 --- a/src/setup.ts +++ b/src/setup.ts @@ -1334,9 +1334,30 @@ async function cmdStatus(sourceDir: string): Promise { return 0; // status is informational; never fail } -function resolveInstallTarget(target: InstallTarget): Exclude { +/** + * Auto-detect used to silently pick `both` whenever a `codex` binary was on + * PATH — surprise-installing cc-settings for Codex when the user only wanted + * Claude, and hard-failing when the binary was a proxy shim (cmux drops one + * under $TMPDIR/cmux-cli-shims that exits 0 while printing "codex not found + * in PATH"). The Codex install is now opt-in on the `auto` path: + * ask, with the probe result as the default. Non-interactive callers (CI, + * piped input) fall through to the default silently — never Codex without + * explicit consent. + * + * Explicit `--target=claude|codex|both` bypasses the prompt entirely. + */ +async function resolveInstallTarget( + target: InstallTarget, +): Promise> { if (target !== "auto") return target; - return hasCommand("codex") ? "both" : "claude"; + const codexLooksInstalled = codexCliAvailable(); + const wantsCodex = await promptYn( + codexLooksInstalled + ? "Codex detected. Install cc-settings for Codex too?" + : "Also install cc-settings for the Codex CLI? (only if you use Codex)", + codexLooksInstalled, + ); + return wantsCodex ? "both" : "claude"; } function includesTarget( @@ -1928,7 +1949,7 @@ async function main(): Promise { for (const message of args.errors) error(message); return 1; } - let target = resolveInstallTarget(args.target); + let target = await resolveInstallTarget(args.target); if (includesTarget(target, "codex")) { await validateProductRootDisjointness(CLAUDE_DIR); } @@ -2314,6 +2335,17 @@ if (import.meta.main) { .join("\n") : String(err); error(`Setup failed: ${detail}`); + // AggregateError.errors carries the real causes — without unwrapping, + // the outer wrapper message swallows them and leaves the operator with + // no way to see what actually failed (e.g. restoreCombinedAfterClaudeFailure + // wraps the underlying Claude install error plus any restore failures). + if (err instanceof AggregateError && Array.isArray(err.errors)) { + for (const [i, cause] of err.errors.entries()) { + const causeDetail = + cause instanceof Error ? (cause.stack ?? cause.message) : String(cause); + error(` cause[${i}]: ${causeDetail}`); + } + } process.exit(1); }); } diff --git a/tests/codex-install.test.ts b/tests/codex-install.test.ts index 89fff39..1ae73f9 100644 --- a/tests/codex-install.test.ts +++ b/tests/codex-install.test.ts @@ -3861,6 +3861,51 @@ describe.skipIf(process.platform === "win32")("codexCliAvailable — shim detect } }); + test("shim that exits 0 with a bogus stdout is treated as unavailable", async () => { + // Real cmux shim behavior observed on macOS: `codex --version` prints + // "Error: codex not found in PATH" to STDOUT and exits 0. Exit code alone + // said "installed", the version-shape check now catches it. + const { codexCliAvailable, resetCodexCliAvailabilityMemoForTests } = await import( + "../src/lib/codex-install.ts" + ); + const shimDir = await mkdtemp(join(tmpdir(), "cc-codex-shim-exit0-")); + try { + const shim = join(shimDir, "codex"); + await writeFile(shim, "#!/bin/sh\necho 'Error: codex not found in PATH'\nexit 0\n"); + await chmod(shim, 0o755); + + const originalPath = process.env.PATH; + process.env.PATH = prependTestPath(shimDir); + resetCodexCliAvailabilityMemoForTests(); + try { + expect(codexCliAvailable()).toBe(false); + } finally { + if (originalPath === undefined) delete process.env.PATH; + else process.env.PATH = originalPath; + resetCodexCliAvailabilityMemoForTests(); + } + } finally { + await rm(shimDir, { recursive: true, force: true }); + } + }); + + // Direct pure-function coverage for the version-shape check, because the + // enclosing spawn+Bun.which path reads PATH from a boot-time snapshot that + // tests cannot repoint at a fixture stub. Without this, no test would + // catch a future over-tightening of the regex that rejects real versions. + test("looksLikeCodexVersion accepts real banners, rejects shim errors", async () => { + const { looksLikeCodexVersion } = await import("../src/lib/codex-install.ts"); + // Real banner shapes seen in the wild. + expect(looksLikeCodexVersion("codex-cli 0.15.2\n")).toBe(true); + expect(looksLikeCodexVersion("codex 0.16.0")).toBe(true); + expect(looksLikeCodexVersion("codex 0.16.0 (release build)\n")).toBe(true); + // The cmux shim behavior: prints an error to stdout and exits 0. + expect(looksLikeCodexVersion("Error: codex not found in PATH\n")).toBe(false); + // Empty / unrelated output. + expect(looksLikeCodexVersion("")).toBe(false); + expect(looksLikeCodexVersion("bash: codex: command not found\n")).toBe(false); + }); + test("no codex on PATH → unavailable (fast path, no spawn)", async () => { const { codexCliAvailable, resetCodexCliAvailabilityMemoForTests } = await import( "../src/lib/codex-install.ts"