diff --git a/codex-rs/codex-mcp/src/connection_manager.rs b/codex-rs/codex-mcp/src/connection_manager.rs index 223d4296d5..2999bb9966 100644 --- a/codex-rs/codex-mcp/src/connection_manager.rs +++ b/codex-rs/codex-mcp/src/connection_manager.rs @@ -781,6 +781,16 @@ impl McpConnectionManager { .server_supports_sandbox_state_meta_capability) } + pub async fn server_supports_sandbox_state_meta_v2_capability( + &self, + server: &str, + ) -> Result { + Ok(self + .client_by_name(server) + .await? + .server_supports_sandbox_state_meta_v2_capability) + } + /// List resources from the specified server. pub async fn list_resources( &self, diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs index 41fc19a594..943912b23d 100644 --- a/codex-rs/codex-mcp/src/connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs @@ -120,6 +120,7 @@ async fn create_test_managed_client(tools: Vec) -> ManagedClient { tool_timeout: None, server_instructions: None, server_supports_sandbox_state_meta_capability: false, + server_supports_sandbox_state_meta_v2_capability: false, codex_apps_tools_cache_context: None, } } diff --git a/codex-rs/codex-mcp/src/lib.rs b/codex-rs/codex-mcp/src/lib.rs index 77128d7970..20bfe3d24a 100644 --- a/codex-rs/codex-mcp/src/lib.rs +++ b/codex-rs/codex-mcp/src/lib.rs @@ -10,6 +10,8 @@ pub use resource_client::McpResourceClientCacheKey; pub use resource_client::McpResourcePage; pub use resource_client::McpResourceReadResult; pub use rmcp_client::MCP_SANDBOX_STATE_META_CAPABILITY; +pub use rmcp_client::MCP_SANDBOX_STATE_META_V2_CAPABILITY; +pub use runtime::LegacySandboxState; pub use runtime::McpRuntimeContext; pub use runtime::SandboxState; pub use tools::ToolInfo; diff --git a/codex-rs/codex-mcp/src/rmcp_client.rs b/codex-rs/codex-mcp/src/rmcp_client.rs index c5ae768cee..658820514a 100644 --- a/codex-rs/codex-mcp/src/rmcp_client.rs +++ b/codex-rs/codex-mcp/src/rmcp_client.rs @@ -76,9 +76,10 @@ use tracing::Instrument; use tracing::instrument; use tracing::warn; -/// MCP server capability indicating that Codex should include [`SandboxState`] -/// in tool-call request `_meta` under this key. +/// Legacy MCP server capability for the mixed native-path/URI sandbox state payload. pub const MCP_SANDBOX_STATE_META_CAPABILITY: &str = "codex/sandbox-state-meta"; +/// MCP server capability for the URI-native [`SandboxState`] payload. +pub const MCP_SANDBOX_STATE_META_V2_CAPABILITY: &str = "codex/sandbox-state-meta-v2"; pub const OPENAI_FORM_CAPABILITY: &str = "openai/form"; pub(crate) const MCP_TOOLS_LIST_DURATION_METRIC: &str = "codex.mcp.tools.list.duration_ms"; @@ -108,6 +109,7 @@ pub(crate) struct ManagedClient { pub(crate) tool_timeout: Option, pub(crate) server_instructions: Option, pub(crate) server_supports_sandbox_state_meta_capability: bool, + pub(crate) server_supports_sandbox_state_meta_v2_capability: bool, pub(crate) codex_apps_tools_cache_context: Option, } @@ -847,6 +849,12 @@ async fn start_server_task( .as_ref() .and_then(|exp| exp.get(MCP_SANDBOX_STATE_META_CAPABILITY)) .is_some(); + let server_supports_sandbox_state_meta_v2_capability = initialize_result + .capabilities + .experimental + .as_ref() + .and_then(|exp| exp.get(MCP_SANDBOX_STATE_META_V2_CAPABILITY)) + .is_some(); let list_start = Instant::now(); let fetch_ticket = codex_apps_tools_cache_context .as_ref() @@ -886,6 +894,7 @@ async fn start_server_task( tool_filter, server_instructions: initialize_result.instructions, server_supports_sandbox_state_meta_capability, + server_supports_sandbox_state_meta_v2_capability, codex_apps_tools_cache_context, }; diff --git a/codex-rs/codex-mcp/src/runtime.rs b/codex-rs/codex-mcp/src/runtime.rs index b9b769ffb4..6264e5f379 100644 --- a/codex-rs/codex-mcp/src/runtime.rs +++ b/codex-rs/codex-mcp/src/runtime.rs @@ -5,6 +5,7 @@ //! tiny shared metrics helper. Transport startup and orchestration live in //! [`crate::rmcp_client`] and [`crate::connection_manager`]. +use std::path::Path; use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; @@ -13,21 +14,176 @@ use codex_exec_server::Environment; use codex_exec_server::EnvironmentManager; use codex_exec_server::HttpClient; use codex_exec_server::ReqwestHttpClient; +use codex_protocol::models::ManagedFileSystemPermissions; use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::FileSystemPath; +use codex_protocol::permissions::FileSystemSandboxEntry; +use codex_utils_path_uri::LegacyAppPathString; use codex_utils_path_uri::PathUri; use serde::Deserialize; +use serde::Deserializer; use serde::Serialize; +use serde::de::Error as _; -#[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)] +#[derive(Debug, Clone, Eq, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] pub struct SandboxState { - pub permission_profile: PermissionProfile, - pub codex_linux_sandbox_exe: Option, + pub permission_profile: PermissionProfile, + pub codex_linux_sandbox_exe: Option, pub sandbox_cwd: PathUri, #[serde(default)] pub use_legacy_landlock: bool, } +/// Historical mixed native-path/URI payload for `codex/sandbox-state-meta`. +#[derive(Debug, Clone, Eq, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct LegacySandboxState { + pub permission_profile: PermissionProfile, + pub codex_linux_sandbox_exe: Option, + pub sandbox_cwd: PathUri, + pub use_legacy_landlock: bool, +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct LegacySandboxStateWire { + permission_profile: PermissionProfile, + codex_linux_sandbox_exe: Option, + sandbox_cwd: PathUri, + #[serde(default)] + use_legacy_landlock: bool, +} + +impl LegacySandboxState { + pub fn from_native_paths( + permission_profile: PermissionProfile, + codex_linux_sandbox_exe: Option<&Path>, + sandbox_cwd: PathUri, + use_legacy_landlock: bool, + ) -> anyhow::Result { + let permission_profile = try_map_permission_profile_paths(permission_profile, |path| { + let path = path.as_path().to_str().ok_or_else(|| { + anyhow::anyhow!("legacy sandbox permission path is not valid UTF-8") + })?; + Ok::<_, anyhow::Error>(LegacyAppPathString::from_path(Path::new(path))) + })?; + let codex_linux_sandbox_exe = codex_linux_sandbox_exe + .map(|path| { + let path = path.to_str().ok_or_else(|| { + anyhow::anyhow!("legacy sandbox helper path is not valid UTF-8") + })?; + Ok::<_, anyhow::Error>(LegacyAppPathString::from_path(Path::new(path))) + }) + .transpose()?; + Ok(Self { + permission_profile, + codex_linux_sandbox_exe, + sandbox_cwd, + use_legacy_landlock, + }) + } +} + +impl<'de> Deserialize<'de> for SandboxState { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let wire = LegacySandboxStateWire::deserialize(deserializer)?; + let permission_profile = + try_map_permission_profile_paths(wire.permission_profile, sandbox_wire_path_to_uri) + .map_err(D::Error::custom)?; + let codex_linux_sandbox_exe = wire + .codex_linux_sandbox_exe + .map(|path| sandbox_wire_helper_path_to_uri(path, &wire.sandbox_cwd)) + .transpose() + .map_err(D::Error::custom)?; + Ok(Self { + permission_profile, + codex_linux_sandbox_exe, + sandbox_cwd: wire.sandbox_cwd, + use_legacy_landlock: wire.use_legacy_landlock, + }) + } +} + +fn try_map_permission_profile_paths( + profile: PermissionProfile, + mut map: impl FnMut(InputPath) -> Result, +) -> Result, E> { + Ok(match profile { + PermissionProfile::Managed { + file_system, + network, + } => { + let file_system = match file_system { + ManagedFileSystemPermissions::Restricted { + entries, + glob_scan_max_depth, + } => ManagedFileSystemPermissions::Restricted { + entries: entries + .into_iter() + .map(|entry| { + let path = match entry.path { + FileSystemPath::Path { path } => { + FileSystemPath::Path { path: map(path)? } + } + FileSystemPath::GlobPattern { pattern } => { + FileSystemPath::GlobPattern { pattern } + } + FileSystemPath::Special { value } => { + FileSystemPath::Special { value } + } + }; + Ok(FileSystemSandboxEntry { + path, + access: entry.access, + }) + }) + .collect::, E>>()?, + glob_scan_max_depth, + }, + ManagedFileSystemPermissions::Unrestricted => { + ManagedFileSystemPermissions::Unrestricted + } + }; + PermissionProfile::Managed { + file_system, + network, + } + } + PermissionProfile::Disabled => PermissionProfile::Disabled, + PermissionProfile::External { network } => PermissionProfile::External { network }, + }) +} + +fn sandbox_wire_path_to_uri(path: LegacyAppPathString) -> Result { + if let Ok(uri) = PathUri::parse(path.as_str()) { + return Ok(uri); + } + path.to_inferred_path_uri().ok_or_else(|| { + format!("sandbox path `{path}` is neither a path URI nor an absolute native path") + }) +} + +fn sandbox_wire_helper_path_to_uri( + path: LegacyAppPathString, + sandbox_cwd: &PathUri, +) -> Result { + if let Ok(uri) = PathUri::parse(path.as_str()) { + return Ok(uri); + } + if let Some(uri) = path.to_inferred_path_uri() { + return Ok(uri); + } + sandbox_cwd.join(path.as_str()).map_err(|err| { + format!( + "sandbox helper path `{path}` cannot be resolved against sandbox cwd URI `{sandbox_cwd}`: {err}" + ) + }) +} + /// Runtime context used when resolving per-server MCP environments. /// /// `McpConfig` describes what servers exist. This value carries the canonical diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index 63d21504b9..f28a03f23c 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -35,6 +35,7 @@ use codex_connectors::AppToolPolicyInput; use codex_features::Feature; use codex_hooks::PermissionRequestDecision; use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; +use codex_mcp::LegacySandboxState; use codex_mcp::MCP_TOOL_CODEX_APPS_META_KEY; use codex_mcp::McpConnectionManager; use codex_mcp::McpPermissionPromptAutoApproveContext; @@ -716,42 +717,85 @@ async fn augment_mcp_tool_request_meta_with_sandbox_state( mut meta: Option, ) -> anyhow::Result> { let turn_context = step_context.turn.as_ref(); - let supports_sandbox_state_meta = manager - .server_supports_sandbox_state_meta_capability(server) + let supports_sandbox_state_meta_v2 = manager + .server_supports_sandbox_state_meta_v2_capability(server) .await .unwrap_or(false); - if !supports_sandbox_state_meta { + let sandbox_state_meta_key = if supports_sandbox_state_meta_v2 { + codex_mcp::MCP_SANDBOX_STATE_META_V2_CAPABILITY + } else if manager + .server_supports_sandbox_state_meta_capability(server) + .await + .unwrap_or(false) + { + codex_mcp::MCP_SANDBOX_STATE_META_CAPABILITY + } else { return Ok(meta); - } + }; let server_environment_id = manager .server_environment_id(server) .unwrap_or(codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID); + let server_is_local = server_environment_id == codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID + || step_context + .environments + .turn_environments + .iter() + .find(|environment| environment.environment_id == server_environment_id) + .is_some_and(|environment| !environment.environment.is_remote()); + if !server_is_local && !supports_sandbox_state_meta_v2 { + return Ok(meta); + } let Some(sandbox_cwd) = sandbox_cwd_for_mcp_server(step_context, server_environment_id) else { return Ok(meta); }; - let permission_profile = turn_context.permission_profile(); - let sandbox_state = serde_json::to_value(SandboxState { - permission_profile, - codex_linux_sandbox_exe: step_context.mcp.config().codex_linux_sandbox_exe.clone(), - sandbox_cwd, - use_legacy_landlock: step_context.mcp.config().use_legacy_landlock, - })?; + let permission_profile = if server_is_local { + turn_context.permission_profile() + } else { + turn_context.config.permissions.permission_profile().clone() + }; + let sandbox_state = if supports_sandbox_state_meta_v2 { + let codex_linux_sandbox_exe = if server_is_local { + match step_context.mcp.config().codex_linux_sandbox_exe.as_deref() { + Some(path) if path.is_absolute() => Some(PathUri::from_host_native_path(path)?), + Some(path) => { + let path = path.to_str().ok_or_else(|| { + anyhow::anyhow!("relative sandbox helper path is not valid UTF-8") + })?; + Some(sandbox_cwd.join(path)?) + } + None => None, + } + } else { + None + }; + serde_json::to_value(SandboxState { + permission_profile: permission_profile.into(), + codex_linux_sandbox_exe, + sandbox_cwd, + use_legacy_landlock: step_context.mcp.config().use_legacy_landlock, + })? + } else { + serde_json::to_value(LegacySandboxState::from_native_paths( + permission_profile, + if server_is_local { + step_context.mcp.config().codex_linux_sandbox_exe.as_deref() + } else { + None + }, + sandbox_cwd, + step_context.mcp.config().use_legacy_landlock, + )?)? + }; match meta.as_mut() { Some(serde_json::Value::Object(map)) => { - map.insert( - codex_mcp::MCP_SANDBOX_STATE_META_CAPABILITY.to_string(), - sandbox_state, - ); + map.insert(sandbox_state_meta_key.to_string(), sandbox_state); } Some(_) => {} None => { let mut map = serde_json::Map::new(); - map.insert( - codex_mcp::MCP_SANDBOX_STATE_META_CAPABILITY.to_string(), - sandbox_state, - ); + map.insert(sandbox_state_meta_key.to_string(), sandbox_state); meta = Some(serde_json::Value::Object(map)); } } diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index e5bc6a34b6..60ba28eefe 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -28,6 +28,7 @@ use codex_exec_server::HttpRedirectPolicy; use codex_exec_server::HttpRequestParams; use codex_login::CodexAuth; use codex_mcp::MCP_SANDBOX_STATE_META_CAPABILITY; +use codex_mcp::MCP_SANDBOX_STATE_META_V2_CAPABILITY; use codex_mcp::SandboxState; use codex_models_manager::manager::RefreshStrategy; use codex_utils_path_uri::LegacyAppPathString; @@ -941,12 +942,36 @@ async fn stdio_mcp_tool_call_includes_sandbox_state_meta() -> anyhow::Result<()> let sandbox_meta = meta .get(MCP_SANDBOX_STATE_META_CAPABILITY) .expect("sandbox state metadata should be present"); + assert!(!meta.contains_key(MCP_SANDBOX_STATE_META_V2_CAPABILITY)); + let expected_permission_profile = serde_json::to_value(PermissionProfile::read_only())?; + assert_eq!( + sandbox_meta.get("permissionProfile"), + Some(&expected_permission_profile) + ); + let expected_helper = if is_remote_test_environment() { + Value::Null + } else { + serde_json::to_value(&fixture.config.codex_linux_sandbox_exe)? + }; + assert_eq!( + sandbox_meta.get("codexLinuxSandboxExe"), + Some(&expected_helper) + ); let sandbox_state: SandboxState = serde_json::from_value(sandbox_meta.clone())?; assert_eq!( sandbox_state, SandboxState { - permission_profile: PermissionProfile::read_only(), - codex_linux_sandbox_exe: fixture.config.codex_linux_sandbox_exe.clone(), + permission_profile: PermissionProfile::read_only().into(), + codex_linux_sandbox_exe: if is_remote_test_environment() { + None + } else { + fixture + .config + .codex_linux_sandbox_exe + .as_deref() + .map(PathUri::from_host_native_path) + .transpose()? + }, sandbox_cwd: PathUri::from_abs_path(&fixture.config.cwd), use_legacy_landlock: false, }