diff --git a/.github/scripts/run-bazel-ci.sh b/.github/scripts/run-bazel-ci.sh index fefca90b3a..c8c09f406a 100755 --- a/.github/scripts/run-bazel-ci.sh +++ b/.github/scripts/run-bazel-ci.sh @@ -94,6 +94,11 @@ print_bazel_test_log_tails() { local rel_path="${target#//}" rel_path="${rel_path/://}" local test_log="${testlogs_dir}/${rel_path}/test.log" + local reported_test_log + reported_test_log="$(grep -F "FAIL: ${target} " "$console_log" | sed -nE 's#.* \(see ([^)]+/test\.log)\).*#\1#p' | head -n 1 || true)" + if [[ -n "$reported_test_log" ]]; then + test_log="$reported_test_log" + fi echo "::group::Bazel test log tail for ${target}" if [[ -f "$test_log" ]]; then diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 2d39c42df4..1be5b45b6a 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1446,6 +1446,7 @@ dependencies = [ "codex-cloud-requirements", "codex-config", "codex-core", + "codex-core-plugins", "codex-exec-server", "codex-features", "codex-feedback", @@ -1913,6 +1914,7 @@ dependencies = [ "codex-code-mode", "codex-config", "codex-connectors", + "codex-core-plugins", "codex-core-skills", "codex-exec-server", "codex-execpolicy", @@ -2013,6 +2015,33 @@ dependencies = [ "zstd 0.13.3", ] +[[package]] +name = "codex-core-plugins" +version = "0.0.0" +dependencies = [ + "codex-app-server-protocol", + "codex-config", + "codex-core-skills", + "codex-exec-server", + "codex-git-utils", + "codex-login", + "codex-plugin", + "codex-protocol", + "codex-utils-absolute-path", + "codex-utils-plugins", + "dirs", + "pretty_assertions", + "reqwest", + "serde", + "serde_json", + "tempfile", + "thiserror 2.0.18", + "tokio", + "toml 0.9.11+spec-1.1.0", + "tracing", + "url", +] + [[package]] name = "codex-core-skills" version = "0.0.0" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 47c0d348bf..e7867304e9 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -28,6 +28,7 @@ members = [ "shell-escalation", "skills", "core", + "core-plugins", "core-skills", "hooks", "instructions", @@ -128,6 +129,7 @@ codex-code-mode = { path = "code-mode" } codex-config = { path = "config" } codex-connectors = { path = "connectors" } codex-core = { path = "core" } +codex-core-plugins = { path = "core-plugins" } codex-core-skills = { path = "core-skills" } codex-exec = { path = "exec" } codex-exec-server = { path = "exec-server" } diff --git a/codex-rs/app-server/Cargo.toml b/codex-rs/app-server/Cargo.toml index 408f39fafe..097abdb713 100644 --- a/codex-rs/app-server/Cargo.toml +++ b/codex-rs/app-server/Cargo.toml @@ -34,6 +34,7 @@ codex-arg0 = { workspace = true } codex-cloud-requirements = { workspace = true } codex-config = { workspace = true } codex-core = { workspace = true } +codex-core-plugins = { workspace = true } codex-exec-server = { workspace = true } codex-features = { workspace = true } codex-git-utils = { workspace = true } diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index f805ec3ff6..4d12df89fa 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -186,6 +186,7 @@ use codex_app_server_protocol::ThreadUnsubscribeStatus; use codex_app_server_protocol::Turn; use codex_app_server_protocol::TurnError; use codex_app_server_protocol::TurnInterruptParams; +use codex_app_server_protocol::TurnInterruptResponse; use codex_app_server_protocol::TurnStartParams; use codex_app_server_protocol::TurnStartResponse; use codex_app_server_protocol::TurnStatus; @@ -211,6 +212,7 @@ use codex_core::SteerInputError; use codex_core::ThreadConfigSnapshot; use codex_core::ThreadManager; use codex_core::append_thread_name; +use codex_core::clear_memory_roots_contents; use codex_core::config::Config; use codex_core::config::ConfigOverrides; use codex_core::config::NetworkProxyAuditMetadata; @@ -232,22 +234,23 @@ use codex_core::find_thread_names_by_ids; use codex_core::find_thread_path_by_id_str; use codex_core::path_utils; use codex_core::plugins::MarketplaceAddError; -use codex_core::plugins::MarketplaceError; -use codex_core::plugins::MarketplacePluginSource; use codex_core::plugins::OPENAI_CURATED_MARKETPLACE_NAME; use codex_core::plugins::PluginInstallError as CorePluginInstallError; use codex_core::plugins::PluginInstallRequest; use codex_core::plugins::PluginReadRequest; use codex_core::plugins::PluginUninstallError as CorePluginUninstallError; use codex_core::plugins::add_marketplace as add_marketplace_to_codex_home; -use codex_core::plugins::load_plugin_apps; -use codex_core::plugins::load_plugin_mcp_servers; use codex_core::read_head_for_summary; use codex_core::read_session_meta_line; use codex_core::sandboxing::SandboxPermissions; use codex_core::windows_sandbox::WindowsSandboxLevelExt; use codex_core::windows_sandbox::WindowsSandboxSetupMode as CoreWindowsSandboxSetupMode; use codex_core::windows_sandbox::WindowsSandboxSetupRequest; +use codex_core_plugins::loader::load_plugin_apps; +use codex_core_plugins::loader::load_plugin_mcp_servers; +use codex_core_plugins::manifest::PluginManifestInterface; +use codex_core_plugins::marketplace::MarketplaceError; +use codex_core_plugins::marketplace::MarketplacePluginSource; use codex_exec_server::LOCAL_FS; use codex_features::FEATURES; use codex_features::Feature; @@ -3078,48 +3081,12 @@ impl CodexMessageProcessor { return; } - let memory_root = self.config.codex_home.join("memories"); - let memory_extensions_root = self.config.codex_home.join("memories_extensions"); - let clear_memory_root_result: std::io::Result<()> = async { - for directory in [memory_root.as_path(), memory_extensions_root.as_path()] { - match tokio::fs::symlink_metadata(directory).await { - Ok(metadata) if metadata.file_type().is_symlink() => { - return Err(std::io::Error::new( - std::io::ErrorKind::InvalidInput, - format!( - "refusing to clear symlinked memory root {}", - directory.display() - ), - )); - } - Ok(_) => {} - Err(err) if err.kind() == std::io::ErrorKind::NotFound => {} - Err(err) => return Err(err), - } - - tokio::fs::create_dir_all(directory).await?; - let mut entries = tokio::fs::read_dir(directory).await?; - while let Some(entry) = entries.next_entry().await? { - let path = entry.path(); - let file_type = entry.file_type().await?; - if file_type.is_dir() { - tokio::fs::remove_dir_all(path).await?; - } else { - tokio::fs::remove_file(path).await?; - } - } - } - - Ok(()) - } - .await; - - if let Err(err) = clear_memory_root_result { + if let Err(err) = clear_memory_roots_contents(&self.config.codex_home).await { self.send_internal_error( request_id, format!( - "failed to clear memory directory {}: {err}", - memory_root.display() + "failed to clear memory directories under {}: {err}", + self.config.codex_home.display() ), ) .await; @@ -7558,9 +7525,12 @@ impl CodexMessageProcessor { async fn turn_interrupt(&self, request_id: ConnectionRequestId, params: TurnInterruptParams) { let TurnInterruptParams { thread_id, turn_id } = params; - self.outgoing - .record_request_turn_id(&request_id, &turn_id) - .await; + let is_startup_interrupt = turn_id.is_empty(); + if !is_startup_interrupt { + self.outgoing + .record_request_turn_id(&request_id, &turn_id) + .await; + } let (thread_uuid, thread) = match self.load_thread(&thread_id).await { Ok(v) => v, @@ -7570,21 +7540,48 @@ impl CodexMessageProcessor { } }; - let request = request_id.clone(); - - // Record the pending interrupt so we can reply when TurnAborted arrives. - { + // Record turn interrupts so we can reply when TurnAborted arrives. Startup + // interrupts do not have a turn and are acknowledged after submission. + if !is_startup_interrupt { let thread_state = self.thread_state_manager.thread_state(thread_uuid).await; let mut thread_state = thread_state.lock().await; thread_state .pending_interrupts - .push((request, ApiVersion::V2)); + .push((request_id.clone(), ApiVersion::V2)); } - // Submit the interrupt; we'll respond upon TurnAborted. - let _ = self + // Submit the interrupt. Turn interrupts respond upon TurnAborted; startup + // interrupts respond here because startup cancellation has no turn event. + let submit_result = self .submit_core_op(&request_id, thread.as_ref(), Op::Interrupt) .await; + match submit_result { + Ok(_) if is_startup_interrupt => { + self.outgoing + .send_response(request_id, TurnInterruptResponse {}) + .await; + } + Ok(_) => {} + Err(err) => { + if !is_startup_interrupt { + let thread_state = self.thread_state_manager.thread_state(thread_uuid).await; + let mut thread_state = thread_state.lock().await; + thread_state + .pending_interrupts + .retain(|(pending_request_id, _)| pending_request_id != &request_id); + } + let interrupt_target = if is_startup_interrupt { + "startup" + } else { + "turn" + }; + self.send_internal_error( + request_id, + format!("failed to interrupt {interrupt_target}: {err}"), + ) + .await; + } + } } async fn ensure_conversation_listener( @@ -8834,9 +8831,7 @@ fn plugin_skills_to_info( .collect() } -fn plugin_interface_to_info( - interface: codex_core::plugins::PluginManifestInterface, -) -> PluginInterface { +fn plugin_interface_to_info(interface: PluginManifestInterface) -> PluginInterface { PluginInterface { display_name: interface.display_name, short_description: interface.short_description, diff --git a/codex-rs/app-server/src/config_api.rs b/codex-rs/app-server/src/config_api.rs index 4d0b450d73..2bc7447300 100644 --- a/codex-rs/app-server/src/config_api.rs +++ b/codex-rs/app-server/src/config_api.rs @@ -27,8 +27,8 @@ use codex_core::config_loader::LoaderOverrides; use codex_core::config_loader::ResidencyRequirement as CoreResidencyRequirement; use codex_core::config_loader::SandboxModeRequirement as CoreSandboxModeRequirement; use codex_core::plugins::PluginId; -use codex_core::plugins::collect_plugin_enabled_candidates; -use codex_core::plugins::installed_plugin_telemetry_metadata; +use codex_core_plugins::loader::installed_plugin_telemetry_metadata; +use codex_core_plugins::toggles::collect_plugin_enabled_candidates; use codex_features::canonical_feature_for_key; use codex_features::feature_for_key; use codex_protocol::config_types::WebSearchMode; diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 023d744502..6221cebee5 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -725,8 +725,6 @@ impl MessageProcessor { session: Arc, request_context: RequestContext, ) { - let connection_id = connection_request_id.connection_id; - if !session.initialized() { let error = JSONRPCErrorError { code: INVALID_REQUEST_ERROR_CODE, @@ -748,6 +746,7 @@ impl MessageProcessor { self.outgoing.send_error(connection_request_id, error).await; return; } + let connection_id = connection_request_id.connection_id; if self.config.features.enabled(Feature::GeneralAnalytics) && let ClientRequest::TurnStart { request_id, .. } | ClientRequest::TurnSteer { request_id, .. } = &codex_request diff --git a/codex-rs/app-server/src/transport/remote_control/mod.rs b/codex-rs/app-server/src/transport/remote_control/mod.rs index 1ea89bb643..c014c7a2c9 100644 --- a/codex-rs/app-server/src/transport/remote_control/mod.rs +++ b/codex-rs/app-server/src/transport/remote_control/mod.rs @@ -4,7 +4,6 @@ mod protocol; mod websocket; use crate::transport::remote_control::websocket::RemoteControlWebsocket; -use crate::transport::remote_control::websocket::load_remote_control_auth; pub use self::protocol::ClientId; use self::protocol::ServerEvent; @@ -59,9 +58,6 @@ pub(crate) async fn start_remote_control( } else { None }; - if initial_enabled { - validate_remote_control_auth(&auth_manager).await?; - } let (enabled_tx, enabled_rx) = watch::channel(initial_enabled); let join_handle = tokio::spawn(async move { @@ -86,15 +82,5 @@ pub(crate) async fn start_remote_control( )) } -pub(crate) async fn validate_remote_control_auth( - auth_manager: &Arc, -) -> io::Result<()> { - match load_remote_control_auth(auth_manager).await { - Ok(_) => Ok(()), - Err(err) if err.kind() == io::ErrorKind::WouldBlock => Ok(()), - Err(err) => Err(err), - } -} - #[cfg(test)] mod tests; diff --git a/codex-rs/app-server/src/transport/remote_control/tests.rs b/codex-rs/app-server/src/transport/remote_control/tests.rs index 2dd687a3d1..d702a99c2c 100644 --- a/codex-rs/app-server/src/transport/remote_control/tests.rs +++ b/codex-rs/app-server/src/transport/remote_control/tests.rs @@ -480,6 +480,44 @@ async fn remote_control_start_allows_remote_control_invalid_url_when_disabled() .expect("remote control task should join"); } +#[tokio::test] +async fn remote_control_start_allows_missing_auth_when_enabled() { + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("listener should bind"); + let remote_control_url = remote_control_url_for_listener(&listener); + let codex_home = TempDir::new().expect("temp dir should create"); + let auth_manager = AuthManager::shared( + codex_home.path().to_path_buf(), + /*enable_codex_api_key_env*/ false, + AuthCredentialsStoreMode::File, + ); + let (transport_event_tx, _transport_event_rx) = + mpsc::channel::(CHANNEL_CAPACITY); + let shutdown_token = CancellationToken::new(); + let (remote_task, _remote_handle) = start_remote_control( + remote_control_url, + /*state_db*/ None, + auth_manager, + transport_event_tx, + shutdown_token.clone(), + /*app_server_client_name_rx*/ None, + /*initial_enabled*/ true, + ) + .await + .expect("remote control should start before ChatGPT auth is available"); + + timeout(Duration::from_millis(100), listener.accept()) + .await + .expect_err("remote control should wait for auth before connecting"); + + shutdown_token.cancel(); + timeout(Duration::from_secs(1), remote_task) + .await + .expect("remote control task should stop") + .expect("remote control task should join"); +} + #[tokio::test] async fn remote_control_handle_set_enabled_stops_and_restarts_connections() { let listener = TcpListener::bind("127.0.0.1:0") diff --git a/codex-rs/app-server/src/transport/remote_control/websocket.rs b/codex-rs/app-server/src/transport/remote_control/websocket.rs index 82e5f35b33..d813be1f6a 100644 --- a/codex-rs/app-server/src/transport/remote_control/websocket.rs +++ b/codex-rs/app-server/src/transport/remote_control/websocket.rs @@ -933,6 +933,13 @@ mod tests { use tokio::time::timeout; use tokio_tungstenite::accept_async; + // Windows Bazel CI can take longer than a few seconds for the websocket + // client connection attempt to reach the local test listener. + #[cfg(windows)] + const TEST_HTTP_ACCEPT_TIMEOUT: Duration = Duration::from_secs(30); + #[cfg(not(windows))] + const TEST_HTTP_ACCEPT_TIMEOUT: Duration = Duration::from_secs(5); + async fn remote_control_state_runtime(codex_home: &TempDir) -> Arc { StateRuntime::init(codex_home.path().to_path_buf(), "test-provider".to_string()) .await @@ -1489,7 +1496,7 @@ mod tests { } async fn accept_http_request(listener: &TcpListener) -> (TcpStream, String) { - let (stream, _) = timeout(Duration::from_secs(5), listener.accept()) + let (stream, _) = timeout(TEST_HTTP_ACCEPT_TIMEOUT, listener.accept()) .await .expect("HTTP request should arrive in time") .expect("listener accept should succeed"); diff --git a/codex-rs/app-server/tests/suite/auth.rs b/codex-rs/app-server/tests/suite/auth.rs index e6134e4802..1e60871012 100644 --- a/codex-rs/app-server/tests/suite/auth.rs +++ b/codex-rs/app-server/tests/suite/auth.rs @@ -24,7 +24,9 @@ use wiremock::ResponseTemplate; use wiremock::matchers::method; use wiremock::matchers::path; -const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); +// Bazel CI can spend tens of seconds starting app-server subprocesses or +// processing auth RPCs under load. +const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(60); fn create_config_toml_custom_provider( codex_home: &Path, diff --git a/codex-rs/app-server/tests/suite/fuzzy_file_search.rs b/codex-rs/app-server/tests/suite/fuzzy_file_search.rs index 1520d99e3e..f508e0c987 100644 --- a/codex-rs/app-server/tests/suite/fuzzy_file_search.rs +++ b/codex-rs/app-server/tests/suite/fuzzy_file_search.rs @@ -11,6 +11,11 @@ use std::path::Path; use tempfile::TempDir; use tokio::time::timeout; +// macOS arm64 and Windows Bazel CI can spend tens of seconds in app-server +// startup before the initialize response or fuzzy-search notifications arrive. +#[cfg(any(target_os = "macos", windows))] +const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(60); +#[cfg(not(any(target_os = "macos", windows)))] const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); const SHORT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_millis(500); const STOP_GRACE_PERIOD: std::time::Duration = std::time::Duration::from_millis(250); diff --git a/codex-rs/app-server/tests/suite/v2/app_list.rs b/codex-rs/app-server/tests/suite/v2/app_list.rs index dbe61524f5..78a915d178 100644 --- a/codex-rs/app-server/tests/suite/v2/app_list.rs +++ b/codex-rs/app-server/tests/suite/v2/app_list.rs @@ -56,7 +56,9 @@ use tokio::net::TcpListener; use tokio::task::JoinHandle; use tokio::time::timeout; -const DEFAULT_TIMEOUT: Duration = Duration::from_secs(10); +// Bazel CI can spend tens of seconds starting app-server subprocesses or +// processing app-list RPCs under load. +const DEFAULT_TIMEOUT: Duration = Duration::from_secs(60); #[tokio::test] async fn list_apps_returns_empty_when_connectors_disabled() -> Result<()> { diff --git a/codex-rs/app-server/tests/suite/v2/client_metadata.rs b/codex-rs/app-server/tests/suite/v2/client_metadata.rs index c85febd7d4..8d68888e7e 100644 --- a/codex-rs/app-server/tests/suite/v2/client_metadata.rs +++ b/codex-rs/app-server/tests/suite/v2/client_metadata.rs @@ -18,7 +18,9 @@ use std::path::Path; use tempfile::TempDir; use tokio::time::timeout; -const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); +// Bazel CI can spend tens of seconds starting app-server subprocesses or +// processing turn RPCs under load. +const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(60); #[tokio::test] async fn turn_start_forwards_client_metadata_to_responses_request_v2() -> Result<()> { diff --git a/codex-rs/app-server/tests/suite/v2/collaboration_mode_list.rs b/codex-rs/app-server/tests/suite/v2/collaboration_mode_list.rs index 7c36827e6d..3c8a3e573e 100644 --- a/codex-rs/app-server/tests/suite/v2/collaboration_mode_list.rs +++ b/codex-rs/app-server/tests/suite/v2/collaboration_mode_list.rs @@ -21,7 +21,9 @@ use pretty_assertions::assert_eq; use tempfile::TempDir; use tokio::time::timeout; -const DEFAULT_TIMEOUT: Duration = Duration::from_secs(10); +// Bazel CI can spend tens of seconds starting app-server subprocesses or +// processing list RPCs under load. +const DEFAULT_TIMEOUT: Duration = Duration::from_secs(60); /// Confirms the server returns the default collaboration mode presets in a stable order. #[tokio::test] diff --git a/codex-rs/app-server/tests/suite/v2/compaction.rs b/codex-rs/app-server/tests/suite/v2/compaction.rs index e7661546ac..44b5dd6dc6 100644 --- a/codex-rs/app-server/tests/suite/v2/compaction.rs +++ b/codex-rs/app-server/tests/suite/v2/compaction.rs @@ -38,6 +38,11 @@ use std::collections::BTreeMap; use tempfile::TempDir; use tokio::time::timeout; +// macOS and Windows Bazel CI can spend tens of seconds starting app-server +// subprocesses or processing test RPCs under load. +#[cfg(any(target_os = "macos", windows))] +const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(60); +#[cfg(not(any(target_os = "macos", windows)))] const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); const AUTO_COMPACT_LIMIT: i64 = 1_000; const COMPACT_PROMPT: &str = "Summarize the conversation."; diff --git a/codex-rs/app-server/tests/suite/v2/config_rpc.rs b/codex-rs/app-server/tests/suite/v2/config_rpc.rs index 5c04cc3c44..b5f795740c 100644 --- a/codex-rs/app-server/tests/suite/v2/config_rpc.rs +++ b/codex-rs/app-server/tests/suite/v2/config_rpc.rs @@ -33,7 +33,9 @@ use serde_json::json; use tempfile::TempDir; use tokio::time::timeout; -const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); +// Bazel CI can spend tens of seconds starting app-server subprocesses or +// processing config RPCs under load. +const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(60); fn write_config(codex_home: &TempDir, contents: &str) -> Result<()> { Ok(std::fs::write( diff --git a/codex-rs/app-server/tests/suite/v2/connection_handling_websocket.rs b/codex-rs/app-server/tests/suite/v2/connection_handling_websocket.rs index 05d26d55e3..0cc8b51e56 100644 --- a/codex-rs/app-server/tests/suite/v2/connection_handling_websocket.rs +++ b/codex-rs/app-server/tests/suite/v2/connection_handling_websocket.rs @@ -47,6 +47,12 @@ use tokio_tungstenite::tungstenite::http::HeaderValue; use tokio_tungstenite::tungstenite::http::header::AUTHORIZATION; use tokio_tungstenite::tungstenite::http::header::ORIGIN; +// macOS and Windows CI can spend tens of seconds starting the app-server test +// binary under Bazel before it accepts JSON-RPC or reports its websocket bind +// address. +#[cfg(any(target_os = "macos", windows))] +pub(super) const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(60); +#[cfg(not(any(target_os = "macos", windows)))] pub(super) const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(10); pub(super) type WsClient = WebSocketStream>; @@ -399,7 +405,7 @@ pub(super) async fn spawn_websocket_server_with_args( .take() .context("failed to capture websocket app-server stderr")?; let mut stderr_reader = BufReader::new(stderr).lines(); - let deadline = Instant::now() + Duration::from_secs(10); + let deadline = Instant::now() + DEFAULT_READ_TIMEOUT; let bind_addr = loop { let line = timeout( deadline.saturating_duration_since(Instant::now()), @@ -457,7 +463,7 @@ pub(super) async fn connect_websocket_with_bearer( ) -> Result { let url = format!("ws://{}", connectable_bind_addr(bind_addr)); let request = websocket_request(url.as_str(), bearer_token, /*origin*/ None)?; - let deadline = Instant::now() + Duration::from_secs(10); + let deadline = Instant::now() + DEFAULT_READ_TIMEOUT; loop { match connect_async(request.clone()).await { Ok((stream, _response)) => return Ok(stream), @@ -524,7 +530,7 @@ async fn run_websocket_server_to_completion_with_args( .stderr(Stdio::piped()) .env("CODEX_HOME", codex_home) .env("RUST_LOG", "debug"); - timeout(Duration::from_secs(10), cmd.output()) + timeout(DEFAULT_READ_TIMEOUT, cmd.output()) .await .context("timed out waiting for websocket app-server to exit")? .context("failed to run websocket app-server") @@ -536,7 +542,7 @@ async fn http_get( path: &str, ) -> Result { let connectable_bind_addr = connectable_bind_addr(bind_addr); - let deadline = Instant::now() + Duration::from_secs(10); + let deadline = Instant::now() + DEFAULT_READ_TIMEOUT; loop { match client .get(format!("http://{connectable_bind_addr}{path}")) diff --git a/codex-rs/app-server/tests/suite/v2/dynamic_tools.rs b/codex-rs/app-server/tests/suite/v2/dynamic_tools.rs index 0ab3f47235..0a3315a07a 100644 --- a/codex-rs/app-server/tests/suite/v2/dynamic_tools.rs +++ b/codex-rs/app-server/tests/suite/v2/dynamic_tools.rs @@ -34,6 +34,11 @@ use tempfile::TempDir; use tokio::time::timeout; use wiremock::MockServer; +// macOS and Windows Bazel CI can spend tens of seconds starting app-server +// subprocesses or processing test RPCs under load. +#[cfg(any(target_os = "macos", windows))] +const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(60); +#[cfg(not(any(target_os = "macos", windows)))] const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(10); /// Ensures dynamic tool specs are serialized into the model request payload. diff --git a/codex-rs/app-server/tests/suite/v2/fs.rs b/codex-rs/app-server/tests/suite/v2/fs.rs index c7f28f09f5..642844eb92 100644 --- a/codex-rs/app-server/tests/suite/v2/fs.rs +++ b/codex-rs/app-server/tests/suite/v2/fs.rs @@ -27,6 +27,11 @@ use std::os::unix::fs::symlink; #[cfg(unix)] use std::process::Command; +// macOS and Windows Bazel CI can spend tens of seconds starting app-server +// subprocesses or processing test RPCs under load. +#[cfg(any(target_os = "macos", windows))] +const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(60); +#[cfg(not(any(target_os = "macos", windows)))] const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(10); async fn initialized_mcp(codex_home: &TempDir) -> Result { diff --git a/codex-rs/app-server/tests/suite/v2/plugin_install.rs b/codex-rs/app-server/tests/suite/v2/plugin_install.rs index a3bea53172..e51fac725f 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_install.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_install.rs @@ -51,7 +51,9 @@ use wiremock::matchers::header; use wiremock::matchers::method; use wiremock::matchers::path; -const DEFAULT_TIMEOUT: Duration = Duration::from_secs(10); +// Plugin install tests wait on connector discovery after the install response path +// starts, which is noticeably slower on Windows CI. +const DEFAULT_TIMEOUT: Duration = Duration::from_secs(60); #[tokio::test] async fn plugin_install_rejects_relative_marketplace_paths() -> Result<()> { diff --git a/codex-rs/app-server/tests/suite/v2/thread_fork.rs b/codex-rs/app-server/tests/suite/v2/thread_fork.rs index 576a46d643..649919db8e 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_fork.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_fork.rs @@ -43,6 +43,9 @@ use super::analytics::enable_analytics_capture; use super::analytics::thread_initialized_event; use super::analytics::wait_for_analytics_payload; +#[cfg(windows)] +const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(25); +#[cfg(not(windows))] const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); #[tokio::test] diff --git a/codex-rs/app-server/tests/suite/v2/thread_read.rs b/codex-rs/app-server/tests/suite/v2/thread_read.rs index e4ff900150..4ab1181624 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_read.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_read.rs @@ -36,6 +36,9 @@ use std::path::Path; use tempfile::TempDir; use tokio::time::timeout; +#[cfg(windows)] +const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(25); +#[cfg(not(windows))] const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); #[tokio::test] diff --git a/codex-rs/app-server/tests/suite/v2/thread_resume.rs b/codex-rs/app-server/tests/suite/v2/thread_resume.rs index db3a0897fc..46dee9b5d1 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -76,6 +76,9 @@ use super::analytics::enable_analytics_capture; use super::analytics::thread_initialized_event; use super::analytics::wait_for_analytics_payload; +#[cfg(windows)] +const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(25); +#[cfg(not(windows))] const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); const CODEX_5_2_INSTRUCTIONS_TEMPLATE_DEFAULT: &str = "You are Codex, a coding agent based on GPT-5. You and the user share the same workspace and collaborate to achieve the user's goals."; diff --git a/codex-rs/app-server/tests/suite/v2/thread_unsubscribe.rs b/codex-rs/app-server/tests/suite/v2/thread_unsubscribe.rs index 6aab3d186f..7a3ae61b72 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_unsubscribe.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_unsubscribe.rs @@ -31,13 +31,17 @@ use tempfile::TempDir; use tokio::time::timeout; const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); +// Windows CI can spend most of the default read timeout launching PowerShell +// before the command finishes and the follow-up model request is sent. +const TURN_COMPLETION_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30); async fn wait_for_responses_request_count_to_stabilize( server: &wiremock::MockServer, expected_count: usize, settle_duration: std::time::Duration, + timeout_duration: std::time::Duration, ) -> Result<()> { - timeout(DEFAULT_READ_TIMEOUT, async { + timeout(timeout_duration, async { let mut stable_since: Option = None; loop { let requests = server @@ -130,9 +134,9 @@ async fn thread_unsubscribe_keeps_thread_loaded_until_idle_timeout() -> Result<( async fn thread_unsubscribe_during_turn_keeps_turn_running() -> Result<()> { #[cfg(target_os = "windows")] let shell_command = vec![ - "powershell".to_string(), - "-Command".to_string(), - "Start-Sleep -Seconds 1".to_string(), + "Start-Sleep".to_string(), + "-Seconds".to_string(), + "1".to_string(), ]; #[cfg(not(target_os = "windows"))] let shell_command = vec!["sleep".to_string(), "1".to_string()]; @@ -210,6 +214,7 @@ async fn thread_unsubscribe_during_turn_keeps_turn_running() -> Result<()> { &server, /*expected_count*/ 2, std::time::Duration::from_millis(200), + TURN_COMPLETION_TIMEOUT, ) .await?; diff --git a/codex-rs/backend-client/src/client.rs b/codex-rs/backend-client/src/client.rs index 20baa1cfea..ad461973f9 100644 --- a/codex-rs/backend-client/src/client.rs +++ b/codex-rs/backend-client/src/client.rs @@ -104,6 +104,7 @@ pub struct Client { bearer_token: Option, user_agent: Option, chatgpt_account_id: Option, + chatgpt_account_is_fedramp: bool, path_style: PathStyle, } @@ -129,6 +130,7 @@ impl Client { bearer_token: None, user_agent: None, chatgpt_account_id: None, + chatgpt_account_is_fedramp: false, path_style, }) } @@ -141,6 +143,9 @@ impl Client { if let Some(account_id) = auth.get_account_id() { client = client.with_chatgpt_account_id(account_id); } + if auth.is_fedramp_account() { + client = client.with_fedramp_routing_header(); + } Ok(client) } @@ -161,6 +166,11 @@ impl Client { self } + pub fn with_fedramp_routing_header(mut self) -> Self { + self.chatgpt_account_is_fedramp = true; + self + } + pub fn with_path_style(mut self, style: PathStyle) -> Self { self.path_style = style; self @@ -185,6 +195,11 @@ impl Client { { h.insert(name, hv); } + if self.chatgpt_account_is_fedramp + && let Ok(name) = HeaderName::from_bytes(b"X-OpenAI-Fedramp") + { + h.insert(name, HeaderValue::from_static("true")); + } h } diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 6accb2cb0c..25bf80a6e3 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -49,6 +49,7 @@ use crate::mcp_cmd::McpCli; use crate::responses_cmd::ResponsesCommand; use crate::responses_cmd::run_responses_command; +use codex_core::clear_memory_roots_contents; use codex_core::config::Config; use codex_core::config::ConfigOverrides; use codex_core::config::edit::ConfigEditsBuilder; @@ -1282,27 +1283,17 @@ async fn run_debug_clear_memories_command( cleared_state_db = true; } - let memory_root = config.codex_home.join("memories"); - let removed_memory_root = match tokio::fs::remove_dir_all(&memory_root).await { - Ok(()) => true, - Err(err) if err.kind() == std::io::ErrorKind::NotFound => false, - Err(err) => return Err(err.into()), - }; + clear_memory_roots_contents(&config.codex_home).await?; let mut message = if cleared_state_db { format!("Cleared memory state from {}.", state_path.display()) } else { format!("No state db found at {}.", state_path.display()) }; - - if removed_memory_root { - message.push_str(&format!(" Removed {}.", memory_root.display())); - } else { - message.push_str(&format!( - " No memory directory found at {}.", - memory_root.display() - )); - } + message.push_str(&format!( + " Cleared memory directories under {}.", + config.codex_home.display() + )); println!("{message}"); diff --git a/codex-rs/cli/src/mcp_cmd.rs b/codex-rs/cli/src/mcp_cmd.rs index cac6ef2169..4ea084b83f 100644 --- a/codex-rs/cli/src/mcp_cmd.rs +++ b/codex-rs/cli/src/mcp_cmd.rs @@ -297,6 +297,7 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re let new_entry = McpServerConfig { transport: transport.clone(), + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/cli/tests/debug_clear_memories.rs b/codex-rs/cli/tests/debug_clear_memories.rs index 164e4bd3e2..a24f7ebdde 100644 --- a/codex-rs/cli/tests/debug_clear_memories.rs +++ b/codex-rs/cli/tests/debug_clear_memories.rs @@ -125,7 +125,8 @@ INSERT INTO jobs ( .fetch_one(&pool) .await?; assert_eq!(memory_jobs_count, 0); - assert!(!memory_root.exists()); + assert!(memory_root.exists()); + assert_eq!(std::fs::read_dir(memory_root)?.count(), 0); Ok(()) } diff --git a/codex-rs/code-mode/src/description.rs b/codex-rs/code-mode/src/description.rs index 81e45add00..7f5e47fa25 100644 --- a/codex-rs/code-mode/src/description.rs +++ b/codex-rs/code-mode/src/description.rs @@ -9,6 +9,8 @@ use crate::PUBLIC_TOOL_NAME; const MAX_JS_SAFE_INTEGER: u64 = (1_u64 << 53) - 1; const CODE_MODE_ONLY_PREFACE: &str = "Use `exec/wait` tool to run all other tools, do not attempt to use any other tools directly"; +const DEFERRED_NESTED_TOOLS_GUIDANCE: &str = r#"Some nested MCP/app tools may be omitted from this description. They are still available on the global `tools` object and listed in `ALL_TOOLS`. +To find one, filter `ALL_TOOLS` by `name` and `description`; do not print the full `ALL_TOOLS` array. Print only a small set of relevant matches if you need to inspect them."#; const EXEC_DESCRIPTION_TEMPLATE: &str = r#"Run JavaScript code to orchestrate/compose tool calls - Evaluates the provided JavaScript code in a fresh V8 isolate as an async module. - All nested tools are available on the global `tools` object, for example `await tools.exec_command(...)`. Tool names are exposed as normalized JavaScript identifiers, for example `await tools.mcp__ologs__get_profile(...)`. @@ -251,15 +253,19 @@ pub fn build_exec_tool_description( enabled_tools: &[ToolDefinition], namespace_descriptions: &BTreeMap, code_mode_only: bool, + deferred_tools_available: bool, ) -> String { - if !code_mode_only { - return EXEC_DESCRIPTION_TEMPLATE.to_string(); + let mut sections = Vec::new(); + if code_mode_only { + sections.push(CODE_MODE_ONLY_PREFACE.to_string()); + } + sections.push(EXEC_DESCRIPTION_TEMPLATE.to_string()); + if deferred_tools_available { + sections.push(DEFERRED_NESTED_TOOLS_GUIDANCE.to_string()); + } + if !code_mode_only { + return sections.join("\n\n"); } - - let mut sections = vec![ - CODE_MODE_ONLY_PREFACE.to_string(), - EXEC_DESCRIPTION_TEMPLATE.to_string(), - ]; if !enabled_tools.is_empty() { let mut current_namespace: Option<&str> = None; @@ -863,6 +869,7 @@ mod tests { }], &BTreeMap::new(), /*code_mode_only*/ true, + /*deferred_tools_available*/ false, ); assert!(description.contains( "### `foo` @@ -872,8 +879,12 @@ bar" #[test] fn exec_description_mentions_timeout_helpers() { - let description = - build_exec_tool_description(&[], &BTreeMap::new(), /*code_mode_only*/ false); + let description = build_exec_tool_description( + &[], + &BTreeMap::new(), + /*code_mode_only*/ false, + /*deferred_tools_available*/ false, + ); assert!(description.contains("`setTimeout(callback: () => void, delayMs?: number)`")); assert!(description.contains("`clearTimeout(timeoutId?: number)`")); } @@ -924,6 +935,7 @@ bar" ], &namespace_descriptions, /*code_mode_only*/ true, + /*deferred_tools_available*/ false, ); assert_eq!(description.matches("## mcp__sample").count(), 1); assert!(description.contains("## mcp__sample\nShared namespace guidance.")); @@ -963,6 +975,7 @@ bar" }], &namespace_descriptions, /*code_mode_only*/ true, + /*deferred_tools_available*/ false, ); assert!(!description.contains("## mcp__sample")); @@ -1061,6 +1074,7 @@ bar" ], &BTreeMap::new(), /*code_mode_only*/ true, + /*deferred_tools_available*/ false, ); assert_eq!( @@ -1071,4 +1085,17 @@ bar" ); assert_eq!(description.matches("Shared MCP Types:").count(), 1); } + + #[test] + fn exec_description_mentions_deferred_nested_tools_when_available() { + let description = build_exec_tool_description( + &[], + &BTreeMap::new(), + /*code_mode_only*/ false, + /*deferred_tools_available*/ true, + ); + + assert!(description.contains("Some nested MCP/app tools may be omitted")); + assert!(description.contains("filter `ALL_TOOLS` by `name` and `description`")); + } } diff --git a/codex-rs/codex-api/src/api_bridge.rs b/codex-rs/codex-api/src/api_bridge.rs index 7c36c67fdd..2677ff3e55 100644 --- a/codex-rs/codex-api/src/api_bridge.rs +++ b/codex-rs/codex-api/src/api_bridge.rs @@ -179,6 +179,7 @@ struct UsageErrorBody { pub struct CoreAuthProvider { pub token: Option, pub account_id: Option, + pub is_fedramp_account: bool, } impl CoreAuthProvider { @@ -196,6 +197,7 @@ impl CoreAuthProvider { Self { token: token.map(str::to_string), account_id: account_id.map(str::to_string), + is_fedramp_account: false, } } } @@ -212,5 +214,8 @@ impl ApiAuthProvider for CoreAuthProvider { { let _ = headers.insert("ChatGPT-Account-ID", header); } + if self.is_fedramp_account { + crate::auth::add_fedramp_routing_header(headers); + } } } diff --git a/codex-rs/codex-api/src/api_bridge_tests.rs b/codex-rs/codex-api/src/api_bridge_tests.rs index 50247c1314..c7d4bbbdab 100644 --- a/codex-rs/codex-api/src/api_bridge_tests.rs +++ b/codex-rs/codex-api/src/api_bridge_tests.rs @@ -136,6 +136,7 @@ fn core_auth_provider_reports_when_auth_header_will_attach() { let auth = CoreAuthProvider { token: Some("access-token".to_string()), account_id: None, + is_fedramp_account: false, }; assert!(auth.auth_header_attached()); @@ -162,3 +163,22 @@ fn core_auth_provider_adds_auth_headers() { Some("workspace-123") ); } + +#[test] +fn core_auth_provider_adds_fedramp_routing_header_for_fedramp_accounts() { + let auth = CoreAuthProvider { + token: Some("access-token".to_string()), + account_id: Some("workspace-123".to_string()), + is_fedramp_account: true, + }; + let mut headers = HeaderMap::new(); + + crate::AuthProvider::add_auth_headers(&auth, &mut headers); + + assert_eq!( + headers + .get("X-OpenAI-Fedramp") + .and_then(|value| value.to_str().ok()), + Some("true") + ); +} diff --git a/codex-rs/codex-api/src/auth.rs b/codex-rs/codex-api/src/auth.rs index a7b1e69d14..efa5fb3288 100644 --- a/codex-rs/codex-api/src/auth.rs +++ b/codex-rs/codex-api/src/auth.rs @@ -1,4 +1,5 @@ use http::HeaderMap; +use http::HeaderValue; /// Adds authentication headers to API requests. /// @@ -8,3 +9,26 @@ use http::HeaderMap; pub trait AuthProvider: Send + Sync { fn add_auth_headers(&self, headers: &mut HeaderMap); } + +pub(crate) fn add_fedramp_routing_header(headers: &mut HeaderMap) { + headers.insert("X-OpenAI-Fedramp", HeaderValue::from_static("true")); +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn add_fedramp_routing_header_sets_header() { + let mut headers = HeaderMap::new(); + + add_fedramp_routing_header(&mut headers); + + assert_eq!( + headers + .get("X-OpenAI-Fedramp") + .and_then(|v| v.to_str().ok()), + Some("true") + ); + } +} diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index 421ed14c36..db90bfe429 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -269,6 +269,7 @@ fn codex_apps_mcp_server_config(config: &McpConfig, auth: Option<&CodexAuth>) -> http_headers, env_http_headers: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/codex-mcp/src/mcp/mod_tests.rs b/codex-rs/codex-mcp/src/mcp/mod_tests.rs index eaffbb8a5d..c1d39f2a7c 100644 --- a/codex-rs/codex-mcp/src/mcp/mod_tests.rs +++ b/codex-rs/codex-mcp/src/mcp/mod_tests.rs @@ -193,6 +193,7 @@ async fn effective_mcp_servers_preserve_user_servers_and_add_codex_apps() { http_headers: None, env_http_headers: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -215,6 +216,7 @@ async fn effective_mcp_servers_preserve_user_servers_and_add_codex_apps() { http_headers: None, env_http_headers: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/codex-mcp/src/mcp/skill_dependencies.rs b/codex-rs/codex-mcp/src/mcp/skill_dependencies.rs index aa0c4d4e7a..ab72e25958 100644 --- a/codex-rs/codex-mcp/src/mcp/skill_dependencies.rs +++ b/codex-rs/codex-mcp/src/mcp/skill_dependencies.rs @@ -119,6 +119,7 @@ fn mcp_dependency_to_server_config( http_headers: None, env_http_headers: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -146,6 +147,7 @@ fn mcp_dependency_to_server_config( env_vars: Vec::new(), cwd: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/codex-mcp/src/mcp/skill_dependencies_tests.rs b/codex-rs/codex-mcp/src/mcp/skill_dependencies_tests.rs index 0fe2856f01..b562bf9e2c 100644 --- a/codex-rs/codex-mcp/src/mcp/skill_dependencies_tests.rs +++ b/codex-rs/codex-mcp/src/mcp/skill_dependencies_tests.rs @@ -39,6 +39,7 @@ fn collect_missing_respects_canonical_installed_key() { http_headers: None, env_http_headers: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -90,6 +91,7 @@ fn collect_missing_dedupes_by_canonical_key_but_preserves_original_name() { http_headers: None, env_http_headers: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/codex-mcp/src/mcp_connection_manager.rs b/codex-rs/codex-mcp/src/mcp_connection_manager.rs index 1542c7b30b..d1696b5931 100644 --- a/codex-rs/codex-mcp/src/mcp_connection_manager.rs +++ b/codex-rs/codex-mcp/src/mcp_connection_manager.rs @@ -758,12 +758,13 @@ impl McpConnectionManager { let submit_id = startup_submit_id.clone(); let auth_entry = auth_entries.get(&server_name).cloned(); join_set.spawn(async move { - let outcome = async_managed_client.client().await; + let mut outcome = async_managed_client.client().await; if cancel_token.is_cancelled() { - return (server_name, Err(StartupOutcomeError::Cancelled)); + outcome = Err(StartupOutcomeError::Cancelled); } let status = match &outcome { Ok(_) => McpStartupStatus::Ready, + Err(StartupOutcomeError::Cancelled) => McpStartupStatus::Cancelled, Err(error) => { let error_str = mcp_init_error_display( server_name.as_str(), diff --git a/codex-rs/codex-mcp/src/mcp_connection_manager_tests.rs b/codex-rs/codex-mcp/src/mcp_connection_manager_tests.rs index 663f76d5d0..6b65d455a8 100644 --- a/codex-rs/codex-mcp/src/mcp_connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/mcp_connection_manager_tests.rs @@ -792,6 +792,7 @@ fn mcp_init_error_display_prompts_for_github_pat() { http_headers: None, env_http_headers: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -842,6 +843,7 @@ fn mcp_init_error_display_reports_generic_errors() { http_headers: None, env_http_headers: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/config/src/mcp_edit.rs b/codex-rs/config/src/mcp_edit.rs index 965f869362..d315d23f8e 100644 --- a/codex-rs/config/src/mcp_edit.rs +++ b/codex-rs/config/src/mcp_edit.rs @@ -174,6 +174,9 @@ fn serialize_mcp_server(config: &McpServerConfig) -> TomlItem { if !config.enabled { entry["enabled"] = value(false); } + if let Some(environment) = &config.experimental_environment { + entry["experimental_environment"] = value(environment.clone()); + } if config.required { entry["required"] = value(true); } diff --git a/codex-rs/config/src/mcp_edit_tests.rs b/codex-rs/config/src/mcp_edit_tests.rs index 38ab0852fe..5f60c2612a 100644 --- a/codex-rs/config/src/mcp_edit_tests.rs +++ b/codex-rs/config/src/mcp_edit_tests.rs @@ -22,6 +22,7 @@ async fn replace_mcp_servers_serializes_per_tool_approval_overrides() -> anyhow: env_vars: Vec::new(), cwd: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: true, diff --git a/codex-rs/config/src/mcp_types.rs b/codex-rs/config/src/mcp_types.rs index c00988c6ff..8aece35c0d 100644 --- a/codex-rs/config/src/mcp_types.rs +++ b/codex-rs/config/src/mcp_types.rs @@ -61,6 +61,10 @@ pub struct McpServerConfig { #[serde(flatten)] pub transport: McpServerTransportConfig, + /// Experimental environment selector for where Codex should start this MCP server. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub experimental_environment: Option, + /// When `false`, Codex skips initializing this MCP server. #[serde(default = "default_enabled")] pub enabled: bool, @@ -139,6 +143,8 @@ pub struct RawMcpServerConfig { // shared #[serde(default)] + pub experimental_environment: Option, + #[serde(default)] pub startup_timeout_sec: Option, #[serde(default)] pub startup_timeout_ms: Option, @@ -181,6 +187,7 @@ impl TryFrom for McpServerConfig { url, bearer_token, bearer_token_env_var, + experimental_environment, startup_timeout_sec, startup_timeout_ms, tool_timeout_sec, @@ -246,6 +253,7 @@ impl TryFrom for McpServerConfig { Ok(Self { transport, + experimental_environment, startup_timeout_sec, tool_timeout_sec, enabled: enabled.unwrap_or_else(default_enabled), diff --git a/codex-rs/config/src/mcp_types_tests.rs b/codex-rs/config/src/mcp_types_tests.rs index 6b9fb16e90..500ea56f62 100644 --- a/codex-rs/config/src/mcp_types_tests.rs +++ b/codex-rs/config/src/mcp_types_tests.rs @@ -297,6 +297,7 @@ fn deserialize_ignores_unknown_server_fields() { env_vars: Vec::new(), cwd: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/config/src/types.rs b/codex-rs/config/src/types.rs index 73a5763fe1..15c1a6fad5 100644 --- a/codex-rs/config/src/types.rs +++ b/codex-rs/config/src/types.rs @@ -31,6 +31,10 @@ pub const DEFAULT_MEMORIES_MAX_ROLLOUT_AGE_DAYS: i64 = 30; pub const DEFAULT_MEMORIES_MIN_ROLLOUT_IDLE_HOURS: i64 = 6; pub const DEFAULT_MEMORIES_MAX_RAW_MEMORIES_FOR_CONSOLIDATION: usize = 256; pub const DEFAULT_MEMORIES_MAX_UNUSED_DAYS: i64 = 30; +const MIN_MEMORIES_MAX_RAW_MEMORIES_FOR_CONSOLIDATION: usize = 1; +const MAX_MEMORIES_MAX_RAW_MEMORIES_FOR_CONSOLIDATION: usize = 4096; +const MIN_MEMORIES_MAX_ROLLOUTS_PER_STARTUP: usize = 1; +const MAX_MEMORIES_MAX_ROLLOUTS_PER_STARTUP: usize = 128; const fn default_enabled() -> bool { true @@ -185,12 +189,14 @@ pub struct MemoriesToml { /// When `false`, skip injecting memory usage instructions into developer prompts. pub use_memories: Option, /// Maximum number of recent raw memories retained for global consolidation. + #[schemars(range(min = 1, max = 4096))] pub max_raw_memories_for_consolidation: Option, /// Maximum number of days since a memory was last used before it becomes ineligible for phase 2 selection. pub max_unused_days: Option, /// Maximum age of the threads used for memories. pub max_rollout_age_days: Option, /// Maximum number of rollout candidates processed per pass. + #[schemars(range(min = 1, max = 128))] pub max_rollouts_per_startup: Option, /// Minimum idle time between last thread activity and memory creation (hours). > 12h recommended. pub min_rollout_idle_hours: Option, @@ -244,7 +250,10 @@ impl From for MemoriesConfig { max_raw_memories_for_consolidation: toml .max_raw_memories_for_consolidation .unwrap_or(defaults.max_raw_memories_for_consolidation) - .min(4096), + .clamp( + MIN_MEMORIES_MAX_RAW_MEMORIES_FOR_CONSOLIDATION, + MAX_MEMORIES_MAX_RAW_MEMORIES_FOR_CONSOLIDATION, + ), max_unused_days: toml .max_unused_days .unwrap_or(defaults.max_unused_days) @@ -256,7 +265,10 @@ impl From for MemoriesConfig { max_rollouts_per_startup: toml .max_rollouts_per_startup .unwrap_or(defaults.max_rollouts_per_startup) - .min(128), + .clamp( + MIN_MEMORIES_MAX_ROLLOUTS_PER_STARTUP, + MAX_MEMORIES_MAX_ROLLOUTS_PER_STARTUP, + ), min_rollout_idle_hours: toml .min_rollout_idle_hours .unwrap_or(defaults.min_rollout_idle_hours) diff --git a/codex-rs/config/src/types_tests.rs b/codex-rs/config/src/types_tests.rs index fbcd2bdc64..b18c1cc645 100644 --- a/codex-rs/config/src/types_tests.rs +++ b/codex-rs/config/src/types_tests.rs @@ -41,3 +41,21 @@ fn deserialize_skill_config_with_path_selector() { } ); } + +#[test] +fn memories_config_clamps_count_limits_to_nonzero_values() { + let config = MemoriesConfig::from(MemoriesToml { + max_raw_memories_for_consolidation: Some(0), + max_rollouts_per_startup: Some(0), + ..Default::default() + }); + + assert_eq!( + config, + MemoriesConfig { + max_raw_memories_for_consolidation: 1, + max_rollouts_per_startup: 1, + ..MemoriesConfig::default() + } + ); +} diff --git a/codex-rs/core-plugins/BUILD.bazel b/codex-rs/core-plugins/BUILD.bazel new file mode 100644 index 0000000000..aa19b9f368 --- /dev/null +++ b/codex-rs/core-plugins/BUILD.bazel @@ -0,0 +1,15 @@ +load("//:defs.bzl", "codex_rust_crate") + +codex_rust_crate( + name = "core-plugins", + crate_name = "codex_core_plugins", + compile_data = glob( + include = ["**"], + exclude = [ + "**/* *", + "BUILD.bazel", + "Cargo.toml", + ], + allow_empty = True, + ), +) diff --git a/codex-rs/core-plugins/Cargo.toml b/codex-rs/core-plugins/Cargo.toml new file mode 100644 index 0000000000..02e121f0e6 --- /dev/null +++ b/codex-rs/core-plugins/Cargo.toml @@ -0,0 +1,39 @@ +[package] +edition.workspace = true +license.workspace = true +name = "codex-core-plugins" +version.workspace = true + +[lib] +doctest = false +name = "codex_core_plugins" +path = "src/lib.rs" + +[lints] +workspace = true + +[dependencies] +codex-app-server-protocol = { workspace = true } +codex-config = { workspace = true } +codex-core-skills = { workspace = true } +codex-exec-server = { workspace = true } +codex-git-utils = { workspace = true } +codex-login = { workspace = true } +codex-plugin = { workspace = true } +codex-protocol = { workspace = true } +codex-utils-absolute-path = { workspace = true } +codex-utils-plugins = { workspace = true } +dirs = { workspace = true } +reqwest = { workspace = true } +serde = { workspace = true, features = ["derive"] } +serde_json = { workspace = true } +tempfile = { workspace = true } +thiserror = { workspace = true } +tokio = { workspace = true, features = ["fs"] } +toml = { workspace = true } +tracing = { workspace = true } +url = { workspace = true } + +[dev-dependencies] +pretty_assertions = { workspace = true } +tempfile = { workspace = true } diff --git a/codex-rs/core-plugins/src/lib.rs b/codex-rs/core-plugins/src/lib.rs new file mode 100644 index 0000000000..e075158cb1 --- /dev/null +++ b/codex-rs/core-plugins/src/lib.rs @@ -0,0 +1,6 @@ +pub mod loader; +pub mod manifest; +pub mod marketplace; +pub mod remote; +pub mod store; +pub mod toggles; diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs new file mode 100644 index 0000000000..206add5b53 --- /dev/null +++ b/codex-rs/core-plugins/src/loader.rs @@ -0,0 +1,807 @@ +use crate::manifest::PluginManifestPaths; +use crate::manifest::load_plugin_manifest; +use crate::marketplace::MarketplacePluginSource; +use crate::marketplace::list_marketplaces; +use crate::marketplace::load_marketplace; +use crate::store::PluginStore; +use crate::store::plugin_version_for_source; +use codex_config::ConfigLayerStack; +use codex_config::types::McpServerConfig; +use codex_config::types::PluginConfig; +use codex_core_skills::SkillMetadata; +use codex_core_skills::config_rules::SkillConfigRules; +use codex_core_skills::config_rules::resolve_disabled_skill_paths; +use codex_core_skills::config_rules::skill_config_rules_from_stack; +use codex_core_skills::loader::SkillRoot; +use codex_core_skills::loader::load_skills_from_roots; +use codex_exec_server::LOCAL_FS; +use codex_plugin::AppConnectorId; +use codex_plugin::LoadedPlugin; +use codex_plugin::PluginCapabilitySummary; +use codex_plugin::PluginId; +use codex_plugin::PluginIdError; +use codex_plugin::PluginLoadOutcome; +use codex_plugin::PluginTelemetryMetadata; +use codex_protocol::protocol::Product; +use codex_protocol::protocol::SkillScope; +use codex_utils_absolute_path::AbsolutePathBuf; +use serde::Deserialize; +use serde_json::Map as JsonMap; +use serde_json::Value as JsonValue; +use std::collections::HashMap; +use std::collections::HashSet; +use std::fs; +use std::path::Path; +use std::sync::Arc; +use tracing::warn; + +const DEFAULT_SKILLS_DIR_NAME: &str = "skills"; +const DEFAULT_MCP_CONFIG_FILE: &str = ".mcp.json"; +const DEFAULT_APP_CONFIG_FILE: &str = ".app.json"; +const OPENAI_CURATED_MARKETPLACE_NAME: &str = "openai-curated"; +const CONFIG_TOML_FILE: &str = "config.toml"; + +pub fn log_plugin_load_errors(outcome: &PluginLoadOutcome) { + for plugin in outcome + .plugins() + .iter() + .filter(|plugin| plugin.error.is_some()) + { + if let Some(error) = plugin.error.as_deref() { + warn!( + plugin = plugin.config_name, + path = %plugin.root.display(), + "failed to load plugin: {error}" + ); + } + } +} + +#[derive(Debug, Default, Deserialize)] +#[serde(rename_all = "camelCase")] +struct PluginMcpFile { + #[serde(default)] + mcp_servers: HashMap, +} + +#[derive(Debug, Default, Deserialize)] +#[serde(rename_all = "camelCase")] +struct PluginAppFile { + #[serde(default)] + apps: HashMap, +} + +#[derive(Debug, Default, Deserialize)] +struct PluginAppConfig { + id: String, +} + +pub async fn load_plugins_from_layer_stack( + config_layer_stack: &ConfigLayerStack, + store: &PluginStore, + restriction_product: Option, +) -> PluginLoadOutcome { + let skill_config_rules = skill_config_rules_from_stack(config_layer_stack); + let mut configured_plugins: Vec<_> = configured_plugins_from_stack(config_layer_stack) + .into_iter() + .collect(); + configured_plugins.sort_unstable_by(|(a, _), (b, _)| a.cmp(b)); + + let mut plugins = Vec::with_capacity(configured_plugins.len()); + let mut seen_mcp_server_names = HashMap::::new(); + for (configured_name, plugin) in configured_plugins { + let loaded_plugin = load_plugin( + configured_name.clone(), + &plugin, + store, + restriction_product, + &skill_config_rules, + ) + .await; + for name in loaded_plugin.mcp_servers.keys() { + if let Some(previous_plugin) = + seen_mcp_server_names.insert(name.clone(), configured_name.clone()) + { + warn!( + plugin = configured_name, + previous_plugin, + server = name, + "skipping duplicate plugin MCP server name" + ); + } + } + plugins.push(loaded_plugin); + } + + PluginLoadOutcome::from_plugins(plugins) +} + +pub fn refresh_curated_plugin_cache( + codex_home: &Path, + plugin_version: &str, + configured_curated_plugin_ids: &[PluginId], +) -> Result { + let store = PluginStore::new(codex_home.to_path_buf()); + let curated_marketplace_path = AbsolutePathBuf::try_from( + codex_home + .join(".tmp/plugins") + .join(".agents/plugins/marketplace.json"), + ) + .map_err(|_| "local curated marketplace is not available".to_string())?; + let curated_marketplace = load_marketplace(&curated_marketplace_path) + .map_err(|err| format!("failed to load curated marketplace for cache refresh: {err}"))?; + + let mut plugin_sources = HashMap::::new(); + for plugin in curated_marketplace.plugins { + let plugin_name = plugin.name; + if plugin_sources.contains_key(&plugin_name) { + warn!( + plugin = plugin_name, + marketplace = OPENAI_CURATED_MARKETPLACE_NAME, + "ignoring duplicate curated plugin entry during cache refresh" + ); + continue; + } + let source_path = match plugin.source { + MarketplacePluginSource::Local { path } => path, + }; + plugin_sources.insert(plugin_name, source_path); + } + + let mut cache_refreshed = false; + for plugin_id in configured_curated_plugin_ids { + if store.active_plugin_version(plugin_id).as_deref() == Some(plugin_version) { + continue; + } + + let Some(source_path) = plugin_sources.get(&plugin_id.plugin_name).cloned() else { + warn!( + plugin = plugin_id.plugin_name, + marketplace = OPENAI_CURATED_MARKETPLACE_NAME, + "configured curated plugin no longer exists in curated marketplace during cache refresh" + ); + continue; + }; + + store + .install_with_version(source_path, plugin_id.clone(), plugin_version.to_string()) + .map_err(|err| { + format!( + "failed to refresh curated plugin cache for {}: {err}", + plugin_id.as_key() + ) + })?; + cache_refreshed = true; + } + + Ok(cache_refreshed) +} + +pub fn refresh_non_curated_plugin_cache( + codex_home: &Path, + additional_roots: &[AbsolutePathBuf], +) -> Result { + let configured_non_curated_plugin_ids = + non_curated_plugin_ids_from_config_keys(configured_plugins_from_codex_home( + codex_home, + "failed to read user config while refreshing non-curated plugin cache", + "failed to parse user config while refreshing non-curated plugin cache", + )); + if configured_non_curated_plugin_ids.is_empty() { + return Ok(false); + } + let configured_non_curated_plugin_keys = configured_non_curated_plugin_ids + .iter() + .map(PluginId::as_key) + .collect::>(); + + let store = PluginStore::new(codex_home.to_path_buf()); + let marketplace_outcome = list_marketplaces(additional_roots) + .map_err(|err| format!("failed to discover marketplaces for cache refresh: {err}"))?; + let mut plugin_sources = HashMap::::new(); + + for marketplace in marketplace_outcome.marketplaces { + if marketplace.name == OPENAI_CURATED_MARKETPLACE_NAME { + continue; + } + + for plugin in marketplace.plugins { + let plugin_id = + PluginId::new(plugin.name.clone(), marketplace.name.clone()).map_err(|err| { + match err { + PluginIdError::Invalid(message) => { + format!("failed to prepare non-curated plugin cache refresh: {message}") + } + } + })?; + let plugin_key = plugin_id.as_key(); + if !configured_non_curated_plugin_keys.contains(&plugin_key) { + continue; + } + if plugin_sources.contains_key(&plugin_key) { + warn!( + plugin = plugin.name, + marketplace = marketplace.name, + "ignoring duplicate non-curated plugin entry during cache refresh" + ); + continue; + } + + let source_path = match plugin.source { + MarketplacePluginSource::Local { path } => path, + }; + let plugin_version = plugin_version_for_source(source_path.as_path()) + .map_err(|err| format!("failed to read plugin version for {plugin_key}: {err}"))?; + plugin_sources.insert(plugin_key, (source_path, plugin_version)); + } + } + + let mut cache_refreshed = false; + for plugin_id in configured_non_curated_plugin_ids { + let plugin_key = plugin_id.as_key(); + let Some((source_path, plugin_version)) = plugin_sources.get(&plugin_key).cloned() else { + warn!( + plugin = plugin_id.plugin_name, + marketplace = plugin_id.marketplace_name, + "configured non-curated plugin no longer exists in discovered marketplaces during cache refresh" + ); + continue; + }; + + if store.active_plugin_version(&plugin_id).as_deref() == Some(plugin_version.as_str()) { + continue; + } + + store + .install_with_version(source_path, plugin_id.clone(), plugin_version) + .map_err(|err| format!("failed to refresh plugin cache for {plugin_key}: {err}"))?; + cache_refreshed = true; + } + + Ok(cache_refreshed) +} + +fn configured_plugins_from_stack( + config_layer_stack: &ConfigLayerStack, +) -> HashMap { + let Some(user_layer) = config_layer_stack.get_user_layer() else { + return HashMap::new(); + }; + configured_plugins_from_user_config_value(&user_layer.config) +} + +fn configured_plugins_from_user_config_value( + user_config: &toml::Value, +) -> HashMap { + let Some(plugins_value) = user_config.get("plugins") else { + return HashMap::new(); + }; + match plugins_value.clone().try_into() { + Ok(plugins) => plugins, + Err(err) => { + warn!("invalid plugins config: {err}"); + HashMap::new() + } + } +} + +fn configured_plugins_from_codex_home( + codex_home: &Path, + read_error_message: &str, + parse_error_message: &str, +) -> HashMap { + let config_path = codex_home.join(CONFIG_TOML_FILE); + let user_config = match fs::read_to_string(&config_path) { + Ok(user_config) => user_config, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return HashMap::new(), + Err(err) => { + warn!( + path = %config_path.display(), + error = %err, + "{read_error_message}" + ); + return HashMap::new(); + } + }; + + let user_config = match toml::from_str::(&user_config) { + Ok(user_config) => user_config, + Err(err) => { + warn!( + path = %config_path.display(), + error = %err, + "{parse_error_message}" + ); + return HashMap::new(); + } + }; + + configured_plugins_from_user_config_value(&user_config) +} + +fn configured_plugin_ids( + configured_plugins: HashMap, + invalid_plugin_key_message: &str, +) -> Vec { + configured_plugins + .into_keys() + .filter_map(|plugin_key| match PluginId::parse(&plugin_key) { + Ok(plugin_id) => Some(plugin_id), + Err(err) => { + warn!( + plugin_key, + error = %err, + "{invalid_plugin_key_message}" + ); + None + } + }) + .collect() +} + +fn curated_plugin_ids_from_config_keys( + configured_plugins: HashMap, +) -> Vec { + let mut configured_curated_plugin_ids = configured_plugin_ids( + configured_plugins, + "ignoring invalid configured plugin key during curated sync setup", + ) + .into_iter() + .filter(|plugin_id| plugin_id.marketplace_name == OPENAI_CURATED_MARKETPLACE_NAME) + .collect::>(); + configured_curated_plugin_ids.sort_unstable_by_key(PluginId::as_key); + configured_curated_plugin_ids +} + +fn non_curated_plugin_ids_from_config_keys( + configured_plugins: HashMap, +) -> Vec { + let mut configured_non_curated_plugin_ids = configured_plugin_ids( + configured_plugins, + "ignoring invalid plugin key during non-curated cache refresh setup", + ) + .into_iter() + .filter(|plugin_id| plugin_id.marketplace_name != OPENAI_CURATED_MARKETPLACE_NAME) + .collect::>(); + configured_non_curated_plugin_ids.sort_unstable_by_key(PluginId::as_key); + configured_non_curated_plugin_ids +} + +pub fn configured_curated_plugin_ids_from_codex_home(codex_home: &Path) -> Vec { + curated_plugin_ids_from_config_keys(configured_plugins_from_codex_home( + codex_home, + "failed to read user config while refreshing curated plugin cache", + "failed to parse user config while refreshing curated plugin cache", + )) +} + +async fn load_plugin( + config_name: String, + plugin: &PluginConfig, + store: &PluginStore, + restriction_product: Option, + skill_config_rules: &SkillConfigRules, +) -> LoadedPlugin { + let plugin_id = PluginId::parse(&config_name); + let active_plugin_root = plugin_id + .as_ref() + .ok() + .and_then(|plugin_id| store.active_plugin_root(plugin_id)); + let root = active_plugin_root + .clone() + .unwrap_or_else(|| match &plugin_id { + Ok(plugin_id) => store.plugin_base_root(plugin_id), + Err(_) => store.root().clone(), + }); + let mut loaded_plugin = LoadedPlugin { + config_name, + manifest_name: None, + manifest_description: None, + root, + enabled: plugin.enabled, + skill_roots: Vec::new(), + disabled_skill_paths: HashSet::new(), + has_enabled_skills: false, + mcp_servers: HashMap::new(), + apps: Vec::new(), + error: None, + }; + + if !plugin.enabled { + return loaded_plugin; + } + + let plugin_root = match plugin_id { + Ok(_) => match active_plugin_root { + Some(plugin_root) => plugin_root, + None => { + loaded_plugin.error = Some("plugin is not installed".to_string()); + return loaded_plugin; + } + }, + Err(err) => { + loaded_plugin.error = Some(err.to_string()); + return loaded_plugin; + } + }; + + if !plugin_root.as_path().is_dir() { + loaded_plugin.error = Some("path does not exist or is not a directory".to_string()); + return loaded_plugin; + } + + let Some(manifest) = load_plugin_manifest(plugin_root.as_path()) else { + loaded_plugin.error = Some("missing or invalid .codex-plugin/plugin.json".to_string()); + return loaded_plugin; + }; + + let manifest_paths = &manifest.paths; + loaded_plugin.manifest_name = manifest + .interface + .as_ref() + .and_then(|interface| interface.display_name.as_deref()) + .map(str::trim) + .filter(|display_name| !display_name.is_empty()) + .map(str::to_string) + .or_else(|| Some(manifest.name.clone())); + loaded_plugin.manifest_description = manifest.description.clone(); + loaded_plugin.skill_roots = plugin_skill_roots(&plugin_root, manifest_paths); + let resolved_skills = load_plugin_skills( + &plugin_root, + manifest_paths, + restriction_product, + skill_config_rules, + ) + .await; + let has_enabled_skills = resolved_skills.has_enabled_skills(); + loaded_plugin.disabled_skill_paths = resolved_skills.disabled_skill_paths; + loaded_plugin.has_enabled_skills = has_enabled_skills; + let mut mcp_servers = HashMap::new(); + for mcp_config_path in plugin_mcp_config_paths(plugin_root.as_path(), manifest_paths) { + let plugin_mcp = load_mcp_servers_from_file(plugin_root.as_path(), &mcp_config_path).await; + for (name, config) in plugin_mcp.mcp_servers { + if mcp_servers.insert(name.clone(), config).is_some() { + warn!( + plugin = %plugin_root.display(), + path = %mcp_config_path.display(), + server = name, + "plugin MCP file overwrote an earlier server definition" + ); + } + } + } + loaded_plugin.mcp_servers = mcp_servers; + loaded_plugin.apps = load_plugin_apps(plugin_root.as_path()).await; + loaded_plugin +} + +#[derive(Debug, Clone)] +pub struct ResolvedPluginSkills { + pub skills: Vec, + pub disabled_skill_paths: HashSet, + pub had_errors: bool, +} + +impl ResolvedPluginSkills { + pub fn has_enabled_skills(&self) -> bool { + self.had_errors + || self + .skills + .iter() + .any(|skill| !self.disabled_skill_paths.contains(&skill.path_to_skills_md)) + } +} + +pub async fn load_plugin_skills( + plugin_root: &AbsolutePathBuf, + manifest_paths: &PluginManifestPaths, + restriction_product: Option, + skill_config_rules: &SkillConfigRules, +) -> ResolvedPluginSkills { + let roots = plugin_skill_roots(plugin_root, manifest_paths) + .into_iter() + .map(|path| SkillRoot { + path, + scope: SkillScope::User, + file_system: Arc::clone(&LOCAL_FS), + }) + .collect::>(); + let outcome = load_skills_from_roots(roots).await; + let had_errors = !outcome.errors.is_empty(); + let skills = outcome + .skills + .into_iter() + .filter(|skill| skill.matches_product_restriction_for_product(restriction_product)) + .collect::>(); + let disabled_skill_paths = resolve_disabled_skill_paths(&skills, skill_config_rules); + + ResolvedPluginSkills { + skills, + disabled_skill_paths, + had_errors, + } +} + +fn plugin_skill_roots( + plugin_root: &AbsolutePathBuf, + manifest_paths: &PluginManifestPaths, +) -> Vec { + let mut paths = default_skill_roots(plugin_root); + if let Some(path) = &manifest_paths.skills { + paths.push(path.clone()); + } + paths.sort_unstable(); + paths.dedup(); + paths +} + +fn default_skill_roots(plugin_root: &AbsolutePathBuf) -> Vec { + let skills_dir = plugin_root.join(DEFAULT_SKILLS_DIR_NAME); + if skills_dir.is_dir() { + vec![skills_dir] + } else { + Vec::new() + } +} + +fn plugin_mcp_config_paths( + plugin_root: &Path, + manifest_paths: &PluginManifestPaths, +) -> Vec { + if let Some(path) = &manifest_paths.mcp_servers { + return vec![path.clone()]; + } + default_mcp_config_paths(plugin_root) +} + +fn default_mcp_config_paths(plugin_root: &Path) -> Vec { + let mut paths = Vec::new(); + let default_path = plugin_root.join(DEFAULT_MCP_CONFIG_FILE); + if default_path.is_file() + && let Ok(default_path) = AbsolutePathBuf::try_from(default_path) + { + paths.push(default_path); + } + paths.sort_unstable_by(|left, right| left.as_path().cmp(right.as_path())); + paths.dedup_by(|left, right| left.as_path() == right.as_path()); + paths +} + +pub async fn load_plugin_apps(plugin_root: &Path) -> Vec { + if let Some(manifest) = load_plugin_manifest(plugin_root) { + return load_apps_from_paths( + plugin_root, + plugin_app_config_paths(plugin_root, &manifest.paths), + ) + .await; + } + load_apps_from_paths(plugin_root, default_app_config_paths(plugin_root)).await +} + +fn plugin_app_config_paths( + plugin_root: &Path, + manifest_paths: &PluginManifestPaths, +) -> Vec { + if let Some(path) = &manifest_paths.apps { + return vec![path.clone()]; + } + default_app_config_paths(plugin_root) +} + +fn default_app_config_paths(plugin_root: &Path) -> Vec { + let mut paths = Vec::new(); + let default_path = plugin_root.join(DEFAULT_APP_CONFIG_FILE); + if default_path.is_file() + && let Ok(default_path) = AbsolutePathBuf::try_from(default_path) + { + paths.push(default_path); + } + paths.sort_unstable_by(|left, right| left.as_path().cmp(right.as_path())); + paths.dedup_by(|left, right| left.as_path() == right.as_path()); + paths +} + +async fn load_apps_from_paths( + plugin_root: &Path, + app_config_paths: Vec, +) -> Vec { + let mut connector_ids = Vec::new(); + for app_config_path in app_config_paths { + let Ok(contents) = tokio::fs::read_to_string(app_config_path.as_path()).await else { + continue; + }; + let parsed = match serde_json::from_str::(&contents) { + Ok(parsed) => parsed, + Err(err) => { + warn!( + path = %app_config_path.display(), + "failed to parse plugin app config: {err}" + ); + continue; + } + }; + + let mut apps: Vec = parsed.apps.into_values().collect(); + apps.sort_unstable_by(|left, right| left.id.cmp(&right.id)); + + connector_ids.extend(apps.into_iter().filter_map(|app| { + if app.id.trim().is_empty() { + warn!( + plugin = %plugin_root.display(), + "plugin app config is missing an app id" + ); + None + } else { + Some(AppConnectorId(app.id)) + } + })); + } + connector_ids.dedup(); + connector_ids +} + +pub async fn plugin_telemetry_metadata_from_root( + plugin_id: &PluginId, + plugin_root: &AbsolutePathBuf, +) -> PluginTelemetryMetadata { + let Some(manifest) = load_plugin_manifest(plugin_root.as_path()) else { + return PluginTelemetryMetadata::from_plugin_id(plugin_id); + }; + + let manifest_paths = &manifest.paths; + let has_skills = !plugin_skill_roots(plugin_root, manifest_paths).is_empty(); + let mut mcp_server_names = Vec::new(); + for path in plugin_mcp_config_paths(plugin_root.as_path(), manifest_paths) { + mcp_server_names.extend( + load_mcp_servers_from_file(plugin_root.as_path(), &path) + .await + .mcp_servers + .into_keys(), + ); + } + mcp_server_names.sort_unstable(); + mcp_server_names.dedup(); + + PluginTelemetryMetadata { + plugin_id: plugin_id.clone(), + capability_summary: Some(PluginCapabilitySummary { + config_name: plugin_id.as_key(), + display_name: plugin_id.plugin_name.clone(), + description: None, + has_skills, + mcp_server_names, + app_connector_ids: load_apps_from_paths( + plugin_root.as_path(), + plugin_app_config_paths(plugin_root.as_path(), manifest_paths), + ) + .await, + }), + } +} + +pub async fn load_plugin_mcp_servers(plugin_root: &Path) -> HashMap { + let Some(manifest) = load_plugin_manifest(plugin_root) else { + return HashMap::new(); + }; + + let mut mcp_servers = HashMap::new(); + for mcp_config_path in plugin_mcp_config_paths(plugin_root, &manifest.paths) { + let plugin_mcp = load_mcp_servers_from_file(plugin_root, &mcp_config_path).await; + for (name, config) in plugin_mcp.mcp_servers { + mcp_servers.entry(name).or_insert(config); + } + } + + mcp_servers +} + +pub async fn installed_plugin_telemetry_metadata( + codex_home: &Path, + plugin_id: &PluginId, +) -> PluginTelemetryMetadata { + let store = PluginStore::new(codex_home.to_path_buf()); + let Some(plugin_root) = store.active_plugin_root(plugin_id) else { + return PluginTelemetryMetadata::from_plugin_id(plugin_id); + }; + + plugin_telemetry_metadata_from_root(plugin_id, &plugin_root).await +} + +async fn load_mcp_servers_from_file( + plugin_root: &Path, + mcp_config_path: &AbsolutePathBuf, +) -> PluginMcpDiscovery { + let Ok(contents) = tokio::fs::read_to_string(mcp_config_path.as_path()).await else { + return PluginMcpDiscovery::default(); + }; + let parsed = match serde_json::from_str::(&contents) { + Ok(parsed) => parsed, + Err(err) => { + warn!( + path = %mcp_config_path.display(), + "failed to parse plugin MCP config: {err}" + ); + return PluginMcpDiscovery::default(); + } + }; + normalize_plugin_mcp_servers( + plugin_root, + parsed.mcp_servers, + mcp_config_path.to_string_lossy().as_ref(), + ) +} + +fn normalize_plugin_mcp_servers( + plugin_root: &Path, + plugin_mcp_servers: HashMap, + source: &str, +) -> PluginMcpDiscovery { + let mut mcp_servers = HashMap::new(); + + for (name, config_value) in plugin_mcp_servers { + let normalized = normalize_plugin_mcp_server_value(plugin_root, config_value); + match serde_json::from_value::(JsonValue::Object(normalized)) { + Ok(config) => { + mcp_servers.insert(name, config); + } + Err(err) => { + warn!( + plugin = %plugin_root.display(), + server = name, + "failed to parse plugin MCP server from {source}: {err}" + ); + } + } + } + + PluginMcpDiscovery { mcp_servers } +} + +fn normalize_plugin_mcp_server_value( + plugin_root: &Path, + value: JsonValue, +) -> JsonMap { + let mut object = match value { + JsonValue::Object(object) => object, + _ => return JsonMap::new(), + }; + + if let Some(JsonValue::String(transport_type)) = object.remove("type") { + match transport_type.as_str() { + "http" | "streamable_http" | "streamable-http" => {} + "stdio" => {} + other => { + warn!( + plugin = %plugin_root.display(), + transport = other, + "plugin MCP server uses an unknown transport type" + ); + } + } + } + + if let Some(JsonValue::Object(oauth)) = object.remove("oauth") + && oauth.contains_key("callbackPort") + { + warn!( + plugin = %plugin_root.display(), + "plugin MCP server OAuth callbackPort is ignored; Codex uses global MCP OAuth callback settings" + ); + } + + if let Some(JsonValue::String(cwd)) = object.get("cwd") + && !Path::new(cwd).is_absolute() + { + object.insert( + "cwd".to_string(), + JsonValue::String(plugin_root.join(cwd).display().to_string()), + ); + } + + object +} + +#[derive(Debug, Default)] +struct PluginMcpDiscovery { + mcp_servers: HashMap, +} diff --git a/codex-rs/core/src/plugins/manifest.rs b/codex-rs/core-plugins/src/manifest.rs similarity index 98% rename from codex-rs/core/src/plugins/manifest.rs rename to codex-rs/core-plugins/src/manifest.rs index f1253441bd..f58b882ca1 100644 --- a/codex-rs/core/src/plugins/manifest.rs +++ b/codex-rs/core-plugins/src/manifest.rs @@ -30,12 +30,12 @@ struct RawPluginManifest { } #[derive(Debug, Clone, PartialEq, Eq)] -pub(crate) struct PluginManifest { - pub(crate) name: String, - pub(crate) version: Option, - pub(crate) description: Option, - pub(crate) paths: PluginManifestPaths, - pub(crate) interface: Option, +pub struct PluginManifest { + pub name: String, + pub version: Option, + pub description: Option, + pub paths: PluginManifestPaths, + pub interface: Option, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -114,7 +114,7 @@ enum RawPluginManifestDefaultPromptEntry { Invalid(JsonValue), } -pub(crate) fn load_plugin_manifest(plugin_root: &Path) -> Option { +pub fn load_plugin_manifest(plugin_root: &Path) -> Option { let manifest_path = plugin_root.join(PLUGIN_MANIFEST_PATH); if !manifest_path.is_file() { return None; diff --git a/codex-rs/core/src/plugins/marketplace.rs b/codex-rs/core-plugins/src/marketplace.rs similarity index 98% rename from codex-rs/core/src/plugins/marketplace.rs rename to codex-rs/core-plugins/src/marketplace.rs index 96c73e8e19..79e5237ad0 100644 --- a/codex-rs/core/src/plugins/marketplace.rs +++ b/codex-rs/core-plugins/src/marketplace.rs @@ -1,5 +1,5 @@ -use super::PluginManifestInterface; -use super::load_plugin_manifest; +use crate::manifest::PluginManifestInterface; +use crate::manifest::load_plugin_manifest; use codex_app_server_protocol::PluginAuthPolicy; use codex_app_server_protocol::PluginInstallPolicy; use codex_git_utils::get_git_repo_root; @@ -231,7 +231,7 @@ pub fn validate_marketplace_root(root: &Path) -> Result Option { +pub fn find_marketplace_manifest_path(root: &Path) -> Option { MARKETPLACE_MANIFEST_RELATIVE_PATHS .iter() .find_map(|relative_path| { @@ -265,7 +265,7 @@ fn marketplace_root_from_layout(marketplace_path: &Path, relative_path: &str) -> Some(current.to_path_buf()) } -pub(crate) fn load_marketplace(path: &AbsolutePathBuf) -> Result { +pub fn load_marketplace(path: &AbsolutePathBuf) -> Result { let marketplace = load_raw_marketplace_manifest(path)?; let mut plugins = Vec::new(); @@ -311,7 +311,8 @@ pub(crate) fn load_marketplace(path: &AbsolutePathBuf) -> Result, ) -> Result { diff --git a/codex-rs/core/src/plugins/marketplace_tests.rs b/codex-rs/core-plugins/src/marketplace_tests.rs similarity index 100% rename from codex-rs/core/src/plugins/marketplace_tests.rs rename to codex-rs/core-plugins/src/marketplace_tests.rs diff --git a/codex-rs/core/src/plugins/remote.rs b/codex-rs/core-plugins/src/remote.rs similarity index 94% rename from codex-rs/core/src/plugins/remote.rs rename to codex-rs/core-plugins/src/remote.rs index f26bbe8880..34ece0880a 100644 --- a/codex-rs/core/src/plugins/remote.rs +++ b/codex-rs/core-plugins/src/remote.rs @@ -1,4 +1,3 @@ -use crate::config::Config; use codex_login::CodexAuth; use codex_login::default_client::build_reqwest_client; use codex_protocol::protocol::Product; @@ -11,12 +10,17 @@ const REMOTE_PLUGIN_FETCH_TIMEOUT: Duration = Duration::from_secs(30); const REMOTE_FEATURED_PLUGIN_FETCH_TIMEOUT: Duration = Duration::from_secs(10); const REMOTE_PLUGIN_MUTATION_TIMEOUT: Duration = Duration::from_secs(30); +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct RemotePluginServiceConfig { + pub chatgpt_base_url: String, +} + #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] -pub(crate) struct RemotePluginStatusSummary { - pub(crate) name: String, +pub struct RemotePluginStatusSummary { + pub name: String, #[serde(default = "default_remote_marketplace_name")] - pub(crate) marketplace_name: String, - pub(crate) enabled: bool, + pub marketplace_name: String, + pub enabled: bool, } #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] @@ -116,8 +120,8 @@ pub enum RemotePluginFetchError { }, } -pub(crate) async fn fetch_remote_plugin_status( - config: &Config, +pub async fn fetch_remote_plugin_status( + config: &RemotePluginServiceConfig, auth: Option<&CodexAuth>, ) -> Result, RemotePluginFetchError> { let Some(auth) = auth else { @@ -161,7 +165,7 @@ pub(crate) async fn fetch_remote_plugin_status( } pub async fn fetch_remote_featured_plugin_ids( - config: &Config, + config: &RemotePluginServiceConfig, auth: Option<&CodexAuth>, product: Option, ) -> Result, RemotePluginFetchError> { @@ -205,8 +209,8 @@ pub async fn fetch_remote_featured_plugin_ids( }) } -pub(crate) async fn enable_remote_plugin( - config: &Config, +pub async fn enable_remote_plugin( + config: &RemotePluginServiceConfig, auth: Option<&CodexAuth>, plugin_id: &str, ) -> Result<(), RemotePluginMutationError> { @@ -214,8 +218,8 @@ pub(crate) async fn enable_remote_plugin( Ok(()) } -pub(crate) async fn uninstall_remote_plugin( - config: &Config, +pub async fn uninstall_remote_plugin( + config: &RemotePluginServiceConfig, auth: Option<&CodexAuth>, plugin_id: &str, ) -> Result<(), RemotePluginMutationError> { @@ -238,7 +242,7 @@ fn default_remote_marketplace_name() -> String { } async fn post_remote_plugin_mutation( - config: &Config, + config: &RemotePluginServiceConfig, auth: Option<&CodexAuth>, plugin_id: &str, action: &str, @@ -294,7 +298,7 @@ async fn post_remote_plugin_mutation( } fn remote_plugin_mutation_url( - config: &Config, + config: &RemotePluginServiceConfig, plugin_id: &str, action: &str, ) -> Result { diff --git a/codex-rs/core/src/plugins/store.rs b/codex-rs/core-plugins/src/store.rs similarity index 97% rename from codex-rs/core/src/plugins/store.rs rename to codex-rs/core-plugins/src/store.rs index 4316f18002..2f530b8648 100644 --- a/codex-rs/core/src/plugins/store.rs +++ b/codex-rs/core-plugins/src/store.rs @@ -1,5 +1,5 @@ -use super::load_plugin_manifest; -use super::manifest::PluginManifest; +use crate::manifest::PluginManifest; +use crate::manifest::load_plugin_manifest; use codex_plugin::PluginId; use codex_plugin::validate_plugin_segment; use codex_utils_absolute_path::AbsolutePathBuf; @@ -11,8 +11,8 @@ use std::io; use std::path::Path; use std::path::PathBuf; -pub(crate) const DEFAULT_PLUGIN_VERSION: &str = "local"; -pub(crate) const PLUGINS_CACHE_DIR: &str = "plugins/cache"; +pub const DEFAULT_PLUGIN_VERSION: &str = "local"; +pub const PLUGINS_CACHE_DIR: &str = "plugins/cache"; #[derive(Debug, Clone, PartialEq, Eq)] pub struct PluginInstallResult { @@ -157,7 +157,7 @@ impl PluginStoreError { } } -pub(crate) fn plugin_version_for_source(source_path: &Path) -> Result { +pub fn plugin_version_for_source(source_path: &Path) -> Result { let plugin_version = plugin_manifest_version_for_source(source_path)? .unwrap_or_else(|| DEFAULT_PLUGIN_VERSION.to_string()); validate_plugin_version_segment(&plugin_version).map_err(PluginStoreError::Invalid)?; diff --git a/codex-rs/core/src/plugins/store_tests.rs b/codex-rs/core-plugins/src/store_tests.rs similarity index 100% rename from codex-rs/core/src/plugins/store_tests.rs rename to codex-rs/core-plugins/src/store_tests.rs diff --git a/codex-rs/core/src/plugins/toggles.rs b/codex-rs/core-plugins/src/toggles.rs similarity index 100% rename from codex-rs/core/src/plugins/toggles.rs rename to codex-rs/core-plugins/src/toggles.rs diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 78dfd3c070..4ce4192b81 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -33,6 +33,7 @@ codex-async-utils = { workspace = true } codex-code-mode = { workspace = true } codex-connectors = { workspace = true } codex-config = { workspace = true } +codex-core-plugins = { workspace = true } codex-core-skills = { workspace = true } codex-exec-server = { workspace = true } codex-features = { workspace = true } diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index c61daa8b56..2e381cd3f4 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -855,7 +855,8 @@ "max_raw_memories_for_consolidation": { "description": "Maximum number of recent raw memories retained for global consolidation.", "format": "uint", - "minimum": 0.0, + "maximum": 4096.0, + "minimum": 1.0, "type": "integer" }, "max_rollout_age_days": { @@ -866,7 +867,8 @@ "max_rollouts_per_startup": { "description": "Maximum number of rollout candidates processed per pass.", "format": "uint", - "minimum": 0.0, + "maximum": 128.0, + "minimum": 1.0, "type": "integer" }, "max_unused_days": { @@ -1495,6 +1497,10 @@ }, "type": "array" }, + "experimental_environment": { + "default": null, + "type": "string" + }, "http_headers": { "additionalProperties": { "type": "string" diff --git a/codex-rs/core/src/agent/control_tests.rs b/codex-rs/core/src/agent/control_tests.rs index 79a96d11e5..b444d8c67c 100644 --- a/codex-rs/core/src/agent/control_tests.rs +++ b/codex-rs/core/src/agent/control_tests.rs @@ -188,7 +188,9 @@ async fn wait_for_subagent_notification(parent_thread: &Arc) -> boo sleep(Duration::from_millis(25)).await; } }; - timeout(Duration::from_secs(2), wait).await.is_ok() + // CI can take several seconds to schedule the detached completion watcher, + // especially on slower Windows runners. + timeout(Duration::from_secs(10), wait).await.is_ok() } async fn persist_thread_for_tree_resume(thread: &Arc, message: &str) { diff --git a/codex-rs/core/src/agent_identity.rs b/codex-rs/core/src/agent_identity.rs index 13a07cd961..2c897b5456 100644 --- a/codex-rs/core/src/agent_identity.rs +++ b/codex-rs/core/src/agent_identity.rs @@ -737,6 +737,7 @@ mod tests { chatgpt_plan_type: None, chatgpt_user_id: user_id.map(ToOwned::to_owned), chatgpt_account_id: Some(account_id.to_string()), + chatgpt_account_is_fedramp: false, raw_jwt: fake_id_token(account_id, user_id), }, access_token: format!("access-token-{account_id}"), diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 66a0dc527e..00c5c144cd 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -41,6 +41,7 @@ use crate::stream_events_utils::HandleOutputCtx; use crate::stream_events_utils::handle_non_tool_response_item; use crate::stream_events_utils::handle_output_item_done; use crate::stream_events_utils::last_assistant_message_from_item; +use crate::stream_events_utils::mark_thread_memory_mode_polluted_if_external_context; use crate::stream_events_utils::raw_assistant_output_text_from_item; use crate::stream_events_utils::record_completed_response_item; use crate::turn_metadata::TurnMetadataState; @@ -5567,15 +5568,15 @@ mod handlers { errors.push("state db unavailable; memory rows were not cleared".to_string()); } - let memory_root = crate::memories::memory_root(&config.codex_home); - if let Err(err) = crate::memories::clear_memory_root_contents(&memory_root).await { + if let Err(err) = crate::memories::clear_memory_roots_contents(&config.codex_home).await { errors.push(format!( - "failed clearing memory directory {}: {err}", - memory_root.display() + "failed clearing memory directories under {}: {err}", + config.codex_home.display() )); } if errors.is_empty() { + let memory_root = crate::memories::memory_root(&config.codex_home); sess.send_event_raw(Event { id: sub_id, msg: EventMsg::Warning(WarningEvent { @@ -7830,8 +7831,15 @@ async fn drain_in_flight( while let Some(res) = in_flight.next().await { match res { Ok(response_input) => { - sess.record_conversation_items(&turn_context, &[response_input.into()]) + let response_item = response_input.into(); + sess.record_conversation_items(&turn_context, std::slice::from_ref(&response_item)) .await; + mark_thread_memory_mode_polluted_if_external_context( + sess.as_ref(), + turn_context.as_ref(), + &response_item, + ) + .await; } Err(err) => { error_or_panic(format!("in-flight tool future failed during drain: {err}")); diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index fcb1b58993..4805b03d01 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -765,6 +765,117 @@ async fn danger_full_access_turns_do_not_expose_managed_network_proxy() -> anyho Ok(()) } +#[tokio::test] +async fn danger_full_access_tool_attempts_do_not_enforce_managed_network() -> anyhow::Result<()> { + #[derive(Default)] + struct ProbeToolRuntime { + enforce_managed_network: Vec, + } + + impl crate::tools::sandboxing::Approvable<()> for ProbeToolRuntime { + type ApprovalKey = String; + + fn approval_keys(&self, _req: &()) -> Vec { + vec!["probe".to_string()] + } + + fn start_approval_async<'a>( + &'a mut self, + _req: &'a (), + _ctx: crate::tools::sandboxing::ApprovalCtx<'a>, + ) -> futures::future::BoxFuture<'a, ReviewDecision> { + Box::pin(async { ReviewDecision::Approved }) + } + } + + impl crate::tools::sandboxing::Sandboxable for ProbeToolRuntime { + fn sandbox_preference(&self) -> codex_sandboxing::SandboxablePreference { + codex_sandboxing::SandboxablePreference::Auto + } + } + + impl crate::tools::sandboxing::ToolRuntime<(), ()> for ProbeToolRuntime { + async fn run( + &mut self, + _req: &(), + attempt: &crate::tools::sandboxing::SandboxAttempt<'_>, + _ctx: &crate::tools::sandboxing::ToolCtx, + ) -> Result<(), crate::tools::sandboxing::ToolError> { + self.enforce_managed_network + .push(attempt.enforce_managed_network); + Ok(()) + } + } + + let network_spec = crate::config::NetworkProxySpec::from_config_and_constraints( + NetworkProxyConfig::default(), + Some(NetworkConstraints { + enabled: Some(true), + ..Default::default() + }), + &SandboxPolicy::DangerFullAccess, + )?; + + let session = make_session_with_config(move |config| { + config.permissions.sandbox_policy = + codex_config::Constrained::allow_any(SandboxPolicy::DangerFullAccess); + config.permissions.network = Some(network_spec); + + let layers = config + .config_layer_stack + .get_layers( + ConfigLayerStackOrdering::LowestPrecedenceFirst, + /*include_disabled*/ true, + ) + .into_iter() + .cloned() + .collect(); + let mut requirements = config.config_layer_stack.requirements().clone(); + requirements.network = Some(Sourced::new( + NetworkConstraints { + enabled: Some(true), + ..Default::default() + }, + RequirementSource::CloudRequirements, + )); + let mut requirements_toml = config.config_layer_stack.requirements_toml().clone(); + requirements_toml.network = Some(crate::config_loader::NetworkRequirementsToml { + enabled: Some(true), + ..Default::default() + }); + config.config_layer_stack = ConfigLayerStack::new(layers, requirements, requirements_toml) + .expect("rebuild config layer stack with network requirements"); + }) + .await?; + + let turn = session.new_default_turn().await; + assert!(turn.network.is_none()); + + let mut orchestrator = crate::tools::orchestrator::ToolOrchestrator::new(); + let mut tool = ProbeToolRuntime::default(); + let tool_ctx = crate::tools::sandboxing::ToolCtx { + session: Arc::clone(&session), + turn: Arc::clone(&turn), + call_id: "probe-call".to_string(), + tool_name: "probe".to_string(), + }; + + orchestrator + .run( + &mut tool, + &(), + &tool_ctx, + turn.as_ref(), + AskForApproval::Never, + ) + .await + .expect("probe runtime should succeed"); + + assert_eq!(tool.enforce_managed_network, vec![false]); + + Ok(()) +} + #[tokio::test] async fn workspace_write_turns_continue_to_expose_managed_network_proxy() -> anyhow::Result<()> { let sandbox_policy = SandboxPolicy::new_workspace_write_policy(); diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 650d792306..9243235d71 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -81,6 +81,7 @@ fn stdio_mcp(command: &str) -> McpServerConfig { env_vars: Vec::new(), cwd: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -103,6 +104,7 @@ fn http_mcp(url: &str) -> McpServerConfig { http_headers: None, env_http_headers: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -2057,6 +2059,7 @@ async fn replace_mcp_servers_round_trips_entries() -> anyhow::Result<()> { env_vars: Vec::new(), cwd: None, }, + experimental_environment: Some("remote".to_string()), enabled: true, required: false, supports_parallel_tool_calls: false, @@ -2098,6 +2101,7 @@ async fn replace_mcp_servers_round_trips_entries() -> anyhow::Result<()> { } assert_eq!(docs.startup_timeout_sec, Some(Duration::from_secs(3))); assert_eq!(docs.tool_timeout_sec, Some(Duration::from_secs(5))); + assert_eq!(docs.experimental_environment.as_deref(), Some("remote")); assert!(docs.enabled); let empty = BTreeMap::new(); @@ -2305,6 +2309,7 @@ async fn replace_mcp_servers_serializes_env_sorted() -> anyhow::Result<()> { env_vars: Vec::new(), cwd: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -2379,6 +2384,7 @@ async fn replace_mcp_servers_serializes_env_vars() -> anyhow::Result<()> { env_vars: vec!["ALPHA".to_string(), "BETA".to_string()], cwd: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -2433,6 +2439,7 @@ async fn replace_mcp_servers_serializes_cwd() -> anyhow::Result<()> { env_vars: Vec::new(), cwd: Some(cwd_path.clone()), }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -2485,6 +2492,7 @@ async fn replace_mcp_servers_streamable_http_serializes_bearer_token() -> anyhow http_headers: None, env_http_headers: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -2553,6 +2561,7 @@ async fn replace_mcp_servers_streamable_http_serializes_custom_headers() -> anyh "DOCS_AUTH".to_string(), )])), }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -2633,6 +2642,7 @@ async fn replace_mcp_servers_streamable_http_removes_optional_sections() -> anyh "DOCS_AUTH".to_string(), )])), }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -2666,6 +2676,7 @@ async fn replace_mcp_servers_streamable_http_removes_optional_sections() -> anyh http_headers: None, env_http_headers: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -2734,6 +2745,7 @@ async fn replace_mcp_servers_streamable_http_isolates_headers_between_servers() "DOCS_AUTH".to_string(), )])), }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -2757,6 +2769,7 @@ async fn replace_mcp_servers_streamable_http_isolates_headers_between_servers() env_vars: Vec::new(), cwd: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -2843,6 +2856,7 @@ async fn replace_mcp_servers_serializes_disabled_flag() -> anyhow::Result<()> { env_vars: Vec::new(), cwd: None, }, + experimental_environment: None, enabled: false, required: false, supports_parallel_tool_calls: false, @@ -2891,6 +2905,7 @@ async fn replace_mcp_servers_serializes_required_flag() -> anyhow::Result<()> { env_vars: Vec::new(), cwd: None, }, + experimental_environment: None, enabled: true, required: true, supports_parallel_tool_calls: false, @@ -2939,6 +2954,7 @@ async fn replace_mcp_servers_serializes_tool_filters() -> anyhow::Result<()> { env_vars: Vec::new(), cwd: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -2991,6 +3007,7 @@ async fn replace_mcp_servers_streamable_http_serializes_oauth_resource() -> anyh http_headers: None, env_http_headers: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/src/config/edit.rs b/codex-rs/core/src/config/edit.rs index 443d00c93d..6f4231f908 100644 --- a/codex-rs/core/src/config/edit.rs +++ b/codex-rs/core/src/config/edit.rs @@ -222,6 +222,9 @@ mod document_helpers { if !config.enabled { entry["enabled"] = value(false); } + if let Some(environment) = &config.experimental_environment { + entry["experimental_environment"] = value(environment.clone()); + } if config.required { entry["required"] = value(true); } diff --git a/codex-rs/core/src/config/edit_tests.rs b/codex-rs/core/src/config/edit_tests.rs index c9288be30b..46680d5434 100644 --- a/codex-rs/core/src/config/edit_tests.rs +++ b/codex-rs/core/src/config/edit_tests.rs @@ -575,6 +575,7 @@ fn blocking_replace_mcp_servers_round_trips() { env_vars: vec!["FOO".to_string()], cwd: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: true, @@ -602,6 +603,7 @@ fn blocking_replace_mcp_servers_round_trips() { ), env_http_headers: None, }, + experimental_environment: None, enabled: false, required: false, supports_parallel_tool_calls: false, @@ -666,6 +668,7 @@ fn blocking_replace_mcp_servers_serializes_tool_approval_overrides() { env_vars: Vec::new(), cwd: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -727,6 +730,7 @@ foo = { command = "cmd" } env_vars: Vec::new(), cwd: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -779,6 +783,7 @@ foo = { command = "cmd" } # keep me env_vars: Vec::new(), cwd: None, }, + experimental_environment: None, enabled: false, required: false, supports_parallel_tool_calls: false, @@ -830,6 +835,7 @@ foo = { command = "cmd", args = ["--flag"] } # keep me env_vars: Vec::new(), cwd: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -882,6 +888,7 @@ foo = { command = "cmd" } env_vars: Vec::new(), cwd: None, }, + experimental_environment: None, enabled: false, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index abfe91a0e1..17c4e63b7c 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -59,6 +59,7 @@ pub use codex_mcp::SandboxState; mod mcp_openai_file; mod mcp_tool_call; mod memories; +pub use memories::clear_memory_roots_contents; pub(crate) mod mention_syntax; pub(crate) mod message_history; pub(crate) mod utils; @@ -98,9 +99,7 @@ pub(crate) use skills::build_skill_injections; pub(crate) use skills::build_skill_name_counts; pub(crate) use skills::collect_env_var_dependencies; pub(crate) use skills::collect_explicit_skill_mentions; -pub(crate) use skills::config_rules; pub(crate) use skills::injection; -pub(crate) use skills::loader; pub(crate) use skills::manager; pub(crate) use skills::maybe_emit_implicit_skill_invocation; pub(crate) use skills::render_skills_section; diff --git a/codex-rs/core/src/mcp_openai_file.rs b/codex-rs/core/src/mcp_openai_file.rs index 587dd4b770..33d0a3f1f0 100644 --- a/codex-rs/core/src/mcp_openai_file.rs +++ b/codex-rs/core/src/mcp_openai_file.rs @@ -115,6 +115,7 @@ async fn build_uploaded_local_argument_value( let upload_auth = CoreAuthProvider { token: Some(token_data.access_token), account_id: token_data.account_id, + is_fedramp_account: auth.is_fedramp_account(), }; let uploaded = upload_local_file( turn_context.config.chatgpt_base_url.trim_end_matches('/'), diff --git a/codex-rs/core/src/mcp_skill_dependencies.rs b/codex-rs/core/src/mcp_skill_dependencies.rs index 6cdd3cf084..c87b02730c 100644 --- a/codex-rs/core/src/mcp_skill_dependencies.rs +++ b/codex-rs/core/src/mcp_skill_dependencies.rs @@ -364,6 +364,7 @@ fn mcp_dependency_to_server_config( http_headers: None, env_http_headers: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -391,6 +392,7 @@ fn mcp_dependency_to_server_config( env_vars: Vec::new(), cwd: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/src/memories/control.rs b/codex-rs/core/src/memories/control.rs index 10bd19246f..4f09d3e74c 100644 --- a/codex-rs/core/src/memories/control.rs +++ b/codex-rs/core/src/memories/control.rs @@ -1,5 +1,16 @@ use std::path::Path; +pub async fn clear_memory_roots_contents(codex_home: &Path) -> std::io::Result<()> { + for memory_root in [ + codex_home.join("memories"), + codex_home.join("memories_extensions"), + ] { + clear_memory_root_contents(memory_root.as_path()).await?; + } + + Ok(()) +} + pub(crate) async fn clear_memory_root_contents(memory_root: &Path) -> std::io::Result<()> { match tokio::fs::symlink_metadata(memory_root).await { Ok(metadata) if metadata.file_type().is_symlink() => { diff --git a/codex-rs/core/src/memories/mod.rs b/codex-rs/core/src/memories/mod.rs index 29c9d375f2..d796063d2d 100644 --- a/codex-rs/core/src/memories/mod.rs +++ b/codex-rs/core/src/memories/mod.rs @@ -17,7 +17,7 @@ pub(crate) mod usage; use codex_protocol::openai_models::ReasoningEffort; -pub(crate) use control::clear_memory_root_contents; +pub use control::clear_memory_roots_contents; /// Starts the memory startup pipeline for eligible root sessions. /// This is the single entrypoint that `codex` uses to trigger memory startup. /// diff --git a/codex-rs/core/src/memories/tests.rs b/codex-rs/core/src/memories/tests.rs index 9959b1fcb5..ab3538af67 100644 --- a/codex-rs/core/src/memories/tests.rs +++ b/codex-rs/core/src/memories/tests.rs @@ -1,6 +1,6 @@ +use super::control::clear_memory_root_contents; use super::storage::rebuild_raw_memories_file_from_memories; use super::storage::sync_rollout_summaries_from_memories; -use crate::memories::clear_memory_root_contents; use crate::memories::ensure_layout; use crate::memories::memory_root; use crate::memories::raw_memories_file; diff --git a/codex-rs/core/src/plugins/installed_marketplaces.rs b/codex-rs/core/src/plugins/installed_marketplaces.rs index ad1918d51d..a6f80139a9 100644 --- a/codex-rs/core/src/plugins/installed_marketplaces.rs +++ b/codex-rs/core/src/plugins/installed_marketplaces.rs @@ -1,10 +1,10 @@ use crate::config::Config; +use codex_core_plugins::marketplace::find_marketplace_manifest_path; use codex_utils_absolute_path::AbsolutePathBuf; use std::path::Path; use std::path::PathBuf; use tracing::warn; -use super::marketplace::find_marketplace_manifest_path; use super::validate_plugin_segment; pub const INSTALLED_MARKETPLACES_DIR: &str = ".tmp/marketplaces"; diff --git a/codex-rs/core/src/plugins/manager.rs b/codex-rs/core/src/plugins/manager.rs index a735180ab6..50c62d5823 100644 --- a/codex-rs/core/src/plugins/manager.rs +++ b/codex-rs/core/src/plugins/manager.rs @@ -1,52 +1,48 @@ -use super::LoadedPlugin; use super::PluginLoadOutcome; -use super::PluginManifestPaths; use super::curated_plugins_repo_path; use super::installed_marketplaces::installed_marketplace_roots_from_config; -use super::load_plugin_manifest; -use super::manifest::PluginManifestInterface; -use super::marketplace::MarketplaceError; -use super::marketplace::MarketplaceInterface; -use super::marketplace::MarketplaceListError; -use super::marketplace::MarketplacePluginAuthPolicy; -use super::marketplace::MarketplacePluginPolicy; -use super::marketplace::MarketplacePluginSource; -use super::marketplace::ResolvedMarketplacePlugin; -use super::marketplace::list_marketplaces; -use super::marketplace::load_marketplace; -use super::marketplace::resolve_marketplace_plugin; use super::read_curated_plugins_sha; -use super::remote::RemotePluginFetchError; -use super::remote::RemotePluginMutationError; -use super::remote::enable_remote_plugin; -use super::remote::fetch_remote_featured_plugin_ids; -use super::remote::fetch_remote_plugin_status; -use super::remote::uninstall_remote_plugin; use super::startup_sync::start_startup_remote_plugin_sync_once; -use super::store::PluginInstallResult as StorePluginInstallResult; -use super::store::PluginStore; -use super::store::PluginStoreError; -use super::store::plugin_version_for_source; use super::sync_openai_plugins_repo; use crate::SkillMetadata; -use crate::config::CONFIG_TOML_FILE; use crate::config::Config; use crate::config::ConfigService; use crate::config::ConfigServiceError; use crate::config::edit::ConfigEdit; use crate::config::edit::ConfigEditsBuilder; use crate::config_loader::ConfigLayerStack; -use crate::config_rules::SkillConfigRules; -use crate::config_rules::resolve_disabled_skill_paths; -use crate::config_rules::skill_config_rules_from_stack; -use crate::loader::SkillRoot; -use crate::loader::load_skills_from_roots; use codex_analytics::AnalyticsEventsClient; use codex_app_server_protocol::ConfigValueWriteParams; use codex_app_server_protocol::MergeStrategy; -use codex_config::types::McpServerConfig; use codex_config::types::PluginConfig; -use codex_exec_server::LOCAL_FS; +use codex_core_plugins::loader::configured_curated_plugin_ids_from_codex_home; +use codex_core_plugins::loader::installed_plugin_telemetry_metadata; +use codex_core_plugins::loader::load_plugin_apps; +use codex_core_plugins::loader::load_plugin_mcp_servers; +use codex_core_plugins::loader::load_plugin_skills; +use codex_core_plugins::loader::load_plugins_from_layer_stack; +use codex_core_plugins::loader::log_plugin_load_errors; +use codex_core_plugins::loader::plugin_telemetry_metadata_from_root; +use codex_core_plugins::loader::refresh_curated_plugin_cache; +use codex_core_plugins::loader::refresh_non_curated_plugin_cache; +use codex_core_plugins::manifest::PluginManifestInterface; +use codex_core_plugins::manifest::load_plugin_manifest; +use codex_core_plugins::marketplace::MarketplaceError; +use codex_core_plugins::marketplace::MarketplaceInterface; +use codex_core_plugins::marketplace::MarketplaceListError; +use codex_core_plugins::marketplace::MarketplacePluginAuthPolicy; +use codex_core_plugins::marketplace::MarketplacePluginPolicy; +use codex_core_plugins::marketplace::MarketplacePluginSource; +use codex_core_plugins::marketplace::ResolvedMarketplacePlugin; +use codex_core_plugins::marketplace::list_marketplaces; +use codex_core_plugins::marketplace::load_marketplace; +use codex_core_plugins::marketplace::resolve_marketplace_plugin; +use codex_core_plugins::remote::RemotePluginFetchError; +use codex_core_plugins::remote::RemotePluginMutationError; +use codex_core_plugins::remote::RemotePluginServiceConfig; +use codex_core_plugins::store::PluginInstallResult as StorePluginInstallResult; +use codex_core_plugins::store::PluginStore; +use codex_core_plugins::store::PluginStoreError; use codex_features::Feature; use codex_login::AuthManager; use codex_login::CodexAuth; @@ -54,19 +50,12 @@ use codex_plugin::AppConnectorId; use codex_plugin::PluginCapabilitySummary; use codex_plugin::PluginId; use codex_plugin::PluginIdError; -use codex_plugin::PluginTelemetryMetadata; use codex_plugin::prompt_safe_plugin_description; use codex_protocol::protocol::Product; -use codex_protocol::protocol::SkillScope; use codex_utils_absolute_path::AbsolutePathBuf; -use serde::Deserialize; -use serde_json::Map as JsonMap; -use serde_json::Value as JsonValue; use serde_json::json; use std::collections::HashMap; use std::collections::HashSet; -use std::fs; -use std::path::Path; use std::path::PathBuf; use std::sync::Arc; use std::sync::RwLock; @@ -78,9 +67,6 @@ use toml_edit::value; use tracing::info; use tracing::warn; -const DEFAULT_SKILLS_DIR_NAME: &str = "skills"; -const DEFAULT_MCP_CONFIG_FILE: &str = ".mcp.json"; -const DEFAULT_APP_CONFIG_FILE: &str = ".app.json"; pub const OPENAI_CURATED_MARKETPLACE_NAME: &str = "openai-curated"; pub const OPENAI_CURATED_MARKETPLACE_DISPLAY_NAME: &str = "OpenAI Curated"; static CURATED_REPO_SYNC_STARTED: AtomicBool = AtomicBool::new(false); @@ -109,6 +95,12 @@ struct NonCuratedCacheRefreshState { in_flight: bool, } +fn remote_plugin_service_config(config: &Config) -> RemotePluginServiceConfig { + RemotePluginServiceConfig { + chatgpt_base_url: config.chatgpt_base_url.clone(), + } +} + fn featured_plugin_ids_cache_key( config: &Config, auth: Option<&CodexAuth>, @@ -503,8 +495,12 @@ impl PluginsManager { if let Some(featured_plugin_ids) = self.cached_featured_plugin_ids(&cache_key) { return Ok(featured_plugin_ids); } - let featured_plugin_ids = - fetch_remote_featured_plugin_ids(config, auth, self.restriction_product).await?; + let featured_plugin_ids = codex_core_plugins::remote::fetch_remote_featured_plugin_ids( + &remote_plugin_service_config(config), + auth, + self.restriction_product, + ) + .await?; self.write_featured_plugin_ids_cache(cache_key, &featured_plugin_ids); Ok(featured_plugin_ids) } @@ -536,9 +532,13 @@ impl PluginsManager { // This only forwards the backend mutation before the local install flow. We rely on // `plugin/list(forceRemoteSync=true)` to sync local state rather than doing an extra // reconcile pass here. - enable_remote_plugin(config, auth, &plugin_id) - .await - .map_err(PluginInstallError::from)?; + codex_core_plugins::remote::enable_remote_plugin( + &remote_plugin_service_config(config), + auth, + &plugin_id, + ) + .await + .map_err(PluginInstallError::from)?; self.install_resolved_plugin(resolved).await } @@ -619,9 +619,13 @@ impl PluginsManager { // This only forwards the backend mutation before the local uninstall flow. We rely on // `plugin/list(forceRemoteSync=true)` to sync local state rather than doing an extra // reconcile pass here. - uninstall_remote_plugin(config, auth, &plugin_key) - .await - .map_err(PluginUninstallError::from)?; + codex_core_plugins::remote::uninstall_remote_plugin( + &remote_plugin_service_config(config), + auth, + &plugin_key, + ) + .await + .map_err(PluginUninstallError::from)?; self.uninstall_plugin_id(plugin_id).await } @@ -670,9 +674,12 @@ impl PluginsManager { } info!("starting remote plugin sync"); - let remote_plugins = fetch_remote_plugin_status(config, auth) - .await - .map_err(PluginRemoteSyncError::from)?; + let remote_plugins = codex_core_plugins::remote::fetch_remote_plugin_status( + &remote_plugin_service_config(config), + auth, + ) + .await + .map_err(PluginRemoteSyncError::from)?; let configured_plugins = configured_plugins_from_stack(&config.config_layer_stack); let curated_marketplace_root = curated_plugins_repo_path(self.codex_home.as_path()); let curated_marketplace_path = AbsolutePathBuf::try_from( @@ -1026,7 +1033,9 @@ impl PluginsManager { })?; let description = manifest.description.clone(); let manifest_paths = &manifest.paths; - let skill_config_rules = skill_config_rules_from_stack(&config.config_layer_stack); + let skill_config_rules = codex_core_skills::config_rules::skill_config_rules_from_stack( + &config.config_layer_stack, + ); let resolved_skills = load_plugin_skills( &source_path, manifest_paths, @@ -1034,21 +1043,11 @@ impl PluginsManager { &skill_config_rules, ) .await; - let apps = load_apps_from_paths( - source_path.as_path(), - plugin_app_config_paths(source_path.as_path(), manifest_paths), - ) - .await; - let mcp_config_paths = plugin_mcp_config_paths(source_path.as_path(), manifest_paths); - let mut mcp_server_names = Vec::new(); - for mcp_config_path in mcp_config_paths { - mcp_server_names.extend( - load_mcp_servers_from_file(source_path.as_path(), &mcp_config_path) - .await - .mcp_servers - .into_keys(), - ); - } + let apps = load_plugin_apps(source_path.as_path()).await; + let mut mcp_server_names = load_plugin_mcp_servers(source_path.as_path()) + .await + .into_keys() + .collect::>(); mcp_server_names.sort_unstable(); mcp_server_names.dedup(); @@ -1160,13 +1159,8 @@ impl PluginsManager { .spawn( move || match sync_openai_plugins_repo(codex_home.as_path()) { Ok(curated_plugin_version) => { - let configured_curated_plugin_ids = curated_plugin_ids_from_config_keys( - configured_plugins_from_codex_home( - codex_home.as_path(), - "failed to read user config while refreshing curated plugin cache", - "failed to parse user config while refreshing curated plugin cache", - ), - ); + let configured_curated_plugin_ids = + configured_curated_plugin_ids_from_codex_home(codex_home.as_path()); match refresh_curated_plugin_cache( codex_home.as_path(), &curated_plugin_version, @@ -1352,224 +1346,6 @@ impl PluginUninstallError { } } -fn log_plugin_load_errors(outcome: &PluginLoadOutcome) { - for plugin in outcome - .plugins() - .iter() - .filter(|plugin| plugin.error.is_some()) - { - if let Some(error) = plugin.error.as_deref() { - warn!( - plugin = plugin.config_name, - path = %plugin.root.display(), - "failed to load plugin: {error}" - ); - } - } -} - -#[derive(Debug, Default, Deserialize)] -#[serde(rename_all = "camelCase")] -struct PluginMcpFile { - #[serde(default)] - mcp_servers: HashMap, -} - -#[derive(Debug, Default, Deserialize)] -#[serde(rename_all = "camelCase")] -struct PluginAppFile { - #[serde(default)] - apps: HashMap, -} - -#[derive(Debug, Default, Deserialize)] -struct PluginAppConfig { - id: String, -} - -pub(crate) async fn load_plugins_from_layer_stack( - config_layer_stack: &ConfigLayerStack, - store: &PluginStore, - restriction_product: Option, -) -> PluginLoadOutcome { - let skill_config_rules = skill_config_rules_from_stack(config_layer_stack); - let mut configured_plugins: Vec<_> = configured_plugins_from_stack(config_layer_stack) - .into_iter() - .collect(); - configured_plugins.sort_unstable_by(|(a, _), (b, _)| a.cmp(b)); - - let mut plugins = Vec::with_capacity(configured_plugins.len()); - let mut seen_mcp_server_names = HashMap::::new(); - for (configured_name, plugin) in configured_plugins { - let loaded_plugin = load_plugin( - configured_name.clone(), - &plugin, - store, - restriction_product, - &skill_config_rules, - ) - .await; - for name in loaded_plugin.mcp_servers.keys() { - if let Some(previous_plugin) = - seen_mcp_server_names.insert(name.clone(), configured_name.clone()) - { - warn!( - plugin = configured_name, - previous_plugin, - server = name, - "skipping duplicate plugin MCP server name" - ); - } - } - plugins.push(loaded_plugin); - } - - PluginLoadOutcome::from_plugins(plugins) -} - -fn refresh_curated_plugin_cache( - codex_home: &Path, - plugin_version: &str, - configured_curated_plugin_ids: &[PluginId], -) -> Result { - let store = PluginStore::new(codex_home.to_path_buf()); - let curated_marketplace_path = AbsolutePathBuf::try_from( - curated_plugins_repo_path(codex_home).join(".agents/plugins/marketplace.json"), - ) - .map_err(|_| "local curated marketplace is not available".to_string())?; - let curated_marketplace = load_marketplace(&curated_marketplace_path) - .map_err(|err| format!("failed to load curated marketplace for cache refresh: {err}"))?; - - let mut plugin_sources = HashMap::::new(); - for plugin in curated_marketplace.plugins { - let plugin_name = plugin.name; - if plugin_sources.contains_key(&plugin_name) { - warn!( - plugin = plugin_name, - marketplace = OPENAI_CURATED_MARKETPLACE_NAME, - "ignoring duplicate curated plugin entry during cache refresh" - ); - continue; - } - let source_path = match plugin.source { - MarketplacePluginSource::Local { path } => path, - }; - plugin_sources.insert(plugin_name, source_path); - } - - let mut cache_refreshed = false; - for plugin_id in configured_curated_plugin_ids { - if store.active_plugin_version(plugin_id).as_deref() == Some(plugin_version) { - continue; - } - - let Some(source_path) = plugin_sources.get(&plugin_id.plugin_name).cloned() else { - warn!( - plugin = plugin_id.plugin_name, - marketplace = OPENAI_CURATED_MARKETPLACE_NAME, - "configured curated plugin no longer exists in curated marketplace during cache refresh" - ); - continue; - }; - - store - .install_with_version(source_path, plugin_id.clone(), plugin_version.to_string()) - .map_err(|err| { - format!( - "failed to refresh curated plugin cache for {}: {err}", - plugin_id.as_key() - ) - })?; - cache_refreshed = true; - } - - Ok(cache_refreshed) -} - -fn refresh_non_curated_plugin_cache( - codex_home: &Path, - additional_roots: &[AbsolutePathBuf], -) -> Result { - let configured_non_curated_plugin_ids = - non_curated_plugin_ids_from_config_keys(configured_plugins_from_codex_home( - codex_home, - "failed to read user config while refreshing non-curated plugin cache", - "failed to parse user config while refreshing non-curated plugin cache", - )); - if configured_non_curated_plugin_ids.is_empty() { - return Ok(false); - } - let configured_non_curated_plugin_keys = configured_non_curated_plugin_ids - .iter() - .map(PluginId::as_key) - .collect::>(); - - let store = PluginStore::new(codex_home.to_path_buf()); - let marketplace_outcome = list_marketplaces(additional_roots) - .map_err(|err| format!("failed to discover marketplaces for cache refresh: {err}"))?; - let mut plugin_sources = HashMap::::new(); - - for marketplace in marketplace_outcome.marketplaces { - if marketplace.name == OPENAI_CURATED_MARKETPLACE_NAME { - continue; - } - - for plugin in marketplace.plugins { - let plugin_id = - PluginId::new(plugin.name.clone(), marketplace.name.clone()).map_err(|err| { - match err { - PluginIdError::Invalid(message) => { - format!("failed to prepare non-curated plugin cache refresh: {message}") - } - } - })?; - let plugin_key = plugin_id.as_key(); - if !configured_non_curated_plugin_keys.contains(&plugin_key) { - continue; - } - if plugin_sources.contains_key(&plugin_key) { - warn!( - plugin = plugin.name, - marketplace = marketplace.name, - "ignoring duplicate non-curated plugin entry during cache refresh" - ); - continue; - } - - let source_path = match plugin.source { - MarketplacePluginSource::Local { path } => path, - }; - let plugin_version = plugin_version_for_source(source_path.as_path()) - .map_err(|err| format!("failed to read plugin version for {plugin_key}: {err}"))?; - plugin_sources.insert(plugin_key, (source_path, plugin_version)); - } - } - - let mut cache_refreshed = false; - for plugin_id in configured_non_curated_plugin_ids { - let plugin_key = plugin_id.as_key(); - let Some((source_path, plugin_version)) = plugin_sources.get(&plugin_key).cloned() else { - warn!( - plugin = plugin_id.plugin_name, - marketplace = plugin_id.marketplace_name, - "configured non-curated plugin no longer exists in discovered marketplaces during cache refresh" - ); - continue; - }; - - if store.active_plugin_version(&plugin_id).as_deref() == Some(plugin_version.as_str()) { - continue; - } - - store - .install_with_version(source_path, plugin_id.clone(), plugin_version) - .map_err(|err| format!("failed to refresh plugin cache for {plugin_key}: {err}"))?; - cache_refreshed = true; - } - - Ok(cache_refreshed) -} - fn configured_plugins_from_stack( config_layer_stack: &ConfigLayerStack, ) -> HashMap { @@ -1595,523 +1371,6 @@ fn configured_plugins_from_user_config_value( } } -fn configured_plugins_from_codex_home( - codex_home: &Path, - read_error_message: &str, - parse_error_message: &str, -) -> HashMap { - let config_path = codex_home.join(CONFIG_TOML_FILE); - let user_config = match fs::read_to_string(&config_path) { - Ok(user_config) => user_config, - Err(err) if err.kind() == std::io::ErrorKind::NotFound => return HashMap::new(), - Err(err) => { - warn!( - path = %config_path.display(), - error = %err, - "{read_error_message}" - ); - return HashMap::new(); - } - }; - - let user_config = match toml::from_str::(&user_config) { - Ok(user_config) => user_config, - Err(err) => { - warn!( - path = %config_path.display(), - error = %err, - "{parse_error_message}" - ); - return HashMap::new(); - } - }; - - configured_plugins_from_user_config_value(&user_config) -} - -fn configured_plugin_ids( - configured_plugins: HashMap, - invalid_plugin_key_message: &str, -) -> Vec { - configured_plugins - .into_keys() - .filter_map(|plugin_key| match PluginId::parse(&plugin_key) { - Ok(plugin_id) => Some(plugin_id), - Err(err) => { - warn!( - plugin_key, - error = %err, - "{invalid_plugin_key_message}" - ); - None - } - }) - .collect() -} - -fn curated_plugin_ids_from_config_keys( - configured_plugins: HashMap, -) -> Vec { - let mut configured_curated_plugin_ids = configured_plugin_ids( - configured_plugins, - "ignoring invalid configured plugin key during curated sync setup", - ) - .into_iter() - .filter(|plugin_id| plugin_id.marketplace_name == OPENAI_CURATED_MARKETPLACE_NAME) - .collect::>(); - configured_curated_plugin_ids.sort_unstable_by_key(PluginId::as_key); - configured_curated_plugin_ids -} - -fn non_curated_plugin_ids_from_config_keys( - configured_plugins: HashMap, -) -> Vec { - let mut configured_non_curated_plugin_ids = configured_plugin_ids( - configured_plugins, - "ignoring invalid plugin key during non-curated cache refresh setup", - ) - .into_iter() - .filter(|plugin_id| plugin_id.marketplace_name != OPENAI_CURATED_MARKETPLACE_NAME) - .collect::>(); - configured_non_curated_plugin_ids.sort_unstable_by_key(PluginId::as_key); - configured_non_curated_plugin_ids -} - -async fn load_plugin( - config_name: String, - plugin: &PluginConfig, - store: &PluginStore, - restriction_product: Option, - skill_config_rules: &SkillConfigRules, -) -> LoadedPlugin { - let plugin_id = PluginId::parse(&config_name); - let active_plugin_root = plugin_id - .as_ref() - .ok() - .and_then(|plugin_id| store.active_plugin_root(plugin_id)); - let root = active_plugin_root - .clone() - .unwrap_or_else(|| match &plugin_id { - Ok(plugin_id) => store.plugin_base_root(plugin_id), - Err(_) => store.root().clone(), - }); - let mut loaded_plugin = LoadedPlugin { - config_name, - manifest_name: None, - manifest_description: None, - root, - enabled: plugin.enabled, - skill_roots: Vec::new(), - disabled_skill_paths: HashSet::new(), - has_enabled_skills: false, - mcp_servers: HashMap::new(), - apps: Vec::new(), - error: None, - }; - - if !plugin.enabled { - return loaded_plugin; - } - - let plugin_root = match plugin_id { - Ok(_) => match active_plugin_root { - Some(plugin_root) => plugin_root, - None => { - loaded_plugin.error = Some("plugin is not installed".to_string()); - return loaded_plugin; - } - }, - Err(err) => { - loaded_plugin.error = Some(err.to_string()); - return loaded_plugin; - } - }; - - if !plugin_root.as_path().is_dir() { - loaded_plugin.error = Some("path does not exist or is not a directory".to_string()); - return loaded_plugin; - } - - let Some(manifest) = load_plugin_manifest(plugin_root.as_path()) else { - loaded_plugin.error = Some("missing or invalid .codex-plugin/plugin.json".to_string()); - return loaded_plugin; - }; - - let manifest_paths = &manifest.paths; - loaded_plugin.manifest_name = manifest - .interface - .as_ref() - .and_then(|interface| interface.display_name.as_deref()) - .map(str::trim) - .filter(|display_name| !display_name.is_empty()) - .map(str::to_string) - .or_else(|| Some(manifest.name.clone())); - loaded_plugin.manifest_description = manifest.description.clone(); - loaded_plugin.skill_roots = plugin_skill_roots(&plugin_root, manifest_paths); - let resolved_skills = load_plugin_skills( - &plugin_root, - manifest_paths, - restriction_product, - skill_config_rules, - ) - .await; - let has_enabled_skills = resolved_skills.has_enabled_skills(); - loaded_plugin.disabled_skill_paths = resolved_skills.disabled_skill_paths; - loaded_plugin.has_enabled_skills = has_enabled_skills; - let mut mcp_servers = HashMap::new(); - for mcp_config_path in plugin_mcp_config_paths(plugin_root.as_path(), manifest_paths) { - let plugin_mcp = load_mcp_servers_from_file(plugin_root.as_path(), &mcp_config_path).await; - for (name, config) in plugin_mcp.mcp_servers { - if mcp_servers.insert(name.clone(), config).is_some() { - warn!( - plugin = %plugin_root.display(), - path = %mcp_config_path.display(), - server = name, - "plugin MCP file overwrote an earlier server definition" - ); - } - } - } - loaded_plugin.mcp_servers = mcp_servers; - loaded_plugin.apps = load_apps_from_paths( - plugin_root.as_path(), - plugin_app_config_paths(plugin_root.as_path(), manifest_paths), - ) - .await; - loaded_plugin -} - -struct ResolvedPluginSkills { - skills: Vec, - disabled_skill_paths: HashSet, - had_errors: bool, -} - -impl ResolvedPluginSkills { - fn has_enabled_skills(&self) -> bool { - // Keep the plugin visible in capability summaries if skill loading was partial. - self.had_errors - || self - .skills - .iter() - .any(|skill| !self.disabled_skill_paths.contains(&skill.path_to_skills_md)) - } -} - -async fn load_plugin_skills( - plugin_root: &AbsolutePathBuf, - manifest_paths: &PluginManifestPaths, - restriction_product: Option, - skill_config_rules: &SkillConfigRules, -) -> ResolvedPluginSkills { - let roots = plugin_skill_roots(plugin_root, manifest_paths) - .into_iter() - .map(|path| SkillRoot { - path, - scope: SkillScope::User, - file_system: Arc::clone(&LOCAL_FS), - }) - .collect::>(); - let outcome = load_skills_from_roots(roots).await; - let had_errors = !outcome.errors.is_empty(); - let skills = outcome - .skills - .into_iter() - .filter(|skill| skill.matches_product_restriction_for_product(restriction_product)) - .collect::>(); - let disabled_skill_paths = resolve_disabled_skill_paths(&skills, skill_config_rules); - - ResolvedPluginSkills { - skills, - disabled_skill_paths, - had_errors, - } -} - -fn plugin_skill_roots( - plugin_root: &AbsolutePathBuf, - manifest_paths: &PluginManifestPaths, -) -> Vec { - let mut paths = default_skill_roots(plugin_root); - if let Some(path) = &manifest_paths.skills { - paths.push(path.clone()); - } - paths.sort_unstable(); - paths.dedup(); - paths -} - -fn default_skill_roots(plugin_root: &AbsolutePathBuf) -> Vec { - let skills_dir = plugin_root.join(DEFAULT_SKILLS_DIR_NAME); - if skills_dir.is_dir() { - vec![skills_dir] - } else { - Vec::new() - } -} - -fn plugin_mcp_config_paths( - plugin_root: &Path, - manifest_paths: &PluginManifestPaths, -) -> Vec { - if let Some(path) = &manifest_paths.mcp_servers { - return vec![path.clone()]; - } - default_mcp_config_paths(plugin_root) -} - -fn default_mcp_config_paths(plugin_root: &Path) -> Vec { - let mut paths = Vec::new(); - let default_path = plugin_root.join(DEFAULT_MCP_CONFIG_FILE); - if default_path.is_file() - && let Ok(default_path) = AbsolutePathBuf::try_from(default_path) - { - paths.push(default_path); - } - paths.sort_unstable(); - paths.dedup(); - paths -} - -pub async fn load_plugin_apps(plugin_root: &Path) -> Vec { - if let Some(manifest) = load_plugin_manifest(plugin_root) { - return load_apps_from_paths( - plugin_root, - plugin_app_config_paths(plugin_root, &manifest.paths), - ) - .await; - } - load_apps_from_paths(plugin_root, default_app_config_paths(plugin_root)).await -} - -fn plugin_app_config_paths( - plugin_root: &Path, - manifest_paths: &PluginManifestPaths, -) -> Vec { - if let Some(path) = &manifest_paths.apps { - return vec![path.clone()]; - } - default_app_config_paths(plugin_root) -} - -fn default_app_config_paths(plugin_root: &Path) -> Vec { - let mut paths = Vec::new(); - let default_path = plugin_root.join(DEFAULT_APP_CONFIG_FILE); - if default_path.is_file() - && let Ok(default_path) = AbsolutePathBuf::try_from(default_path) - { - paths.push(default_path); - } - paths.sort_unstable(); - paths.dedup(); - paths -} - -async fn load_apps_from_paths( - plugin_root: &Path, - app_config_paths: Vec, -) -> Vec { - let mut connector_ids = Vec::new(); - for app_config_path in app_config_paths { - let Ok(contents) = tokio::fs::read_to_string(app_config_path.as_path()).await else { - continue; - }; - let parsed = match serde_json::from_str::(&contents) { - Ok(parsed) => parsed, - Err(err) => { - warn!( - path = %app_config_path.display(), - "failed to parse plugin app config: {err}" - ); - continue; - } - }; - - let mut apps: Vec = parsed.apps.into_values().collect(); - apps.sort_unstable_by(|left, right| left.id.cmp(&right.id)); - - connector_ids.extend(apps.into_iter().filter_map(|app| { - if app.id.trim().is_empty() { - warn!( - plugin = %plugin_root.display(), - "plugin app config is missing an app id" - ); - None - } else { - Some(AppConnectorId(app.id)) - } - })); - } - connector_ids.dedup(); - connector_ids -} - -pub async fn plugin_telemetry_metadata_from_root( - plugin_id: &PluginId, - plugin_root: &AbsolutePathBuf, -) -> PluginTelemetryMetadata { - let Some(manifest) = load_plugin_manifest(plugin_root.as_path()) else { - return PluginTelemetryMetadata::from_plugin_id(plugin_id); - }; - - let manifest_paths = &manifest.paths; - let has_skills = !plugin_skill_roots(plugin_root, manifest_paths).is_empty(); - let mut mcp_server_names = Vec::new(); - for path in plugin_mcp_config_paths(plugin_root.as_path(), manifest_paths) { - mcp_server_names.extend( - load_mcp_servers_from_file(plugin_root.as_path(), &path) - .await - .mcp_servers - .into_keys(), - ); - } - mcp_server_names.sort_unstable(); - mcp_server_names.dedup(); - - PluginTelemetryMetadata { - plugin_id: plugin_id.clone(), - capability_summary: Some(PluginCapabilitySummary { - config_name: plugin_id.as_key(), - display_name: plugin_id.plugin_name.clone(), - description: None, - has_skills, - mcp_server_names, - app_connector_ids: load_apps_from_paths( - plugin_root.as_path(), - plugin_app_config_paths(plugin_root.as_path(), manifest_paths), - ) - .await, - }), - } -} - -pub async fn load_plugin_mcp_servers(plugin_root: &Path) -> HashMap { - let Some(manifest) = load_plugin_manifest(plugin_root) else { - return HashMap::new(); - }; - - let mut mcp_servers = HashMap::new(); - for mcp_config_path in plugin_mcp_config_paths(plugin_root, &manifest.paths) { - let plugin_mcp = load_mcp_servers_from_file(plugin_root, &mcp_config_path).await; - for (name, config) in plugin_mcp.mcp_servers { - mcp_servers.entry(name).or_insert(config); - } - } - - mcp_servers -} - -pub async fn installed_plugin_telemetry_metadata( - codex_home: &Path, - plugin_id: &PluginId, -) -> PluginTelemetryMetadata { - let store = PluginStore::new(codex_home.to_path_buf()); - let Some(plugin_root) = store.active_plugin_root(plugin_id) else { - return PluginTelemetryMetadata::from_plugin_id(plugin_id); - }; - - plugin_telemetry_metadata_from_root(plugin_id, &plugin_root).await -} - -async fn load_mcp_servers_from_file( - plugin_root: &Path, - mcp_config_path: &AbsolutePathBuf, -) -> PluginMcpDiscovery { - let Ok(contents) = tokio::fs::read_to_string(mcp_config_path.as_path()).await else { - return PluginMcpDiscovery::default(); - }; - let parsed = match serde_json::from_str::(&contents) { - Ok(parsed) => parsed, - Err(err) => { - warn!( - path = %mcp_config_path.display(), - "failed to parse plugin MCP config: {err}" - ); - return PluginMcpDiscovery::default(); - } - }; - normalize_plugin_mcp_servers( - plugin_root, - parsed.mcp_servers, - mcp_config_path.to_string_lossy().as_ref(), - ) -} - -fn normalize_plugin_mcp_servers( - plugin_root: &Path, - plugin_mcp_servers: HashMap, - source: &str, -) -> PluginMcpDiscovery { - let mut mcp_servers = HashMap::new(); - - for (name, config_value) in plugin_mcp_servers { - let normalized = normalize_plugin_mcp_server_value(plugin_root, config_value); - match serde_json::from_value::(JsonValue::Object(normalized)) { - Ok(config) => { - mcp_servers.insert(name, config); - } - Err(err) => { - warn!( - plugin = %plugin_root.display(), - server = name, - "failed to parse plugin MCP server from {source}: {err}" - ); - } - } - } - - PluginMcpDiscovery { mcp_servers } -} - -fn normalize_plugin_mcp_server_value( - plugin_root: &Path, - value: JsonValue, -) -> JsonMap { - let mut object = match value { - JsonValue::Object(object) => object, - _ => return JsonMap::new(), - }; - - if let Some(JsonValue::String(transport_type)) = object.remove("type") { - match transport_type.as_str() { - "http" | "streamable_http" | "streamable-http" => {} - "stdio" => {} - other => { - warn!( - plugin = %plugin_root.display(), - transport = other, - "plugin MCP server uses an unknown transport type" - ); - } - } - } - - if let Some(JsonValue::Object(oauth)) = object.remove("oauth") - && oauth.contains_key("callbackPort") - { - warn!( - plugin = %plugin_root.display(), - "plugin MCP server OAuth callbackPort is ignored; Codex uses global MCP OAuth callback settings" - ); - } - - if let Some(JsonValue::String(cwd)) = object.get("cwd") - && !Path::new(cwd).is_absolute() - { - object.insert( - "cwd".to_string(), - JsonValue::String(plugin_root.join(cwd).display().to_string()), - ); - } - - object -} - -#[derive(Debug, Default)] -struct PluginMcpDiscovery { - mcp_servers: HashMap, -} - #[cfg(test)] #[path = "manager_tests.rs"] mod tests; diff --git a/codex-rs/core/src/plugins/manager_tests.rs b/codex-rs/core/src/plugins/manager_tests.rs index 23b202f1a8..e12f52c82b 100644 --- a/codex-rs/core/src/plugins/manager_tests.rs +++ b/codex-rs/core/src/plugins/manager_tests.rs @@ -6,7 +6,6 @@ use crate::config_loader::ConfigLayerStack; use crate::config_loader::ConfigRequirements; use crate::config_loader::ConfigRequirementsToml; use crate::plugins::LoadedPlugin; -use crate::plugins::MarketplacePluginInstallPolicy; use crate::plugins::PluginLoadOutcome; use crate::plugins::marketplace_install_root; use crate::plugins::test_support::TEST_CURATED_PLUGIN_SHA; @@ -14,12 +13,15 @@ use crate::plugins::test_support::write_curated_plugin_sha_with as write_curated use crate::plugins::test_support::write_file; use crate::plugins::test_support::write_openai_curated_marketplace; use codex_app_server_protocol::ConfigLayerSource; +use codex_config::McpServerConfig; use codex_config::types::McpServerTransportConfig; +use codex_core_plugins::marketplace::MarketplacePluginInstallPolicy; use codex_login::CodexAuth; use codex_protocol::protocol::Product; use codex_utils_absolute_path::test_support::PathBufExt; use pretty_assertions::assert_eq; use std::fs; +use std::path::Path; use tempfile::TempDir; use toml::Value; use wiremock::Mock; @@ -172,6 +174,7 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() { http_headers: None, env_http_headers: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -508,6 +511,7 @@ async fn load_plugins_uses_manifest_configured_component_paths() { http_headers: None, env_http_headers: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -617,6 +621,7 @@ async fn load_plugins_ignores_manifest_component_paths_without_dot_slash() { http_headers: None, env_http_headers: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -774,6 +779,7 @@ fn capability_index_filters_inactive_and_zero_capability_plugins() { http_headers: None, env_http_headers: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -2477,14 +2483,10 @@ enabled = true ); assert_eq!( - curated_plugin_ids_from_config_keys(configured_plugins_from_codex_home( - tmp.path(), - "failed to read user config while refreshing curated plugin cache", - "failed to parse user config while refreshing curated plugin cache", - )) - .into_iter() - .map(|plugin_id| plugin_id.as_key()) - .collect::>(), + configured_curated_plugin_ids_from_codex_home(tmp.path()) + .into_iter() + .map(|plugin_id| plugin_id.as_key()) + .collect::>(), vec!["slack@openai-curated".to_string()] ); @@ -2496,11 +2498,7 @@ plugins = true ); assert_eq!( - curated_plugin_ids_from_config_keys(configured_plugins_from_codex_home( - tmp.path(), - "failed to read user config while refreshing curated plugin cache", - "failed to parse user config while refreshing curated plugin cache", - )), + configured_curated_plugin_ids_from_codex_home(tmp.path()), Vec::::new() ); } diff --git a/codex-rs/core/src/plugins/marketplace_add/metadata.rs b/codex-rs/core/src/plugins/marketplace_add/metadata.rs index b9412e7f0b..f21ed0faa2 100644 --- a/codex-rs/core/src/plugins/marketplace_add/metadata.rs +++ b/codex-rs/core/src/plugins/marketplace_add/metadata.rs @@ -1,10 +1,10 @@ use super::MarketplaceAddError; use super::MarketplaceSource; use crate::plugins::installed_marketplaces::resolve_configured_marketplace_root; -use crate::plugins::validate_marketplace_root; use codex_config::CONFIG_TOML_FILE; use codex_config::MarketplaceConfigUpdate; use codex_config::record_user_marketplace; +use codex_core_plugins::marketplace::validate_marketplace_root; use std::fs; use std::io::ErrorKind; use std::path::Path; diff --git a/codex-rs/core/src/plugins/marketplace_add/source.rs b/codex-rs/core/src/plugins/marketplace_add/source.rs index 53d52058aa..9a0b666c60 100644 --- a/codex-rs/core/src/plugins/marketplace_add/source.rs +++ b/codex-rs/core/src/plugins/marketplace_add/source.rs @@ -1,6 +1,6 @@ use super::MarketplaceAddError; -use crate::plugins::validate_marketplace_root; use crate::plugins::validate_plugin_segment; +use codex_core_plugins::marketplace::validate_marketplace_root; use std::path::Path; use std::path::PathBuf; @@ -126,7 +126,9 @@ fn normalize_git_url(url: &str) -> String { fn looks_like_local_path(source: &str) -> bool { Path::new(source).is_absolute() || source.starts_with("./") + || source.starts_with(".\\") || source.starts_with("../") + || source.starts_with("..\\") || source.starts_with("~/") || source == "." || source == ".." diff --git a/codex-rs/core/src/plugins/mod.rs b/codex-rs/core/src/plugins/mod.rs index 7ffb80b449..57997eb134 100644 --- a/codex-rs/core/src/plugins/mod.rs +++ b/codex-rs/core/src/plugins/mod.rs @@ -4,17 +4,12 @@ mod discoverable; mod injection; mod installed_marketplaces; mod manager; -mod manifest; -mod marketplace; mod marketplace_add; mod mentions; -mod remote; mod render; mod startup_sync; -mod store; #[cfg(test)] pub(crate) mod test_support; -mod toggles; pub use codex_plugin::AppConnectorId; pub use codex_plugin::EffectiveSkillRoots; @@ -45,32 +40,15 @@ pub use manager::PluginRemoteSyncError; pub use manager::PluginUninstallError; pub use manager::PluginsManager; pub use manager::RemotePluginSyncResult; -pub use manager::installed_plugin_telemetry_metadata; -pub use manager::load_plugin_apps; -pub use manager::load_plugin_mcp_servers; -pub use manager::plugin_telemetry_metadata_from_root; -pub use manifest::PluginManifestInterface; -pub(crate) use manifest::PluginManifestPaths; -pub(crate) use manifest::load_plugin_manifest; -pub use marketplace::MarketplaceError; -pub use marketplace::MarketplaceListError; -pub use marketplace::MarketplacePluginAuthPolicy; -pub use marketplace::MarketplacePluginInstallPolicy; -pub use marketplace::MarketplacePluginPolicy; -pub use marketplace::MarketplacePluginSource; -pub use marketplace::validate_marketplace_root; pub use marketplace_add::MarketplaceAddError; pub use marketplace_add::MarketplaceAddOutcome; pub use marketplace_add::MarketplaceAddRequest; pub use marketplace_add::add_marketplace; -pub use remote::RemotePluginFetchError; -pub use remote::fetch_remote_featured_plugin_ids; pub(crate) use render::render_explicit_plugin_instructions; pub(crate) use render::render_plugins_section; pub(crate) use startup_sync::curated_plugins_repo_path; pub(crate) use startup_sync::read_curated_plugins_sha; pub(crate) use startup_sync::sync_openai_plugins_repo; -pub use toggles::collect_plugin_enabled_candidates; pub(crate) use mentions::build_connector_slug_counts; pub(crate) use mentions::build_skill_name_counts; diff --git a/codex-rs/core/src/stream_events_utils.rs b/codex-rs/core/src/stream_events_utils.rs index d491bb6adb..3838d6ed01 100644 --- a/codex-rs/core/src/stream_events_utils.rs +++ b/codex-rs/core/src/stream_events_utils.rs @@ -136,11 +136,20 @@ pub(crate) async fn record_completed_response_item( sess.defer_mailbox_delivery_to_next_turn(&turn_context.sub_id) .await; } - maybe_mark_thread_memory_mode_polluted_from_web_search(sess, turn_context, item).await; + mark_thread_memory_mode_polluted_if_external_context(sess, turn_context, item).await; record_stage1_output_usage_for_completed_item(turn_context, item).await; } -async fn maybe_mark_thread_memory_mode_polluted_from_web_search( +fn response_item_may_include_external_context(item: &ResponseItem) -> bool { + matches!( + item, + ResponseItem::ToolSearchCall { .. } + | ResponseItem::ToolSearchOutput { .. } + | ResponseItem::WebSearchCall { .. } + ) +} + +pub(crate) async fn mark_thread_memory_mode_polluted_if_external_context( sess: &Session, turn_context: &TurnContext, item: &ResponseItem, @@ -149,7 +158,7 @@ async fn maybe_mark_thread_memory_mode_polluted_from_web_search( .config .memories .no_memories_if_mcp_or_web_search - || !matches!(item, ResponseItem::WebSearchCall { .. }) + || !response_item_may_include_external_context(item) { return; } diff --git a/codex-rs/core/src/stream_events_utils_tests.rs b/codex-rs/core/src/stream_events_utils_tests.rs index e3947a06ed..2634a88742 100644 --- a/codex-rs/core/src/stream_events_utils_tests.rs +++ b/codex-rs/core/src/stream_events_utils_tests.rs @@ -2,11 +2,16 @@ use super::completed_item_defers_mailbox_delivery_to_next_turn; use super::handle_non_tool_response_item; use super::image_generation_artifact_path; use super::last_assistant_message_from_item; +use super::response_item_may_include_external_context; use super::save_image_generation_result; use crate::codex::make_session_and_context; use codex_protocol::error::CodexErr; use codex_protocol::items::TurnItem; use codex_protocol::models::ContentItem; +use codex_protocol::models::FunctionCallOutputPayload; +use codex_protocol::models::LocalShellAction; +use codex_protocol::models::LocalShellExecAction; +use codex_protocol::models::LocalShellStatus; use codex_protocol::models::MessagePhase; use codex_protocol::models::ResponseItem; use codex_utils_absolute_path::test_support::PathExt; @@ -28,6 +33,84 @@ fn assistant_output_text_with_phase(text: &str, phase: Option) -> } } +#[test] +fn external_context_pollution_items_include_web_search_and_tool_search() { + let polluting_items = [ + ResponseItem::WebSearchCall { + id: None, + status: Some("completed".to_string()), + action: None, + }, + ResponseItem::ToolSearchCall { + id: None, + call_id: Some("search-1".to_string()), + status: None, + execution: "client".to_string(), + arguments: serde_json::json!({"query": "calendar"}), + }, + ResponseItem::ToolSearchOutput { + call_id: Some("search-1".to_string()), + status: "completed".to_string(), + execution: "client".to_string(), + tools: Vec::new(), + }, + ]; + + assert!( + polluting_items + .iter() + .all(response_item_may_include_external_context) + ); +} + +#[test] +fn external_context_pollution_items_exclude_local_tool_calls() { + let non_polluting_items = [ + ResponseItem::LocalShellCall { + id: None, + call_id: Some("shell-1".to_string()), + status: LocalShellStatus::Completed, + action: LocalShellAction::Exec(LocalShellExecAction { + command: vec!["cat".to_string(), "README.md".to_string()], + timeout_ms: None, + working_directory: None, + env: None, + user: None, + }), + }, + ResponseItem::FunctionCall { + id: None, + name: "shell".to_string(), + namespace: None, + arguments: "{}".to_string(), + call_id: "call-1".to_string(), + }, + ResponseItem::FunctionCallOutput { + call_id: "call-1".to_string(), + output: FunctionCallOutputPayload::from_text("ok".to_string()), + }, + ResponseItem::CustomToolCall { + id: None, + status: None, + call_id: "custom-1".to_string(), + name: "apply_patch".to_string(), + input: "*** Begin Patch\n*** End Patch\n".to_string(), + }, + ResponseItem::CustomToolCallOutput { + call_id: "custom-1".to_string(), + name: Some("apply_patch".to_string()), + output: FunctionCallOutputPayload::from_text("ok".to_string()), + }, + assistant_output_text("plain assistant text"), + ]; + + assert!( + !non_polluting_items + .iter() + .any(response_item_may_include_external_context) + ); +} + #[tokio::test] async fn handle_non_tool_response_item_strips_citations_from_assistant_message() { let (session, turn_context) = make_session_and_context().await; diff --git a/codex-rs/core/src/tools/js_repl/mod.rs b/codex-rs/core/src/tools/js_repl/mod.rs index 5c0884d9ee..48bfb8ac41 100644 --- a/codex-rs/core/src/tools/js_repl/mod.rs +++ b/codex-rs/core/src/tools/js_repl/mod.rs @@ -1033,18 +1033,13 @@ impl JsReplManager { } let sandbox = SandboxManager::new(); - let has_managed_network_requirements = turn - .config - .config_layer_stack - .requirements_toml() - .network - .is_some(); + let managed_network_active = turn.network.is_some(); let sandbox_type = sandbox.select_initial( &turn.file_system_sandbox_policy, turn.network_sandbox_policy, SandboxablePreference::Auto, turn.windows_sandbox_level, - has_managed_network_requirements, + managed_network_active, ); let command = SandboxCommand { program: node_path.into_os_string(), @@ -1067,7 +1062,7 @@ impl JsReplManager { file_system_policy: &turn.file_system_sandbox_policy, network_policy: turn.network_sandbox_policy, sandbox: sandbox_type, - enforce_managed_network: has_managed_network_requirements, + enforce_managed_network: managed_network_active, network: None, sandbox_policy_cwd: &turn.cwd, codex_linux_sandbox_exe: turn.codex_linux_sandbox_exe.as_deref(), diff --git a/codex-rs/core/src/tools/network_approval.rs b/codex-rs/core/src/tools/network_approval.rs index c91b6df7ca..de689ea25d 100644 --- a/codex-rs/core/src/tools/network_approval.rs +++ b/codex-rs/core/src/tools/network_approval.rs @@ -578,11 +578,11 @@ pub(crate) fn build_network_policy_decider( pub(crate) async fn begin_network_approval( session: &Session, turn_id: &str, - has_managed_network_requirements: bool, + managed_network_active: bool, spec: Option, ) -> Option { let spec = spec?; - if !has_managed_network_requirements || spec.network.is_none() { + if !managed_network_active || spec.network.is_none() { return None; } diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index 36450da3f7..f9b2a64966 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -55,7 +55,7 @@ impl ToolOrchestrator { req: &Rq, tool_ctx: &ToolCtx, attempt: &SandboxAttempt<'_>, - has_managed_network_requirements: bool, + managed_network_active: bool, ) -> (Result, Option) where T: ToolRuntime, @@ -63,7 +63,7 @@ impl ToolOrchestrator { let network_approval = begin_network_approval( &tool_ctx.session, &tool_ctx.turn.sub_id, - has_managed_network_requirements, + managed_network_active, tool.network_approval_spec(req, tool_ctx), ) .await; @@ -180,12 +180,7 @@ impl ToolOrchestrator { } // 2) First attempt under the selected sandbox. - let has_managed_network_requirements = turn_ctx - .config - .config_layer_stack - .requirements_toml() - .network - .is_some(); + let managed_network_active = turn_ctx.network.is_some(); let initial_sandbox = match tool.sandbox_mode_for_first_attempt(req) { SandboxOverride::BypassSandboxFirstAttempt => SandboxType::None, SandboxOverride::NoOverride => self.sandbox.select_initial( @@ -193,7 +188,7 @@ impl ToolOrchestrator { turn_ctx.network_sandbox_policy, tool.sandbox_preference(), turn_ctx.windows_sandbox_level, - has_managed_network_requirements, + managed_network_active, ), }; @@ -204,7 +199,7 @@ impl ToolOrchestrator { policy: &turn_ctx.sandbox_policy, file_system_policy: &turn_ctx.file_system_sandbox_policy, network_policy: turn_ctx.network_sandbox_policy, - enforce_managed_network: has_managed_network_requirements, + enforce_managed_network: managed_network_active, manager: &self.sandbox, sandbox_cwd: &turn_ctx.cwd, codex_linux_sandbox_exe: turn_ctx.codex_linux_sandbox_exe.as_ref(), @@ -221,7 +216,7 @@ impl ToolOrchestrator { req, tool_ctx, &initial_attempt, - has_managed_network_requirements, + managed_network_active, ) .await; match first_result { @@ -236,7 +231,7 @@ impl ToolOrchestrator { output, network_policy_decision, }))) => { - let network_approval_context = if has_managed_network_requirements { + let network_approval_context = if managed_network_active { network_policy_decision .as_ref() .and_then(network_approval_context_from_payload) @@ -341,7 +336,7 @@ impl ToolOrchestrator { policy: &turn_ctx.sandbox_policy, file_system_policy: &turn_ctx.file_system_sandbox_policy, network_policy: turn_ctx.network_sandbox_policy, - enforce_managed_network: has_managed_network_requirements, + enforce_managed_network: managed_network_active, manager: &self.sandbox, sandbox_cwd: &turn_ctx.cwd, codex_linux_sandbox_exe: None, @@ -359,7 +354,7 @@ impl ToolOrchestrator { req, tool_ctx, &escalated_attempt, - has_managed_network_requirements, + managed_network_active, ) .await; retry_result.map(|output| OrchestratorRunResult { diff --git a/codex-rs/core/src/tools/spec_tests.rs b/codex-rs/core/src/tools/spec_tests.rs index fa52aa6b0f..9de9c81a92 100644 --- a/codex-rs/core/src/tools/spec_tests.rs +++ b/codex-rs/core/src/tools/spec_tests.rs @@ -417,6 +417,7 @@ async fn test_build_specs_gpt5_codex_default() { "request_user_input", "apply_patch", "web_search", + "image_generation", "view_image", "spawn_agent", "send_input", @@ -441,6 +442,7 @@ async fn test_build_specs_gpt51_codex_default() { "request_user_input", "apply_patch", "web_search", + "image_generation", "view_image", "spawn_agent", "send_input", @@ -467,6 +469,7 @@ async fn test_build_specs_gpt5_codex_unified_exec_web_search() { "request_user_input", "apply_patch", "web_search", + "image_generation", "view_image", "spawn_agent", "send_input", @@ -493,6 +496,7 @@ async fn test_build_specs_gpt51_codex_unified_exec_web_search() { "request_user_input", "apply_patch", "web_search", + "image_generation", "view_image", "spawn_agent", "send_input", @@ -517,6 +521,7 @@ async fn test_gpt_5_1_codex_max_defaults() { "request_user_input", "apply_patch", "web_search", + "image_generation", "view_image", "spawn_agent", "send_input", @@ -541,6 +546,7 @@ async fn test_codex_5_1_mini_defaults() { "request_user_input", "apply_patch", "web_search", + "image_generation", "view_image", "spawn_agent", "send_input", @@ -564,6 +570,7 @@ async fn test_gpt_5_defaults() { "update_plan", "request_user_input", "web_search", + "image_generation", "view_image", "spawn_agent", "send_input", @@ -588,6 +595,7 @@ async fn test_gpt_5_1_defaults() { "request_user_input", "apply_patch", "web_search", + "image_generation", "view_image", "spawn_agent", "send_input", @@ -614,6 +622,7 @@ async fn test_gpt_5_1_codex_max_unified_exec_web_search() { "request_user_input", "apply_patch", "web_search", + "image_generation", "view_image", "spawn_agent", "send_input", diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index 92176c338f..a5aba950c9 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -7,6 +7,7 @@ use std::process::Command; use std::sync::Arc; use std::sync::atomic::AtomicU64; use std::sync::atomic::Ordering; +use std::time::Duration; use anyhow::Context; use anyhow::Result; @@ -49,8 +50,8 @@ use crate::responses::WebSocketTestServer; use crate::responses::output_value_to_text; use crate::responses::start_mock_server; use crate::streaming_sse::StreamingSseServer; -use crate::wait_for_event; use crate::wait_for_event_match; +use crate::wait_for_event_with_timeout; use wiremock::Match; use wiremock::matchers::path_regex; @@ -61,6 +62,7 @@ type WorkspaceSetup = dyn FnOnce(AbsolutePathBuf, Arc) - const TEST_MODEL_WITH_EXPERIMENTAL_TOOLS: &str = "test-gpt-5.1-codex"; const REMOTE_EXEC_SERVER_URL_ENV_VAR: &str = "CODEX_TEST_REMOTE_EXEC_SERVER_URL"; static REMOTE_TEST_INSTANCE_COUNTER: AtomicU64 = AtomicU64::new(0); +const SUBMIT_TURN_COMPLETE_TIMEOUT: Duration = Duration::from_secs(30); #[derive(Debug)] pub struct TestEnv { @@ -637,10 +639,14 @@ impl TestCodex { _ => None, }) .await; - wait_for_event(&self.codex, |event| match event { - EventMsg::TurnComplete(event) => event.turn_id == turn_id, - _ => false, - }) + wait_for_event_with_timeout( + &self.codex, + |event| match event { + EventMsg::TurnComplete(event) => event.turn_id == turn_id, + _ => false, + }, + SUBMIT_TURN_COMPLETE_TIMEOUT, + ) .await; Ok(()) } diff --git a/codex-rs/core/tests/responses_headers.rs b/codex-rs/core/tests/responses_headers.rs index 8498956517..db4dc794be 100644 --- a/codex-rs/core/tests/responses_headers.rs +++ b/codex-rs/core/tests/responses_headers.rs @@ -575,16 +575,17 @@ async fn responses_stream_includes_turn_metadata_header_for_git_workspace_e2e() .and_then(serde_json::Value::as_str), Some(expected_head.as_str()) ); - let actual_origin = workspace + if let Some(actual_origin) = workspace .get("associated_remote_urls") .and_then(serde_json::Value::as_object) .and_then(|remotes| remotes.get("origin")) .and_then(serde_json::Value::as_str) - .expect("origin remote should be present"); - assert_eq!( - normalize_git_remote_url(actual_origin), - normalize_git_remote_url(&expected_origin) - ); + { + assert_eq!( + normalize_git_remote_url(actual_origin), + normalize_git_remote_url(&expected_origin) + ); + } assert_eq!( workspace .get("has_changes") diff --git a/codex-rs/core/tests/suite/approvals.rs b/codex-rs/core/tests/suite/approvals.rs index c5f7760195..208b45d594 100644 --- a/codex-rs/core/tests/suite/approvals.rs +++ b/codex-rs/core/tests/suite/approvals.rs @@ -226,12 +226,11 @@ impl ActionKind { let _ = fs::remove_file(&path); let patch = build_add_file_patch(&patch_path, content); let command = shell_apply_patch_command(&patch); - let event = shell_event( - call_id, - &command, - /*timeout_ms*/ 30_000, - sandbox_permissions, - )?; + // Bazel may need to launch the configured Codex helper binary + // to apply the verified patch, which can exceed the normal + // short command timeout on slower CI runners. + let timeout_ms = 30_000; + let event = shell_event(call_id, &command, timeout_ms, sandbox_permissions)?; Ok((event, Some(command))) } } diff --git a/codex-rs/core/tests/suite/code_mode.rs b/codex-rs/core/tests/suite/code_mode.rs index d514e801e9..a428b32ba2 100644 --- a/codex-rs/core/tests/suite/code_mode.rs +++ b/codex-rs/core/tests/suite/code_mode.rs @@ -6,6 +6,8 @@ use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; use codex_config::types::McpServerConfig; use codex_config::types::McpServerTransportConfig; use codex_features::Feature; +use codex_login::CodexAuth; +use codex_models_manager::bundled_models_response; use codex_protocol::dynamic_tools::DynamicToolCallOutputContentItem; use codex_protocol::dynamic_tools::DynamicToolResponse; use codex_protocol::dynamic_tools::DynamicToolSpec; @@ -14,6 +16,7 @@ use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::Op; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::user_input::UserInput; +use core_test_support::apps_test_server::AppsTestServer; use core_test_support::assert_regex_match; use core_test_support::responses; use core_test_support::responses::ResponseMock; @@ -229,6 +232,7 @@ async fn run_code_mode_turn_with_rmcp_config( env_vars: Vec::new(), cwd: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -346,6 +350,131 @@ async fn code_mode_only_restricts_prompt_tools() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn code_mode_only_guides_all_tools_search_and_calls_deferred_app_tools() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = responses::start_mock_server().await; + let apps_server = AppsTestServer::mount_searchable(&server).await?; + let resp_mock = responses::mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-1"), + ev_custom_tool_call( + "call-1", + "exec", + r#" +const tool = ALL_TOOLS.find( + ({ name }) => name === "mcp__codex_apps__calendar_timezone_option_99" +); +if (!tool) { + text(JSON.stringify({ found: false })); +} else { + const result = await tools[tool.name]({ timezone: "UTC" }); + text(JSON.stringify({ + found: true, + isError: Boolean(result.isError), + text: result.content?.[0]?.text ?? "", + })); +} +"#, + ), + ev_completed("resp-1"), + ]), + ) + .await; + let follow_up_mock = responses::mount_sse_once( + &server, + sse(vec![ + ev_assistant_message("msg-1", "done"), + ev_completed("resp-2"), + ]), + ) + .await; + + let apps_base_url = apps_server.chatgpt_base_url.clone(); + let mut builder = test_codex() + .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) + .with_config(move |config| { + config + .features + .enable(Feature::Apps) + .expect("test config should allow feature update"); + config + .features + .enable(Feature::ToolSearch) + .expect("test config should allow feature update"); + config + .features + .enable(Feature::CodeMode) + .expect("test config should allow feature update"); + config + .features + .enable(Feature::CodeModeOnly) + .expect("test config should allow feature update"); + config.chatgpt_base_url = apps_base_url; + config.model = Some("gpt-5-codex".to_string()); + + let mut model_catalog = bundled_models_response() + .unwrap_or_else(|err| panic!("bundled models.json should parse: {err}")); + let model = model_catalog + .models + .iter_mut() + .find(|model| model.slug == "gpt-5-codex") + .expect("gpt-5-codex exists in bundled models.json"); + model.supports_search_tool = true; + config.model_catalog = Some(model_catalog); + }); + let test = builder.build(&server).await?; + test.submit_turn("inspect tools in code mode only").await?; + + let first_body = resp_mock.single_request().body_json(); + assert_eq!( + tool_names(&first_body), + vec!["exec".to_string(), "wait".to_string()] + ); + + let exec_description = first_body + .get("tools") + .and_then(Value::as_array) + .and_then(|tools| { + tools.iter().find_map(|tool| { + if tool + .get("name") + .or_else(|| tool.get("type")) + .and_then(Value::as_str) + == Some("exec") + { + tool.get("description").and_then(Value::as_str) + } else { + None + } + }) + }) + .expect("exec description should be present"); + assert!(exec_description.contains("filter `ALL_TOOLS` by `name` and `description`")); + assert!(!exec_description.contains("calendar_timezone_option_99")); + + let request = follow_up_mock.single_request(); + let (output, success) = custom_tool_output_body_and_success(&request, "call-1"); + assert_ne!( + success, + Some(false), + "code_mode_only deferred app tool call failed unexpectedly: {output}" + ); + let parsed: Value = serde_json::from_str(&output)?; + assert_eq!( + parsed, + serde_json::json!({ + "found": true, + "isError": false, + "text": "called calendar_timezone_option_99 for at with ", + }) + ); + + Ok(()) +} + #[cfg_attr(windows, ignore = "no exec_command on Windows")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn code_mode_only_can_call_nested_tools() -> Result<()> { diff --git a/codex-rs/core/tests/suite/compact.rs b/codex-rs/core/tests/suite/compact.rs index dc5f127df5..bdbc1edbd7 100644 --- a/codex-rs/core/tests/suite/compact.rs +++ b/codex-rs/core/tests/suite/compact.rs @@ -2838,10 +2838,12 @@ async fn auto_compact_counts_encrypted_reasoning_before_last_user() { ]; let compact_mock = mount_compact_json_once(&server, serde_json::json!({ "output": compacted_history })).await; + let chatgpt_base_url = format!("{}/backend-api", server.uri()); let codex = test_codex() .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) - .with_config(|config| { + .with_config(move |config| { + config.chatgpt_base_url = chatgpt_base_url; set_test_compact_prompt(config); config.model_auto_compact_token_limit = Some(300); }) diff --git a/codex-rs/core/tests/suite/compact_remote.rs b/codex-rs/core/tests/suite/compact_remote.rs index 8322046d10..4b1eaa44d2 100644 --- a/codex-rs/core/tests/suite/compact_remote.rs +++ b/codex-rs/core/tests/suite/compact_remote.rs @@ -34,8 +34,10 @@ use core_test_support::test_codex::TestCodexHarness; use core_test_support::test_codex::test_codex; use core_test_support::wait_for_event; use core_test_support::wait_for_event_match; +use core_test_support::wait_for_event_with_timeout; use pretty_assertions::assert_eq; use serde_json::json; +use tokio::time::Duration; use wiremock::ResponseTemplate; fn approx_token_count(text: &str) -> i64 { @@ -55,6 +57,7 @@ fn estimate_compact_payload_tokens(request: &responses::ResponsesRequest) -> i64 const PRETURN_CONTEXT_DIFF_CWD: &str = "/tmp/PRETURN_CONTEXT_DIFF_CWD"; const DUMMY_FUNCTION_NAME: &str = "test_tool"; +const REMOTE_COMPACT_TURN_COMPLETE_TIMEOUT: Duration = Duration::from_secs(30); fn summary_with_prefix(summary: &str) -> String { format!("{SUMMARY_PREFIX}\n{summary}") @@ -197,6 +200,15 @@ fn assert_request_contains_realtime_end(request: &responses::ResponsesRequest) { ); } +async fn wait_for_turn_complete(codex: &codex_core::CodexThread) { + wait_for_event_with_timeout( + codex, + |ev| matches!(ev, EventMsg::TurnComplete(_)), + REMOTE_COMPACT_TURN_COMPLETE_TIMEOUT, + ) + .await; +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn remote_compact_replaces_history_for_followups() -> Result<()> { skip_if_no_network!(Ok(())); @@ -242,10 +254,10 @@ async fn remote_compact_replaces_history_for_followups() -> Result<()> { responsesapi_client_metadata: None, }) .await?; - wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + wait_for_turn_complete(&codex).await; codex.submit(Op::Compact).await?; - wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + wait_for_turn_complete(&codex).await; codex .submit(Op::UserInput { @@ -257,7 +269,7 @@ async fn remote_compact_replaces_history_for_followups() -> Result<()> { responsesapi_client_metadata: None, }) .await?; - wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + wait_for_turn_complete(&codex).await; let compact_request = compact_mock.single_request(); assert_eq!(compact_request.path(), "/v1/responses/compact"); diff --git a/codex-rs/core/tests/suite/js_repl.rs b/codex-rs/core/tests/suite/js_repl.rs index 2016897b3b..9c5eaef191 100644 --- a/codex-rs/core/tests/suite/js_repl.rs +++ b/codex-rs/core/tests/suite/js_repl.rs @@ -622,6 +622,7 @@ async fn js_repl_can_invoke_mcp_tools_by_display_name() -> Result<()> { env_vars: Vec::new(), cwd: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/tests/suite/responses_api_proxy_headers.rs b/codex-rs/core/tests/suite/responses_api_proxy_headers.rs index 8199956ee0..67f6e86ab0 100644 --- a/codex-rs/core/tests/suite/responses_api_proxy_headers.rs +++ b/codex-rs/core/tests/suite/responses_api_proxy_headers.rs @@ -1,6 +1,5 @@ -//! Exercises a real `responses-api-proxy` process with request dumping enabled, then verifies that -//! parent and spawned subagent requests carry the expected window, parent-thread, and subagent -//! identity headers in the dumped Responses API requests. +//! Verifies that parent and spawned subagent Responses API requests carry the expected window, +//! parent-thread, and subagent identity headers. use anyhow::Result; use anyhow::anyhow; @@ -10,6 +9,8 @@ use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::Op; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::user_input::UserInput; +use core_test_support::responses::ResponseMock; +use core_test_support::responses::ResponsesRequest; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_function_call; @@ -21,119 +22,28 @@ use core_test_support::skip_if_no_network; use core_test_support::test_codex::TestCodex; use core_test_support::test_codex::test_codex; use pretty_assertions::assert_eq; -use serde_json::Value; use serde_json::json; -use std::io::ErrorKind; -use std::io::Write; -use std::path::Path; -use std::process::Child; -use std::process::Command as StdCommand; -use std::process::Stdio; use std::time::Duration; -use std::time::Instant; -use tempfile::TempDir; const PARENT_PROMPT: &str = "spawn a subagent and report when it is started"; const CHILD_PROMPT: &str = "child: say done"; const SPAWN_CALL_ID: &str = "spawn-call-1"; -const PROXY_START_TIMEOUT: Duration = Duration::from_secs(/*secs*/ 30); -const PROXY_POLL_INTERVAL: Duration = Duration::from_millis(/*millis*/ 20); +const REQUEST_POLL_INTERVAL: Duration = Duration::from_millis(/*millis*/ 20); const TURN_TIMEOUT: Duration = Duration::from_secs(/*secs*/ 60); -struct ResponsesApiProxy { - child: Child, - port: u16, -} - -impl ResponsesApiProxy { - fn start(upstream_url: &str, dump_dir: &Path) -> Result { - let server_info = dump_dir.join("server-info.json"); - let (proxy_program, use_codex_multitool) = - match codex_utils_cargo_bin::cargo_bin("codex-responses-api-proxy") { - Ok(path) => (path, false), - Err(_) => (codex_utils_cargo_bin::cargo_bin("codex")?, true), - }; - let mut command = StdCommand::new(proxy_program); - if use_codex_multitool { - command.arg("responses-api-proxy"); - } - let mut child = command - .args(["--server-info"]) - .arg(&server_info) - .args(["--upstream-url", upstream_url, "--dump-dir"]) - .arg(dump_dir) - .stdin(Stdio::piped()) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .spawn()?; - - child - .stdin - .take() - .ok_or_else(|| anyhow!("responses-api-proxy stdin was not piped"))? - .write_all(b"dummy\n")?; - - let deadline = Instant::now() + PROXY_START_TIMEOUT; - loop { - match std::fs::read_to_string(&server_info) { - Ok(info) => { - if !info.trim().is_empty() { - match serde_json::from_str::(&info) { - Ok(info) => { - let port = info - .get("port") - .and_then(Value::as_u64) - .ok_or_else(|| anyhow!("proxy server info missing port"))?; - return Ok(Self { - child, - port: u16::try_from(port)?, - }); - } - Err(err) if err.is_eof() => {} - Err(err) => return Err(err.into()), - } - } - } - Err(err) if err.kind() == ErrorKind::NotFound => {} - Err(err) => return Err(err.into()), - } - if let Some(status) = child.try_wait()? { - return Err(anyhow!( - "responses-api-proxy exited before writing server info: {status}" - )); - } - if Instant::now() >= deadline { - return Err(anyhow!("timed out waiting for responses-api-proxy")); - } - std::thread::sleep(PROXY_POLL_INTERVAL); - } - } - - fn base_url(&self) -> String { - format!("http://127.0.0.1:{}/v1", self.port) - } -} - -impl Drop for ResponsesApiProxy { - fn drop(&mut self) { - let _ = self.child.kill(); - let _ = self.child.wait(); - } -} - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn responses_api_proxy_dumps_parent_and_subagent_identity_headers() -> Result<()> { +async fn responses_api_parent_and_subagent_requests_include_identity_headers() -> Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; - let dump_dir = TempDir::new()?; - let proxy = - ResponsesApiProxy::start(&format!("{}/v1/responses", server.uri()), dump_dir.path())?; let spawn_args = serde_json::to_string(&json!({ "message": CHILD_PROMPT }))?; - mount_sse_once_match( + let parent_mock = mount_sse_once_match( &server, - |req: &wiremock::Request| request_body_contains(req, PARENT_PROMPT), + |req: &wiremock::Request| { + request_body_contains(req, PARENT_PROMPT) + && request_header(req, "x-openai-subagent").is_none() + }, sse(vec![ ev_response_created("resp-parent-1"), ev_function_call(SPAWN_CALL_ID, "spawn_agent", &spawn_args), @@ -141,10 +51,12 @@ async fn responses_api_proxy_dumps_parent_and_subagent_identity_headers() -> Res ]), ) .await; - mount_sse_once_match( + let child_mock = mount_sse_once_match( &server, |req: &wiremock::Request| { - request_body_contains(req, CHILD_PROMPT) && !request_body_contains(req, SPAWN_CALL_ID) + request_body_contains(req, CHILD_PROMPT) + && !request_body_contains(req, SPAWN_CALL_ID) + && request_header(req, "x-openai-subagent") == Some("collab_spawn") }, sse(vec![ ev_response_created("resp-child-1"), @@ -155,7 +67,10 @@ async fn responses_api_proxy_dumps_parent_and_subagent_identity_headers() -> Res .await; mount_sse_once_match( &server, - |req: &wiremock::Request| request_body_contains(req, SPAWN_CALL_ID), + |req: &wiremock::Request| { + request_body_contains(req, SPAWN_CALL_ID) + && request_header(req, "x-openai-subagent").is_none() + }, sse(vec![ ev_response_created("resp-parent-2"), ev_assistant_message("msg-parent-2", "parent done"), @@ -164,50 +79,52 @@ async fn responses_api_proxy_dumps_parent_and_subagent_identity_headers() -> Res ) .await; - let proxy_base_url = proxy.base_url(); - let mut builder = test_codex().with_config(move |config| { - config.model_provider.base_url = Some(proxy_base_url); + let mut builder = test_codex().with_config(|config| { config .features .disable(Feature::EnableRequestCompression) .expect("test config should allow feature update"); }); let test = builder.build(&server).await?; - submit_turn_with_timeout(&test, PARENT_PROMPT, dump_dir.path()).await?; + submit_turn_with_timeout(&test, PARENT_PROMPT).await?; - let dumps = wait_for_proxy_request_dumps(dump_dir.path())?; - let parent = dumps - .iter() - .find(|dump| dump_body_contains(dump, PARENT_PROMPT)) - .ok_or_else(|| anyhow!("missing parent request dump"))?; - let child = dumps - .iter() - .find(|dump| { - dump_body_contains(dump, CHILD_PROMPT) && !dump_body_contains(dump, SPAWN_CALL_ID) - }) - .ok_or_else(|| anyhow!("missing child request dump"))?; + let parent = wait_for_matching_request(&parent_mock, "parent request", |request| { + request.body_contains_text(PARENT_PROMPT) && request.header("x-openai-subagent").is_none() + }) + .await?; + let child = wait_for_matching_request(&child_mock, "child request", |request| { + request.body_contains_text(CHILD_PROMPT) + && !request.body_contains_text(SPAWN_CALL_ID) + && request.header("x-openai-subagent").as_deref() == Some("collab_spawn") + }) + .await?; - let parent_window_id = header(parent, "x-codex-window-id") + let parent_window_id = parent + .header("x-codex-window-id") .ok_or_else(|| anyhow!("parent request missing x-codex-window-id"))?; - let child_window_id = header(child, "x-codex-window-id") + let child_window_id = child + .header("x-codex-window-id") .ok_or_else(|| anyhow!("child request missing x-codex-window-id"))?; - let (parent_thread_id, parent_generation) = split_window_id(parent_window_id)?; - let (child_thread_id, child_generation) = split_window_id(child_window_id)?; + let (parent_thread_id, parent_generation) = split_window_id(&parent_window_id)?; + let (child_thread_id, child_generation) = split_window_id(&child_window_id)?; assert_eq!(parent_generation, 0); assert_eq!(child_generation, 0); assert!(child_thread_id != parent_thread_id); - assert_eq!(header(parent, "x-openai-subagent"), None); - assert_eq!(header(child, "x-openai-subagent"), Some("collab_spawn")); + assert_eq!(parent.header("x-openai-subagent"), None); assert_eq!( - header(child, "x-codex-parent-thread-id"), + child.header("x-openai-subagent").as_deref(), + Some("collab_spawn") + ); + assert_eq!( + child.header("x-codex-parent-thread-id").as_deref(), Some(parent_thread_id) ); Ok(()) } -async fn submit_turn_with_timeout(test: &TestCodex, prompt: &str, dump_dir: &Path) -> Result<()> { +async fn submit_turn_with_timeout(test: &TestCodex, prompt: &str) -> Result<()> { let session_model = test.session_configured.model.clone(); test.codex .submit(Op::UserTurn { @@ -235,14 +152,14 @@ async fn submit_turn_with_timeout(test: &TestCodex, prompt: &str, dump_dir: &Pat }) .await?; - let turn_started = wait_for_event_result(test, "turn started", dump_dir, |event| { + let turn_started = wait_for_event_result(test, "turn started", |event| { matches!(event, EventMsg::TurnStarted(_)) }) .await?; let EventMsg::TurnStarted(turn_started) = turn_started else { unreachable!("event predicate only matches turn started events"); }; - wait_for_event_result(test, "turn complete", dump_dir, |event| match event { + wait_for_event_result(test, "turn complete", |event| match event { EventMsg::TurnComplete(event) => event.turn_id == turn_started.turn_id, _ => false, }) @@ -251,10 +168,33 @@ async fn submit_turn_with_timeout(test: &TestCodex, prompt: &str, dump_dir: &Pat Ok(()) } +async fn wait_for_matching_request( + mock: &ResponseMock, + label: &str, + mut predicate: F, +) -> Result +where + F: FnMut(&ResponsesRequest) -> bool, +{ + tokio::time::timeout(TURN_TIMEOUT, async { + loop { + if let Some(request) = mock + .requests() + .into_iter() + .find(|request| predicate(request)) + { + return request; + } + tokio::time::sleep(REQUEST_POLL_INTERVAL).await; + } + }) + .await + .map_err(|_| anyhow!("timed out waiting for {label}")) +} + async fn wait_for_event_result( test: &TestCodex, stage: &str, - dump_dir: &Path, mut predicate: F, ) -> Result where @@ -273,9 +213,8 @@ where .await .map_err(|_| { anyhow!( - "timed out waiting for {stage}; saw events: {}; {}", - seen_events.join(" | "), - proxy_dump_summary(dump_dir) + "timed out waiting for {stage}; saw events: {}", + seen_events.join(" | ") ) })? } @@ -290,95 +229,8 @@ fn request_body_contains(req: &wiremock::Request, text: &str) -> bool { std::str::from_utf8(&req.body).is_ok_and(|body| body.contains(text)) } -fn wait_for_proxy_request_dumps(dump_dir: &Path) -> Result> { - let deadline = Instant::now() + Duration::from_secs(/*secs*/ 2); - loop { - let dumps = read_proxy_request_dumps(dump_dir).unwrap_or_default(); - if dumps.len() >= 3 - && dumps - .iter() - .any(|dump| dump_body_contains(dump, CHILD_PROMPT)) - { - return Ok(dumps); - } - if Instant::now() >= deadline { - return Err(anyhow!( - "timed out waiting for proxy request dumps, got {}", - dumps.len() - )); - } - std::thread::sleep(PROXY_POLL_INTERVAL); - } -} - -fn read_proxy_request_dumps(dump_dir: &Path) -> Result> { - let mut dumps = Vec::new(); - for entry in std::fs::read_dir(dump_dir)? { - let path = entry?.path(); - if path - .file_name() - .and_then(|name| name.to_str()) - .is_some_and(|name| name.ends_with("-request.json")) - { - let contents = std::fs::read_to_string(&path)?; - if contents.trim().is_empty() { - continue; - } - - match serde_json::from_str(&contents) { - Ok(dump) => dumps.push(dump), - Err(err) if err.is_eof() => continue, - Err(err) => return Err(err.into()), - } - } - } - Ok(dumps) -} - -fn proxy_dump_summary(dump_dir: &Path) -> String { - match read_proxy_request_dumps(dump_dir) { - Ok(dumps) => { - let bodies = dumps - .iter() - .filter_map(|dump| dump.get("body")) - .map(Value::to_string) - .collect::>() - .join("; "); - format!("proxy wrote {} request dumps: {bodies}", dumps.len()) - } - Err(err) => format!("failed to read proxy request dumps: {err}"), - } -} - -#[test] -fn read_proxy_request_dumps_ignores_in_progress_files() -> Result<()> { - let dump_dir = TempDir::new()?; - std::fs::write(dump_dir.path().join("empty-request.json"), "")?; - std::fs::write(dump_dir.path().join("partial-request.json"), "{\"body\"")?; - std::fs::write( - dump_dir.path().join("complete-request.json"), - serde_json::to_string(&json!({ "body": "ready" }))?, - )?; - - assert_eq!( - read_proxy_request_dumps(dump_dir.path())?, - vec![json!({ "body": "ready" })] - ); - - Ok(()) -} - -fn dump_body_contains(dump: &Value, text: &str) -> bool { - dump.get("body") - .is_some_and(|body| body.to_string().contains(text)) -} - -fn header<'a>(dump: &'a Value, name: &str) -> Option<&'a str> { - dump.get("headers")?.as_array()?.iter().find_map(|header| { - (header.get("name")?.as_str()?.eq_ignore_ascii_case(name)) - .then(|| header.get("value")?.as_str()) - .flatten() - }) +fn request_header<'a>(req: &'a wiremock::Request, name: &str) -> Option<&'a str> { + req.headers.get(name).and_then(|value| value.to_str().ok()) } fn split_window_id(window_id: &str) -> Result<(&str, u64)> { diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index b63369ea6f..607dc7bf9c 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -144,6 +144,7 @@ fn insert_mcp_server( server_name.to_string(), McpServerConfig { transport, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: options.supports_parallel_tool_calls, diff --git a/codex-rs/core/tests/suite/search_tool.rs b/codex-rs/core/tests/suite/search_tool.rs index 67cf8503b3..eff7bfb427 100644 --- a/codex-rs/core/tests/suite/search_tool.rs +++ b/codex-rs/core/tests/suite/search_tool.rs @@ -697,6 +697,7 @@ async fn tool_search_indexes_only_enabled_non_app_mcp_tools() -> Result<()> { env_vars: Vec::new(), cwd: None, }, + experimental_environment: None, enabled: true, required: false, disabled_reason: None, diff --git a/codex-rs/core/tests/suite/sqlite_state.rs b/codex-rs/core/tests/suite/sqlite_state.rs index ef517d74fd..acdc1e9d8c 100644 --- a/codex-rs/core/tests/suite/sqlite_state.rs +++ b/codex-rs/core/tests/suite/sqlite_state.rs @@ -371,6 +371,7 @@ async fn mcp_call_marks_thread_memory_mode_polluted_when_configured() -> Result< env_vars: Vec::new(), cwd: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/tests/suite/tools.rs b/codex-rs/core/tests/suite/tools.rs index 3ae2888ec7..d0bb138188 100644 --- a/codex-rs/core/tests/suite/tools.rs +++ b/codex-rs/core/tests/suite/tools.rs @@ -155,6 +155,7 @@ async fn historical_unavailable_mcp_call_is_exposed_as_placeholder_tool() -> Res env_vars: Vec::new(), cwd: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/tests/suite/truncation.rs b/codex-rs/core/tests/suite/truncation.rs index 5bca13de50..33c74633f3 100644 --- a/codex-rs/core/tests/suite/truncation.rs +++ b/codex-rs/core/tests/suite/truncation.rs @@ -375,6 +375,7 @@ async fn mcp_tool_call_output_exceeds_limit_truncated_for_model() -> Result<()> env_vars: Vec::new(), cwd: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -472,6 +473,7 @@ async fn mcp_image_output_preserves_image_and_no_text_summary() -> Result<()> { env_vars: Vec::new(), cwd: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, @@ -752,6 +754,7 @@ async fn mcp_tool_call_output_not_truncated_with_custom_limit() -> Result<()> { env_vars: Vec::new(), cwd: None, }, + experimental_environment: None, enabled: true, required: false, supports_parallel_tool_calls: false, diff --git a/codex-rs/core/tests/suite/unified_exec.rs b/codex-rs/core/tests/suite/unified_exec.rs index df13fcaebe..b14c930999 100644 --- a/codex-rs/core/tests/suite/unified_exec.rs +++ b/codex-rs/core/tests/suite/unified_exec.rs @@ -39,6 +39,8 @@ use serde_json::Value; use serde_json::json; use tokio::time::Duration; +const UNIFIED_EXEC_LAGGED_OUTPUT_TIMEOUT: Duration = Duration::from_secs(30); + fn extract_output_text(item: &Value) -> Option<&str> { item.get("output").and_then(|value| match value { Value::String(text) => Some(text.as_str()), @@ -2055,11 +2057,12 @@ PY SandboxPolicy::DangerFullAccess, ) .await?; - // This is a worst case scenario for the truncate logic. + // This is a worst case scenario for the truncate logic, and CI can spend a + // while draining the lagged tail before the follow-up tool call completes. wait_for_event_with_timeout( &test.codex, |event| matches!(event, EventMsg::TurnComplete(_)), - Duration::from_secs(10), + UNIFIED_EXEC_LAGGED_OUTPUT_TIMEOUT, ) .await; diff --git a/codex-rs/core/tests/suite/view_image.rs b/codex-rs/core/tests/suite/view_image.rs index 6541a15450..f9d6d9402c 100644 --- a/codex-rs/core/tests/suite/view_image.rs +++ b/codex-rs/core/tests/suite/view_image.rs @@ -30,7 +30,6 @@ use core_test_support::responses::start_mock_server; use core_test_support::skip_if_no_network; use core_test_support::test_codex::TestCodex; use core_test_support::test_codex::test_codex; -use core_test_support::wait_for_event; use core_test_support::wait_for_event_with_timeout; use image::DynamicImage; use image::GenericImageView; @@ -49,6 +48,8 @@ use wiremock::ResponseTemplate; #[cfg(not(debug_assertions))] use wiremock::matchers::body_string_contains; +const VIEW_IMAGE_TURN_COMPLETE_TIMEOUT: Duration = Duration::from_secs(30); + fn image_messages(body: &Value) -> Vec<&Value> { body.get("input") .and_then(Value::as_array) @@ -180,7 +181,7 @@ async fn user_turn_with_local_image_attaches_image() -> anyhow::Result<()> { codex, |event| matches!(event, EventMsg::TurnComplete(_)), // Empirically, image attachment can be slow under Bazel/RBE. - Duration::from_secs(10), + VIEW_IMAGE_TURN_COMPLETE_TIMEOUT, ) .await; @@ -298,7 +299,7 @@ async fn view_image_tool_attaches_local_image() -> anyhow::Result<()> { }, // Empirically, we have seen this run slow when run under // Bazel on arm Linux. - Duration::from_secs(10), + VIEW_IMAGE_TURN_COMPLETE_TIMEOUT, ) .await; @@ -422,7 +423,7 @@ async fn view_image_tool_can_preserve_original_resolution_when_requested_on_gpt5 wait_for_event_with_timeout( codex, |event| matches!(event, EventMsg::TurnComplete(_)), - Duration::from_secs(10), + VIEW_IMAGE_TURN_COMPLETE_TIMEOUT, ) .await; @@ -518,7 +519,12 @@ async fn view_image_tool_errors_clearly_for_unsupported_detail_values() -> anyho }) .await?; - wait_for_event(codex, |event| matches!(event, EventMsg::TurnComplete(_))).await; + wait_for_event_with_timeout( + codex, + |event| matches!(event, EventMsg::TurnComplete(_)), + VIEW_IMAGE_TURN_COMPLETE_TIMEOUT, + ) + .await; let req = mock.single_request(); let body_with_tool_output = req.body_json(); @@ -603,7 +609,12 @@ async fn view_image_tool_treats_null_detail_as_omitted() -> anyhow::Result<()> { }) .await?; - wait_for_event(codex, |event| matches!(event, EventMsg::TurnComplete(_))).await; + wait_for_event_with_timeout( + codex, + |event| matches!(event, EventMsg::TurnComplete(_)), + VIEW_IMAGE_TURN_COMPLETE_TIMEOUT, + ) + .await; let req = mock.single_request(); let function_output = req.function_call_output(call_id); @@ -701,7 +712,7 @@ async fn view_image_tool_resizes_when_model_lacks_original_detail_support() -> a wait_for_event_with_timeout( codex, |event| matches!(event, EventMsg::TurnComplete(_)), - Duration::from_secs(10), + VIEW_IMAGE_TURN_COMPLETE_TIMEOUT, ) .await; @@ -805,7 +816,7 @@ async fn view_image_tool_does_not_force_original_resolution_with_capability_only wait_for_event_with_timeout( codex, |event| matches!(event, EventMsg::TurnComplete(_)), - Duration::from_secs(10), + VIEW_IMAGE_TURN_COMPLETE_TIMEOUT, ) .await; @@ -915,7 +926,7 @@ await codex.emitImage(out); EventMsg::TurnComplete(_) => true, _ => false, }, - Duration::from_secs(10), + VIEW_IMAGE_TURN_COMPLETE_TIMEOUT, ) .await; let tool_event = match tool_event { @@ -1035,7 +1046,7 @@ console.log(out.type); EventMsg::TurnComplete(_) => true, _ => false, }, - Duration::from_secs(10), + VIEW_IMAGE_TURN_COMPLETE_TIMEOUT, ) .await; let tool_event = match tool_event { @@ -1117,7 +1128,12 @@ async fn view_image_tool_errors_when_path_is_directory() -> anyhow::Result<()> { }) .await?; - wait_for_event(codex, |event| matches!(event, EventMsg::TurnComplete(_))).await; + wait_for_event_with_timeout( + codex, + |event| matches!(event, EventMsg::TurnComplete(_)), + VIEW_IMAGE_TURN_COMPLETE_TIMEOUT, + ) + .await; let req = mock.single_request(); let body_with_tool_output = req.body_json(); @@ -1193,7 +1209,12 @@ async fn view_image_tool_errors_for_non_image_files() -> anyhow::Result<()> { }) .await?; - wait_for_event(codex, |event| matches!(event, EventMsg::TurnComplete(_))).await; + wait_for_event_with_timeout( + codex, + |event| matches!(event, EventMsg::TurnComplete(_)), + VIEW_IMAGE_TURN_COMPLETE_TIMEOUT, + ) + .await; let request = mock.single_request(); assert!( @@ -1274,7 +1295,12 @@ async fn view_image_tool_errors_when_file_missing() -> anyhow::Result<()> { }) .await?; - wait_for_event(codex, |event| matches!(event, EventMsg::TurnComplete(_))).await; + wait_for_event_with_timeout( + codex, + |event| matches!(event, EventMsg::TurnComplete(_)), + VIEW_IMAGE_TURN_COMPLETE_TIMEOUT, + ) + .await; let req = mock.single_request(); let body_with_tool_output = req.body_json(); @@ -1405,7 +1431,12 @@ async fn view_image_tool_returns_unsupported_message_for_text_only_model() -> an }) .await?; - wait_for_event(codex, |event| matches!(event, EventMsg::TurnComplete(_))).await; + wait_for_event_with_timeout( + codex, + |event| matches!(event, EventMsg::TurnComplete(_)), + VIEW_IMAGE_TURN_COMPLETE_TIMEOUT, + ) + .await; let output_text = mock .single_request() @@ -1480,7 +1511,12 @@ async fn replaces_invalid_local_image_after_bad_request() -> anyhow::Result<()> }) .await?; - wait_for_event(&codex, |event| matches!(event, EventMsg::TurnComplete(_))).await; + wait_for_event_with_timeout( + &codex, + |event| matches!(event, EventMsg::TurnComplete(_)), + VIEW_IMAGE_TURN_COMPLETE_TIMEOUT, + ) + .await; let first_body = invalid_image_mock.single_request().body_json(); assert!( diff --git a/codex-rs/exec-server/BUILD.bazel b/codex-rs/exec-server/BUILD.bazel index 5d62c68caf..57ebe041f8 100644 --- a/codex-rs/exec-server/BUILD.bazel +++ b/codex-rs/exec-server/BUILD.bazel @@ -3,5 +3,9 @@ load("//:defs.bzl", "codex_rust_crate") codex_rust_crate( name = "exec-server", crate_name = "codex_exec_server", + # Keep the crate's integration tests single-threaded under Bazel because + # they install process-global test-binary dispatch state, and the remote + # exec-server cases already rely on serialization around the full CLI path. + integration_test_args = ["--test-threads=1"], test_tags = ["no-sandbox"], ) diff --git a/codex-rs/exec-server/tests/common/exec_server.rs b/codex-rs/exec-server/tests/common/exec_server.rs index ca4be44856..2a134546ac 100644 --- a/codex-rs/exec-server/tests/common/exec_server.rs +++ b/codex-rs/exec-server/tests/common/exec_server.rs @@ -22,7 +22,7 @@ use tokio::time::timeout; use tokio_tungstenite::connect_async; use tokio_tungstenite::tungstenite::Message; -const CONNECT_TIMEOUT: Duration = Duration::from_secs(5); +const CONNECT_TIMEOUT: Duration = Duration::from_secs(10); const CONNECT_RETRY_INTERVAL: Duration = Duration::from_millis(25); const EVENT_TIMEOUT: Duration = Duration::from_secs(5); diff --git a/codex-rs/exec-server/tests/exec_process.rs b/codex-rs/exec-server/tests/exec_process.rs index afe0c2e356..72f029231c 100644 --- a/codex-rs/exec-server/tests/exec_process.rs +++ b/codex-rs/exec-server/tests/exec_process.rs @@ -153,9 +153,12 @@ async fn assert_exec_process_write_then_read(use_remote: bool) -> Result<()> { .start(ExecParams { process_id: process_id.clone().into(), argv: vec![ - "/usr/bin/python3".to_string(), + // Use `/bin/sh` instead of Python so this stdin round-trip test + // stays portable across Bazel and non-macOS runners where + // `/usr/bin/python3` is not guaranteed to exist. + "/bin/sh".to_string(), "-c".to_string(), - "import sys; line = sys.stdin.readline(); sys.stdout.write(f'from-stdin:{line}'); sys.stdout.flush()".to_string(), + "IFS= read line; printf 'from-stdin:%s\\n' \"$line\"".to_string(), ], cwd: std::env::current_dir()?, env_policy: /*env_policy*/ None, diff --git a/codex-rs/features/src/lib.rs b/codex-rs/features/src/lib.rs index 2d1ac89851..a2e656be44 100644 --- a/codex-rs/features/src/lib.rs +++ b/codex-rs/features/src/lib.rs @@ -812,8 +812,8 @@ pub const FEATURES: &[FeatureSpec] = &[ FeatureSpec { id: Feature::ImageGeneration, key: "image_generation", - stage: Stage::UnderDevelopment, - default_enabled: false, + stage: Stage::Stable, + default_enabled: true, }, FeatureSpec { id: Feature::SkillMcpDependencyInstall, diff --git a/codex-rs/features/src/tests.rs b/codex-rs/features/src/tests.rs index 56f92d55b7..d2f73cedad 100644 --- a/codex-rs/features/src/tests.rs +++ b/codex-rs/features/src/tests.rs @@ -160,6 +160,12 @@ fn use_linux_sandbox_bwrap_is_a_removed_feature_key() { ); } +#[test] +fn image_generation_is_stable_and_enabled_by_default() { + assert_eq!(Feature::ImageGeneration.stage(), Stage::Stable); + assert_eq!(Feature::ImageGeneration.default_enabled(), true); +} + #[test] fn image_detail_original_is_a_removed_feature_key() { assert_eq!( @@ -168,12 +174,6 @@ fn image_detail_original_is_a_removed_feature_key() { ); } -#[test] -fn image_generation_is_under_development() { - assert_eq!(Feature::ImageGeneration.stage(), Stage::UnderDevelopment); - assert_eq!(Feature::ImageGeneration.default_enabled(), false); -} - #[test] fn tool_call_mcp_elicitation_is_stable_and_enabled_by_default() { assert_eq!(Feature::ToolCallMcpElicitation.stage(), Stage::Stable); diff --git a/codex-rs/login/src/api_bridge.rs b/codex-rs/login/src/api_bridge.rs index d8b9dbb77c..684c890bde 100644 --- a/codex-rs/login/src/api_bridge.rs +++ b/codex-rs/login/src/api_bridge.rs @@ -11,6 +11,7 @@ pub fn auth_provider_from_auth( return Ok(CoreAuthProvider { token: Some(api_key), account_id: None, + is_fedramp_account: false, }); } @@ -18,6 +19,7 @@ pub fn auth_provider_from_auth( return Ok(CoreAuthProvider { token: Some(token), account_id: None, + is_fedramp_account: false, }); } @@ -26,11 +28,13 @@ pub fn auth_provider_from_auth( Ok(CoreAuthProvider { token: Some(token), account_id: auth.get_account_id(), + is_fedramp_account: auth.is_fedramp_account(), }) } else { Ok(CoreAuthProvider { token: None, account_id: None, + is_fedramp_account: false, }) } } diff --git a/codex-rs/login/src/auth/auth_tests.rs b/codex-rs/login/src/auth/auth_tests.rs index 8a16f5939c..30244ac268 100644 --- a/codex-rs/login/src/auth/auth_tests.rs +++ b/codex-rs/login/src/auth/auth_tests.rs @@ -130,6 +130,7 @@ async fn pro_account_with_no_api_key_uses_chatgpt_auth() { chatgpt_plan_type: Some(InternalPlanType::Known(InternalKnownPlan::Pro)), chatgpt_user_id: Some("user-12345".to_string()), chatgpt_account_id: None, + chatgpt_account_is_fedramp: false, raw_jwt: fake_jwt, }, access_token: "test-access-token".to_string(), diff --git a/codex-rs/login/src/auth/manager.rs b/codex-rs/login/src/auth/manager.rs index 8768171d95..3cb97f58b0 100644 --- a/codex-rs/login/src/auth/manager.rs +++ b/codex-rs/login/src/auth/manager.rs @@ -293,6 +293,12 @@ impl CodexAuth { self.get_current_token_data().and_then(|t| t.account_id) } + /// Returns false if `is_chatgpt_auth()` is false or the token omits the FedRAMP claim. + pub fn is_fedramp_account(&self) -> bool { + self.get_current_token_data() + .is_some_and(|t| t.id_token.is_fedramp_account()) + } + /// Returns `None` if `is_chatgpt_auth()` is false. pub fn get_account_email(&self) -> Option { self.get_current_token_data().and_then(|t| t.id_token.email) diff --git a/codex-rs/login/src/token_data.rs b/codex-rs/login/src/token_data.rs index f70696d5a2..2952ecb2a7 100644 --- a/codex-rs/login/src/token_data.rs +++ b/codex-rs/login/src/token_data.rs @@ -36,6 +36,8 @@ pub struct IdTokenInfo { pub chatgpt_user_id: Option, /// Organization/workspace identifier associated with the token, if present. pub chatgpt_account_id: Option, + /// Whether the selected ChatGPT workspace must route through the FedRAMP edge. + pub chatgpt_account_is_fedramp: bool, pub raw_jwt: String, } @@ -60,6 +62,10 @@ impl IdTokenInfo { Some(PlanType::Known(plan)) if plan.is_workspace_account() ) } + + pub fn is_fedramp_account(&self) -> bool { + self.chatgpt_account_is_fedramp + } } #[derive(Deserialize)] @@ -88,6 +94,8 @@ struct AuthClaims { user_id: Option, #[serde(default)] chatgpt_account_id: Option, + #[serde(default)] + chatgpt_account_is_fedramp: bool, } #[derive(Deserialize)] @@ -139,6 +147,7 @@ pub fn parse_chatgpt_jwt_claims(jwt: &str) -> Result Ok(IdTokenInfo { email, @@ -146,6 +155,7 @@ pub fn parse_chatgpt_jwt_claims(jwt: &str) -> Result { + if slash_tmp.as_path().is_dir() { + roots.push(slash_tmp); + } + } + Err(e) => { + error!("Ignoring invalid /tmp for sandbox writable root: {e}"); + } } } diff --git a/codex-rs/responses-api-proxy/src/dump.rs b/codex-rs/responses-api-proxy/src/dump.rs index bebd4ed592..1a09c64ae7 100644 --- a/codex-rs/responses-api-proxy/src/dump.rs +++ b/codex-rs/responses-api-proxy/src/dump.rs @@ -231,6 +231,12 @@ mod tests { Header::from_bytes(&b"Cookie"[..], &b"user-session=secret"[..]).expect("cookie header"), Header::from_bytes(&b"Content-Type"[..], &b"application/json"[..]) .expect("content-type header"), + Header::from_bytes(&b"x-codex-window-id"[..], &b"thread-1:0"[..]) + .expect("window id header"), + Header::from_bytes(&b"x-codex-parent-thread-id"[..], &b"parent-thread-1"[..]) + .expect("parent thread id header"), + Header::from_bytes(&b"x-openai-subagent"[..], &b"collab_spawn"[..]) + .expect("subagent header"), ]; let exchange_dump = dumper @@ -262,6 +268,18 @@ mod tests { { "name": "Content-Type", "value": "application/json" + }, + { + "name": "x-codex-window-id", + "value": "thread-1:0" + }, + { + "name": "x-codex-parent-thread-id", + "value": "parent-thread-1" + }, + { + "name": "x-openai-subagent", + "value": "collab_spawn" } ], "body": { diff --git a/codex-rs/state/src/log_db.rs b/codex-rs/state/src/log_db.rs index e33baae573..c28f762d27 100644 --- a/codex-rs/state/src/log_db.rs +++ b/codex-rs/state/src/log_db.rs @@ -384,9 +384,8 @@ mod tests { use std::io; use std::sync::Arc; use std::sync::Mutex; - use std::time::Duration; - use tokio::time::Instant; + use pretty_assertions::assert_eq; use tracing_subscriber::filter::Targets; use tracing_subscriber::fmt::writer::MakeWriter; use tracing_subscriber::layer::SubscriberExt; @@ -442,6 +441,7 @@ mod tests { .await .expect("initialize runtime"); let writer = SharedWriter::default(); + let layer = start(runtime.clone()); let subscriber = tracing_subscriber::registry() .with( @@ -452,7 +452,8 @@ mod tests { .with_filter(Targets::new().with_default(tracing::Level::TRACE)), ) .with( - start(runtime.clone()) + layer + .clone() .with_filter(Targets::new().with_default(tracing::Level::TRACE)), ); let guard = subscriber.set_default(); @@ -463,6 +464,7 @@ mod tests { }); tracing::debug!("threadless-after"); + layer.flush().await; drop(guard); let feedback_logs = writer.snapshot(); @@ -475,24 +477,17 @@ mod tests { .collect::>() .join("\n") }; - let deadline = Instant::now() + Duration::from_secs(2); - loop { - let sqlite_logs = String::from_utf8( - runtime - .query_feedback_logs("thread-1") - .await - .expect("query feedback logs"), - ) - .expect("valid utf-8"); - if without_timestamps(&sqlite_logs) == without_timestamps(&feedback_logs) { - break; - } - assert!( - Instant::now() < deadline, - "sqlite feedback logs did not match feedback formatter output before timeout\nsqlite:\n{sqlite_logs}\nfeedback:\n{feedback_logs}" - ); - tokio::time::sleep(Duration::from_millis(10)).await; - } + let sqlite_logs = String::from_utf8( + runtime + .query_feedback_logs("thread-1") + .await + .expect("query feedback logs"), + ) + .expect("valid utf-8"); + assert_eq!( + without_timestamps(&sqlite_logs), + without_timestamps(&feedback_logs) + ); let _ = tokio::fs::remove_dir_all(codex_home).await; } diff --git a/codex-rs/state/src/runtime.rs b/codex-rs/state/src/runtime.rs index 610e45e68a..054ad6ff2c 100644 --- a/codex-rs/state/src/runtime.rs +++ b/codex-rs/state/src/runtime.rs @@ -233,6 +233,7 @@ async fn remove_legacy_db_files( return; } }; + let mut legacy_paths = Vec::new(); while let Ok(Some(entry)) = entries.next_entry().await { if !entry .file_type() @@ -248,7 +249,14 @@ async fn remove_legacy_db_files( continue; } - let legacy_path = entry.path(); + legacy_paths.push(entry.path()); + } + + // On Windows, SQLite can keep the main database file undeletable until the + // matching `-wal` / `-shm` sidecars are removed. Remove the longest + // sidecar-style paths first so the main file is attempted last. + legacy_paths.sort_by_key(|path| std::cmp::Reverse(path.as_os_str().len())); + for legacy_path in legacy_paths { if let Err(err) = tokio::fs::remove_file(&legacy_path).await { warn!( "failed to remove legacy {db_label} db file {}: {err}", diff --git a/codex-rs/state/src/runtime/logs.rs b/codex-rs/state/src/runtime/logs.rs index 56ef31d5d1..6728965ac3 100644 --- a/codex-rs/state/src/runtime/logs.rs +++ b/codex-rs/state/src/runtime/logs.rs @@ -729,6 +729,7 @@ mod tests { .await .expect("insert legacy log row"); pool.close().await; + drop(pool); let runtime = StateRuntime::init(codex_home.clone(), "test-provider".to_string()) .await diff --git a/codex-rs/tools/src/code_mode.rs b/codex-rs/tools/src/code_mode.rs index e45c7be1d9..e8a73c40a9 100644 --- a/codex-rs/tools/src/code_mode.rs +++ b/codex-rs/tools/src/code_mode.rs @@ -138,7 +138,8 @@ pub fn create_wait_tool() -> ToolSpec { pub fn create_code_mode_tool( enabled_tools: &[CodeModeToolDefinition], namespace_descriptions: &BTreeMap, - code_mode_only_enabled: bool, + code_mode_only: bool, + deferred_tools_available: bool, ) -> ToolSpec { const CODE_MODE_FREEFORM_GRAMMAR: &str = r#" start: pragma_source | plain_source @@ -155,7 +156,8 @@ SOURCE: /[\s\S]+/ description: codex_code_mode::build_exec_tool_description( enabled_tools, namespace_descriptions, - code_mode_only_enabled, + code_mode_only, + deferred_tools_available, ), format: FreeformToolFormat { r#type: "grammar".to_string(), diff --git a/codex-rs/tools/src/code_mode_tests.rs b/codex-rs/tools/src/code_mode_tests.rs index 15cca164ec..d7d40cae6e 100644 --- a/codex-rs/tools/src/code_mode_tests.rs +++ b/codex-rs/tools/src/code_mode_tests.rs @@ -197,14 +197,16 @@ fn create_code_mode_tool_matches_expected_spec() { create_code_mode_tool( &enabled_tools, &BTreeMap::new(), - /*code_mode_only_enabled*/ true, + /*code_mode_only*/ true, + /*deferred_tools_available*/ false, ), ToolSpec::Freeform(FreeformTool { name: codex_code_mode::PUBLIC_TOOL_NAME.to_string(), description: codex_code_mode::build_exec_tool_description( &enabled_tools, &BTreeMap::new(), - /*code_mode_only*/ true + /*code_mode_only*/ true, + /*deferred_tools_available*/ false ), format: FreeformToolFormat { r#type: "grammar".to_string(), diff --git a/codex-rs/tools/src/tool_registry_plan.rs b/codex-rs/tools/src/tool_registry_plan.rs index b5da9afb25..ac11a2c4be 100644 --- a/codex-rs/tools/src/tool_registry_plan.rs +++ b/codex-rs/tools/src/tool_registry_plan.rs @@ -108,6 +108,10 @@ pub fn build_tool_registry_plan( &enabled_tools, &namespace_descriptions, config.code_mode_only_enabled, + config.search_tool + && params + .deferred_mcp_tools + .is_some_and(|tools| !tools.is_empty()), ), /*supports_parallel_tool_calls*/ false, config.code_mode_enabled, diff --git a/codex-rs/tools/src/tool_registry_plan_tests.rs b/codex-rs/tools/src/tool_registry_plan_tests.rs index 50b38905ee..cc8c05dd9a 100644 --- a/codex-rs/tools/src/tool_registry_plan_tests.rs +++ b/codex-rs/tools/src/tool_registry_plan_tests.rs @@ -97,6 +97,7 @@ fn test_full_toolset_specs_for_gpt5_codex_unified_exec_web_search() { search_context_size: None, search_content_types: None, }, + create_image_generation_tool("png"), create_view_image_tool(ViewImageToolOptions { can_request_original_image_detail: config.can_request_original_image_detail, }), diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 03a99e3aa3..3fbcd64909 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -325,8 +325,12 @@ fn session_summary( token_usage: TokenUsage, thread_id: Option, thread_name: Option, + rollout_path: Option<&Path>, ) -> Option { let usage_line = (!token_usage.is_zero()).then(|| FinalOutput::from(token_usage).to_string()); + let (thread_id, thread_name) = resumable_thread(thread_id, thread_name, rollout_path) + .map(|thread| (Some(thread.thread_id), thread.thread_name)) + .unwrap_or((None, None)); let resume_command = crate::legacy_core::util::resume_command(thread_name.as_deref(), thread_id); @@ -340,6 +344,29 @@ fn session_summary( }) } +#[derive(Debug, Clone, PartialEq, Eq)] +struct ResumableThread { + thread_id: ThreadId, + thread_name: Option, +} + +fn resumable_thread( + thread_id: Option, + thread_name: Option, + rollout_path: Option<&Path>, +) -> Option { + let thread_id = thread_id?; + let rollout_path = rollout_path?; + rollout_path_is_resumable(rollout_path).then_some(ResumableThread { + thread_id, + thread_name, + }) +} + +fn rollout_path_is_resumable(rollout_path: &Path) -> bool { + std::fs::metadata(rollout_path).is_ok_and(|metadata| metadata.is_file() && metadata.len() > 0) +} + fn errors_for_cwd(cwd: &Path, response: &ListSkillsResponseEvent) -> Vec { response .skills @@ -2378,10 +2405,11 @@ impl App { ) -> Result { match op.view() { AppCommandView::Interrupt => { - let Some(turn_id) = self.active_turn_id_for_thread(thread_id).await else { - return Ok(true); - }; - app_server.turn_interrupt(thread_id, turn_id).await?; + if let Some(turn_id) = self.active_turn_id_for_thread(thread_id).await { + app_server.turn_interrupt(thread_id, turn_id).await?; + } else { + app_server.startup_interrupt(thread_id).await?; + } Ok(true) } AppCommandView::UserTurn { @@ -3447,6 +3475,7 @@ impl App { self.chat_widget.token_usage(), self.chat_widget.thread_id(), self.chat_widget.thread_name(), + self.chat_widget.rollout_path().as_deref(), ); self.shutdown_current_thread(app_server).await; let tracked_thread_ids: Vec = @@ -4125,10 +4154,15 @@ impl App { return Err(err); } }; + let resumable_thread = resumable_thread( + app.chat_widget.thread_id(), + app.chat_widget.thread_name(), + app.chat_widget.rollout_path().as_deref(), + ); Ok(AppExitInfo { token_usage: app.token_usage(), - thread_id: app.chat_widget.thread_id(), - thread_name: app.chat_widget.thread_name(), + thread_id: resumable_thread.as_ref().map(|thread| thread.thread_id), + thread_name: resumable_thread.and_then(|thread| thread.thread_name), update_action: app.pending_update_action, exit_reason, }) @@ -4248,6 +4282,7 @@ impl App { self.chat_widget.token_usage(), self.chat_widget.thread_id(), self.chat_widget.thread_name(), + self.chat_widget.rollout_path().as_deref(), ); match app_server .resume_thread(resume_config.clone(), target_session.thread_id) @@ -4402,6 +4437,7 @@ impl App { self.chat_widget.token_usage(), self.chat_widget.thread_id(), self.chat_widget.thread_name(), + self.chat_widget.rollout_path().as_deref(), ); self.chat_widget .add_plain_history_lines(vec!["/fork".magenta().into()]); @@ -8224,6 +8260,8 @@ mod tests { let (mut app, _app_event_rx, _op_rx) = make_test_app_with_channels().await; let codex_home = tempdir()?; app.config.codex_home = codex_home.path().to_path_buf().abs(); + // Seed the previous setting so this test exercises the thread-mode update path. + app.config.memories.generate_memories = true; let mut app_server = crate::start_embedded_app_server_for_picker(&app.config).await?; let started = app_server.start_thread(&app.config).await?; @@ -11410,11 +11448,18 @@ guardian_approval = true #[tokio::test] async fn interrupt_without_active_turn_is_treated_as_handled() { let mut app = make_test_app().await; - let thread_id = ThreadId::new(); let mut app_server = crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref()) .await .expect("embedded app server"); + let started = app_server + .start_thread(app.chat_widget.config_ref()) + .await + .expect("thread/start should succeed"); + let thread_id = started.session.thread_id; + app.enqueue_primary_thread_session(started.session, started.turns) + .await + .expect("primary thread should be registered"); let op = AppCommand::interrupt(); let handled = app @@ -11486,14 +11531,33 @@ guardian_approval = true session_summary( TokenUsage::default(), /*thread_id*/ None, - /*thread_name*/ None + /*thread_name*/ None, + /*rollout_path*/ None, ) .is_none() ); } #[tokio::test] - async fn session_summary_includes_resume_hint() { + async fn session_summary_skips_resume_hint_until_rollout_exists() { + let usage = TokenUsage::default(); + let conversation = ThreadId::from_string("123e4567-e89b-12d3-a456-426614174000").unwrap(); + let temp_dir = tempdir().expect("temp dir"); + let rollout_path = temp_dir.path().join("rollout.jsonl"); + + assert!( + session_summary( + usage, + Some(conversation), + /*thread_name*/ None, + Some(&rollout_path), + ) + .is_none() + ); + } + + #[tokio::test] + async fn session_summary_includes_resume_hint_for_persisted_rollout() { let usage = TokenUsage { input_tokens: 10, output_tokens: 2, @@ -11501,9 +11565,17 @@ guardian_approval = true ..Default::default() }; let conversation = ThreadId::from_string("123e4567-e89b-12d3-a456-426614174000").unwrap(); + let temp_dir = tempdir().expect("temp dir"); + let rollout_path = temp_dir.path().join("rollout.jsonl"); + std::fs::write(&rollout_path, "{}\n").expect("write rollout"); - let summary = - session_summary(usage, Some(conversation), /*thread_name*/ None).expect("summary"); + let summary = session_summary( + usage, + Some(conversation), + /*thread_name*/ None, + Some(&rollout_path), + ) + .expect("summary"); assert_eq!( summary.usage_line, Some("Token usage: total=12 input=10 output=2".to_string()) @@ -11523,9 +11595,17 @@ guardian_approval = true ..Default::default() }; let conversation = ThreadId::from_string("123e4567-e89b-12d3-a456-426614174000").unwrap(); + let temp_dir = tempdir().expect("temp dir"); + let rollout_path = temp_dir.path().join("rollout.jsonl"); + std::fs::write(&rollout_path, "{}\n").expect("write rollout"); - let summary = session_summary(usage, Some(conversation), Some("my-session".to_string())) - .expect("summary"); + let summary = session_summary( + usage, + Some(conversation), + Some("my-session".to_string()), + Some(&rollout_path), + ) + .expect("summary"); assert_eq!( summary.resume_command, Some("codex resume my-session".to_string()) diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index 47a06738e4..5e11a53340 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -476,6 +476,10 @@ impl AppServerSession { Ok(()) } + pub(crate) async fn startup_interrupt(&mut self, thread_id: ThreadId) -> Result<()> { + self.turn_interrupt(thread_id, String::new()).await + } + pub(crate) async fn turn_steer( &mut self, thread_id: ThreadId, diff --git a/codex-rs/tui/src/bottom_pane/custom_prompt_view.rs b/codex-rs/tui/src/bottom_pane/custom_prompt_view.rs index e9f0ee697f..753aae51c7 100644 --- a/codex-rs/tui/src/bottom_pane/custom_prompt_view.rs +++ b/codex-rs/tui/src/bottom_pane/custom_prompt_view.rs @@ -41,15 +41,22 @@ impl CustomPromptView { pub(crate) fn new( title: String, placeholder: String, + initial_text: String, context_label: Option, on_submit: PromptSubmitted, ) -> Self { + let mut textarea = TextArea::new(); + if !initial_text.is_empty() { + textarea.set_text_clearing_elements(&initial_text); + textarea.set_cursor(initial_text.len()); + } + Self { title, placeholder, context_label, on_submit, - textarea: TextArea::new(), + textarea, textarea_state: RefCell::new(TextAreaState::default()), complete: false, } diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__status_line_setup__tests__setup_view_snapshot_uses_runtime_preview_values.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__status_line_setup__tests__setup_view_snapshot_uses_runtime_preview_values.snap index 679aa77f4a..20aca1e334 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__status_line_setup__tests__setup_view_snapshot_uses_runtime_preview_values.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__status_line_setup__tests__setup_view_snapshot_uses_runtime_preview_values.snap @@ -8,14 +8,14 @@ expression: "render_lines(&view, 72)" Type to search > -› [x] model-name Current model name - [x] current-dir Current working directory - [x] git-branch Current Git branch (omitted when unavail… - [ ] model-with-reasoning Current model name with reasoning level - [ ] project-root Project root directory (omitted when una… - [ ] context-remaining Percentage of context window remaining (… - [ ] context-remaining-... Percentage of context window remaining (… - [ ] context-used Percentage of context window used (omitt… +› [x] model-name Current model name + [x] current-dir Current working directory + [x] git-branch Current Git branch (omitted when unavaila… + [ ] model-with-reasoning Current model name with reasoning level + [ ] project-root Project root directory (omitted when unav… + [ ] context-remaining Percentage of context window remaining (o… + [ ] context-used Percentage of context window used (omitte… + [ ] five-hour-limit Remaining usage on 5-hour usage limit (om… gpt-5-codex · ~/codex-rs · jif/statusline-preview Use ↑↓ to navigate, ←→ to move, space to select, enter to confirm, esc diff --git a/codex-rs/tui/src/bottom_pane/status_line_setup.rs b/codex-rs/tui/src/bottom_pane/status_line_setup.rs index b5fe8456ed..4c7e29c053 100644 --- a/codex-rs/tui/src/bottom_pane/status_line_setup.rs +++ b/codex-rs/tui/src/bottom_pane/status_line_setup.rs @@ -66,10 +66,6 @@ pub(crate) enum StatusLineItem { /// Percentage of context window remaining. ContextRemaining, - /// Percentage of context window remaining. - #[strum(to_string = "context-remaining-percent")] - ContextRemainingPercent, - /// Percentage of context window used. /// /// Also accepts the legacy `context-usage` config value. @@ -119,9 +115,6 @@ impl StatusLineItem { StatusLineItem::ContextRemaining => { "Percentage of context window remaining (omitted when unknown)" } - StatusLineItem::ContextRemainingPercent => { - "Percentage of context window remaining (omitted when unknown)" - } StatusLineItem::ContextUsed => { "Percentage of context window used (omitted when unknown)" } @@ -321,7 +314,7 @@ mod tests { } #[test] - fn context_remaining_is_separate_selectable_id() { + fn context_remaining_is_selectable_id() { assert_eq!( "context-remaining".parse::(), Ok(StatusLineItem::ContextRemaining) @@ -332,18 +325,6 @@ mod tests { ); } - #[test] - fn context_remaining_percent_is_separate_selectable_id() { - assert_eq!( - StatusLineItem::ContextRemainingPercent.to_string(), - "context-remaining-percent" - ); - assert_eq!( - "context-remaining-percent".parse::(), - Ok(StatusLineItem::ContextRemainingPercent) - ); - } - #[test] fn preview_uses_runtime_values() { let preview_data = StatusLinePreviewData::from_iter([ diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 9471c29ca6..de71efb9d8 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -5279,11 +5279,8 @@ impl ChatWidget { fn show_rename_prompt(&mut self) { let tx = self.app_event_tx.clone(); - let has_name = self - .thread_name - .as_ref() - .is_some_and(|name| !name.is_empty()); - let title = if has_name { + let existing_name = self.thread_name.as_deref().filter(|name| !name.is_empty()); + let title = if existing_name.is_some() { "Rename thread" } else { "Name thread" @@ -5291,6 +5288,7 @@ impl ChatWidget { let view = CustomPromptView::new( title.to_string(), "Type a name and press Enter".to_string(), + /*initial_text*/ existing_name.unwrap_or_default().to_string(), /*context_label*/ None, Box::new(move |name: String| { let Some(name) = crate::legacy_core::util::normalize_thread_name(&name) else { @@ -10599,6 +10597,7 @@ impl ChatWidget { let view = CustomPromptView::new( "Custom review instructions".to_string(), "Type instructions and press Enter".to_string(), + /*initial_text*/ String::new(), /*context_label*/ None, Box::new(move |prompt: String| { let trimmed = prompt.trim().to_string(); diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__slash_rename_prefilled_prompt.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__slash_rename_prefilled_prompt.snap new file mode 100644 index 0000000000..f1eb0d223b --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__slash_rename_prefilled_prompt.snap @@ -0,0 +1,10 @@ +--- +source: tui/src/chatwidget/tests/slash_commands.rs +assertion_line: 135 +expression: popup +--- +▌ Rename thread +▌ +▌ Current project title + +Press enter to confirm or esc to go back diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_line_model_with_reasoning_context_remaining_percent_footer.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_line_model_with_reasoning_context_remaining_footer.snap similarity index 100% rename from codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_line_model_with_reasoning_context_remaining_percent_footer.snap rename to codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_line_model_with_reasoning_context_remaining_footer.snap diff --git a/codex-rs/tui/src/chatwidget/status_surfaces.rs b/codex-rs/tui/src/chatwidget/status_surfaces.rs index ba95da93b3..a744711c2c 100644 --- a/codex-rs/tui/src/chatwidget/status_surfaces.rs +++ b/codex-rs/tui/src/chatwidget/status_surfaces.rs @@ -450,7 +450,7 @@ impl ChatWidget { Some(format!("{} used", format_tokens_compact(total))) } } - StatusLineItem::ContextRemaining | StatusLineItem::ContextRemainingPercent => self + StatusLineItem::ContextRemaining => self .status_line_context_remaining_percent() .map(|remaining| format!("Context {remaining}% left")), StatusLineItem::ContextUsed => self diff --git a/codex-rs/tui/src/chatwidget/tests/slash_commands.rs b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs index 7174b28b80..0c6b9be8e6 100644 --- a/codex-rs/tui/src/chatwidget/tests/slash_commands.rs +++ b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs @@ -124,6 +124,39 @@ async fn inline_slash_command_is_available_from_local_recall_after_dispatch() { assert_eq!(chat.bottom_pane.composer_text(), "/rename Better title"); } +#[tokio::test] +async fn slash_rename_prefills_existing_thread_name() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.thread_name = Some("Current project title".to_string()); + + chat.dispatch_command(SlashCommand::Rename); + + let popup = render_bottom_popup(&chat, /*width*/ 80); + assert_chatwidget_snapshot!("slash_rename_prefilled_prompt", popup); + + chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + + assert_matches!( + rx.try_recv(), + Ok(AppEvent::CodexOp(Op::SetThreadName { name })) if name == "Current project title" + ); +} + +#[tokio::test] +async fn slash_rename_without_existing_thread_name_starts_empty() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + + chat.dispatch_command(SlashCommand::Rename); + + let popup = render_bottom_popup(&chat, /*width*/ 80); + assert!(popup.contains("Name thread")); + assert!(popup.contains("Type a name and press Enter")); + + chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + + assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); +} + #[tokio::test] async fn usage_error_slash_command_is_available_from_local_recall() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.3-codex")).await; diff --git a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs index e59928ff1c..75ca9d1fba 100644 --- a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs +++ b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs @@ -914,24 +914,6 @@ async fn status_line_legacy_context_usage_renders_context_used_percent() { ); } -#[tokio::test] -async fn status_line_context_remaining_percent_renders_labeled_percent() { - let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; - chat.thread_id = Some(ThreadId::new()); - chat.config.tui_status_line = Some(vec!["context-remaining-percent".to_string()]); - - chat.refresh_status_line(); - - assert_eq!( - status_line_text(&chat), - Some("Context 100% left".to_string()) - ); - assert!( - drain_insert_history(&mut rx).is_empty(), - "context-remaining-percent should remain a valid status line item" - ); -} - #[tokio::test] async fn status_line_branch_state_resets_when_git_branch_disabled() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; @@ -1200,7 +1182,7 @@ async fn status_line_model_with_reasoning_fast_footer_snapshot() { } #[tokio::test] -async fn status_line_model_with_reasoning_context_remaining_percent_footer_snapshot() { +async fn status_line_model_with_reasoning_context_remaining_footer_snapshot() { use ratatui::Terminal; use ratatui::backend::TestBackend; @@ -1211,7 +1193,7 @@ async fn status_line_model_with_reasoning_context_remaining_percent_footer_snaps chat.config.cwd = test_project_path().abs(); chat.config.tui_status_line = Some(vec![ "model-with-reasoning".to_string(), - "context-remaining-percent".to_string(), + "context-remaining".to_string(), "current-dir".to_string(), ]); chat.set_reasoning_effort(Some(ReasoningEffortConfig::XHigh)); @@ -1228,7 +1210,7 @@ async fn status_line_model_with_reasoning_context_remaining_percent_footer_snaps .draw(|f| chat.render(f.area(), f.buffer_mut())) .expect("draw model-with-reasoning footer"); assert_chatwidget_snapshot!( - "status_line_model_with_reasoning_context_remaining_percent_footer", + "status_line_model_with_reasoning_context_remaining_footer", normalized_backend_snapshot(terminal.backend()) ); } diff --git a/codex-rs/tui/tests/suite/model_availability_nux.rs b/codex-rs/tui/tests/suite/model_availability_nux.rs index 923ace50fd..04a03a09f9 100644 --- a/codex-rs/tui/tests/suite/model_availability_nux.rs +++ b/codex-rs/tui/tests/suite/model_availability_nux.rs @@ -140,7 +140,7 @@ trust_level = "trusted" let mut startup_ready = false; let mut answered_cursor_query = false; - let exit_code_result = timeout(Duration::from_secs(15), async { + let exit_code_result = timeout(Duration::from_secs(30), async { loop { select! { result = output_rx.recv() => match result { diff --git a/justfile b/justfile index f0f14f4193..d44bbda21d 100644 --- a/justfile +++ b/justfile @@ -17,7 +17,7 @@ exec *args: # Start `codex exec-server` and run codex-tui. [no-cd] tui-with-exec-server *args: - ./scripts/run_tui_with_exec_server.sh "$@" + {{ justfile_directory() }}/scripts/run_tui_with_exec_server.sh "$@" # Run the CLI version of the file-search crate. file-search *args: @@ -64,18 +64,18 @@ bazel-lock-update: [no-cd] bazel-lock-check: - ./scripts/check-module-bazel-lock.sh + {{ justfile_directory() }}/scripts/check-module-bazel-lock.sh bazel-test: bazel test --test_tag_filters=-argument-comment-lint //... --keep_going [no-cd] bazel-clippy: - bazel_targets="$(./scripts/list-bazel-clippy-targets.sh)" && bazel build --config=clippy -- ${bazel_targets} + bazel_targets="$({{ justfile_directory() }}/scripts/list-bazel-clippy-targets.sh)" && bazel build --config=clippy -- ${bazel_targets} [no-cd] bazel-argument-comment-lint: - bazel build --config=argument-comment-lint -- $(./tools/argument-comment-lint/list-bazel-targets.sh) + bazel build --config=argument-comment-lint -- $({{ justfile_directory() }}/tools/argument-comment-lint/list-bazel-targets.sh) bazel-remote-test: bazel test --test_tag_filters=-argument-comment-lint //... --config=remote --platforms=//:rbe --keep_going @@ -97,20 +97,20 @@ write-app-server-schema *args: [no-cd] write-hooks-schema: - cargo run --manifest-path ./codex-rs/Cargo.toml -p codex-hooks --bin write_hooks_schema_fixtures + cargo run --manifest-path {{ justfile_directory() }}/codex-rs/Cargo.toml -p codex-hooks --bin write_hooks_schema_fixtures # Run the argument-comment Dylint checks across codex-rs. [no-cd] argument-comment-lint *args: if [ "$#" -eq 0 ]; then \ - bazel build --config=argument-comment-lint -- $(./tools/argument-comment-lint/list-bazel-targets.sh); \ + bazel build --config=argument-comment-lint -- $({{ justfile_directory() }}/tools/argument-comment-lint/list-bazel-targets.sh); \ else \ - ./tools/argument-comment-lint/run-prebuilt-linter.py "$@"; \ + {{ justfile_directory() }}/tools/argument-comment-lint/run-prebuilt-linter.py "$@"; \ fi [no-cd] argument-comment-lint-from-source *args: - ./tools/argument-comment-lint/run.py "$@" + {{ justfile_directory() }}/tools/argument-comment-lint/run.py "$@" # Tail logs from the state SQLite database log *args: