diff --git a/codex-rs/app-server/tests/suite/v2/mcp_tool.rs b/codex-rs/app-server/tests/suite/v2/mcp_tool.rs index 03f3db95f1..141761e88a 100644 --- a/codex-rs/app-server/tests/suite/v2/mcp_tool.rs +++ b/codex-rs/app-server/tests/suite/v2/mcp_tool.rs @@ -13,10 +13,16 @@ use axum::Router; use codex_app_server_protocol::ItemCompletedNotification; use codex_app_server_protocol::JSONRPCError; use codex_app_server_protocol::JSONRPCResponse; +use codex_app_server_protocol::McpElicitationSchema; +use codex_app_server_protocol::McpServerElicitationAction; +use codex_app_server_protocol::McpServerElicitationRequest; +use codex_app_server_protocol::McpServerElicitationRequestParams; +use codex_app_server_protocol::McpServerElicitationRequestResponse; use codex_app_server_protocol::McpServerToolCallParams; use codex_app_server_protocol::McpServerToolCallResponse; use codex_app_server_protocol::McpToolCallStatus; use codex_app_server_protocol::RequestId; +use codex_app_server_protocol::ServerRequest; use codex_app_server_protocol::ThreadItem; use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; @@ -27,12 +33,17 @@ use codex_utils_pty::DEFAULT_OUTPUT_BYTES_CAP; use core_test_support::responses; use pretty_assertions::assert_eq; use rmcp::handler::server::ServerHandler; +use rmcp::model::BooleanSchema; use rmcp::model::CallToolRequestParams; use rmcp::model::CallToolResult; use rmcp::model::Content; +use rmcp::model::CreateElicitationRequestParams; +use rmcp::model::ElicitationAction; +use rmcp::model::ElicitationSchema; use rmcp::model::JsonObject; use rmcp::model::ListToolsResult; use rmcp::model::Meta; +use rmcp::model::PrimitiveSchema; use rmcp::model::ServerCapabilities; use rmcp::model::ServerInfo; use rmcp::model::Tool; @@ -52,6 +63,8 @@ const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(10); const TEST_SERVER_NAME: &str = "tool_server"; const TEST_TOOL_NAME: &str = "echo_tool"; const LARGE_RESPONSE_MESSAGE: &str = "large"; +const ELICITATION_TRIGGER_MESSAGE: &str = "confirm"; +const ELICITATION_MESSAGE: &str = "Allow this request?"; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn mcp_server_tool_call_returns_tool_result() -> Result<()> { @@ -171,6 +184,116 @@ async fn mcp_server_tool_call_returns_error_for_unknown_thread() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn mcp_server_tool_call_round_trips_elicitation() -> Result<()> { + let responses_server = responses::start_mock_server().await; + let (mcp_server_url, mcp_server_handle) = start_mcp_server().await?; + let codex_home = TempDir::new()?; + write_mock_responses_config_toml( + codex_home.path(), + &responses_server.uri(), + &BTreeMap::new(), + /*auto_compact_limit*/ 1024, + /*requires_openai_auth*/ None, + "mock_provider", + "compact", + )?; + + let config_path = codex_home.path().join("config.toml"); + let mut config_toml = std::fs::read_to_string(&config_path)?; + config_toml.push_str(&format!( + r#" +[mcp_servers.{TEST_SERVER_NAME}] +url = "{mcp_server_url}/mcp" +"# + )); + std::fs::write(config_path, config_toml)?; + + let mut mcp = McpProcess::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let thread_start_id = mcp + .send_thread_start_request(ThreadStartParams { + model: Some("mock-model".to_string()), + approval_policy: Some(codex_app_server_protocol::AskForApproval::UnlessTrusted), + ..Default::default() + }) + .await?; + let thread_start_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(thread_start_id)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response(thread_start_resp)?; + + let tool_call_request_id = mcp + .send_mcp_server_tool_call_request(McpServerToolCallParams { + thread_id: thread.id.clone(), + server: TEST_SERVER_NAME.to_string(), + tool: TEST_TOOL_NAME.to_string(), + arguments: Some(json!({ + "message": ELICITATION_TRIGGER_MESSAGE, + })), + meta: None, + }) + .await?; + + let server_req = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_request_message(), + ) + .await??; + let ServerRequest::McpServerElicitationRequest { request_id, params } = server_req else { + panic!("expected McpServerElicitationRequest request, got: {server_req:?}"); + }; + let requested_schema: McpElicitationSchema = serde_json::from_value(serde_json::to_value( + ElicitationSchema::builder() + .required_property("confirmed", PrimitiveSchema::Boolean(BooleanSchema::new())) + .build() + .map_err(anyhow::Error::msg)?, + )?)?; + assert_eq!( + params, + McpServerElicitationRequestParams { + thread_id: thread.id, + turn_id: None, + server_name: TEST_SERVER_NAME.to_string(), + request: McpServerElicitationRequest::Form { + meta: None, + message: ELICITATION_MESSAGE.to_string(), + requested_schema, + }, + } + ); + + mcp.send_response( + request_id, + serde_json::to_value(McpServerElicitationRequestResponse { + action: McpServerElicitationAction::Accept, + content: Some(json!({ + "confirmed": true, + })), + meta: None, + })?, + ) + .await?; + + let tool_call_response: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(tool_call_request_id)), + ) + .await??; + let response: McpServerToolCallResponse = to_response(tool_call_response)?; + assert_eq!(response.content.len(), 1); + assert_eq!(response.content[0].get("type"), Some(&json!("text"))); + assert_eq!(response.content[0].get("text"), Some(&json!("accepted"))); + + mcp_server_handle.abort(); + let _ = mcp_server_handle.await; + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn mcp_tool_call_completion_notification_contains_truncated_large_result() -> Result<()> { let call_id = "call-large-mcp"; @@ -375,6 +498,36 @@ impl ServerHandler for ToolAppsMcpServer { return Ok(result); } + if message == ELICITATION_TRIGGER_MESSAGE { + let requested_schema = ElicitationSchema::builder() + .required_property("confirmed", PrimitiveSchema::Boolean(BooleanSchema::new())) + .build() + .map_err(|err| rmcp::ErrorData::internal_error(err.to_string(), None))?; + let result = context + .peer + .create_elicitation(CreateElicitationRequestParams::FormElicitationParams { + meta: None, + message: ELICITATION_MESSAGE.to_string(), + requested_schema, + }) + .await + .map_err(|err| rmcp::ErrorData::internal_error(err.to_string(), None))?; + let output = match result.action { + ElicitationAction::Accept => { + assert_eq!( + result.content, + Some(json!({ + "confirmed": true, + })) + ); + "accepted" + } + ElicitationAction::Decline => "declined", + ElicitationAction::Cancel => "cancelled", + }; + return Ok(CallToolResult::success(vec![Content::text(output)])); + } + let mut result = CallToolResult::structured(json!({ "echoed": message, "threadId": thread_id, diff --git a/codex-rs/code-mode/src/runtime/globals.rs b/codex-rs/code-mode/src/runtime/globals.rs index b40136c44c..2ec6953f09 100644 --- a/codex-rs/code-mode/src/runtime/globals.rs +++ b/codex-rs/code-mode/src/runtime/globals.rs @@ -12,11 +12,10 @@ use super::callbacks::yield_control_callback; pub(super) fn install_globals(scope: &mut v8::PinScope<'_, '_>) -> Result<(), String> { let global = scope.get_current_context().global(scope); - let console = v8::String::new(scope, "console") - .ok_or_else(|| "failed to allocate global `console`".to_string())?; - if global.delete(scope, console.into()) != Some(true) { - return Err("failed to remove global `console`".to_string()); - } + delete_global(scope, global, "console")?; + delete_global(scope, global, "Atomics")?; + delete_global(scope, global, "SharedArrayBuffer")?; + delete_global(scope, global, "WebAssembly")?; let tools = build_tools_object(scope)?; let all_tools = build_all_tools_value(scope)?; @@ -142,3 +141,17 @@ fn set_global<'s>( Err(format!("failed to set global `{name}`")) } } + +fn delete_global<'s>( + scope: &mut v8::PinScope<'s, '_>, + global: v8::Local<'s, v8::Object>, + name: &str, +) -> Result<(), String> { + let key = v8::String::new(scope, name) + .ok_or_else(|| format!("failed to allocate global `{name}`"))?; + if global.delete(scope, key.into()) == Some(true) { + Ok(()) + } else { + Err(format!("failed to remove global `{name}`")) + } +} diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs index 3fcef0c06b..01b1161b73 100644 --- a/codex-rs/codex-mcp/src/connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs @@ -26,7 +26,6 @@ use pretty_assertions::assert_eq; use rmcp::model::CreateElicitationRequestParams; use rmcp::model::ElicitationAction; use rmcp::model::ElicitationCapability; -use rmcp::model::FormElicitationCapability; use rmcp::model::JsonObject; use rmcp::model::Meta; use rmcp::model::NumberOrString; @@ -801,18 +800,14 @@ async fn list_all_tools_uses_startup_snapshot_when_client_startup_fails() { } #[test] -fn elicitation_capability_enabled_for_custom_servers() { +fn elicitation_capability_uses_2025_06_18_shape_for_all_servers() { for server_name in [CODEX_APPS_MCP_SERVER_NAME, "custom_mcp"] { let capability = elicitation_capability_for_server(server_name); - assert!(matches!( - capability, - Some(ElicitationCapability { - form: Some(FormElicitationCapability { - schema_validation: None - }), - url: None, - }) - )); + assert_eq!(capability, Some(ElicitationCapability::default())); + assert_eq!( + serde_json::to_value(capability).expect("serialize elicitation capability"), + serde_json::json!({}) + ); } } diff --git a/codex-rs/codex-mcp/src/rmcp_client.rs b/codex-rs/codex-mcp/src/rmcp_client.rs index b88942c4e9..38ef96919f 100644 --- a/codex-rs/codex-mcp/src/rmcp_client.rs +++ b/codex-rs/codex-mcp/src/rmcp_client.rs @@ -55,7 +55,6 @@ use futures::future::FutureExt; use futures::future::Shared; use rmcp::model::ClientCapabilities; use rmcp::model::ElicitationCapability; -use rmcp::model::FormElicitationCapability; use rmcp::model::Implementation; use rmcp::model::InitializeRequestParams; use rmcp::model::ProtocolVersion; @@ -323,12 +322,7 @@ pub(crate) fn elicitation_capability_for_server( ) -> Option { // https://modelcontextprotocol.io/specification/2025-06-18/client/elicitation#capabilities // indicates this should be an empty object. - Some(ElicitationCapability { - form: Some(FormElicitationCapability { - schema_validation: None, - }), - url: None, - }) + Some(ElicitationCapability::default()) } pub(crate) async fn list_tools_for_client_uncached( diff --git a/codex-rs/core/src/arc_monitor_tests.rs b/codex-rs/core/src/arc_monitor_tests.rs index 4c2429cf5f..643042ec99 100644 --- a/codex-rs/core/src/arc_monitor_tests.rs +++ b/codex-rs/core/src/arc_monitor_tests.rs @@ -1,6 +1,5 @@ use std::env; use std::ffi::OsStr; -use std::path::PathBuf; use std::sync::Arc; use pretty_assertions::assert_eq; @@ -74,8 +73,7 @@ async fn build_arc_monitor_request_includes_relevant_history_and_null_policies() .record_into_history( &[ContextualUserFragment::into( crate::context::EnvironmentContext::new( - Some(PathBuf::from("/tmp")), - "zsh".to_string(), + Vec::new(), /*current_date*/ None, /*timezone*/ None, /*network*/ None, diff --git a/codex-rs/core/src/context/environment_context.rs b/codex-rs/core/src/context/environment_context.rs index c4e77624f8..1f7313de28 100644 --- a/codex-rs/core/src/context/environment_context.rs +++ b/codex-rs/core/src/context/environment_context.rs @@ -1,21 +1,85 @@ use crate::session::turn_context::TurnContext; -use crate::shell::Shell; use codex_protocol::protocol::TurnContextItem; use codex_protocol::protocol::TurnContextNetworkItem; -use std::path::PathBuf; +use codex_utils_absolute_path::AbsolutePathBuf; use super::ContextualUserFragment; #[derive(Debug, Clone, PartialEq)] pub(crate) struct EnvironmentContext { - pub(crate) cwd: Option, - pub(crate) shell: String, + pub(crate) environments: EnvironmentContextEnvironments, pub(crate) current_date: Option, pub(crate) timezone: Option, pub(crate) network: Option, pub(crate) subagents: Option, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct EnvironmentContextEnvironment { + pub(crate) id: String, + pub(crate) cwd: AbsolutePathBuf, + pub(crate) shell: String, +} + +impl EnvironmentContextEnvironment { + fn legacy(cwd: AbsolutePathBuf, shell: String) -> Self { + Self { + id: String::new(), + cwd, + shell, + } + } + + fn from_turn_environments( + environments: &[crate::session::turn_context::TurnEnvironment], + ) -> Vec { + environments + .iter() + .map(|environment| Self { + id: environment.environment_id.clone(), + cwd: environment.cwd.clone(), + shell: environment.shell.clone(), + }) + .collect() + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum EnvironmentContextEnvironments { + None, + Single(EnvironmentContextEnvironment), + Multiple(Vec), +} + +impl EnvironmentContextEnvironments { + fn from_vec(environments: Vec) -> Self { + let mut environments = environments; + match environments.pop() { + None => Self::None, + Some(environment) if environments.is_empty() => Self::Single(environment), + Some(environment) => { + environments.push(environment); + Self::Multiple(environments) + } + } + } + + fn equals_except_shell(&self, other: &Self) -> bool { + match (self, other) { + (Self::None, Self::None) => true, + (Self::Single(left), Self::Single(right)) => left.cwd == right.cwd, + (Self::Multiple(left), Self::Multiple(right)) => { + left.len() == right.len() + && left + .iter() + .zip(right.iter()) + .all(|(left, right)| left.id == right.id && left.cwd == right.cwd) + } + _ => false, + } + } +} + #[derive(Debug, Clone, PartialEq, Eq, Default)] pub(crate) struct NetworkContext { allowed_domains: Vec, @@ -33,16 +97,30 @@ impl NetworkContext { impl EnvironmentContext { pub(crate) fn new( - cwd: Option, - shell: String, + environments: Vec, current_date: Option, timezone: Option, network: Option, subagents: Option, ) -> Self { Self { - cwd, - shell, + environments: EnvironmentContextEnvironments::from_vec(environments), + current_date, + timezone, + network, + subagents, + } + } + + fn new_with_environments( + environments: EnvironmentContextEnvironments, + current_date: Option, + timezone: Option, + network: Option, + subagents: Option, + ) -> Self { + Self { + environments, current_date, timezone, network, @@ -54,19 +132,11 @@ impl EnvironmentContext { /// comparing turn to turn, since the initial environment_context will /// include the shell, and then it is not configurable from turn to turn. pub(crate) fn equals_except_shell(&self, other: &EnvironmentContext) -> bool { - let EnvironmentContext { - cwd, - current_date, - timezone, - network, - subagents, - shell: _, - } = other; - self.cwd == *cwd - && self.current_date == *current_date - && self.timezone == *timezone - && self.network == *network - && self.subagents == *subagents + self.environments.equals_except_shell(&other.environments) + && self.current_date == other.current_date + && self.timezone == other.timezone + && self.network == other.network + && self.subagents == other.subagents } pub(crate) fn diff_from_turn_context_item( @@ -74,18 +144,29 @@ impl EnvironmentContext { after: &EnvironmentContext, ) -> Self { let before_network = Self::network_from_turn_context_item(before); - let cwd = match &after.cwd { - Some(cwd) if before.cwd.as_path() != cwd.as_path() => Some(cwd.clone()), - _ => None, + let environments = match &after.environments { + EnvironmentContextEnvironments::Single(environment) => { + if before.cwd.as_path() != environment.cwd.as_path() { + EnvironmentContextEnvironments::Single(EnvironmentContextEnvironment::legacy( + environment.cwd.clone(), + environment.shell.clone(), + )) + } else { + EnvironmentContextEnvironments::None + } + } + EnvironmentContextEnvironments::Multiple(environments) => { + EnvironmentContextEnvironments::Multiple(environments.clone()) + } + EnvironmentContextEnvironments::None => EnvironmentContextEnvironments::None, }; let network = if before_network != after.network { after.network.clone() } else { before_network }; - EnvironmentContext::new( - cwd, - after.shell.clone(), + EnvironmentContext::new_with_environments( + environments, after.current_date.clone(), after.timezone.clone(), network, @@ -93,10 +174,9 @@ impl EnvironmentContext { ) } - pub(crate) fn from_turn_context(turn_context: &TurnContext, shell: &Shell) -> Self { + pub(crate) fn from_turn_context(turn_context: &TurnContext) -> Self { Self::new( - Some(turn_context.cwd.to_path_buf()), - shell.name().to_string(), + EnvironmentContextEnvironment::from_turn_environments(&turn_context.environments), turn_context.current_date.clone(), turn_context.timezone.clone(), Self::network_from_turn_context(turn_context), @@ -108,9 +188,12 @@ impl EnvironmentContext { turn_context_item: &TurnContextItem, shell: String, ) -> Self { + let cwd = match AbsolutePathBuf::try_from(turn_context_item.cwd.clone()) { + Ok(cwd) => cwd, + Err(_) => AbsolutePathBuf::resolve_path_against_base(&turn_context_item.cwd, "/"), + }; Self::new( - Some(turn_context_item.cwd.clone()), - shell, + vec![EnvironmentContextEnvironment::legacy(cwd, shell)], turn_context_item.current_date.clone(), turn_context_item.timezone.clone(), Self::network_from_turn_context_item(turn_context_item), @@ -168,11 +251,29 @@ impl ContextualUserFragment for EnvironmentContext { fn body(&self) -> String { let mut lines = Vec::new(); - if let Some(cwd) = &self.cwd { - lines.push(format!(" {}", cwd.to_string_lossy())); + match &self.environments { + EnvironmentContextEnvironments::Single(environment) => { + lines.push(format!( + " {}", + environment.cwd.to_string_lossy() + )); + lines.push(format!(" {}", environment.shell)); + } + EnvironmentContextEnvironments::Multiple(environments) => { + lines.push(" ".to_string()); + for environment in environments { + lines.push(format!(" ", environment.id)); + lines.push(format!( + " {}", + environment.cwd.to_string_lossy() + )); + lines.push(format!(" {}", environment.shell)); + lines.push(" ".to_string()); + } + lines.push(" ".to_string()); + } + EnvironmentContextEnvironments::None => {} } - - lines.push(format!(" {}", self.shell)); if let Some(current_date) = &self.current_date { lines.push(format!(" {current_date}")); } diff --git a/codex-rs/core/src/context/environment_context_tests.rs b/codex-rs/core/src/context/environment_context_tests.rs index 84f8c0d99f..24ff4bbfff 100644 --- a/codex-rs/core/src/context/environment_context_tests.rs +++ b/codex-rs/core/src/context/environment_context_tests.rs @@ -1,6 +1,7 @@ use crate::shell::ShellType; use super::*; +use codex_utils_absolute_path::test_support::PathBufExt; use core_test_support::test_path_buf; use pretty_assertions::assert_eq; use std::path::PathBuf; @@ -14,12 +15,19 @@ fn fake_shell_name() -> String { shell.name().to_string() } +fn test_abs_path(unix_path: &str) -> AbsolutePathBuf { + test_path_buf(unix_path).abs() +} + #[test] fn serialize_workspace_write_environment_context() { let cwd = test_path_buf("/repo"); let context = EnvironmentContext::new( - Some(cwd.clone()), - fake_shell_name(), + vec![EnvironmentContextEnvironment { + id: "local".to_string(), + cwd: cwd.abs(), + shell: fake_shell_name(), + }], Some("2026-02-26".to_string()), Some("America/Los_Angeles".to_string()), /*network*/ None, @@ -46,8 +54,11 @@ fn serialize_environment_context_with_network() { vec!["blocked.example.com".to_string()], ); let context = EnvironmentContext::new( - Some(test_path_buf("/repo")), - fake_shell_name(), + vec![EnvironmentContextEnvironment { + id: "local".to_string(), + cwd: test_path_buf("/repo").abs(), + shell: fake_shell_name(), + }], Some("2026-02-26".to_string()), Some("America/Los_Angeles".to_string()), Some(network), @@ -75,8 +86,7 @@ fn serialize_environment_context_with_network() { #[test] fn serialize_read_only_environment_context() { let context = EnvironmentContext::new( - /*cwd*/ None, - fake_shell_name(), + Vec::new(), Some("2026-02-26".to_string()), Some("America/Los_Angeles".to_string()), /*network*/ None, @@ -84,7 +94,6 @@ fn serialize_read_only_environment_context() { ); let expected = r#" - bash 2026-02-26 America/Los_Angeles "#; @@ -95,16 +104,22 @@ fn serialize_read_only_environment_context() { #[test] fn equals_except_shell_compares_cwd() { let context1 = EnvironmentContext::new( - Some(PathBuf::from("/repo")), - fake_shell_name(), + vec![EnvironmentContextEnvironment { + id: "local".to_string(), + cwd: test_abs_path("/repo"), + shell: fake_shell_name(), + }], /*current_date*/ None, /*timezone*/ None, /*network*/ None, /*subagents*/ None, ); let context2 = EnvironmentContext::new( - Some(PathBuf::from("/repo")), - fake_shell_name(), + vec![EnvironmentContextEnvironment { + id: "local".to_string(), + cwd: test_abs_path("/repo"), + shell: fake_shell_name(), + }], /*current_date*/ None, /*timezone*/ None, /*network*/ None, @@ -116,16 +131,22 @@ fn equals_except_shell_compares_cwd() { #[test] fn equals_except_shell_compares_cwd_differences() { let context1 = EnvironmentContext::new( - Some(PathBuf::from("/repo1")), - fake_shell_name(), + vec![EnvironmentContextEnvironment { + id: "local".to_string(), + cwd: test_abs_path("/repo1"), + shell: fake_shell_name(), + }], /*current_date*/ None, /*timezone*/ None, /*network*/ None, /*subagents*/ None, ); let context2 = EnvironmentContext::new( - Some(PathBuf::from("/repo2")), - fake_shell_name(), + vec![EnvironmentContextEnvironment { + id: "local".to_string(), + cwd: test_abs_path("/repo2"), + shell: fake_shell_name(), + }], /*current_date*/ None, /*timezone*/ None, /*network*/ None, @@ -138,16 +159,22 @@ fn equals_except_shell_compares_cwd_differences() { #[test] fn equals_except_shell_ignores_shell() { let context1 = EnvironmentContext::new( - Some(PathBuf::from("/repo")), - "bash".to_string(), + vec![EnvironmentContextEnvironment { + id: "local".to_string(), + cwd: test_abs_path("/repo"), + shell: "bash".to_string(), + }], /*current_date*/ None, /*timezone*/ None, /*network*/ None, /*subagents*/ None, ); let context2 = EnvironmentContext::new( - Some(PathBuf::from("/repo")), - "zsh".to_string(), + vec![EnvironmentContextEnvironment { + id: "other".to_string(), + cwd: test_abs_path("/repo"), + shell: "zsh".to_string(), + }], /*current_date*/ None, /*timezone*/ None, /*network*/ None, @@ -160,8 +187,11 @@ fn equals_except_shell_ignores_shell() { #[test] fn serialize_environment_context_with_subagents() { let context = EnvironmentContext::new( - Some(test_path_buf("/repo")), - fake_shell_name(), + vec![EnvironmentContextEnvironment { + id: "local".to_string(), + cwd: test_path_buf("/repo").abs(), + shell: fake_shell_name(), + }], Some("2026-02-26".to_string()), Some("America/Los_Angeles".to_string()), /*network*/ None, @@ -184,3 +214,48 @@ fn serialize_environment_context_with_subagents() { assert_eq!(context.render(), expected); } + +#[test] +fn serialize_environment_context_with_multiple_selected_environments() { + let local_cwd = test_path_buf("/repo/local"); + let remote_cwd = test_path_buf("/repo/remote"); + let context = EnvironmentContext::new( + vec![ + EnvironmentContextEnvironment { + id: "local".to_string(), + cwd: local_cwd.abs(), + shell: "bash".to_string(), + }, + EnvironmentContextEnvironment { + id: "remote".to_string(), + cwd: remote_cwd.abs(), + shell: "bash".to_string(), + }, + ], + Some("2026-02-26".to_string()), + Some("America/Los_Angeles".to_string()), + /*network*/ None, + /*subagents*/ None, + ); + + let expected = format!( + r#" + + + {} + bash + + + {} + bash + + + 2026-02-26 + America/Los_Angeles +"#, + local_cwd.display(), + remote_cwd.display() + ); + + assert_eq!(context.render(), expected); +} diff --git a/codex-rs/core/src/context_manager/updates.rs b/codex-rs/core/src/context_manager/updates.rs index 1bc2cb0895..db7850008f 100644 --- a/codex-rs/core/src/context_manager/updates.rs +++ b/codex-rs/core/src/context_manager/updates.rs @@ -29,7 +29,7 @@ fn build_environment_update_item( let prev = previous?; let prev_context = EnvironmentContext::from_turn_context_item(prev, shell.name().to_string()); - let next_context = EnvironmentContext::from_turn_context(next, shell); + let next_context = EnvironmentContext::from_turn_context(next); if prev_context.equals_except_shell(&next_context) { return None; } diff --git a/codex-rs/core/src/environment_selection.rs b/codex-rs/core/src/environment_selection.rs index 06f2dcba01..bbf059069d 100644 --- a/codex-rs/core/src/environment_selection.rs +++ b/codex-rs/core/src/environment_selection.rs @@ -75,6 +75,9 @@ pub(crate) fn resolve_environment_selections( environment_id, environment, cwd: selected_environment.cwd.clone(), + // TODO(starr): Resolve shell metadata per environment instead of + // hardcoding bash. + shell: "bash".to_string(), }); } diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 860b1e0f30..c18976fde1 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -2534,7 +2534,6 @@ impl Session { ) -> Vec { let mut developer_sections = Vec::::with_capacity(8); let mut contextual_user_sections = Vec::::with_capacity(2); - let shell = self.user_shell(); let ( reference_context_item, previous_turn_settings, @@ -2695,7 +2694,7 @@ impl Session { .format_environment_context_subagents(self.conversation_id) .await; contextual_user_sections.push( - crate::context::EnvironmentContext::from_turn_context(turn_context, shell.as_ref()) + crate::context::EnvironmentContext::from_turn_context(turn_context) .with_subagents(subagents) .render(), ); diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 5f791ff887..89633daf35 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -2949,6 +2949,7 @@ fn turn_environments_for_tests( environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), environment: Arc::clone(environment), cwd: cwd.clone(), + shell: "bash".to_string(), }] } @@ -4488,6 +4489,7 @@ async fn primary_environment_uses_first_turn_environment() { environment_id: "second".to_string(), environment: Arc::clone(&first_environment.environment), cwd: second_cwd.clone(), + shell: first_environment.shell.clone(), }); assert_eq!( diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index 622588fbe8..a87d2ab178 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -34,6 +34,7 @@ pub(crate) struct TurnEnvironment { pub(crate) environment_id: String, pub(crate) environment: Arc, pub(crate) cwd: AbsolutePathBuf, + pub(crate) shell: String, } impl TurnEnvironment { diff --git a/codex-rs/core/tests/suite/code_mode.rs b/codex-rs/core/tests/suite/code_mode.rs index af94252c02..3bcb37e7b2 100644 --- a/codex-rs/core/tests/suite/code_mode.rs +++ b/codex-rs/core/tests/suite/code_mode.rs @@ -2371,7 +2371,6 @@ text(JSON.stringify(Object.getOwnPropertyNames(globalThis).sort())); "Array", "ArrayBuffer", "AsyncDisposableStack", - "Atomics", "BigInt", "BigInt64Array", "BigUint64Array", @@ -2406,7 +2405,6 @@ text(JSON.stringify(Object.getOwnPropertyNames(globalThis).sort())); "Reflect", "RegExp", "Set", - "SharedArrayBuffer", "String", "SuppressedError", "Symbol", @@ -2421,7 +2419,6 @@ text(JSON.stringify(Object.getOwnPropertyNames(globalThis).sort())); "WeakMap", "WeakRef", "WeakSet", - "WebAssembly", "__codexContentItems", "add_content", "decodeURI", diff --git a/codex-rs/core/tests/suite/prompt_caching.rs b/codex-rs/core/tests/suite/prompt_caching.rs index 12f4ab76aa..cc8e57f0f8 100644 --- a/codex-rs/core/tests/suite/prompt_caching.rs +++ b/codex-rs/core/tests/suite/prompt_caching.rs @@ -1,8 +1,6 @@ #![allow(clippy::unwrap_used)] use codex_apply_patch::APPLY_PATCH_TOOL_INSTRUCTIONS; -use codex_core::shell::Shell; -use codex_core::shell::default_user_shell; use codex_features::Feature; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; @@ -46,8 +44,7 @@ fn text_user_input_parts(texts: Vec) -> serde_json::Value { }) } -fn assert_default_env_context(text: &str, cwd: &str, shell: &Shell) { - let shell_name = shell.name(); +fn assert_default_env_context(text: &str, cwd: &str) { assert!( text.starts_with(ENVIRONMENT_CONTEXT_OPEN_TAG), "expected environment context fragment: {text}" @@ -57,7 +54,7 @@ fn assert_default_env_context(text: &str, cwd: &str, shell: &Shell) { "expected cwd in environment context: {text}" ); assert!( - text.contains(&format!("{shell_name}")), + text.contains("bash"), "expected shell in environment context: {text}" ); assert!( @@ -365,12 +362,11 @@ async fn prefixes_context_and_instructions_once_and_consistently_across_requests "expected user instructions in UI message: {ui_text}" ); - let shell = default_user_shell(); let cwd_str = config.cwd.to_string_lossy(); let env_text = input1[1]["content"][1]["text"] .as_str() .expect("environment context text"); - assert_default_env_context(env_text, &cwd_str, &shell); + assert_default_env_context(env_text, &cwd_str); assert_eq!( input1[1]["content"][1]["type"].as_str(), Some("input_text"), @@ -785,9 +781,8 @@ async fn per_turn_overrides_keep_cached_prefix_and_key_constant() -> anyhow::Res let env_text = expected_env_msg_2["content"][0]["text"] .as_str() .expect("environment context text"); - let shell = default_user_shell(); let expected_cwd = new_cwd.path().display().to_string(); - assert_default_env_context(env_text, &expected_cwd, &shell); + assert_default_env_context(env_text, &expected_cwd); let mut expected_body2 = body1_input.to_vec(); expected_body2.push(expected_settings_update_msg); expected_body2.push(expected_env_msg_2); @@ -891,13 +886,12 @@ async fn send_user_turn_with_no_changes_does_not_send_environment_context() -> a let expected_permissions_msg = body1["input"][0].clone(); let expected_ui_msg = body1["input"][1].clone(); - let shell = default_user_shell(); let default_cwd_lossy = default_cwd.to_string_lossy(); let expected_env_text_1 = expected_ui_msg["content"][1]["text"] .as_str() .expect("cached environment context text") .to_string(); - assert_default_env_context(&expected_env_text_1, &default_cwd_lossy, &shell); + assert_default_env_context(&expected_env_text_1, &default_cwd_lossy); let expected_contextual_user_msg_1 = text_user_input_parts(vec![ expected_ui_msg["content"][0]["text"] @@ -1023,12 +1017,11 @@ async fn send_user_turn_with_changes_sends_environment_context() -> anyhow::Resu let expected_permissions_msg = body1["input"][0].clone(); let expected_ui_msg = body1["input"][1].clone(); - let shell = default_user_shell(); let expected_env_text_1 = expected_ui_msg["content"][1]["text"] .as_str() .expect("cached environment context text") .to_string(); - assert_default_env_context(&expected_env_text_1, &default_cwd.to_string_lossy(), &shell); + assert_default_env_context(&expected_env_text_1, &default_cwd.to_string_lossy()); let expected_contextual_user_msg_1 = text_user_input_parts(vec![ expected_ui_msg["content"][0]["text"] .as_str() diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index c9621199e3..e315f0d60f 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -2938,6 +2938,12 @@ impl ChatWidget { fn finalize_turn(&mut self) { // Ensure any spinner is replaced by a red ✗ and flushed into history. self.finalize_active_cell_as_failed(); + // Turn-scoped hook rows are transient live state; once the turn is over, + // do not leave an orphaned running row behind if no matching completion + // event arrived before cancellation. + if self.active_hook_cell.take().is_some() { + self.bump_active_cell_revision(); + } // Reset running state and clear streaming buffers. self.user_turn_pending_start = false; self.agent_turn_running = false; diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__interrupted_turn_clears_visible_running_hook.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__interrupted_turn_clears_visible_running_hook.snap new file mode 100644 index 0000000000..22c70e3357 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__interrupted_turn_clears_visible_running_hook.snap @@ -0,0 +1,8 @@ +--- +source: tui/src/chatwidget/tests/status_and_layout.rs +expression: "format!(\"before interrupt:\\n{before_interrupt}after interrupt:\\n{}\",\nactive_hook_blob(&chat))" +--- +before interrupt: +• Running PreToolUse hook: checking command policy +after interrupt: + diff --git a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs index e91bb28851..73f0d3b7ac 100644 --- a/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs +++ b/codex-rs/tui/src/chatwidget/tests/status_and_layout.rs @@ -1331,6 +1331,32 @@ async fn status_line_branch_refreshes_after_interrupt() { assert!(chat.status_line_branch_pending); } +#[tokio::test] +async fn interrupted_turn_clears_visible_running_hook() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + + handle_hook_started( + &mut chat, + hook_started_run( + "pre-tool-use:0:/tmp/hooks.json", + codex_app_server_protocol::HookEventName::PreToolUse, + Some("checking command policy"), + ), + ); + reveal_running_hooks(&mut chat); + let before_interrupt = active_hook_blob(&chat); + + handle_turn_interrupted(&mut chat, "turn-1"); + + assert_chatwidget_snapshot!( + "interrupted_turn_clears_visible_running_hook", + format!( + "before interrupt:\n{before_interrupt}after interrupt:\n{}", + active_hook_blob(&chat) + ) + ); +} + #[tokio::test] async fn status_line_fast_mode_renders_on_and_off() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;