Skip to content

Support multiple managed environments#18401

Merged
starr-openai merged 20 commits intomainfrom
env-manager-map-20260417
Apr 21, 2026
Merged

Support multiple managed environments#18401
starr-openai merged 20 commits intomainfrom
env-manager-map-20260417

Conversation

@starr-openai
Copy link
Copy Markdown
Contributor

Summary

  • refactor EnvironmentManager to own keyed environments with default/local lookup helpers
  • keep remote exec-server client creation lazy until exec/fs use
  • preserve disabled agent environment access separately from internal local environment access

Validation

  • not run (per Codex worktree instruction to avoid tests/builds unless requested)

Comment thread codex-rs/exec-server/src/environment.rs
Comment thread codex-rs/exec-server/src/remote_file_system.rs Outdated
@starr-openai starr-openai force-pushed the env-manager-map-20260417 branch 6 times, most recently from ec3596e to 8f1d61b Compare April 20, 2026 19:04
@starr-openai starr-openai marked this pull request as ready for review April 20, 2026 19:09
@starr-openai starr-openai requested a review from a team as a code owner April 20, 2026 19:09
@starr-openai starr-openai force-pushed the env-manager-map-20260417 branch from 8f1d61b to 70d3a74 Compare April 20, 2026 19:32
Comment thread codex-rs/exec/src/lib.rs Outdated
Some(local_runtime_paths),
)),
environment_manager: std::sync::Arc::new(EnvironmentManager::new(EnvironmentManagerArgs {
exec_server_url: std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok(),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: Should we hide the env-var reading?

Comment thread codex-rs/app-server/src/message_processor/tracing_tests.rs
Comment thread codex-rs/app-server/src/codex_message_processor.rs Outdated
Comment thread codex-rs/core/src/session/session.rs
Comment thread codex-rs/core/tests/suite/tools.rs Outdated
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn exec_server_none_omits_environment_backed_tools() -> Result<()> {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

though we had this one

Comment thread codex-rs/exec-server/src/client.rs
Comment thread codex-rs/exec-server/src/environment.rs Outdated
@starr-openai starr-openai force-pushed the env-manager-map-20260417 branch from 3b5e90b to 154be3f Compare April 21, 2026 17:36
@starr-openai starr-openai enabled auto-merge (squash) April 21, 2026 17:52
@starr-openai starr-openai force-pushed the env-manager-map-20260417 branch from 8620bea to f748352 Compare April 21, 2026 18:23
Comment thread codex-rs/exec-server/src/environment.rs Outdated
fn local_with_runtime_paths(local_runtime_paths: Option<ExecServerRuntimePaths>) -> Self {
let filesystem: Arc<dyn ExecutorFileSystem> = match local_runtime_paths.clone() {
Some(runtime_paths) => Arc::new(LocalFileSystem::with_runtime_paths(runtime_paths)),
None => Arc::new(LocalFileSystem::unsandboxed()),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is this a security incident waiting to happen?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

good point - let me try to make ExecServerRuntimePaths required so we don't silently drop

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

that said in codex-rs/exec-server/src/local_file_system.rs we do throw if we try to make a sandboxed fs operation but have no configured runtime operations

exec_server_url: None,
remote_exec_server_client: None,
exec_backend: Arc::new(LocalProcess::default()),
filesystem: Arc::new(LocalFileSystem::unsandboxed()),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm a bit worried how freely we are creationg unsandboxed fs impls.
Not blocking. Longer term we need to reduce how many ways to create environment there are.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

made this default_for_tests only, remved Default constructor to avoid change it's created accidentally

@starr-openai starr-openai disabled auto-merge April 21, 2026 20:32
@starr-openai starr-openai force-pushed the env-manager-map-20260417 branch from 67d3e68 to 981cdd6 Compare April 21, 2026 21:18
starr-openai and others added 4 commits April 21, 2026 14:47
Refactor EnvironmentManager to own a keyed environment registry with explicit default and local lookups. Keep remote exec-server connections lazy at environment use sites and preserve disabled agent environment access separately from internal local environment access.

Co-authored-by: Codex <noreply@openai.com>
Use EnvironmentManager::new for args-struct construction and keep from_env methods as the env-var factory entrypoints.

Co-authored-by: Codex <noreply@openai.com>
Return concrete default and local environments from EnvironmentManager now that the manager always installs local and default entries. Keep arbitrary ID lookup optional.

Co-authored-by: Codex <noreply@openai.com>
Keep the lazy remote exec-server client wrapper alongside ExecServerClient and import it from the client module for environment-backed exec and filesystem use.

Co-authored-by: Codex <noreply@openai.com>
starr-openai and others added 16 commits April 21, 2026 14:48
Use EnvironmentManager::new with EnvironmentManagerArgs for runtime-path-aware construction and keep from_env only for the no-args env-var factory.

Co-authored-by: Codex <noreply@openai.com>
Add high-level EnvironmentManager docs for local/remote initialization, default environment selection, disabled agent access, and lazy remote connection behavior.

Co-authored-by: Codex <noreply@openai.com>
Drop the unused local_environment helper and keep local lookups on the generic get_environment API.

Co-authored-by: Codex <noreply@openai.com>
Clarify that SessionServices carries an Arc handle to the process-level EnvironmentManager rather than owning a session-specific manager.

Co-authored-by: Codex <noreply@openai.com>
Restore CODEX_EXEC_SERVER_URL=none semantics by making EnvironmentManager::default_environment return None when environment access is disabled. Remove the separate disabled-for-agent flag and derive tool availability from the optional default environment.

Add an end-to-end tool exposure test for CODEX_EXEC_SERVER_URL=none.

Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Create one lazy exec-server client per remote environment and pass clones into the remote process and filesystem backends. This keeps ExecServerClient as the connected-client type while avoiding duplicate websocket clients for one environment.

Co-authored-by: Codex <noreply@openai.com>
Make EnvironmentManagerArgs::default() own CODEX_EXEC_SERVER_URL parsing so production entrypoints can keep using EnvironmentManager::new with struct update syntax for runtime paths. Add explicit test defaults so test managers do not depend on the process environment.

Co-authored-by: Codex <noreply@openai.com>
Drop the networked integration test for CODEX_EXEC_SERVER_URL=none omitting environment-backed tools. Lower-level coverage already verifies disabled environments omit those tools.

Co-authored-by: Codex <noreply@openai.com>
Make EnvironmentManagerArgs carry ExecServerRuntimePaths for production construction and route test-only unsandboxed setup through explicit _for_tests helpers. Use the manager local environment for MCP and app-server filesystem fallbacks instead of constructing a fresh default environment.

Co-authored-by: Codex <noreply@openai.com>
Remove the app-server-client re-export now that environment-manager construction owns CODEX_EXEC_SERVER_URL reading directly in exec-server.

Co-authored-by: Codex <noreply@openai.com>
Add a connector loading helper that accepts the existing EnvironmentManager and switch app-server paths to use it. Keep the config-only helper as a temporary fallback for callers such as TUI that do not yet pass the manager through.

Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Codex <noreply@openai.com>
@starr-openai starr-openai force-pushed the env-manager-map-20260417 branch from 981cdd6 to 3ec48a2 Compare April 21, 2026 21:50
@starr-openai starr-openai merged commit ddbe253 into main Apr 21, 2026
35 of 36 checks passed
@starr-openai starr-openai deleted the env-manager-map-20260417 branch April 21, 2026 22:29
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 21, 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