fix(install-cli): match Codex's temp-home warning, not our own temp root - #36
Merged
Merged
Conversation
install-cli rejected every installation whose Codex home sits under a temporary directory: installed Codex returned unexpected version diagnostics The check rebuilt Codex's warning locally from tempfile.gettempdir() and compared the whole string for equality. Codex reports the temporary root it resolved -- "/tmp" -- while any caller that redirects TMPDIR for isolation makes gettempdir() return something else. Every harness lane redirects it, so the two never agreed and a warning that is entirely expected was treated as unknown output. Read the root out of the warning instead of assuming it. Strictness is kept by binding the message to this invocation: the codex_home it names must be exactly our resolved target, and that target must genuinely sit under the root it named, so a lookalike message about some other directory still fails. The failure also quoted nothing. A mismatch against an unseen string, on a stream produced by a third-party binary whose wording we do not control, is a bisect rather than a fix; it now includes the diagnostics, already bounded by VERSION_OUTPUT_MAX_BYTES. Verified: `run_harnesses.py --module nddev-codex-app --lane platform --platform ubuntu` goes from exit 2 to PASS. Claude-Session: https://claude.ai/code/session_017fG88hR3mfP7YMFABzbKPB
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.
Problem
install-clifailed for every installation whose Codex home sits under a temporary directory:is_expected_temporary_codex_home_warningrebuilt the whole warning locally fromtempfile.gettempdir()and compared it for string equality. Codex reports the temporary root it resolved. Captured from a real run of the pinnedrust-v0.146.0:Codex says
/tmp; the manager computed the caller's redirectedTMPDIR. Any caller that redirects it — every harness lane does, for isolation — made the two disagree permanently.Change
Read the root out of the warning instead of assuming it matches ours.
Strictness is preserved by binding the message to the invocation rather than to a guessed temp root: the
codex_homeit names must be exactly our resolved target, and that target must genuinely sit under the root it named. A lookalike message about a different directory still fails.The failure path also quoted nothing at all. A mismatch against an unseen string, on a stream produced by a third-party binary whose wording is outside our control, is a bisect rather than a fix — it now includes the diagnostics, already bounded by
VERSION_OUTPUT_MAX_BYTES.Verification
python3 scripts/run_harnesses.py --module nddev-codex-app --lane platform --platform ubuntuagainst the pinned release:exit 2,codex runtime proof: FAILvalidate_platform.sh (ubuntu) — nddev-codex-app: PASSruff checkandruff format --checkclean.https://claude.ai/code/session_017fG88hR3mfP7YMFABzbKPB