diff --git a/codex-rs/core/src/mcp_skill_dependencies.rs b/codex-rs/core/src/mcp_skill_dependencies.rs index 61d5c54a59..d32dd0d59e 100644 --- a/codex-rs/core/src/mcp_skill_dependencies.rs +++ b/codex-rs/core/src/mcp_skill_dependencies.rs @@ -17,6 +17,7 @@ use tracing::warn; use crate::SkillMetadata; use crate::session::session::Session; +use crate::session::turn_context::McpAccess; use crate::session::turn_context::TurnContext; use crate::skills::model::SkillToolDependency; use codex_mcp::ElicitationReviewerHandle; @@ -45,7 +46,8 @@ pub(crate) async fn maybe_prompt_and_install_mcp_dependencies( } let config = turn_context.config.clone(); - if mentioned_skills.is_empty() + if turn_context.mcp_access() == McpAccess::Disabled + || mentioned_skills.is_empty() || !config .features .enabled(codex_features::Feature::SkillMcpDependencyInstall) diff --git a/codex-rs/core/src/session/mcp.rs b/codex-rs/core/src/session/mcp.rs index 8cdba7989a..5a2c1eec7e 100644 --- a/codex-rs/core/src/session/mcp.rs +++ b/codex-rs/core/src/session/mcp.rs @@ -173,7 +173,7 @@ impl Session { .await } - pub(crate) async fn isolated_mcp_runtime_for_step( + pub(crate) fn disabled_mcp_runtime_for_step( &self, turn_context: &TurnContext, environments: &TurnEnvironmentSnapshot, @@ -181,16 +181,11 @@ impl Session { ) -> Arc { let available_environment_ids = Self::available_selected_environment_ids(selected_capability_roots); - let mcp_config = self - .services - .mcp_manager - .runtime_config_for_step( - &turn_context.config, - &self.services.mcp_thread_init, - &self.services.thread_extension_data, - &available_environment_ids, - ) - .await; + let mut mcp_config = self.services.latest_mcp_runtime().config().clone(); + mcp_config.apps_enabled = false; + mcp_config.skill_mcp_dependency_install_enabled = false; + mcp_config.mcp_server_catalog = Default::default(); + mcp_config.connector_snapshot = Default::default(); let environment_manager = self.services.turn_environments.environment_manager(); #[allow(deprecated)] let cwd = environments diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 6c1b84dc3d..4426fa462a 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -239,6 +239,7 @@ use self::turn::AssistantMessageStreamParsers; #[cfg(test)] use self::turn::collect_explicit_app_ids_from_skill_items; use self::turn::realtime_text_for_event; +use self::turn_context::McpAccess; use self::turn_context::TurnContext; use self::turn_context::TurnSkillsContext; #[cfg(test)] @@ -2855,20 +2856,20 @@ impl Session { let selected_capability_roots = self .resolve_selected_capability_roots_for_step(&environments) .await; - let mcp = if turn_context.is_review_subagent() { - self.isolated_mcp_runtime_for_step( + let mcp = match turn_context.mcp_access() { + McpAccess::Disabled => self.disabled_mcp_runtime_for_step( turn_context.as_ref(), &environments, &selected_capability_roots, - ) - .await - } else { - self.mcp_runtime_for_step( - turn_context.as_ref(), - &environments, - &selected_capability_roots, - ) - .await + ), + McpAccess::Enabled => { + self.mcp_runtime_for_step( + turn_context.as_ref(), + &environments, + &selected_capability_roots, + ) + .await + } }; Arc::new(StepContext::new( turn_context, diff --git a/codex-rs/core/src/session/review.rs b/codex-rs/core/src/session/review.rs index fe201c568d..5a0e294b6d 100644 --- a/codex-rs/core/src/session/review.rs +++ b/codex-rs/core/src/session/review.rs @@ -1,5 +1,4 @@ use super::*; -use std::collections::HashMap; use std::sync::atomic::AtomicBool; /// Spawn a review thread using the given prompt. @@ -45,7 +44,7 @@ pub(super) async fn spawn_review_thread( // Build per‑turn client with the requested model/family. let mut per_turn_config = (*config).clone(); per_turn_config.model = Some(model.clone()); - per_turn_config.features = review_features.clone(); + per_turn_config.features = review_features; per_turn_config.permissions.shell_environment_policy = parent_turn_context .config .permissions @@ -54,13 +53,6 @@ pub(super) async fn spawn_review_thread( per_turn_config.codex_linux_sandbox_exe = parent_turn_context.config.codex_linux_sandbox_exe.clone(); per_turn_config.compact_prompt = parent_turn_context.config.compact_prompt.clone(); - per_turn_config.include_apps_instructions = false; - per_turn_config.orchestrator_mcp_enabled = false; - per_turn_config.mcp_servers = codex_config::Constrained::allow_only(HashMap::new()); - let _ = per_turn_config.features.disable(Feature::Apps); - let _ = per_turn_config - .features - .disable(Feature::SkillMcpDependencyInstall); if let Err(err) = per_turn_config.web_search_mode.set(review_web_search_mode) { let fallback_value = per_turn_config.web_search_mode.value(); tracing::warn!( diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index eae48e605f..826b1fd616 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -677,10 +677,7 @@ impl Session { let mcp_runtime_context = McpRuntimeContext::new(Arc::clone(&environment_manager), mcp_runtime_cwd); let mcp_runtime_context_for_auth = mcp_runtime_context.clone(); - let start_mcp_clients = !matches!( - session_configuration.session_source, - SessionSource::SubAgent(SubAgentSource::Review) - ); + let mcp_access = McpAccess::for_session_source(&session_configuration.session_source); let auth_and_mcp_fut = async move { let auth = auth_manager_clone.auth().await; let mcp_config = mcp_manager_for_mcp @@ -693,17 +690,18 @@ impl Session { .await; let mcp_servers = codex_mcp::effective_mcp_servers(&mcp_config, auth.as_ref()); let tool_plugin_provenance = codex_mcp::tool_plugin_provenance(&mcp_config); - let auth_statuses = if start_mcp_clients { - compute_auth_statuses( - mcp_servers.iter(), - config_for_mcp.mcp_oauth_credentials_store_mode, - config_for_mcp.auth_keyring_backend_kind(), - auth.as_ref(), - &mcp_runtime_context_for_auth, - ) - .await - } else { - Default::default() + let auth_statuses = match mcp_access { + McpAccess::Enabled => { + compute_auth_statuses( + mcp_servers.iter(), + config_for_mcp.mcp_oauth_credentials_store_mode, + config_for_mcp.auth_keyring_backend_kind(), + auth.as_ref(), + &mcp_runtime_context_for_auth, + ) + .await + } + McpAccess::Disabled => Default::default(), }; ( auth, @@ -1201,8 +1199,8 @@ impl Session { *cancel_guard = cancel_token.clone(); cancel_token }; - let mcp_connection_manager = if start_mcp_clients { - McpConnectionManager::new( + let mcp_connection_manager = match mcp_access { + McpAccess::Enabled => McpConnectionManager::new( &mcp_servers, config.mcp_oauth_credentials_store_mode, config.auth_keyring_backend_kind(), @@ -1230,13 +1228,12 @@ impl Session { "session_init.mcp_manager_init", otel.name = "session_init.mcp_manager_init", )) - .await - } else { - McpConnectionManager::new_uninitialized_with_permission_profile( + .await, + McpAccess::Disabled => McpConnectionManager::new_uninitialized_with_permission_profile( &session_configuration.approval_policy, &session_configuration.permission_profile(), config.prefix_mcp_tool_names(), - ) + ), }; sess.services .install_mcp_connection_manager( diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 6477ca1713..d0973ef05e 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -7790,7 +7790,7 @@ async fn built_tools_uses_the_step_mcp_runtime() -> anyhow::Result<()> { } #[tokio::test] -async fn review_step_uses_isolated_mcp_runtime_without_publishing_it() { +async fn review_step_uses_disabled_mcp_runtime_without_publishing_it() { let (mut session, mut turn_context) = make_session_and_context().await; session.services.selected_capability_roots = vec![SelectedCapabilityRoot { id: "review-root".to_string(), diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index 1806054206..f16b525002 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -1181,16 +1181,11 @@ pub(crate) async fn built_tools( ) -> CodexResult> { let turn_context = step_context.turn.as_ref(); let mcp_connection_manager = step_context.mcp.manager(); - let mcp_tools_enabled = !turn_context.is_review_subagent(); - let has_mcp_servers = mcp_tools_enabled && mcp_connection_manager.has_servers(); - let all_mcp_tools = if mcp_tools_enabled { - mcp_connection_manager - .list_all_tools() - .or_cancel(cancellation_token) - .await? - } else { - Vec::new() - }; + let has_mcp_servers = mcp_connection_manager.has_servers(); + let all_mcp_tools = mcp_connection_manager + .list_all_tools() + .or_cancel(cancellation_token) + .await?; let loaded_plugins = sess .services .plugins_manager diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index 348c81fea5..eecfc9e0a3 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -149,12 +149,36 @@ enum TurnMultiAgentRuntime { Preview, } +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum McpAccess { + Enabled, + Disabled, +} + +impl McpAccess { + pub(crate) fn for_session_source(session_source: &SessionSource) -> Self { + match session_source { + SessionSource::SubAgent(SubAgentSource::Review) => Self::Disabled, + SessionSource::Cli + | SessionSource::VSCode + | SessionSource::Exec + | SessionSource::Mcp + | SessionSource::Custom(_) + | SessionSource::Internal(_) + | SessionSource::Unknown + | SessionSource::SubAgent( + SubAgentSource::Compact + | SubAgentSource::MemoryConsolidation + | SubAgentSource::ThreadSpawn { .. } + | SubAgentSource::Other(_), + ) => Self::Enabled, + } + } +} + impl TurnContext { - pub(crate) fn is_review_subagent(&self) -> bool { - matches!( - self.session_source, - SessionSource::SubAgent(SubAgentSource::Review) - ) + pub(crate) fn mcp_access(&self) -> McpAccess { + McpAccess::for_session_source(&self.session_source) } pub(crate) fn permission_profile(&self) -> PermissionProfile { @@ -207,9 +231,11 @@ impl TurnContext { .auth_manager .as_deref() .is_some_and(AuthManager::current_auth_uses_codex_backend); - self.config - .features - .apps_enabled_for_auth(uses_codex_backend) + self.mcp_access() == McpAccess::Enabled + && self + .config + .features + .apps_enabled_for_auth(uses_codex_backend) && self.config.orchestrator_mcp_enabled } diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index 812e2d3aea..821fcc1c73 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -1,6 +1,7 @@ use crate::agent::exceeds_thread_spawn_depth_limit; use crate::agent::next_thread_spawn_depth; use crate::session::step_context::StepContext; +use crate::session::turn_context::McpAccess; use crate::session::turn_context::TurnContext; use crate::tools::code_mode::execute_spec::create_code_mode_tool; use crate::tools::context::ToolInvocation; @@ -334,7 +335,7 @@ pub(crate) fn search_tool_enabled(turn_context: &TurnContext) -> bool { pub(crate) fn tool_suggest_enabled(turn_context: &TurnContext) -> bool { let features = turn_context.config.features.get(); - !turn_context.is_review_subagent() + turn_context.mcp_access() == McpAccess::Enabled && features.enabled(Feature::ToolSuggest) && features.enabled(Feature::Apps) && features.enabled(Feature::Plugins) diff --git a/codex-rs/core/tests/suite/review.rs b/codex-rs/core/tests/suite/review.rs index 7c589cb12f..6dce40c1e2 100644 --- a/codex-rs/core/tests/suite/review.rs +++ b/codex-rs/core/tests/suite/review.rs @@ -1,14 +1,10 @@ use codex_config::test_support::CloudConfigBundleFixture; -use codex_config::types::McpServerConfig; -use codex_config::types::McpServerTransportConfig; use codex_core::CodexThread; use codex_core::REVIEW_PROMPT; use codex_core::config::Config; use codex_core::review_format::render_review_output_text; use codex_features::Feature; use codex_login::CodexAuth; -use codex_plugin::PluginHookSource; -use codex_plugin::PluginId; use codex_protocol::models::ContentItem; use codex_protocol::models::PermissionProfile; use codex_protocol::models::ResponseItem; @@ -25,26 +21,22 @@ use codex_protocol::protocol::ReviewTarget; use codex_protocol::protocol::RolloutItem; use codex_protocol::protocol::RolloutLine; use codex_protocol::user_input::UserInput; -use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; use core_test_support::PathBufExt; use core_test_support::apps_test_server::AppsTestServer; -use core_test_support::hooks::trust_hooks; +use core_test_support::apps_test_server::apps_enabled_builder; use core_test_support::responses; use core_test_support::responses::ResponseMock; use core_test_support::responses::mount_sse_sequence; use core_test_support::responses::start_mock_server; use core_test_support::skip_if_no_network; -use core_test_support::stdio_server_bin; +use core_test_support::test_codex::TestCodexBuilder; use core_test_support::test_codex::local_selections; use core_test_support::test_codex::test_codex; use core_test_support::wait_for_event; -use core_test_support::wait_for_mcp_server; use pretty_assertions::assert_eq; -use std::collections::HashMap; use std::path::PathBuf; use std::sync::Arc; -use std::time::Duration; use tempfile::TempDir; use tokio::io::AsyncWriteExt as _; use uuid::Uuid; @@ -271,72 +263,80 @@ async fn review_op_with_plain_text_emits_review_fallback() { server.verify().await; } +async fn submit_custom_review(codex: &CodexThread, instructions: &str) -> anyhow::Result<()> { + codex + .submit(Op::Review { + review_request: ReviewRequest { + target: ReviewTarget::Custom { + instructions: instructions.to_string(), + }, + user_facing_hint: None, + }, + }) + .await?; + Ok(()) +} + +fn pinned_apps_builder(codex_home: Arc, apps_base_url: String) -> TestCodexBuilder { + test_codex() + .with_home(codex_home) + .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) + .with_cloud_config_bundle( + CloudConfigBundleFixture::loader_with_enterprise_requirement( + r#"[features] +apps = true +"#, + ), + ) + .with_config(move |config| config.chatgpt_base_url = apps_base_url) +} + +fn write_enabled_test_plugin(codex_home: &TempDir) -> anyhow::Result { + let plugin_root = codex_home.path().join("plugins/cache/test/sample/local"); + std::fs::create_dir_all(plugin_root.join(".codex-plugin"))?; + std::fs::write( + plugin_root.join(".codex-plugin/plugin.json"), + r#"{"name":"sample"}"#, + )?; + std::fs::write( + codex_home.path().join("config.toml"), + r#"[features] +plugins = true + +[plugins."sample@test"] +enabled = true +"#, + )?; + Ok(plugin_root) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn review_does_not_expose_mcp_tools() -> anyhow::Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; let apps_server = AppsTestServer::mount(&server).await?; - let apps_base_url = apps_server.chatgpt_base_url.clone(); - let rmcp_test_server_bin = stdio_server_bin()?; let codex_home = Arc::new(TempDir::new()?); - let codex = test_codex() - .with_home(codex_home) - .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) - .with_config(move |config| { - config - .features - .enable(Feature::Apps) - .expect("test config should allow apps"); - config.chatgpt_base_url = apps_base_url; - config - .mcp_servers - .set(HashMap::from([( - "rmcp".to_string(), - McpServerConfig { - auth: Default::default(), - transport: McpServerTransportConfig::Stdio { - command: rmcp_test_server_bin, - args: Vec::new(), - env: None, - env_vars: Vec::new(), - cwd: None, - }, - environment_id: codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID.to_string(), - enabled: true, - required: false, - supports_parallel_tool_calls: false, - disabled_reason: None, - startup_timeout_sec: Some(Duration::from_secs(10)), - tool_timeout_sec: None, - default_tools_approval_mode: None, - enabled_tools: None, - disabled_tools: None, - scopes: None, - oauth: None, - oauth_resource: None, - tools: HashMap::new(), - }, - )])) - .expect("test MCP server config should be allowed"); - }) - .build_with_auto_env(&server) - .await? - .codex; - wait_for_mcp_server(&codex, "rmcp").await?; - let request_log = mount_sse_sequence(&server, vec![responses::sse(completed_sse())]).await; + let mut builder = + apps_enabled_builder(apps_server.chatgpt_base_url.clone()).with_home(codex_home); + let test = builder.build_with_auto_env(&server).await?; + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::McpStartupComplete(_)) + }) + .await; + let request_log = mount_sse_sequence( + &server, + vec![ + responses::sse(completed_sse()), + responses::sse(completed_sse()), + ], + ) + .await; - codex - .submit(Op::Review { - review_request: ReviewRequest { - target: ReviewTarget::Custom { - instructions: "Review without MCP tools".to_string(), - }, - user_facing_hint: None, - }, - }) - .await?; - let entered = wait_for_event(&codex, |event| { + test.submit_turn("Inspect available Apps tools").await?; + + submit_custom_review(&test.codex, "Review without MCP tools").await?; + let entered = wait_for_event(&test.codex, |event| { matches!( event, EventMsg::EnteredReviewMode(_) | EventMsg::Error(_) | EventMsg::TurnComplete(_) @@ -347,20 +347,16 @@ async fn review_does_not_expose_mcp_tools() -> anyhow::Result<()> { matches!(entered, EventMsg::EnteredReviewMode(_)), "review did not start: {entered:?}" ); - wait_for_event(&codex, |event| matches!(event, EventMsg::TurnComplete(_))).await; + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; - let request = request_log.single_request(); - assert!( - !request.body_contains_text("mcp__codex_apps"), - "review request unexpectedly exposed MCP tools: {:?}", - request.body_json()["tools"] - ); - assert!( - !request.body_contains_text("mcp__rmcp"), - "review request unexpectedly exposed configured MCP tools: {:?}", - request.body_json()["tools"] - ); + let requests = request_log.requests(); + assert!(requests[0].body_contains_text("list_mcp_resources")); + let request = &requests[1]; for tool_name in [ + "mcp__codex_apps", "list_mcp_resources", "list_mcp_resource_templates", "read_mcp_resource", @@ -405,20 +401,9 @@ async fn review_preserves_plugin_pre_tool_use_hooks() -> anyhow::Result<()> { .await; let codex_home = Arc::new(TempDir::new()?); - let plugin_root = codex_home.path().join("plugins/cache/test/sample/local"); + let plugin_root = write_enabled_test_plugin(&codex_home)?; let hooks_dir = plugin_root.join("hooks"); - std::fs::create_dir_all(plugin_root.join(".codex-plugin"))?; std::fs::create_dir_all(&hooks_dir)?; - std::fs::write( - plugin_root.join(".codex-plugin/plugin.json"), - r#"{"name":"sample"}"#, - )?; - std::fs::write( - codex_home.path().join("config.toml"), - r#"[plugins."sample@test"] -enabled = true -"#, - )?; let hook_script = hooks_dir.join("pre_tool_use_hook.py"); std::fs::write( @@ -447,53 +432,25 @@ print(json.dumps({ }] } }"#; - let plugin_hooks_path = hooks_dir.join("hooks.json"); - std::fs::write(&plugin_hooks_path, plugin_hooks_json)?; - let plugin_hook_sources = vec![PluginHookSource { - plugin_id: PluginId::parse("sample@test")?, - plugin_root: AbsolutePathBuf::try_from(plugin_root.clone())?, - plugin_data_root: AbsolutePathBuf::try_from(plugin_root.join("data"))?, - source_path: AbsolutePathBuf::try_from(plugin_hooks_path)?, - source_relative_path: "hooks/hooks.json".to_string(), - hooks: serde_json::from_str::(plugin_hooks_json)?.hooks, - }]; + std::fs::write(hooks_dir.join("hooks.json"), plugin_hooks_json)?; let test = test_codex() .with_home(Arc::clone(&codex_home)) .with_config(move |config| { - config - .features - .enable(Feature::Plugins) - .expect("test config should allow plugins"); config .features .enable(Feature::CodexHooks) .expect("test config should allow hooks"); + config.bypass_hook_trust = true; config .permissions .set_permission_profile(PermissionProfile::Disabled) .expect("test config should allow disabling the sandbox"); - let listed = codex_hooks::list_hooks(codex_hooks::HooksConfig { - feature_enabled: true, - config_layer_stack: Some(config.config_layer_stack.clone()), - plugin_hook_sources, - ..codex_hooks::HooksConfig::default() - }); - trust_hooks(config, listed.hooks); }) .build_with_auto_env(&server) .await?; - test.codex - .submit(Op::Review { - review_request: ReviewRequest { - target: ReviewTarget::Custom { - instructions: "Apply the patch while reviewing".to_string(), - }, - user_facing_hint: None, - }, - }) - .await?; + submit_custom_review(&test.codex, "Apply the patch while reviewing").await?; wait_for_event(&test.codex, |event| { matches!(event, EventMsg::TurnComplete(_)) }) @@ -527,23 +484,9 @@ async fn review_does_not_start_mcp_when_apps_feature_is_pinned_on() -> anyhow::R let server = start_mock_server().await; let apps_server = AppsTestServer::mount(&server).await?; - let apps_base_url = apps_server.chatgpt_base_url.clone(); let codex_home = Arc::new(TempDir::new()?); - let test = test_codex() - .with_home(codex_home) - .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) - .with_cloud_config_bundle( - CloudConfigBundleFixture::loader_with_enterprise_requirement( - r#"[features] -apps = true -"#, - ), - ) - .with_config(move |config| { - config.chatgpt_base_url = apps_base_url; - }) - .build_with_auto_env(&server) - .await?; + let mut builder = pinned_apps_builder(codex_home, apps_server.chatgpt_base_url.clone()); + let test = builder.build_with_auto_env(&server).await?; let mut pinned_features = test.config.features.clone(); pinned_features @@ -560,16 +503,7 @@ apps = true vec![responses::sse(vec![responses::ev_completed("resp-1")])], ) .await; - test.codex - .submit(Op::Review { - review_request: ReviewRequest { - target: ReviewTarget::Custom { - instructions: "Review without starting pinned Apps MCP".to_string(), - }, - user_facing_hint: None, - }, - }) - .await?; + submit_custom_review(&test.codex, "Review without starting pinned Apps MCP").await?; wait_for_event(&test.codex, |event| match event { EventMsg::McpStartupUpdate(update) => { @@ -608,18 +542,8 @@ async fn review_does_not_suggest_tools_when_apps_feature_is_pinned_on() -> anyho }))) .mount(&server) .await; - let apps_base_url = apps_server.chatgpt_base_url.clone(); let codex_home = Arc::new(TempDir::new()?); - let test = test_codex() - .with_home(codex_home) - .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) - .with_cloud_config_bundle( - CloudConfigBundleFixture::loader_with_enterprise_requirement( - r#"[features] -apps = true -"#, - ), - ) + let mut builder = pinned_apps_builder(codex_home, apps_server.chatgpt_base_url.clone()) .with_config(move |config| { for feature in [ Feature::Plugins, @@ -631,10 +555,8 @@ apps = true .enable(feature) .expect("test config should allow feature update"); } - config.chatgpt_base_url = apps_base_url; - }) - .build_with_auto_env(&server) - .await?; + }); + let test = builder.build_with_auto_env(&server).await?; let suggested_requests_before_review = server .received_requests() @@ -648,16 +570,7 @@ apps = true vec![responses::sse(vec![responses::ev_completed("resp-1")])], ) .await; - test.codex - .submit(Op::Review { - review_request: ReviewRequest { - target: ReviewTarget::Custom { - instructions: "Review without tool suggestions".to_string(), - }, - user_facing_hint: None, - }, - }) - .await?; + submit_custom_review(&test.codex, "Review without tool suggestions").await?; wait_for_event(&test.codex, |event| { matches!(event, EventMsg::TurnComplete(_)) @@ -691,12 +604,7 @@ async fn review_does_not_discover_auth_for_plugin_mcp_servers() -> anyhow::Resul let server = start_mock_server().await; let plugin_mcp_server = MockServer::start().await; let codex_home = Arc::new(TempDir::new()?); - let plugin_root = codex_home.path().join("plugins/cache/test/sample/local"); - std::fs::create_dir_all(plugin_root.join(".codex-plugin"))?; - std::fs::write( - plugin_root.join(".codex-plugin/plugin.json"), - r#"{"name":"sample"}"#, - )?; + let plugin_root = write_enabled_test_plugin(&codex_home)?; std::fs::write( plugin_root.join(".mcp.json"), format!( @@ -711,15 +619,6 @@ async fn review_does_not_discover_auth_for_plugin_mcp_servers() -> anyhow::Resul plugin_mcp_server.uri() ), )?; - std::fs::write( - codex_home.path().join("config.toml"), - r#"[features] -plugins = true - -[plugins."sample@test"] -enabled = true -"#, - )?; let test = test_codex() .with_home(codex_home) @@ -744,16 +643,7 @@ enabled = true vec![responses::sse(vec![responses::ev_completed("resp-1")])], ) .await; - test.codex - .submit(Op::Review { - review_request: ReviewRequest { - target: ReviewTarget::Custom { - instructions: "Review without MCP auth discovery".to_string(), - }, - user_facing_hint: None, - }, - }) - .await?; + submit_custom_review(&test.codex, "Review without MCP auth discovery").await?; wait_for_event(&test.codex, |event| { matches!(event, EventMsg::TurnComplete(_))