agent/codex
1
0
Fork 0
mirror of https://github.com/pchuan98/codex.git synced 2026-07-01 00:31:56 +08:00
Code Issues Packages Projects Releases Wiki Activity

Remove core MCP list tools op (#21281)

Browse Source 
## Why

The core `Op::ListMcpTools` request path is no longer needed. Keeping it
around left a dead request/response surface alongside the app-server MCP
inventory APIs that own current server status listing.

## What Changed

- Removed `Op::ListMcpTools`, `EventMsg::McpListToolsResponse`, and the
core handler that built the MCP snapshot response.
- Removed the now-unused `codex-mcp` snapshot wrapper/export and passive
event handling arms in rollout and MCP-server consumers.
- Updated tests that used the old op as a synchronization hook to wait
on existing startup/skills events, and deleted the plugin test that only
exercised the removed listing op.

## Validation

- `cargo test -p codex-protocol`
- `cargo test -p codex-mcp`
- `cargo test -p codex-rollout -p codex-rollout-trace -p
codex-mcp-server`
- `cargo test -p codex-core --test all
pending_input::queued_inter_agent_mail`
- `cargo test -p codex-core --test all
rmcp_client::stdio_mcp_tool_call_includes_sandbox_state_meta`
- `cargo test -p codex-core --test all
rmcp_client::stdio_image_responses`
- `just fix -p codex-core -p codex-protocol -p codex-mcp -p
codex-rollout -p codex-rollout-trace -p codex-mcp-server`
This commit is contained in:
pakrym-oai
2026-05-06 11:20:34 -07:00
committed by GitHub
Unverified
parent 123ec8b035
commit 712305be47
13 changed files with 46 additions and 220 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/Cargo.lock
Generated
+1
View File
@@ -4275,6 +4275,7 @@ dependencies = [
"reqwest",
"serde_json",
"shlex",
"similar",
"tempfile",
"tokio",
"tokio-tungstenite",
codex-rs/codex-mcp/src/lib.rs
-1
View File
@@ -32,7 +32,6 @@ pub use mcp::with_codex_apps_mcp;
pub use mcp::McpServerStatusSnapshot;
pub use mcp::McpSnapshotDetail;
pub use mcp::collect_mcp_server_status_snapshot_with_detail;
pub use mcp::collect_mcp_snapshot_from_manager;
pub use mcp::read_mcp_resource;
pub use mcp::McpAuthStatusEntry;
codex-rs/codex-mcp/src/mcp/mod.rs
-51
View File
@@ -31,7 +31,6 @@ use codex_protocol::mcp::Tool;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::McpAuthStatus;
use codex_protocol::protocol::McpListToolsResponseEvent;
use rmcp::model::ReadResourceRequestParams;
use rmcp::model::ReadResourceResult;
use serde_json::Value;
@@ -347,18 +346,6 @@ pub async fn collect_mcp_server_status_snapshot_with_detail(
snapshot
}
pub async fn collect_mcp_snapshot_from_manager(
mcp_connection_manager: &McpConnectionManager,
auth_status_entries: HashMap<String, McpAuthStatusEntry>,
) -> McpListToolsResponseEvent {
collect_mcp_snapshot_from_manager_with_detail(
mcp_connection_manager,
auth_status_entries,
McpSnapshotDetail::Full,
)
.await
}
pub(crate) fn codex_apps_mcp_url(config: &McpConfig) -> String {
codex_apps_mcp_url_for_base_url(
&config.chatgpt_base_url,
@@ -595,44 +582,6 @@ async fn collect_mcp_server_status_snapshot_from_manager(
}
}
async fn collect_mcp_snapshot_from_manager_with_detail(
mcp_connection_manager: &McpConnectionManager,
auth_status_entries: HashMap<String, McpAuthStatusEntry>,
detail: McpSnapshotDetail,
) -> McpListToolsResponseEvent {
let (tools, resources, resource_templates) = tokio::join!(
mcp_connection_manager.list_all_tools(),
async {
if detail.include_resources() {
mcp_connection_manager.list_all_resources().await
} else {
HashMap::new()
}
},
async {
if detail.include_resources() {
mcp_connection_manager.list_all_resource_templates().await
} else {
HashMap::new()
}
},
);
let tools = tools
.into_iter()
.filter_map(|(name, tool)| {
protocol_tool_from_rmcp_tool(&name, &tool.tool).map(|tool| (name, tool))
})
.collect::<HashMap<_, _>>();
McpListToolsResponseEvent {
tools,
resources: convert_mcp_resources(resources),
resource_templates: convert_mcp_resource_templates(resource_templates),
auth_statuses: auth_statuses_from_entries(&auth_status_entries),
}
}
#[cfg(test)]
#[path = "mod_tests.rs"]
mod tests;
codex-rs/core/src/session/handlers.rs
-35
View File
@@ -24,8 +24,6 @@ use crate::tasks::CompactTask;
use crate::tasks::UserShellCommandMode;
use crate::tasks::UserShellCommandTask;
use crate::tasks::execute_user_shell_command;
use codex_mcp::collect_mcp_snapshot_from_manager;
use codex_mcp::compute_auth_statuses;
use codex_protocol::models::ContentItem;
use codex_protocol::models::ResponseInputItem;
use codex_protocol::protocol::CodexErrorInfo;
@@ -469,35 +467,6 @@ pub async fn reload_user_config(sess: &Arc<Session>) {
sess.reload_user_config_layer().await;
}
#[expect(
clippy::await_holding_invalid_type,
reason = "MCP tool listing reads through the session-owned manager guard"
)]
pub async fn list_mcp_tools(sess: &Session, config: &Arc<Config>, sub_id: String) {
let mcp_connection_manager = sess.services.mcp_connection_manager.read().await;
let auth = sess.services.auth_manager.auth().await;
let mcp_servers = sess
.services
.mcp_manager
.effective_servers(config, auth.as_ref())
.await;
let snapshot = collect_mcp_snapshot_from_manager(
&mcp_connection_manager,
compute_auth_statuses(
mcp_servers.iter(),
config.mcp_oauth_credentials_store_mode,
auth.as_ref(),
)
.await,
)
.await;
let event = Event {
id: sub_id,
msg: EventMsg::McpListToolsResponse(snapshot),
};
sess.send_event_raw(event).await;
}
pub async fn compact(sess: &Arc<Session>, sub_id: String) {
let turn_context = sess.new_default_turn_with_sub_id(sub_id).await;
@@ -863,10 +832,6 @@ pub(super) async fn submission_loop(
dynamic_tool_response(&sess, id, response).await;
false
}
Op::ListMcpTools => {
list_mcp_tools(&sess, &config, sub.id.clone()).await;
false
}
Op::RefreshMcpServers { config } => {
refresh_mcp_servers(&sess, config).await;
false
codex-rs/core/src/session/turn.rs
-1
View File
@@ -1509,7 +1509,6 @@ pub(super) fn realtime_text_for_event(msg: &EventMsg) -> Option<String> {
| EventMsg::DeprecationNotice(_)
| EventMsg::StreamError(_)
| EventMsg::TurnDiff(_)
| EventMsg::McpListToolsResponse(_)
| EventMsg::RealtimeConversationListVoicesResponse(_)
| EventMsg::SkillsUpdateAvailable
| EventMsg::PlanUpdate(_)
codex-rs/core/tests/common/Cargo.toml
+1
View File
@@ -33,6 +33,7 @@ opentelemetry = { workspace = true }
opentelemetry_sdk = { workspace = true }
regex-lite = { workspace = true }
serde_json = { workspace = true }
similar = { workspace = true }
tempfile = { workspace = true }
tokio = { workspace = true, features = ["net", "time"] }
tokio-tungstenite = { workspace = true }
codex-rs/core/tests/suite/pending_input.rs
+3 -3
View File
@@ -164,11 +164,11 @@ async fn submit_queue_only_agent_mail(codex: &CodexThread, text: &str) {
.await
.unwrap_or_else(|err| panic!("submit queue-only agent mail: {err}"));
codex
.submit(Op::ListMcpTools)
.submit(Op::RealtimeConversationListVoices)
.await
.unwrap_or_else(|err| panic!("submit list-mcp-tools barrier: {err}"));
.unwrap_or_else(|err| panic!("submit list-voices barrier: {err}"));
wait_for_event(codex, |event| {
matches!(event, EventMsg::McpListToolsResponse(_))
matches!(event, EventMsg::RealtimeConversationListVoicesResponse(_))
})
.await;
}
codex-rs/core/tests/suite/plugins.rs
-45
View File
@@ -99,20 +99,6 @@ fn write_plugin_app_plugin(home: &TempDir) {
.expect("write plugin app config");
}
async fn build_plugin_test_codex(
server: &MockServer,
codex_home: Arc<TempDir>,
) -> Result<Arc<codex_core::CodexThread>> {
let mut builder = test_codex()
.with_home(codex_home)
.with_auth(CodexAuth::from_api_key("Test API Key"));
Ok(builder
.build(server)
.await
.expect("create new conversation")
.codex)
}
async fn build_analytics_plugin_test_codex(
server: &MockServer,
codex_home: Arc<TempDir>,
@@ -448,34 +434,3 @@ async fn explicit_plugin_mentions_track_plugin_used_analytics() -> Result<()> {
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
async fn plugin_mcp_tools_are_listed() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let codex_home = Arc::new(TempDir::new()?);
let rmcp_test_server_bin = stdio_server_bin()?;
write_plugin_mcp_plugin(codex_home.as_ref(), &rmcp_test_server_bin);
let codex = build_plugin_test_codex(&server, codex_home).await?;
wait_for_sample_mcp_ready(&codex).await?;
codex.submit(Op::ListMcpTools).await?;
let list_event = wait_for_event_with_timeout(
&codex,
|ev| matches!(ev, EventMsg::McpListToolsResponse(_)),
Duration::from_secs(10),
)
.await;
let EventMsg::McpListToolsResponse(tool_list) = list_event else {
unreachable!("event guard guarantees McpListToolsResponse");
};
let mut available_tools: Vec<&str> = tool_list.tools.keys().map(String::as_str).collect();
available_tools.sort_unstable();
assert!(
tool_list.tools.contains_key("mcp__sample__echo")
&& tool_list.tools.contains_key("mcp__sample__image"),
"expected plugin MCP tools to be listed; discovered tools: {available_tools:?}"
);
Ok(())
}
codex-rs/core/tests/suite/rmcp_client.rs
+41 -57
View File
@@ -242,31 +242,42 @@ fn copy_binary_to_remote_env(
Ok(remote_path)
}
async fn wait_for_mcp_tool(fixture: &TestCodex, tool_name: &str) -> anyhow::Result<()> {
let tools_ready_deadline = Instant::now() + Duration::from_secs(30);
loop {
fixture.codex.submit(Op::ListMcpTools).await?;
let list_event = wait_for_event_with_timeout(
&fixture.codex,
|ev| matches!(ev, EventMsg::McpListToolsResponse(_)),
Duration::from_secs(10),
)
.await;
let EventMsg::McpListToolsResponse(tool_list) = list_event else {
unreachable!("event guard guarantees McpListToolsResponse");
};
if tool_list.tools.contains_key(tool_name) {
return Ok(());
}
let available_tools: Vec<&str> = tool_list.tools.keys().map(String::as_str).collect();
if Instant::now() >= tools_ready_deadline {
panic!(
"timed out waiting for MCP tool {tool_name} to become available; discovered tools: {available_tools:?}"
);
}
sleep(Duration::from_millis(200)).await;
async fn wait_for_mcp_server(fixture: &TestCodex, server_name: &str) -> anyhow::Result<()> {
let startup_event = wait_for_event_with_timeout(
&fixture.codex,
|ev| match ev {
EventMsg::McpStartupComplete(summary) => {
summary.ready.iter().any(|server| server == server_name)
|| summary
.failed
.iter()
.any(|failure| failure.server == server_name)
|| summary.cancelled.iter().any(|server| server == server_name)
}
_ => false,
},
Duration::from_secs(70),
)
.await;
let EventMsg::McpStartupComplete(summary) = startup_event else {
unreachable!("event guard guarantees McpStartupComplete");
};
if let Some(failure) = summary
.failed
.iter()
.find(|failure| failure.server == server_name)
{
let error = &failure.error;
anyhow::bail!("MCP server {server_name} failed to start: {error}");
}
if summary.cancelled.iter().any(|server| server == server_name) {
anyhow::bail!("MCP server {server_name} startup was cancelled");
}
ensure!(
summary.ready.iter().any(|server| server == server_name),
"expected MCP server {server_name} to be ready; startup summary: {summary:?}"
);
Ok(())
}
#[derive(Default)]
@@ -731,7 +742,6 @@ async fn stdio_mcp_tool_call_includes_sandbox_state_meta() -> anyhow::Result<()>
let call_id = "sandbox-meta-call";
let server_name = "rmcp";
let namespace = format!("mcp__{server_name}__");
let tool_name = format!("{namespace}sandbox_meta");
let call_mock = mount_sse_once(
&server,
@@ -767,30 +777,7 @@ async fn stdio_mcp_tool_call_includes_sandbox_state_meta() -> anyhow::Result<()>
.build_remote_aware(&server)
.await?;
let tools_ready_deadline = Instant::now() + Duration::from_secs(30);
loop {
fixture.codex.submit(Op::ListMcpTools).await?;
let list_event = wait_for_event_with_timeout(
&fixture.codex,
|ev| matches!(ev, EventMsg::McpListToolsResponse(_)),
Duration::from_secs(10),
)
.await;
let EventMsg::McpListToolsResponse(tool_list) = list_event else {
unreachable!("event guard guarantees McpListToolsResponse");
};
if tool_list.tools.contains_key(&tool_name) {
break;
}
let available_tools: Vec<&str> = tool_list.tools.keys().map(String::as_str).collect();
if Instant::now() >= tools_ready_deadline {
panic!(
"timed out waiting for MCP tool {tool_name} to become available; discovered tools: {available_tools:?}"
);
}
sleep(Duration::from_millis(200)).await;
}
wait_for_mcp_server(&fixture, server_name).await?;
fixture
.submit_turn_with_permission_profile(
@@ -1039,7 +1026,6 @@ async fn stdio_image_responses_round_trip() -> anyhow::Result<()> {
let call_id = "img-1";
let server_name = "rmcp";
let tool_name = format!("mcp__{server_name}__image");
let namespace = format!("mcp__{server_name}__");
// First stream: model decides to call the image tool.
@@ -1086,7 +1072,7 @@ async fn stdio_image_responses_round_trip() -> anyhow::Result<()> {
})
.build_remote_aware(&server)
.await?;
wait_for_mcp_tool(&fixture, &tool_name).await?;
wait_for_mcp_server(&fixture, server_name).await?;
fixture
.codex
@@ -1176,7 +1162,6 @@ async fn stdio_image_responses_preserve_original_detail_metadata() -> anyhow::Re
let call_id = "img-original-detail-1";
let server_name = "rmcp";
let tool_name = format!("mcp__{server_name}__image_scenario");
let namespace = format!("mcp__{server_name}__");
mount_sse_once(
@@ -1219,7 +1204,7 @@ async fn stdio_image_responses_preserve_original_detail_metadata() -> anyhow::Re
})
.build_remote_aware(&server)
.await?;
wait_for_mcp_tool(&fixture, &tool_name).await?;
wait_for_mcp_server(&fixture, server_name).await?;
fixture
.codex
@@ -1955,7 +1940,6 @@ async fn streamable_http_with_oauth_round_trip_impl() -> anyhow::Result<()> {
let call_id = "call-789";
let server_name = "rmcp_http_oauth";
let tool_name = format!("mcp__{server_name}__echo");
let namespace = format!("mcp__{server_name}__");
mount_sse_once(
@@ -2036,9 +2020,9 @@ async fn streamable_http_with_oauth_round_trip_impl() -> anyhow::Result<()> {
})
.build_remote_aware(&server)
.await?;
// Phase 5: wait for MCP discovery to publish the expected tool before the
// turn is submitted, which keeps failures tied to server startup/discovery.
wait_for_mcp_tool(&fixture, &tool_name).await?;
// Phase 5: wait for MCP startup before the turn is submitted, which keeps
// failures tied to server startup/discovery.
wait_for_mcp_server(&fixture, server_name).await?;
// Phase 6: submit the user turn that should invoke the OAuth-backed tool.
fixture
codex-rs/mcp-server/src/codex_tool_runner.rs
-1
View File
@@ -334,7 +334,6 @@ async fn run_codex_tool_session_inner(
| EventMsg::AgentReasoningSectionBreak(_)
| EventMsg::McpToolCallBegin(_)
| EventMsg::McpToolCallEnd(_)
| EventMsg::McpListToolsResponse(_)
| EventMsg::RealtimeConversationListVoicesResponse(_)
| EventMsg::ExecCommandBegin(_)
| EventMsg::TerminalInteraction(_)
codex-rs/protocol/src/protocol.rs
-23
View File
@@ -30,9 +30,6 @@ use crate::dynamic_tools::DynamicToolSpec;
use crate::items::TurnItem;
use crate::mcp::CallToolResult;
use crate::mcp::RequestId;
use crate::mcp::Resource as McpResource;
use crate::mcp::ResourceTemplate as McpResourceTemplate;
use crate::mcp::Tool as McpTool;
use crate::memory_citation::MemoryCitation;
use crate::models::ActivePermissionProfile;
use crate::models::BaseInstructions;
@@ -722,10 +719,6 @@ pub enum Op {
response: DynamicToolResponse,
},
/// Request the list of MCP tools available across all configured servers.
/// Reply is delivered via `EventMsg::McpListToolsResponse`.
ListMcpTools,
/// Request MCP servers to reinitialize and refresh cached tool lists.
RefreshMcpServers { config: McpServerRefreshConfig },
@@ -862,7 +855,6 @@ impl Op {
Self::UserInputAnswer { .. } => "user_input_answer",
Self::RequestPermissionsResponse { .. } => "request_permissions_response",
Self::DynamicToolResponse { .. } => "dynamic_tool_response",
Self::ListMcpTools => "list_mcp_tools",
Self::RefreshMcpServers { .. } => "refresh_mcp_servers",
Self::ReloadUserConfig => "reload_user_config",
Self::Compact => "compact",
@@ -1406,9 +1398,6 @@ pub enum EventMsg {
TurnDiff(TurnDiffEvent),
/// List of MCP tools available to the agent.
McpListToolsResponse(McpListToolsResponseEvent),
/// List of voices supported by realtime conversation streams.
RealtimeConversationListVoicesResponse(RealtimeConversationListVoicesResponseEvent),
@@ -3247,18 +3236,6 @@ pub struct TurnDiffEvent {
pub unified_diff: String,
}
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
pub struct McpListToolsResponseEvent {
/// Fully qualified tool name -> tool definition.
pub tools: std::collections::HashMap<String, McpTool>,
/// Known resources grouped by server name.
pub resources: std::collections::HashMap<String, Vec<McpResource>>,
/// Known resource templates grouped by server name.
pub resource_templates: std::collections::HashMap<String, Vec<McpResourceTemplate>>,
/// Authentication status for each configured MCP server.
pub auth_statuses: std::collections::HashMap<String, McpAuthStatus>,
}
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema, TS)]
pub struct McpStartupUpdateEvent {
/// Server name being started.
codex-rs/rollout-trace/src/protocol_event.rs
-2
View File
@@ -259,7 +259,6 @@ pub(crate) fn tool_runtime_trace_event(event: &EventMsg) -> Option<ToolRuntimeTr
| EventMsg::StreamError(_)
| EventMsg::PatchApplyUpdated(_)
| EventMsg::TurnDiff(_)
| EventMsg::McpListToolsResponse(_)
| EventMsg::RealtimeConversationListVoicesResponse(_)
| EventMsg::SkillsUpdateAvailable
| EventMsg::PlanUpdate(_)
@@ -333,7 +332,6 @@ pub(crate) fn wrapped_protocol_event_type(event: &EventMsg) -> Option<&'static s
| EventMsg::PatchApplyUpdated(_)
| EventMsg::PatchApplyEnd(_)
| EventMsg::TurnDiff(_)
| EventMsg::McpListToolsResponse(_)
| EventMsg::RealtimeConversationListVoicesResponse(_)
| EventMsg::SkillsUpdateAvailable
| EventMsg::PlanUpdate(_)
codex-rs/rollout/src/policy.rs
-1
View File
@@ -154,7 +154,6 @@ fn event_msg_persistence_mode(ev: &EventMsg) -> Option<EventPersistenceMode> {
| EventMsg::PatchApplyBegin(_)
| EventMsg::PatchApplyUpdated(_)
| EventMsg::TurnDiff(_)
| EventMsg::McpListToolsResponse(_)
| EventMsg::RealtimeConversationListVoicesResponse(_)
| EventMsg::McpStartupUpdate(_)
| EventMsg::McpStartupComplete(_)