diff --git a/codex-rs/tui/src/app/app_server_events.rs b/codex-rs/tui/src/app/app_server_events.rs index ee2c798545..81a9963cbd 100644 --- a/codex-rs/tui/src/app/app_server_events.rs +++ b/codex-rs/tui/src/app/app_server_events.rs @@ -1,4 +1,5 @@ //! App-server event stream handling for the TUI app. +//! Hidden structured threads reject requests instead of entering interactive routing. use super::App; use super::ThreadBufferedEvent; @@ -550,6 +551,23 @@ impl App { app_server_client: &AppServerSession, request: ServerRequest, ) { + let thread_id = server_request_thread_id(&request); + if thread_id + .is_some_and(|thread_id| self.temporary_structured_requests.contains_key(&thread_id)) + { + if let Err(err) = self + .reject_app_server_request( + app_server_client, + request.id().clone(), + "temporary structured threads cannot request tools or user interaction" + .to_string(), + ) + .await + { + tracing::debug!("{err}"); + } + return; + } if let ServerRequest::DynamicToolCall { request_id, params } = &request { if self.dynamic_tool_tasks.contains_key(request_id) || (params.namespace.as_deref() != Some(crate::dynamic_tools::NAMESPACE) @@ -638,7 +656,6 @@ impl App { return; } - let thread_id = server_request_thread_id(&request); let background_voice = self.background_voice.as_ref().is_some_and(|owner| { owner.realtime_conversation_is_running() && owner.thread_id().is_some() diff --git a/codex-rs/tui/src/app/tests/recap_generation_tests.rs b/codex-rs/tui/src/app/tests/recap_generation_tests.rs index bc1f954022..6199df7fd2 100644 --- a/codex-rs/tui/src/app/tests/recap_generation_tests.rs +++ b/codex-rs/tui/src/app/tests/recap_generation_tests.rs @@ -463,11 +463,31 @@ async fn temporary_recap_threads_disable_memories_and_remote_mcp_servers() -> Re .map(|profile| profile.id), mcp_server_names: Vec::new(), }; - crate::temporary_structured_request::start_temporary_thread( + let response = crate::temporary_structured_request::start_temporary_thread( &app_server.request_handle(), options, ) .await?; + let thread_id = ThreadId::from_string(&response.thread.id)?; + let (sender, _receiver) = tokio::sync::mpsc::unbounded_channel(); + app.temporary_structured_requests.insert(thread_id, sender); + // Even if a hidden thread reaches visible routing, it must not prompt. + app.primary_thread_id = Some(thread_id); + app.active_thread_id = Some(thread_id); + for request in [ + exec_approval_request(thread_id, "turn", "item", /*approval_id*/ None), + request_user_input_request(thread_id, "turn", "item"), + ] { + app.handle_app_server_event( + &app_server, + codex_app_server_client::AppServerEvent::ServerRequest(Box::new(request.clone())), + ) + .await; + assert!( + !app.pending_app_server_requests + .contains_server_request(&request) + ); + } let starts = recorded_params(&requests, "thread/start"); assert_eq!(starts.len(), 1); @@ -484,6 +504,24 @@ async fn temporary_recap_threads_disable_memories_and_remote_mcp_servers() -> Re app_server.shutdown().await?; proxy.await??; + assert_eq!( + requests + .lock() + .expect("request recorder lock") + .iter() + .filter(|request| request.method == "server/request/error") + .map(|request| (request.id.clone(), request.params.clone())) + .collect::>(), + [1, 2] + .map(|id| ( + AppServerRequestId::Integer(id), + Some(serde_json::json!({ + "code": -32000, + "message": "temporary structured threads cannot request tools or user interaction" + })) + )) + .to_vec() + ); Ok(()) } diff --git a/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs b/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs index debe018b2a..908d621183 100644 --- a/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs +++ b/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs @@ -762,7 +762,17 @@ pub(super) async fn start_recording_app_server_with_realtime_speech( trace: None, }); } - JSONRPCMessage::Error(_) => {} + JSONRPCMessage::Error(error) => { + request_sink + .lock() + .expect("request recorder lock") + .push(JSONRPCRequest { + id: error.id, + method: "server/request/error".to_string(), + params: Some(serde_json::to_value(error.error)?), + trace: None, + }); + } } } embedded.shutdown().await?; diff --git a/codex-rs/tui/src/temporary_structured_request.rs b/codex-rs/tui/src/temporary_structured_request.rs index 574ab35ef0..8722aaa325 100644 --- a/codex-rs/tui/src/temporary_structured_request.rs +++ b/codex-rs/tui/src/temporary_structured_request.rs @@ -2,7 +2,6 @@ use codex_app_server_client::AppServerRequestHandle; use codex_app_server_client::TypedRequestError; -use codex_app_server_protocol::AskForApproval; use codex_app_server_protocol::ClientRequest; use codex_app_server_protocol::ConfigReadParams; use codex_app_server_protocol::ConfigReadResponse; @@ -48,6 +47,7 @@ pub(crate) struct TemporaryStructuredThreadOptions { /// /// Structured prompts can contain untrusted transcript text, so the effective app-server config is /// read first and every MCP server is explicitly disabled alongside built-in and extension tools. +/// Approval policy is inherited from the server; the TUI rejects unexpected interaction requests. pub(crate) async fn start_temporary_thread( request_handle: &AppServerRequestHandle, options: TemporaryStructuredThreadOptions, @@ -143,7 +143,6 @@ pub(crate) async fn start_temporary_thread( model: Some(model), model_provider: Some(model_provider), cwd: Some(cwd), - approval_policy: Some(AskForApproval::Never), sandbox: custom_permission_profile .is_none() .then_some(SandboxMode::ReadOnly), diff --git a/codex-rs/tui/src/temporary_structured_request_tests.rs b/codex-rs/tui/src/temporary_structured_request_tests.rs index 36915c4bc4..88c24a40b1 100644 --- a/codex-rs/tui/src/temporary_structured_request_tests.rs +++ b/codex-rs/tui/src/temporary_structured_request_tests.rs @@ -5,6 +5,7 @@ use super::start_temporary_thread; use crate::legacy_core::config::ConfigBuilder; use crate::test_support::PathBufExt; use codex_app_server_client::AppServerClient; +use codex_app_server_client::AppServerEvent; use codex_app_server_protocol::AskForApproval; use codex_app_server_protocol::ItemCompletedNotification; use codex_app_server_protocol::SandboxPolicy; @@ -61,7 +62,8 @@ async fn managed_workspace_default_respects_read_only_availability() -> color_ey std::fs::write( &requirements_path, format!( - "default_permissions = \":workspace\"\n\n\ + "allowed_approval_policies = [\"on-request\"]\n\ + default_permissions = \":workspace\"\n\n\ [allowed_permission_profiles]\n\ \":read-only\" = {read_only_allowed}\n\ \":workspace\" = true\n" @@ -103,8 +105,29 @@ async fn managed_workspace_default_respects_read_only_availability() -> color_ey Default::default(), ) .await?; - let app_server = AppServerClient::InProcess(client); + let mut app_server = AppServerClient::InProcess(client); let result = start_temporary_thread(&app_server.request_handle(), options).await; + // Thread/start warnings precede thread/started on the connection. + tokio::time::timeout(std::time::Duration::from_secs(/*secs*/ 5), async { + loop { + let AppServerEvent::ServerNotification(notification) = + app_server.next_event().await.expect("thread startup event") + else { + continue; + }; + match *notification { + ServerNotification::ConfigWarning(warning) => { + assert!( + !warning.summary.contains("`approval_policy`"), + "{warning:?}" + ); + } + ServerNotification::ThreadStarted(_) => break, + _ => {} + } + } + }) + .await?; app_server.shutdown().await?; if read_only_allowed { @@ -117,7 +140,7 @@ async fn managed_workspace_default_respects_read_only_availability() -> color_ey ), ( Some(":read-only".to_string()), - AskForApproval::Never, + AskForApproval::OnRequest, SandboxPolicy::ReadOnly { network_access: false, },