From b97abdbe304cbece7e9a6388fbdce40546902997 Mon Sep 17 00:00:00 2001 From: Rennie Date: Wed, 16 Sep 2026 22:09:07 +0000 Subject: [PATCH] Add read-only policy support to MCP tool requests (#46042) ## Why Read-only MCP discovery and invocation need a consistent policy across requests, connection reuse, and tool catalogs. Reusing an unrestricted connection or cached catalog can bypass filtered discovery. ## What changed - Thread `requires_read_only_mcp_tools` through MCP configuration and client setup, leaving it disabled by default. - When enabled, set `openai/readOnly` to `true` in `tools/list` and `tools/call` request metadata, overriding a caller-supplied `false` while preserving other metadata, pagination, and arguments. - Include the policy in connection identity and bypass shared Apps and persistent tool catalog caches for read-only connections. ## Testing Add coverage for metadata propagation in legacy and modern protocol modes, preservation through session recovery, and isolation from unrestricted connections and shared Apps catalogs. GitOrigin-RevId: f4a3a4021a54e302bffd53f08092b080c056246b --- codex-rs/codex-mcp/src/connection_manager.rs | 7 +- .../codex-mcp/src/connection_manager_tests.rs | 97 +++++++++++++++++++ codex-rs/codex-mcp/src/mcp/mod.rs | 2 + codex-rs/codex-mcp/src/mcp/mod_tests.rs | 1 + codex-rs/codex-mcp/src/rmcp_client.rs | 4 +- codex-rs/codex-mcp/src/server.rs | 14 +++ codex-rs/core/src/config/mod.rs | 1 + .../context_snapshot_tests.rs | 2 + .../common/context_snapshot/normalize.rs | 12 ++- .../tests/suite/scenarios_read_only_mcp.rs | 1 + ...read_only_mcp__read_only_mcp_requests.snap | 47 +++++++++ codex-rs/rmcp-client/src/rmcp_client.rs | 36 +++++-- .../rmcp-client/tests/mcp_2026_discovery.rs | 76 +++++++++++++++ codex-rs/rmcp-client/tests/mcp_2026_mrtr.rs | 10 +- codex-rs/tui/src/app/tests.rs | 44 ++++----- 15 files changed, 312 insertions(+), 42 deletions(-) create mode 100644 codex-rs/core/tests/suite/scenarios_read_only_mcp.rs create mode 100644 codex-rs/core/tests/suite/snapshots/all__suite__scenarios__read_only_mcp__read_only_mcp_requests.snap diff --git a/codex-rs/codex-mcp/src/connection_manager.rs b/codex-rs/codex-mcp/src/connection_manager.rs index 6022f1c119..dda2edd914 100644 --- a/codex-rs/codex-mcp/src/connection_manager.rs +++ b/codex-rs/codex-mcp/src/connection_manager.rs @@ -289,6 +289,7 @@ impl McpConnectionSet { .into_iter() .filter(|(_, server)| server.enabled()) { + let server = server.with_read_only_mcp_tools(config.requires_read_only_mcp_tools); let registration = config.mcp_server_catalog.server(&server_name); let client_mcp_extensions = crate::client_capabilities::server_mcp_extensions( &client_mcp_extensions, @@ -349,7 +350,9 @@ impl McpConnectionSet { } => bearer_token_env_var.is_some(), McpServerTransportConfig::Stdio { .. } => false, }; + // Filtered catalogs must not read or populate an unrestricted shared cache. let shares_codex_apps_tools_cache = is_host_owned_codex_apps + && !server.requires_read_only_mcp_tools() && should_share_codex_apps_tools_cache(&server_name, uses_env_bearer_token); let codex_apps_tools_cache_context = shares_codex_apps_tools_cache.then(|| { // Only equivalent discovery inputs may share executable Apps tools. @@ -551,7 +554,9 @@ impl McpConnectionSet { } } let cancel_token = startup_cancellation_token.child_token(); - let tool_catalog_cache_context = if server_name == CODEX_APPS_MCP_SERVER_NAME { + let tool_catalog_cache_context = if server_name == CODEX_APPS_MCP_SERVER_NAME + || server.requires_read_only_mcp_tools() + { None } else if let Ok(environment) = resolved_environment.as_ref() { tool_catalog_cache.context( diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs index 0d10a93e70..aadc5d688c 100644 --- a/codex-rs/codex-mcp/src/connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs @@ -1995,6 +1995,74 @@ fn codex_apps_env_bearer_token_bypasses_shared_tools_cache() { )); } +#[tokio::test] +async fn read_only_apps_discovery_never_uses_a_shared_writable_catalog() -> anyhow::Result<()> { + let codex_home = tempdir()?; + let cache = ConnectorRuntimeManager::new_without_cache(); + let cache_key = ConnectorRuntimeContextKey::personal( + /*account_id*/ None, /*chatgpt_user_id*/ None, + ); + let cache_context = cache.context(codex_home.path().to_path_buf(), cache_key.clone()); + store_current_tools( + &cache_context, + vec![create_test_tool(CODEX_APPS_MCP_SERVER_NAME, "write")], + ); + let server_config: McpServerConfig = serde_json::from_value(serde_json::json!({ + "url": "http://127.0.0.1:1/mcp", + "http_headers": {"authorization": "Bearer fixture"}, + }))?; + for read_only in [false, true] { + let mut config = crate::mcp::tests::test_mcp_config(codex_home.path().to_path_buf()); + config.requires_read_only_mcp_tools = read_only; + let mut catalog = crate::ResolvedMcpCatalog::builder(); + catalog.register(crate::McpServerRegistration::from_hosted_apps( + "fixture", + /*contribution_order*/ 0, + server_config.clone(), + )); + config.mcp_server_catalog = catalog.build(); + let cancellation = CancellationToken::new(); + cancellation.cancel(); + let manager = McpConnectionSet::new( + /*previous*/ None, + McpPublicationGate::already_published(), + McpRuntimeInput { + startup_policy: McpStartupPolicy::Eager, + config: Arc::new(config), + plugins_available: false, + ready_selected_capability_roots: Vec::new(), + mcp_servers: HashMap::from([( + CODEX_APPS_MCP_SERVER_NAME.to_string(), + EffectiveMcpServer::configured(server_config.clone()), + )]), + submit_id: "test".to_string(), + tx_event: None, + startup_cancellation_token: cancellation, + runtime_context: reusable_server_runtime_context(), + codex_apps_tools_cache: cache.clone(), + tool_catalog_cache: McpToolCatalogCache::default(), + codex_apps_tools_cache_key: cache_key.clone(), + client_mcp_extensions: ClientMcpExtensions::default(), + auth: None, + auth_manager: None, + elicitation_reviewer: None, + elicitation_lifecycle: None, + }, + ElicitationRequestRouter::default(), + ) + .await; + let tools = manager.list_all_tools().await; + assert_eq!( + tools + .iter() + .map(|tool| tool.callable_name.as_str()) + .collect::>(), + if read_only { vec![] } else { vec!["write"] }, + ); + } + Ok(()) +} + #[tokio::test] async fn hosted_apps_protocol_mode_is_independent_of_generic_mode() -> anyhow::Result<()> { let codex_home = tempdir()?; @@ -5116,6 +5184,35 @@ async fn reconcile_reusable_server_with_mcp_config( .await } +#[tokio::test] +async fn read_only_policy_does_not_reuse_a_writable_connection() { + let codex_home = tempdir().expect("tempdir"); + let runtime_context = reusable_server_runtime_context(); + let config = reusable_server_config("http://127.0.0.1:1"); + let previous = manager_with_reusable_ready_server( + &config, + &runtime_context, + vec![create_test_tool("docs", "write")], + ) + .await; + for read_only in [false, true] { + let mut mcp_config = crate::mcp::tests::test_mcp_config(codex_home.path().to_path_buf()); + mcp_config.requires_read_only_mcp_tools = read_only; + let reconciled = reconcile_reusable_server_with_mcp_config( + &previous, + "docs", + config.clone(), + runtime_context.clone(), + mcp_config, + ) + .await; + assert_eq!( + previous.shares_test_connection_with(&reconciled, "docs"), + !read_only + ); + } +} + #[tokio::test] async fn apps_catalog_broadcast_preserves_running_calls_and_rejects_stale_calls() -> anyhow::Result<()> { diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index 43d9a43c0b..a65da6e697 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -126,6 +126,8 @@ pub struct McpConfig { pub chatgpt_base_url: String, /// Optional product SKU forwarded to the host-owned apps MCP server. pub apps_mcp_product_sku: Option, + /// Requests server-side read-only filtering and invocation checks for MCP tools. + pub requires_read_only_mcp_tools: bool, /// Codex home directory used for MCP OAuth state and app-tool cache files. pub codex_home: PathBuf, /// Trusted enterprise IdP inherited after normal catalog and policy resolution. diff --git a/codex-rs/codex-mcp/src/mcp/mod_tests.rs b/codex-rs/codex-mcp/src/mcp/mod_tests.rs index e45d6bcacc..b7d0a18af9 100644 --- a/codex-rs/codex-mcp/src/mcp/mod_tests.rs +++ b/codex-rs/codex-mcp/src/mcp/mod_tests.rs @@ -86,6 +86,7 @@ pub(crate) fn test_mcp_config(codex_home: PathBuf) -> McpConfig { McpConfig { chatgpt_base_url: "https://chatgpt.com".to_string(), apps_mcp_product_sku: None, + requires_read_only_mcp_tools: false, codex_home, mcp_enterprise_managed_auth: None, xaa_enabled: false, diff --git a/codex-rs/codex-mcp/src/rmcp_client.rs b/codex-rs/codex-mcp/src/rmcp_client.rs index 06ea54e861..d185c6f578 100644 --- a/codex-rs/codex-mcp/src/rmcp_client.rs +++ b/codex-rs/codex-mcp/src/rmcp_client.rs @@ -367,7 +367,9 @@ impl ManagedClientStartup { ) .await { - Ok(result) => Arc::new(result?), + Ok(result) => Arc::new( + result?.with_read_only_tools(server.requires_read_only_mcp_tools()), + ), Err(_) => { return Err(StartupOutcomeError::from(anyhow!( "MCP client startup timed out after {startup_timeout:?}" diff --git a/codex-rs/codex-mcp/src/server.rs b/codex-rs/codex-mcp/src/server.rs index 1b6c1ecb5b..a7ad0583c5 100644 --- a/codex-rs/codex-mcp/src/server.rs +++ b/codex-rs/codex-mcp/src/server.rs @@ -27,6 +27,7 @@ use tracing::warn; pub struct EffectiveMcpServer { config: McpServerConfig, agent_plugin: bool, + requires_read_only_mcp_tools: bool, } impl EffectiveMcpServer { @@ -34,6 +35,7 @@ impl EffectiveMcpServer { Self { config, agent_plugin: false, + requires_read_only_mcp_tools: false, } } @@ -42,6 +44,15 @@ impl EffectiveMcpServer { self } + pub(crate) fn with_read_only_mcp_tools(mut self, requires_read_only_mcp_tools: bool) -> Self { + self.requires_read_only_mcp_tools = requires_read_only_mcp_tools; + self + } + + pub(crate) fn requires_read_only_mcp_tools(&self) -> bool { + self.requires_read_only_mcp_tools + } + pub fn config(&self) -> &McpServerConfig { &self.config } @@ -112,6 +123,7 @@ pub(crate) struct McpServerConnectionIdentity { client_elicitation_capability: ElicitationCapability, client_mcp_extensions: ClientMcpExtensions, agent_plugin: bool, + requires_read_only_mcp_tools: bool, } impl McpServerConnectionIdentity { @@ -233,6 +245,7 @@ impl McpServerConnectionIdentity { client_elicitation_capability, client_mcp_extensions, agent_plugin: server.is_agent_plugin(), + requires_read_only_mcp_tools: server.requires_read_only_mcp_tools(), } } @@ -265,6 +278,7 @@ impl McpServerConnectionIdentity { && self.client_elicitation_capability == other.client_elicitation_capability && self.client_mcp_extensions == other.client_mcp_extensions && self.agent_plugin == other.agent_plugin + && self.requires_read_only_mcp_tools == other.requires_read_only_mcp_tools } pub(crate) fn oauth_credentials(&self) -> Result, &String> { diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 8cea0f3dc4..2c17d68779 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -1758,6 +1758,7 @@ impl Config { McpConfig { chatgpt_base_url: self.chatgpt_base_url.clone(), apps_mcp_product_sku: self.apps_mcp_product_sku.clone(), + requires_read_only_mcp_tools: false, codex_home: self.codex_home.to_path_buf(), mcp_enterprise_managed_auth: self.mcp_enterprise_managed_auth.clone(), xaa_enabled: self.features.enabled(Feature::UseXaa) diff --git a/codex-rs/core/tests/common/context_snapshot/context_snapshot_tests.rs b/codex-rs/core/tests/common/context_snapshot/context_snapshot_tests.rs index f6ed09a766..c9cfbed1b2 100644 --- a/codex-rs/core/tests/common/context_snapshot/context_snapshot_tests.rs +++ b/codex-rs/core/tests/common/context_snapshot/context_snapshot_tests.rs @@ -59,6 +59,7 @@ fn lite_tool_catalog_and_code_calls_are_visible() { json!({ "type": "custom_tool_call_output", "output": { "content": "plan updated", "success": true } }), + json!({ "type": "function_call_output", "output": "Wall time: 0.0025 seconds\nOutput: Report ready." }), ], &ContextSnapshotOptions::default(), ); @@ -72,6 +73,7 @@ fn lite_tool_catalog_and_code_calls_are_visible() { )); assert!(rendered.contains(" | ready")); assert!(rendered.contains("03:custom_tool_call_output:success=true:plan updated")); + assert!(rendered.contains("04:function_call_output:Wall time: seconds")); } #[test] diff --git a/codex-rs/core/tests/common/context_snapshot/normalize.rs b/codex-rs/core/tests/common/context_snapshot/normalize.rs index 0ce164babd..0f5868e1cf 100644 --- a/codex-rs/core/tests/common/context_snapshot/normalize.rs +++ b/codex-rs/core/tests/common/context_snapshot/normalize.rs @@ -109,15 +109,17 @@ impl Normalizer { } fn normalize_values(&mut self, text: &str) -> String { - // Code Mode reports elapsed wall time in an otherwise stable tool output header. - let text = if text.starts_with("Script completed\nWall time ") { + // Tool calls report elapsed wall time in an otherwise stable output header. + let text = if text.starts_with("Script completed\nWall time ") + || text.starts_with("Wall time: ") + { static WALL_TIME: OnceLock = OnceLock::new(); WALL_TIME .get_or_init(|| { - Regex::new(r"(?m)^Wall time [0-9]+(?:\.[0-9]+)? seconds$") - .expect("code mode wall time regex") + Regex::new(r"(?m)^(Wall time:?) [0-9]+(?:\.[0-9]+)? seconds$") + .expect("tool wall time regex") }) - .replace(text, "Wall time seconds") + .replace(text, "${1} seconds") .into_owned() } else { text.to_string() diff --git a/codex-rs/core/tests/suite/scenarios_read_only_mcp.rs b/codex-rs/core/tests/suite/scenarios_read_only_mcp.rs new file mode 100644 index 0000000000..2cad1c9307 --- /dev/null +++ b/codex-rs/core/tests/suite/scenarios_read_only_mcp.rs @@ -0,0 +1 @@ +//! Thread policy reaches MCP discovery and execution without relying on model arguments. diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__read_only_mcp__read_only_mcp_requests.snap b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__read_only_mcp__read_only_mcp_requests.snap new file mode 100644 index 0000000000..4a6f4c6e4c --- /dev/null +++ b/codex-rs/core/tests/suite/snapshots/all__suite__scenarios__read_only_mcp__read_only_mcp_requests.snap @@ -0,0 +1,47 @@ +--- +source: core/tests/suite/scenarios_read_only_mcp.rs +expression: "context_snapshot::format_request_history_snapshot(\"A read-only thread receives only read tools from the MCP server and executes a read.\",\n&mock.requests(),\n&ContextSnapshotOptions::default().rewrite_known_segments().include_request_settings(),)" +--- +Scenario: A read-only thread receives only read tools from the MCP server and executes a read. + +## Window 1 +Settings: + include: ["reasoning.encrypted_content"] + instructions: + model: "gpt-5.5" + parallel_tool_calls: true + prompt_cache_key: "" + reasoning: {"effort":"medium"} + store: false + stream: true + text: {"verbosity":"low"} + tool_choice: "auto" + tools (11; hash=b74b49b673a090be): + - function/exec_command: Runs a command in a PTY, returning output or a session ID for ongoing interaction.; args=[cmd, justification, login, max_output_tokens, prefix_rule, sandbox_permissions, shell, tty, workdir, yield_time_ms]; hash=c04108e51a8548c5 + - function/write_stdin: Writes characters to an existing unified exec session and returns recent output.; args=[chars, max_output_tokens, session_id, yield_time_ms]; hash=7fb62d8a3e2df4e2 + - function/list_mcp_resources: Lists resources provided by MCP servers. Resources allow servers to share data that provides cont...; args=[cursor, server]; hash=6792edaff13561bd + - function/list_mcp_resource_templates: Lists resource templates provided by MCP servers. Parameterized resource templates allow servers ...; args=[cursor, server]; hash=60f17c90ef7a4ebb + - function/read_mcp_resource: Read a specific resource from an MCP server given the server name and resource URI.; args=[server, uri]; hash=24246896b39008ae + - function/request_user_input: Request user input for one to three short questions and wait for the response. This tool is only ...; args=[questions]; hash=64a729b25c424fc6 + - custom/apply_patch: The `apply_patch` tool can be used to edit files. This is a FREEFORM tool, so do not wrap the pat...; hash=dea9f12b0111a4ba + - function/view_image: View a local image file from the filesystem when visual inspection is needed. Use this for images...; args=[detail, path]; hash=3b766d5bdec5d50e + - namespace/multi_agent_v1: Tools for spawning and managing sub-agents.; hash=f2a2334f757ddd80 + - function/close_agent + - function/resume_agent + - function/send_input + - function/spawn_agent + - function/wait_agent + - namespace/mcp__policy: Tools in the mcp__policy namespace.; hash=4868c429e15f1bec + - function/read + - web_search; hash=e214d4db1bc49e89 +-- request 1 (turn) -- +00:message/developer: + +01:message/user: + +02:message/user: + Read the report. +-- request 2 (turn) -- +03:function_call/mcp__policy.read:{} +04:function_call_output:Wall time: seconds + Output: | Report ready. diff --git a/codex-rs/rmcp-client/src/rmcp_client.rs b/codex-rs/rmcp-client/src/rmcp_client.rs index 004f55d0cf..6d50c63d9c 100644 --- a/codex-rs/rmcp-client/src/rmcp_client.rs +++ b/codex-rs/rmcp-client/src/rmcp_client.rs @@ -107,6 +107,8 @@ use self::streamable_http_retry::HandshakeError; use self::streamable_http_retry::STREAMABLE_HTTP_RETRY_DELAYS_MS; use self::streamable_http_retry::sleep_with_retry_deadline; +const READ_ONLY_TOOLS_META_KEY: &str = "openai/readOnly"; + enum PendingTransport { InProcess { transport: tokio::io::DuplexStream, @@ -396,12 +398,26 @@ pub struct RmcpClient { stdio_process: Option, transport_recipe: TransportRecipe, protocol_mode: McpProtocolMode, + requires_read_only_tools: bool, initialize_context: Mutex>, session_recovery_lock: Semaphore, elicitation_pause_state: ElicitationPauseState, } impl RmcpClient { + /// Requests server-side read-only filtering and invocation checks for every tool request. + pub fn with_read_only_tools(mut self, requires_read_only_tools: bool) -> Self { + self.requires_read_only_tools = requires_read_only_tools; + self + } + + fn apply_read_only_tools_meta(&self, meta: &mut Option) { + if self.requires_read_only_tools { + meta.get_or_insert_default() + .insert(READ_ONLY_TOOLS_META_KEY.to_string(), Value::Bool(true)); + } + } + /// Returns the protocol compatibility policy captured when this client was created. pub fn protocol_mode(&self) -> McpProtocolMode { self.protocol_mode @@ -422,6 +438,7 @@ impl RmcpClient { stdio_process: None, transport_recipe, protocol_mode: McpProtocolMode::Legacy, + requires_read_only_tools: false, initialize_context: Mutex::new(None), session_recovery_lock: Semaphore::new(/*permits*/ 1), elicitation_pause_state: ElicitationPauseState::new(), @@ -496,6 +513,7 @@ impl RmcpClient { transport_recipe, protocol_mode, initialize_context: Mutex::new(None), + requires_read_only_tools: false, session_recovery_lock: Semaphore::new(/*permits*/ 1), elicitation_pause_state: ElicitationPauseState::new(), }) @@ -598,6 +616,7 @@ impl RmcpClient { transport_recipe, protocol_mode, initialize_context: Mutex::new(None), + requires_read_only_tools: false, session_recovery_lock: Semaphore::new(/*permits*/ 1), elicitation_pause_state: ElicitationPauseState::new(), }) @@ -666,10 +685,13 @@ impl RmcpClient { pub async fn list_tools( &self, - params: Option, + mut params: Option, timeout: Option, ) -> Result { self.refresh_oauth_if_needed().await?; + if self.requires_read_only_tools { + self.apply_read_only_tools_meta(&mut params.get_or_insert_default().meta); + } let result = self .run_service_operation("tools/list", timeout, move |service| { let params = params.clone(); @@ -686,13 +708,7 @@ impl RmcpClient { params: Option, timeout: Option, ) -> Result { - self.refresh_oauth_if_needed().await?; - let result = self - .run_service_operation("tools/list", timeout, move |service| { - let params = params.clone(); - async move { service.list_tools(params).await }.boxed() - }) - .await?; + let result = self.list_tools(params, timeout).await?; let tools = result .tools .into_iter() @@ -711,7 +727,6 @@ impl RmcpClient { }) }) .collect::>>()?; - self.persist_oauth_tokens().await; Ok(ListToolsWithConnectorIdResult { next_cursor: result.next_cursor, tools, @@ -823,7 +838,7 @@ impl RmcpClient { } None => None, }; - let meta = match meta { + let mut meta = match meta { Some(Value::Object(map)) => Some(RequestMetaObject::from(map)), Some(other) => { return Err(anyhow!( @@ -832,6 +847,7 @@ impl RmcpClient { } None => None, }; + self.apply_read_only_tools_meta(&mut meta); let mut rmcp_params = CallToolRequestParams::new(name); rmcp_params.arguments = arguments; let requested_modern = self.protocol_mode == McpProtocolMode::V20260728; diff --git a/codex-rs/rmcp-client/tests/mcp_2026_discovery.rs b/codex-rs/rmcp-client/tests/mcp_2026_discovery.rs index 1495438d75..8ec5b12764 100644 --- a/codex-rs/rmcp-client/tests/mcp_2026_discovery.rs +++ b/codex-rs/rmcp-client/tests/mcp_2026_discovery.rs @@ -113,6 +113,82 @@ fn legacy_initialize_response(request: &Value) -> ResponseTemplate { })) } +#[tokio::test] +async fn read_only_tool_requests_preserve_host_policy_and_caller_metadata() -> anyhow::Result<()> { + for mode in [McpProtocolMode::Legacy, McpProtocolMode::V20260728] { + for read_only in [false, true] { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/mcp")) + .respond_with(|request: &Request| { + let body: Value = request.body_json().expect("JSON-RPC request"); + let result = match body["method"].as_str() { + Some("server/discover") => return modern_discover_response(&body), + Some("initialize") => return legacy_initialize_response(&body), + Some("notifications/initialized") => return ResponseTemplate::new(202), + Some("tools/list") => json!({"resultType": "complete", "tools": []}), + Some("tools/call") => json!({"resultType": "complete", "content": []}), + other => panic!("unexpected request: {other:?}"), + }; + ResponseTemplate::new(200).set_body_json(json!({ + "jsonrpc": "2.0", "id": body["id"], "result": result, + })) + }) + .mount(&server) + .await; + let client = create_client(&server, mode) + .await? + .with_read_only_tools(read_only); + initialize_client(&client).await?; + client.list_tools(/*params*/ None, /*timeout*/ None).await?; + let caller_meta = json!({"openai/readOnly": false, "callId": "preserved"}); + let mut page = rmcp::model::PaginatedRequestParams::default() + .with_cursor(Some("next".to_string())); + page.meta = Some(serde_json::from_value(caller_meta.clone())?); + client + .list_tools_with_connector_ids(Some(page), /*timeout*/ None) + .await?; + client + .call_tool( + "write".to_string(), + Some(json!({"message": "hello"})), + Some(caller_meta), + /*timeout*/ None, + ) + .await?; + + let requests = server + .received_requests() + .await + .expect("recorded requests") + .into_iter() + .map(|request| request.body_json::().expect("JSON-RPC request")) + .filter(|body| matches!(body["method"].as_str(), Some("tools/list" | "tools/call"))) + .map(|body| { + let params = &body["params"]; + json!({ + "method": body["method"], + "readOnly": params["_meta"]["openai/readOnly"], + "callId": params["_meta"]["callId"], + "cursor": params["cursor"], + "arguments": params["arguments"], + }) + }) + .collect::>(); + assert_eq!( + requests, + vec![ + json!({"method": "tools/list", "readOnly": read_only.then_some(true), "callId": null, "cursor": null, "arguments": null}), + json!({"method": "tools/list", "readOnly": read_only, "callId": "preserved", "cursor": "next", "arguments": null}), + json!({"method": "tools/call", "readOnly": read_only, "callId": "preserved", "cursor": null, "arguments": {"message": "hello"}}), + ] + ); + client.shutdown().await; + } + } + Ok(()) +} + #[tokio::test] async fn modern_mode_uses_sdk_discovery_and_self_contained_request_metadata() -> anyhow::Result<()> { diff --git a/codex-rs/rmcp-client/tests/mcp_2026_mrtr.rs b/codex-rs/rmcp-client/tests/mcp_2026_mrtr.rs index 5d86e92219..887139ea00 100644 --- a/codex-rs/rmcp-client/tests/mcp_2026_mrtr.rs +++ b/codex-rs/rmcp-client/tests/mcp_2026_mrtr.rs @@ -378,6 +378,10 @@ async fn modern_tool_mrtr_uses_recovered_protocol_after_legacy_session_expiry() body.pointer("/params/_meta/requestContext"), Some(&json!("caller-context")) ); + assert_eq!( + body.pointer("/params/_meta/openai~1readOnly"), + Some(&json!(true)) + ); let attempt = { let mut calls = recorded_calls.lock().expect("requests lock"); calls.push(body.clone()); @@ -449,12 +453,14 @@ async fn modern_tool_mrtr_uses_recovered_protocol_after_legacy_session_expiry() .await; let elicitation_modes = Arc::new(Mutex::new(Vec::new())); - let client = create_client(&server, Arc::clone(&elicitation_modes)).await?; + let client = create_client(&server, Arc::clone(&elicitation_modes)) + .await? + .with_read_only_tools(/*requires_read_only_tools*/ true); let result = client .call_tool( "confirm".into(), Some(json!({})), - Some(json!({"requestContext": "caller-context"})), + Some(json!({"requestContext": "caller-context", "openai/readOnly": false})), Some(Duration::from_secs(5)), ) .await?; diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 715ffbf4a9..40d16d6025 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -7608,10 +7608,10 @@ async fn in_app_resume_session_cwd_without_metadata_is_non_fatal() -> Result<()> #[tokio::test] async fn remote_resume_keeps_server_only_cwd_out_of_local_config() -> Result<()> { - let (mut app, _app_event_rx, _op_rx) = make_test_app_with_channels().await; + // Keep the large setup and resume futures off the Windows test stack. + let (mut app, _app_event_rx, _op_rx) = Box::pin(make_test_app_with_channels()).await; let local_cwd = app.config.cwd.to_path_buf(); - let local_workspace_roots = app - .rebuild_config_for_cwd(local_cwd.clone()) + let local_workspace_roots = Box::pin(app.rebuild_config_for_cwd(local_cwd.clone())) .await? .workspace_roots; let remote_cwd = if cfg!(windows) { @@ -7642,14 +7642,13 @@ async fn remote_resume_keeps_server_only_cwd_out_of_local_config() -> Result<()> }; app.harness_overrides.cwd = Some(remote_cwd.clone()); let mut app_server = Box::pin(crate::start_embedded_app_server_for_picker(&app.config)).await?; - app_server - .resume_thread( - &crate::local_settings::LocalSettings::from(&app.config), - app.config.clone(), - ThreadId::from_string(&thread_id)?, - crate::app_server_session::ResumeModelSettings::RestoreFromThread, - ) - .await?; + Box::pin(app_server.resume_thread( + &crate::local_settings::LocalSettings::from(&app.config), + app.config.clone(), + ThreadId::from_string(&thread_id)?, + crate::app_server_session::ResumeModelSettings::RestoreFromThread, + )) + .await?; let mut tui = crate::tui::test_support::make_test_tui()?; app.runtime_approval_policy_override = Some(RuntimeApprovalPolicyOverride::Restored( AskForApproval::Never, @@ -7662,18 +7661,17 @@ async fn remote_resume_keeps_server_only_cwd_out_of_local_config() -> Result<()> RuntimePermissionProfileOverride::from_restored_config(&prior_config), ); - let control = app - .resume_target_session( - &mut tui, - &mut app_server, - crate::resume_picker::SessionTarget { - path: Some(rollout_path), - thread_id: ThreadId::from_string(&thread_id)?, - cwd: None, - history_mode: None, - }, - ) - .await?; + let control = Box::pin(app.resume_target_session( + &mut tui, + &mut app_server, + crate::resume_picker::SessionTarget { + path: Some(rollout_path), + thread_id: ThreadId::from_string(&thread_id)?, + cwd: None, + history_mode: None, + }, + )) + .await?; assert!(matches!(control, AppRunControl::Continue)); assert_eq!(app.harness_overrides.cwd, Some(remote_cwd));