fix(codex): a CLI that cannot execute no longer reports CODEX_MODE: ready - #2745
Draft
ukheni50 wants to merge 2 commits into
Draft
fix(codex): a CLI that cannot execute no longer reports CODEX_MODE: ready#2745ukheni50 wants to merge 2 commits into
ukheni50 wants to merge 2 commits into
Conversation
added 2 commits
August 31, 2026 15:33
…eady Follow-up to garrytan#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 garrytan#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 garrytan#2742
The preflight chain is embedded in the ship skill, so changing it changes the rendered SKILL.md. Only the factory golden moved; the claude golden tracks the committed render (already regenerated) and the codex one was unaffected. The diff is exactly the new branch and its mode documentation.
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
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.
Fixes #2742.
Why (in your own words)
Codex passes can be skipped for months without anyone noticing.
@openai/codexwas installed and on myPATH, butnode_modules/@openai/codex-darwin-arm64/vendor/aarch64-apple-darwin/codex/was an empty directory — the binary had been removed some time after install while its siblingvendor/.../path/rgstayed intact. Every invocation died withspawn ... ENOENT, and gstack reportedCODEX_MODE: readythroughout./ship,/reviewand/autoplanskipped their Codex passes silently and reported themselves complete. Reviews I believed were cross-model were single-model for two months.The cause of the deletion was local and isn't the point. The detection gap is: #2477 added
_gstack_codex_model_probeprecisely so "auth exists" would stop being mistaken for "Codex works", and it does a real round trip — but its final branch is theelseof a "model 400" grep. A spawn ENOENT has neither a 400 nor the word "model", so it landed in the bucket whose comment reads "Timeout (124) or transient failure: fail-open... not to gate on network luck." That reasoning is right for a timeout. A missing binary is deterministic: retrying never helps.This adds a
broken_installstate, keeps fail-open for genuine transients, and stops the version check reading a broken CLI as healthy.Live evidence
Before — unmodified
main(253d1df), with acodexon PATH that cannot execute:A CLI that cannot run resolves to
ready, and the one hint printed points the user at their model configuration.After — this branch, same shim, same chain:
The shim reproduces the real failure exactly (node's spawn dump on stderr, non-zero exit) and the chain is the one from
scripts/resolvers/constants.ts, run verbatim.All three original probes passed on the broken install — this is what the issue reproduces, and why nothing caught it:
Tests — the suites that cover the changed files:
The fix caught its own regression. My first version of the version-check warned whenever
$_verwas empty, which broke the existingempty output → OK (silent, no crash)case — that behavior is deliberate, so the condition is now narrowed to a non-zero exit only. The real broken CLI exits non-zero, so the detection is unaffected.Scope
bin/gstack-codex-probe—_gstack_codex_model_probeclassifies deterministic install failures (exit 126/127, or stderr matchingENOENT|ENOEXEC|EACCES|cannot execute binary file|no such file or directory|permission denied) asMODEL_UNUSABLE_INSTALL, exit 2, never cached so a reinstall is picked up on the next probe. Exit 124 and genuine transients still fail open._gstack_codex_version_checkcaptures codex's own exit code instead ofhead's (codex --version 2>/dev/null | head -1reported the pipeline's last status) and warns on non-zero;2>/dev/nullalso discarded the only diagnostic, so stderr is kept.scripts/resolvers/constants.ts— the chain captures the probe's code rather than testing truthiness, so exit 2 routes to a newbroken_installmode whose remedy isnpm install -g @openai/codex, not "check your model pin". Regenerated the six section files and the factory ship golden.test/codex-hardening.test.ts— 6 tests using the existingrunProbeharness.broken_installshould also be surfaced by/autoplan's own preflight copy, if it has one.On the full suite — the measurement, and two wrong explanations I gave before it.
Full-suite numbers on this machine:
main(253d1df)Every one of the 115 is a
browsetest. I twice guessed at a mechanism and was wrong both times — first "two suites running concurrently", disproved when a non-concurrent run failed the same way; then "orphaned browse daemons holding the lock", disproved when I reaped every one of them (pgrep -f <clone> → 0) and a clean run still failed 115. I am not offering a third guess.Here is the controlled comparison instead. Same browse suite, both clones, back to back, daemons killed between runs:
Identical, to the test and to a third of a second. The same two fail on both —
Path traversal prevention > eval rejects path traversal with ..and> cookie-import rejects path traversal— so they are pre-existing onmain, not from this change.So:
browsebehaves the same with and without this diff when measured in isolation. The full-suite difference is variance in how this machine runs all 592 files together; whatever drives it, it is not this change, and I would rather show you the isolation comparison than assert a cause I have not proven. Worth noting the suite is already known to be non-deterministic here (#2536, #2597).What the diff can actually reach is green. It is 10 files —
bin/gstack-codex-probe, 6 generated section.mds,scripts/resolvers/constants.ts,test/codex-hardening.test.ts, and the factory golden — nothing inbrowseimports any of them, and every suite that covers them passes:The one non-
browsefailure across all my runs,setup: ensure_emoji_font ... (color=False), passes in isolation on both this branch andmain.I am flagging the corrections rather than quietly editing them, since a maintainer may have read the earlier versions.
Liveness proof (required)
This PR was prepared with an AI coding agent on the author's machine. The liveness screenshot exists to confirm a human opened the PR, so it would defeat its own purpose for the agent to produce one; it is deliberately left for the human author rather than faked. Marked as a draft until then.
Checklist
GSTACK PRtyped live into a real surface (not edited onto the image)bin/gstack-codex-probe+scripts/resolvers/constants.tsand ranbun run gen:skill-docs)