From 15801f69d9cde324724cb1a390ed1b9909640c8f Mon Sep 17 00:00:00 2001 From: Richard Lee Date: Tue, 7 Jul 2026 13:31:15 -0700 Subject: [PATCH] [codex] support pathless hosted thread managers --- codex-rs/agent-graph-store/src/store.rs | 8 + codex-rs/app-server/src/mcp_refresh.rs | 2 +- codex-rs/app-server/src/message_processor.rs | 2 +- codex-rs/codex-mcp/src/connection_manager.rs | 15 +- .../codex-mcp/src/connection_manager_tests.rs | 2 +- codex-rs/codex-mcp/src/mcp/mod.rs | 16 +- codex-rs/codex-mcp/src/mcp/mod_tests.rs | 28 +- codex-rs/config/src/types.rs | 8 + codex-rs/core-api/src/lib.rs | 3 + codex-rs/core-plugins/src/manager.rs | 294 +++++++--- codex-rs/core-plugins/src/manager_tests.rs | 48 ++ codex-rs/core-skills/src/service.rs | 79 ++- codex-rs/core-skills/src/service_tests.rs | 114 ++++ codex-rs/core/src/agent/control_tests.rs | 2 +- codex-rs/core/src/agent/role.rs | 17 + codex-rs/core/src/client.rs | 4 +- codex-rs/core/src/client_tests.rs | 34 +- codex-rs/core/src/codex_delegate.rs | 2 + codex-rs/core/src/codex_thread.rs | 5 + codex-rs/core/src/config/mod.rs | 2 +- codex-rs/core/src/connectors.rs | 2 +- codex-rs/core/src/exec_policy.rs | 3 + codex-rs/core/src/lib.rs | 4 + codex-rs/core/src/local_runtime_paths.rs | 110 ++++ codex-rs/core/src/mcp_skill_dependencies.rs | 74 ++- codex-rs/core/src/mcp_tool_call.rs | 12 +- codex-rs/core/src/mcp_tool_call_tests.rs | 62 ++- codex-rs/core/src/prompt_debug.rs | 2 +- codex-rs/core/src/realtime_conversation.rs | 3 +- codex-rs/core/src/responses_metadata.rs | 18 +- codex-rs/core/src/session/config_lock.rs | 25 + codex-rs/core/src/session/mcp.rs | 33 +- codex-rs/core/src/session/mcp_runtime.rs | 2 +- codex-rs/core/src/session/mod.rs | 125 ++++- codex-rs/core/src/session/review.rs | 4 +- codex-rs/core/src/session/session.rs | 64 ++- codex-rs/core/src/session/tests.rs | 63 ++- .../core/src/session/tests/guardian_tests.rs | 3 +- codex-rs/core/src/session/turn_context.rs | 6 +- .../core/src/session_rollout_init_error.rs | 12 +- codex-rs/core/src/test_support.rs | 2 +- codex-rs/core/src/thread_manager.rs | 138 ++++- codex-rs/core/src/thread_manager_tests.rs | 502 +++++++++++++++++- .../src/tools/handlers/multi_agents/spawn.rs | 10 +- .../src/tools/handlers/multi_agents_tests.rs | 2 +- .../tools/handlers/multi_agents_v2/spawn.rs | 10 +- .../tools/handlers/request_plugin_install.rs | 14 +- codex-rs/core/src/turn_metadata.rs | 33 +- codex-rs/core/src/turn_metadata_tests.rs | 69 ++- codex-rs/core/tests/common/test_codex.rs | 12 +- codex-rs/core/tests/suite/client.rs | 2 +- codex-rs/mcp-server/src/message_processor.rs | 2 +- codex-rs/memories/write/src/runtime.rs | 9 +- codex-rs/memories/write/src/startup_tests.rs | 73 +++ .../model-provider/src/amazon_bedrock/mod.rs | 10 + codex-rs/model-provider/src/provider.rs | 38 +- codex-rs/models-manager/src/cache.rs | 41 +- codex-rs/models-manager/src/manager.rs | 25 +- codex-rs/models-manager/src/manager_tests.rs | 18 + codex-rs/rmcp-client/src/oauth.rs | 47 +- codex-rs/thread-manager-sample/src/main.rs | 2 +- codex-rs/thread-store/src/in_memory.rs | 4 + codex-rs/thread-store/src/store.rs | 8 + 63 files changed, 2142 insertions(+), 241 deletions(-) create mode 100644 codex-rs/core/src/local_runtime_paths.rs diff --git a/codex-rs/agent-graph-store/src/store.rs b/codex-rs/agent-graph-store/src/store.rs index 0760cb15d4..7054a0bc0e 100644 --- a/codex-rs/agent-graph-store/src/store.rs +++ b/codex-rs/agent-graph-store/src/store.rs @@ -15,6 +15,14 @@ pub type AgentGraphStoreFuture<'a, T> = /// Implementations are expected to return stable ordering for list methods so callers can merge /// persisted graph state with live in-memory state without introducing nondeterministic output. pub trait AgentGraphStore: Send + Sync { + /// Whether this store requires host-local runtime paths. + /// + /// The default fails closed. Remote or in-memory implementations must opt + /// out explicitly before they can be attached to a remote-only manager. + fn requires_local_runtime_paths(&self) -> bool { + true + } + /// Insert or replace the directional parent/child edge for a spawned thread. /// /// `child_thread_id` has at most one persisted parent. Re-inserting the same child should diff --git a/codex-rs/app-server/src/mcp_refresh.rs b/codex-rs/app-server/src/mcp_refresh.rs index 848d201f2e..9ac9c0b3af 100644 --- a/codex-rs/app-server/src/mcp_refresh.rs +++ b/codex-rs/app-server/src/mcp_refresh.rs @@ -248,7 +248,7 @@ mod tests { /*analytics_events_client*/ None, Arc::clone(&thread_store), codex_core::local_agent_graph_store_from_state_db(Some(&state_db)), - "11111111-1111-4111-8111-111111111111".to_string(), + Some("11111111-1111-4111-8111-111111111111".to_string()), /*attestation_provider*/ None, /*external_time_provider*/ None, ) diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 62eff72660..f0a168d2d5 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -287,7 +287,7 @@ impl MessageProcessor { Some(analytics_events_client.clone()), Arc::clone(&thread_store), codex_core::local_agent_graph_store_from_state_db(state_db.as_ref()), - installation_id, + Some(installation_id), Some(app_server_attestation_provider( outgoing.clone(), thread_state_manager.clone(), diff --git a/codex-rs/codex-mcp/src/connection_manager.rs b/codex-rs/codex-mcp/src/connection_manager.rs index 2e6c518d11..de7be38d56 100644 --- a/codex-rs/codex-mcp/src/connection_manager.rs +++ b/codex-rs/codex-mcp/src/connection_manager.rs @@ -134,7 +134,7 @@ impl McpConnectionManager { startup_cancellation_token: CancellationToken, initial_permission_profile: PermissionProfile, runtime_context: McpRuntimeContext, - codex_home: PathBuf, + codex_home: impl Into>, codex_apps_tools_cache: CodexAppsToolsCache, codex_apps_tools_cache_key: CodexAppsToolsCacheKey, prefix_mcp_tool_names: bool, @@ -147,6 +147,7 @@ impl McpConnectionManager { elicitation_lifecycle: Option, elicitation_router: ElicitationRequestRouter, ) -> Self { + let codex_home = codex_home.into(); let mut required_servers = mcp_servers .iter() .filter(|(_, server)| server.enabled() && server.required()) @@ -203,10 +204,14 @@ impl McpConnectionManager { }); let shares_codex_apps_tools_cache = should_share_codex_apps_tools_cache(&server_name, uses_env_bearer_token); - let codex_apps_tools_cache_context = shares_codex_apps_tools_cache.then(|| { - codex_apps_tools_cache - .context(codex_home.clone(), codex_apps_tools_cache_key.clone()) - }); + let codex_apps_tools_cache_context = if shares_codex_apps_tools_cache { + codex_home.as_ref().map(|codex_home| { + codex_apps_tools_cache + .context(codex_home.clone(), codex_apps_tools_cache_key.clone()) + }) + } else { + None + }; // The reserved Codex Apps registration follows the shared // AuthManager across refreshes. In the hosted-plugin path, this // is the ChatGPT /ps/mcp connection. User-configured MCP diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs index faaa198b68..7fe0c4df0d 100644 --- a/codex-rs/codex-mcp/src/connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs @@ -1471,7 +1471,7 @@ async fn no_local_runtime_fails_local_stdio_but_keeps_local_http_server() { Arc::new(EnvironmentManager::without_environments()), PathBuf::from("/tmp"), ), - codex_home.path().to_path_buf(), + Some(codex_home.path().to_path_buf()), CodexAppsToolsCache::default(), CodexAppsToolsCacheKey { account_id: None, diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index 8a1847bc96..a5c1824b52 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -119,7 +119,8 @@ pub struct McpConfig { /// Optional product SKU forwarded to the host-owned apps MCP server. pub apps_mcp_product_sku: Option, /// Codex home directory used for MCP OAuth state and app-tool cache files. - pub codex_home: PathBuf, + /// Remote-only runtimes omit it and keep the shared app-tool cache in memory. + pub codex_home: Option, /// Preferred credential store for MCP OAuth tokens. pub mcp_oauth_credentials_store_mode: OAuthCredentialsStoreMode, /// Backend used when MCP OAuth storage is configured for keyring-backed persistence. @@ -153,6 +154,19 @@ pub struct McpConfig { pub connector_snapshot: ConnectorSnapshot, } +impl McpConfig { + /// Applies the host-local storage capability before auth discovery or client startup. + /// + /// Without a local state root, OAuth must not consult process-global file + /// or keyring storage shared by other hosted sessions. + pub fn apply_local_storage_policy(&mut self, codex_home: Option) { + if codex_home.is_none() { + self.mcp_oauth_credentials_store_mode = OAuthCredentialsStoreMode::Disabled; + } + self.codex_home = codex_home; + } +} + #[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct ToolPluginProvenance { plugin_display_names_by_connector_id: HashMap>, diff --git a/codex-rs/codex-mcp/src/mcp/mod_tests.rs b/codex-rs/codex-mcp/src/mcp/mod_tests.rs index 99143673e4..38f1eff919 100644 --- a/codex-rs/codex-mcp/src/mcp/mod_tests.rs +++ b/codex-rs/codex-mcp/src/mcp/mod_tests.rs @@ -4,6 +4,7 @@ use crate::McpServerRegistration; use codex_config::Constrained; use codex_config::types::AppToolApproval; use codex_config::types::AuthKeyringBackendKind; +use codex_config::types::OAuthCredentialsStoreMode; use codex_login::CodexAuth; use codex_plugin::AppConnectorId; use codex_plugin::PluginCapabilitySummary; @@ -21,7 +22,7 @@ fn test_mcp_config(codex_home: PathBuf) -> McpConfig { McpConfig { chatgpt_base_url: "https://chatgpt.com".to_string(), apps_mcp_product_sku: None, - codex_home, + codex_home: Some(codex_home), mcp_oauth_credentials_store_mode: OAuthCredentialsStoreMode::default(), auth_keyring_backend_kind: AuthKeyringBackendKind::default(), mcp_oauth_callback_port: None, @@ -38,6 +39,31 @@ fn test_mcp_config(codex_home: PathBuf) -> McpConfig { } } +#[test] +fn local_storage_policy_disables_oauth_storage_only_without_a_local_root() { + let configured_home = PathBuf::from("/configured/codex/home"); + let mut local_config = test_mcp_config(PathBuf::new()); + local_config.mcp_oauth_credentials_store_mode = OAuthCredentialsStoreMode::File; + local_config.apply_local_storage_policy(Some(configured_home.clone())); + let mut pathless_config = local_config.clone(); + pathless_config.apply_local_storage_policy(None); + + assert_eq!( + ( + local_config.codex_home, + local_config.mcp_oauth_credentials_store_mode, + ), + (Some(configured_home), OAuthCredentialsStoreMode::File) + ); + assert_eq!( + ( + pathless_config.codex_home, + pathless_config.mcp_oauth_credentials_store_mode, + ), + (None, OAuthCredentialsStoreMode::Disabled) + ); +} + #[test] fn qualified_mcp_tool_name_prefix_sanitizes_server_names_without_lowercasing() { assert_eq!( diff --git a/codex-rs/config/src/types.rs b/codex-rs/config/src/types.rs index a66ff3f3a7..033d9fb8d9 100644 --- a/codex-rs/config/src/types.rs +++ b/codex-rs/config/src/types.rs @@ -114,6 +114,14 @@ pub enum OAuthCredentialsStoreMode { File, /// Keyring when available, otherwise fail. Keyring, + /// Internal runtime mode that neither reads nor persists MCP OAuth credentials. + /// + /// Hosted sessions use this when host-local storage is unavailable. It is + /// deliberately excluded from user configuration because OAuth login + /// cannot hand an unpersisted token to a later client connection. + #[serde(skip)] + #[schemars(skip)] + Disabled, } /// Determine how auth credentials should use keyring-backed storage. diff --git a/codex-rs/core-api/src/lib.rs b/codex-rs/core-api/src/lib.rs index df15483dcc..c65d08c5c3 100644 --- a/codex-rs/core-api/src/lib.rs +++ b/codex-rs/core-api/src/lib.rs @@ -28,12 +28,15 @@ pub use codex_config::types::UriBasedFileOpener; pub use codex_core::CodexThread; pub use codex_core::ForkSnapshot; pub use codex_core::LoadedAgentsMd; +pub use codex_core::LocalRuntimePaths; pub use codex_core::McpManager; pub use codex_core::NewThread; pub use codex_core::StartThreadOptions; pub use codex_core::StateDbHandle; pub use codex_core::ThreadManager; +pub use codex_core::ThreadManagerInitError; pub use codex_core::ThreadShutdownReport; +pub use codex_core::ThreadStartError; pub use codex_core::config::Config; pub use codex_core::config::Constrained; pub use codex_core::config::ExtraConfig; diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 16fb4c8663..31026ee0f8 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -353,8 +353,7 @@ impl From for PluginCapabilitySummary { } pub struct PluginsManager { - codex_home: PathBuf, - store: PluginStore, + local_storage: Option, featured_plugin_ids_cache: RwLock>, recommended_plugins_cache: RwLock>, recommended_plugins_refreshes: @@ -374,6 +373,12 @@ pub struct PluginsManager { plugin_install_source: PluginInstallSource, } +#[derive(Clone)] +struct LocalPluginStorage { + codex_home: PathBuf, + store: PluginStore, +} + #[derive(Clone)] struct LoadedPluginsCacheEntry { key: PluginLoadCacheKey, @@ -428,9 +433,37 @@ impl PluginsManager { // already-admitted plugins may continue exposing MCP servers/tools from shared local state. // // This assumes a single CODEX_HOME is only used by one product. + let store = PluginStore::new(codex_home.clone()); + Self::new_inner( + Some(LocalPluginStorage { codex_home, store }), + restriction_product, + auth_mode, + ) + } + + /// Creates a manager with local plugin discovery, mutation, and caches disabled. + pub fn new_without_host_filesystem() -> Self { + Self::new_without_host_filesystem_with_options( + Some(Product::Codex), + /*auth_mode*/ None, + ) + } + + /// Creates a host-filesystem-free manager with explicit runtime options. + pub fn new_without_host_filesystem_with_options( + restriction_product: Option, + auth_mode: Option, + ) -> Self { + Self::new_inner(/*local_storage*/ None, restriction_product, auth_mode) + } + + fn new_inner( + local_storage: Option, + restriction_product: Option, + auth_mode: Option, + ) -> Self { Self { - codex_home: codex_home.clone(), - store: PluginStore::new(codex_home), + local_storage, featured_plugin_ids_cache: RwLock::new(None), recommended_plugins_cache: RwLock::new(HashMap::new()), recommended_plugins_refreshes: RwLock::new(HashMap::new()), @@ -460,6 +493,14 @@ impl PluginsManager { self } + fn local_storage(&self) -> Result<&LocalPluginStorage, PluginStoreError> { + self.local_storage.as_ref().ok_or_else(|| { + PluginStoreError::Invalid( + "plugin operation requires host filesystem access".to_string(), + ) + }) + } + pub fn set_auth_mode(&self, auth_mode: Option) -> bool { let mut stored_auth_mode = match self.auth_mode.write() { Ok(auth_mode_guard) => auth_mode_guard, @@ -480,7 +521,9 @@ impl PluginsManager { } fn remote_global_catalog_active(&self, config: &PluginsConfigInput) -> bool { - config.remote_plugin_enabled && self.auth_mode().is_some_and(AuthMode::uses_codex_backend) + self.local_storage.is_some() + && config.remote_plugin_enabled + && self.auth_mode().is_some_and(AuthMode::uses_codex_backend) } pub fn set_analytics_events_client(&self, analytics_events_client: AnalyticsEventsClient) { @@ -511,12 +554,13 @@ impl PluginsManager { &self, config: &PluginsConfigInput, ) -> Option { + let local_storage = self.local_storage.as_ref()?; if !config.plugins_enabled { return None; } let key = PluginLoadCacheKey::from_config( config, - self.codex_home.as_path(), + local_storage.codex_home.as_path(), self.remote_global_catalog_active(config), ); self.loaded_plugins_cache @@ -543,6 +587,9 @@ impl PluginsManager { config: &PluginsConfigInput, force_reload: bool, ) -> PluginLoadOutcome { + let Some(local_storage) = self.local_storage.as_ref() else { + return PluginLoadOutcome::default(); + }; if !config.plugins_enabled { return PluginLoadOutcome::default(); } @@ -550,7 +597,7 @@ impl PluginsManager { let remote_global_catalog_active = self.remote_global_catalog_active(config); let cache_key = PluginLoadCacheKey::from_config( config, - self.codex_home.as_path(), + local_storage.codex_home.as_path(), remote_global_catalog_active, ); if !force_reload && let Some(plugins) = self.cached_loaded_plugins(&cache_key) { @@ -569,7 +616,7 @@ impl PluginsManager { let plugins = load_plugins_from_layer_stack( &config.config_layer_stack, self.remote_installed_plugin_configs(), - &self.store, + &local_storage.store, Some(&plugin_skill_snapshots), self.restriction_product, remote_global_catalog_active, @@ -648,13 +695,16 @@ impl PluginsManager { config_layer_stack: &ConfigLayerStack, config: &PluginsConfigInput, ) -> PluginLoadOutcome { + let Some(local_storage) = self.local_storage.as_ref() else { + return PluginLoadOutcome::default(); + }; if !config.plugins_enabled { return PluginLoadOutcome::default(); } let plugins = load_plugins_from_layer_stack( config_layer_stack, self.remote_installed_plugin_configs(), - &self.store, + &local_storage.store, /*plugin_skill_snapshots*/ None, self.restriction_product, self.remote_global_catalog_active(config), @@ -669,13 +719,16 @@ impl PluginsManager { config_layer_stack: &ConfigLayerStack, config: &PluginsConfigInput, ) -> PluginHookLoadOutcome { + let Some(local_storage) = self.local_storage.as_ref() else { + return PluginHookLoadOutcome::default(); + }; if !config.plugins_enabled { return PluginHookLoadOutcome::default(); } load_plugin_hooks_from_layer_stack( config_layer_stack, self.remote_installed_plugin_configs(), - &self.store, + &local_storage.store, self.remote_global_catalog_active(config), ) .await @@ -730,6 +783,9 @@ impl PluginsManager { } fn remote_installed_plugin_configs(&self) -> HashMap { + let Some(local_storage) = self.local_storage.as_ref() else { + return HashMap::new(); + }; let cache = match self.remote_installed_plugins_cache.read() { Ok(cache) => cache, Err(err) => err.into_inner(), @@ -738,7 +794,7 @@ impl PluginsManager { return HashMap::new(); }; - remote_installed_plugins_to_config(plugins, &self.store) + remote_installed_plugins_to_config(plugins, &local_storage.store) } fn remote_plugin_id_for(&self, plugin_id: &PluginId) -> Option { @@ -759,7 +815,8 @@ impl PluginsManager { return cached_remote_plugin_id; } - match self.store.remote_plugin_id(plugin_id) { + let local_storage = self.local_storage.as_ref()?; + match local_storage.store.remote_plugin_id(plugin_id) { Ok(remote_plugin_id) => remote_plugin_id, Err(err) => { tracing::warn!( @@ -777,7 +834,11 @@ impl PluginsManager { plugin_id: &PluginId, ) -> PluginTelemetryMetadata { let mut metadata = self.telemetry_metadata_for_plugin_id(plugin_id); - metadata.capability_summary = match self.store.active_plugin_root(plugin_id) { + metadata.capability_summary = match self + .local_storage + .as_ref() + .and_then(|local_storage| local_storage.store.active_plugin_root(plugin_id)) + { Some(plugin_root) => plugin_capability_summary_from_root(plugin_id, &plugin_root).await, None => None, }; @@ -791,7 +852,11 @@ impl PluginsManager { ) -> PluginTelemetryMetadata { let mut metadata = self.telemetry_metadata_for_plugin_id_with_remote_id(plugin_id, remote_plugin_id); - metadata.capability_summary = match self.store.active_plugin_root(plugin_id) { + metadata.capability_summary = match self + .local_storage + .as_ref() + .and_then(|local_storage| local_storage.store.active_plugin_root(plugin_id)) + { Some(plugin_root) => plugin_capability_summary_from_root(plugin_id, &plugin_root).await, None => None, }; @@ -836,6 +901,7 @@ impl PluginsManager { &self, visible_marketplaces: &[&str], ) -> Option> { + self.local_storage.as_ref()?; let cache = match self.remote_installed_plugins_cache.read() { Ok(cache) => cache, Err(err) => err.into_inner(), @@ -854,6 +920,9 @@ impl PluginsManager { config: &PluginsConfigInput, auth: Option<&CodexAuth>, ) -> Vec { + let Some(local_storage) = self.local_storage.as_ref() else { + return Vec::new(); + }; if !config.plugins_enabled || !config.remote_plugin_enabled { return Vec::new(); } @@ -868,7 +937,7 @@ impl PluginsManager { } crate::remote::cached_global_remote_discoverable_plugins( - self.codex_home.as_path(), + local_storage.codex_home.as_path(), &remote_plugin_service_config(config), auth, ) @@ -881,6 +950,9 @@ impl PluginsManager { visible_marketplaces: &[&str], on_effective_plugins_changed: Option>, ) -> Result, RemotePluginCatalogError> { + if self.local_storage.is_none() { + return Ok(Vec::new()); + } let plugins = crate::remote::fetch_remote_installed_plugins( &remote_plugin_service_config(config), auth, @@ -898,6 +970,9 @@ impl PluginsManager { } fn write_remote_installed_plugins_cache(&self, plugins: Vec) -> bool { + if self.local_storage.is_none() { + return false; + } let mut cache = match self.remote_installed_plugins_cache.write() { Ok(cache) => cache, Err(err) => err.into_inner(), @@ -912,6 +987,9 @@ impl PluginsManager { } pub fn clear_remote_installed_plugins_cache(&self) -> bool { + if self.local_storage.is_none() { + return false; + } let mut cache = match self.remote_installed_plugins_cache.write() { Ok(cache) => cache, Err(err) => err.into_inner(), @@ -931,6 +1009,9 @@ impl PluginsManager { auth: Option, on_effective_plugins_changed: Option>, ) { + if self.local_storage.is_none() { + return; + } self.maybe_start_remote_installed_plugins_cache_refresh_with_notify( config, auth.clone(), @@ -953,6 +1034,9 @@ impl PluginsManager { auth: Option, on_effective_plugins_changed: Option>, ) { + if self.local_storage.is_none() { + return; + } self.maybe_start_remote_installed_plugins_cache_refresh_with_notify( config, auth, @@ -968,7 +1052,7 @@ impl PluginsManager { notify: RemoteInstalledPluginsCacheRefreshNotify, on_effective_plugins_changed: Option>, ) { - if !config.plugins_enabled { + if self.local_storage.is_none() || !config.plugins_enabled { return; } @@ -988,6 +1072,9 @@ impl PluginsManager { auth: Option, on_effective_plugins_changed: Option>, ) { + let Some(local_storage) = self.local_storage.as_ref() else { + return; + }; if !config.plugins_enabled { return; } @@ -1004,7 +1091,7 @@ impl PluginsManager { }); crate::remote::maybe_start_remote_installed_plugin_bundle_sync( - self.codex_home.clone(), + local_storage.codex_home.clone(), remote_plugin_service_config(config), auth, Some(on_local_cache_changed), @@ -1016,7 +1103,8 @@ impl PluginsManager { config: &PluginsConfigInput, auth: Option, ) { - if !config.plugins_enabled || !config.remote_plugin_enabled { + if self.local_storage.is_none() || !config.plugins_enabled || !config.remote_plugin_enabled + { return; } @@ -1034,6 +1122,9 @@ impl PluginsManager { options: PluginListBackgroundTaskOptions, on_effective_plugins_changed: Option>, ) { + if self.local_storage.is_none() { + return; + } self.maybe_start_non_curated_plugin_cache_refresh(config, roots); if options.refresh_global_remote_catalog_cache { self.maybe_start_global_remote_catalog_cache_refresh(config, auth.clone()); @@ -1103,7 +1194,7 @@ impl PluginsManager { config: &PluginsConfigInput, auth: Option<&CodexAuth>, ) -> Result, RemotePluginFetchError> { - if !config.plugins_enabled { + if self.local_storage.is_none() || !config.plugins_enabled { return Ok(Vec::new()); } @@ -1134,7 +1225,8 @@ impl PluginsManager { config: &PluginsConfigInput, auth: Option<&CodexAuth>, ) -> RecommendedPluginsMode { - if !config.plugins_enabled + if self.local_storage.is_none() + || !config.plugins_enabled || !config.remote_plugin_enabled || !auth.is_some_and(CodexAuth::uses_codex_backend) { @@ -1283,6 +1375,7 @@ impl PluginsManager { config_layer_stack: &ConfigLayerStack, request: PluginInstallRequest, ) -> Result { + self.local_storage()?; let resolved = self.resolve_installable_plugin(config_layer_stack, &request)?; let plugin_id = resolved.plugin_id.clone(); match self.install_resolved_plugin(resolved).await { @@ -1304,6 +1397,7 @@ impl PluginsManager { config_layer_stack: &ConfigLayerStack, request: &PluginInstallRequest, ) -> Result { + let local_storage = self.local_storage()?; let resolved = match find_installable_marketplace_plugin( &request.marketplace_path, &request.plugin_name, @@ -1319,7 +1413,7 @@ impl PluginsManager { MarketplacePolicy::from_requirements(config_layer_stack.requirements()) .validate_install( config_layer_stack, - self.codex_home.as_path(), + local_storage.codex_home.as_path(), &request.marketplace_path, &resolved.plugin_id.marketplace_name, ) @@ -1340,6 +1434,7 @@ impl PluginsManager { auth: Option<&CodexAuth>, request: PluginInstallRequest, ) -> Result { + self.local_storage()?; let resolved = self.resolve_installable_plugin(&config.config_layer_stack, &request)?; let plugin_id = resolved.plugin_id.as_key(); // This only forwards the backend mutation before the local install flow. @@ -1437,21 +1532,24 @@ impl PluginsManager { &self, resolved: ResolvedMarketplacePlugin, ) -> Result { + let local_storage = self.local_storage()?; let auth_policy = resolved.policy.authentication; let plugin_version = if is_openai_curated_marketplace_name(&resolved.plugin_id.marketplace_name) { - let curated_plugin_version = read_curated_plugins_sha(self.codex_home.as_path()) - .ok_or_else(|| { - PluginStoreError::Invalid( - "local curated marketplace sha is not available".to_string(), - ) - })?; + let curated_plugin_version = read_curated_plugins_sha( + local_storage.codex_home.as_path(), + ) + .ok_or_else(|| { + PluginStoreError::Invalid( + "local curated marketplace sha is not available".to_string(), + ) + })?; Some(curated_plugin_cache_version(&curated_plugin_version)) } else { None }; - let store = self.store.clone(); - let codex_home = self.codex_home.clone(); + let store = local_storage.store.clone(); + let codex_home = local_storage.codex_home.clone(); let manifest_fallback_contents = resolved .manifest_fallback .contents_if_has_metadata() @@ -1484,7 +1582,7 @@ impl PluginsManager { .map_err(PluginInstallError::join)??; set_user_plugin_enabled( - &self.codex_home, + &local_storage.codex_home, result.plugin_id.as_key(), /*enabled*/ true, ) @@ -1511,6 +1609,7 @@ impl PluginsManager { } pub async fn uninstall_plugin(&self, plugin_id: String) -> Result<(), PluginUninstallError> { + self.local_storage()?; let plugin_id = PluginId::parse(&plugin_id)?; self.uninstall_plugin_id(plugin_id).await } @@ -1521,6 +1620,7 @@ impl PluginsManager { auth: Option<&CodexAuth>, plugin_id: String, ) -> Result<(), PluginUninstallError> { + self.local_storage()?; // TODO: Remove this legacy remote-sync path once remote plugins have // their own manager and installed-state API. let plugin_id = PluginId::parse(&plugin_id)?; @@ -1537,7 +1637,8 @@ impl PluginsManager { } async fn uninstall_plugin_id(&self, plugin_id: PluginId) -> Result<(), PluginUninstallError> { - let plugin_telemetry = if self.store.active_plugin_root(&plugin_id).is_some() { + let local_storage = self.local_storage()?; + let plugin_telemetry = if local_storage.store.active_plugin_root(&plugin_id).is_some() { Some( self.telemetry_metadata_for_installed_plugin(&plugin_id) .await, @@ -1545,13 +1646,13 @@ impl PluginsManager { } else { None }; - let store = self.store.clone(); + let store = local_storage.store.clone(); let plugin_id_for_store = plugin_id.clone(); tokio::task::spawn_blocking(move || store.uninstall(&plugin_id_for_store)) .await .map_err(PluginUninstallError::join)??; - clear_user_plugin(&self.codex_home, plugin_id.as_key()) + clear_user_plugin(&local_storage.codex_home, plugin_id.as_key()) .await .map_err(anyhow::Error::from)?; @@ -1574,6 +1675,9 @@ impl PluginsManager { additional_roots: &[AbsolutePathBuf], include_openai_curated: bool, ) -> Result { + let Some(local_storage) = self.local_storage.as_ref() else { + return Ok(ConfiguredMarketplaceListOutcome::default()); + }; if !config.plugins_enabled { return Ok(ConfiguredMarketplaceListOutcome::default()); } @@ -1603,9 +1707,9 @@ impl PluginsManager { PluginId::new(plugin.name.clone(), marketplace_name.clone()).ok(); let installed = installed_plugins.contains(&plugin_key); let installed_version = installed.then_some(()).and_then(|_| { - plugin_id - .as_ref() - .and_then(|plugin_id| self.store.active_plugin_version(plugin_id)) + plugin_id.as_ref().and_then(|plugin_id| { + local_storage.store.active_plugin_version(plugin_id) + }) }); let enabled = enabled_plugins.contains(&plugin_key); let mut interface = plugin.interface; @@ -1614,7 +1718,8 @@ impl PluginsManager { if installed && plugin.source.is_install_materialized() && let Some(plugin_id) = plugin_id.as_ref() - && let Some(plugin_root) = self.store.active_plugin_root(plugin_id) + && let Some(plugin_root) = + local_storage.store.active_plugin_root(plugin_id) && let Some(manifest) = load_plugin_manifest(plugin_root.as_path()) { local_version = manifest.version.clone(); @@ -1666,7 +1771,7 @@ impl PluginsManager { config: &PluginsConfigInput, additional_roots: &[AbsolutePathBuf], ) -> Result { - if !config.plugins_enabled { + if self.local_storage.is_none() || !config.plugins_enabled { return Ok(MarketplaceListOutcome::default()); } @@ -1684,6 +1789,9 @@ impl PluginsManager { plugin: &ConfiguredMarketplacePlugin, skill_config_rules: &SkillConfigRules, ) -> Result { + if self.local_storage.is_none() { + return Err(MarketplaceError::PluginsDisabled); + } let fragment = self .tool_suggest_metadata_cache .metadata_for_plugin(marketplace_name, plugin, self.restriction_product) @@ -1696,6 +1804,9 @@ impl PluginsManager { config: &PluginsConfigInput, request: &PluginReadRequest, ) -> Result { + let Some(local_storage) = self.local_storage.as_ref() else { + return Err(MarketplaceError::PluginsDisabled); + }; if !config.plugins_enabled { return Err(MarketplaceError::PluginsDisabled); } @@ -1704,7 +1815,7 @@ impl PluginsManager { MarketplacePolicy::from_requirements(config.config_layer_stack.requirements()) .validate_install( &config.config_layer_stack, - self.codex_home.as_path(), + local_storage.codex_home.as_path(), &request.marketplace_path, &plugin.plugin_id.marketplace_name, ) @@ -1728,7 +1839,7 @@ impl PluginsManager { let (installed_plugins, enabled_plugins) = self.configured_plugin_states(config); let installed = installed_plugins.contains(&plugin_key); let installed_version = if installed { - self.store.active_plugin_version(&plugin.plugin_id) + local_storage.store.active_plugin_version(&plugin.plugin_id) } else { None }; @@ -1773,6 +1884,9 @@ impl PluginsManager { marketplace_name: &str, plugin: ConfiguredMarketplacePlugin, ) -> Result { + let Some(local_storage) = self.local_storage.as_ref() else { + return Err(MarketplaceError::PluginsDisabled); + }; if !self.restriction_product_matches(plugin.policy.products.as_deref()) { return Err(MarketplaceError::PluginNotFound { plugin_name: plugin.name, @@ -1813,13 +1927,16 @@ impl PluginsManager { } let source_path = if plugin.source.is_install_materialized() && plugin.installed { - self.store.active_plugin_root(&plugin_id).ok_or_else(|| { - MarketplaceError::InvalidPlugin(format!( - "installed plugin cache entry is missing for {plugin_key}" - )) - })? + local_storage + .store + .active_plugin_root(&plugin_id) + .ok_or_else(|| { + MarketplaceError::InvalidPlugin(format!( + "installed plugin cache entry is missing for {plugin_key}" + )) + })? } else { - let codex_home = self.codex_home.clone(); + let codex_home = local_storage.codex_home.clone(); let source = plugin.source.clone(); let materialized = tokio::task::spawn_blocking(move || { materialize_marketplace_plugin_source(codex_home.as_path(), &source) @@ -1870,7 +1987,7 @@ impl PluginsManager { /*plugin_skill_snapshots*/ None, ) .await; - let plugin_data_root = self.store.plugin_data_root(&plugin_id); + let plugin_data_root = local_storage.store.plugin_data_root(&plugin_id); let (hook_sources, _hook_load_warnings) = load_plugin_hooks(&source_path, &plugin_id, &plugin_data_root, &manifest.paths); let hooks = plugin_hook_declarations(&hook_sources) @@ -1938,6 +2055,9 @@ impl PluginsManager { auth_manager: Arc, on_effective_plugins_changed: Option>, ) { + let Some(local_storage) = self.local_storage.as_ref() else { + return; + }; if config.plugins_enabled { let use_remote_global_catalog = config.remote_plugin_enabled && auth_manager.current_auth_uses_codex_backend(); @@ -1996,6 +2116,7 @@ impl PluginsManager { } } let config_for_remote_sync = config.clone(); + let codex_home_for_remote_sync = local_storage.codex_home.clone(); let manager = Arc::clone(self); let auth_manager_for_remote_sync = auth_manager.clone(); let on_effective_plugins_changed = on_effective_plugins_changed.clone(); @@ -2013,7 +2134,7 @@ impl PluginsManager { ); if config_for_remote_sync.remote_plugin_enabled { match crate::remote::fetch_and_cache_global_remote_plugin_catalog( - manager.codex_home.as_path(), + codex_home_for_remote_sync.as_path(), &remote_plugin_service_config(&config_for_remote_sync), auth.as_ref(), ) @@ -2056,8 +2177,12 @@ impl PluginsManager { config: &PluginsConfigInput, marketplace_name: Option<&str>, ) -> Result { + let local_storage = self + .local_storage + .as_ref() + .ok_or_else(|| "plugin operation requires host filesystem access".to_string())?; let mut outcome = upgrade_configured_git_marketplaces( - self.codex_home.as_path(), + local_storage.codex_home.as_path(), &config.config_layer_stack, marketplace_name, ); @@ -2071,13 +2196,13 @@ impl PluginsManager { if !outcome.upgraded_roots.is_empty() { let mut configured_plugin_keys = configured_plugins_from_stack( &config.config_layer_stack, - self.codex_home.as_path(), + local_storage.codex_home.as_path(), ) .into_keys() .collect::>(); configured_plugin_keys.sort_unstable(); match refresh_non_curated_plugin_cache_force_reinstall_detailed( - self.codex_home.as_path(), + local_storage.codex_home.as_path(), &outcome.upgraded_roots, &configured_plugin_keys, ) { @@ -2126,6 +2251,9 @@ impl PluginsManager { self: &Arc, mut request: RemoteInstalledPluginsCacheRefreshRequest, ) { + if self.local_storage.is_none() { + return; + } let should_spawn = { let mut state = match self.remote_installed_plugins_cache_refresh_state.write() { Ok(state) => state, @@ -2168,6 +2296,9 @@ impl PluginsManager { self: &Arc, request: GlobalRemoteCatalogCacheRefreshRequest, ) { + if self.local_storage.is_none() { + return; + } let should_spawn = { let mut state = match self.global_remote_catalog_cache_refresh_state.write() { Ok(state) => state, @@ -2197,6 +2328,9 @@ impl PluginsManager { roots: &[AbsolutePathBuf], mode: NonCuratedCacheRefreshMode, ) { + let Some(local_storage) = self.local_storage.as_ref() else { + return; + }; let marketplace_roots = self.marketplace_roots(config, roots, /*include_openai_curated*/ false); let outcome = match self.list_marketplaces_with_policy(config, &marketplace_roots) { @@ -2214,7 +2348,7 @@ impl PluginsManager { .filter_map(|marketplace| { match policy.validate_install( &config.config_layer_stack, - self.codex_home.as_path(), + local_storage.codex_home.as_path(), &marketplace.path, &marketplace.name, ) { @@ -2233,10 +2367,12 @@ impl PluginsManager { .collect::>(); roots.sort_unstable(); roots.dedup(); - let mut configured_plugin_keys = - configured_plugins_from_stack(&config.config_layer_stack, self.codex_home.as_path()) - .into_keys() - .collect::>(); + let mut configured_plugin_keys = configured_plugins_from_stack( + &config.config_layer_stack, + local_storage.codex_home.as_path(), + ) + .into_keys() + .collect::>(); configured_plugin_keys.sort_unstable(); if roots.is_empty() || configured_plugin_keys.is_empty() { return; @@ -2299,11 +2435,14 @@ impl PluginsManager { } fn start_curated_repo_sync(self: &Arc) { + let Some(local_storage) = self.local_storage.as_ref() else { + return; + }; if CURATED_REPO_SYNC_STARTED.swap(true, Ordering::SeqCst) { return; } let manager = Arc::clone(self); - let codex_home = self.codex_home.clone(); + let codex_home = local_storage.codex_home.clone(); if let Err(err) = std::thread::Builder::new() .name("plugins-curated-repo-sync".to_string()) .spawn( @@ -2340,6 +2479,9 @@ impl PluginsManager { } async fn run_remote_installed_plugins_cache_refresh_loop(self: Arc) { + if self.local_storage.is_none() { + return; + } loop { let request = { let mut state = match self.remote_installed_plugins_cache_refresh_state.write() { @@ -2400,6 +2542,9 @@ impl PluginsManager { } async fn run_global_remote_catalog_cache_refresh_loop(self: Arc) { + let Some(local_storage) = self.local_storage.clone() else { + return; + }; loop { let request = { let mut state = match self.global_remote_catalog_cache_refresh_state.write() { @@ -2416,7 +2561,7 @@ impl PluginsManager { }; match crate::remote::fetch_and_cache_global_remote_plugin_catalog( - self.codex_home.as_path(), + local_storage.codex_home.as_path(), &request.service_config, request.auth.as_ref(), ) @@ -2438,6 +2583,9 @@ impl PluginsManager { } fn run_non_curated_plugin_cache_refresh_loop(self: Arc) { + let Some(local_storage) = self.local_storage.clone() else { + return; + }; loop { let request = { let state = match self.non_curated_cache_refresh_state.read() { @@ -2459,14 +2607,14 @@ impl PluginsManager { let refresh_result = match request.mode { NonCuratedCacheRefreshMode::IfVersionChanged => { refresh_non_curated_plugin_cache_detailed( - self.codex_home.as_path(), + local_storage.codex_home.as_path(), &request.roots, &request.configured_plugin_keys, ) } NonCuratedCacheRefreshMode::ForceReinstall => { refresh_non_curated_plugin_cache_force_reinstall_detailed( - self.codex_home.as_path(), + local_storage.codex_home.as_path(), &request.roots, &request.configured_plugin_keys, ) @@ -2512,14 +2660,19 @@ impl PluginsManager { &self, config: &PluginsConfigInput, ) -> (HashSet, HashSet) { - let configured_plugins = - configured_plugins_from_stack(&config.config_layer_stack, self.codex_home.as_path()); + let Some(local_storage) = self.local_storage.as_ref() else { + return (HashSet::new(), HashSet::new()); + }; + let configured_plugins = configured_plugins_from_stack( + &config.config_layer_stack, + local_storage.codex_home.as_path(), + ); let installed_plugins = configured_plugins .keys() .filter(|plugin_key| { PluginId::parse(plugin_key) .ok() - .is_some_and(|plugin_id| self.store.is_installed(&plugin_id)) + .is_some_and(|plugin_id| local_storage.store.is_installed(&plugin_id)) }) .cloned() .collect::>(); @@ -2536,12 +2689,15 @@ impl PluginsManager { additional_roots: &[AbsolutePathBuf], include_openai_curated: bool, ) -> Vec { + let Some(local_storage) = self.local_storage.as_ref() else { + return Vec::new(); + }; // Treat the curated catalog as an extra marketplace root so plugin listing can surface it // without requiring every caller to know where it is stored. let mut roots = additional_roots.to_vec(); roots.extend(installed_marketplace_roots_from_layer_stack( &config.config_layer_stack, - self.codex_home.as_path(), + local_storage.codex_home.as_path(), )); let curated_marketplace_path = if include_openai_curated { if matches!( @@ -2549,12 +2705,13 @@ impl PluginsManager { Some(AuthMode::ApiKey | AuthMode::BedrockApiKey) ) { let api_marketplace_path = - curated_plugins_api_marketplace_path(self.codex_home.as_path()); + curated_plugins_api_marketplace_path(local_storage.codex_home.as_path()); api_marketplace_path .is_file() .then_some(api_marketplace_path) } else { - let curated_repo_root = curated_plugins_repo_path(self.codex_home.as_path()); + let curated_repo_root = + curated_plugins_repo_path(local_storage.codex_home.as_path()); curated_repo_root.is_dir().then_some(curated_repo_root) } } else { @@ -2576,6 +2733,9 @@ impl PluginsManager { config: &PluginsConfigInput, roots: &[AbsolutePathBuf], ) -> Result { + let Some(local_storage) = self.local_storage.as_ref() else { + return Ok(MarketplaceListOutcome::default()); + }; let mut outcome = list_marketplaces_with_home(roots, home_dir().as_deref())?; let policy = MarketplacePolicy::from_requirements(config.config_layer_stack.requirements()); if !policy.is_restricted() { @@ -2583,7 +2743,7 @@ impl PluginsManager { } let allowed_marketplace_names = allowed_configured_marketplace_names( &config.config_layer_stack, - self.codex_home.as_path(), + local_storage.codex_home.as_path(), ); outcome.marketplaces.retain(|marketplace| { is_openai_curated_marketplace_name(&marketplace.name) diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 5e743416f2..e37a9760b0 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -135,6 +135,54 @@ fn plugins_manager_tracks_auth_mode() { assert_eq!(manager_with_auth.auth_mode(), Some(AuthMode::Chatgpt)); } +#[tokio::test] +async fn manager_without_host_filesystem_ignores_configured_local_plugins() { + let codex_home = TempDir::new().expect("create Codex home"); + write_auth_projection_plugin(codex_home.path(), "sample", /*include_app*/ true); + let config = auth_projection_config(codex_home.path()).await; + let manager = PluginsManager::new_without_host_filesystem_with_options( + Some(Product::Codex), + Some(AuthMode::Chatgpt), + ); + + let outcome = manager.plugins_for_config(&config).await; + + assert_eq!(outcome, PluginLoadOutcome::default()); + assert!(manager.plugin_skill_snapshots_for_config(&config).is_none()); +} + +#[tokio::test] +async fn manager_without_host_filesystem_rejects_local_mutations() { + let root = TempDir::new().expect("create root"); + let missing_marketplace = root.path().join("marketplace.json").abs(); + let manager = PluginsManager::new_without_host_filesystem(); + + let install_error = manager + .install_plugin( + &unrestricted_config_layer_stack(), + PluginInstallRequest { + plugin_name: "sample".to_string(), + marketplace_path: missing_marketplace.clone(), + }, + ) + .await + .expect_err("install should require host filesystem access"); + let uninstall_error = manager + .uninstall_plugin("sample@test".to_string()) + .await + .expect_err("uninstall should require host filesystem access"); + + assert!(matches!( + install_error, + PluginInstallError::Store(PluginStoreError::Invalid(_)) + )); + assert!(matches!( + uninstall_error, + PluginUninstallError::Store(PluginStoreError::Invalid(_)) + )); + assert!(!missing_marketplace.exists()); +} + #[tokio::test] async fn marketplace_policy_projection_disables_installed_plugin_and_invalidates_cache() { let codex_home = TempDir::new().expect("create Codex home"); diff --git a/codex-rs/core-skills/src/service.rs b/codex-rs/core-skills/src/service.rs index b4e0d7396c..cb45699f1c 100644 --- a/codex-rs/core-skills/src/service.rs +++ b/codex-rs/core-skills/src/service.rs @@ -5,6 +5,7 @@ use std::sync::RwLock; use codex_config::ConfigLayerStack; use codex_exec_server::ExecutorFileSystem; +use codex_exec_server::LOCAL_FS; use codex_protocol::protocol::Product; use codex_protocol::protocol::SkillScope; use codex_utils_absolute_path::AbsolutePathBuf; @@ -66,7 +67,7 @@ impl SkillsLoadInput { /// /// Source-specific model exposure remains the responsibility of the skills extension. pub struct SkillsService { - codex_home: AbsolutePathBuf, + codex_home: Option, restriction_product: Option, extra_roots: RwLock>, cache_by_cwd: RwLock>, @@ -82,6 +83,36 @@ impl SkillsService { codex_home: AbsolutePathBuf, bundled_skills_enabled: bool, restriction_product: Option, + ) -> Self { + Self::new_inner( + Some(codex_home), + bundled_skills_enabled, + restriction_product, + ) + } + + /// Creates a service that does not discover or mutate host-local skills. + /// + /// Project skills exposed through a non-local executor filesystem remain available. + pub fn new_without_host_filesystem() -> Self { + Self::new_without_host_filesystem_with_restriction_product(Some(Product::Codex)) + } + + /// Creates a host-filesystem-free service with an explicit product restriction. + pub fn new_without_host_filesystem_with_restriction_product( + restriction_product: Option, + ) -> Self { + Self::new_inner( + /*codex_home*/ None, + /*bundled_skills_enabled*/ false, + restriction_product, + ) + } + + fn new_inner( + codex_home: Option, + bundled_skills_enabled: bool, + restriction_product: Option, ) -> Self { let service = Self { codex_home, @@ -90,11 +121,14 @@ impl SkillsService { cache_by_cwd: RwLock::new(HashMap::new()), cache_by_config: RwLock::new(HashMap::new()), }; - if !bundled_skills_enabled { + if !bundled_skills_enabled && let Some(codex_home) = service.codex_home.as_ref() { // The loader caches bundled skills under `skills/.system`. Clearing that directory is // best-effort cleanup; root selection still enforces the config even if removal fails. - uninstall_system_skills(&service.codex_home); - } else if let Err(err) = install_system_skills(&service.codex_home) { + uninstall_system_skills(codex_home); + } else if bundled_skills_enabled + && let Some(codex_home) = service.codex_home.as_ref() + && let Err(err) = install_system_skills(codex_home) + { tracing::error!("failed to install system skills: {err}"); } service @@ -130,8 +164,14 @@ impl SkillsService { ) -> HostSkillsSnapshot { let roots = self.skill_roots_for_config(input, fs).await; let skill_config_rules = skill_config_rules_from_stack(&input.config_layer_stack); - let cache_key = config_skills_cache_key(&roots, &skill_config_rules); - if let Some(snapshot) = self.cached_snapshot_for_config(&cache_key) { + let cache_key = self + .codex_home + .is_some() + .then(|| config_skills_cache_key(&roots, &skill_config_rules)); + if let Some(snapshot) = cache_key + .as_ref() + .and_then(|cache_key| self.cached_snapshot_for_config(cache_key)) + { return snapshot; } @@ -139,11 +179,13 @@ impl SkillsService { self.build_skill_outcome(input, roots, &skill_config_rules) .await, )); - let mut cache = self - .cache_by_config - .write() - .unwrap_or_else(std::sync::PoisonError::into_inner); - cache.insert(cache_key, snapshot.clone()); + if let Some(cache_key) = cache_key { + let mut cache = self + .cache_by_config + .write() + .unwrap_or_else(std::sync::PoisonError::into_inner); + cache.insert(cache_key, snapshot.clone()); + } snapshot } @@ -160,6 +202,7 @@ impl SkillsService { self.extra_roots(), ) .await; + self.remove_host_filesystem_roots(&mut roots); if !input.bundled_skills_enabled { roots.retain(|root| root.scope != SkillScope::System); } @@ -172,7 +215,9 @@ impl SkillsService { force_reload: bool, fs: Option>, ) -> HostSkillsSnapshot { - let use_cwd_cache = fs.is_some(); + // Executor paths are not globally unique: separate remote environments commonly expose + // the same `/workspace`. Without a host runtime, caching by path could cross sessions. + let use_cwd_cache = self.codex_home.is_some() && fs.is_some(); if use_cwd_cache && !force_reload && let Some(snapshot) = self.cached_snapshot_for_cwd(&input.cwd) @@ -188,6 +233,7 @@ impl SkillsService { self.extra_roots(), ) .await; + self.remove_host_filesystem_roots(&mut roots); if !bundled_skills_enabled_from_stack(&input.config_layer_stack) { roots.retain(|root| root.scope != SkillScope::System); } @@ -266,6 +312,15 @@ impl SkillsService { Err(err) => err.into_inner().clone(), } } + + fn remove_host_filesystem_roots(&self, roots: &mut Vec) { + if self.codex_home.is_none() { + // The loader assigns this canonical filesystem to every user, system, plugin, and + // extra root it derives from the host. Executor-backed project roots use the caller's + // filesystem and remain available to remote-only runtimes. + roots.retain(|root| !Arc::ptr_eq(&root.file_system, &LOCAL_FS)); + } + } } #[derive(Debug, Clone, PartialEq, Eq, Hash)] diff --git a/codex-rs/core-skills/src/service_tests.rs b/codex-rs/core-skills/src/service_tests.rs index 6026975097..ddcb7f4066 100644 --- a/codex-rs/core-skills/src/service_tests.rs +++ b/codex-rs/core-skills/src/service_tests.rs @@ -7,7 +7,9 @@ use codex_config::ConfigLayerEntry; use codex_config::ConfigLayerSource; use codex_config::ConfigLayerStack; use codex_config::ConfigRequirementsToml; +use codex_exec_server::ExecutorFileSystem; use codex_exec_server::LOCAL_FS; +use codex_exec_server::LocalFileSystem; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::test_support::PathBufExt; use codex_utils_absolute_path::test_support::PathExt; @@ -201,6 +203,118 @@ fn new_with_disabled_bundled_skills_removes_stale_cached_system_skills() { ); } +#[tokio::test] +async fn service_without_host_filesystem_ignores_local_skill_roots() { + let codex_home = tempfile::tempdir().expect("tempdir"); + let cwd = tempfile::tempdir().expect("tempdir"); + let extra_root = tempfile::tempdir().expect("tempdir"); + write_user_skill(&codex_home, "user", "user-skill", "user skill"); + let plugin_skill_path = write_plugin_skill( + &codex_home, + "test", + "sample", + "plugin", + "plugin-skill", + "plugin skill", + ); + let extra_skills_root = extra_root.path().join("skills"); + let extra_skill_dir = extra_skills_root.join("extra"); + fs::create_dir_all(&extra_skill_dir).expect("create extra skill dir"); + fs::write( + extra_skill_dir.join("SKILL.md"), + "---\nname: extra-skill\ndescription: extra skill\n---\n", + ) + .expect("write extra skill"); + let config_layer_stack = config_stack(&codex_home, ""); + let service = SkillsService::new_without_host_filesystem(); + service.set_extra_roots(vec![extra_skills_root.abs()]); + let input = SkillsLoadInput::new( + cwd.path().abs(), + vec![plugin_skill_root_for_skill_path( + &plugin_skill_path, + "sample@test", + "sample", + )], + config_layer_stack, + /*bundled_skills_enabled*/ true, + ); + + let outcome = service + .snapshot_for_config(&input, Some(Arc::clone(&LOCAL_FS))) + .await; + + assert!(outcome.outcome().skills.is_empty()); + assert!(outcome.outcome().errors.is_empty()); + assert!(plugin_skill_path.exists()); + assert!(extra_skill_dir.join("SKILL.md").exists()); +} + +#[tokio::test] +async fn service_without_host_filesystem_keeps_executor_backed_project_roots() { + let codex_home = tempfile::tempdir().expect("tempdir"); + let cwd = tempfile::tempdir().expect("tempdir"); + let repo_dot_codex = cwd.path().join(".codex"); + let repo_skill_dir = repo_dot_codex.join("skills/repo"); + fs::create_dir_all(&repo_skill_dir).expect("create repo skill dir"); + fs::write( + repo_skill_dir.join("SKILL.md"), + "---\nname: repo-skill\ndescription: executor-backed skill\n---\n", + ) + .expect("write repo skill"); + write_user_skill(&codex_home, "user", "user-skill", "host-local skill"); + let config_layer_stack = ConfigLayerStack::new( + vec![ + user_config_layer(&codex_home, ""), + ConfigLayerEntry::new( + ConfigLayerSource::Project { + dot_codex_folder: repo_dot_codex.abs(), + }, + toml::Value::Table(toml::map::Map::new()), + ), + ], + Default::default(), + ConfigRequirementsToml::default(), + ) + .expect("valid config layer stack"); + let input = SkillsLoadInput::new( + cwd.path().abs(), + Vec::new(), + config_layer_stack, + /*bundled_skills_enabled*/ true, + ); + let service = SkillsService::new_without_host_filesystem(); + // A distinct filesystem instance models an executor-owned namespace. LOCAL_FS roots must be + // removed without discarding the project roots assigned to this executor. + let executor_fs: Arc = Arc::new(LocalFileSystem::unsandboxed()); + + let outcome = service + .snapshot_for_config(&input, Some(Arc::clone(&executor_fs))) + .await; + let names = outcome + .outcome() + .skills + .iter() + .map(|skill| skill.name.as_str()) + .collect::>(); + + assert!(names.contains("repo-skill")); + assert!(!names.contains("user-skill")); + + fs::write( + repo_skill_dir.join("SKILL.md"), + "---\nname: repo-skill\ndescription: updated executor skill\n---\n", + ) + .expect("update repo skill"); + let refreshed = service.snapshot_for_config(&input, Some(executor_fs)).await; + let repo_skill = refreshed + .outcome() + .skills + .iter() + .find(|skill| skill.name == "repo-skill") + .expect("repo skill should remain visible"); + assert_eq!(repo_skill.description, "updated executor skill"); +} + #[tokio::test] async fn skills_for_config_reuses_cache_for_same_effective_config() { let codex_home = tempfile::tempdir().expect("tempdir"); diff --git a/codex-rs/core/src/agent/control_tests.rs b/codex-rs/core/src/agent/control_tests.rs index 355f45d9af..10aabc7c26 100644 --- a/codex-rs/core/src/agent/control_tests.rs +++ b/codex-rs/core/src/agent/control_tests.rs @@ -2395,7 +2395,7 @@ async fn resume_thread_subagent_restores_stored_metadata() { /*analytics_events_client*/ None, thread_store.clone(), /*agent_graph_store*/ None, - uuid::Uuid::new_v4().to_string(), + Some(uuid::Uuid::new_v4().to_string()), /*attestation_provider*/ None, /*external_time_provider*/ None, ); diff --git a/codex-rs/core/src/agent/role.rs b/codex-rs/core/src/agent/role.rs index b34aa8aa21..40cc6d123e 100644 --- a/codex-rs/core/src/agent/role.rs +++ b/codex-rs/core/src/agent/role.rs @@ -53,6 +53,23 @@ pub(crate) async fn apply_role_to_config( }) } +/// Applies only roles whose complete definition is already in memory. +/// +/// Remote-only sessions use this path so selecting an agent role cannot read a role file or +/// rebuild config through the host filesystem. +pub(crate) async fn apply_role_to_config_without_host_filesystem( + config: &mut Config, + role_name: Option<&str>, +) -> Result<(), String> { + let role_name = role_name.unwrap_or(DEFAULT_ROLE_NAME); + let role = resolve_role_config(config, role_name) + .ok_or_else(|| format!("unknown agent_type '{role_name}'"))?; + if role.config_file.is_some() { + return Err(AGENT_TYPE_UNAVAILABLE_ERROR.to_string()); + } + Ok(()) +} + async fn apply_role_to_config_inner( config: &mut Config, role_name: &str, diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 251b7c1dcf..d97d2f1376 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -596,7 +596,9 @@ impl ModelClient { }; let mut extra_headers = ApiHeaderMap::new(); - if let Ok(header_value) = HeaderValue::from_str(&responses_metadata.installation_id) { + if let Some(installation_id) = &responses_metadata.installation_id + && let Ok(header_value) = HeaderValue::from_str(installation_id) + { extra_headers.insert(X_CODEX_INSTALLATION_ID_HEADER, header_value); } extra_headers.extend(build_responses_headers( diff --git a/codex-rs/core/src/client_tests.rs b/codex-rs/core/src/client_tests.rs index ddf9c4ebec..bd037a2647 100644 --- a/codex-rs/core/src/client_tests.rs +++ b/codex-rs/core/src/client_tests.rs @@ -112,7 +112,7 @@ fn test_model_client_with_thread_id( } #[tokio::test] -async fn compact_uses_bearer_after_agent_identity_session_fallback() -> anyhow::Result<()> { +async fn compact_without_installation_id_omits_header_after_auth_fallback() -> anyhow::Result<()> { let server = MockServer::start().await; let registration_count = Arc::new(AtomicUsize::new(0)); let response_count = Arc::clone(®istration_count); @@ -170,13 +170,14 @@ async fn compact_uses_bearer_after_agent_identity_session_fallback() -> anyhow:: }, ..Default::default() }; - let responses_metadata = test_responses_metadata_for_client( + let mut responses_metadata = test_responses_metadata_for_client( &client, /*turn_id*/ None, format!("{}:0", client.state.thread_id), /*parent_thread_id*/ None, TestCodexResponsesRequestKind::Turn, ); + responses_metadata.installation_id = None; let output = client .compact_conversation_history( @@ -218,6 +219,11 @@ async fn compact_uses_bearer_after_agent_identity_session_fallback() -> anyhow:: .and_then(|value| value.to_str().ok()), Some("account-123") ); + assert!( + !compact_request + .headers + .contains_key(X_CODEX_INSTALLATION_ID_HEADER) + ); Ok(()) } @@ -555,6 +561,30 @@ fn build_ws_client_metadata_includes_window_lineage_and_turn_metadata() { ); } +#[test] +fn build_ws_client_metadata_omits_absent_installation_id() { + let client = test_model_client(SessionSource::Exec); + let mut responses_metadata = test_responses_metadata_for_client( + &client, + Some("turn-123"), + format!("{}:0", client.state.thread_id), + /*parent_thread_id*/ None, + TestCodexResponsesRequestKind::Turn, + ); + responses_metadata.installation_id = None; + + let client_metadata = + client.build_ws_client_metadata(&responses_metadata, /*use_responses_lite*/ false); + assert!(!client_metadata.contains_key(X_CODEX_INSTALLATION_ID_HEADER)); + let turn_metadata: serde_json::Value = serde_json::from_str( + client_metadata + .get(X_CODEX_TURN_METADATA_HEADER) + .expect("turn metadata"), + ) + .expect("valid turn metadata"); + assert!(turn_metadata.get("installation_id").is_none()); +} + #[tokio::test] async fn summarize_memories_returns_empty_for_empty_input() { let client = test_model_client(SessionSource::Cli); diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index d2911df407..10300a060e 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -91,8 +91,10 @@ pub(crate) async fn run_codex_thread_interactive( instructions: parent_session.user_instructions().await, warnings: Vec::new(), }; + let local_runtime_paths = parent_session.local_runtime_paths().await; let CodexSpawnOk { codex, .. } = Box::pin(Codex::spawn(CodexSpawnArgs { config, + local_runtime_paths, allow_provider_model_fallback: false, user_instructions, installation_id: parent_session.installation_id.clone(), diff --git a/codex-rs/core/src/codex_thread.rs b/codex-rs/core/src/codex_thread.rs index 257a32733b..e3f5feb9f7 100644 --- a/codex-rs/core/src/codex_thread.rs +++ b/codex-rs/core/src/codex_thread.rs @@ -208,6 +208,11 @@ impl CodexThread { self.codex.session.services.session_telemetry.clone() } + /// Returns the caller-supplied installation identity, if this session has one. + pub fn installation_id(&self) -> Option<&str> { + self.codex.session.installation_id.as_deref() + } + pub async fn shutdown_and_wait(&self) -> CodexResult<()> { self.codex.shutdown_and_wait().await } diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 5cd6748c05..1c62c1d96f 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -1587,7 +1587,7 @@ impl Config { McpConfig { chatgpt_base_url: self.chatgpt_base_url.clone(), apps_mcp_product_sku: self.apps_mcp_product_sku.clone(), - codex_home: self.codex_home.to_path_buf(), + codex_home: Some(self.codex_home.to_path_buf()), mcp_oauth_credentials_store_mode: self.mcp_oauth_credentials_store_mode, auth_keyring_backend_kind: self.auth_keyring_backend_kind(), mcp_oauth_callback_port: self.mcp_oauth_callback_port, diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index a35b717cad..4dff7e1dca 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -264,7 +264,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_mcp_manager( // Connector discovery is threadless. Use an actually configured env if // one exists, but do not reintroduce the old hidden-local fallback. runtime_context, - config.codex_home.to_path_buf(), + Some(config.codex_home.to_path_buf()), mcp_manager.codex_apps_tools_cache(), codex_apps_tools_cache_key(auth.as_ref()), mcp_config.prefix_mcp_tool_names, diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 8f31f150b3..a7648d35a9 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -218,6 +218,9 @@ pub enum ExecPolicyError { #[derive(Debug, Error)] pub enum ExecPolicyUpdateError { + #[error("local runtime paths are required to persist an execpolicy amendment")] + LocalRuntimePathsUnavailable, + #[error("failed to update rules file {path}: {source}")] AppendRule { path: PathBuf, source: AmendError }, diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index cfc26c5711..29a601da57 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -53,6 +53,7 @@ mod hook_runtime; mod image_preparation; mod installation_id; pub(crate) mod landlock; +mod local_runtime_paths; pub use landlock::spawn_command_under_linux_sandbox; pub(crate) mod mcp; mod mcp_skill_dependencies; @@ -114,6 +115,9 @@ pub use codex_prompts as review_prompts; mod thread_manager; pub(crate) mod web_search; pub(crate) mod windows_sandbox_read_grants; +pub use local_runtime_paths::LocalRuntimePaths; +pub use local_runtime_paths::ThreadManagerInitError; +pub use local_runtime_paths::ThreadStartError; pub use thread_manager::ForkSnapshot; pub use thread_manager::NewThread; pub use thread_manager::StartThreadOptions; diff --git a/codex-rs/core/src/local_runtime_paths.rs b/codex-rs/core/src/local_runtime_paths.rs new file mode 100644 index 0000000000..7455f65c70 --- /dev/null +++ b/codex-rs/core/src/local_runtime_paths.rs @@ -0,0 +1,110 @@ +use crate::config::Config; +use codex_exec_server::EnvironmentManager; +use codex_protocol::error::CodexErr; +use codex_protocol::protocol::TurnEnvironmentSelection; +use codex_utils_absolute_path::AbsolutePathBuf; + +/// Host-local paths available to a [`crate::ThreadManager`]. +/// +/// The capability is all-or-none: remote-only managers receive `None` instead +/// of placeholder paths that could accidentally target a shared host. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct LocalRuntimePaths { + /// Stable host-local state root owned by the manager. + pub codex_home: AbsolutePathBuf, + /// Initial local workspace. Each local thread refreshes this from its + /// effective [`Config`] so per-thread cwd overrides keep working. + pub cwd: AbsolutePathBuf, +} + +impl From<&Config> for LocalRuntimePaths { + fn from(config: &Config) -> Self { + Self { + codex_home: config.codex_home.clone(), + cwd: config.cwd.clone(), + } + } +} + +/// Failure to construct a thread manager with a coherent runtime capability. +#[derive(Clone, Copy, Debug, PartialEq, Eq, thiserror::Error)] +pub enum ThreadManagerInitError { + #[error("the configured thread store requires local runtime paths")] + ThreadStoreRequiresLocalRuntimePaths, + #[error("the configured agent graph store requires local runtime paths")] + AgentGraphStoreRequiresLocalRuntimePaths, +} + +/// Invalid environment configuration for a remote-only thread. +#[derive(Clone, Debug, PartialEq, Eq, thiserror::Error)] +pub enum ThreadStartError { + #[error("a thread without local runtime paths requires at least one remote environment")] + MissingRemoteEnvironment, + #[error("unknown turn environment id `{environment_id}`")] + UnknownEnvironment { environment_id: String }, + #[error( + "a thread without local runtime paths cannot select local environment `{environment_id}`" + )] + LocalEnvironmentNotAllowed { environment_id: String }, + #[error( + "remote environment `{environment_id}` cwd cannot be used as the thread's legacy cwd: {message}" + )] + InvalidRemoteCwd { + environment_id: String, + message: String, + }, + #[error( + "remote-only thread legacy cwd `{actual}` must match the first remote environment cwd `{expected}`" + )] + LegacyCwdMismatch { + expected: AbsolutePathBuf, + actual: AbsolutePathBuf, + }, + #[error("a thread without local runtime paths cannot use command-backed model auth")] + CommandModelAuthNotAllowed, +} + +impl From for CodexErr { + fn from(error: ThreadStartError) -> Self { + Self::InvalidRequest(error.to_string()) + } +} + +pub(crate) fn validate_remote_environment_selections( + environment_manager: &EnvironmentManager, + selections: &[TurnEnvironmentSelection], +) -> Result<(), ThreadStartError> { + if selections.is_empty() { + return Err(ThreadStartError::MissingRemoteEnvironment); + } + + for selection in selections { + let environment = environment_manager + .get_environment(&selection.environment_id) + .ok_or_else(|| ThreadStartError::UnknownEnvironment { + environment_id: selection.environment_id.clone(), + })?; + if !environment.is_remote() { + return Err(ThreadStartError::LocalEnvironmentNotAllowed { + environment_id: selection.environment_id.clone(), + }); + } + } + + Ok(()) +} + +pub(crate) fn remote_legacy_cwd( + environment_manager: &EnvironmentManager, + selections: &[TurnEnvironmentSelection], +) -> Result { + validate_remote_environment_selections(environment_manager, selections)?; + let selection = &selections[0]; + selection + .cwd + .to_abs_path() + .map_err(|error| ThreadStartError::InvalidRemoteCwd { + environment_id: selection.environment_id.clone(), + message: error.to_string(), + }) +} diff --git a/codex-rs/core/src/mcp_skill_dependencies.rs b/codex-rs/core/src/mcp_skill_dependencies.rs index 61d5c54a59..475a931adc 100644 --- a/codex-rs/core/src/mcp_skill_dependencies.rs +++ b/codex-rs/core/src/mcp_skill_dependencies.rs @@ -52,6 +52,9 @@ pub(crate) async fn maybe_prompt_and_install_mcp_dependencies( { return; } + if sess.local_runtime_paths().await.is_none() { + return; + } let installed = sess.runtime_mcp_servers(config.as_ref()).await; let missing = collect_missing_mcp_dependencies(mentioned_skills, &installed); @@ -93,7 +96,12 @@ pub(crate) async fn maybe_install_mcp_dependencies( return; } - let codex_home = config.codex_home.clone(); + let Some(local_runtime_paths) = sess.local_runtime_paths().await else { + // Installing a dependency mutates config and may persist OAuth credentials. A remote-only + // session has no host-local namespace in which either operation is safe. + return; + }; + let codex_home = local_runtime_paths.codex_home; let installed = sess.runtime_mcp_servers(config).await; let missing = collect_missing_mcp_dependencies(mentioned_skills, &installed); if missing.is_empty() { @@ -464,3 +472,67 @@ fn collect_missing_mcp_dependencies( missing } + +#[cfg(test)] +mod tests { + use super::*; + use crate::session::tests::make_session_and_context_without_local_runtime_paths; + use crate::skills::model::SkillDependencies; + use codex_config::CONFIG_TOML_FILE; + use codex_features::Feature; + use codex_protocol::protocol::SkillScope; + + #[tokio::test] + async fn pathless_dependency_install_does_not_write_config_or_oauth_state() { + let (session, turn_context) = make_session_and_context_without_local_runtime_paths().await; + let mut config = (*turn_context.config).clone(); + config + .features + .enable(Feature::SkillMcpDependencyInstall) + .expect("test config should allow skill dependency installation"); + std::fs::create_dir_all(&config.codex_home).expect("create sentinel config directory"); + let config_path = config.codex_home.join(CONFIG_TOML_FILE); + let original_config = "model = \"sentinel\"\n"; + std::fs::write(&config_path, original_config).expect("write sentinel config"); + let skill = SkillMetadata { + name: "remote-skill".to_string(), + description: "requires an MCP dependency".to_string(), + short_description: None, + interface: None, + dependencies: Some(SkillDependencies { + tools: vec![SkillToolDependency { + r#type: "mcp".to_string(), + value: "remote-docs".to_string(), + description: None, + transport: Some("streamable_http".to_string()), + command: None, + url: Some("https://example.com/mcp".to_string()), + }], + }), + policy: None, + path_to_skills_md: config.codex_home.join("skills/remote/SKILL.md"), + scope: SkillScope::Repo, + plugin_id: None, + }; + + maybe_install_mcp_dependencies( + &session, + &turn_context, + &config, + &[skill], + /*elicitation_reviewer*/ None, + ) + .await; + + assert_eq!( + std::fs::read_to_string(&config_path).expect("read sentinel config"), + original_config + ); + let mut entries = std::fs::read_dir(&config.codex_home) + .expect("read sentinel config directory") + .map(|entry| entry.expect("read directory entry").file_name()) + .collect::>(); + entries.sort(); + assert_eq!(entries, vec![std::ffi::OsString::from(CONFIG_TOML_FILE)]); + } +} diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index d268ea4b34..6dda1bfdc4 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -1227,7 +1227,9 @@ async fn maybe_request_mcp_tool_approval( } let session_approval_key = session_mcp_tool_approval_key(invocation, metadata, approval_mode); - let persistent_approval_key = if manager.is_selected_plugin_mcp_server(&invocation.server) { + let persistent_approval_key = if sess.local_runtime_paths().await.is_none() + || manager.is_selected_plugin_mcp_server(&invocation.server) + { None } else { persistent_mcp_tool_approval_key(invocation, metadata, approval_mode) @@ -1979,6 +1981,14 @@ async fn maybe_persist_mcp_tool_approval( turn_context: &TurnContext, key: McpToolApprovalKey, ) { + if sess.local_runtime_paths().await.is_none() { + // Clients may return an old or forged "always" response even when the current prompt does + // not offer persistence. Preserve the approval for this session without touching host + // config or project files. + remember_mcp_tool_approval(sess, key).await; + return; + } + let tool_name = key.tool_name.clone(); let persist_result = if key.server == CODEX_APPS_MCP_SERVER_NAME { diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index 8c76c60334..74b0b6b6df 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -4,6 +4,7 @@ use crate::config::ManagedFeatures; use crate::session::step_context::StepContext; use crate::session::tests::make_session_and_context; use crate::session::tests::make_session_and_context_with_rx; +use crate::session::tests::make_session_and_context_without_local_runtime_paths; use crate::session::turn_context::TurnEnvironment; use crate::state::ActiveTurn; use crate::test_support::models_manager_with_provider; @@ -1452,7 +1453,7 @@ async fn host_owned_codex_apps_manager( turn_context.cwd.to_path_buf() }, ), - turn_context.config.codex_home.to_path_buf(), + Some(turn_context.config.codex_home.to_path_buf()), session.services.mcp_manager.codex_apps_tools_cache(), codex_mcp::codex_apps_tools_cache_key(auth.as_ref()), turn_context.config.prefix_mcp_tool_names(), @@ -2494,6 +2495,65 @@ async fn maybe_persist_mcp_tool_approval_writes_project_config_for_project_serve assert_eq!(mcp_tool_approval_is_remembered(&session, &key).await, true); } +#[tokio::test] +async fn pathless_accept_and_remember_is_session_only() { + let (session, mut turn_context) = make_session_and_context_without_local_runtime_paths().await; + let codex_home = turn_context.config.codex_home.clone(); + let project_dir = tempdir().expect("tempdir"); + std::fs::write(project_dir.path().join(".git"), "gitdir: nowhere").expect("seed git marker"); + let project_codex_dir = project_dir.path().join(".codex"); + std::fs::create_dir_all(&project_codex_dir).expect("create project .codex dir"); + let project_config_path = project_codex_dir.join(CONFIG_TOML_FILE); + std::fs::write( + &project_config_path, + "[mcp_servers.docs]\ncommand = \"docs-server\"\n", + ) + .expect("seed project config"); + ConfigEditsBuilder::new(&codex_home) + .set_project_trust_level( + project_dir.path(), + codex_protocol::config_types::TrustLevel::Trusted, + ) + .apply() + .await + .expect("trust project"); + let config = ConfigBuilder::default() + .codex_home(codex_home.to_path_buf()) + .fallback_cwd(Some(project_dir.path().to_path_buf())) + .build() + .await + .expect("load project config"); + turn_context.config = Arc::new(config); + let user_config_path = codex_home.join(CONFIG_TOML_FILE); + let user_config_before = std::fs::read_to_string(&user_config_path).expect("read user config"); + let project_config_before = + std::fs::read_to_string(&project_config_path).expect("read project config"); + let key = McpToolApprovalKey { + server: "docs".to_string(), + connector_id: None, + tool_name: "search".to_string(), + }; + + apply_mcp_tool_approval_decision( + &session, + &turn_context, + &McpToolApprovalDecision::AcceptAndRemember, + Some(key.clone()), + Some(key.clone()), + ) + .await; + + assert_eq!( + std::fs::read_to_string(&user_config_path).expect("read user config"), + user_config_before + ); + assert_eq!( + std::fs::read_to_string(&project_config_path).expect("read project config"), + project_config_before + ); + assert!(mcp_tool_approval_is_remembered(&session, &key).await); +} + #[tokio::test] async fn approve_mode_skips_when_annotations_do_not_require_approval() { let (session, turn_context) = make_session_and_context().await; diff --git a/codex-rs/core/src/prompt_debug.rs b/codex-rs/core/src/prompt_debug.rs index cac9bb41ae..f633e6af01 100644 --- a/codex-rs/core/src/prompt_debug.rs +++ b/codex-rs/core/src/prompt_debug.rs @@ -58,7 +58,7 @@ pub async fn build_prompt_input( /*analytics_events_client*/ None, thread_store, crate::local_agent_graph_store_from_state_db(state_db.as_ref()), - installation_id, + Some(installation_id), /*attestation_provider*/ None, /*external_time_provider*/ None, ); diff --git a/codex-rs/core/src/realtime_conversation.rs b/codex-rs/core/src/realtime_conversation.rs index 3915c8190d..41886303b4 100644 --- a/codex-rs/core/src/realtime_conversation.rs +++ b/codex-rs/core/src/realtime_conversation.rs @@ -863,11 +863,12 @@ pub(crate) async fn build_realtime_session_config( let startup_context = if params.include_startup_context { match config.experimental_realtime_ws_startup_context.clone() { Some(startup_context) => startup_context, - None => { + None if sess.local_runtime_paths().await.is_some() => { build_realtime_startup_context(sess.as_ref(), REALTIME_STARTUP_CONTEXT_TOKEN_BUDGET) .await .unwrap_or_default() } + None => String::new(), } } else { String::new() diff --git a/codex-rs/core/src/responses_metadata.rs b/codex-rs/core/src/responses_metadata.rs index c76d3078cd..eda0e31f89 100644 --- a/codex-rs/core/src/responses_metadata.rs +++ b/codex-rs/core/src/responses_metadata.rs @@ -152,7 +152,7 @@ pub(crate) struct TurnMetadataWorkspace { /// truth. #[derive(Clone, Debug)] pub struct CodexResponsesMetadata { - pub(crate) installation_id: String, + pub(crate) installation_id: Option, pub(crate) session_id: String, pub(crate) thread_id: String, pub(crate) turn_id: Option, @@ -171,7 +171,7 @@ pub struct CodexResponsesMetadata { impl CodexResponsesMetadata { pub(crate) fn new( - installation_id: String, + installation_id: Option, session_id: String, thread_id: String, window_id: String, @@ -209,14 +209,16 @@ impl CodexResponsesMetadata { pub(crate) fn client_metadata(&self) -> HashMap { let mut client_metadata = HashMap::from([ - ( - X_CODEX_INSTALLATION_ID_HEADER.to_string(), - self.installation_id.clone(), - ), (SESSION_ID_KEY.to_string(), self.session_id.clone()), (THREAD_ID_KEY.to_string(), self.thread_id.clone()), (X_CODEX_WINDOW_ID_HEADER.to_string(), self.window_id.clone()), ]); + if let Some(installation_id) = &self.installation_id { + client_metadata.insert( + X_CODEX_INSTALLATION_ID_HEADER.to_string(), + installation_id.clone(), + ); + } if let Some(turn_id) = &self.turn_id { client_metadata.insert(TURN_ID_KEY.to_string(), turn_id.clone()); } @@ -278,7 +280,9 @@ impl CodexResponsesMetadata { let has_request_identity = request_kind.is_some_and(CodexResponsesRequestKind::has_turn_identity); CodexTurnMetadataPayload { - installation_id: has_request_identity.then_some(self.installation_id.as_str()), + installation_id: has_request_identity + .then_some(self.installation_id.as_deref()) + .flatten(), session_id: has_turn_identity.then_some(self.session_id.as_str()), thread_id: has_turn_identity.then_some(self.thread_id.as_str()), turn_id: has_turn_identity diff --git a/codex-rs/core/src/session/config_lock.rs b/codex-rs/core/src/session/config_lock.rs index 606d9d2be0..9ff8bd7e7f 100644 --- a/codex-rs/core/src/session/config_lock.rs +++ b/codex-rs/core/src/session/config_lock.rs @@ -49,6 +49,9 @@ pub(crate) async fn export_config_lock_if_configured( session_configuration: &SessionConfiguration, conversation_id: ThreadId, ) -> anyhow::Result<()> { + if session_configuration.codex_home().is_none() { + return Ok(()); + } let config = session_configuration.original_config_do_not_use.as_ref(); let Some(export_dir) = config.config_lock_export_dir.as_ref() else { return Ok(()); @@ -238,9 +241,31 @@ where #[cfg(test)] mod tests { use super::*; + use codex_utils_absolute_path::test_support::PathExt; use pretty_assertions::assert_eq; use std::sync::Arc; + #[tokio::test] + async fn pathless_session_does_not_export_config_lock() { + let mut session_configuration = + crate::session::tests::make_session_configuration_for_tests().await; + let export_dir = tempfile::tempdir() + .expect("tempdir") + .path() + .join("exports") + .abs(); + let mut config = (*session_configuration.original_config_do_not_use).clone(); + config.config_lock_export_dir = Some(export_dir.clone()); + session_configuration.original_config_do_not_use = Arc::new(config); + session_configuration.codex_home = None; + + export_config_lock_if_configured(&session_configuration, ThreadId::new()) + .await + .expect("pathless export should be skipped"); + + assert!(!export_dir.exists()); + } + #[tokio::test] async fn lock_contains_prompts_and_materializes_features() { let mut sc = crate::session::tests::make_session_configuration_for_tests().await; diff --git a/codex-rs/core/src/session/mcp.rs b/codex-rs/core/src/session/mcp.rs index 2c33cc041b..79aac0f105 100644 --- a/codex-rs/core/src/session/mcp.rs +++ b/codex-rs/core/src/session/mcp.rs @@ -85,7 +85,8 @@ impl Session { .await; let available_environment_ids = Self::available_selected_environment_ids(&selected_capability_roots); - self.services + let mut mcp_config = self + .services .mcp_manager .runtime_config_for_step( config, @@ -95,7 +96,16 @@ impl Session { &available_environment_ids, ) .await - .config + .config; + let codex_home = self + .state + .lock() + .await + .session_configuration + .codex_home() + .map(AbsolutePathBuf::to_path_buf); + mcp_config.apply_local_storage_policy(codex_home); + mcp_config } pub(crate) async fn runtime_mcp_servers( @@ -122,6 +132,10 @@ impl Session { return current; } + let codex_home = self + .local_runtime_paths() + .await + .map(|paths| paths.codex_home.to_path_buf()); let _guard = self.services.mcp_projection_lock.lock().await; let current = self.services.latest_mcp_runtime(); if current.available_environment_ids() == available_environment_ids { @@ -173,6 +187,7 @@ impl Session { self.refresh_mcp_servers_inner( turn_context, mcp_projection, + codex_home, environments, &available_environment_ids, Some(self.mcp_elicitation_reviewer()), @@ -322,11 +337,13 @@ impl Session { async fn refresh_mcp_servers_inner( &self, turn_context: &TurnContext, - mcp_projection: McpRuntimeProjection, + mut mcp_projection: McpRuntimeProjection, + codex_home: Option, environments: &TurnEnvironmentSnapshot, available_environment_ids: &[String], elicitation_reviewer: Option, ) -> Arc { + mcp_projection.config.apply_local_storage_policy(codex_home); let auth = self.services.auth_manager.auth().await; let McpRuntimeProjection { config: mcp_config, @@ -465,6 +482,10 @@ impl Session { return; } + let codex_home = self + .local_runtime_paths() + .await + .map(|paths| paths.codex_home.to_path_buf()); let _guard = self.services.mcp_projection_lock.lock().await; let available_environment_ids = self .services @@ -489,6 +510,7 @@ impl Session { self.refresh_mcp_servers_inner( turn_context, mcp_projection, + codex_home, &turn_context.environments, &available_environment_ids, elicitation_reviewer, @@ -534,6 +556,10 @@ impl Session { refresh_config: &Config, elicitation_reviewer: Option, ) { + let codex_home = self + .local_runtime_paths() + .await + .map(|paths| paths.codex_home.to_path_buf()); let _guard = self.services.mcp_projection_lock.lock().await; let available_environment_ids = self .services @@ -554,6 +580,7 @@ impl Session { self.refresh_mcp_servers_inner( turn_context, mcp_projection, + codex_home, &turn_context.environments, &available_environment_ids, elicitation_reviewer, diff --git a/codex-rs/core/src/session/mcp_runtime.rs b/codex-rs/core/src/session/mcp_runtime.rs index 5e7aeffcee..3f9dcfd8f4 100644 --- a/codex-rs/core/src/session/mcp_runtime.rs +++ b/codex-rs/core/src/session/mcp_runtime.rs @@ -65,7 +65,7 @@ impl McpRuntimeSnapshot { let mcp_config = McpConfig { chatgpt_base_url: config.chatgpt_base_url.clone(), apps_mcp_product_sku: config.apps_mcp_product_sku.clone(), - codex_home: config.codex_home.to_path_buf(), + codex_home: Some(config.codex_home.to_path_buf()), mcp_oauth_credentials_store_mode: config.mcp_oauth_credentials_store_mode, auth_keyring_backend_kind: config.auth_keyring_backend_kind(), mcp_oauth_callback_port: config.mcp_oauth_callback_port, diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 9339413b33..66a5d5bb20 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -37,6 +37,9 @@ use crate::default_skill_metadata_budget; use crate::environment_selection::TurnEnvironmentSnapshot; use crate::exec_policy::ExecPolicyManager; use crate::image_preparation::prepare_response_items; +use crate::local_runtime_paths::LocalRuntimePaths; +use crate::local_runtime_paths::ThreadStartError; +use crate::local_runtime_paths::remote_legacy_cwd; use crate::parse_turn_item; use crate::realtime_conversation::RealtimeConversationManager; use crate::session::step_context::StepContext; @@ -181,6 +184,7 @@ use crate::codex_thread::ThreadConfigSnapshot; use crate::compact::collect_user_messages; use crate::config::Config; use crate::config::Constrained; +use crate::config::ConstraintError; use crate::config::ConstraintResult; use crate::config::PermissionProfileSnapshot; use crate::config::PermissionProfileState; @@ -407,9 +411,10 @@ pub struct CodexSpawnOk { pub(crate) struct CodexSpawnArgs { pub(crate) config: Config, + pub(crate) local_runtime_paths: Option, pub(crate) allow_provider_model_fallback: bool, pub(crate) user_instructions: LoadedUserInstructions, - pub(crate) installation_id: String, + pub(crate) installation_id: Option, pub(crate) auth_manager: Arc, pub(crate) models_manager: SharedModelsManager, pub(crate) environment_manager: Arc, @@ -499,6 +504,7 @@ impl Codex { async fn spawn_internal(args: CodexSpawnArgs) -> CodexResult { let CodexSpawnArgs { mut config, + local_runtime_paths, allow_provider_model_fallback, user_instructions, installation_id, @@ -534,6 +540,22 @@ impl Codex { external_time_provider, inherited_multi_agent_version, } = args; + let effective_cwd = match local_runtime_paths.as_ref() { + Some(paths) => paths.cwd.clone(), + None => remote_legacy_cwd(&environment_manager, &environment_selections)?, + }; + let local_codex_home = local_runtime_paths + .as_ref() + .map(|paths| paths.codex_home.clone()); + if local_runtime_paths.is_none() { + config.cwd = effective_cwd.clone(); + if !config.workspace_roots_explicit { + config.workspace_roots = vec![effective_cwd.clone()]; + config + .permissions + .set_workspace_roots(vec![effective_cwd.clone()]); + } + } let (tx_sub, rx_sub) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY); let (tx_event, rx_event) = async_channel::unbounded(); @@ -552,6 +574,8 @@ impl Codex { Arc::new(ExecPolicyManager::default()) } else if let Some(exec_policy) = &inherited_exec_policy { Arc::clone(exec_policy) + } else if local_runtime_paths.is_none() { + Arc::new(ExecPolicyManager::default()) } else { Arc::new( ExecPolicyManager::load(&config.config_layer_stack) @@ -560,6 +584,17 @@ impl Codex { ) }; + if local_runtime_paths.is_none() && config.model_provider.auth.is_some() { + return Err(ThreadStartError::CommandModelAuthNotAllowed.into()); + } + if local_runtime_paths.is_none() { + // Custom role files are host-local capabilities. Keep inline roles, but do not retain + // declarations that could later make tool-schema construction probe the server. + config + .agent_roles + .retain(|_, role| role.config_file.is_none()); + } + let config = Arc::new(config); let refresh_strategy = if session_source.is_non_root_agent() { codex_models_manager::manager::RefreshStrategy::Offline @@ -654,12 +689,9 @@ impl Codex { approvals_reviewer: config.approvals_reviewer, permission_profile_state: session_permission_profile_state_from_config(&config)?, windows_sandbox_level: WindowsSandboxLevel::from_config(&config), - environments: TurnEnvironmentSelections::new( - config.cwd.clone(), - environment_selections, - ), + environments: TurnEnvironmentSelections::new(effective_cwd, environment_selections), workspace_roots: config.workspace_roots.clone(), - codex_home: config.codex_home.clone(), + codex_home: local_codex_home.clone(), thread_name: None, original_config_do_not_use: Arc::clone(&config), metrics_service_name, @@ -711,7 +743,7 @@ impl Codex { .await .map_err(|e| { error!("Failed to create session: {e:#}"); - map_session_init_error(&e, &config.codex_home) + map_session_init_error(&e, local_codex_home.as_deref()) })?; if let Some(message) = initial_service_tier_warning { session @@ -1167,7 +1199,11 @@ impl Session { #[cfg(test)] pub(crate) async fn codex_home(&self) -> AbsolutePathBuf { let state = self.state.lock().await; - state.session_configuration.codex_home().clone() + state + .session_configuration + .codex_home() + .cloned() + .expect("test session has local runtime paths") } pub(crate) fn subscribe_elicitation_pause_state(&self) -> watch::Receiver { @@ -1521,6 +1557,7 @@ impl Session { let notify_config_contributors = !self.services.extensions.config_contributors().is_empty(); let (previous_config, new_config, permission_profile_changed) = { let mut state = self.state.lock().await; + self.validate_environment_update(&state.session_configuration, &updates)?; let updated = match state.session_configuration.apply(&updates) { Ok(updated) => updated, Err(err) => { @@ -1559,12 +1596,61 @@ impl Session { updates: &SessionSettingsUpdate, ) -> ConstraintResult { let state = self.state.lock().await; + self.validate_environment_update(&state.session_configuration, updates)?; state .session_configuration .apply(updates) .map(|configuration| configuration.thread_config_snapshot()) } + fn validate_environment_update( + &self, + session_configuration: &SessionConfiguration, + updates: &SessionSettingsUpdate, + ) -> ConstraintResult<()> { + if session_configuration.codex_home().is_some() { + return Ok(()); + } + let Some(environments) = updates.environments.as_ref() else { + return Ok(()); + }; + let validation = remote_legacy_cwd( + self.services + .turn_environments + .environment_manager() + .as_ref(), + &environments.environments, + ) + .and_then(|expected| { + if environments.legacy_fallback_cwd == expected { + Ok(()) + } else { + Err(ThreadStartError::LegacyCwdMismatch { + expected, + actual: environments.legacy_fallback_cwd.clone(), + }) + } + }); + validation.map_err(|error| ConstraintError::InvalidValue { + field_name: "environments", + candidate: error.to_string(), + allowed: "a nonempty set of known remote environments".to_string(), + requirement_source: codex_config::RequirementSource::Unknown, + }) + } + + pub(crate) async fn local_runtime_paths(&self) -> Option { + let state = self.state.lock().await; + state + .session_configuration + .codex_home() + .cloned() + .map(|codex_home| LocalRuntimePaths { + codex_home, + cwd: state.session_configuration.cwd().clone(), + }) + } + pub(crate) async fn set_session_startup_prewarm( &self, startup_prewarm: SessionStartupPrewarmHandle, @@ -1660,6 +1746,9 @@ impl Session { } pub(crate) async fn reload_user_config_layer(&self) { + if self.local_runtime_paths().await.is_none() { + return; + } // Refresh layer-backed runtime state for an existing session, including enabled plugin, // skill, and hook state. Derived config fields such as feature gates and legacy notify // settings remain session-static. @@ -1682,12 +1771,10 @@ impl Session { }) .collect::>(); if user_config_paths.is_empty() { - vec![ - state - .session_configuration - .codex_home - .join(CONFIG_TOML_FILE), - ] + let Some(codex_home) = state.session_configuration.codex_home() else { + return; + }; + vec![codex_home.join(CONFIG_TOML_FILE)] } else { user_config_paths } @@ -2026,7 +2113,8 @@ impl Session { .await .session_configuration .codex_home() - .clone(); + .cloned() + .ok_or(ExecPolicyUpdateError::LocalRuntimePathsUnavailable)?; self.services .exec_policy @@ -2090,7 +2178,12 @@ impl Session { .await .session_configuration .codex_home() - .clone(); + .cloned() + .ok_or_else(|| { + anyhow::anyhow!( + "local runtime paths are required to persist a network policy amendment" + ) + })?; let execpolicy_amendment = execpolicy_network_rule_amendment(amendment, network_approval_context, &host); diff --git a/codex-rs/core/src/session/review.rs b/codex-rs/core/src/session/review.rs index 6b68447b92..85be7278bc 100644 --- a/codex-rs/core/src/session/review.rs +++ b/codex-rs/core/src/session/review.rs @@ -78,11 +78,12 @@ pub(super) async fn spawn_review_thread( .model_reasoning_summary .unwrap_or(model_info.default_reasoning_summary); let session_source = parent_turn_context.session_source.clone(); - let (forked_from_thread_id, thread_source) = { + let (forked_from_thread_id, thread_source, allow_host_git_enrichment) = { let state = sess.state.lock().await; ( state.session_configuration.forked_from_thread_id, state.session_configuration.thread_source.clone(), + state.session_configuration.codex_home().is_some(), ) }; @@ -98,6 +99,7 @@ pub(super) async fn spawn_review_thread( review_turn_id.clone(), #[allow(deprecated)] parent_turn_context.cwd.clone(), + allow_host_git_enrichment, &parent_turn_context.permission_profile, parent_turn_context.windows_sandbox_level, parent_turn_context.network.is_some(), diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 7c59ec22b7..41dbd53640 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -27,7 +27,7 @@ use tokio::sync::Semaphore; /// A session has at most 1 running task at a time, and can be interrupted by user input. pub(crate) struct Session { pub(crate) thread_id: ThreadId, - pub(crate) installation_id: String, + pub(crate) installation_id: Option, pub(super) tx_event: Sender, pub(super) agent_status: watch::Sender, pub(super) state: Mutex, @@ -83,8 +83,8 @@ pub(crate) struct SessionConfiguration { /// Thread-scoped runtime workspace roots for materializing symbolic /// workspace permissions at session runtime. pub(super) workspace_roots: Vec, - /// Directory containing all Codex state for this session. - pub(super) codex_home: AbsolutePathBuf, + /// Host-local Codex state for this session. Remote-only sessions omit it. + pub(super) codex_home: Option, /// Optional user-facing name for the thread, updated during the session. pub(super) thread_name: Option, @@ -119,8 +119,8 @@ impl SessionConfiguration { &self.environments.environments } - pub(crate) fn codex_home(&self) -> &AbsolutePathBuf { - &self.codex_home + pub(crate) fn codex_home(&self) -> Option<&AbsolutePathBuf> { + self.codex_home.as_ref() } pub(super) fn permission_profile_state(&self) -> &PermissionProfileState { @@ -481,7 +481,7 @@ impl Session { mut session_configuration: SessionConfiguration, config: Arc, user_instructions: Option, - installation_id: String, + installation_id: Option, auth_manager: Arc, models_manager: SharedModelsManager, exec_policy: Arc, @@ -682,9 +682,12 @@ impl Session { let mcp_runtime_context = McpRuntimeContext::new(Arc::clone(&environment_manager), mcp_runtime_cwd); let mcp_runtime_context_for_auth = mcp_runtime_context.clone(); + let mcp_codex_home = session_configuration + .codex_home() + .map(AbsolutePathBuf::to_path_buf); let auth_and_mcp_fut = async move { let auth = auth_manager_clone.auth().await; - let mcp_projection = mcp_manager_for_mcp + let mut mcp_projection = mcp_manager_for_mcp .runtime_config_for_step( &config_for_mcp, mcp_thread_init_for_startup, @@ -693,13 +696,16 @@ impl Session { /*available_environment_ids*/ &[], ) .await; + mcp_projection + .config + .apply_local_storage_policy(mcp_codex_home); let mcp_config = &mcp_projection.config; let mcp_servers = codex_mcp::effective_mcp_servers(mcp_config, auth.as_ref()); let tool_plugin_provenance = codex_mcp::tool_plugin_provenance(mcp_config); let auth_statuses = compute_auth_statuses( mcp_servers.iter(), - config_for_mcp.mcp_oauth_credentials_store_mode, - config_for_mcp.auth_keyring_backend_kind(), + mcp_config.mcp_oauth_credentials_store_mode, + mcp_config.auth_keyring_backend_kind, auth.as_ref(), &mcp_runtime_context_for_auth, ) @@ -786,7 +792,9 @@ impl Session { }), }); } - let config_path = config.codex_home.join(CONFIG_TOML_FILE); + let config_path = session_configuration + .codex_home() + .map(|codex_home| codex_home.join(CONFIG_TOML_FILE)); if let Some(event) = unstable_features_warning_event( config .config_layer_stack @@ -795,7 +803,9 @@ impl Session { .and_then(TomlValue::as_table), config.suppress_unstable_features_warning, &config.features, - &config_path.display().to_string(), + &config_path + .as_ref() + .map_or_else(|| "".to_string(), |path| path.display().to_string()), ) { post_session_configured_events.push(event); } @@ -843,7 +853,9 @@ impl Session { /*inc*/ 1, &[( "is_git", - if get_git_repo_root(session_configuration.cwd()).is_some() { + if session_configuration.codex_home().is_some() + && get_git_repo_root(session_configuration.cwd()).is_some() + { "true" } else { "false" @@ -887,9 +899,11 @@ impl Session { } else { shell::default_user_shell() }; - let shell_snapshot = if config.features.enabled(Feature::ShellSnapshot) { + let shell_snapshot = if config.features.enabled(Feature::ShellSnapshot) + && let Some(codex_home) = session_configuration.codex_home() + { ShellSnapshot::new( - config.codex_home.clone(), + codex_home.clone(), thread_id, session_telemetry.clone(), state_db_ctx.clone(), @@ -1001,12 +1015,16 @@ impl Session { (None, None) }; - let hooks = build_hooks_for_config( - &config, - plugins_manager.as_ref(), - resolved_environments.single_local_environment(), - ) - .await; + let hooks = if session_configuration.codex_home().is_some() { + build_hooks_for_config( + &config, + plugins_manager.as_ref(), + resolved_environments.single_local_environment(), + ) + .await + } else { + Hooks::default() + }; for warning in hooks.startup_warnings() { post_session_configured_events.push(Event { id: INITIAL_SUBMIT_ID.to_owned(), @@ -1210,8 +1228,8 @@ impl Session { .then(|| Arc::clone(&sess.services.auth_manager)); let mcp_connection_manager = McpConnectionManager::new( &mcp_servers, - config.mcp_oauth_credentials_store_mode, - config.auth_keyring_backend_kind(), + mcp_projection.config.mcp_oauth_credentials_store_mode, + mcp_projection.config.auth_keyring_backend_kind, auth_statuses, &session_configuration.approval_policy, INITIAL_SUBMIT_ID.to_owned(), @@ -1219,7 +1237,7 @@ impl Session { mcp_startup_cancellation_token, session_configuration.permission_profile(), mcp_runtime_context.clone(), - config.codex_home.to_path_buf(), + mcp_projection.config.codex_home.clone(), sess.services.mcp_manager.codex_apps_tools_cache(), codex_apps_tools_cache_key(auth), config.prefix_mcp_tool_names(), diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 696a8adbd8..7e3f883ed8 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -3747,7 +3747,7 @@ async fn set_rate_limits_retains_previous_credits() { windows_sandbox_level: WindowsSandboxLevel::from_config(&config), environments: TurnEnvironmentSelections::new(config.cwd.clone(), Vec::new()), workspace_roots: config.workspace_roots.clone(), - codex_home: config.codex_home.clone(), + codex_home: Some(config.codex_home.clone()), thread_name: None, original_config_do_not_use: Arc::clone(&config), metrics_service_name: None, @@ -3854,7 +3854,7 @@ async fn set_rate_limits_updates_plan_type_when_present() { windows_sandbox_level: WindowsSandboxLevel::from_config(&config), environments: TurnEnvironmentSelections::new(config.cwd.clone(), Vec::new()), workspace_roots: config.workspace_roots.clone(), - codex_home: config.codex_home.clone(), + codex_home: Some(config.codex_home.clone()), thread_name: None, original_config_do_not_use: Arc::clone(&config), metrics_service_name: None, @@ -4391,7 +4391,7 @@ pub(crate) async fn make_session_configuration_for_tests() -> SessionConfigurati windows_sandbox_level: WindowsSandboxLevel::from_config(&config), environments: TurnEnvironmentSelections::new(config.cwd.clone(), Vec::new()), workspace_roots: config.workspace_roots.clone(), - codex_home: config.codex_home.clone(), + codex_home: Some(config.codex_home.clone()), thread_name: None, original_config_do_not_use: Arc::clone(&config), metrics_service_name: None, @@ -5262,7 +5262,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_packaged_zsh() { windows_sandbox_level: WindowsSandboxLevel::from_config(&config), environments: TurnEnvironmentSelections::new(config.cwd.clone(), Vec::new()), workspace_roots: config.workspace_roots.clone(), - codex_home: config.codex_home.clone(), + codex_home: Some(config.codex_home.clone()), thread_name: None, original_config_do_not_use: Arc::clone(&config), metrics_service_name: None, @@ -5291,7 +5291,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_packaged_zsh() { session_configuration, Arc::clone(&config), /*user_instructions*/ None, - "11111111-1111-4111-8111-111111111111".to_string(), + Some("11111111-1111-4111-8111-111111111111".to_string()), auth_manager, models_manager, Arc::new(ExecPolicyManager::default()), @@ -5394,7 +5394,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { windows_sandbox_level: WindowsSandboxLevel::from_config(&config), environments: TurnEnvironmentSelections::new(config.cwd.clone(), default_environments), workspace_roots: config.workspace_roots.clone(), - codex_home: config.codex_home.clone(), + codex_home: Some(config.codex_home.clone()), thread_name: None, original_config_do_not_use: Arc::clone(&config), metrics_service_name: None, @@ -5569,7 +5569,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { ); let session = Session { thread_id, - installation_id: "11111111-1111-4111-8111-111111111111".to_string(), + installation_id: Some("11111111-1111-4111-8111-111111111111".to_string()), tx_event, agent_status: agent_status_tx, state: Mutex::new(state), @@ -5588,6 +5588,13 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { (session, turn_context) } +pub(crate) async fn make_session_and_context_without_local_runtime_paths() -> (Session, TurnContext) +{ + let (session, turn_context) = make_session_and_context().await; + session.state.lock().await.session_configuration.codex_home = None; + (session, turn_context) +} + async fn make_session_with_config( mutator: impl FnOnce(&mut Config), ) -> anyhow::Result> { @@ -5648,7 +5655,7 @@ async fn make_session_with_config_and_rx( windows_sandbox_level: WindowsSandboxLevel::from_config(&config), environments: TurnEnvironmentSelections::new(config.cwd.clone(), default_environments), workspace_roots: config.workspace_roots.clone(), - codex_home: config.codex_home.clone(), + codex_home: Some(config.codex_home.clone()), thread_name: None, original_config_do_not_use: Arc::clone(&config), metrics_service_name: None, @@ -5678,7 +5685,7 @@ async fn make_session_with_config_and_rx( session_configuration, Arc::clone(&config), /*user_instructions*/ None, - "11111111-1111-4111-8111-111111111111".to_string(), + Some("11111111-1111-4111-8111-111111111111".to_string()), auth_manager, models_manager, Arc::new(ExecPolicyManager::default()), @@ -5756,7 +5763,7 @@ async fn make_session_with_history_source_and_agent_control_and_rx( windows_sandbox_level: WindowsSandboxLevel::from_config(&config), environments: TurnEnvironmentSelections::new(config.cwd.clone(), default_environments), workspace_roots: config.workspace_roots.clone(), - codex_home: config.codex_home.clone(), + codex_home: Some(config.codex_home.clone()), thread_name: None, original_config_do_not_use: Arc::clone(&config), metrics_service_name: None, @@ -5786,7 +5793,7 @@ async fn make_session_with_history_source_and_agent_control_and_rx( session_configuration, Arc::clone(&config), /*user_instructions*/ None, - "11111111-1111-4111-8111-111111111111".to_string(), + Some("11111111-1111-4111-8111-111111111111".to_string()), auth_manager, models_manager, Arc::new(ExecPolicyManager::default()), @@ -7526,7 +7533,7 @@ where windows_sandbox_level: WindowsSandboxLevel::from_config(&config), environments: TurnEnvironmentSelections::new(config.cwd.clone(), default_environments), workspace_roots: config.workspace_roots.clone(), - codex_home: config.codex_home.clone(), + codex_home: Some(config.codex_home.clone()), thread_name: None, original_config_do_not_use: Arc::clone(&config), metrics_service_name: None, @@ -7700,7 +7707,7 @@ where )); let session = Arc::new(Session { thread_id, - installation_id: "11111111-1111-4111-8111-111111111111".to_string(), + installation_id: Some("11111111-1111-4111-8111-111111111111".to_string()), tx_event, agent_status: agent_status_tx, state: Mutex::new(state), @@ -7744,6 +7751,36 @@ pub(crate) async fn make_session_and_context_with_rx() -> ( make_session_and_context_with_dynamic_tools_and_rx(Vec::new()).await } +#[tokio::test] +async fn pathless_mcp_refresh_keeps_oauth_storage_disabled() { + let (session, turn_context, _rx) = make_session_and_context_with_auth_and_config_and_rx( + CodexAuth::from_api_key("Test API Key"), + Vec::new(), + |config| { + config.mcp_oauth_credentials_store_mode = OAuthCredentialsStoreMode::File; + }, + ) + .await; + session.state.lock().await.session_configuration.codex_home = None; + + session + .refresh_mcp_servers_now( + &turn_context, + &turn_context.config, + /*elicitation_reviewer*/ None, + ) + .await; + + let runtime = session.services.latest_mcp_runtime(); + assert_eq!( + ( + runtime.config().codex_home.clone(), + runtime.config().mcp_oauth_credentials_store_mode, + ), + (None, OAuthCredentialsStoreMode::Disabled) + ); +} + #[tokio::test] async fn refresh_mcp_servers_keeps_the_previous_runtime_alive() { let (session, turn_context) = make_session_and_context().await; diff --git a/codex-rs/core/src/session/tests/guardian_tests.rs b/codex-rs/core/src/session/tests/guardian_tests.rs index d13239d810..0ef9c1d6b0 100644 --- a/codex-rs/core/src/session/tests/guardian_tests.rs +++ b/codex-rs/core/src/session/tests/guardian_tests.rs @@ -717,10 +717,11 @@ async fn guardian_subagent_does_not_inherit_parent_exec_policy_rules() { )); let CodexSpawnOk { codex, .. } = Codex::spawn(CodexSpawnArgs { + local_runtime_paths: Some(crate::LocalRuntimePaths::from(&config)), config, allow_provider_model_fallback: false, user_instructions: Default::default(), - installation_id: "11111111-1111-4111-8111-111111111111".to_string(), + installation_id: Some("11111111-1111-4111-8111-111111111111".to_string()), auth_manager, models_manager, environment_manager: Arc::new(EnvironmentManager::default_for_tests()), diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index e200413d3d..4f708e50ca 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -533,6 +533,7 @@ impl Session { session_configuration.thread_source.clone(), sub_id.clone(), cwd.clone(), + /*allow_host_git_enrichment*/ session_configuration.codex_home().is_some(), &session_configuration.permission_profile(), session_configuration.windows_sandbox_level, network.is_some(), @@ -591,7 +592,10 @@ impl Session { let notify_config_contributors = !self.services.extensions.config_contributors().is_empty(); let update_result: CodexResult<_> = { let mut state = self.state.lock().await; - match state.session_configuration.clone().apply(&updates) { + let updated = self + .validate_environment_update(&state.session_configuration, &updates) + .and_then(|()| state.session_configuration.clone().apply(&updates)); + match updated { Ok(next) => { let previous_permission_profile = state.session_configuration.permission_profile(); diff --git a/codex-rs/core/src/session_rollout_init_error.rs b/codex-rs/core/src/session_rollout_init_error.rs index db71dfcd37..7fd06b8b09 100644 --- a/codex-rs/core/src/session_rollout_init_error.rs +++ b/codex-rs/core/src/session_rollout_init_error.rs @@ -5,7 +5,7 @@ use crate::rollout::SESSIONS_SUBDIR; use codex_protocol::error::CodexErr; use codex_thread_store::ThreadStoreError; -pub(crate) fn map_session_init_error(err: &anyhow::Error, codex_home: &Path) -> CodexErr { +pub(crate) fn map_session_init_error(err: &anyhow::Error, codex_home: Option<&Path>) -> CodexErr { if let Some(ThreadStoreError::Unsupported { operation }) = err .chain() .find_map(|cause| cause.downcast_ref::()) @@ -13,11 +13,11 @@ pub(crate) fn map_session_init_error(err: &anyhow::Error, codex_home: &Path) -> return CodexErr::UnsupportedOperation(format!("{operation} is not supported yet")); } - if let Some(mapped) = err - .chain() - .filter_map(|cause| cause.downcast_ref::()) - .find_map(|io_err| map_rollout_io_error(io_err, codex_home)) - { + if let Some(mapped) = codex_home.and_then(|codex_home| { + err.chain() + .filter_map(|cause| cause.downcast_ref::()) + .find_map(|io_err| map_rollout_io_error(io_err, codex_home)) + }) { return mapped; } diff --git a/codex-rs/core/src/test_support.rs b/codex-rs/core/src/test_support.rs index 8c84330857..23e58c79f9 100644 --- a/codex-rs/core/src/test_support.rs +++ b/codex-rs/core/src/test_support.rs @@ -202,7 +202,7 @@ pub fn responses_metadata( subagent_header: subagent_header_value(session_source), subagent_kind: request_kind.and_then(|_| subagent_metadata_kind(session_source)), ..CodexResponsesMetadata::new( - installation_id.to_string(), + Some(installation_id.to_string()), session_id.to_string(), thread_id.to_string(), window_id, diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index de37484f8b..94a05e3fd3 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -7,6 +7,9 @@ use crate::config::ThreadStoreConfig; use crate::current_time::TimeProvider; use crate::environment_selection::TurnEnvironmentSnapshot; use crate::environment_selection::default_thread_environment_selections; +use crate::local_runtime_paths::LocalRuntimePaths; +use crate::local_runtime_paths::ThreadManagerInitError; +use crate::local_runtime_paths::validate_remote_environment_selections; use crate::mcp::McpManager; use crate::rollout::truncation; use crate::session::Codex; @@ -253,7 +256,10 @@ pub(crate) struct ThreadManagerState { attestation_provider: Option>, external_time_provider: Option>, session_source: SessionSource, - installation_id: String, + local_runtime_paths: Option, + // Local entry points resolve and persist this before construction. `None` is a deliberate + // hosted-mode choice and must remain absent throughout the session. + installation_id: Option, analytics_events_client: Option, // Captures submitted ops for testing purpose when test mode is enabled. ops_log: Option, @@ -312,32 +318,103 @@ impl ThreadManager { analytics_events_client: Option, thread_store: Arc, agent_graph_store: Option>, - installation_id: String, + installation_id: impl Into>, attestation_provider: Option>, external_time_provider: Option>, ) -> Self { - let codex_home = config.codex_home.clone(); + match Self::try_new( + config, + Some(LocalRuntimePaths::from(config)), + auth_manager, + session_source, + environment_manager, + extensions, + user_instructions_provider, + analytics_events_client, + thread_store, + agent_graph_store, + installation_id.into(), + attestation_provider, + external_time_provider, + ) { + Ok(manager) => manager, + Err(error) => panic!("local thread manager construction failed: {error}"), + } + } + + /// Construct a manager with an explicit host-local runtime capability. + /// + /// Remote-only callers pass `None`. Construction fails before managers or + /// background work are created when the selected thread store needs local paths. + #[allow(clippy::too_many_arguments)] + pub fn try_new( + config: &Config, + local_runtime_paths: Option, + auth_manager: Arc, + session_source: SessionSource, + environment_manager: Arc, + extensions: Arc>, + user_instructions_provider: Arc, + analytics_events_client: Option, + thread_store: Arc, + agent_graph_store: Option>, + installation_id: impl Into>, + attestation_provider: Option>, + external_time_provider: Option>, + ) -> Result { + let installation_id = installation_id.into(); + if local_runtime_paths.is_none() && thread_store.requires_local_runtime_paths() { + return Err(ThreadManagerInitError::ThreadStoreRequiresLocalRuntimePaths); + } + if local_runtime_paths.is_none() + && agent_graph_store + .as_ref() + .is_some_and(|store| store.requires_local_runtime_paths()) + { + return Err(ThreadManagerInitError::AgentGraphStoreRequiresLocalRuntimePaths); + } + let restriction_product = session_source.restriction_product(); let (thread_created_tx, _) = broadcast::channel(THREAD_CREATED_CHANNEL_CAPACITY); - let plugins_manager = Arc::new(PluginsManager::new_with_options( - codex_home.to_path_buf(), - restriction_product, - auth_manager.get_api_auth_mode(), - )); + let plugins_manager = Arc::new(match local_runtime_paths.as_ref() { + Some(paths) => PluginsManager::new_with_options( + paths.codex_home.to_path_buf(), + restriction_product, + auth_manager.get_api_auth_mode(), + ), + None => PluginsManager::new_without_host_filesystem_with_options( + restriction_product, + auth_manager.get_api_auth_mode(), + ), + }); let mcp_manager = Arc::new(McpManager::new_with_extensions( Arc::clone(&plugins_manager), Arc::clone(&extensions), )); - let skills_service = Arc::new(SkillsService::new_with_restriction_product( - codex_home, - config.bundled_skills_enabled(), - restriction_product, - )); - Self { + let skills_service = Arc::new(match local_runtime_paths.as_ref() { + Some(paths) => SkillsService::new_with_restriction_product( + paths.codex_home.clone(), + config.bundled_skills_enabled(), + restriction_product, + ), + None => SkillsService::new_without_host_filesystem_with_restriction_product( + restriction_product, + ), + }); + let model_provider = create_model_provider( + config.model_provider.clone(), + Some(Arc::clone(&auth_manager)), + ); + let models_manager = match local_runtime_paths.as_ref() { + Some(paths) => model_provider + .models_manager(paths.codex_home.to_path_buf(), config.model_catalog.clone()), + None => model_provider.models_manager_without_disk_cache(config.model_catalog.clone()), + }; + Ok(Self { state: Arc::new(ThreadManagerState { threads: Arc::new(RwLock::new(HashMap::new())), thread_created_tx, - models_manager: build_models_manager(config, auth_manager.clone()), + models_manager, environment_manager, skills_service, plugins_manager, @@ -355,13 +432,14 @@ impl ThreadManager { external_time_provider, auth_manager, session_source, + local_runtime_paths, installation_id, analytics_events_client, ops_log: should_use_test_thread_manager_behavior() .then(|| Arc::new(std::sync::Mutex::new(Vec::new()))), }), _test_codex_home_guard: None, - } + }) } pub(crate) fn with_code_mode_host_program_for_tests(mut self, host_program: PathBuf) -> Self { @@ -423,11 +501,15 @@ impl ThreadManager { ) -> Self { set_thread_manager_test_mode_for_tests(/*enabled*/ true); let auth_manager = AuthManager::from_auth_for_testing(auth); - let installation_id = uuid::Uuid::new_v4().to_string(); + let installation_id = Some(uuid::Uuid::new_v4().to_string()); let skills_codex_home = match AbsolutePathBuf::from_absolute_path_checked(&codex_home) { Ok(codex_home) => codex_home, Err(err) => panic!("test codex_home should be absolute: {err}"), }; + let local_runtime_paths = Some(LocalRuntimePaths { + codex_home: skills_codex_home.clone(), + cwd: skills_codex_home.clone(), + }); let (thread_created_tx, _) = broadcast::channel(THREAD_CREATED_CHANNEL_CAPACITY); let restriction_product = SessionSource::Exec.restriction_product(); let plugins_manager = Arc::new(PluginsManager::new_with_options( @@ -473,6 +555,7 @@ impl ThreadManager { external_time_provider: None, auth_manager, session_source: SessionSource::Exec, + local_runtime_paths, installation_id, analytics_events_client: None, ops_log: should_use_test_thread_manager_behavior() @@ -506,6 +589,11 @@ impl ThreadManager { self.state.environment_manager.clone() } + /// Whether this manager owns host-local paths that runtime services may inspect. + pub fn has_local_runtime_paths(&self) -> bool { + self.state.local_runtime_paths.is_some() + } + pub fn default_environment_selections( &self, cwd: &AbsolutePathBuf, @@ -517,6 +605,12 @@ impl ThreadManager { &self, environments: &[TurnEnvironmentSelection], ) -> CodexResult<()> { + if self.state.local_runtime_paths.is_none() { + validate_remote_environment_selections( + self.state.environment_manager.as_ref(), + environments, + )?; + } let mut environment_ids = HashSet::with_capacity(environments.len()); for environment in environments { if !environment_ids.insert(environment.environment_id.as_str()) { @@ -1584,10 +1678,20 @@ impl ThreadManagerState { forked_from_thread_id, ) .await; + // A local manager owns a stable Codex home, while each thread may load + // a different workspace cwd from its effective Config. + let local_runtime_paths = + self.local_runtime_paths + .as_ref() + .map(|paths| LocalRuntimePaths { + codex_home: paths.codex_home.clone(), + cwd: config.cwd.clone(), + }); let CodexSpawnOk { codex, thread_id, .. } = Box::pin(Codex::spawn(CodexSpawnArgs { config, + local_runtime_paths, allow_provider_model_fallback, user_instructions, installation_id: self.installation_id.clone(), diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index d29d6d04e5..b9e1e94593 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -1,4 +1,6 @@ use super::*; +use crate::ThreadStartError; +use crate::config::ConstraintError; use crate::config::test_config; use crate::init_state_db; use crate::installation_id::INSTALLATION_ID_FILENAME; @@ -8,10 +10,12 @@ use crate::session::tests::build_world_state_from_turn_context; use crate::session::tests::make_session_and_context; use crate::tasks::InterruptedTurnHistoryMarker; use crate::tasks::interrupted_turn_history_marker; +use codex_config::types::OAuthCredentialsStoreMode; use codex_extension_api::empty_extension_registry; use codex_models_manager::manager::RefreshStrategy; use codex_protocol::capabilities::CapabilityRootLocation; use codex_protocol::capabilities::SelectedCapabilityRoot; +use codex_protocol::config_types::ModelProviderAuthInfo; use codex_protocol::models::ContentItem; use codex_protocol::models::ReasoningItemReasoningSummary; use codex_protocol::models::ResponseItem; @@ -24,6 +28,7 @@ use codex_protocol::protocol::SessionMeta; use codex_protocol::protocol::SessionMetaLine; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::ThreadSource; +use codex_protocol::protocol::TurnEnvironmentSelections; use codex_protocol::protocol::TurnStartedEvent; use codex_protocol::protocol::UserMessageEvent; use codex_utils_path_uri::PathUri; @@ -600,7 +605,7 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors() /*analytics_events_client*/ None, thread_store_from_config(&config, /*state_db*/ None), /*agent_graph_store*/ None, - TEST_INSTALLATION_ID.to_string(), + Some(TEST_INSTALLATION_ID.to_string()), /*attestation_provider*/ None, /*external_time_provider*/ None, ); @@ -819,7 +824,7 @@ async fn resume_and_fork_do_not_restore_thread_environments_from_rollout() { /*analytics_events_client*/ None, thread_store_from_config(&config, /*state_db*/ None), /*agent_graph_store*/ None, - TEST_INSTALLATION_ID.to_string(), + Some(TEST_INSTALLATION_ID.to_string()), /*attestation_provider*/ None, /*external_time_provider*/ None, ); @@ -945,7 +950,7 @@ async fn explicit_installation_id_skips_codex_home_file() { /*analytics_events_client*/ None, thread_store, local_agent_graph_store_from_state_db(state_db.as_ref()), - installation_id.clone(), + Some(installation_id.clone()), /*attestation_provider*/ None, /*external_time_provider*/ None, ); @@ -956,7 +961,53 @@ async fn explicit_installation_id_skips_codex_home_file() { .expect("start thread with explicit installation id"); assert!(!config.codex_home.join(INSTALLATION_ID_FILENAME).exists()); - assert_eq!(thread.thread.codex.session.installation_id, installation_id); + assert_eq!( + thread.thread.codex.session.installation_id, + Some(installation_id) + ); + + thread + .thread + .shutdown_and_wait() + .await + .expect("shutdown thread"); + let _ = manager.remove_thread(&thread.thread_id).await; +} + +#[tokio::test] +async fn absent_installation_id_skips_codex_home_file() { + let temp_dir = tempdir().expect("tempdir"); + let mut config = test_config().await; + config.codex_home = temp_dir.path().join("codex-home").abs(); + config.cwd = config.codex_home.abs(); + std::fs::create_dir_all(&config.codex_home).expect("create codex home"); + + let auth_manager = + AuthManager::from_auth_for_testing(CodexAuth::create_dummy_chatgpt_auth_for_testing()); + let state_db = init_state_db(&config).await; + let thread_store = thread_store_from_config(&config, state_db.clone()); + let manager = ThreadManager::new( + &config, + auth_manager, + SessionSource::Exec, + Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), + empty_extension_registry(), + Arc::new(crate::test_support::EmptyUserInstructionsProvider), + /*analytics_events_client*/ None, + thread_store, + local_agent_graph_store_from_state_db(state_db.as_ref()), + /*installation_id*/ None, + /*attestation_provider*/ None, + /*external_time_provider*/ None, + ); + + let thread = manager + .start_thread(config.clone()) + .await + .expect("start thread without installation id"); + + assert!(!config.codex_home.join(INSTALLATION_ID_FILENAME).exists()); + assert_eq!(thread.thread.codex.session.installation_id, None); thread .thread @@ -986,7 +1037,7 @@ async fn resume_active_thread_from_rollout_returns_running_thread() { /*analytics_events_client*/ None, thread_store_from_config(&config, /*state_db*/ None), /*agent_graph_store*/ None, - TEST_INSTALLATION_ID.to_string(), + Some(TEST_INSTALLATION_ID.to_string()), /*attestation_provider*/ None, /*external_time_provider*/ None, ); @@ -1046,7 +1097,7 @@ async fn resume_stopped_thread_from_rollout_spawns_new_thread() { /*analytics_events_client*/ None, thread_store_from_config(&config, /*state_db*/ None), /*agent_graph_store*/ None, - TEST_INSTALLATION_ID.to_string(), + Some(TEST_INSTALLATION_ID.to_string()), /*attestation_provider*/ None, /*external_time_provider*/ None, ); @@ -1113,7 +1164,7 @@ async fn resume_stopped_thread_from_rollout_preserves_thread_source() { /*analytics_events_client*/ None, thread_store, local_agent_graph_store_from_state_db(state_db.as_ref()), - TEST_INSTALLATION_ID.to_string(), + Some(TEST_INSTALLATION_ID.to_string()), /*attestation_provider*/ None, /*external_time_provider*/ None, ); @@ -1205,7 +1256,7 @@ async fn subtree_listing_uses_injected_graph_store_without_state_db() { /*analytics_events_client*/ None, thread_store_from_config(&config, /*state_db*/ None), Some(agent_graph_store), - TEST_INSTALLATION_ID.to_string(), + Some(TEST_INSTALLATION_ID.to_string()), /*attestation_provider*/ None, /*external_time_provider*/ None, ); @@ -1221,6 +1272,431 @@ async fn subtree_listing_uses_injected_graph_store_without_state_db() { ); } +fn test_thread_manager_with_runtime_paths( + config: &Config, + local_runtime_paths: Option, + environment_manager: Arc, + thread_store: Arc, + agent_graph_store: Option>, +) -> Result { + ThreadManager::try_new( + config, + local_runtime_paths, + AuthManager::from_auth_for_testing(CodexAuth::create_dummy_chatgpt_auth_for_testing()), + SessionSource::Exec, + environment_manager, + empty_extension_registry(), + Arc::new(crate::test_support::EmptyUserInstructionsProvider), + /*analytics_events_client*/ None, + thread_store, + agent_graph_store, + /*installation_id*/ None, + /*attestation_provider*/ None, + /*external_time_provider*/ None, + ) +} + +fn pathless_start_options( + config: Config, + environments: Vec, +) -> StartThreadOptions { + StartThreadOptions { + config, + allow_provider_model_fallback: false, + initial_history: InitialHistory::New, + history_mode: None, + session_source: None, + thread_source: None, + dynamic_tools: Vec::new(), + metrics_service_name: None, + parent_trace: None, + environments, + thread_extension_init: Default::default(), + supports_openai_form_elicitation: false, + } +} + +fn pathless_in_memory_store(test_name: &str) -> Arc { + InMemoryThreadStore::for_id(format!("{test_name}-{}", uuid::Uuid::new_v4())) +} + +fn pathless_thread_manager( + config: &Config, + environment_manager: Arc, + test_name: &str, +) -> ThreadManager { + test_thread_manager_with_runtime_paths( + config, + /*local_runtime_paths*/ None, + environment_manager, + pathless_in_memory_store(test_name), + /*agent_graph_store*/ None, + ) + .expect("in-memory storage supports a pathless manager") +} + +async fn assert_pathless_start_error( + manager: &ThreadManager, + config: Config, + environments: Vec, + expected: ThreadStartError, +) { + let Err(CodexErr::InvalidRequest(message)) = manager + .start_thread_with_options(pathless_start_options(config, environments)) + .await + else { + panic!("pathless thread should reject invalid environment selections"); + }; + assert_eq!(message, expected.to_string()); +} + +async fn remote_and_local_environment_manager() -> Arc { + Arc::new( + codex_exec_server::EnvironmentManager::create_for_tests_with_local( + Some("ws://127.0.0.1:9".to_string()), + codex_exec_server::ExecServerRuntimePaths::new( + std::env::current_exe().expect("current test executable"), + /*codex_linux_sandbox_exe*/ None, + ) + .expect("test runtime paths"), + ) + .await, + ) +} + +#[tokio::test] +async fn pathless_manager_rejects_a_local_thread_store() { + let config = test_config().await; + let local_store: Arc = Arc::new(LocalThreadStore::new( + LocalThreadStoreConfig::from_config(&config), + /*state_db*/ None, + )); + + let error = test_thread_manager_with_runtime_paths( + &config, + /*local_runtime_paths*/ None, + Arc::new(codex_exec_server::EnvironmentManager::without_environments()), + local_store, + /*agent_graph_store*/ None, + ) + .err(); + + assert_eq!( + error, + Some(ThreadManagerInitError::ThreadStoreRequiresLocalRuntimePaths) + ); +} + +#[tokio::test] +async fn pathless_manager_rejects_an_agent_graph_store_that_requires_local_paths() { + let config = test_config().await; + let agent_graph_store: Arc = Arc::new(FakeAgentGraphStore { + root_thread_id: ThreadId::new(), + descendant_thread_ids: Vec::new(), + }); + + let error = test_thread_manager_with_runtime_paths( + &config, + /*local_runtime_paths*/ None, + Arc::new(codex_exec_server::EnvironmentManager::without_environments()), + pathless_in_memory_store("pathless-agent-graph-store"), + Some(agent_graph_store), + ) + .err(); + + assert_eq!( + error, + Some(ThreadManagerInitError::AgentGraphStoreRequiresLocalRuntimePaths) + ); +} + +#[tokio::test] +async fn pathless_manager_accepts_an_in_memory_thread_store() { + let config = test_config().await; + + let _manager = pathless_thread_manager( + &config, + Arc::new(codex_exec_server::EnvironmentManager::without_environments()), + "pathless-manager-init", + ); +} + +#[tokio::test] +async fn pathless_thread_requires_a_remote_environment() { + let config = test_config().await; + let manager = pathless_thread_manager( + &config, + Arc::new(codex_exec_server::EnvironmentManager::without_environments()), + "pathless-missing-environment", + ); + assert_pathless_start_error( + &manager, + config, + Vec::new(), + ThreadStartError::MissingRemoteEnvironment, + ) + .await; +} + +#[tokio::test] +async fn pathless_thread_rejects_an_unknown_environment() { + let config = test_config().await; + let manager = pathless_thread_manager( + &config, + Arc::new(codex_exec_server::EnvironmentManager::without_environments()), + "pathless-unknown-environment", + ); + let environments = vec![TurnEnvironmentSelection { + environment_id: "missing".to_string(), + cwd: PathUri::from_abs_path(&config.cwd), + }]; + + assert_pathless_start_error( + &manager, + config, + environments, + ThreadStartError::UnknownEnvironment { + environment_id: "missing".to_string(), + }, + ) + .await; +} + +#[tokio::test] +async fn pathless_thread_rejects_a_local_environment() { + let config = test_config().await; + let manager = pathless_thread_manager( + &config, + Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), + "pathless-local-environment", + ); + let environments = vec![TurnEnvironmentSelection { + environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), + cwd: PathUri::from_abs_path(&config.cwd), + }]; + + assert_pathless_start_error( + &manager, + config, + environments, + ThreadStartError::LocalEnvironmentNotAllowed { + environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), + }, + ) + .await; +} + +#[tokio::test] +async fn pathless_thread_rejects_a_local_environment_after_a_remote_environment() { + let config = test_config().await; + let environment_manager = remote_and_local_environment_manager().await; + let manager = + pathless_thread_manager(&config, environment_manager, "pathless-mixed-environments"); + let remote_root = tempdir().expect("remote root placeholder"); + let environments = vec![ + TurnEnvironmentSelection { + environment_id: codex_exec_server::REMOTE_ENVIRONMENT_ID.to_string(), + cwd: PathUri::from_abs_path(&remote_root.path().join("workspace").abs()), + }, + TurnEnvironmentSelection { + environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), + cwd: PathUri::from_abs_path(&config.cwd), + }, + ]; + + assert_pathless_start_error( + &manager, + config, + environments, + ThreadStartError::LocalEnvironmentNotAllowed { + environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), + }, + ) + .await; +} + +#[tokio::test] +async fn pathless_thread_rejects_command_backed_model_auth() { + let mut config = test_config().await; + config.model_provider.auth = Some( + serde_json::from_value::(serde_json::json!({ + "command": "host-token-command" + })) + .expect("valid command auth config"), + ); + let environment_manager = Arc::new( + codex_exec_server::EnvironmentManager::create_for_tests( + Some("ws://127.0.0.1:9".to_string()), + /*local_runtime_paths*/ None, + ) + .await, + ); + let manager = + pathless_thread_manager(&config, environment_manager, "pathless-command-model-auth"); + let remote_cwd = tempdir() + .expect("remote root placeholder") + .path() + .join("workspace") + .abs(); + let environments = vec![TurnEnvironmentSelection { + environment_id: codex_exec_server::REMOTE_ENVIRONMENT_ID.to_string(), + cwd: PathUri::from_abs_path(&remote_cwd), + }]; + + assert_pathless_start_error( + &manager, + config, + environments, + ThreadStartError::CommandModelAuthNotAllowed, + ) + .await; +} + +#[tokio::test] +async fn pathless_thread_uses_the_first_remote_environment_cwd() { + let mut config = test_config().await; + config.mcp_oauth_credentials_store_mode = OAuthCredentialsStoreMode::File; + let config_for_runtime = config.clone(); + let local_config_home = config.codex_home.clone(); + assert!(!local_config_home.exists()); + let environment_manager = Arc::new( + codex_exec_server::EnvironmentManager::create_for_tests( + Some("ws://127.0.0.1:9".to_string()), + /*local_runtime_paths*/ None, + ) + .await, + ); + let manager = + pathless_thread_manager(&config, environment_manager, "pathless-remote-environment"); + let remote_root = tempdir().expect("remote root placeholder"); + let remote_cwd = remote_root.path().join("workspace").abs(); + assert!(!remote_cwd.exists()); + let environments = vec![TurnEnvironmentSelection { + environment_id: codex_exec_server::REMOTE_ENVIRONMENT_ID.to_string(), + cwd: PathUri::from_abs_path(&remote_cwd), + }]; + + let started = manager + .start_thread_with_options(pathless_start_options(config, environments)) + .await + .expect("explicit remote environment should start a pathless thread"); + + let initialized_mcp = started.thread.codex.session.services.latest_mcp_runtime(); + let runtime_mcp = started.thread.runtime_mcp_config(&config_for_runtime).await; + assert_eq!(started.session_configured.cwd, remote_cwd.clone()); + assert_eq!( + ( + initialized_mcp.config().codex_home.clone(), + initialized_mcp.config().mcp_oauth_credentials_store_mode, + ), + (None, OAuthCredentialsStoreMode::Disabled) + ); + assert_eq!( + ( + runtime_mcp.codex_home, + runtime_mcp.mcp_oauth_credentials_store_mode, + ), + (None, OAuthCredentialsStoreMode::Disabled) + ); + started + .thread + .shutdown_and_wait() + .await + .expect("shutdown pathless remote thread"); + let _ = manager.remove_thread(&started.thread_id).await; + assert!(!remote_cwd.exists()); + assert!(!local_config_home.exists()); +} + +#[tokio::test] +async fn pathless_thread_rejects_selecting_a_local_environment_later() { + let config = test_config().await; + let local_cwd = config.cwd.clone(); + let manager = pathless_thread_manager( + &config, + remote_and_local_environment_manager().await, + "pathless-later-local-environment", + ); + let remote_root = tempdir().expect("remote root placeholder"); + let remote_cwd = remote_root.path().join("workspace").abs(); + let started = manager + .start_thread_with_options(pathless_start_options( + config, + vec![TurnEnvironmentSelection { + environment_id: codex_exec_server::REMOTE_ENVIRONMENT_ID.to_string(), + cwd: PathUri::from_abs_path(&remote_cwd), + }], + )) + .await + .expect("explicit remote environment should start a pathless thread"); + + let mismatched_cwd_error = started + .thread + .codex + .session + .update_settings(SessionSettingsUpdate { + environments: Some(TurnEnvironmentSelections::new( + local_cwd.clone(), + vec![TurnEnvironmentSelection { + environment_id: codex_exec_server::REMOTE_ENVIRONMENT_ID.to_string(), + cwd: PathUri::from_abs_path(&remote_cwd), + }], + )), + ..Default::default() + }) + .await + .expect_err("pathless thread must derive its fallback cwd from the remote environment"); + assert_eq!( + mismatched_cwd_error, + ConstraintError::InvalidValue { + field_name: "environments", + candidate: ThreadStartError::LegacyCwdMismatch { + expected: remote_cwd.clone(), + actual: local_cwd.clone(), + } + .to_string(), + allowed: "a nonempty set of known remote environments".to_string(), + requirement_source: codex_config::RequirementSource::Unknown, + } + ); + + let error = started + .thread + .codex + .session + .update_settings(SessionSettingsUpdate { + environments: Some(TurnEnvironmentSelections::new( + remote_cwd, + vec![TurnEnvironmentSelection { + environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), + cwd: PathUri::from_abs_path(&local_cwd), + }], + )), + ..Default::default() + }) + .await + .expect_err("pathless thread must remain remote-only"); + + assert_eq!( + error, + ConstraintError::InvalidValue { + field_name: "environments", + candidate: ThreadStartError::LocalEnvironmentNotAllowed { + environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), + } + .to_string(), + allowed: "a nonempty set of known remote environments".to_string(), + requirement_source: codex_config::RequirementSource::Unknown, + } + ); + started + .thread + .shutdown_and_wait() + .await + .expect("shutdown pathless remote thread"); + let _ = manager.remove_thread(&started.thread_id).await; +} + #[tokio::test] async fn rollout_path_resume_and_fork_read_history_through_thread_store() { let temp_dir = tempdir().expect("tempdir"); @@ -1250,7 +1726,7 @@ async fn rollout_path_resume_and_fork_read_history_through_thread_store() { /*analytics_events_client*/ None, thread_store.clone(), local_agent_graph_store_from_state_db(state_db.as_ref()), - TEST_INSTALLATION_ID.to_string(), + Some(TEST_INSTALLATION_ID.to_string()), /*attestation_provider*/ None, /*external_time_provider*/ None, ); @@ -1355,7 +1831,7 @@ async fn new_uses_active_provider_for_model_refresh() { /*analytics_events_client*/ None, thread_store_from_config(&config, /*state_db*/ None), /*agent_graph_store*/ None, - TEST_INSTALLATION_ID.to_string(), + Some(TEST_INSTALLATION_ID.to_string()), /*attestation_provider*/ None, /*external_time_provider*/ None, ); @@ -1582,7 +2058,7 @@ async fn interrupted_fork_snapshot_does_not_synthesize_turn_id_for_legacy_histor /*analytics_events_client*/ None, thread_store_from_config(&config, state_db.clone()), local_agent_graph_store_from_state_db(state_db.as_ref()), - TEST_INSTALLATION_ID.to_string(), + Some(TEST_INSTALLATION_ID.to_string()), /*attestation_provider*/ None, /*external_time_provider*/ None, ); @@ -1691,7 +2167,7 @@ async fn interrupted_fork_snapshot_preserves_explicit_turn_id() { /*analytics_events_client*/ None, thread_store_from_config(&config, state_db.clone()), local_agent_graph_store_from_state_db(state_db.as_ref()), - TEST_INSTALLATION_ID.to_string(), + Some(TEST_INSTALLATION_ID.to_string()), /*attestation_provider*/ None, /*external_time_provider*/ None, ); @@ -1790,7 +2266,7 @@ async fn interrupted_fork_snapshot_uses_persisted_mid_turn_history_without_live_ /*analytics_events_client*/ None, thread_store_from_config(&config, state_db.clone()), local_agent_graph_store_from_state_db(state_db.as_ref()), - TEST_INSTALLATION_ID.to_string(), + Some(TEST_INSTALLATION_ID.to_string()), /*attestation_provider*/ None, /*external_time_provider*/ None, ); diff --git a/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs b/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs index e5923bab79..929f6d5e6e 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents/spawn.rs @@ -6,6 +6,7 @@ use crate::agent::exceeds_thread_spawn_depth_limit; use crate::agent::next_thread_spawn_depth; use crate::agent::role::DEFAULT_ROLE_NAME; use crate::agent::role::apply_role_to_config; +use crate::agent::role::apply_role_to_config_without_host_filesystem; use crate::tools::handlers::multi_agents_spec::SpawnAgentToolOptions; use crate::tools::handlers::multi_agents_spec::create_spawn_agent_tool_v1; use codex_tools::ToolSpec; @@ -106,9 +107,12 @@ async fn handle_spawn_agent( args.reasoning_effort.clone(), ) .await?; - apply_role_to_config(&mut config, role_name) - .await - .map_err(FunctionCallError::RespondToModel)?; + let role_result = if session.local_runtime_paths().await.is_some() { + apply_role_to_config(&mut config, role_name).await + } else { + apply_role_to_config_without_host_filesystem(&mut config, role_name).await + }; + role_result.map_err(FunctionCallError::RespondToModel)?; } apply_spawn_agent_service_tier( &session, diff --git a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs index 7d50d567b9..24145edacc 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_tests.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_tests.rs @@ -4261,7 +4261,7 @@ async fn tool_handlers_cascade_close_and_resume_and_keep_explicitly_closed_subtr /*analytics_events_client*/ None, thread_store_from_config(&config, state_db.clone()), local_agent_graph_store_from_state_db(state_db.as_ref()), - "11111111-1111-4111-8111-111111111111".to_string(), + Some("11111111-1111-4111-8111-111111111111".to_string()), /*attestation_provider*/ None, /*external_time_provider*/ None, ); diff --git a/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs b/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs index 1c68849c5e..85a4141b86 100644 --- a/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs +++ b/codex-rs/core/src/tools/handlers/multi_agents_v2/spawn.rs @@ -4,6 +4,7 @@ use crate::agent::control::SpawnAgentOptions; use crate::agent::next_thread_spawn_depth; use crate::agent::role::DEFAULT_ROLE_NAME; use crate::agent::role::apply_role_to_config; +use crate::agent::role::apply_role_to_config_without_host_filesystem; use crate::agent_communication::AgentCommunicationContext; use crate::agent_communication::AgentCommunicationKind; use crate::tools::handlers::multi_agents_spec::SpawnAgentToolOptions; @@ -79,9 +80,12 @@ async fn handle_spawn_agent( args.reasoning_effort.clone(), ) .await?; - apply_role_to_config(&mut config, role_name) - .await - .map_err(FunctionCallError::RespondToModel)?; + let role_result = if session.local_runtime_paths().await.is_some() { + apply_role_to_config(&mut config, role_name).await + } else { + apply_role_to_config_without_host_filesystem(&mut config, role_name).await + }; + role_result.map_err(FunctionCallError::RespondToModel)?; } apply_spawn_agent_service_tier( &session, diff --git a/codex-rs/core/src/tools/handlers/request_plugin_install.rs b/codex-rs/core/src/tools/handlers/request_plugin_install.rs index e44ed41fa4..678972fa7d 100644 --- a/codex-rs/core/src/tools/handlers/request_plugin_install.rs +++ b/codex-rs/core/src/tools/handlers/request_plugin_install.rs @@ -222,7 +222,7 @@ impl RequestPluginInstallHandler { .await; let response = elicitation.response; if let Some(response) = response.as_ref() { - maybe_persist_disabled_install_request(&session, &turn, &tool, response).await; + maybe_persist_disabled_install_request(&session, &tool, response).await; } let user_confirmed = response .as_ref() @@ -289,7 +289,6 @@ impl CoreToolRuntime for RequestPluginInstallHandler {} async fn maybe_persist_disabled_install_request( session: &crate::session::session::Session, - turn: &crate::session::turn_context::TurnContext, tool: &DiscoverableTool, response: &ElicitationResponse, ) { @@ -297,7 +296,16 @@ async fn maybe_persist_disabled_install_request( return; } - if let Err(err) = persist_disabled_install_request(&turn.config.codex_home, tool).await { + let Some(local_runtime_paths) = session.local_runtime_paths().await else { + warn!( + tool_id = tool.id(), + "cannot persist a disabled tool suggestion without local runtime paths" + ); + return; + }; + + if let Err(err) = persist_disabled_install_request(&local_runtime_paths.codex_home, tool).await + { warn!( error = %err, tool_id = tool.id(), diff --git a/codex-rs/core/src/turn_metadata.rs b/codex-rs/core/src/turn_metadata.rs index 0a1f781b38..e20a858c3c 100644 --- a/codex-rs/core/src/turn_metadata.rs +++ b/codex-rs/core/src/turn_metadata.rs @@ -65,19 +65,24 @@ impl From for TurnMetadataWorkspace { #[allow(clippy::too_many_arguments)] pub async fn detached_memory_responses_metadata( - installation_id: String, + installation_id: Option, session_id: String, thread_id: String, window_id: String, session_source: &SessionSource, cwd: &AbsolutePathBuf, + allow_host_git_enrichment: bool, sandbox: Option<&str>, ) -> CodexResponsesMetadata { CodexResponsesMetadata { request_kind: Some(CodexResponsesRequestKind::Memory), subagent_header: subagent_header_value(session_source), sandbox: sandbox.map(ToString::to_string), - workspaces: memory_workspaces(cwd).await, + workspaces: if allow_host_git_enrichment { + memory_workspaces(cwd).await + } else { + BTreeMap::new() + }, ..CodexResponsesMetadata::new(installation_id, session_id, thread_id, window_id) } } @@ -113,11 +118,17 @@ impl TurnMetadataState { thread_source: Option, turn_id: String, cwd: AbsolutePathBuf, + allow_host_git_enrichment: bool, permission_profile: &PermissionProfile, windows_sandbox_level: WindowsSandboxLevel, enforce_managed_network: bool, ) -> Self { - let repo_root = get_git_repo_root(&cwd).map(|root| root.to_string_lossy().into_owned()); + // A path from a remote executor may also exist on the shared host. Never inspect it with + // host git commands unless the session explicitly owns local runtime paths. + let repo_root = allow_host_git_enrichment + .then(|| get_git_repo_root(&cwd)) + .flatten() + .map(|root| root.to_string_lossy().into_owned()); let sandbox = Some( permission_profile_sandbox_tag( permission_profile, @@ -186,7 +197,7 @@ impl TurnMetadataState { pub(crate) fn to_responses_metadata( &self, - installation_id: String, + installation_id: Option, window_id: String, request_kind: CodexResponsesRequestKind, ) -> CodexResponsesMetadata { @@ -239,7 +250,7 @@ impl TurnMetadataState { .unwrap_or_else(std::sync::PoisonError::into_inner) .clone(), ..CodexResponsesMetadata::new( - String::new(), + None, self.session_id.clone(), self.thread_id.clone(), String::new(), @@ -312,6 +323,18 @@ impl TurnMetadataState { } } + #[cfg(test)] + pub(crate) async fn wait_for_git_enrichment_task_for_test(&self) { + let task = self + .enrichment_task + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .take(); + if let Some(task) = task { + let _ = task.await; + } + } + async fn fetch_workspace_git_metadata(&self) -> WorkspaceGitMetadata { let (head_commit_hash, associated_remote_urls, has_changes) = tokio::join!( get_head_commit_hash(&self.cwd), diff --git a/codex-rs/core/src/turn_metadata_tests.rs b/codex-rs/core/src/turn_metadata_tests.rs index 83649b9187..daa7621b1b 100644 --- a/codex-rs/core/src/turn_metadata_tests.rs +++ b/codex-rs/core/src/turn_metadata_tests.rs @@ -38,7 +38,7 @@ fn test_responses_metadata_json( ) -> String { state .to_responses_metadata( - "installation-a".to_string(), + Some("installation-a".to_string()), window_id.to_string(), request_kind, ) @@ -114,12 +114,13 @@ async fn detached_memory_responses_metadata_omits_turn_identity() { let (_temp_dir, repo_path) = create_clean_git_repo("repo-東京").await; let header = detached_memory_responses_metadata( - String::new(), + None, String::new(), String::new(), String::new(), &SessionSource::Unknown, &repo_path, + /*allow_host_git_enrichment*/ true, Some("none"), ) .await @@ -160,12 +161,35 @@ async fn detached_memory_responses_metadata_omits_empty_workspace_metadata() { let cwd = temp_dir.path().abs(); let header = detached_memory_responses_metadata( - String::new(), + None, String::new(), String::new(), String::new(), &SessionSource::Unknown, &cwd, + /*allow_host_git_enrichment*/ true, + /*sandbox*/ None, + ) + .await + .turn_metadata_json() + .expect("detached memory should emit its request kind"); + let parsed: Value = serde_json::from_str(&header).expect("valid json"); + + assert_eq!(parsed, serde_json::json!({"request_kind": "memory"})); +} + +#[tokio::test] +async fn pathless_detached_memory_metadata_ignores_existing_host_git_repo() { + let (_temp_dir, repo_path) = create_clean_git_repo("host-memory-repo").await; + + let header = detached_memory_responses_metadata( + None, + String::new(), + String::new(), + String::new(), + &SessionSource::Unknown, + &repo_path, + /*allow_host_git_enrichment*/ false, /*sandbox*/ None, ) .await @@ -191,6 +215,7 @@ fn turn_metadata_state_uses_platform_sandbox_tag() { /*thread_source*/ None, "turn-a".to_string(), cwd, + /*allow_host_git_enrichment*/ true, &permission_profile, WindowsSandboxLevel::Disabled, /*enforce_managed_network*/ false, @@ -234,6 +259,7 @@ fn turn_metadata_state_includes_root_fork_lineage() { /*thread_source*/ None, "turn-a".to_string(), cwd, + /*allow_host_git_enrichment*/ true, &permission_profile, WindowsSandboxLevel::Disabled, /*enforce_managed_network*/ false, @@ -273,6 +299,7 @@ fn turn_metadata_state_includes_thread_spawn_subagent_parent_without_fork() { /*thread_source*/ None, "turn-a".to_string(), cwd, + /*allow_host_git_enrichment*/ true, &permission_profile, WindowsSandboxLevel::Disabled, /*enforce_managed_network*/ false, @@ -312,6 +339,7 @@ fn turn_metadata_state_includes_forked_thread_spawn_subagent_lineage() { /*thread_source*/ None, "turn-a".to_string(), cwd, + /*allow_host_git_enrichment*/ true, &permission_profile, WindowsSandboxLevel::Disabled, /*enforce_managed_network*/ false, @@ -357,6 +385,7 @@ fn turn_metadata_state_includes_known_parent_for_non_thread_spawn_subagents_with /*thread_source*/ None, "turn-a".to_string(), cwd.clone(), + /*allow_host_git_enrichment*/ true, &permission_profile, WindowsSandboxLevel::Disabled, /*enforce_managed_network*/ false, @@ -389,6 +418,7 @@ fn turn_metadata_state_includes_turn_started_at_unix_ms_after_start() { /*thread_source*/ None, "turn-a".to_string(), cwd, + /*allow_host_git_enrichment*/ true, &permission_profile, WindowsSandboxLevel::Disabled, /*enforce_managed_network*/ false, @@ -419,6 +449,7 @@ fn turn_metadata_state_includes_model_and_reasoning_effort_only_in_request_meta( /*thread_source*/ None, "turn-a".to_string(), cwd, + /*allow_host_git_enrichment*/ true, &permission_profile, WindowsSandboxLevel::Disabled, /*enforce_managed_network*/ false, @@ -468,6 +499,7 @@ fn turn_metadata_state_marks_user_input_requested_during_turn_only_for_mcp_reque /*thread_source*/ None, "turn-a".to_string(), cwd, + /*allow_host_git_enrichment*/ true, &permission_profile, WindowsSandboxLevel::Disabled, /*enforce_managed_network*/ false, @@ -521,6 +553,7 @@ fn turn_metadata_state_ignores_client_reserved_metadata_before_start() { /*thread_source*/ None, "turn-a".to_string(), cwd, + /*allow_host_git_enrichment*/ true, &permission_profile, WindowsSandboxLevel::Disabled, /*enforce_managed_network*/ false, @@ -575,6 +608,7 @@ fn turn_metadata_state_merges_client_metadata_without_replacing_reserved_fields( Some(ThreadSource::Feature("automation".to_string())), "turn-a".to_string(), cwd, + /*allow_host_git_enrichment*/ true, &permission_profile, WindowsSandboxLevel::Disabled, /*enforce_managed_network*/ false, @@ -697,6 +731,7 @@ fn turn_metadata_state_overlays_compaction_only_on_compaction_requests() { /*thread_source*/ None, "turn-a".to_string(), cwd, + /*allow_host_git_enrichment*/ true, &permission_profile, WindowsSandboxLevel::Disabled, /*enforce_managed_network*/ false, @@ -738,6 +773,33 @@ fn turn_metadata_state_overlays_compaction_only_on_compaction_requests() { assert!(regular_json.get("compaction").is_none()); } +#[tokio::test] +async fn pathless_turn_metadata_ignores_existing_host_git_repo() { + let (_temp_dir, repo_path) = create_clean_git_repo("host-repo").await; + let permission_profile = PermissionProfile::read_only(); + let state = TurnMetadataState::new( + "session-a".to_string(), + "thread-a".to_string(), + /*forked_from_thread_id*/ None, + /*parent_thread_id*/ None, + &SessionSource::Exec, + /*thread_source*/ None, + "turn-a".to_string(), + repo_path, + /*allow_host_git_enrichment*/ false, + &permission_profile, + WindowsSandboxLevel::Disabled, + /*enforce_managed_network*/ false, + ); + + state.spawn_git_enrichment_task(); + state.wait_for_git_enrichment_task_for_test().await; + + let header = test_turn_metadata_header(&state); + let json: Value = serde_json::from_str(&header).expect("json"); + assert!(json.get("workspaces").is_none()); +} + #[tokio::test] async fn turn_metadata_state_preserves_lineage_after_git_enrichment() { let (_temp_dir, repo_path) = create_clean_git_repo("repo").await; @@ -760,6 +822,7 @@ async fn turn_metadata_state_preserves_lineage_after_git_enrichment() { /*thread_source*/ None, "turn-a".to_string(), repo_path, + /*allow_host_git_enrichment*/ true, &permission_profile, WindowsSandboxLevel::Disabled, /*enforce_managed_network*/ false, diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index b95a003c5b..1e025ca16f 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -303,6 +303,7 @@ pub struct TestCodexBuilder { supports_openai_form_elicitation: bool, external_time_provider: Option>, code_mode_host_program: Option, + installation_id_override: Option>, } impl TestCodexBuilder { @@ -414,6 +415,11 @@ impl TestCodexBuilder { self } + pub fn with_installation_id(mut self, installation_id: Option) -> Self { + self.installation_id_override = Some(installation_id); + self + } + pub fn with_windows_cmd_shell(self) -> Self { if cfg!(windows) { self.with_user_shell(get_shell_by_model_provided_path(&PathBuf::from("cmd.exe"))) @@ -610,7 +616,10 @@ impl TestCodexBuilder { let auth = self.auth.clone(); let state_db = codex_core::init_state_db(&config).await; let thread_store = thread_store_from_config(&config, state_db.clone()); - let installation_id = resolve_installation_id(&config.codex_home).await?; + let installation_id = match &self.installation_id_override { + Some(installation_id) => installation_id.clone(), + None => Some(resolve_installation_id(&config.codex_home).await?), + }; let user_instructions_provider = self.user_instructions_provider.clone().unwrap_or_else(|| { Arc::new(CodexHomeUserInstructionsProvider::new( @@ -1255,6 +1264,7 @@ pub fn test_codex() -> TestCodexBuilder { supports_openai_form_elicitation: false, external_time_provider: None, code_mode_host_program: None, + installation_id_override: None, } } diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 4d298998af..e2cc613f6e 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -1575,7 +1575,7 @@ async fn prefers_apikey_when_config_prefers_apikey_even_with_chatgpt_tokens() { /*analytics_events_client*/ None, thread_store_from_config(&config, /*state_db*/ None), /*agent_graph_store*/ None, - installation_id, + Some(installation_id), /*attestation_provider*/ None, /*external_time_provider*/ None, ); diff --git a/codex-rs/mcp-server/src/message_processor.rs b/codex-rs/mcp-server/src/message_processor.rs index 98bb84b899..f5e7d497ab 100644 --- a/codex-rs/mcp-server/src/message_processor.rs +++ b/codex-rs/mcp-server/src/message_processor.rs @@ -82,7 +82,7 @@ impl MessageProcessor { /*analytics_events_client*/ None, codex_core::thread_store_from_config(config.as_ref(), state_db.clone()), codex_core::local_agent_graph_store_from_state_db(state_db.as_ref()), - installation_id, + Some(installation_id), /*attestation_provider*/ None, /*external_time_provider*/ None, )); diff --git a/codex-rs/memories/write/src/runtime.rs b/codex-rs/memories/write/src/runtime.rs index 903a1743ff..499ff6e983 100644 --- a/codex-rs/memories/write/src/runtime.rs +++ b/codex-rs/memories/write/src/runtime.rs @@ -8,7 +8,6 @@ use codex_core::ThreadManager; use codex_core::config::Config; use codex_core::content_items_to_text; use codex_core::detached_memory_responses_metadata; -use codex_core::resolve_installation_id; use codex_features::Feature; use codex_login::AuthManager; use codex_login::CodexAuth; @@ -71,6 +70,8 @@ pub(crate) struct MemoryStartupContext { thread_id: ThreadId, thread: Arc, thread_manager: Arc, + // Detached requests inherit the session decision; resolving here would write host-local state. + installation_id: Option, auth_manager: Arc, provider: SharedModelProvider, session_telemetry: SessionTelemetry, @@ -163,6 +164,7 @@ impl MemoryStartupContext { provider: SharedModelProvider, ) -> Self { let model = config.model.as_deref().unwrap_or("unknown"); + let installation_id = thread.installation_id().map(ToOwned::to_owned); let session_telemetry = build_session_telemetry( &auth_manager, thread_id, @@ -176,6 +178,7 @@ impl MemoryStartupContext { thread_id, thread, thread_manager, + installation_id, auth_manager, provider, session_telemetry, @@ -244,7 +247,6 @@ impl MemoryStartupContext { prompt: &Prompt, context: &StageOneRequestContext, ) -> anyhow::Result<(String, Option)> { - let installation_id = resolve_installation_id(&config.codex_home).await?; let config_snapshot = self.thread.config_snapshot().await; let session_source = config_snapshot.session_source; let session_id = SessionId::from(self.thread_id); @@ -269,12 +271,13 @@ impl MemoryStartupContext { let mut client_session = model_client.new_session(); let window_id = format!("{}:0", self.thread_id); let responses_metadata = detached_memory_responses_metadata( - installation_id, + self.installation_id.clone(), session_id_string, self.thread_id.to_string(), window_id, &session_source, &config.cwd, + self.thread_manager.has_local_runtime_paths(), /*sandbox*/ None, ) .await; diff --git a/codex-rs/memories/write/src/startup_tests.rs b/codex-rs/memories/write/src/startup_tests.rs index fad1ce14a4..c3eed16adc 100644 --- a/codex-rs/memories/write/src/startup_tests.rs +++ b/codex-rs/memories/write/src/startup_tests.rs @@ -349,6 +349,71 @@ async fn memories_startup_phase1_uses_live_thread_service_tier_and_detached_meta Ok(()) } +#[tokio::test] +async fn detached_memory_preserves_absent_installation_id() -> anyhow::Result<()> { + let server = start_mock_server().await; + let home = Arc::new(TempDir::new()?); + let test = test_codex() + .with_home(home) + .with_installation_id(/*installation_id*/ None) + .build(&server) + .await?; + let installation_id_path = test.config.codex_home.join("installation_id"); + assert!(!installation_id_path.exists()); + assert_eq!(test.codex.installation_id(), None); + + let config_snapshot = test.codex.config_snapshot().await; + let context = MemoryStartupContext::new( + Arc::clone(&test.thread_manager), + test.thread_manager.auth_manager(), + test.session_configured.thread_id, + Arc::clone(&test.codex), + &test.config, + config_snapshot.session_source, + ); + let request_context = context + .stage_one_request_context( + &test.config, + test.config.model.as_deref().unwrap_or("gpt-5.4-mini"), + ReasoningEffort::Low, + ) + .await; + let stage_one = mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-phase1-no-installation"), + ev_assistant_message("msg-phase1-no-installation", "phase1 complete"), + ev_completed("resp-phase1-no-installation"), + ]), + ) + .await; + + context + .stream_stage_one_prompt( + &test.config, + &codex_core::Prompt::default(), + &request_context, + ) + .await?; + + let request = wait_for_single_request(&stage_one).await; + let request_body = request.body_json(); + let client_metadata = request_body["client_metadata"] + .as_object() + .expect("client metadata"); + assert!(!client_metadata.contains_key("x-codex-installation-id")); + let turn_metadata: serde_json::Value = serde_json::from_str( + client_metadata["x-codex-turn-metadata"] + .as_str() + .expect("turn metadata"), + )?; + assert!(turn_metadata.get("installation_id").is_none()); + assert!(!installation_id_path.exists()); + + shutdown_test_codex(&test).await?; + Ok(()) +} + #[tokio::test] async fn memories_startup_phase1_provider_default_drives_request_model() -> anyhow::Result<()> { let server = start_mock_server().await; @@ -632,6 +697,14 @@ impl ModelProvider for MockMemoryModelProvider { self.delegate .models_manager(codex_home, config_model_catalog) } + + fn models_manager_without_disk_cache( + &self, + config_model_catalog: Option, + ) -> codex_models_manager::manager::SharedModelsManager { + self.delegate + .models_manager_without_disk_cache(config_model_catalog) + } } async fn seed_stage1_output( diff --git a/codex-rs/model-provider/src/amazon_bedrock/mod.rs b/codex-rs/model-provider/src/amazon_bedrock/mod.rs index 9fdf937bcb..19cb879621 100644 --- a/codex-rs/model-provider/src/amazon_bedrock/mod.rs +++ b/codex-rs/model-provider/src/amazon_bedrock/mod.rs @@ -172,6 +172,16 @@ impl ModelProvider for AmazonBedrockModelProvider { config_model_catalog.map_or_else(static_model_catalog, with_default_only_service_tier), )) } + + fn models_manager_without_disk_cache( + &self, + config_model_catalog: Option, + ) -> SharedModelsManager { + Arc::new(StaticModelsManager::new( + /*auth_manager*/ None, + config_model_catalog.map_or_else(static_model_catalog, with_default_only_service_tier), + )) + } } #[cfg(test)] diff --git a/codex-rs/model-provider/src/provider.rs b/codex-rs/model-provider/src/provider.rs index 6bc532ee50..f0ffb1b889 100644 --- a/codex-rs/model-provider/src/provider.rs +++ b/codex-rs/model-provider/src/provider.rs @@ -199,6 +199,20 @@ pub trait ModelProvider: fmt::Debug + Send + Sync { codex_home: PathBuf, config_model_catalog: Option, ) -> SharedModelsManager; + + /// Creates a model manager that retains fetched catalog state in memory only. + fn models_manager_without_disk_cache( + &self, + config_model_catalog: Option, + ) -> SharedModelsManager { + // Keep this capability source-compatible for external providers. Providers that can + // fetch a catalog without a disk cache should override it; the safe default is an + // authoritative in-memory catalog with no host filesystem dependency. + let model_catalog = config_model_catalog + .or_else(|| codex_models_manager::bundled_models_response().ok()) + .unwrap_or_default(); + Arc::new(StaticModelsManager::new(self.auth_manager(), model_catalog)) + } } pub type ModelProviderFuture<'a, T> = Pin + Send + 'a>>; @@ -334,6 +348,28 @@ impl ModelProvider for ConfiguredModelProvider { } } } + + fn models_manager_without_disk_cache( + &self, + config_model_catalog: Option, + ) -> SharedModelsManager { + match config_model_catalog { + Some(model_catalog) => Arc::new(StaticModelsManager::new( + self.auth_manager.clone(), + model_catalog, + )), + None => { + let endpoint = Arc::new(OpenAiModelsEndpoint::new( + self.info.clone(), + self.auth_manager.clone(), + )); + Arc::new(OpenAiModelsManager::new_without_disk_cache( + endpoint, + self.auth_manager.clone(), + )) + } + } + } } #[cfg(test)] @@ -656,7 +692,7 @@ mod tests { /*auth_manager*/ None, ); let manager = - provider.models_manager(test_codex_home(), /*config_model_catalog*/ None); + provider.models_manager_without_disk_cache(/*config_model_catalog*/ None); let catalog = manager .raw_model_catalog( diff --git a/codex-rs/models-manager/src/cache.rs b/codex-rs/models-manager/src/cache.rs index 812716d29c..a9d1e62f1a 100644 --- a/codex-rs/models-manager/src/cache.rs +++ b/codex-rs/models-manager/src/cache.rs @@ -14,7 +14,7 @@ use tracing::info; /// Manages loading and saving of models cache to disk. #[derive(Debug)] pub(crate) struct ModelsCacheManager { - cache_path: PathBuf, + cache_path: Option, cache_ttl: Duration, } @@ -22,15 +22,24 @@ impl ModelsCacheManager { /// Create a new cache manager with the given path and TTL. pub(crate) fn new(cache_path: PathBuf, cache_ttl: Duration) -> Self { Self { - cache_path, + cache_path: Some(cache_path), + cache_ttl, + } + } + + /// Create a cache manager that retains model state in memory only. + pub(crate) fn without_disk_cache(cache_ttl: Duration) -> Self { + Self { + cache_path: None, cache_ttl, } } /// Attempt to load a fresh cache entry. Returns `None` if the cache doesn't exist or is stale. pub(crate) async fn load_fresh(&self, expected_version: &str) -> Option { + let cache_path = self.cache_path.as_ref()?; info!( - cache_path = %self.cache_path.display(), + cache_path = %cache_path.display(), expected_version, "models cache: attempting load_fresh" ); @@ -42,14 +51,14 @@ impl ModelsCacheManager { } }; info!( - cache_path = %self.cache_path.display(), + cache_path = %cache_path.display(), cached_version = ?cache.client_version, fetched_at = %cache.fetched_at, "models cache: loaded cache file" ); if cache.client_version.as_deref() != Some(expected_version) { info!( - cache_path = %self.cache_path.display(), + cache_path = %cache_path.display(), expected_version, cached_version = ?cache.client_version, "models cache: cache version mismatch" @@ -58,7 +67,7 @@ impl ModelsCacheManager { } if !cache.is_fresh(self.cache_ttl) { info!( - cache_path = %self.cache_path.display(), + cache_path = %cache_path.display(), cache_ttl_secs = self.cache_ttl.as_secs(), fetched_at = %cache.fetched_at, "models cache: cache is stale" @@ -66,7 +75,7 @@ impl ModelsCacheManager { return None; } info!( - cache_path = %self.cache_path.display(), + cache_path = %cache_path.display(), cache_ttl_secs = self.cache_ttl.as_secs(), "models cache: cache hit" ); @@ -80,6 +89,9 @@ impl ModelsCacheManager { etag: Option, client_version: String, ) { + if self.cache_path.is_none() { + return; + } let cache = ModelsCache { fetched_at: Utc::now(), etag, @@ -93,6 +105,9 @@ impl ModelsCacheManager { /// Renew the cache TTL by updating the fetched_at timestamp to now. pub(crate) async fn renew_cache_ttl(&self) -> io::Result<()> { + if self.cache_path.is_none() { + return Ok(()); + } let mut cache = match self.load().await? { Some(cache) => cache, None => return Err(io::Error::new(ErrorKind::NotFound, "cache not found")), @@ -102,7 +117,10 @@ impl ModelsCacheManager { } async fn load(&self) -> io::Result> { - match fs::read(&self.cache_path).await { + let Some(cache_path) = self.cache_path.as_ref() else { + return Ok(None); + }; + match fs::read(cache_path).await { Ok(contents) => { let cache = serde_json::from_slice(&contents) .map_err(|err| io::Error::new(ErrorKind::InvalidData, err.to_string()))?; @@ -114,12 +132,15 @@ impl ModelsCacheManager { } async fn save_internal(&self, cache: &ModelsCache) -> io::Result<()> { - if let Some(parent) = self.cache_path.parent() { + let Some(cache_path) = self.cache_path.as_ref() else { + return Ok(()); + }; + if let Some(parent) = cache_path.parent() { fs::create_dir_all(parent).await?; } let json = serde_json::to_vec_pretty(cache) .map_err(|err| io::Error::new(ErrorKind::InvalidData, err.to_string()))?; - fs::write(&self.cache_path, json).await + fs::write(cache_path, json).await } #[cfg(test)] diff --git a/codex-rs/models-manager/src/manager.rs b/codex-rs/models-manager/src/manager.rs index 3439ceafc5..6b949f2b39 100644 --- a/codex-rs/models-manager/src/manager.rs +++ b/codex-rs/models-manager/src/manager.rs @@ -235,7 +235,30 @@ impl OpenAiModelsManager { auth_manager: Option>, ) -> Self { let cache_path = codex_home.join(MODEL_CACHE_FILE); - let cache_manager = ModelsCacheManager::new(cache_path, DEFAULT_MODEL_CACHE_TTL); + Self::new_with_cache_manager( + ModelsCacheManager::new(cache_path, DEFAULT_MODEL_CACHE_TTL), + endpoint_client, + auth_manager, + ) + } + + /// Construct an OpenAI-compatible model manager with memory-only catalog state. + pub fn new_without_disk_cache( + endpoint_client: Arc, + auth_manager: Option>, + ) -> Self { + Self::new_with_cache_manager( + ModelsCacheManager::without_disk_cache(DEFAULT_MODEL_CACHE_TTL), + endpoint_client, + auth_manager, + ) + } + + fn new_with_cache_manager( + cache_manager: ModelsCacheManager, + endpoint_client: Arc, + auth_manager: Option>, + ) -> Self { let remote_models = load_remote_models_from_file().unwrap_or_default(); Self { remote_models: RwLock::new(remote_models), diff --git a/codex-rs/models-manager/src/manager_tests.rs b/codex-rs/models-manager/src/manager_tests.rs index e74c7cac4c..51cbc66526 100644 --- a/codex-rs/models-manager/src/manager_tests.rs +++ b/codex-rs/models-manager/src/manager_tests.rs @@ -210,6 +210,24 @@ fn static_manager_for_tests(model_catalog: ModelsResponse) -> StaticModelsManage StaticModelsManager::new(/*auth_manager*/ None, model_catalog) } +#[tokio::test] +async fn manager_without_disk_cache_fetches_and_retains_models_in_memory() { + let remote_models = vec![remote_model("remote", "Remote", /*priority*/ 0)]; + let endpoint = TestModelsEndpoint::new(vec![remote_models.clone()]); + let manager = OpenAiModelsManager::new_without_disk_cache( + endpoint.clone(), + Some(AuthManager::from_auth_for_testing( + CodexAuth::create_dummy_chatgpt_auth_for_testing(), + )), + ); + + let catalog = manager.raw_model_catalog(RefreshStrategy::Online).await; + + assert_eq!(catalog.models, remote_models); + assert_eq!(manager.get_remote_models().await, remote_models); + assert_eq!(endpoint.fetch_count(), 1); +} + async fn chatgpt_auth_tokens_for_tests(codex_home: &Path) -> CodexAuth { let auth_dot_json = codex_login::AuthDotJson { auth_mode: Some(AuthMode::ChatgptAuthTokens), diff --git a/codex-rs/rmcp-client/src/oauth.rs b/codex-rs/rmcp-client/src/oauth.rs index 601d0f45f4..4d66348645 100644 --- a/codex-rs/rmcp-client/src/oauth.rs +++ b/codex-rs/rmcp-client/src/oauth.rs @@ -244,6 +244,9 @@ pub fn save_oauth_tokens( ) -> Result<()> { let keyring_store = DefaultKeyringStore; match store_mode { + OAuthCredentialsStoreMode::Disabled => { + anyhow::bail!("MCP OAuth credential persistence is disabled for this runtime") + } OAuthCredentialsStoreMode::Auto => save_oauth_tokens_with_keyring_with_fallback_to_file( &keyring_store, keyring_backend_kind, @@ -404,6 +407,10 @@ fn delete_oauth_tokens_from_keyring_and_file( server_name: &str, url: &str, ) -> Result { + if store_mode == OAuthCredentialsStoreMode::Disabled { + return Ok(false); + } + let key = compute_store_key(server_name, url)?; let keyring_result = delete_oauth_tokens_from_keyring(keyring_store, keyring_backend_kind, server_name, url); @@ -416,7 +423,7 @@ fn delete_oauth_tokens_from_keyring_and_file( OAuthCredentialsStoreMode::Auto | OAuthCredentialsStoreMode::Keyring => { return Err(error).context("failed to delete OAuth tokens from keyring"); } - OAuthCredentialsStoreMode::File => false, + OAuthCredentialsStoreMode::File | OAuthCredentialsStoreMode::Disabled => false, } } }; @@ -1016,6 +1023,44 @@ mod tests { Ok(()) } + #[test] + fn disabled_store_mode_does_not_read_or_mutate_file_credentials() -> Result<()> { + let _env = TempCodexHome::new(); + let tokens = sample_tokens(); + super::save_oauth_tokens_to_file(&tokens)?; + let fallback_path = super::fallback_file_path()?; + let original_file = fs::read(&fallback_path)?; + + let loaded = super::load_oauth_tokens( + &tokens.server_name, + &tokens.url, + OAuthCredentialsStoreMode::Disabled, + AuthKeyringBackendKind::Direct, + )?; + let save_error = super::save_oauth_tokens( + &tokens.server_name, + &tokens, + OAuthCredentialsStoreMode::Disabled, + AuthKeyringBackendKind::Direct, + ) + .expect_err("disabled storage must reject credential persistence"); + let removed = super::delete_oauth_tokens( + &tokens.server_name, + &tokens.url, + OAuthCredentialsStoreMode::Disabled, + AuthKeyringBackendKind::Direct, + )?; + + assert_eq!(loaded, None); + assert_eq!( + save_error.to_string(), + "MCP OAuth credential persistence is disabled for this runtime" + ); + assert!(!removed); + assert_eq!(fs::read(fallback_path)?, original_file); + Ok(()) + } + #[test] fn save_oauth_tokens_with_secrets_backend_writes_encrypted_storage() -> Result<()> { let env = TempCodexHome::new(); diff --git a/codex-rs/thread-manager-sample/src/main.rs b/codex-rs/thread-manager-sample/src/main.rs index c35daaa497..c5aae190c8 100644 --- a/codex-rs/thread-manager-sample/src/main.rs +++ b/codex-rs/thread-manager-sample/src/main.rs @@ -140,7 +140,7 @@ async fn run_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { /*analytics_events_client*/ None, Arc::clone(&thread_store), local_agent_graph_store_from_state_db(state_db.as_ref()), - installation_id, + Some(installation_id), /*attestation_provider*/ None, /*external_time_provider*/ None, ); diff --git a/codex-rs/thread-store/src/in_memory.rs b/codex-rs/thread-store/src/in_memory.rs index 0d6ea80f2a..fe21594725 100644 --- a/codex-rs/thread-store/src/in_memory.rs +++ b/codex-rs/thread-store/src/in_memory.rs @@ -612,6 +612,10 @@ impl ThreadStore for InMemoryThreadStore { self } + fn requires_local_runtime_paths(&self) -> bool { + false + } + fn create_thread(&self, params: CreateThreadParams) -> ThreadStoreFuture<'_, ()> { Box::pin(InMemoryThreadStore::create_thread(self, params)) } diff --git a/codex-rs/thread-store/src/store.rs b/codex-rs/thread-store/src/store.rs index 67ee603f49..0b008bb347 100644 --- a/codex-rs/thread-store/src/store.rs +++ b/codex-rs/thread-store/src/store.rs @@ -34,6 +34,14 @@ pub trait ThreadStore: Any + Send + Sync { /// Return this store as [`Any`] for implementation-owned escape hatches. fn as_any(&self) -> &dyn Any; + /// Whether constructing and operating this store requires host-local runtime paths. + /// + /// The default fails closed. Stores backed entirely by remote or in-memory + /// state must opt out explicitly. + fn requires_local_runtime_paths(&self) -> bool { + true + } + /// Returns the history mode to use when history does not carry a persisted mode. /// /// The default is legacy so existing stores stay compatible. Stores whose durable contract is