Add CODEX_ENV_FILE hook persistence#24650
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fab7852a0c
ℹ️ 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".
|
@codex review again |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e37f412cc
ℹ️ 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".
| if let Some(path) = env_path(codex_hooks::CODEX_ENV_FILE_ENV_VAR) | ||
| .or_else(|| env_path(codex_hooks::CLAUDE_ENV_FILE_ENV_VAR)) | ||
| { | ||
| return Self { path }; |
There was a problem hiding this comment.
When the parent process has CODEX_ENV_FILE or CLAUDE_ENV_FILE set to a path outside Codex-managed storage, this accepts that arbitrary path as the session env file; later local shell_command/exec_command calls expose it in the command environment and add an exact read permission so the sandbox can source it. That means launching Codex with, for example, CODEX_ENV_FILE=$HOME/.ssh/id_rsa lets a sandboxed model command read a file the workspace sandbox would otherwise block, so inherited paths need to be constrained or tracked separately from the auto-granted managed temp file.
Useful? React with 👍 / 👎.
| handler.env.insert( | ||
| crate::CODEX_ENV_FILE_ENV_VAR.to_string(), | ||
| env_file_path.clone(), | ||
| ); |
There was a problem hiding this comment.
When more than one matching SessionStart handler uses CODEX_ENV_FILE, every handler gets the same path here, but dispatcher::execute_handlers launches the selected handlers concurrently with FuturesUnordered. Hooks that append PATH or override the same variable therefore race, so the resulting env file order is nondeterministic and can reverse or corrupt setup for subsequent shell commands; this needs ordered execution or an ordered merge for env-file-producing hooks.
Useful? React with 👍 / 👎.
| if run_pending_session_start_hooks(&session, &turn_context).await { | ||
| return; |
There was a problem hiding this comment.
This runs pending SessionStart hooks for both standalone /shell turns and /shell commands attached to an already active turn. If a user fires /shell while the newly spawned model turn has not yet reached its own run_pending_session_start_hooks call, the auxiliary task can pop the pending startup hook first; a hook that returns continue: false then only cancels the shell command while the main model turn proceeds without seeing the stop decision. Gate this path to StandaloneTurn so active-turn auxiliary commands cannot steal pending start hooks.
Useful? React with 👍 / 👎.
| Self { | ||
| path: codex_home | ||
| .join("tmp") | ||
| .join(HOOK_ENV_DIR) | ||
| .join(format!("{thread_id}-{}.sh", uuid::Uuid::new_v4())), |
There was a problem hiding this comment.
The automatically chosen env file lives under codex_home/tmp/env, but the new HookEnvFile has no cleanup path (repo-wide references only prepare and source it). SessionStart hooks can write exports containing tokens or other transient setup into this shell script, so every session leaves a plaintext file in Codex home indefinitely; add session-end or stale-file cleanup for files created by Codex instead of relying on the tmp directory name.
Useful? React with 👍 / 👎.
1 participant
Why
CC has
CLAUDE_ENV_FILEas a shell script path that gets sourced before Bash commands so hook-written exports can persist across later commands - https://code.claude.com/docs/en/env-vars#variablesCodex needs the same so
SessionStarthooks can makePATH, virtualenv, conda, and similar setup visible to shell tools without mutating persistent config or rewriting the user-visible commandWhat
CODEX_ENV_FILE, withCLAUDE_ENV_FILEas a compatibility alias for realSessionStarthooks.shell_command,exec_command, and/shellcommand execution while preserving the original command for approval, hooks, and display.tmp/envunless the process explicitly providesCODEX_ENV_FILEorCLAUDE_ENV_FILE, and create the parent directory lazily only when a hook or local shell command needs it.