openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-28 18:58:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

Use MCP in reviews after startup completes

Browse Source
This commit is contained in:
Charlie Marsh
2026-06-28 22:37:39 -04:00
parent 133958a7ba
commit 28d3673af2
13 changed files with 243 additions and 104 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/codex-mcp/src/connection_manager.rs
+7
View File
@@ -370,6 +370,13 @@ impl McpConnectionManager {
!self.clients.is_empty()
}
/// Returns immediately with whether every configured client has finished starting.
pub fn startup_is_complete(&self) -> bool {
self.clients
.values()
.all(|client| client.startup_complete.load(Ordering::Acquire))
}
pub(crate) fn contains_server(&self, server_name: &str) -> bool {
self.clients.contains_key(server_name)
}
codex-rs/core/src/codex_delegate.rs
+5
View File
@@ -51,6 +51,7 @@ use crate::session::CodexSpawnOk;
use crate::session::SUBMISSION_CHANNEL_CAPACITY;
use crate::session::emit_subagent_session_started;
use crate::session::session::Session;
use crate::session::turn_context::McpAccess;
use crate::session::turn_context::TurnContext;
use codex_login::AuthManager;
use codex_models_manager::manager::SharedModelsManager;
@@ -81,6 +82,7 @@ pub(crate) async fn run_codex_thread_interactive(
parent_ctx: Arc<TurnContext>,
cancel_token: CancellationToken,
subagent_source: SubAgentSource,
mcp_access: McpAccess,
initial_history: Option<InitialHistory>,
) -> Result<Codex, CodexErr> {
let (tx_sub, rx_sub) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY);
@@ -110,6 +112,7 @@ pub(crate) async fn run_codex_thread_interactive(
conversation_history,
requested_history_mode: None,
session_source: SessionSource::SubAgent(subagent_source.clone()),
mcp_access,
forked_from_thread_id,
parent_thread_id: Some(parent_session.thread_id),
thread_source: Some(ThreadSource::Subagent),
@@ -202,6 +205,7 @@ pub(crate) async fn run_codex_thread_one_shot(
parent_ctx: Arc<TurnContext>,
cancel_token: CancellationToken,
subagent_source: SubAgentSource,
mcp_access: McpAccess,
final_output_json_schema: Option<Value>,
initial_history: Option<InitialHistory>,
) -> Result<Codex, CodexErr> {
@@ -216,6 +220,7 @@ pub(crate) async fn run_codex_thread_one_shot(
parent_ctx,
child_cancel.clone(),
subagent_source,
mcp_access,
initial_history,
))
.await?;
codex-rs/core/src/codex_delegate_tests.rs
+1
View File
@@ -174,6 +174,7 @@ async fn run_codex_thread_interactive_respects_pre_cancelled_spawn() {
parent_ctx,
cancel_token,
SubAgentSource::Review,
crate::session::turn_context::McpAccess::Enabled,
/*initial_history*/ None,
),
)
codex-rs/core/src/guardian/review_session.rs
+1
View File
@@ -670,6 +670,7 @@ async fn spawn_guardian_review_session(
Arc::clone(parent_turn),
cancel_token.clone(),
SubAgentSource::Other(GUARDIAN_REVIEWER_NAME.to_string()),
crate::session::turn_context::McpAccess::Enabled,
initial_history,
))
.await?;
codex-rs/core/src/session/mod.rs
+3
View File
@@ -423,6 +423,7 @@ pub(crate) struct CodexSpawnArgs {
pub(crate) conversation_history: InitialHistory,
pub(crate) requested_history_mode: Option<ThreadHistoryMode>,
pub(crate) session_source: SessionSource,
pub(crate) mcp_access: McpAccess,
pub(crate) forked_from_thread_id: Option<ThreadId>,
pub(crate) parent_thread_id: Option<ThreadId>,
pub(crate) thread_source: Option<ThreadSource>,
@@ -515,6 +516,7 @@ impl Codex {
conversation_history,
requested_history_mode,
session_source,
mcp_access,
forked_from_thread_id,
parent_thread_id,
thread_source,
@@ -687,6 +689,7 @@ impl Codex {
agent_status_tx.clone(),
conversation_history,
session_source_clone,
mcp_access,
skills_service,
plugins_manager,
mcp_manager.clone(),
codex-rs/core/src/session/review.rs
+1
View File
@@ -117,6 +117,7 @@ pub(super) async fn spawn_review_thread(
reasoning_effort,
reasoning_summary,
session_source,
mcp_access: parent_turn_context.mcp_access,
parent_thread_id: parent_turn_context.parent_thread_id,
originator: parent_turn_context.originator.clone(),
environments: parent_turn_context.environments.clone(),
codex-rs/core/src/session/session.rs
+3 -1
View File
@@ -38,6 +38,7 @@ pub(crate) struct Session {
/// The set of enabled features should be invariant for the lifetime of the
/// session.
pub(super) features: ManagedFeatures,
pub(super) mcp_access: McpAccess,
pub(super) multi_agent_version: OnceLock<MultiAgentVersion>,
pub(super) pending_mcp_server_refresh_config: Mutex<Option<McpServerRefreshConfig>>,
pub(crate) conversation: Arc<RealtimeConversationManager>,
@@ -490,6 +491,7 @@ impl Session {
agent_status: watch::Sender<AgentStatus>,
initial_history: InitialHistory,
session_source: SessionSource,
mcp_access: McpAccess,
skills_service: Arc<SkillsService>,
plugins_manager: Arc<PluginsManager>,
mcp_manager: Arc<McpManager>,
@@ -677,7 +679,6 @@ impl Session {
let mcp_runtime_context =
McpRuntimeContext::new(Arc::clone(&environment_manager), mcp_runtime_cwd);
let mcp_runtime_context_for_auth = mcp_runtime_context.clone();
let mcp_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
@@ -1142,6 +1143,7 @@ impl Session {
state: Mutex::new(state),
managed_network_proxy_refresh_lock: Semaphore::new(/*permits*/ 1),
features: config.features.clone(),
mcp_access,
multi_agent_version,
pending_mcp_server_refresh_config: Mutex::new(None),
conversation: Arc::new(RealtimeConversationManager::new()),
codex-rs/core/src/session/tests.rs
+10 -2
View File
@@ -5229,6 +5229,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_packaged_zsh() {
agent_status_tx,
InitialHistory::New,
SessionSource::Exec,
McpAccess::Enabled,
skills_service,
plugins_manager,
mcp_manager,
@@ -5478,6 +5479,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
&session_telemetry,
session_configuration.provider.clone(),
&session_configuration,
McpAccess::Enabled,
config.multi_agent_version_from_features(),
services.user_shell.as_ref(),
services.shell_zsh_path.as_ref(),
@@ -5500,6 +5502,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) {
state: Mutex::new(state),
managed_network_proxy_refresh_lock: Semaphore::new(/*permits*/ 1),
features: config.features.clone(),
mcp_access: McpAccess::Enabled,
multi_agent_version: OnceLock::from(config.multi_agent_version_from_features()),
pending_mcp_server_refresh_config: Mutex::new(None),
conversation: Arc::new(RealtimeConversationManager::new()),
@@ -5611,6 +5614,7 @@ async fn make_session_with_config_and_rx(
agent_status_tx,
InitialHistory::New,
SessionSource::Exec,
McpAccess::Enabled,
skills_service,
plugins_manager,
mcp_manager,
@@ -5719,6 +5723,7 @@ async fn make_session_with_history_source_and_agent_control_and_rx(
agent_status_tx,
initial_history,
session_source,
McpAccess::Enabled,
skills_service,
plugins_manager,
mcp_manager,
@@ -7604,6 +7609,7 @@ where
&session_telemetry,
session_configuration.provider.clone(),
&session_configuration,
McpAccess::Enabled,
config.multi_agent_version_from_features(),
services.user_shell.as_ref(),
services.shell_zsh_path.as_ref(),
@@ -7626,6 +7632,7 @@ where
state: Mutex::new(state),
managed_network_proxy_refresh_lock: Semaphore::new(/*permits*/ 1),
features: config.features.clone(),
mcp_access: McpAccess::Enabled,
multi_agent_version: OnceLock::from(config.multi_agent_version_from_features()),
pending_mcp_server_refresh_config: Mutex::new(None),
conversation: Arc::new(RealtimeConversationManager::new()),
@@ -7790,7 +7797,7 @@ async fn built_tools_uses_the_step_mcp_runtime() -> anyhow::Result<()> {
}
#[tokio::test]
async fn review_step_uses_disabled_mcp_runtime_without_publishing_it() {
async fn disabled_mcp_step_uses_isolated_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(),
@@ -7799,7 +7806,8 @@ async fn review_step_uses_disabled_mcp_runtime_without_publishing_it() {
path: PathUri::from_abs_path(&turn_context.config.cwd),
},
}];
turn_context.session_source = SessionSource::SubAgent(SubAgentSource::Review);
session.mcp_access = McpAccess::Disabled;
turn_context.mcp_access = McpAccess::Disabled;
let session = Arc::new(session);
let turn_context = Arc::new(turn_context);
let published_before = session.services.latest_mcp_runtime();
codex-rs/core/src/session/tests/guardian_tests.rs
+1
View File
@@ -734,6 +734,7 @@ async fn guardian_subagent_does_not_inherit_parent_exec_policy_rules() {
session_source: SessionSource::SubAgent(SubAgentSource::Other(
GUARDIAN_REVIEWER_NAME.to_string(),
)),
mcp_access: crate::session::turn_context::McpAccess::Enabled,
forked_from_thread_id: None,
parent_thread_id: None,
thread_source: None,
codex-rs/core/src/session/turn_context.rs
+6 -23
View File
@@ -10,7 +10,6 @@ use codex_protocol::ThreadId;
use codex_protocol::models::AdditionalPermissionProfile;
use codex_protocol::openai_models::ModelInfo;
use codex_protocol::protocol::MultiAgentVersion;
use codex_protocol::protocol::SubAgentSource;
use codex_protocol::protocol::TurnEnvironmentSelection;
use codex_sandboxing::compatibility_sandbox_policy_for_permission_profile;
use codex_sandboxing::policy_transforms::effective_file_system_sandbox_policy;
@@ -112,6 +111,7 @@ pub struct TurnContext {
pub(crate) reasoning_effort: Option<ReasoningEffortConfig>,
pub(crate) reasoning_summary: ReasoningSummaryConfig,
pub(crate) session_source: SessionSource,
pub(crate) mcp_access: McpAccess,
pub(crate) parent_thread_id: Option<ThreadId>,
pub(crate) originator: String,
pub(crate) environments: TurnEnvironmentSnapshot,
@@ -155,30 +155,9 @@ pub(crate) enum McpAccess {
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 mcp_access(&self) -> McpAccess {
McpAccess::for_session_source(&self.session_source)
self.mcp_access
}
pub(crate) fn permission_profile(&self) -> PermissionProfile {
@@ -296,6 +275,7 @@ impl TurnContext {
reasoning_effort,
reasoning_summary: self.reasoning_summary,
session_source: self.session_source.clone(),
mcp_access: self.mcp_access,
parent_thread_id: self.parent_thread_id,
originator: self.originator.clone(),
environments: self.environments.clone(),
@@ -506,6 +486,7 @@ impl Session {
session_telemetry: &SessionTelemetry,
provider: ModelProviderInfo,
session_configuration: &SessionConfiguration,
mcp_access: McpAccess,
multi_agent_version: MultiAgentVersion,
user_shell: &shell::Shell,
shell_zsh_path: Option<&PathBuf>,
@@ -574,6 +555,7 @@ impl Session {
reasoning_effort,
reasoning_summary,
session_source,
mcp_access,
parent_thread_id: session_configuration.parent_thread_id,
originator: session_configuration.originator.clone(),
environments,
@@ -775,6 +757,7 @@ impl Session {
&self.services.session_telemetry,
session_configuration.provider.clone(),
&session_configuration,
self.mcp_access,
multi_agent_version,
self.services.user_shell.as_ref(),
self.services.shell_zsh_path.as_ref(),
codex-rs/core/src/tasks/review.rs
+13
View File
@@ -22,6 +22,7 @@ use crate::review_format::format_review_findings_block;
use crate::review_format::render_review_output_text;
use crate::session::TurnInput;
use crate::session::session::Session;
use crate::session::turn_context::McpAccess;
use crate::session::turn_context::TurnContext;
use crate::state::TaskKind;
use codex_features::Feature;
@@ -122,6 +123,17 @@ async fn start_review_conversation(
.clone()
.unwrap_or_else(|| ctx.model_info.slug.clone());
sub_agent_config.model = Some(model);
let mcp_access = if session
.clone_session()
.services
.latest_mcp_runtime()
.manager()
.startup_is_complete()
{
McpAccess::Enabled
} else {
McpAccess::Disabled
};
(run_codex_thread_one_shot(
sub_agent_config,
session.auth_manager(),
@@ -131,6 +143,7 @@ async fn start_review_conversation(
ctx.clone(),
cancellation_token,
SubAgentSource::Review,
mcp_access,
/*final_output_json_schema*/ None,
/*initial_history*/ None,
)
codex-rs/core/src/thread_manager.rs
+1
View File
@@ -1588,6 +1588,7 @@ impl ThreadManagerState {
conversation_history: initial_history,
requested_history_mode: history_mode,
session_source,
mcp_access: crate::session::turn_context::McpAccess::Enabled,
forked_from_thread_id,
parent_thread_id,
thread_source,
codex-rs/core/tests/suite/review.rs
+191 -78
View File
@@ -37,8 +37,10 @@ use core_test_support::wait_for_event;
use pretty_assertions::assert_eq;
use std::path::PathBuf;
use std::sync::Arc;
use std::time::Duration;
use tempfile::TempDir;
use tokio::io::AsyncWriteExt as _;
use tokio::time::timeout;
use uuid::Uuid;
use wiremock::Mock;
use wiremock::MockServer;
@@ -291,6 +293,42 @@ apps = true
.with_config(move |config| config.chatgpt_base_url = apps_base_url)
}
fn pinned_apps_with_tool_suggest_builder(
codex_home: Arc<TempDir>,
apps_base_url: String,
) -> TestCodexBuilder {
pinned_apps_builder(codex_home, apps_base_url).with_config(|config| {
for feature in [
Feature::Plugins,
Feature::RemotePlugin,
Feature::ToolSuggest,
] {
config
.features
.enable(feature)
.expect("test config should allow feature update");
}
})
}
async fn mount_suggested_plugin(server: &MockServer) {
Mock::given(method("GET"))
.and(path("/ps/plugins/suggested"))
.and(query_param("scope", "GLOBAL"))
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
"enabled": true,
"plugins": [{
"id": "plugin_github",
"name": "github",
"status": "ENABLED",
"installation_policy": "AVAILABLE",
"release": {"display_name": "GitHub"}
}]
})))
.mount(server)
.await;
}
fn write_enabled_test_plugin(codex_home: &TempDir) -> anyhow::Result<PathBuf> {
let plugin_root = codex_home.path().join("plugins/cache/test/sample/local");
std::fs::create_dir_all(plugin_root.join(".codex-plugin"))?;
@@ -310,19 +348,60 @@ enabled = true
Ok(plugin_root)
}
fn write_test_plugin_mcp(plugin_root: &std::path::Path, server_url: &str) -> anyhow::Result<()> {
std::fs::write(
plugin_root.join(".mcp.json"),
format!(r#"{{"mcpServers":{{"sample":{{"type":"http","url":"{server_url}/mcp"}}}}}}"#),
)?;
Ok(())
}
async fn mount_pending_mcp_server(server: &MockServer) {
Mock::given(method("POST"))
.and(path("/mcp"))
.respond_with(ResponseTemplate::new(200).set_delay(Duration::from_secs(60)))
.mount(server)
.await;
}
async fn wait_for_pending_mcp_request(server: &MockServer) -> anyhow::Result<usize> {
timeout(Duration::from_secs(5), async {
loop {
let request_count = server
.received_requests()
.await
.unwrap_or_default()
.into_iter()
.filter(|request| request.method.as_str() == "POST" && request.url.path() == "/mcp")
.count();
if request_count > 0 {
return request_count;
}
tokio::task::yield_now().await;
}
})
.await
.map_err(|_| anyhow::anyhow!("MCP server did not receive a startup request"))
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn review_does_not_expose_mcp_tools() -> anyhow::Result<()> {
async fn review_exposes_mcp_tools_when_parent_mcp_is_ready() -> anyhow::Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let apps_server = AppsTestServer::mount(&server).await?;
let unavailable_mcp_server = MockServer::start().await;
let codex_home = Arc::new(TempDir::new()?);
let plugin_root = write_enabled_test_plugin(&codex_home)?;
std::fs::write(
plugin_root.join(".mcp.json"),
format!(
r#"{{"mcpServers":{{"sample":{{"type":"http","url":"{}/api/codex/apps"}}}}}}"#,
apps_server.chatgpt_base_url
r#"{{"mcpServers":{{
"sample":{{"type":"http","url":"{}/api/codex/apps"}},
"unavailable":{{"type":"http","url":"{}/mcp"}}
}}}}"#,
apps_server.chatgpt_base_url,
unavailable_mcp_server.uri()
),
)?;
let mut builder = apps_enabled_builder(apps_server.chatgpt_base_url.clone())
@@ -346,7 +425,7 @@ async fn review_does_not_expose_mcp_tools() -> anyhow::Result<()> {
test.submit_turn("Inspect available Apps tools").await?;
submit_custom_review(&test.codex, "Review without MCP tools").await?;
submit_custom_review(&test.codex, "Review with ready MCP tools").await?;
let entered = wait_for_event(&test.codex, |event| {
matches!(
event,
@@ -373,26 +452,14 @@ async fn review_does_not_expose_mcp_tools() -> anyhow::Result<()> {
requests[0].body_json()["tools"]
);
let request = &requests[1];
assert!(request.body_contains_text("list_mcp_resources"));
assert!(
request
.tool_by_name("mcp__sample", "calendar_create_event")
.is_none(),
"review request unexpectedly exposed the configured MCP tool: {:?}",
.is_some(),
"review request should expose the ready MCP tool: {:?}",
request.body_json()["tools"]
);
for tool_name in [
"mcp__codex_apps",
"mcp__sample",
"list_mcp_resources",
"list_mcp_resource_templates",
"read_mcp_resource",
] {
assert!(
!request.body_contains_text(tool_name),
"review request unexpectedly exposed {tool_name}: {:?}",
request.body_json()["tools"]
);
}
Ok(())
}
@@ -505,12 +572,16 @@ print(json.dumps({
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn review_does_not_start_mcp_when_apps_feature_is_pinned_on() -> anyhow::Result<()> {
async fn review_skips_pending_mcp_even_if_apps_is_pinned() -> anyhow::Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let apps_server = AppsTestServer::mount(&server).await?;
let codex_home = Arc::new(TempDir::new()?);
let plugin_mcp_server = MockServer::start().await;
mount_pending_mcp_server(&plugin_mcp_server).await;
let plugin_root = write_enabled_test_plugin(&codex_home)?;
write_test_plugin_mcp(&plugin_root, &plugin_mcp_server.uri())?;
let mut builder = pinned_apps_builder(codex_home, apps_server.chatgpt_base_url.clone());
let test = builder.build_with_auto_env(&server).await?;
@@ -519,10 +590,7 @@ async fn review_does_not_start_mcp_when_apps_feature_is_pinned_on() -> anyhow::R
.disable(Feature::Apps)
.expect("managed feature normalization should succeed");
assert!(pinned_features.enabled(Feature::Apps));
wait_for_event(&test.codex, |event| {
matches!(event, EventMsg::McpStartupComplete(_))
})
.await;
let mcp_requests_before_review = wait_for_pending_mcp_request(&plugin_mcp_server).await?;
let responses = mount_sse_sequence(
&server,
@@ -531,11 +599,16 @@ async fn review_does_not_start_mcp_when_apps_feature_is_pinned_on() -> anyhow::R
.await;
submit_custom_review(&test.codex, "Review without starting pinned Apps MCP").await?;
let mut review_started = false;
wait_for_event(&test.codex, |event| match event {
EventMsg::McpStartupUpdate(update) => {
EventMsg::EnteredReviewMode(_) => {
review_started = true;
false
}
EventMsg::McpStartupUpdate(update) if review_started => {
panic!("review emitted an MCP startup update: {update:?}")
}
EventMsg::McpStartupComplete(complete) => {
EventMsg::McpStartupComplete(complete) if review_started => {
panic!("review emitted an MCP startup completion: {complete:?}")
}
EventMsg::TurnComplete(_) => true,
@@ -544,45 +617,44 @@ async fn review_does_not_start_mcp_when_apps_feature_is_pinned_on() -> anyhow::R
.await;
assert_eq!(responses.requests().len(), 1);
let request = responses.single_request();
for tool_name in [
"mcp__codex_apps",
"mcp__sample",
"list_mcp_resources",
"list_mcp_resource_templates",
"read_mcp_resource",
] {
assert!(
!request.body_contains_text(tool_name),
"pending review unexpectedly exposed {tool_name}: {:?}",
request.body_json()["tools"]
);
}
assert_eq!(
wait_for_pending_mcp_request(&plugin_mcp_server).await?,
mcp_requests_before_review,
"review should not start a second MCP client while the parent MCP is pending"
);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn review_does_not_suggest_tools_when_apps_feature_is_pinned_on() -> anyhow::Result<()> {
async fn review_skips_tool_suggestions_with_pending_mcp_and_pinned_apps() -> anyhow::Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let apps_server = AppsTestServer::mount(&server).await?;
Mock::given(method("GET"))
.and(path("/ps/plugins/suggested"))
.and(query_param("scope", "GLOBAL"))
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
"enabled": true,
"plugins": [{
"id": "plugin_github",
"name": "github",
"status": "ENABLED",
"installation_policy": "AVAILABLE",
"release": {"display_name": "GitHub"}
}]
})))
.mount(&server)
.await;
mount_suggested_plugin(&server).await;
let codex_home = Arc::new(TempDir::new()?);
let mut builder = pinned_apps_builder(codex_home, apps_server.chatgpt_base_url.clone())
.with_config(move |config| {
for feature in [
Feature::Plugins,
Feature::RemotePlugin,
Feature::ToolSuggest,
] {
config
.features
.enable(feature)
.expect("test config should allow feature update");
}
});
let plugin_mcp_server = MockServer::start().await;
mount_pending_mcp_server(&plugin_mcp_server).await;
let plugin_root = write_enabled_test_plugin(&codex_home)?;
write_test_plugin_mcp(&plugin_root, &plugin_mcp_server.uri())?;
let mut builder =
pinned_apps_with_tool_suggest_builder(codex_home, apps_server.chatgpt_base_url.clone());
let test = builder.build_with_auto_env(&server).await?;
wait_for_pending_mcp_request(&plugin_mcp_server).await?;
let suggested_requests_before_review = server
.received_requests()
@@ -624,45 +696,86 @@ async fn review_does_not_suggest_tools_when_apps_feature_is_pinned_on() -> anyho
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn review_does_not_discover_auth_for_plugin_mcp_servers() -> anyhow::Result<()> {
async fn review_keeps_tool_suggestions_with_ready_mcp_and_pinned_apps() -> anyhow::Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let apps_server = AppsTestServer::mount(&server).await?;
mount_suggested_plugin(&server).await;
let codex_home = Arc::new(TempDir::new()?);
let mut builder =
pinned_apps_with_tool_suggest_builder(codex_home, apps_server.chatgpt_base_url.clone());
let test = builder.build_with_auto_env(&server).await?;
wait_for_event(&test.codex, |event| {
matches!(event, EventMsg::McpStartupComplete(_))
})
.await;
let suggested_requests_before_review = server
.received_requests()
.await
.unwrap_or_default()
.into_iter()
.filter(|request| request.url.path() == "/ps/plugins/suggested")
.count();
let responses = mount_sse_sequence(
&server,
vec![responses::sse(vec![responses::ev_completed("resp-1")])],
)
.await;
submit_custom_review(&test.codex, "Review with tool suggestions").await?;
wait_for_event(&test.codex, |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
let request = responses.single_request();
assert!(
request.body_json()["tools"]
.as_array()
.is_some_and(|tools| tools.iter().any(|tool| {
tool.get("name").and_then(serde_json::Value::as_str)
== Some("request_plugin_install")
})),
"ready review should expose request_plugin_install: {:?}",
request.body_json()["tools"]
);
let suggested_requests_after_review = server
.received_requests()
.await
.unwrap_or_default()
.into_iter()
.filter(|request| request.url.path() == "/ps/plugins/suggested")
.count();
assert!(
suggested_requests_after_review > suggested_requests_before_review,
"ready review should fetch recommended plugins"
);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn review_skips_auth_discovery_when_parent_mcp_is_pending() -> anyhow::Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let plugin_mcp_server = MockServer::start().await;
mount_pending_mcp_server(&plugin_mcp_server).await;
let codex_home = Arc::new(TempDir::new()?);
let plugin_root = write_enabled_test_plugin(&codex_home)?;
std::fs::write(
plugin_root.join(".mcp.json"),
format!(
r#"{{
"mcpServers": {{
"sample": {{
"type": "http",
"url": "{}/mcp"
}}
}}
}}"#,
plugin_mcp_server.uri()
),
)?;
write_test_plugin_mcp(&plugin_root, &plugin_mcp_server.uri())?;
let test = test_codex()
.with_home(codex_home)
.build_with_auto_env(&server)
.await?;
wait_for_event(&test.codex, |event| {
matches!(event, EventMsg::McpStartupComplete(_))
})
.await;
wait_for_pending_mcp_request(&plugin_mcp_server).await?;
let plugin_mcp_requests_before_review = plugin_mcp_server
.received_requests()
.await
.unwrap_or_default()
.len();
assert!(
plugin_mcp_requests_before_review > 0,
"parent session should exercise the plugin MCP endpoint"
);
let responses = mount_sse_sequence(
&server,