From 4dc3516bcc8cbcbb9216ac20da77ceb9bb284ba2 Mon Sep 17 00:00:00 2001 From: Udhdhav kheni Date: Mon, 31 Aug 2026 15:33:26 +1000 Subject: [PATCH] fix(codex): a CLI that cannot execute no longer reports CODEX_MODE: ready MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #2477. The model probe it added does a real round trip, but its final branch is the `else` of a "model 400" grep, so it swallowed spawn ENOENT, non-executable binaries and missing vendor payloads alongside genuine network timeouts. All three are deterministic — retrying never helps — yet they landed in the fail-open bucket and resolved to `ready`, so every Codex pass was skipped in silence and the review reported itself complete. Observed live: @openai/codex was on PATH with an empty vendor/aarch64-apple-darwin/codex/ directory. gstack said `ready` for two months while no Codex pass ran. Three changes: - `_gstack_codex_model_probe` classifies deterministic install failures (exit 126/127, or stderr matching ENOENT/ENOEXEC/EACCES/"cannot execute binary file") as MODEL_UNUSABLE_INSTALL, exit 2, never cached — a reinstall is picked up on the next probe. Exit 124 and genuine transients still fail open, which is what #2477 intended. - The preflight chain captures the probe's code instead of testing it for truthiness, so exit 2 routes to a new `broken_install` mode whose remedy is `npm install -g @openai/codex` rather than "check your model pin". A missing binary and an unusable model are different problems with different fixes. - `_gstack_codex_version_check` no longer reads a broken CLI as healthy. It ran `codex --version 2>/dev/null | head -1`, which captures head's status, not codex's — and 2>/dev/null discarded the one diagnostic available. It now captures the real exit code and warns on non-zero. Empty-but-successful output stays silent, per the existing "empty output → OK" case. Tests: 6 added to test/codex-hardening.test.ts covering both broken-install shapes, the exit-2 contract, no caching, the transient still failing open, the model 400 still classifying as MODEL_UNUSABLE, and the version-check warning. 845 pass / 0 fail across all 8 suites touching the changed files. Closes #2742 Wave-amended: autoplan hand-maintained preflight chain completed (tmpl+render); install-signature grep gated on failed spawn only; goldens regenerated against the wave tree (author's golden commit 5797d326 superseded); +2 tests --- autoplan/SKILL.md | 19 +- autoplan/SKILL.md.tmpl | 19 +- bin/gstack-codex-probe | 37 +++- document-release/sections/release-body.md | 14 +- plan-ceo-review/sections/review-sections.md | 14 +- plan-devex-review/sections/review-sections.md | 14 +- plan-eng-review/sections/review-sections.md | 14 +- review/sections/adversarial.md | 14 +- scripts/resolvers/constants.ts | 14 +- ship/sections/adversarial.md | 14 +- test/codex-hardening.test.ts | 165 ++++++++++++++++++ test/fixtures/golden/factory-ship-SKILL.md | 14 +- 12 files changed, 316 insertions(+), 36 deletions(-) diff --git a/autoplan/SKILL.md b/autoplan/SKILL.md index d423cc174..bd13c8d41 100644 --- a/autoplan/SKILL.md +++ b/autoplan/SKILL.md @@ -793,12 +793,21 @@ elif ! _gstack_codex_auth_probe >/dev/null; then # Round-trip model probe (#2477): auth can pass while the account's configured # model is rejected with an HTTP 400 (stale `model =` pin in ~/.codex/config.toml). # ~10s on first run, cached 1h; timeouts fail open (probe returns 0). -elif ! _gstack_codex_model_probe; then - echo "[codex-unavailable: configured model rejected] — proceeding with Claude subagent only. Fix the \`model =\` pin in ~/.codex/config.toml (see [notice.model_migrations] there for the replacement)." - _CODEX_AVAILABLE=false +# Exit 2 = broken install (#2742: spawn ENOENT / non-executable binary / +# missing vendor payload) — a different problem with a different fix, so +# capture the code instead of testing truthiness. else - _gstack_codex_version_check # non-blocking warn if known-bad - _CODEX_AVAILABLE=true + _gstack_codex_model_probe; _CODEX_MP=$? + if [ "$_CODEX_MP" -eq 2 ]; then + echo "[codex-unavailable: binary cannot run] — proceeding with Claude subagent only. Reinstall: \`npm install -g @openai/codex\` (#2742)." + _CODEX_AVAILABLE=false + elif [ "$_CODEX_MP" -ne 0 ]; then + echo "[codex-unavailable: configured model rejected] — proceeding with Claude subagent only. Fix the \`model =\` pin in ~/.codex/config.toml (see [notice.model_migrations] there for the replacement)." + _CODEX_AVAILABLE=false + else + _gstack_codex_version_check # non-blocking warn if known-bad + _CODEX_AVAILABLE=true + fi fi ``` diff --git a/autoplan/SKILL.md.tmpl b/autoplan/SKILL.md.tmpl index 13b5d9e61..6a034e15e 100644 --- a/autoplan/SKILL.md.tmpl +++ b/autoplan/SKILL.md.tmpl @@ -278,12 +278,21 @@ elif ! _gstack_codex_auth_probe >/dev/null; then # Round-trip model probe (#2477): auth can pass while the account's configured # model is rejected with an HTTP 400 (stale `model =` pin in ~/.codex/config.toml). # ~10s on first run, cached 1h; timeouts fail open (probe returns 0). -elif ! _gstack_codex_model_probe; then - echo "[codex-unavailable: configured model rejected] — proceeding with Claude subagent only. Fix the \`model =\` pin in ~/.codex/config.toml (see [notice.model_migrations] there for the replacement)." - _CODEX_AVAILABLE=false +# Exit 2 = broken install (#2742: spawn ENOENT / non-executable binary / +# missing vendor payload) — a different problem with a different fix, so +# capture the code instead of testing truthiness. else - _gstack_codex_version_check # non-blocking warn if known-bad - _CODEX_AVAILABLE=true + _gstack_codex_model_probe; _CODEX_MP=$? + if [ "$_CODEX_MP" -eq 2 ]; then + echo "[codex-unavailable: binary cannot run] — proceeding with Claude subagent only. Reinstall: \`npm install -g @openai/codex\` (#2742)." + _CODEX_AVAILABLE=false + elif [ "$_CODEX_MP" -ne 0 ]; then + echo "[codex-unavailable: configured model rejected] — proceeding with Claude subagent only. Fix the \`model =\` pin in ~/.codex/config.toml (see [notice.model_migrations] there for the replacement)." + _CODEX_AVAILABLE=false + else + _gstack_codex_version_check # non-blocking warn if known-bad + _CODEX_AVAILABLE=true + fi fi ``` diff --git a/bin/gstack-codex-probe b/bin/gstack-codex-probe index 2d151ef60..89700aae6 100755 --- a/bin/gstack-codex-probe +++ b/bin/gstack-codex-probe @@ -53,6 +53,10 @@ _gstack_codex_model_probe() { # section, forever. Editing config.toml (the fix) changes the cache # signature and re-probes immediately; the short TTL covers server-side # entitlement recovery the signature can't see. + # MODEL_UNUSABLE_INSTALL (exit 2) — the CLI cannot execute at all (spawn + # ENOENT, non-executable binary, missing vendor payload). Deterministic, + # so fail-open is wrong: retrying never helps. Never cached — a reinstall + # fixes it and must be picked up on the very next probe (#2742). # MODEL_PROBE_INCONCLUSIVE (exit 0) — timeout/transient; FAIL-OPEN so a # slow network never wedges codex mode (the per-invocation Error # Handling entry still covers a later 400). Never cached. @@ -114,6 +118,20 @@ _gstack_codex_model_probe() { _gstack_codex_log_event "codex_model_unusable" 2>/dev/null || true return 1 fi + # A CLI that cannot execute is deterministic, not transient: the fail-open + # below exists for network luck, and swallowing this here is what let a + # missing vendor binary report CODEX_MODE: ready while every Codex pass was + # silently skipped (#2742). 126 = found but not executable, 127 = not found. + # String signatures only count on a FAILED spawn: a successful response whose + # text merely mentions "permission denied" must not classify as broken. + if [ "$_code" -eq 126 ] || [ "$_code" -eq 127 ] || { [ "$_code" -ne 0 ] && printf '%s' "$_out" | grep -qiE 'ENOENT|ENOEXEC|EACCES|no such file or directory|cannot execute binary file|not executable|permission denied'; }; then + echo "MODEL_UNUSABLE_INSTALL" + printf '%s\n' "$_out" | grep -iE 'ENOENT|ENOEXEC|EACCES|no such file or directory|cannot execute|permission denied' | head -3 + echo "HINT: the Codex CLI is on PATH but cannot run — its binary or vendor payload is missing." + echo "HINT: reinstall with: npm install -g @openai/codex" + _gstack_codex_log_event "codex_broken_install" 2>/dev/null || true + return 2 + fi # Timeout (124) or transient failure: fail-open with a warning. The probe # exists to catch the deterministic model 400, not to gate on network luck. echo "MODEL_PROBE_INCONCLUSIVE (exit $_code) — proceeding; if invocations fail with a model 400, see the codex skill's Error Handling entry." @@ -127,8 +145,23 @@ _gstack_codex_version_check() { # positives like 0.120.10 or 0.120.20 from matching. 0.120.2-beta still # matches the bad release and gets warned (it IS buggy). # Update this list when a new Codex CLI version regresses. - local _ver - _ver=$(codex --version 2>/dev/null | head -1) + local _ver _vcode + # Capture the code from codex, not from `head` — a pipeline reports the LAST + # command's status, which is why a CLI that only ever printed a spawn error + # still read as healthy here (#2742). Keep stderr: it carries the diagnosis. + _ver=$(codex --version 2>&1) + _vcode=$? + _ver=$(printf '%s' "$_ver" | head -1) + # Only a NON-ZERO exit is evidence of a broken CLI. Empty-but-successful + # output stays silent by design (a CLI may legitimately print nothing), which + # the "empty output → OK" case in this file's suite pins. + if [ "$_vcode" -ne 0 ]; then + echo "WARN: \`codex --version\` failed (exit $_vcode) — the CLI is on PATH but may not be runnable." + [ -n "$_ver" ] && echo "WARN: it said: $_ver" + echo "WARN: if Codex passes are being skipped, reinstall with: npm install -g @openai/codex" + _gstack_codex_log_event "codex_version_unreadable" 2>/dev/null || true + return 0 + fi [ -z "$_ver" ] && return 0 if echo "$_ver" | grep -Eq '(^|[^0-9.])0\.120\.(0|1|2)([^0-9.]|$)'; then echo "WARN: Codex CLI $_ver has known stdin deadlock bugs. Run: npm install -g @openai/codex@latest" diff --git a/document-release/sections/release-body.md b/document-release/sections/release-body.md index 1c645c9bc..a211879a6 100644 --- a/document-release/sections/release-body.md +++ b/document-release/sections/release-body.md @@ -447,10 +447,17 @@ elif ! command -v codex >/dev/null 2>&1; then _CODEX_MODE="not_installed"; _gstack_codex_log_event "codex_cli_missing" 2>/dev/null || true elif ! _gstack_codex_auth_probe >/dev/null 2>&1; then _CODEX_MODE="not_authed"; _gstack_codex_log_event "codex_auth_failed" 2>/dev/null || true -elif ! _gstack_codex_model_probe; then - _CODEX_MODE="model_unusable" else - _CODEX_MODE="ready"; _gstack_codex_version_check 2>/dev/null || true + # Capture the probe's code: 2 means the CLI cannot execute at all, which is a + # different problem (and a different fix) from a model the account can't use. + _gstack_codex_model_probe; _CODEX_MP=$? + if [ "$_CODEX_MP" -eq 2 ]; then + _CODEX_MODE="broken_install" + elif [ "$_CODEX_MP" -ne 0 ]; then + _CODEX_MODE="model_unusable" + else + _CODEX_MODE="ready"; _gstack_codex_version_check 2>/dev/null || true + fi fi echo "CODEX_MODE: $_CODEX_MODE" ``` @@ -460,6 +467,7 @@ Branch on the echoed `CODEX_MODE`: - **`not_installed`** — Codex CLI absent. Print: "Codex not installed — using Claude subagent. Install for cross-model coverage: `npm install -g @openai/codex`." Fall back to the Claude subagent path. - **`under_codex`** — this session is already running INSIDE a Codex host, so spawning codex again is the same model reviewing itself at multiplied token cost (#2519). Print exactly one line: "[running under Codex — nested codex passes skipped; set GSTACK_FORCE_CODEX_REVIEW=1 to force]" and skip the codex invocations below; run the section's free in-host pass instead if it defines one. - **`not_authed`** — installed but no credentials. Print: "Codex installed but not authenticated — using Claude subagent. Run `codex login` or set `$CODEX_API_KEY`." Fall back to the Claude subagent path. +- **`broken_install`** — the CLI is on PATH but cannot execute (spawn ENOENT, non-executable binary, missing vendor payload). Print: "Codex is installed but its binary cannot run — Codex passes skipped. Reinstall: `npm install -g @openai/codex`." Relay the probe's HINT lines and fall back to the Claude subagent path. This state exists because a missing binary used to land in the model probe's fail-open bucket and report `ready`, so every Codex pass was skipped silently (#2742). - **`model_unusable`** — authed but the account cannot use its configured model (#2477: HTTP 400 on every call, usually a stale `model =` pin in `~/.codex/config.toml`). Relay the probe's HINT lines, tell the user the one-line fix (update the pin; `[notice.model_migrations]` names the replacement), and fall back to the Claude subagent path. The ~10s round trip is cached for 1h; timeouts fail open to `ready`. - **`ready`** — run the Codex pass below. diff --git a/plan-ceo-review/sections/review-sections.md b/plan-ceo-review/sections/review-sections.md index 3f4cb7a72..d24648b8b 100644 --- a/plan-ceo-review/sections/review-sections.md +++ b/plan-ceo-review/sections/review-sections.md @@ -282,10 +282,17 @@ elif ! command -v codex >/dev/null 2>&1; then _CODEX_MODE="not_installed"; _gstack_codex_log_event "codex_cli_missing" 2>/dev/null || true elif ! _gstack_codex_auth_probe >/dev/null 2>&1; then _CODEX_MODE="not_authed"; _gstack_codex_log_event "codex_auth_failed" 2>/dev/null || true -elif ! _gstack_codex_model_probe; then - _CODEX_MODE="model_unusable" else - _CODEX_MODE="ready"; _gstack_codex_version_check 2>/dev/null || true + # Capture the probe's code: 2 means the CLI cannot execute at all, which is a + # different problem (and a different fix) from a model the account can't use. + _gstack_codex_model_probe; _CODEX_MP=$? + if [ "$_CODEX_MP" -eq 2 ]; then + _CODEX_MODE="broken_install" + elif [ "$_CODEX_MP" -ne 0 ]; then + _CODEX_MODE="model_unusable" + else + _CODEX_MODE="ready"; _gstack_codex_version_check 2>/dev/null || true + fi fi echo "CODEX_MODE: $_CODEX_MODE" ``` @@ -295,6 +302,7 @@ Branch on the echoed `CODEX_MODE`: - **`not_installed`** — Codex CLI absent. Print: "Codex not installed — using Claude subagent. Install for cross-model coverage: `npm install -g @openai/codex`." Fall back to the Claude subagent path. - **`under_codex`** — this session is already running INSIDE a Codex host, so spawning codex again is the same model reviewing itself at multiplied token cost (#2519). Print exactly one line: "[running under Codex — nested codex passes skipped; set GSTACK_FORCE_CODEX_REVIEW=1 to force]" and skip the codex invocations below; run the section's free in-host pass instead if it defines one. - **`not_authed`** — installed but no credentials. Print: "Codex installed but not authenticated — using Claude subagent. Run `codex login` or set `$CODEX_API_KEY`." Fall back to the Claude subagent path. +- **`broken_install`** — the CLI is on PATH but cannot execute (spawn ENOENT, non-executable binary, missing vendor payload). Print: "Codex is installed but its binary cannot run — Codex passes skipped. Reinstall: `npm install -g @openai/codex`." Relay the probe's HINT lines and fall back to the Claude subagent path. This state exists because a missing binary used to land in the model probe's fail-open bucket and report `ready`, so every Codex pass was skipped silently (#2742). - **`model_unusable`** — authed but the account cannot use its configured model (#2477: HTTP 400 on every call, usually a stale `model =` pin in `~/.codex/config.toml`). Relay the probe's HINT lines, tell the user the one-line fix (update the pin; `[notice.model_migrations]` names the replacement), and fall back to the Claude subagent path. The ~10s round trip is cached for 1h; timeouts fail open to `ready`. - **`ready`** — run the Codex pass below. diff --git a/plan-devex-review/sections/review-sections.md b/plan-devex-review/sections/review-sections.md index 2a5cfa517..bd1cd0b5b 100644 --- a/plan-devex-review/sections/review-sections.md +++ b/plan-devex-review/sections/review-sections.md @@ -268,10 +268,17 @@ elif ! command -v codex >/dev/null 2>&1; then _CODEX_MODE="not_installed"; _gstack_codex_log_event "codex_cli_missing" 2>/dev/null || true elif ! _gstack_codex_auth_probe >/dev/null 2>&1; then _CODEX_MODE="not_authed"; _gstack_codex_log_event "codex_auth_failed" 2>/dev/null || true -elif ! _gstack_codex_model_probe; then - _CODEX_MODE="model_unusable" else - _CODEX_MODE="ready"; _gstack_codex_version_check 2>/dev/null || true + # Capture the probe's code: 2 means the CLI cannot execute at all, which is a + # different problem (and a different fix) from a model the account can't use. + _gstack_codex_model_probe; _CODEX_MP=$? + if [ "$_CODEX_MP" -eq 2 ]; then + _CODEX_MODE="broken_install" + elif [ "$_CODEX_MP" -ne 0 ]; then + _CODEX_MODE="model_unusable" + else + _CODEX_MODE="ready"; _gstack_codex_version_check 2>/dev/null || true + fi fi echo "CODEX_MODE: $_CODEX_MODE" ``` @@ -281,6 +288,7 @@ Branch on the echoed `CODEX_MODE`: - **`not_installed`** — Codex CLI absent. Print: "Codex not installed — using Claude subagent. Install for cross-model coverage: `npm install -g @openai/codex`." Fall back to the Claude subagent path. - **`under_codex`** — this session is already running INSIDE a Codex host, so spawning codex again is the same model reviewing itself at multiplied token cost (#2519). Print exactly one line: "[running under Codex — nested codex passes skipped; set GSTACK_FORCE_CODEX_REVIEW=1 to force]" and skip the codex invocations below; run the section's free in-host pass instead if it defines one. - **`not_authed`** — installed but no credentials. Print: "Codex installed but not authenticated — using Claude subagent. Run `codex login` or set `$CODEX_API_KEY`." Fall back to the Claude subagent path. +- **`broken_install`** — the CLI is on PATH but cannot execute (spawn ENOENT, non-executable binary, missing vendor payload). Print: "Codex is installed but its binary cannot run — Codex passes skipped. Reinstall: `npm install -g @openai/codex`." Relay the probe's HINT lines and fall back to the Claude subagent path. This state exists because a missing binary used to land in the model probe's fail-open bucket and report `ready`, so every Codex pass was skipped silently (#2742). - **`model_unusable`** — authed but the account cannot use its configured model (#2477: HTTP 400 on every call, usually a stale `model =` pin in `~/.codex/config.toml`). Relay the probe's HINT lines, tell the user the one-line fix (update the pin; `[notice.model_migrations]` names the replacement), and fall back to the Claude subagent path. The ~10s round trip is cached for 1h; timeouts fail open to `ready`. - **`ready`** — run the Codex pass below. diff --git a/plan-eng-review/sections/review-sections.md b/plan-eng-review/sections/review-sections.md index 733de6fdd..d4da8f3fd 100644 --- a/plan-eng-review/sections/review-sections.md +++ b/plan-eng-review/sections/review-sections.md @@ -363,10 +363,17 @@ elif ! command -v codex >/dev/null 2>&1; then _CODEX_MODE="not_installed"; _gstack_codex_log_event "codex_cli_missing" 2>/dev/null || true elif ! _gstack_codex_auth_probe >/dev/null 2>&1; then _CODEX_MODE="not_authed"; _gstack_codex_log_event "codex_auth_failed" 2>/dev/null || true -elif ! _gstack_codex_model_probe; then - _CODEX_MODE="model_unusable" else - _CODEX_MODE="ready"; _gstack_codex_version_check 2>/dev/null || true + # Capture the probe's code: 2 means the CLI cannot execute at all, which is a + # different problem (and a different fix) from a model the account can't use. + _gstack_codex_model_probe; _CODEX_MP=$? + if [ "$_CODEX_MP" -eq 2 ]; then + _CODEX_MODE="broken_install" + elif [ "$_CODEX_MP" -ne 0 ]; then + _CODEX_MODE="model_unusable" + else + _CODEX_MODE="ready"; _gstack_codex_version_check 2>/dev/null || true + fi fi echo "CODEX_MODE: $_CODEX_MODE" ``` @@ -376,6 +383,7 @@ Branch on the echoed `CODEX_MODE`: - **`not_installed`** — Codex CLI absent. Print: "Codex not installed — using Claude subagent. Install for cross-model coverage: `npm install -g @openai/codex`." Fall back to the Claude subagent path. - **`under_codex`** — this session is already running INSIDE a Codex host, so spawning codex again is the same model reviewing itself at multiplied token cost (#2519). Print exactly one line: "[running under Codex — nested codex passes skipped; set GSTACK_FORCE_CODEX_REVIEW=1 to force]" and skip the codex invocations below; run the section's free in-host pass instead if it defines one. - **`not_authed`** — installed but no credentials. Print: "Codex installed but not authenticated — using Claude subagent. Run `codex login` or set `$CODEX_API_KEY`." Fall back to the Claude subagent path. +- **`broken_install`** — the CLI is on PATH but cannot execute (spawn ENOENT, non-executable binary, missing vendor payload). Print: "Codex is installed but its binary cannot run — Codex passes skipped. Reinstall: `npm install -g @openai/codex`." Relay the probe's HINT lines and fall back to the Claude subagent path. This state exists because a missing binary used to land in the model probe's fail-open bucket and report `ready`, so every Codex pass was skipped silently (#2742). - **`model_unusable`** — authed but the account cannot use its configured model (#2477: HTTP 400 on every call, usually a stale `model =` pin in `~/.codex/config.toml`). Relay the probe's HINT lines, tell the user the one-line fix (update the pin; `[notice.model_migrations]` names the replacement), and fall back to the Claude subagent path. The ~10s round trip is cached for 1h; timeouts fail open to `ready`. - **`ready`** — run the Codex pass below. diff --git a/review/sections/adversarial.md b/review/sections/adversarial.md index ce7b1b04f..ac76c083a 100644 --- a/review/sections/adversarial.md +++ b/review/sections/adversarial.md @@ -35,10 +35,17 @@ elif ! command -v codex >/dev/null 2>&1; then _CODEX_MODE="not_installed"; _gstack_codex_log_event "codex_cli_missing" 2>/dev/null || true elif ! _gstack_codex_auth_probe >/dev/null 2>&1; then _CODEX_MODE="not_authed"; _gstack_codex_log_event "codex_auth_failed" 2>/dev/null || true -elif ! _gstack_codex_model_probe; then - _CODEX_MODE="model_unusable" else - _CODEX_MODE="ready"; _gstack_codex_version_check 2>/dev/null || true + # Capture the probe's code: 2 means the CLI cannot execute at all, which is a + # different problem (and a different fix) from a model the account can't use. + _gstack_codex_model_probe; _CODEX_MP=$? + if [ "$_CODEX_MP" -eq 2 ]; then + _CODEX_MODE="broken_install" + elif [ "$_CODEX_MP" -ne 0 ]; then + _CODEX_MODE="model_unusable" + else + _CODEX_MODE="ready"; _gstack_codex_version_check 2>/dev/null || true + fi fi echo "CODEX_MODE: $_CODEX_MODE" ``` @@ -48,6 +55,7 @@ Branch on the echoed `CODEX_MODE`: - **`not_installed`** — Codex CLI absent. Print: "Codex not installed — using Claude subagent. Install for cross-model coverage: `npm install -g @openai/codex`." Fall back to the Claude subagent path. - **`under_codex`** — this session is already running INSIDE a Codex host, so spawning codex again is the same model reviewing itself at multiplied token cost (#2519). Print exactly one line: "[running under Codex — nested codex passes skipped; set GSTACK_FORCE_CODEX_REVIEW=1 to force]" and skip the codex invocations below; run the section's free in-host pass instead if it defines one. - **`not_authed`** — installed but no credentials. Print: "Codex installed but not authenticated — using Claude subagent. Run `codex login` or set `$CODEX_API_KEY`." Fall back to the Claude subagent path. +- **`broken_install`** — the CLI is on PATH but cannot execute (spawn ENOENT, non-executable binary, missing vendor payload). Print: "Codex is installed but its binary cannot run — Codex passes skipped. Reinstall: `npm install -g @openai/codex`." Relay the probe's HINT lines and fall back to the Claude subagent path. This state exists because a missing binary used to land in the model probe's fail-open bucket and report `ready`, so every Codex pass was skipped silently (#2742). - **`model_unusable`** — authed but the account cannot use its configured model (#2477: HTTP 400 on every call, usually a stale `model =` pin in `~/.codex/config.toml`). Relay the probe's HINT lines, tell the user the one-line fix (update the pin; `[notice.model_migrations]` names the replacement), and fall back to the Claude subagent path. The ~10s round trip is cached for 1h; timeouts fail open to `ready`. - **`ready`** — run the Codex pass below. diff --git a/scripts/resolvers/constants.ts b/scripts/resolvers/constants.ts index 5d2b967ea..b805cd3ae 100644 --- a/scripts/resolvers/constants.ts +++ b/scripts/resolvers/constants.ts @@ -130,10 +130,17 @@ elif ! command -v codex >/dev/null 2>&1; then ${m}="not_installed"; _gstack_codex_log_event "codex_cli_missing" 2>/dev/null || true elif ! _gstack_codex_auth_probe >/dev/null 2>&1; then ${m}="not_authed"; _gstack_codex_log_event "codex_auth_failed" 2>/dev/null || true -elif ! _gstack_codex_model_probe; then - ${m}="model_unusable" else - ${m}="ready"; _gstack_codex_version_check 2>/dev/null || true + # Capture the probe's code: 2 means the CLI cannot execute at all, which is a + # different problem (and a different fix) from a model the account can't use. + _gstack_codex_model_probe; _CODEX_MP=$? + if [ "$_CODEX_MP" -eq 2 ]; then + ${m}="broken_install" + elif [ "$_CODEX_MP" -ne 0 ]; then + ${m}="model_unusable" + else + ${m}="ready"; _gstack_codex_version_check 2>/dev/null || true + fi fi echo "CODEX_MODE: $${m}" \`\`\` @@ -143,6 +150,7 @@ Branch on the echoed \`CODEX_MODE\`: - **\`not_installed\`** — Codex CLI absent. Print: "Codex not installed — using Claude subagent. Install for cross-model coverage: \`npm install -g @openai/codex\`." Fall back to the Claude subagent path. - **\`under_codex\`** — this session is already running INSIDE a Codex host, so spawning codex again is the same model reviewing itself at multiplied token cost (#2519). Print exactly one line: "[running under Codex — nested codex passes skipped; set GSTACK_FORCE_CODEX_REVIEW=1 to force]" and skip the codex invocations below; run the section's free in-host pass instead if it defines one. - **\`not_authed\`** — installed but no credentials. Print: "Codex installed but not authenticated — using Claude subagent. Run \`codex login\` or set \`$CODEX_API_KEY\`." Fall back to the Claude subagent path. +- **\`broken_install\`** — the CLI is on PATH but cannot execute (spawn ENOENT, non-executable binary, missing vendor payload). Print: "Codex is installed but its binary cannot run — Codex passes skipped. Reinstall: \`npm install -g @openai/codex\`." Relay the probe's HINT lines and fall back to the Claude subagent path. This state exists because a missing binary used to land in the model probe's fail-open bucket and report \`ready\`, so every Codex pass was skipped silently (#2742). - **\`model_unusable\`** — authed but the account cannot use its configured model (#2477: HTTP 400 on every call, usually a stale \`model =\` pin in \`~/.codex/config.toml\`). Relay the probe's HINT lines, tell the user the one-line fix (update the pin; \`[notice.model_migrations]\` names the replacement), and fall back to the Claude subagent path. The ~10s round trip is cached for 1h; timeouts fail open to \`ready\`. - **\`ready\`** — run the Codex pass below.`; } diff --git a/ship/sections/adversarial.md b/ship/sections/adversarial.md index 1161fc536..4d4ebaf29 100644 --- a/ship/sections/adversarial.md +++ b/ship/sections/adversarial.md @@ -35,10 +35,17 @@ elif ! command -v codex >/dev/null 2>&1; then _CODEX_MODE="not_installed"; _gstack_codex_log_event "codex_cli_missing" 2>/dev/null || true elif ! _gstack_codex_auth_probe >/dev/null 2>&1; then _CODEX_MODE="not_authed"; _gstack_codex_log_event "codex_auth_failed" 2>/dev/null || true -elif ! _gstack_codex_model_probe; then - _CODEX_MODE="model_unusable" else - _CODEX_MODE="ready"; _gstack_codex_version_check 2>/dev/null || true + # Capture the probe's code: 2 means the CLI cannot execute at all, which is a + # different problem (and a different fix) from a model the account can't use. + _gstack_codex_model_probe; _CODEX_MP=$? + if [ "$_CODEX_MP" -eq 2 ]; then + _CODEX_MODE="broken_install" + elif [ "$_CODEX_MP" -ne 0 ]; then + _CODEX_MODE="model_unusable" + else + _CODEX_MODE="ready"; _gstack_codex_version_check 2>/dev/null || true + fi fi echo "CODEX_MODE: $_CODEX_MODE" ``` @@ -48,6 +55,7 @@ Branch on the echoed `CODEX_MODE`: - **`not_installed`** — Codex CLI absent. Print: "Codex not installed — using Claude subagent. Install for cross-model coverage: `npm install -g @openai/codex`." Fall back to the Claude subagent path. - **`under_codex`** — this session is already running INSIDE a Codex host, so spawning codex again is the same model reviewing itself at multiplied token cost (#2519). Print exactly one line: "[running under Codex — nested codex passes skipped; set GSTACK_FORCE_CODEX_REVIEW=1 to force]" and skip the codex invocations below; run the section's free in-host pass instead if it defines one. - **`not_authed`** — installed but no credentials. Print: "Codex installed but not authenticated — using Claude subagent. Run `codex login` or set `$CODEX_API_KEY`." Fall back to the Claude subagent path. +- **`broken_install`** — the CLI is on PATH but cannot execute (spawn ENOENT, non-executable binary, missing vendor payload). Print: "Codex is installed but its binary cannot run — Codex passes skipped. Reinstall: `npm install -g @openai/codex`." Relay the probe's HINT lines and fall back to the Claude subagent path. This state exists because a missing binary used to land in the model probe's fail-open bucket and report `ready`, so every Codex pass was skipped silently (#2742). - **`model_unusable`** — authed but the account cannot use its configured model (#2477: HTTP 400 on every call, usually a stale `model =` pin in `~/.codex/config.toml`). Relay the probe's HINT lines, tell the user the one-line fix (update the pin; `[notice.model_migrations]` names the replacement), and fall back to the Claude subagent path. The ~10s round trip is cached for 1h; timeouts fail open to `ready`. - **`ready`** — run the Codex pass below. diff --git a/test/codex-hardening.test.ts b/test/codex-hardening.test.ts index ee170d88f..e203f1589 100644 --- a/test/codex-hardening.test.ts +++ b/test/codex-hardening.test.ts @@ -599,3 +599,168 @@ describe('codex skeleton+sections union: review sandbox + fail-closed gate + tim }); } }); + +// #2742: a Codex CLI that is on PATH but cannot execute (spawn ENOENT, missing +// vendor payload, non-executable binary) used to land in the model probe's +// fail-open bucket and resolve to CODEX_MODE: ready — so every Codex pass was +// skipped in silence. These pin the classification, the exit-code contract, and +// the fact that the fail-open path still exists for genuine transients. +describe('codex broken-install detection (#2742)', () => { + // A fake `codex` on PATH that reproduces the real failure: node's spawn dump + // on stderr, non-zero exit. `mode` picks which failure shape to emit. + function shimHome(mode: 'enoent' | 'notexec' | 'timeout' | 'model400' | 'oksuspicious') { + const home = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-codex-shim-')); + const bin = path.join(home, 'bin'); + fs.mkdirSync(bin, { recursive: true }); + // auth.json so the auth probe passes and we reach the model probe. + fs.mkdirSync(path.join(home, '.codex'), { recursive: true }); + fs.writeFileSync(path.join(home, '.codex/auth.json'), '{}'); + const bodies: Record = { + enoent: + `echo "Error: spawn /x/vendor/aarch64-apple-darwin/codex/codex ENOENT" >&2\n` + + `echo " errno: -2, code: 'ENOENT'" >&2\nexit 1\n`, + notexec: `echo "bash: codex: cannot execute binary file" >&2\nexit 126\n`, + timeout: `echo "network hiccup" >&2\nexit 124\n`, + model400: `echo "The 'gpt-x' model is not supported when using Codex with a ChatGPT account" >&2\nexit 1\n`, + oksuspicious: `echo "OK — note: the log you pasted mentions permission denied on /var/log"\nexit 0\n`, + }; + fs.writeFileSync(path.join(bin, 'codex'), `#!/usr/bin/env bash\n${bodies[mode]}`, { mode: 0o755 }); + return { home, bin }; + } + + const cases: Array<[string, 'enoent' | 'notexec', string]> = [ + ['spawn ENOENT', 'enoent', 'ENOENT'], + ['non-executable binary (exit 126)', 'notexec', 'cannot execute binary file'], + ]; + + for (const [label, mode, needle] of cases) { + test(`${label} is classified as a broken install, not a transient`, () => { + const { home, bin } = shimHome(mode); + try { + const r = runProbe({ + snippet: '_gstack_codex_model_probe; echo "EXIT:$?"', + home, + env: { PATH: `${bin}:${process.env.PATH ?? ''}`, GSTACK_HOME: home }, + }); + expect(r.stdout).toContain('MODEL_UNUSABLE_INSTALL'); + // Exit 2 is what lets the preflight tell this apart from a model 400. + expect(r.stdout).toContain('EXIT:2'); + // It must NOT fail open — that was the whole defect. + expect(r.stdout).not.toContain('MODEL_PROBE_INCONCLUSIVE'); + // The remedy names the install, not the model pin. + expect(r.stdout).toContain('npm install -g @openai/codex'); + expect(r.stdout.toLowerCase()).toContain(needle.toLowerCase()); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + } + + test('a broken install is never cached — a reinstall is picked up next probe', () => { + const { home, bin } = shimHome('enoent'); + try { + runProbe({ + snippet: '_gstack_codex_model_probe >/dev/null 2>&1', + home, + env: { PATH: `${bin}:${process.env.PATH ?? ''}`, GSTACK_HOME: home }, + }); + const cache = path.join(home, '.codex-model-probe'); + if (fs.existsSync(cache)) { + expect(fs.readFileSync(cache, 'utf8')).not.toContain('MODEL_OK'); + } + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + + test('a genuine transient (exit 124) still fails open', () => { + const { home, bin } = shimHome('timeout'); + try { + const r = runProbe({ + snippet: '_gstack_codex_model_probe; echo "EXIT:$?"', + home, + env: { PATH: `${bin}:${process.env.PATH ?? ''}`, GSTACK_HOME: home }, + }); + expect(r.stdout).toContain('MODEL_PROBE_INCONCLUSIVE'); + expect(r.stdout).toContain('EXIT:0'); + expect(r.stdout).not.toContain('MODEL_UNUSABLE_INSTALL'); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + + test('the model 400 still classifies as MODEL_UNUSABLE, not a broken install', () => { + const { home, bin } = shimHome('model400'); + try { + const r = runProbe({ + snippet: '_gstack_codex_model_probe; echo "EXIT:$?"', + home, + env: { PATH: `${bin}:${process.env.PATH ?? ''}`, GSTACK_HOME: home }, + }); + expect(r.stdout).toContain('MODEL_UNUSABLE'); + expect(r.stdout).not.toContain('MODEL_UNUSABLE_INSTALL'); + expect(r.stdout).toContain('EXIT:1'); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + + test('version check warns instead of returning silently when codex cannot report a version', () => { + const { home, bin } = shimHome('enoent'); + try { + const r = runProbe({ + snippet: '_gstack_codex_version_check; echo "EXIT:$?"', + home, + env: { PATH: `${bin}:${process.env.PATH ?? ''}`, GSTACK_HOME: home }, + }); + // Previously this printed nothing: `codex --version 2>/dev/null | head -1` + // captured head's status, so a CLI that only ever errored read as healthy. + expect(r.stdout).toContain('WARN'); + expect(r.stdout).toContain('npm install -g @openai/codex'); + // Still non-fatal — the version check has never gated anything. + expect(r.stdout).toContain('EXIT:0'); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + + // Wave-amended (#2745 absorption): string signatures only count on a FAILED + // spawn — a SUCCESSFUL response whose text mentions "permission denied" + // (e.g. the model quoting a log the user pasted) must stay healthy. + test('exit-0 response mentioning "permission denied" is NOT a broken install', () => { + const { home, bin } = shimHome('oksuspicious'); + try { + const r = runProbe({ + snippet: '_gstack_codex_model_probe; echo "EXIT:$?"', + home, + env: { PATH: `${bin}:${process.env.PATH ?? ''}`, GSTACK_HOME: home }, + }); + expect(r.stdout).not.toContain('MODEL_UNUSABLE_INSTALL'); + expect(r.stdout).toContain('EXIT:0'); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + + // Wave-amended (#2745 absorption): autoplan's preflight chain is the one + // hand-maintained copy that isn't resolver-generated — it must capture the + // probe's exit code and route 2 to its own broken-install arm, or /autoplan + // prints the wrong remedy for a broken binary. + test('autoplan preflight (tmpl + rendered) captures the probe exit and routes 2 to broken-install', () => { + for (const rel of ['autoplan/SKILL.md.tmpl', 'autoplan/SKILL.md']) { + const src = fs.readFileSync(path.join(ROOT, rel), 'utf-8'); + expect(src).toContain('_gstack_codex_model_probe; _CODEX_MP=$?'); + expect(src).toMatch(/_CODEX_MP" -eq 2/); + expect(src).toContain('binary cannot run'); + expect(src).not.toContain('elif ! _gstack_codex_model_probe'); + } + }); + + test('the preflight resolver routes exit 2 to broken_install', () => { + const src = fs.readFileSync(path.join(ROOT, 'scripts/resolvers/constants.ts'), 'utf8'); + expect(src).toContain('broken_install'); + // The chain must capture the probe's code; `elif ! _gstack_codex_model_probe` + // collapses 1 and 2 into one branch and loses the distinction. + expect(src).toContain('_CODEX_MP=$?'); + }); +}); diff --git a/test/fixtures/golden/factory-ship-SKILL.md b/test/fixtures/golden/factory-ship-SKILL.md index 250ddb527..6414b6ded 100644 --- a/test/fixtures/golden/factory-ship-SKILL.md +++ b/test/fixtures/golden/factory-ship-SKILL.md @@ -2148,10 +2148,17 @@ elif ! command -v codex >/dev/null 2>&1; then _CODEX_MODE="not_installed"; _gstack_codex_log_event "codex_cli_missing" 2>/dev/null || true elif ! _gstack_codex_auth_probe >/dev/null 2>&1; then _CODEX_MODE="not_authed"; _gstack_codex_log_event "codex_auth_failed" 2>/dev/null || true -elif ! _gstack_codex_model_probe; then - _CODEX_MODE="model_unusable" else - _CODEX_MODE="ready"; _gstack_codex_version_check 2>/dev/null || true + # Capture the probe's code: 2 means the CLI cannot execute at all, which is a + # different problem (and a different fix) from a model the account can't use. + _gstack_codex_model_probe; _CODEX_MP=$? + if [ "$_CODEX_MP" -eq 2 ]; then + _CODEX_MODE="broken_install" + elif [ "$_CODEX_MP" -ne 0 ]; then + _CODEX_MODE="model_unusable" + else + _CODEX_MODE="ready"; _gstack_codex_version_check 2>/dev/null || true + fi fi echo "CODEX_MODE: $_CODEX_MODE" ``` @@ -2161,6 +2168,7 @@ Branch on the echoed `CODEX_MODE`: - **`not_installed`** — Codex CLI absent. Print: "Codex not installed — using Claude subagent. Install for cross-model coverage: `npm install -g @openai/codex`." Fall back to the Claude subagent path. - **`under_codex`** — this session is already running INSIDE a Codex host, so spawning codex again is the same model reviewing itself at multiplied token cost (#2519). Print exactly one line: "[running under Codex — nested codex passes skipped; set GSTACK_FORCE_CODEX_REVIEW=1 to force]" and skip the codex invocations below; run the section's free in-host pass instead if it defines one. - **`not_authed`** — installed but no credentials. Print: "Codex installed but not authenticated — using Claude subagent. Run `codex login` or set `$CODEX_API_KEY`." Fall back to the Claude subagent path. +- **`broken_install`** — the CLI is on PATH but cannot execute (spawn ENOENT, non-executable binary, missing vendor payload). Print: "Codex is installed but its binary cannot run — Codex passes skipped. Reinstall: `npm install -g @openai/codex`." Relay the probe's HINT lines and fall back to the Claude subagent path. This state exists because a missing binary used to land in the model probe's fail-open bucket and report `ready`, so every Codex pass was skipped silently (#2742). - **`model_unusable`** — authed but the account cannot use its configured model (#2477: HTTP 400 on every call, usually a stale `model =` pin in `~/.codex/config.toml`). Relay the probe's HINT lines, tell the user the one-line fix (update the pin; `[notice.model_migrations]` names the replacement), and fall back to the Claude subagent path. The ~10s round trip is cached for 1h; timeouts fail open to `ready`. - **`ready`** — run the Codex pass below.