Skip to content

arg0: keep dispatch aliases alive during async main#18999

Merged
bolinfest merged 1 commit intomainfrom
pr18999
Apr 22, 2026
Merged

arg0: keep dispatch aliases alive during async main#18999
bolinfest merged 1 commit intomainfrom
pr18999

Conversation

@bolinfest
Copy link
Copy Markdown
Collaborator

@bolinfest bolinfest commented Apr 22, 2026

Why

The Ubuntu GNU remote Cargo run has been regularly failing sandboxed suite::remote_env filesystem tests with No such file or directory, while the same cases pass under Bazel. The Cargo remote-env setup starts target/debug/codex exec-server inside Docker via scripts/test-remote-env.sh. That CLI builds codex-linux-sandbox and other arg0 helper aliases in a temporary directory, then passes those alias paths into the exec-server runtime.

arg0_dispatch_or_else constructed Arg0DispatchPaths from that temporary alias guard, but then awaited the async CLI entry point without otherwise keeping the guard live. That allowed the guard to be dropped while the exec-server was still running, removing the helper alias directory. Later sandboxed filesystem calls tried to spawn the now-deleted codex-linux-sandbox path and surfaced as ENOENT.

The relevant distinction I found is that core/tests/common stores the result of arg0_dispatch() in a process-lifetime OnceLock<Option<Arg0PathEntryGuard>> for test binaries. The Cargo remote-env setup exercises a real codex exec-server process instead, so it depends on the normal CLI lifetime behavior fixed here.

What Changed

  • Keep the arg0 tempdir guard alive until main_fn(paths).await completes.
  • Keep the helper on the real arg0_dispatch() shape, where alias setup can fail and return None in production.
  • Add a regression test that uses an explicit guard, yields once, and verifies the generated helper alias path still exists while the async entry point is running.

Verification

  • cargo test -p codex-arg0
  • just argument-comment-lint -p codex-arg0
  • just fix -p codex-arg0

Copy link
Copy Markdown
Contributor

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d7716e0f26

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread codex-rs/arg0/src/lib.rs Outdated
Comment on lines +495 to +497
unsafe {
std::env::set_var("CODEX_HOME", codex_home.path());
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Serialize environment mutation in this test

This test mutates CODEX_HOME/PATH via unsafe std::env::set_var/remove_var without any serialization, but Rust tests run in parallel by default. If any other test thread reads environment variables concurrently, this violates std::env's safety contract and can cause undefined behavior or flaky CI failures. Please wrap these mutations in a process-wide lock (or mark the test serial) so environment access is synchronized.

Useful? React with 👍 / 👎.

@bolinfest bolinfest force-pushed the pr18999 branch 4 times, most recently from 884a49d to ac0da77 Compare April 22, 2026 17:08
@bolinfest bolinfest enabled auto-merge (squash) April 22, 2026 17:16
@bolinfest
Copy link
Copy Markdown
Collaborator Author

I'm not sure why Bazel tests appear "frozen" on the PR page, but I see them running here: https://github.com/openai/codex/actions/runs/24792741756/job/72554548095?pr=18999

@bolinfest bolinfest disabled auto-merge April 22, 2026 17:50
@bolinfest bolinfest enabled auto-merge (squash) April 22, 2026 17:51
@bolinfest bolinfest disabled auto-merge April 22, 2026 18:06
@bolinfest bolinfest merged commit ddde50c into main Apr 22, 2026
39 checks passed
@bolinfest bolinfest deleted the pr18999 branch April 22, 2026 18:06
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 22, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants