From 5d14c00b6cbcaee299576a126397c392b0f0139c Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Thu, 5 Mar 2026 00:15:51 -0300 Subject: [PATCH 01/25] feat(app-server): add in-process runtime and shared client facade Add a minimal in-process runtime mode in `codex-app-server` and wire typed client request handling through `MessageProcessor` with session-source injection. This introduces startup handshake, bounded channels, typed in-process events, and shutdown behavior for embedded app-server usage. Introduce the new `codex-app-server-client` crate as the shared facade for in-process startup, request/notify APIs, typed request decoding, and server-request resolution. This centralizes client behavior for later TUI and exec migration while keeping transport-backed app-server paths intact. --- codex-rs/Cargo.lock | 18 + codex-rs/Cargo.toml | 2 + codex-rs/app-server-client/Cargo.toml | 30 + codex-rs/app-server-client/README.md | 47 ++ codex-rs/app-server-client/src/lib.rs | 667 ++++++++++++++++++ .../app-server-protocol/src/jsonrpc_lite.rs | 11 + .../src/protocol/common.rs | 9 + codex-rs/app-server/src/app_server_tracing.rs | 55 ++ codex-rs/app-server/src/in_process.rs | 661 +++++++++++++++++ codex-rs/app-server/src/lib.rs | 3 + codex-rs/app-server/src/message_processor.rs | 422 ++++++----- 11 files changed, 1744 insertions(+), 181 deletions(-) create mode 100644 codex-rs/app-server-client/Cargo.toml create mode 100644 codex-rs/app-server-client/README.md create mode 100644 codex-rs/app-server-client/src/lib.rs create mode 100644 codex-rs/app-server/src/in_process.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 7fc34083f235..4b8852d4af1a 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1458,6 +1458,24 @@ dependencies = [ "wiremock", ] +[[package]] +name = "codex-app-server-client" +version = "0.0.0" +dependencies = [ + "codex-app-server", + "codex-app-server-protocol", + "codex-arg0", + "codex-core", + "codex-feedback", + "codex-protocol", + "pretty_assertions", + "serde", + "serde_json", + "tokio", + "toml 0.9.11+spec-1.1.0", + "tracing", +] + [[package]] name = "codex-app-server-protocol" version = "0.0.0" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index cbf7bb6abf28..681487c099e9 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -4,6 +4,7 @@ members = [ "ansi-escape", "async-utils", "app-server", + "app-server-client", "app-server-protocol", "app-server-test-client", "debug-client", @@ -86,6 +87,7 @@ codex-api = { path = "codex-api" } codex-artifacts = { path = "artifacts" } codex-package-manager = { path = "package-manager" } codex-app-server = { path = "app-server" } +codex-app-server-client = { path = "app-server-client" } codex-app-server-protocol = { path = "app-server-protocol" } codex-app-server-test-client = { path = "app-server-test-client" } codex-apply-patch = { path = "apply-patch" } diff --git a/codex-rs/app-server-client/Cargo.toml b/codex-rs/app-server-client/Cargo.toml new file mode 100644 index 000000000000..addde4e5290a --- /dev/null +++ b/codex-rs/app-server-client/Cargo.toml @@ -0,0 +1,30 @@ +[package] +name = "codex-app-server-client" +version.workspace = true +edition.workspace = true +license.workspace = true + +[lib] +name = "codex_app_server_client" +path = "src/lib.rs" + +[lints] +workspace = true + +[dependencies] +codex-app-server = { workspace = true } +codex-app-server-protocol = { workspace = true } +codex-arg0 = { workspace = true } +codex-core = { workspace = true } +codex-feedback = { workspace = true } +codex-protocol = { workspace = true } +serde = { workspace = true } +serde_json = { workspace = true } +tokio = { workspace = true, features = ["sync", "time", "rt"] } +toml = { workspace = true } +tracing = { workspace = true } + +[dev-dependencies] +pretty_assertions = { workspace = true } +serde_json = { workspace = true } +tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } diff --git a/codex-rs/app-server-client/README.md b/codex-rs/app-server-client/README.md new file mode 100644 index 000000000000..ad8e7b93328f --- /dev/null +++ b/codex-rs/app-server-client/README.md @@ -0,0 +1,47 @@ +# codex-app-server-client + +Shared in-process app-server client used by conversational CLI surfaces: + +- `codex-exec` +- `codex-tui` + +## Purpose + +This crate centralizes startup and lifecycle management for an in-process +`codex-app-server` runtime, so CLI clients do not need to duplicate: + +- app-server bootstrap and initialize handshake +- in-memory request/event transport wiring +- session source selection per client surface +- graceful shutdown behavior + +## Client surfaces and session source + +`ClientSurface` controls which `SessionSource` is used when starting +app-server: + +- `ClientSurface::Exec` -> `SessionSource::Exec` +- `ClientSurface::Tui` -> `SessionSource::Cli` + +This ensures thread metadata (for example in `thread/list` and `thread/read`) +matches the originating runtime. + +## Transport model + +The in-process path uses typed channels: + +- client -> server: `ClientRequest` / `ClientNotification` +- server -> client: `InProcessServerEvent` + - `ServerRequest` + - `ServerNotification` + - `LegacyNotification` + +JSON serialization is still used at external transport boundaries +(stdio/websocket), but the in-process hot path is typed. + +## Backpressure and shutdown + +- Queues are bounded and use `DEFAULT_IN_PROCESS_CHANNEL_CAPACITY` by default. +- Full queues return explicit overload behavior instead of unbounded growth. +- `shutdown()` performs a bounded graceful shutdown and then aborts if timeout + is exceeded. diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs new file mode 100644 index 000000000000..3152710e8563 --- /dev/null +++ b/codex-rs/app-server-client/src/lib.rs @@ -0,0 +1,667 @@ +//! Shared in-process app-server client facade for CLI surfaces. +//! +//! This crate wraps `codex_app_server::in_process` behind a single async API +//! used by surfaces like TUI and exec. It centralizes: +//! - runtime startup and initialize capabilities +//! - surface-to-session-source policy +//! - request/notification dispatch +//! - server request resolution and rejection +//! - event consumption and shutdown behavior +//! +//! The facade uses bounded channels between caller and worker task so overload +//! is visible instead of unbounded memory growth. + +use std::error::Error; +use std::fmt; +use std::io::Error as IoError; +use std::io::ErrorKind; +use std::io::Result as IoResult; +use std::sync::Arc; +use std::time::Duration; + +pub use codex_app_server::in_process::DEFAULT_IN_PROCESS_CHANNEL_CAPACITY; +pub use codex_app_server::in_process::InProcessServerEvent; +use codex_app_server::in_process::InProcessStartArgs; +use codex_app_server_protocol::ClientInfo; +use codex_app_server_protocol::ClientNotification; +use codex_app_server_protocol::ClientRequest; +use codex_app_server_protocol::ConfigWarningNotification; +use codex_app_server_protocol::InitializeCapabilities; +use codex_app_server_protocol::InitializeParams; +use codex_app_server_protocol::JSONRPCErrorError; +use codex_app_server_protocol::RequestId; +use codex_app_server_protocol::Result as JsonRpcResult; +use codex_arg0::Arg0DispatchPaths; +use codex_core::config::Config; +use codex_core::config_loader::CloudRequirementsLoader; +use codex_core::config_loader::LoaderOverrides; +use codex_feedback::CodexFeedback; +use codex_protocol::protocol::SessionSource; +use serde::de::DeserializeOwned; +use tokio::sync::mpsc; +use tokio::sync::oneshot; +use tokio::time::timeout; +use toml::Value as TomlValue; +use tracing::warn; + +const SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(5); + +pub type RequestResult = std::result::Result; + +/// Layered error for [`InProcessAppServerClient::request_typed`]. +/// +/// This keeps transport failures, server-side JSON-RPC failures, and response +/// shape mismatches distinct so callers can decide whether to retry, surface a +/// server error, or treat the response as protocol drift. +#[derive(Debug)] +pub enum TypedRequestError { + Transport { + method: String, + source: IoError, + }, + Server { + method: String, + source: JSONRPCErrorError, + }, + Deserialize { + method: String, + source: serde_json::Error, + }, +} + +impl fmt::Display for TypedRequestError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Transport { method, source } => { + write!(f, "{method} transport error: {source}") + } + Self::Server { method, source } => { + write!(f, "{method} failed: {}", source.message) + } + Self::Deserialize { method, source } => { + write!(f, "{method} invalid response payload: {source}") + } + } + } +} + +impl Error for TypedRequestError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + match self { + Self::Transport { source, .. } => Some(source), + Self::Server { source, .. } => Some(source), + Self::Deserialize { source, .. } => Some(source), + } + } +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum ClientSurface { + /// Non-interactive execution surface. + Exec, + /// Interactive terminal UI surface. + Tui, +} + +/// Maps facade surface identity to app-server `SessionSource`. +pub fn session_source_for_surface(surface: ClientSurface) -> SessionSource { + match surface { + ClientSurface::Exec => SessionSource::Exec, + ClientSurface::Tui => SessionSource::Cli, + } +} + +impl ClientSurface { + fn default_client_name(self) -> &'static str { + match self { + ClientSurface::Exec => "codex-exec", + ClientSurface::Tui => "codex-tui", + } + } +} + +#[derive(Clone)] +pub struct InProcessClientStartArgs { + /// Resolved argv0 dispatch paths used by command execution internals. + pub arg0_paths: Arg0DispatchPaths, + /// Shared config used to initialize app-server runtime. + pub config: Arc, + /// CLI config overrides that are already parsed into TOML values. + pub cli_overrides: Vec<(String, TomlValue)>, + /// Loader override knobs used by config API paths. + pub loader_overrides: LoaderOverrides, + /// Preloaded cloud requirements provider. + pub cloud_requirements: CloudRequirementsLoader, + /// Feedback sink used by app-server/core telemetry and logs. + pub feedback: CodexFeedback, + /// Startup warnings emitted after initialize succeeds. + pub config_warnings: Vec, + /// Surface identity that drives session source and default client name. + pub surface: ClientSurface, + /// Optional explicit client name; falls back to surface default. + pub client_name: Option, + /// Client version reported during initialize. + pub client_version: String, + /// Whether experimental APIs are requested at initialize time. + pub experimental_api: bool, + /// Notification methods this client opts out of receiving. + pub opt_out_notification_methods: Vec, + /// Queue capacity for command/event channels (clamped to at least 1). + pub channel_capacity: usize, +} + +impl InProcessClientStartArgs { + /// Builds initialize params from surface and caller-provided metadata. + pub fn initialize_params(&self) -> InitializeParams { + let client_name = self + .client_name + .clone() + .unwrap_or_else(|| self.surface.default_client_name().to_string()); + let capabilities = InitializeCapabilities { + experimental_api: self.experimental_api, + opt_out_notification_methods: if self.opt_out_notification_methods.is_empty() { + None + } else { + Some(self.opt_out_notification_methods.clone()) + }, + }; + + InitializeParams { + client_info: ClientInfo { + name: client_name, + title: None, + version: self.client_version.clone(), + }, + capabilities: Some(capabilities), + } + } + + fn into_runtime_start_args(self) -> InProcessStartArgs { + let initialize = self.initialize_params(); + InProcessStartArgs { + arg0_paths: self.arg0_paths, + config: self.config, + cli_overrides: self.cli_overrides, + loader_overrides: self.loader_overrides, + cloud_requirements: self.cloud_requirements, + feedback: self.feedback, + config_warnings: self.config_warnings, + session_source: session_source_for_surface(self.surface), + initialize, + channel_capacity: self.channel_capacity, + } + } +} + +enum ClientCommand { + Request { + request: Box, + response_tx: oneshot::Sender>, + }, + Notify { + notification: ClientNotification, + response_tx: oneshot::Sender>, + }, + ResolveServerRequest { + request_id: RequestId, + result: JsonRpcResult, + response_tx: oneshot::Sender>, + }, + RejectServerRequest { + request_id: RequestId, + error: JSONRPCErrorError, + response_tx: oneshot::Sender>, + }, + Shutdown { + response_tx: oneshot::Sender>, + }, +} + +pub struct InProcessAppServerClient { + command_tx: mpsc::Sender, + event_rx: mpsc::Receiver, + worker_handle: tokio::task::JoinHandle<()>, +} + +impl InProcessAppServerClient { + /// Starts the in-process runtime and facade worker task. + /// + /// The returned client is ready for requests and event consumption. If the + /// internal event queue is saturated later, server requests are rejected + /// with overload error instead of being silently dropped. + pub async fn start(args: InProcessClientStartArgs) -> IoResult { + let channel_capacity = args.channel_capacity.max(1); + let mut handle = + codex_app_server::in_process::start(args.into_runtime_start_args()).await?; + let (command_tx, mut command_rx) = mpsc::channel::(channel_capacity); + let (event_tx, event_rx) = mpsc::channel::(channel_capacity); + + let worker_handle = tokio::spawn(async move { + let mut event_stream_enabled = true; + let mut skipped_events = 0usize; + loop { + tokio::select! { + command = command_rx.recv() => { + match command { + Some(ClientCommand::Request { request, response_tx }) => { + let result = handle.request(*request).await; + let _ = response_tx.send(result); + } + Some(ClientCommand::Notify { + notification, + response_tx, + }) => { + let result = handle.notify(notification); + let _ = response_tx.send(result); + } + Some(ClientCommand::ResolveServerRequest { + request_id, + result, + response_tx, + }) => { + let send_result = handle.respond_to_server_request(request_id, result); + let _ = response_tx.send(send_result); + } + Some(ClientCommand::RejectServerRequest { + request_id, + error, + response_tx, + }) => { + let send_result = handle.fail_server_request(request_id, error); + let _ = response_tx.send(send_result); + } + Some(ClientCommand::Shutdown { response_tx }) => { + let shutdown_result = handle.shutdown().await; + let _ = response_tx.send(shutdown_result); + break; + } + None => { + let _ = handle.shutdown().await; + break; + } + } + } + event = handle.next_event(), if event_stream_enabled => { + let Some(event) = event else { + break; + }; + + if skipped_events > 0 { + match event_tx.try_send(InProcessServerEvent::Lagged { + skipped: skipped_events, + }) { + Ok(()) => { + skipped_events = 0; + } + Err(mpsc::error::TrySendError::Full(_)) => { + skipped_events = skipped_events.saturating_add(1); + warn!( + "dropping in-process app-server event because consumer queue is full" + ); + if let InProcessServerEvent::ServerRequest(request) = event { + let _ = handle.fail_server_request( + request.id().clone(), + JSONRPCErrorError { + code: -32001, + message: "in-process app-server event queue is full".to_string(), + data: None, + }, + ); + } + continue; + } + Err(mpsc::error::TrySendError::Closed(_)) => { + event_stream_enabled = false; + continue; + } + } + } + + match event_tx.try_send(event) { + Ok(()) => {} + Err(mpsc::error::TrySendError::Full(event)) => { + skipped_events = skipped_events.saturating_add(1); + warn!("dropping in-process app-server event because consumer queue is full"); + if let InProcessServerEvent::ServerRequest(request) = event { + let _ = handle.fail_server_request( + request.id().clone(), + JSONRPCErrorError { + code: -32001, + message: "in-process app-server event queue is full".to_string(), + data: None, + }, + ); + } + } + Err(mpsc::error::TrySendError::Closed(_)) => { + event_stream_enabled = false; + } + } + } + } + } + }); + + Ok(Self { + command_tx, + event_rx, + worker_handle, + }) + } + + /// Sends a typed client request and returns raw JSON-RPC result. + /// + /// Callers that expect a concrete response type should usually prefer + /// [`request_typed`](Self::request_typed). + pub async fn request(&self, request: ClientRequest) -> IoResult { + let (response_tx, response_rx) = oneshot::channel(); + self.command_tx + .send(ClientCommand::Request { + request: Box::new(request), + response_tx, + }) + .await + .map_err(|_| { + IoError::new( + ErrorKind::BrokenPipe, + "in-process app-server worker channel is closed", + ) + })?; + response_rx.await.map_err(|_| { + IoError::new( + ErrorKind::BrokenPipe, + "in-process app-server request channel is closed", + ) + })? + } + + /// Sends a typed client request and decodes the successful response body. + /// + pub async fn request_typed(&self, request: ClientRequest) -> Result + where + T: DeserializeOwned, + { + let method = request_method_name(&request); + let response = + self.request(request) + .await + .map_err(|source| TypedRequestError::Transport { + method: method.clone(), + source, + })?; + let result = response.map_err(|source| TypedRequestError::Server { + method: method.clone(), + source, + })?; + serde_json::from_value(result) + .map_err(|source| TypedRequestError::Deserialize { method, source }) + } + + /// Sends a typed client notification. + pub async fn notify(&self, notification: ClientNotification) -> IoResult<()> { + let (response_tx, response_rx) = oneshot::channel(); + self.command_tx + .send(ClientCommand::Notify { + notification, + response_tx, + }) + .await + .map_err(|_| { + IoError::new( + ErrorKind::BrokenPipe, + "in-process app-server worker channel is closed", + ) + })?; + response_rx.await.map_err(|_| { + IoError::new( + ErrorKind::BrokenPipe, + "in-process app-server notify channel is closed", + ) + })? + } + + /// Resolves a pending server request. + /// + /// This should only be called with request IDs obtained from the current + /// client's event stream. + pub async fn resolve_server_request( + &self, + request_id: RequestId, + result: JsonRpcResult, + ) -> IoResult<()> { + let (response_tx, response_rx) = oneshot::channel(); + self.command_tx + .send(ClientCommand::ResolveServerRequest { + request_id, + result, + response_tx, + }) + .await + .map_err(|_| { + IoError::new( + ErrorKind::BrokenPipe, + "in-process app-server worker channel is closed", + ) + })?; + response_rx.await.map_err(|_| { + IoError::new( + ErrorKind::BrokenPipe, + "in-process app-server resolve channel is closed", + ) + })? + } + + /// Rejects a pending server request with JSON-RPC error payload. + pub async fn reject_server_request( + &self, + request_id: RequestId, + error: JSONRPCErrorError, + ) -> IoResult<()> { + let (response_tx, response_rx) = oneshot::channel(); + self.command_tx + .send(ClientCommand::RejectServerRequest { + request_id, + error, + response_tx, + }) + .await + .map_err(|_| { + IoError::new( + ErrorKind::BrokenPipe, + "in-process app-server worker channel is closed", + ) + })?; + response_rx.await.map_err(|_| { + IoError::new( + ErrorKind::BrokenPipe, + "in-process app-server reject channel is closed", + ) + })? + } + + /// Returns the next in-process event, or `None` when worker exits. + pub async fn next_event(&mut self) -> Option { + self.event_rx.recv().await + } + + /// Shuts down worker and in-process runtime with bounded wait. + /// + /// If graceful shutdown exceeds timeout, the worker task is aborted to + /// avoid leaking background tasks in embedding callers. + pub async fn shutdown(self) -> IoResult<()> { + let mut worker_handle = self.worker_handle; + let (response_tx, response_rx) = oneshot::channel(); + if self + .command_tx + .send(ClientCommand::Shutdown { response_tx }) + .await + .is_ok() + && let Ok(command_result) = timeout(SHUTDOWN_TIMEOUT, response_rx).await + { + command_result.map_err(|_| { + IoError::new( + ErrorKind::BrokenPipe, + "in-process app-server shutdown channel is closed", + ) + })??; + } + + if let Err(_elapsed) = timeout(SHUTDOWN_TIMEOUT, &mut worker_handle).await { + worker_handle.abort(); + let _ = worker_handle.await; + } + Ok(()) + } +} + +fn request_method_name(request: &ClientRequest) -> String { + serde_json::to_value(request) + .ok() + .and_then(|value| { + value + .get("method") + .and_then(serde_json::Value::as_str) + .map(ToOwned::to_owned) + }) + .unwrap_or_else(|| "".to_string()) +} + +#[cfg(test)] +mod tests { + use super::*; + use codex_app_server_protocol::ConfigRequirementsReadResponse; + use codex_app_server_protocol::SessionSource as ApiSessionSource; + use codex_app_server_protocol::ThreadStartParams; + use codex_app_server_protocol::ThreadStartResponse; + use codex_core::config::ConfigBuilder; + use pretty_assertions::assert_eq; + + async fn build_test_config() -> Config { + match ConfigBuilder::default().build().await { + Ok(config) => config, + Err(_) => Config::load_default_with_cli_overrides(Vec::new()) + .expect("default config should load"), + } + } + + async fn start_test_client_with_capacity( + surface: ClientSurface, + channel_capacity: usize, + ) -> InProcessAppServerClient { + InProcessAppServerClient::start(InProcessClientStartArgs { + arg0_paths: Arg0DispatchPaths::default(), + config: Arc::new(build_test_config().await), + cli_overrides: Vec::new(), + loader_overrides: LoaderOverrides::default(), + cloud_requirements: CloudRequirementsLoader::default(), + feedback: CodexFeedback::new(), + config_warnings: Vec::new(), + surface, + client_name: Some("codex-app-server-client-test".to_string()), + client_version: "0.0.0-test".to_string(), + experimental_api: true, + opt_out_notification_methods: Vec::new(), + channel_capacity, + }) + .await + .expect("in-process app-server client should start") + } + + async fn start_test_client(surface: ClientSurface) -> InProcessAppServerClient { + start_test_client_with_capacity(surface, DEFAULT_IN_PROCESS_CHANNEL_CAPACITY).await + } + + #[tokio::test] + async fn typed_request_roundtrip_works() { + let client = start_test_client(ClientSurface::Exec).await; + let _response: ConfigRequirementsReadResponse = client + .request_typed(ClientRequest::ConfigRequirementsRead { + request_id: RequestId::Integer(1), + params: None, + }) + .await + .expect("typed request should succeed"); + client.shutdown().await.expect("shutdown should complete"); + } + + #[tokio::test] + async fn typed_request_reports_json_rpc_errors() { + let client = start_test_client(ClientSurface::Exec).await; + let err = client + .request_typed::(ClientRequest::ThreadRead { + request_id: RequestId::Integer(99), + params: codex_app_server_protocol::ThreadReadParams { + thread_id: "missing-thread".to_string(), + include_turns: false, + }, + }) + .await + .expect_err("missing thread should return a JSON-RPC error"); + assert!( + err.to_string().starts_with("thread/read failed:"), + "expected method-qualified JSON-RPC failure message" + ); + client.shutdown().await.expect("shutdown should complete"); + } + + #[tokio::test] + async fn surface_to_session_source_mapping_is_applied() { + for (surface, expected_source) in [ + (ClientSurface::Exec, ApiSessionSource::Exec), + (ClientSurface::Tui, ApiSessionSource::Cli), + ] { + let client = start_test_client(surface).await; + let parsed: ThreadStartResponse = client + .request_typed(ClientRequest::ThreadStart { + request_id: RequestId::Integer(2), + params: ThreadStartParams { + ephemeral: Some(true), + ..ThreadStartParams::default() + }, + }) + .await + .expect("thread/start should succeed"); + assert_eq!(parsed.thread.source, expected_source); + client.shutdown().await.expect("shutdown should complete"); + } + } + + #[tokio::test] + async fn tiny_channel_capacity_still_supports_request_roundtrip() { + let client = start_test_client_with_capacity(ClientSurface::Exec, 1).await; + let _response: ConfigRequirementsReadResponse = client + .request_typed(ClientRequest::ConfigRequirementsRead { + request_id: RequestId::Integer(1), + params: None, + }) + .await + .expect("typed request should succeed"); + client.shutdown().await.expect("shutdown should complete"); + } + + #[test] + fn typed_request_error_exposes_sources() { + let transport = TypedRequestError::Transport { + method: "config/read".to_string(), + source: IoError::new(ErrorKind::BrokenPipe, "closed"), + }; + assert_eq!(std::error::Error::source(&transport).is_some(), true); + + let server = TypedRequestError::Server { + method: "thread/read".to_string(), + source: JSONRPCErrorError { + code: -32603, + data: None, + message: "internal".to_string(), + }, + }; + assert_eq!(std::error::Error::source(&server).is_some(), true); + + let deserialize = TypedRequestError::Deserialize { + method: "thread/start".to_string(), + source: serde_json::from_str::("\"nope\"") + .expect_err("invalid integer should return deserialize error"), + }; + assert_eq!(std::error::Error::source(&deserialize).is_some(), true); + } +} diff --git a/codex-rs/app-server-protocol/src/jsonrpc_lite.rs b/codex-rs/app-server-protocol/src/jsonrpc_lite.rs index 13d3e0cc9ac5..7d4ba63fbaf3 100644 --- a/codex-rs/app-server-protocol/src/jsonrpc_lite.rs +++ b/codex-rs/app-server-protocol/src/jsonrpc_lite.rs @@ -1,6 +1,9 @@ //! We do not do true JSON-RPC 2.0, as we neither send nor expect the //! "jsonrpc": "2.0" field. +use std::error::Error; +use std::fmt; + use codex_protocol::protocol::W3cTraceContext; use schemars::JsonSchema; use serde::Deserialize; @@ -76,3 +79,11 @@ pub struct JSONRPCErrorError { pub data: Option, pub message: String, } + +impl fmt::Display for JSONRPCErrorError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{} (code {})", self.message, self.code) + } +} + +impl Error for JSONRPCErrorError {} diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index 54c0425256c2..8d6a27732308 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -110,6 +110,15 @@ macro_rules! client_request_definitions { )* } + impl ClientRequest { + /// Returns the request id regardless of method variant. + pub fn id(&self) -> &RequestId { + match self { + $(Self::$variant { request_id, .. } => request_id,)* + } + } + } + impl crate::experimental_api::ExperimentalApi for ClientRequest { fn experimental_reason(&self) -> Option<&'static str> { match self { diff --git a/codex-rs/app-server/src/app_server_tracing.rs b/codex-rs/app-server/src/app_server_tracing.rs index 779cef96717e..4a2de97a4a11 100644 --- a/codex-rs/app-server/src/app_server_tracing.rs +++ b/codex-rs/app-server/src/app_server_tracing.rs @@ -1,6 +1,7 @@ use crate::message_processor::ConnectionSessionState; use crate::outgoing_message::ConnectionId; use crate::transport::AppServerTransport; +use codex_app_server_protocol::ClientRequest; use codex_app_server_protocol::InitializeParams; use codex_app_server_protocol::JSONRPCRequest; use codex_otel::set_parent_from_context; @@ -65,6 +66,50 @@ pub(crate) fn request_span( span } +/// Builds tracing span metadata for typed in-process requests. +/// +/// This mirrors `request_span` semantics while stamping transport as +/// `in-process` and deriving client info either from initialize params or +/// from existing connection session state. +pub(crate) fn typed_request_span( + request: &ClientRequest, + connection_id: ConnectionId, + session: &ConnectionSessionState, +) -> Span { + let span = info_span!( + "app_server.request", + otel.kind = "server", + otel.name = "in_process", + rpc.system = "jsonrpc", + rpc.method = "in_process", + rpc.transport = "in-process", + rpc.request_id = ?request.id(), + app_server.connection_id = ?connection_id, + app_server.api_version = "v2", + app_server.client_name = field::Empty, + app_server.client_version = field::Empty, + ); + + if let Some((client_name, client_version)) = initialize_client_info_from_typed_request(request) + { + span.record("app_server.client_name", client_name); + span.record("app_server.client_version", client_version); + } else { + if let Some(client_name) = session.app_server_client_name.as_deref() { + span.record("app_server.client_name", client_name); + } + if let Some(client_version) = session.client_version.as_deref() { + span.record("app_server.client_version", client_version); + } + } + + if let Some(context) = traceparent_context_from_env() { + set_parent_from_context(&span, context); + } + + span +} + fn transport_name(transport: AppServerTransport) -> &'static str { match transport { AppServerTransport::Stdio => "stdio", @@ -99,3 +144,13 @@ fn initialize_client_info(request: &JSONRPCRequest) -> Option let params = request.params.clone()?; serde_json::from_value(params).ok() } + +fn initialize_client_info_from_typed_request(request: &ClientRequest) -> Option<(&str, &str)> { + match request { + ClientRequest::Initialize { params, .. } => Some(( + params.client_info.name.as_str(), + params.client_info.version.as_str(), + )), + _ => None, + } +} diff --git a/codex-rs/app-server/src/in_process.rs b/codex-rs/app-server/src/in_process.rs new file mode 100644 index 000000000000..7686fd03edad --- /dev/null +++ b/codex-rs/app-server/src/in_process.rs @@ -0,0 +1,661 @@ +//! In-process app-server runtime host for local embedders. +//! +//! This module runs the existing `MessageProcessor` and outbound routing logic +//! on Tokio tasks, but replaces socket/stdio transports with bounded in-memory +//! channels. The intent is to preserve app-server semantics while avoiding a +//! process boundary for CLI surfaces that run in the same process. +//! +//! The lifecycle is: +//! - construct runtime state with [`InProcessStartArgs`] +//! - perform `initialize` + `initialized` handshake in [`start`] +//! - interact through [`InProcessClientHandle`] request/notify/event methods +//! - terminate with [`InProcessClientHandle::shutdown`] +//! +//! Backpressure is explicit: command submission uses `try_send` and can return +//! `WouldBlock`, while event fanout may drop notifications under saturation. +//! Server requests are never silently abandoned: if they cannot be queued they +//! are failed back into `MessageProcessor` with overload/internal errors. + +use std::collections::HashMap; +use std::collections::HashSet; +use std::collections::hash_map::Entry; +use std::io::Error as IoError; +use std::io::ErrorKind; +use std::io::Result as IoResult; +use std::sync::Arc; +use std::sync::RwLock; +use std::sync::atomic::AtomicBool; +use std::sync::atomic::Ordering; +use std::time::Duration; + +use crate::error_code::INTERNAL_ERROR_CODE; +use crate::error_code::INVALID_REQUEST_ERROR_CODE; +use crate::error_code::OVERLOADED_ERROR_CODE; +use crate::message_processor::ConnectionSessionState; +use crate::message_processor::MessageProcessor; +use crate::message_processor::MessageProcessorArgs; +use crate::outgoing_message::ConnectionId; +use crate::outgoing_message::OutgoingEnvelope; +use crate::outgoing_message::OutgoingMessage; +use crate::outgoing_message::OutgoingMessageSender; +use crate::transport::CHANNEL_CAPACITY; +use crate::transport::OutboundConnectionState; +use crate::transport::route_outgoing_envelope; +use codex_app_server_protocol::ClientNotification; +use codex_app_server_protocol::ClientRequest; +use codex_app_server_protocol::ConfigWarningNotification; +use codex_app_server_protocol::InitializeParams; +use codex_app_server_protocol::JSONRPCError; +use codex_app_server_protocol::JSONRPCErrorError; +use codex_app_server_protocol::JSONRPCNotification; +use codex_app_server_protocol::JSONRPCResponse; +use codex_app_server_protocol::RequestId; +use codex_app_server_protocol::Result; +use codex_app_server_protocol::ServerNotification; +use codex_app_server_protocol::ServerRequest; +use codex_arg0::Arg0DispatchPaths; +use codex_core::config::Config; +use codex_core::config_loader::CloudRequirementsLoader; +use codex_core::config_loader::LoaderOverrides; +use codex_feedback::CodexFeedback; +use codex_protocol::protocol::SessionSource; +use tokio::sync::mpsc; +use tokio::sync::oneshot; +use tokio::time::timeout; +use toml::Value as TomlValue; +use tracing::warn; + +const IN_PROCESS_CONNECTION_ID: ConnectionId = ConnectionId(0); +const SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(5); +/// Default bounded channel capacity for in-process runtime queues. +pub const DEFAULT_IN_PROCESS_CHANNEL_CAPACITY: usize = CHANNEL_CAPACITY; + +type PendingClientRequestResponse = std::result::Result; + +/// Input needed to start an in-process app-server runtime. +#[derive(Clone)] +pub struct InProcessStartArgs { + /// Resolved argv0 dispatch paths used by command execution internals. + pub arg0_paths: Arg0DispatchPaths, + /// Shared base config used to initialize core components. + pub config: Arc, + /// CLI config overrides that are already parsed into TOML values. + pub cli_overrides: Vec<(String, TomlValue)>, + /// Loader override knobs used by config API paths. + pub loader_overrides: LoaderOverrides, + /// Preloaded cloud requirements provider. + pub cloud_requirements: CloudRequirementsLoader, + /// Feedback sink used by app-server/core telemetry and logs. + pub feedback: CodexFeedback, + /// Startup warnings emitted after initialize succeeds. + pub config_warnings: Vec, + /// Session source stamped into thread/session metadata. + pub session_source: SessionSource, + /// Initialize params used for initial handshake. + pub initialize: InitializeParams, + /// Capacity used for all runtime queues (clamped to at least 1). + pub channel_capacity: usize, +} + +/// Event emitted from the app-server to the in-process client. +#[derive(Debug, Clone)] +pub enum InProcessServerEvent { + /// Server request that requires client response/rejection. + ServerRequest(ServerRequest), + /// App-server notification directed to the embedded client. + ServerNotification(ServerNotification), + /// Legacy JSON-RPC notification from core event bridge. + LegacyNotification(JSONRPCNotification), + /// Indicates one or more events were dropped due to backpressure. + Lagged { skipped: usize }, +} + +enum InProcessClientMessage { + Request { + request: Box, + response_tx: oneshot::Sender, + }, + Notification { + notification: ClientNotification, + }, + ServerRequestResponse { + request_id: RequestId, + result: Result, + }, + ServerRequestError { + request_id: RequestId, + error: JSONRPCErrorError, + }, + Shutdown { + done_tx: oneshot::Sender<()>, + }, +} + +/// Handle used by an in-process client to call app-server and consume events. +pub struct InProcessClientHandle { + client_tx: mpsc::Sender, + event_rx: mpsc::Receiver, + runtime_handle: tokio::task::JoinHandle<()>, +} + +impl InProcessClientHandle { + /// Sends a typed client request into the in-process runtime. + /// + /// The returned value is a transport-level `IoResult` containing either a + /// JSON-RPC success payload or JSON-RPC error payload. Callers must keep + /// request IDs unique among concurrent requests; reusing an in-flight ID + /// produces an `INVALID_REQUEST` response and can make request routing + /// ambiguous in the caller. + pub async fn request(&self, request: ClientRequest) -> IoResult { + let (response_tx, response_rx) = oneshot::channel(); + self.try_send_client_message(InProcessClientMessage::Request { + request: Box::new(request), + response_tx, + })?; + response_rx.await.map_err(|err| { + IoError::new( + ErrorKind::BrokenPipe, + format!("in-process request response channel closed: {err}"), + ) + }) + } + + /// Sends a typed client notification into the in-process runtime. + /// + /// Notifications do not have an application-level response. Transport + /// errors indicate queue saturation or closed runtime. + pub fn notify(&self, notification: ClientNotification) -> IoResult<()> { + self.try_send_client_message(InProcessClientMessage::Notification { notification }) + } + + /// Resolves a pending [`ServerRequest`](InProcessServerEvent::ServerRequest). + /// + /// This should be used only with request IDs received from the current + /// runtime event stream; sending arbitrary IDs has no effect on app-server + /// state and can mask a stuck approval flow in the caller. + pub fn respond_to_server_request(&self, request_id: RequestId, result: Result) -> IoResult<()> { + self.try_send_client_message(InProcessClientMessage::ServerRequestResponse { + request_id, + result, + }) + } + + /// Rejects a pending [`ServerRequest`](InProcessServerEvent::ServerRequest). + /// + /// Use this when the embedder cannot satisfy a server request; leaving + /// requests unanswered can stall turn progress. + pub fn fail_server_request( + &self, + request_id: RequestId, + error: JSONRPCErrorError, + ) -> IoResult<()> { + self.try_send_client_message(InProcessClientMessage::ServerRequestError { + request_id, + error, + }) + } + + /// Receives the next server event from the in-process runtime. + /// + /// Returns `None` when the runtime task exits and no more events are + /// available. + pub async fn next_event(&mut self) -> Option { + self.event_rx.recv().await + } + + /// Requests runtime shutdown and waits for worker termination. + /// + /// Shutdown is bounded by internal timeouts and may abort background tasks + /// if graceful drain does not complete in time. + pub async fn shutdown(self) -> IoResult<()> { + let mut runtime_handle = self.runtime_handle; + let (done_tx, done_rx) = oneshot::channel(); + + if self + .client_tx + .send(InProcessClientMessage::Shutdown { done_tx }) + .await + .is_ok() + { + let _ = timeout(SHUTDOWN_TIMEOUT, done_rx).await; + } + + if let Err(_elapsed) = timeout(SHUTDOWN_TIMEOUT, &mut runtime_handle).await { + runtime_handle.abort(); + let _ = runtime_handle.await; + } + Ok(()) + } + + fn try_send_client_message(&self, message: InProcessClientMessage) -> IoResult<()> { + match self.client_tx.try_send(message) { + Ok(()) => Ok(()), + Err(mpsc::error::TrySendError::Full(_)) => Err(IoError::new( + ErrorKind::WouldBlock, + "in-process app-server client queue is full", + )), + Err(mpsc::error::TrySendError::Closed(_)) => Err(IoError::new( + ErrorKind::BrokenPipe, + "in-process app-server runtime is closed", + )), + } + } +} + +/// Starts an in-process app-server runtime and performs initialize handshake. +/// +/// This function sends `initialize` followed by `initialized` before returning +/// the handle, so callers receive a ready-to-use runtime. If initialize fails, +/// the runtime is shut down and an `InvalidData` error is returned. +pub async fn start(args: InProcessStartArgs) -> IoResult { + let initialize = args.initialize.clone(); + let client = start_uninitialized(args); + + let initialize_response = client + .request(ClientRequest::Initialize { + request_id: RequestId::Integer(0), + params: initialize, + }) + .await?; + if let Err(error) = initialize_response { + let _ = client.shutdown().await; + return Err(IoError::new( + ErrorKind::InvalidData, + format!("in-process initialize failed: {}", error.message), + )); + } + client.notify(ClientNotification::Initialized)?; + + Ok(client) +} + +fn start_uninitialized(args: InProcessStartArgs) -> InProcessClientHandle { + let channel_capacity = args.channel_capacity.max(1); + let (client_tx, mut client_rx) = mpsc::channel::(channel_capacity); + let (event_tx, event_rx) = mpsc::channel::(channel_capacity); + + let runtime_handle = tokio::spawn(async move { + let (outgoing_tx, mut outgoing_rx) = mpsc::channel::(channel_capacity); + let outgoing_message_sender = Arc::new(OutgoingMessageSender::new(outgoing_tx)); + + let (writer_tx, mut writer_rx) = mpsc::channel::(channel_capacity); + let outbound_initialized = Arc::new(AtomicBool::new(false)); + let outbound_experimental_api_enabled = Arc::new(AtomicBool::new(false)); + let outbound_opted_out_notification_methods = Arc::new(RwLock::new(HashSet::new())); + + let mut outbound_connections = HashMap::::new(); + outbound_connections.insert( + IN_PROCESS_CONNECTION_ID, + OutboundConnectionState::new( + writer_tx, + Arc::clone(&outbound_initialized), + Arc::clone(&outbound_experimental_api_enabled), + Arc::clone(&outbound_opted_out_notification_methods), + None, + ), + ); + let mut outbound_handle = tokio::spawn(async move { + while let Some(envelope) = outgoing_rx.recv().await { + route_outgoing_envelope(&mut outbound_connections, envelope).await; + } + }); + + let mut processor = MessageProcessor::new(MessageProcessorArgs { + outgoing: outgoing_message_sender, + arg0_paths: args.arg0_paths, + config: args.config, + cli_overrides: args.cli_overrides, + loader_overrides: args.loader_overrides, + cloud_requirements: args.cloud_requirements, + feedback: args.feedback, + log_db: None, + config_warnings: args.config_warnings, + session_source: args.session_source, + }); + let mut thread_created_rx = processor.thread_created_receiver(); + let mut session = ConnectionSessionState::default(); + let mut pending_request_responses = + HashMap::>::new(); + let mut shutdown_ack = None; + let mut listen_for_threads = true; + + loop { + tokio::select! { + message = client_rx.recv() => { + match message { + Some(InProcessClientMessage::Request { request, response_tx }) => { + let request = *request; + let request_id = request.id().clone(); + match pending_request_responses.entry(request_id.clone()) { + Entry::Vacant(entry) => { + entry.insert(response_tx); + } + Entry::Occupied(_) => { + let _ = response_tx.send(Err(JSONRPCErrorError { + code: INVALID_REQUEST_ERROR_CODE, + message: format!("duplicate request id: {request_id:?}"), + data: None, + })); + continue; + } + } + + let was_initialized = session.initialized; + processor + .process_client_request( + IN_PROCESS_CONNECTION_ID, + request, + &mut session, + &outbound_initialized, + ) + .await; + if let Ok(mut opted_out_notification_methods) = + outbound_opted_out_notification_methods.write() + { + *opted_out_notification_methods = + session.opted_out_notification_methods.clone(); + } else { + warn!("failed to update outbound opted-out notifications"); + } + outbound_experimental_api_enabled.store( + session.experimental_api_enabled, + Ordering::Release, + ); + if !was_initialized && session.initialized { + processor.send_initialize_notifications().await; + } + } + Some(InProcessClientMessage::Notification { notification }) => { + processor.process_client_notification(notification).await; + } + Some(InProcessClientMessage::ServerRequestResponse { request_id, result }) => { + processor + .process_response(JSONRPCResponse { + id: request_id, + result, + }) + .await; + } + Some(InProcessClientMessage::ServerRequestError { request_id, error }) => { + processor + .process_error(JSONRPCError { + error, + id: request_id, + }) + .await; + } + Some(InProcessClientMessage::Shutdown { done_tx }) => { + shutdown_ack = Some(done_tx); + break; + } + None => { + break; + } + } + } + outgoing_message = writer_rx.recv() => { + let Some(outgoing_message) = outgoing_message else { + break; + }; + match outgoing_message { + OutgoingMessage::Response(response) => { + if let Some(response_tx) = pending_request_responses.remove(&response.id) { + let _ = response_tx.send(Ok(response.result)); + } else { + warn!( + request_id = ?response.id, + "dropping unmatched in-process response" + ); + } + } + OutgoingMessage::Error(error) => { + if let Some(response_tx) = pending_request_responses.remove(&error.id) { + let _ = response_tx.send(Err(error.error)); + } else { + warn!( + request_id = ?error.id, + "dropping unmatched in-process error response" + ); + } + } + OutgoingMessage::Request(request) => { + if let Err(send_error) = event_tx + .try_send(InProcessServerEvent::ServerRequest(request.clone())) + { + match send_error { + mpsc::error::TrySendError::Full(_) => { + processor + .process_error(JSONRPCError { + id: request.id().clone(), + error: JSONRPCErrorError { + code: OVERLOADED_ERROR_CODE, + message: "in-process server request queue is full" + .to_string(), + data: None, + }, + }) + .await; + } + mpsc::error::TrySendError::Closed(_) => { + processor + .process_error(JSONRPCError { + id: request.id().clone(), + error: JSONRPCErrorError { + code: INTERNAL_ERROR_CODE, + message: "in-process server request consumer is closed" + .to_string(), + data: None, + }, + }) + .await; + } + } + } + } + OutgoingMessage::AppServerNotification(notification) => { + if let Err(send_error) = event_tx + .try_send(InProcessServerEvent::ServerNotification(notification)) + { + match send_error { + mpsc::error::TrySendError::Full(_) => { + warn!("dropping in-process server notification (queue full)"); + } + mpsc::error::TrySendError::Closed(_) => { + break; + } + } + } + } + OutgoingMessage::Notification(notification) => { + if let Err(send_error) = event_tx.try_send( + InProcessServerEvent::LegacyNotification(JSONRPCNotification { + method: notification.method, + params: notification.params, + }), + ) { + match send_error { + mpsc::error::TrySendError::Full(_) => { + warn!("dropping in-process legacy notification (queue full)"); + } + mpsc::error::TrySendError::Closed(_) => { + break; + } + } + } + } + } + } + created = thread_created_rx.recv(), if listen_for_threads => { + match created { + Ok(thread_id) => { + let connection_ids = if session.initialized { + vec![IN_PROCESS_CONNECTION_ID] + } else { + Vec::::new() + }; + processor + .try_attach_thread_listener(thread_id, connection_ids) + .await; + } + Err(tokio::sync::broadcast::error::RecvError::Lagged(_)) => { + warn!("thread_created receiver lagged; skipping resync"); + } + Err(tokio::sync::broadcast::error::RecvError::Closed) => { + listen_for_threads = false; + } + } + } + } + } + + processor.connection_closed(IN_PROCESS_CONNECTION_ID).await; + for (_, response_tx) in pending_request_responses { + let _ = response_tx.send(Err(JSONRPCErrorError { + code: INTERNAL_ERROR_CODE, + message: "in-process app-server runtime is shutting down".to_string(), + data: None, + })); + } + + drop(processor); + if let Err(_elapsed) = timeout(SHUTDOWN_TIMEOUT, &mut outbound_handle).await { + outbound_handle.abort(); + let _ = outbound_handle.await; + } + + if let Some(done_tx) = shutdown_ack { + let _ = done_tx.send(()); + } + }); + + InProcessClientHandle { + client_tx, + event_rx, + runtime_handle, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use codex_app_server_protocol::ClientInfo; + use codex_app_server_protocol::ConfigRequirementsReadResponse; + use codex_app_server_protocol::SessionSource as ApiSessionSource; + use codex_app_server_protocol::ThreadStartParams; + use codex_app_server_protocol::ThreadStartResponse; + use codex_core::config::ConfigBuilder; + use pretty_assertions::assert_eq; + + async fn build_test_config() -> Config { + match ConfigBuilder::default().build().await { + Ok(config) => config, + Err(_) => Config::load_default_with_cli_overrides(Vec::new()) + .expect("default config should load"), + } + } + + async fn start_test_client_with_capacity( + session_source: SessionSource, + channel_capacity: usize, + ) -> InProcessClientHandle { + let args = InProcessStartArgs { + arg0_paths: Arg0DispatchPaths::default(), + config: Arc::new(build_test_config().await), + cli_overrides: Vec::new(), + loader_overrides: LoaderOverrides::default(), + cloud_requirements: CloudRequirementsLoader::default(), + feedback: CodexFeedback::new(), + config_warnings: Vec::new(), + session_source, + initialize: InitializeParams { + client_info: ClientInfo { + name: "codex-in-process-test".to_string(), + title: None, + version: "0.0.0".to_string(), + }, + capabilities: None, + }, + channel_capacity, + }; + start(args).await.expect("in-process runtime should start") + } + + async fn start_test_client(session_source: SessionSource) -> InProcessClientHandle { + start_test_client_with_capacity(session_source, DEFAULT_IN_PROCESS_CHANNEL_CAPACITY).await + } + + #[tokio::test] + async fn in_process_start_initializes_and_handles_typed_v2_request() { + let client = start_test_client(SessionSource::Cli).await; + let response = client + .request(ClientRequest::ConfigRequirementsRead { + request_id: RequestId::Integer(1), + params: None, + }) + .await + .expect("request transport should work") + .expect("request should succeed"); + assert!(response.is_object()); + + let _parsed: ConfigRequirementsReadResponse = + serde_json::from_value(response).expect("response should match v2 schema"); + client + .shutdown() + .await + .expect("in-process runtime should shutdown cleanly"); + } + + #[tokio::test] + async fn in_process_start_uses_requested_session_source_for_thread_start() { + for (requested_source, expected_source) in [ + (SessionSource::Cli, ApiSessionSource::Cli), + (SessionSource::Exec, ApiSessionSource::Exec), + ] { + let client = start_test_client(requested_source).await; + let response = client + .request(ClientRequest::ThreadStart { + request_id: RequestId::Integer(2), + params: ThreadStartParams { + ephemeral: Some(true), + ..ThreadStartParams::default() + }, + }) + .await + .expect("request transport should work") + .expect("thread/start should succeed"); + let parsed: ThreadStartResponse = + serde_json::from_value(response).expect("thread/start response should parse"); + assert_eq!(parsed.thread.source, expected_source); + client + .shutdown() + .await + .expect("in-process runtime should shutdown cleanly"); + } + } + + #[tokio::test] + async fn in_process_start_clamps_zero_channel_capacity() { + let client = start_test_client_with_capacity(SessionSource::Cli, 0).await; + let response = loop { + match client + .request(ClientRequest::ConfigRequirementsRead { + request_id: RequestId::Integer(4), + params: None, + }) + .await + { + Ok(response) => break response.expect("request should succeed"), + Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => { + tokio::task::yield_now().await; + } + Err(err) => panic!("request transport should work: {err}"), + } + }; + let _parsed: ConfigRequirementsReadResponse = + serde_json::from_value(response).expect("response should match v2 schema"); + client + .shutdown() + .await + .expect("in-process runtime should shutdown cleanly"); + } +} diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index 34796c575f1a..4852216d3fad 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -39,6 +39,7 @@ use codex_core::check_execpolicy_for_warnings; use codex_core::config_loader::ConfigLoadError; use codex_core::config_loader::TextRange as CoreTextRange; use codex_feedback::CodexFeedback; +use codex_protocol::protocol::SessionSource; use codex_state::log_db; use tokio::sync::mpsc; use tokio::task::JoinHandle; @@ -65,6 +66,7 @@ mod error_code; mod external_agent_config_api; mod filters; mod fuzzy_file_search; +pub mod in_process; mod message_processor; mod models; mod outgoing_message; @@ -597,6 +599,7 @@ pub async fn run_main_with_transport( feedback: feedback.clone(), log_db, config_warnings, + session_source: SessionSource::VSCode, }); let mut thread_created_rx = processor.thread_created_receiver(); let mut running_turn_count_rx = processor.subscribe_running_assistant_turn_count(); diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 51049e624a3a..3848bb96700a 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -18,6 +18,7 @@ use codex_app_server_protocol::ChatgptAuthTokensRefreshParams; use codex_app_server_protocol::ChatgptAuthTokensRefreshReason; use codex_app_server_protocol::ChatgptAuthTokensRefreshResponse; use codex_app_server_protocol::ClientInfo; +use codex_app_server_protocol::ClientNotification; use codex_app_server_protocol::ClientRequest; use codex_app_server_protocol::ConfigBatchWriteParams; use codex_app_server_protocol::ConfigReadParams; @@ -157,6 +158,7 @@ pub(crate) struct MessageProcessorArgs { pub(crate) feedback: CodexFeedback, pub(crate) log_db: Option, pub(crate) config_warnings: Vec, + pub(crate) session_source: SessionSource, } impl MessageProcessor { @@ -173,6 +175,7 @@ impl MessageProcessor { feedback, log_db, config_warnings, + session_source, } = args; let auth_manager = AuthManager::shared( config.codex_home.clone(), @@ -186,7 +189,7 @@ impl MessageProcessor { let thread_manager = Arc::new(ThreadManager::new( config.codex_home.clone(), auth_manager.clone(), - SessionSource::VSCode, + session_source, config.model_catalog.clone(), CollaborationModesConfig { default_mode_request_user_input: config @@ -274,187 +277,50 @@ impl MessageProcessor { } }; - match codex_request { - // Handle Initialize internally so CodexMessageProcessor does not have to concern - // itself with the `initialized` bool. - ClientRequest::Initialize { request_id, params } => { - let request_id = ConnectionRequestId { - connection_id, - request_id, - }; - if session.initialized { - let error = JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: "Already initialized".to_string(), - data: None, - }; - self.outgoing.send_error(request_id, error).await; - return; - } else { - // TODO(maxj): Revisit capability scoping for `experimental_api_enabled`. - // Current behavior is per-connection. Reviewer feedback notes this can - // create odd cross-client behavior (for example dynamic tool calls on a - // shared thread when another connected client did not opt into - // experimental API). Proposed direction is instance-global first-write-wins - // with initialize-time mismatch rejection. - let (experimental_api_enabled, opt_out_notification_methods) = - match params.capabilities { - Some(capabilities) => ( - capabilities.experimental_api, - capabilities - .opt_out_notification_methods - .unwrap_or_default(), - ), - None => (false, Vec::new()), - }; - session.experimental_api_enabled = experimental_api_enabled; - session.opted_out_notification_methods = - opt_out_notification_methods.into_iter().collect(); - let ClientInfo { - name, - title: _title, - version, - } = params.client_info; - session.app_server_client_name = Some(name.clone()); - session.client_version = Some(version.clone()); - if let Err(error) = set_default_originator(name.clone()) { - match error { - SetOriginatorError::InvalidHeaderValue => { - let error = JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: format!( - "Invalid clientInfo.name: '{name}'. Must be a valid HTTP header value." - ), - data: None, - }; - self.outgoing.send_error(request_id.clone(), error).await; - return; - } - SetOriginatorError::AlreadyInitialized => { - // No-op. This is expected to happen if the originator is already set via env var. - // TODO(owen): Once we remove support for CODEX_INTERNAL_ORIGINATOR_OVERRIDE, - // this will be an unexpected state and we can return a JSON-RPC error indicating - // internal server error. - } - } - } - set_default_client_residency_requirement(self.config.enforce_residency.value()); - let user_agent_suffix = format!("{name}; {version}"); - if let Ok(mut suffix) = USER_AGENT_SUFFIX.lock() { - *suffix = Some(user_agent_suffix); - } - - let user_agent = get_codex_user_agent(); - let response = InitializeResponse { user_agent }; - self.outgoing.send_response(request_id, response).await; - - session.initialized = true; - outbound_initialized.store(true, Ordering::Release); - self.codex_message_processor - .connection_initialized(connection_id) - .await; - return; - } - } - _ => { - if !session.initialized { - let error = JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: "Not initialized".to_string(), - data: None, - }; - self.outgoing.send_error(request_id, error).await; - return; - } - } - } - if let Some(reason) = codex_request.experimental_reason() - && !session.experimental_api_enabled - { - let error = JSONRPCErrorError { - code: INVALID_REQUEST_ERROR_CODE, - message: experimental_required_message(reason), - data: None, - }; - self.outgoing.send_error(request_id, error).await; - return; - } + self.handle_client_request( + connection_id, + request_id, + codex_request, + session, + outbound_initialized, + ) + .await; + } + .instrument(request_span) + .await; + } - match codex_request { - ClientRequest::ConfigRead { request_id, params } => { - self.handle_config_read( - ConnectionRequestId { - connection_id, - request_id, - }, - params, - ) - .await; - } - ClientRequest::ExternalAgentConfigDetect { request_id, params } => { - self.handle_external_agent_config_detect( - ConnectionRequestId { - connection_id, - request_id, - }, - params, - ) - .await; - } - ClientRequest::ExternalAgentConfigImport { request_id, params } => { - self.handle_external_agent_config_import( - ConnectionRequestId { - connection_id, - request_id, - }, - params, - ) - .await; - } - ClientRequest::ConfigValueWrite { request_id, params } => { - self.handle_config_value_write( - ConnectionRequestId { - connection_id, - request_id, - }, - params, - ) - .await; - } - ClientRequest::ConfigBatchWrite { request_id, params } => { - self.handle_config_batch_write( - ConnectionRequestId { - connection_id, - request_id, - }, - params, - ) - .await; - } - ClientRequest::ConfigRequirementsRead { - request_id, - params: _, - } => { - self.handle_config_requirements_read(ConnectionRequestId { - connection_id, - request_id, - }) - .await; - } - other => { - // Box the delegated future so this wrapper's async state machine does not - // inline the full `CodexMessageProcessor::process_request` future, which - // can otherwise push worker-thread stack usage over the edge. - self.codex_message_processor - .process_request( - connection_id, - other, - session.app_server_client_name.clone(), - ) - .boxed() - .await; - } - } + /// Handles a typed request path used by in-process embedders. + /// + /// This bypasses JSON request deserialization but keeps identical request + /// semantics by delegating to `handle_client_request`. + pub(crate) async fn process_client_request( + &mut self, + connection_id: ConnectionId, + request: ClientRequest, + session: &mut ConnectionSessionState, + outbound_initialized: &AtomicBool, + ) { + let request_span = + crate::app_server_tracing::typed_request_span(&request, connection_id, session); + async { + let request_id = ConnectionRequestId { + connection_id, + request_id: request.id().clone(), + }; + tracing::trace!( + ?connection_id, + request_id = ?request_id.request_id, + "app-server typed request" + ); + self.handle_client_request( + connection_id, + request_id, + request, + session, + outbound_initialized, + ) + .await; } .instrument(request_span) .await; @@ -466,6 +332,13 @@ impl MessageProcessor { tracing::info!("<- notification: {:?}", notification); } + /// Handles typed notifications from in-process clients. + pub(crate) async fn process_client_notification(&self, notification: ClientNotification) { + // Currently, we do not expect to receive any typed notifications from + // in-process clients, so we just log them. + tracing::info!("<- typed notification: {:?}", notification); + } + pub(crate) fn thread_created_receiver(&self) -> broadcast::Receiver { self.codex_message_processor.thread_created_receiver() } @@ -512,6 +385,193 @@ impl MessageProcessor { self.outgoing.notify_client_error(err.id, err.error).await; } + async fn handle_client_request( + &mut self, + connection_id: ConnectionId, + request_id: ConnectionRequestId, + codex_request: ClientRequest, + session: &mut ConnectionSessionState, + outbound_initialized: &AtomicBool, + ) { + match codex_request { + // Handle Initialize internally so CodexMessageProcessor does not have to concern + // itself with the `initialized` bool. + ClientRequest::Initialize { request_id, params } => { + let request_id = ConnectionRequestId { + connection_id, + request_id, + }; + if session.initialized { + let error = JSONRPCErrorError { + code: INVALID_REQUEST_ERROR_CODE, + message: "Already initialized".to_string(), + data: None, + }; + self.outgoing.send_error(request_id, error).await; + return; + } + + // TODO(maxj): Revisit capability scoping for `experimental_api_enabled`. + // Current behavior is per-connection. Reviewer feedback notes this can + // create odd cross-client behavior (for example dynamic tool calls on a + // shared thread when another connected client did not opt into + // experimental API). Proposed direction is instance-global first-write-wins + // with initialize-time mismatch rejection. + let (experimental_api_enabled, opt_out_notification_methods) = + match params.capabilities { + Some(capabilities) => ( + capabilities.experimental_api, + capabilities + .opt_out_notification_methods + .unwrap_or_default(), + ), + None => (false, Vec::new()), + }; + session.experimental_api_enabled = experimental_api_enabled; + session.opted_out_notification_methods = + opt_out_notification_methods.into_iter().collect(); + let ClientInfo { + name, + title: _title, + version, + } = params.client_info; + session.app_server_client_name = Some(name.clone()); + session.client_version = Some(version.clone()); + if let Err(error) = set_default_originator(name.clone()) { + match error { + SetOriginatorError::InvalidHeaderValue => { + let error = JSONRPCErrorError { + code: INVALID_REQUEST_ERROR_CODE, + message: format!( + "Invalid clientInfo.name: '{name}'. Must be a valid HTTP header value." + ), + data: None, + }; + self.outgoing.send_error(request_id.clone(), error).await; + return; + } + SetOriginatorError::AlreadyInitialized => { + // No-op. This is expected to happen if the originator is already set via env var. + // TODO(owen): Once we remove support for CODEX_INTERNAL_ORIGINATOR_OVERRIDE, + // this will be an unexpected state and we can return a JSON-RPC error indicating + // internal server error. + } + } + } + set_default_client_residency_requirement(self.config.enforce_residency.value()); + let user_agent_suffix = format!("{name}; {version}"); + if let Ok(mut suffix) = USER_AGENT_SUFFIX.lock() { + *suffix = Some(user_agent_suffix); + } + + let user_agent = get_codex_user_agent(); + let response = InitializeResponse { user_agent }; + self.outgoing.send_response(request_id, response).await; + + session.initialized = true; + outbound_initialized.store(true, Ordering::Release); + self.codex_message_processor + .connection_initialized(connection_id) + .await; + return; + } + _ => { + if !session.initialized { + let error = JSONRPCErrorError { + code: INVALID_REQUEST_ERROR_CODE, + message: "Not initialized".to_string(), + data: None, + }; + self.outgoing.send_error(request_id, error).await; + return; + } + } + } + if let Some(reason) = codex_request.experimental_reason() + && !session.experimental_api_enabled + { + let error = JSONRPCErrorError { + code: INVALID_REQUEST_ERROR_CODE, + message: experimental_required_message(reason), + data: None, + }; + self.outgoing.send_error(request_id, error).await; + return; + } + + match codex_request { + ClientRequest::ConfigRead { request_id, params } => { + self.handle_config_read( + ConnectionRequestId { + connection_id, + request_id, + }, + params, + ) + .await; + } + ClientRequest::ExternalAgentConfigDetect { request_id, params } => { + self.handle_external_agent_config_detect( + ConnectionRequestId { + connection_id, + request_id, + }, + params, + ) + .await; + } + ClientRequest::ExternalAgentConfigImport { request_id, params } => { + self.handle_external_agent_config_import( + ConnectionRequestId { + connection_id, + request_id, + }, + params, + ) + .await; + } + ClientRequest::ConfigValueWrite { request_id, params } => { + self.handle_config_value_write( + ConnectionRequestId { + connection_id, + request_id, + }, + params, + ) + .await; + } + ClientRequest::ConfigBatchWrite { request_id, params } => { + self.handle_config_batch_write( + ConnectionRequestId { + connection_id, + request_id, + }, + params, + ) + .await; + } + ClientRequest::ConfigRequirementsRead { + request_id, + params: _, + } => { + self.handle_config_requirements_read(ConnectionRequestId { + connection_id, + request_id, + }) + .await; + } + other => { + // Box the delegated future so this wrapper's async state machine does not + // inline the full `CodexMessageProcessor::process_request` future, which + // can otherwise push worker-thread stack usage over the edge. + self.codex_message_processor + .process_request(connection_id, other, session.app_server_client_name.clone()) + .boxed() + .await; + } + } + } + async fn handle_config_read(&self, request_id: ConnectionRequestId, params: ConfigReadParams) { match self.config_api.read(params).await { Ok(response) => self.outgoing.send_response(request_id, response).await, From ac7c7271d2d786215d101061d2b0de4e32d3997e Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Thu, 5 Mar 2026 16:38:34 -0300 Subject: [PATCH 02/25] feat(exec): migrate exec to in-process app-server Route `codex-exec` through `InProcessAppServerClient` instead of using direct `ThreadManager` and `CodexThread` wiring. This moves startup, resume, turn start, review start, interrupt, and shutdown through the app-server path while keeping existing exec behavior working under the current protocol constraints. Restore exec-specific auth and request handling needed for parity, including `CODEX_API_KEY` env loading, local handling of app-server server requests, config-aware start/resume params, scoped error tracking, and foreground-turn `Ctrl-C` interruption. --- codex-rs/Cargo.lock | 3 + codex-rs/app-server/src/message_processor.rs | 3 +- codex-rs/exec/Cargo.toml | 3 + codex-rs/exec/src/lib.rs | 1071 ++++++++++++++---- 4 files changed, 836 insertions(+), 244 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 4b8852d4af1a..008cd5722d47 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1917,10 +1917,13 @@ dependencies = [ "anyhow", "assert_cmd", "clap", + "codex-app-server-client", + "codex-app-server-protocol", "codex-apply-patch", "codex-arg0", "codex-cloud-requirements", "codex-core", + "codex-feedback", "codex-otel", "codex-protocol", "codex-utils-absolute-path", diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 3848bb96700a..f7186744982b 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -177,9 +177,10 @@ impl MessageProcessor { config_warnings, session_source, } = args; + let enable_codex_api_key_env = matches!(session_source, SessionSource::Exec); let auth_manager = AuthManager::shared( config.codex_home.clone(), - false, + enable_codex_api_key_env, config.cli_auth_credentials_store_mode, ); auth_manager.set_forced_chatgpt_workspace_id(config.forced_chatgpt_workspace_id.clone()); diff --git a/codex-rs/exec/Cargo.toml b/codex-rs/exec/Cargo.toml index 1eb4f7e2773a..378523560653 100644 --- a/codex-rs/exec/Cargo.toml +++ b/codex-rs/exec/Cargo.toml @@ -19,8 +19,11 @@ workspace = true anyhow = { workspace = true } clap = { workspace = true, features = ["derive"] } codex-arg0 = { workspace = true } +codex-app-server-client = { workspace = true } +codex-app-server-protocol = { workspace = true } codex-cloud-requirements = { workspace = true } codex-core = { workspace = true } +codex-feedback = { workspace = true } codex-otel = { workspace = true } codex-protocol = { workspace = true } codex-utils-absolute-path = { workspace = true } diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 55978021f34f..f678ece85963 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -13,13 +13,39 @@ pub mod exec_events; pub use cli::Cli; pub use cli::Command; pub use cli::ReviewArgs; +use codex_app_server_client::ClientSurface; +use codex_app_server_client::DEFAULT_IN_PROCESS_CHANNEL_CAPACITY; +use codex_app_server_client::InProcessAppServerClient; +use codex_app_server_client::InProcessClientStartArgs; +use codex_app_server_client::InProcessServerEvent; +use codex_app_server_protocol::ChatgptAuthTokensRefreshResponse; +use codex_app_server_protocol::ClientRequest; +use codex_app_server_protocol::ConfigWarningNotification; +use codex_app_server_protocol::JSONRPCErrorError; +use codex_app_server_protocol::JSONRPCNotification; +use codex_app_server_protocol::McpServerElicitationAction; +use codex_app_server_protocol::McpServerElicitationRequestResponse; +use codex_app_server_protocol::RequestId; +use codex_app_server_protocol::ReviewStartParams; +use codex_app_server_protocol::ReviewStartResponse; +use codex_app_server_protocol::ReviewTarget as ApiReviewTarget; +use codex_app_server_protocol::ServerNotification; +use codex_app_server_protocol::ServerRequest; +use codex_app_server_protocol::ThreadResumeParams; +use codex_app_server_protocol::ThreadResumeResponse; +use codex_app_server_protocol::ThreadStartParams; +use codex_app_server_protocol::ThreadStartResponse; +use codex_app_server_protocol::ThreadUnsubscribeParams; +use codex_app_server_protocol::ThreadUnsubscribeResponse; +use codex_app_server_protocol::TurnInterruptParams; +use codex_app_server_protocol::TurnInterruptResponse; +use codex_app_server_protocol::TurnStartParams; +use codex_app_server_protocol::TurnStartResponse; use codex_arg0::Arg0DispatchPaths; use codex_cloud_requirements::cloud_requirements_loader; use codex_core::AuthManager; use codex_core::LMSTUDIO_OSS_PROVIDER_ID; -use codex_core::NewThread; use codex_core::OLLAMA_OSS_PROVIDER_ID; -use codex_core::ThreadManager; use codex_core::auth::enforce_login_restrictions; use codex_core::check_execpolicy_for_warnings; use codex_core::config::Config; @@ -29,23 +55,21 @@ use codex_core::config::find_codex_home; use codex_core::config::load_config_as_toml_with_cli_overrides; use codex_core::config::resolve_oss_provider; use codex_core::config_loader::ConfigLoadError; +use codex_core::config_loader::LoaderOverrides; use codex_core::config_loader::format_config_error_with_source; use codex_core::format_exec_policy_error_with_source; use codex_core::git_info::get_git_repo_root; -use codex_core::models_manager::collaboration_mode_presets::CollaborationModesConfig; -use codex_core::models_manager::manager::RefreshStrategy; +use codex_feedback::CodexFeedback; use codex_otel::set_parent_from_context; use codex_otel::traceparent_context_from_env; -use codex_protocol::approvals::ElicitationAction; +use codex_protocol::account::PlanType as AccountPlanType; use codex_protocol::config_types::SandboxMode; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; -use codex_protocol::protocol::Op; use codex_protocol::protocol::ReviewRequest; use codex_protocol::protocol::ReviewTarget; -use codex_protocol::protocol::SessionSource; -use codex_protocol::protocol::SubAgentSource; +use codex_protocol::protocol::SessionConfiguredEvent; use codex_protocol::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_oss::ensure_oss_provider_ready; @@ -54,14 +78,15 @@ use event_processor_with_human_output::EventProcessorWithHumanOutput; use event_processor_with_jsonl_output::EventProcessorWithJsonOutput; use serde_json::Value; use std::collections::HashSet; +use std::collections::VecDeque; use std::io::IsTerminal; use std::io::Read; use std::path::PathBuf; -use std::sync::Arc; use supports_color::Stream; -use tokio::sync::Mutex; +use tokio::sync::mpsc; +use tokio::time::Duration; +use tokio::time::timeout; use tracing::Instrument; -use tracing::debug; use tracing::error; use tracing::field; use tracing::info; @@ -91,15 +116,24 @@ enum InitialOperation { }, } -#[derive(Clone)] -struct ThreadEventEnvelope { - thread_id: codex_protocol::ThreadId, - thread: Arc, - event: Event, - suppress_output: bool, +struct RequestIdSequencer { + next: i64, +} + +impl RequestIdSequencer { + fn new() -> Self { + Self { next: 1 } + } + + fn next(&mut self) -> RequestId { + let id = self.next; + self.next += 1; + RequestId::Integer(id) + } } struct ExecRunArgs { + in_process_start_args: InProcessClientStartArgs, command: Option, config: Config, cursor_ansi: bool, @@ -265,6 +299,9 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result // TODO(gt): Make cloud requirements failures blocking once we can fail-closed. let cloud_requirements = cloud_requirements_loader(cloud_auth_manager, chatgpt_base_url, codex_home.clone()); + let run_cli_overrides = cli_kv_overrides.clone(); + let run_loader_overrides = LoaderOverrides::default(); + let run_cloud_requirements = cloud_requirements.clone(); let model_provider = if oss { let resolved = resolve_oss_provider( @@ -382,7 +419,33 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result if let Some(context) = traceparent_context_from_env() { set_parent_from_context(&exec_span, context); } + let config_warnings: Vec = config + .startup_warnings + .iter() + .map(|warning| ConfigWarningNotification { + summary: warning.clone(), + details: None, + path: None, + range: None, + }) + .collect(); + let in_process_start_args = InProcessClientStartArgs { + arg0_paths, + config: std::sync::Arc::new(config.clone()), + cli_overrides: run_cli_overrides, + loader_overrides: run_loader_overrides, + cloud_requirements: run_cloud_requirements, + feedback: CodexFeedback::new(), + config_warnings, + surface: ClientSurface::Exec, + client_name: Some("codex-exec".to_string()), + client_version: env!("CARGO_PKG_VERSION").to_string(), + experimental_api: true, + opt_out_notification_methods: Vec::new(), + channel_capacity: DEFAULT_IN_PROCESS_CHANNEL_CAPACITY, + }; run_exec_session(ExecRunArgs { + in_process_start_args, command, config, cursor_ansi, @@ -404,6 +467,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> { let ExecRunArgs { + in_process_start_args, command, config, cursor_ansi, @@ -469,49 +533,88 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> { std::process::exit(1); } - let auth_manager = AuthManager::shared( - config.codex_home.clone(), - true, - config.cli_auth_credentials_store_mode, - ); - let thread_manager = Arc::new(ThreadManager::new( - config.codex_home.clone(), - auth_manager.clone(), - SessionSource::Exec, - config.model_catalog.clone(), - CollaborationModesConfig { - default_mode_request_user_input: config - .features - .enabled(codex_core::features::Feature::DefaultModeRequestUserInput), - }, - )); - let default_model = thread_manager - .get_models_manager() - .get_default_model(&config.model, RefreshStrategy::OnlineIfUncached) - .await; + let mut request_ids = RequestIdSequencer::new(); + let mut client = InProcessAppServerClient::start(in_process_start_args) + .await + .map_err(|err| { + anyhow::anyhow!("failed to initialize in-process app-server client: {err}") + })?; // Handle resume subcommand by resolving a rollout path and using explicit resume API. - let NewThread { - thread_id: primary_thread_id, - thread, - session_configured, - } = if let Some(ExecCommand::Resume(args)) = command.as_ref() { - let resume_path = resolve_resume_path(&config, args).await?; - - if let Some(path) = resume_path { - thread_manager - .resume_thread_from_rollout(config.clone(), path, auth_manager.clone()) - .await? + let (primary_thread_id, fallback_session_configured) = + if let Some(ExecCommand::Resume(args)) = command.as_ref() { + let resume_path = resolve_resume_path(&config, args).await?; + + if let Some(path) = resume_path { + let response: ThreadResumeResponse = send_request_with_response( + &client, + ClientRequest::ThreadResume { + request_id: request_ids.next(), + params: thread_resume_params_from_config(&config, Some(path)), + }, + "thread/resume", + ) + .await + .map_err(anyhow::Error::msg)?; + let session_configured = session_configured_from_thread_resume_response(&response) + .map_err(anyhow::Error::msg)?; + (session_configured.session_id, session_configured) + } else { + let response: ThreadStartResponse = send_request_with_response( + &client, + ClientRequest::ThreadStart { + request_id: request_ids.next(), + params: thread_start_params_from_config(&config), + }, + "thread/start", + ) + .await + .map_err(anyhow::Error::msg)?; + let session_configured = session_configured_from_thread_start_response(&response) + .map_err(anyhow::Error::msg)?; + (session_configured.session_id, session_configured) + } } else { - thread_manager.start_thread(config.clone()).await? + let response: ThreadStartResponse = send_request_with_response( + &client, + ClientRequest::ThreadStart { + request_id: request_ids.next(), + params: thread_start_params_from_config(&config), + }, + "thread/start", + ) + .await + .map_err(anyhow::Error::msg)?; + let session_configured = session_configured_from_thread_start_response(&response) + .map_err(anyhow::Error::msg)?; + (session_configured.session_id, session_configured) + }; + + let primary_thread_id_for_span = primary_thread_id.to_string(); + let mut error_seen = false; + let mut buffered_events = VecDeque::new(); + let session_configured = match await_session_configured_event( + &mut client, + &config, + &primary_thread_id_for_span, + &mut error_seen, + &mut buffered_events, + ) + .await + { + Ok(session_configured) => session_configured, + Err(err) => { + warn!( + "failed to receive session configured event for thread {}: {err}; using startup response fallback", + primary_thread_id_for_span + ); + fallback_session_configured } - } else { - thread_manager.start_thread(config.clone()).await? }; - let primary_thread_id_for_span = primary_thread_id.to_string(); + exec_span.record("thread.id", primary_thread_id_for_span.as_str()); - let (initial_operation, prompt_summary) = match (command, prompt, images) { + let (initial_operation, prompt_summary) = match (command.as_ref(), prompt, images) { (Some(ExecCommand::Review(review_cli)), _, _) => { let review_request = build_review_request(review_cli)?; let summary = codex_core::review_prompts::user_facing_hint(&review_request.target); @@ -532,7 +635,7 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> { let prompt_text = resolve_prompt(prompt_arg); let mut items: Vec = imgs .into_iter() - .chain(args.images.into_iter()) + .chain(args.images.iter().cloned()) .map(|path| UserInput::LocalImage { path }) .collect(); items.push(UserInput::Text { @@ -577,84 +680,62 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> { info!("Codex initialized with event: {session_configured:?}"); - let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel::(); - let attached_threads = Arc::new(Mutex::new(HashSet::from([primary_thread_id]))); - spawn_thread_listener(primary_thread_id, thread.clone(), tx.clone(), false); - - { - let thread = thread.clone(); - tokio::spawn(async move { - if tokio::signal::ctrl_c().await.is_ok() { - tracing::debug!("Keyboard interrupt"); - // Immediately notify Codex to abort any in-flight task. - thread.submit(Op::Interrupt).await.ok(); - } - }); - } - - { - let thread_manager = Arc::clone(&thread_manager); - let attached_threads = Arc::clone(&attached_threads); - let tx = tx.clone(); - let mut thread_created_rx = thread_manager.subscribe_thread_created(); - tokio::spawn(async move { - loop { - match thread_created_rx.recv().await { - Ok(thread_id) => { - if attached_threads.lock().await.contains(&thread_id) { - continue; - } - match thread_manager.get_thread(thread_id).await { - Ok(thread) => { - attached_threads.lock().await.insert(thread_id); - let suppress_output = - is_agent_job_subagent(&thread.config_snapshot().await); - spawn_thread_listener( - thread_id, - thread, - tx.clone(), - suppress_output, - ); - } - Err(err) => { - warn!("failed to attach listener for thread {thread_id}: {err}") - } - } - } - Err(tokio::sync::broadcast::error::RecvError::Lagged(_)) => { - warn!("thread_created receiver lagged; skipping resync"); - } - Err(tokio::sync::broadcast::error::RecvError::Closed) => break, - } - } - }); - } + let (interrupt_tx, mut interrupt_rx) = mpsc::unbounded_channel::<()>(); + tokio::spawn(async move { + if tokio::signal::ctrl_c().await.is_ok() { + tracing::debug!("Keyboard interrupt"); + let _ = interrupt_tx.send(()); + } + }); let task_id = match initial_operation { InitialOperation::UserTurn { items, output_schema, } => { - let task_id = thread - .submit(Op::UserTurn { - items, - cwd: default_cwd, - approval_policy: default_approval_policy, - sandbox_policy: default_sandbox_policy.clone(), - model: default_model, - effort: default_effort, - summary: None, - service_tier: None, - final_output_json_schema: output_schema, - collaboration_mode: None, - personality: None, - }) - .await?; + let response: TurnStartResponse = send_request_with_response( + &client, + ClientRequest::TurnStart { + request_id: request_ids.next(), + params: TurnStartParams { + thread_id: primary_thread_id_for_span.clone(), + input: items.into_iter().map(Into::into).collect(), + cwd: Some(default_cwd), + approval_policy: Some(default_approval_policy.into()), + sandbox_policy: Some(default_sandbox_policy.clone().into()), + model: None, + service_tier: None, + effort: default_effort.map(Into::into), + summary: None, + personality: None, + output_schema, + collaboration_mode: None, + }, + }, + "turn/start", + ) + .await + .map_err(anyhow::Error::msg)?; + let task_id = response.turn.id; info!("Sent prompt with event ID: {task_id}"); task_id } InitialOperation::Review { review_request } => { - let task_id = thread.submit(Op::Review { review_request }).await?; + let response: ReviewStartResponse = send_request_with_response( + &client, + ClientRequest::ReviewStart { + request_id: request_ids.next(), + params: ReviewStartParams { + thread_id: primary_thread_id_for_span.clone(), + target: review_target_to_api(review_request.target), + delivery: None, + }, + }, + "review/start", + ) + .await + .map_err(anyhow::Error::msg)?; + let task_id = response.turn.id; info!("Sent review request with event ID: {task_id}"); task_id } @@ -665,70 +746,134 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> { // Track whether a fatal error was reported by the server so we can // exit with a non-zero status for automation-friendly signaling. let mut error_seen = false; - let mut shutdown_requested = false; - while let Some(envelope) = rx.recv().await { - let ThreadEventEnvelope { - thread_id, - thread, - event, - suppress_output, - } = envelope; - if suppress_output && should_suppress_agent_job_event(&event.msg) { - continue; - } - if matches!(event.msg, EventMsg::Error(_)) { - error_seen = true; - } - if shutdown_requested - && !matches!(&event.msg, EventMsg::ShutdownComplete | EventMsg::Error(_)) - { - continue; - } - if let EventMsg::ElicitationRequest(ev) = &event.msg { - // Automatically cancel elicitation requests in exec mode. - thread - .submit(Op::ResolveElicitation { - server_name: ev.server_name.clone(), - request_id: ev.id.clone(), - decision: ElicitationAction::Cancel, - content: None, - meta: None, - }) - .await?; - } - if let EventMsg::McpStartupUpdate(update) = &event.msg - && required_mcp_servers.contains(&update.server) - && let codex_protocol::protocol::McpStartupStatus::Failed { error } = &update.status - { - error_seen = true; - eprintln!( - "Required MCP server '{}' failed to initialize: {error}", - update.server - ); - if !shutdown_requested { - thread.submit(Op::Shutdown).await?; - shutdown_requested = true; + let primary_thread_id_for_requests = primary_thread_id.to_string(); + loop { + let server_event = if let Some(event) = buffered_events.pop_front() { + Some(event) + } else { + tokio::select! { + maybe_interrupt = interrupt_rx.recv() => { + if maybe_interrupt.is_none() { + continue; + } + if let Err(err) = send_request_with_response::( + &client, + ClientRequest::TurnInterrupt { + request_id: request_ids.next(), + params: TurnInterruptParams { + thread_id: primary_thread_id_for_requests.clone(), + turn_id: task_id.clone(), + }, + }, + "turn/interrupt", + ) + .await + { + warn!("turn/interrupt failed: {err}"); + } + continue; + } + maybe_event = client.next_event() => maybe_event, } - } - if thread_id != primary_thread_id && matches!(&event.msg, EventMsg::TurnComplete(_)) { - continue; - } - let shutdown = event_processor.process_event(event); - if thread_id != primary_thread_id && matches!(shutdown, CodexStatus::InitiateShutdown) { - continue; - } - match shutdown { - CodexStatus::Running => continue, - CodexStatus::InitiateShutdown => { - if !shutdown_requested { - thread.submit(Op::Shutdown).await?; - shutdown_requested = true; + }; + + let Some(server_event) = server_event else { + break; + }; + + match server_event { + InProcessServerEvent::ServerRequest(request) => { + handle_server_request( + &client, + request, + &config, + &primary_thread_id_for_requests, + &mut error_seen, + ) + .await; + } + InProcessServerEvent::ServerNotification(notification) => { + if let ServerNotification::Error(payload) = ¬ification + && payload.thread_id == primary_thread_id_for_requests + && payload.turn_id == task_id + && !payload.will_retry + { + error_seen = true; + } + } + InProcessServerEvent::LegacyNotification(notification) => { + let event = match legacy_notification_to_event(notification) { + Ok(event) => event, + Err(err) => { + warn!("{err}"); + continue; + } + }; + if matches!(event.msg, EventMsg::SessionConfigured(_)) { + continue; + } + match &event.msg { + EventMsg::TurnComplete(payload) => { + if payload.turn_id != task_id { + continue; + } + } + EventMsg::TurnAborted(payload) => { + if payload.turn_id.as_deref() != Some(task_id.as_str()) { + continue; + } + } + EventMsg::McpStartupUpdate(update) => { + if required_mcp_servers.contains(&update.server) + && let codex_protocol::protocol::McpStartupStatus::Failed { error } = + &update.status + { + error_seen = true; + eprintln!( + "Required MCP server '{}' failed to initialize: {error}", + update.server + ); + if let Err(err) = request_shutdown( + &client, + &mut request_ids, + &primary_thread_id_for_requests, + ) + .await + { + warn!("thread/unsubscribe failed during shutdown: {err}"); + } + break; + } + } + _ => {} + } + + match event_processor.process_event(event) { + CodexStatus::Running => {} + CodexStatus::InitiateShutdown => { + if let Err(err) = request_shutdown( + &client, + &mut request_ids, + &primary_thread_id_for_requests, + ) + .await + { + warn!("thread/unsubscribe failed during shutdown: {err}"); + } + break; + } + CodexStatus::Shutdown => break, } } - CodexStatus::Shutdown if thread_id == primary_thread_id => break, - CodexStatus::Shutdown => continue, + InProcessServerEvent::Lagged { skipped } => { + warn!("in-process app-server event stream lagged; dropped {skipped} events"); + } } } + + if let Err(err) = client.shutdown().await { + warn!("in-process app-server shutdown failed: {err}"); + } event_processor.print_final_output(); if error_seen { std::process::exit(1); @@ -737,66 +882,506 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> { Ok(()) } -fn spawn_thread_listener( - thread_id: codex_protocol::ThreadId, - thread: Arc, - tx: tokio::sync::mpsc::UnboundedSender, - suppress_output: bool, -) { - tokio::spawn(async move { +fn sandbox_mode_from_policy( + sandbox_policy: &codex_protocol::protocol::SandboxPolicy, +) -> Option { + match sandbox_policy { + codex_protocol::protocol::SandboxPolicy::DangerFullAccess => { + Some(codex_app_server_protocol::SandboxMode::DangerFullAccess) + } + codex_protocol::protocol::SandboxPolicy::ReadOnly { .. } => { + Some(codex_app_server_protocol::SandboxMode::ReadOnly) + } + codex_protocol::protocol::SandboxPolicy::WorkspaceWrite { .. } => { + Some(codex_app_server_protocol::SandboxMode::WorkspaceWrite) + } + codex_protocol::protocol::SandboxPolicy::ExternalSandbox { .. } => None, + } +} + +fn thread_start_params_from_config(config: &Config) -> ThreadStartParams { + ThreadStartParams { + model: config.model.clone(), + model_provider: Some(config.model_provider_id.clone()), + cwd: Some(config.cwd.to_string_lossy().to_string()), + approval_policy: Some(config.permissions.approval_policy.value().into()), + sandbox: sandbox_mode_from_policy(&config.permissions.sandbox_policy.get()), + ephemeral: Some(config.ephemeral), + ..ThreadStartParams::default() + } +} + +fn thread_resume_params_from_config(config: &Config, path: Option) -> ThreadResumeParams { + ThreadResumeParams { + thread_id: "resume".to_string(), + path, + model: config.model.clone(), + model_provider: Some(config.model_provider_id.clone()), + cwd: Some(config.cwd.to_string_lossy().to_string()), + approval_policy: Some(config.permissions.approval_policy.value().into()), + sandbox: sandbox_mode_from_policy(&config.permissions.sandbox_policy.get()), + ..ThreadResumeParams::default() + } +} + +async fn send_request_with_response( + client: &InProcessAppServerClient, + request: ClientRequest, + method: &str, +) -> Result +where + T: serde::de::DeserializeOwned, +{ + client.request_typed(request).await.map_err(|err| { + if method.is_empty() { + err.to_string() + } else { + format!("{method}: {err}") + } + }) +} + +async fn await_session_configured_event( + client: &mut InProcessAppServerClient, + config: &Config, + thread_id: &str, + error_seen: &mut bool, + buffered_events: &mut VecDeque, +) -> Result { + const SESSION_CONFIGURED_TIMEOUT: Duration = Duration::from_secs(10); + let wait_result = timeout(SESSION_CONFIGURED_TIMEOUT, async { loop { - match thread.next_event().await { - Ok(event) => { - debug!("Received event: {event:?}"); - - let is_shutdown_complete = matches!(event.msg, EventMsg::ShutdownComplete); - if let Err(err) = tx.send(ThreadEventEnvelope { - thread_id, - thread: Arc::clone(&thread), - event, - suppress_output, - }) { - error!("Error sending event: {err:?}"); - break; - } - if is_shutdown_complete { - info!( - "Received shutdown event for thread {thread_id}, exiting event loop." - ); - break; + let server_event = client.next_event().await; + let Some(server_event) = server_event else { + return Err("event stream ended before session configured".to_string()); + }; + + match &server_event { + InProcessServerEvent::ServerRequest(request) => { + handle_server_request(client, request.clone(), config, thread_id, error_seen) + .await; + } + InProcessServerEvent::ServerNotification(_) => { + buffered_events.push_back(server_event); + } + InProcessServerEvent::LegacyNotification(notification) => { + let event = match legacy_notification_to_event(notification.clone()) { + Ok(event) => event, + Err(err) => { + warn!("{err}"); + buffered_events.push_back(server_event); + continue; + } + }; + + if let EventMsg::SessionConfigured(session_configured) = event.msg + && session_configured.session_id.to_string() == thread_id + { + return Ok(session_configured); } + buffered_events.push_back(server_event); } - Err(err) => { - error!("Error receiving event: {err:?}"); - break; + InProcessServerEvent::Lagged { .. } => { + buffered_events.push_back(server_event); } } } - }); + }) + .await; + + match wait_result { + Ok(result) => result, + Err(_) => Err(format!( + "timed out waiting for session configured event after {}s", + SESSION_CONFIGURED_TIMEOUT.as_secs() + )), + } +} + +fn session_configured_from_thread_start_response( + response: &ThreadStartResponse, +) -> Result { + session_configured_from_thread_response( + &response.thread.id, + response.thread.name.clone(), + response.thread.path.clone(), + response.model.clone(), + response.model_provider.clone(), + response.service_tier.clone(), + response.approval_policy.to_core(), + response.sandbox.to_core(), + response.cwd.clone(), + response.reasoning_effort, + ) +} + +fn session_configured_from_thread_resume_response( + response: &ThreadResumeResponse, +) -> Result { + session_configured_from_thread_response( + &response.thread.id, + response.thread.name.clone(), + response.thread.path.clone(), + response.model.clone(), + response.model_provider.clone(), + response.service_tier.clone(), + response.approval_policy.to_core(), + response.sandbox.to_core(), + response.cwd.clone(), + response.reasoning_effort, + ) +} + +#[expect( + clippy::too_many_arguments, + reason = "session mapping keeps explicit fields" +)] +fn session_configured_from_thread_response( + thread_id: &str, + thread_name: Option, + rollout_path: Option, + model: String, + model_provider_id: String, + service_tier: Option, + approval_policy: AskForApproval, + sandbox_policy: codex_protocol::protocol::SandboxPolicy, + cwd: PathBuf, + reasoning_effort: Option, +) -> Result { + let session_id = codex_protocol::ThreadId::from_string(thread_id) + .map_err(|err| format!("thread id `{thread_id}` is invalid: {err}"))?; + + Ok(SessionConfiguredEvent { + session_id, + forked_from_id: None, + thread_name, + model, + model_provider_id, + service_tier, + approval_policy, + sandbox_policy, + cwd, + reasoning_effort, + history_log_id: 0, + history_entry_count: 0, + initial_messages: None, + network_proxy: None, + rollout_path, + }) } -fn is_agent_job_subagent(config: &codex_core::ThreadConfigSnapshot) -> bool { - match &config.session_source { - SessionSource::SubAgent(SubAgentSource::Other(source)) => source.starts_with("agent_job:"), - _ => false, +fn review_target_to_api(target: ReviewTarget) -> ApiReviewTarget { + match target { + ReviewTarget::UncommittedChanges => ApiReviewTarget::UncommittedChanges, + ReviewTarget::BaseBranch { branch } => ApiReviewTarget::BaseBranch { branch }, + ReviewTarget::Commit { sha, title } => ApiReviewTarget::Commit { sha, title }, + ReviewTarget::Custom { instructions } => ApiReviewTarget::Custom { instructions }, } } -fn should_suppress_agent_job_event(msg: &EventMsg) -> bool { - !matches!( +fn normalize_legacy_notification_method(method: &str) -> &str { + method.strip_prefix("codex/event/").unwrap_or(method) +} + +fn legacy_notification_to_event(notification: JSONRPCNotification) -> Result { + let value = notification + .params + .unwrap_or_else(|| serde_json::Value::Object(serde_json::Map::new())); + let method = notification.method; + let normalized_method = normalize_legacy_notification_method(&method).to_string(); + let serde_json::Value::Object(object) = value else { + return Err(format!( + "legacy notification `{method}` params were not an object" + )); + }; + let mut event_payload = if let Some(serde_json::Value::Object(msg_payload)) = object.get("msg") + { + serde_json::Value::Object(msg_payload.clone()) + } else { + serde_json::Value::Object(object) + }; + let serde_json::Value::Object(ref mut object) = event_payload else { + return Err(format!( + "legacy notification `{method}` event payload was not an object" + )); + }; + object.insert( + "type".to_string(), + serde_json::Value::String(normalized_method), + ); + + let msg: EventMsg = serde_json::from_value(event_payload) + .map_err(|err| format!("failed to decode event: {err}"))?; + Ok(Event { + id: String::new(), msg, - EventMsg::ExecApprovalRequest(_) - | EventMsg::ApplyPatchApprovalRequest(_) - | EventMsg::RequestUserInput(_) - | EventMsg::DynamicToolCallRequest(_) - | EventMsg::DynamicToolCallResponse(_) - | EventMsg::ElicitationRequest(_) - | EventMsg::Error(_) - | EventMsg::Warning(_) - | EventMsg::DeprecationNotice(_) - | EventMsg::StreamError(_) - | EventMsg::ShutdownComplete - ) + }) +} + +async fn request_shutdown( + client: &InProcessAppServerClient, + request_ids: &mut RequestIdSequencer, + thread_id: &str, +) -> Result<(), String> { + let request = ClientRequest::ThreadUnsubscribe { + request_id: request_ids.next(), + params: ThreadUnsubscribeParams { + thread_id: thread_id.to_string(), + }, + }; + send_request_with_response::(client, request, "thread/unsubscribe") + .await + .map(|_| ()) +} + +async fn resolve_server_request( + client: &InProcessAppServerClient, + request_id: RequestId, + value: serde_json::Value, + method: &str, +) -> Result<(), String> { + client + .resolve_server_request(request_id, value) + .await + .map_err(|err| format!("failed to resolve `{method}` server request: {err}")) +} + +async fn reject_server_request( + client: &InProcessAppServerClient, + request_id: RequestId, + method: &str, + reason: String, +) -> Result<(), String> { + client + .reject_server_request( + request_id, + JSONRPCErrorError { + code: -32000, + message: reason, + data: None, + }, + ) + .await + .map_err(|err| format!("failed to reject `{method}` server request: {err}")) +} + +fn server_request_method_name(request: &ServerRequest) -> String { + serde_json::to_value(request) + .ok() + .and_then(|value| { + value + .get("method") + .and_then(serde_json::Value::as_str) + .map(str::to_owned) + }) + .unwrap_or_else(|| "unknown".to_string()) +} + +async fn handle_server_request( + client: &InProcessAppServerClient, + request: ServerRequest, + config: &Config, + thread_id: &str, + error_seen: &mut bool, +) { + let method = server_request_method_name(&request); + let handle_result = match request { + ServerRequest::McpServerElicitationRequest { request_id, params } => { + if params.thread_id != thread_id { + reject_server_request( + client, + request_id, + &method, + format!( + "request targets thread `{}`, but active thread is `{thread_id}`", + params.thread_id + ), + ) + .await + } else { + let response = McpServerElicitationRequestResponse { + action: McpServerElicitationAction::Cancel, + content: None, + }; + match serde_json::to_value(response) { + Ok(value) => { + resolve_server_request( + client, + request_id, + value, + "mcpServer/elicitation/request", + ) + .await + } + Err(err) => Err(format!("failed to encode mcp elicitation response: {err}")), + } + } + } + ServerRequest::ChatgptAuthTokensRefresh { request_id, params } => { + let refresh_result = tokio::task::spawn_blocking({ + let config = config.clone(); + move || local_external_chatgpt_tokens(&config) + }) + .await; + + match refresh_result { + Err(err) => { + reject_server_request( + client, + request_id, + &method, + format!("local chatgpt auth refresh task failed in exec: {err}"), + ) + .await + } + Ok(Err(reason)) => reject_server_request(client, request_id, &method, reason).await, + Ok(Ok(response)) => { + if let Some(previous_account_id) = params.previous_account_id.as_deref() + && previous_account_id != response.chatgpt_account_id + { + warn!( + "local auth refresh account mismatch: expected `{previous_account_id}`, got `{}`", + response.chatgpt_account_id + ); + } + match serde_json::to_value(response) { + Ok(value) => { + resolve_server_request( + client, + request_id, + value, + "account/chatgptAuthTokens/refresh", + ) + .await + } + Err(err) => Err(format!( + "failed to serialize chatgpt auth refresh response: {err}" + )), + } + } + } + } + ServerRequest::CommandExecutionRequestApproval { request_id, params } => { + reject_server_request( + client, + request_id, + &method, + format!( + "command execution approval is not supported in exec mode for thread `{}`", + params.thread_id + ), + ) + .await + } + ServerRequest::FileChangeRequestApproval { request_id, params } => { + reject_server_request( + client, + request_id, + &method, + format!( + "file change approval is not supported in exec mode for thread `{}`", + params.thread_id + ), + ) + .await + } + ServerRequest::ToolRequestUserInput { request_id, params } => { + reject_server_request( + client, + request_id, + &method, + format!( + "request_user_input is not supported in exec mode for thread `{}`", + params.thread_id + ), + ) + .await + } + ServerRequest::DynamicToolCall { request_id, params } => { + reject_server_request( + client, + request_id, + &method, + format!( + "dynamic tool calls are not supported in exec mode for thread `{}`", + params.thread_id + ), + ) + .await + } + ServerRequest::ApplyPatchApproval { request_id, params } => { + reject_server_request( + client, + request_id, + &method, + format!( + "apply_patch approval is not supported in exec mode for thread `{}`", + params.conversation_id + ), + ) + .await + } + ServerRequest::ExecCommandApproval { request_id, params } => { + reject_server_request( + client, + request_id, + &method, + format!( + "exec command approval is not supported in exec mode for thread `{}`", + params.conversation_id + ), + ) + .await + } + }; + + if let Err(err) = handle_result { + *error_seen = true; + warn!("{err}"); + } +} + +fn local_external_chatgpt_tokens( + config: &Config, +) -> Result { + let auth_manager = AuthManager::shared( + config.codex_home.clone(), + false, + config.cli_auth_credentials_store_mode, + ); + auth_manager.set_forced_chatgpt_workspace_id(config.forced_chatgpt_workspace_id.clone()); + auth_manager.reload(); + + let auth = auth_manager + .auth_cached() + .ok_or_else(|| "no cached auth available for local token refresh".to_string())?; + if !auth.is_external_chatgpt_tokens() { + return Err("external ChatGPT token auth is not active".to_string()); + } + + let access_token = auth + .get_token() + .map_err(|err| format!("failed to read external access token: {err}"))?; + let chatgpt_account_id = auth + .get_account_id() + .ok_or_else(|| "external token auth is missing chatgpt account id".to_string())?; + let chatgpt_plan_type = auth.account_plan_type().map(|plan_type| match plan_type { + AccountPlanType::Free => "free".to_string(), + AccountPlanType::Go => "go".to_string(), + AccountPlanType::Plus => "plus".to_string(), + AccountPlanType::Pro => "pro".to_string(), + AccountPlanType::Team => "team".to_string(), + AccountPlanType::Business => "business".to_string(), + AccountPlanType::Enterprise => "enterprise".to_string(), + AccountPlanType::Edu => "edu".to_string(), + AccountPlanType::Unknown => "unknown".to_string(), + }); + + Ok(ChatgptAuthTokensRefreshResponse { + access_token, + chatgpt_account_id, + chatgpt_plan_type, + }) } async fn resolve_resume_path( @@ -980,17 +1565,17 @@ fn resolve_prompt(prompt_arg: Option) -> String { } } -fn build_review_request(args: ReviewArgs) -> anyhow::Result { +fn build_review_request(args: &ReviewArgs) -> anyhow::Result { let target = if args.uncommitted { ReviewTarget::UncommittedChanges - } else if let Some(branch) = args.base { + } else if let Some(branch) = args.base.clone() { ReviewTarget::BaseBranch { branch } - } else if let Some(sha) = args.commit { + } else if let Some(sha) = args.commit.clone() { ReviewTarget::Commit { sha, - title: args.commit_title, + title: args.commit_title.clone(), } - } else if let Some(prompt_arg) = args.prompt { + } else if let Some(prompt_arg) = args.prompt.clone() { let prompt = resolve_prompt(Some(prompt_arg)).trim().to_string(); if prompt.is_empty() { anyhow::bail!("Review prompt cannot be empty"); @@ -1053,14 +1638,14 @@ mod tests { #[test] fn builds_uncommitted_review_request() { - let request = build_review_request(ReviewArgs { + let args = ReviewArgs { uncommitted: true, base: None, commit: None, commit_title: None, prompt: None, - }) - .expect("builds uncommitted review request"); + }; + let request = build_review_request(&args).expect("builds uncommitted review request"); let expected = ReviewRequest { target: ReviewTarget::UncommittedChanges, @@ -1072,14 +1657,14 @@ mod tests { #[test] fn builds_commit_review_request_with_title() { - let request = build_review_request(ReviewArgs { + let args = ReviewArgs { uncommitted: false, base: None, commit: Some("123456789".to_string()), commit_title: Some("Add review command".to_string()), prompt: None, - }) - .expect("builds commit review request"); + }; + let request = build_review_request(&args).expect("builds commit review request"); let expected = ReviewRequest { target: ReviewTarget::Commit { @@ -1094,14 +1679,14 @@ mod tests { #[test] fn builds_custom_review_request_trims_prompt() { - let request = build_review_request(ReviewArgs { + let args = ReviewArgs { uncommitted: false, base: None, commit: None, commit_title: None, prompt: Some(" custom review instructions ".to_string()), - }) - .expect("builds custom review request"); + }; + let request = build_review_request(&args).expect("builds custom review request"); let expected = ReviewRequest { target: ReviewTarget::Custom { From 0b781413ca99c76bf0b014c093462bf603205c0a Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Thu, 5 Mar 2026 17:03:01 -0300 Subject: [PATCH 03/25] refactor(core): restore protocol parity for in-process clients Keep `ClientRequest` request-id extraction local to `app-server` instead of extending `codex-app-server-protocol`, and remove the extra `JSONRPCErrorError` trait impls that were only needed by that drift. Add focused regression coverage for in-process lag/bootstrap handling in `codex-app-server-client` and `codex-exec` so the client contract stays explicit while protocol parity with `main` is preserved. --- codex-rs/app-server-client/src/lib.rs | 34 +++- .../app-server-protocol/src/jsonrpc_lite.rs | 11 -- .../src/protocol/common.rs | 9 -- codex-rs/app-server/src/app_server_tracing.rs | 65 +++++++- codex-rs/app-server/src/in_process.rs | 3 +- codex-rs/app-server/src/message_processor.rs | 2 +- codex-rs/exec/src/lib.rs | 146 +++++++++++++++--- 7 files changed, 220 insertions(+), 50 deletions(-) diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index 3152710e8563..29082b416d2a 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -89,7 +89,7 @@ impl Error for TypedRequestError { fn source(&self) -> Option<&(dyn Error + 'static)> { match self { Self::Transport { source, .. } => Some(source), - Self::Server { source, .. } => Some(source), + Self::Server { .. } => None, Self::Deserialize { source, .. } => Some(source), } } @@ -535,6 +535,8 @@ mod tests { use codex_app_server_protocol::ThreadStartResponse; use codex_core::config::ConfigBuilder; use pretty_assertions::assert_eq; + use tokio::time::Duration; + use tokio::time::timeout; async fn build_test_config() -> Config { match ConfigBuilder::default().build().await { @@ -655,7 +657,7 @@ mod tests { message: "internal".to_string(), }, }; - assert_eq!(std::error::Error::source(&server).is_some(), true); + assert_eq!(std::error::Error::source(&server).is_some(), false); let deserialize = TypedRequestError::Deserialize { method: "thread/start".to_string(), @@ -664,4 +666,32 @@ mod tests { }; assert_eq!(std::error::Error::source(&deserialize).is_some(), true); } + + #[tokio::test] + async fn next_event_surfaces_lagged_markers() { + let (command_tx, _command_rx) = mpsc::channel(1); + let (event_tx, event_rx) = mpsc::channel(1); + let worker_handle = tokio::spawn(async {}); + event_tx + .send(InProcessServerEvent::Lagged { skipped: 3 }) + .await + .expect("lagged marker should enqueue"); + drop(event_tx); + + let mut client = InProcessAppServerClient { + command_tx, + event_rx, + worker_handle, + }; + + let event = timeout(Duration::from_secs(2), client.next_event()) + .await + .expect("lagged marker should arrive before timeout"); + assert!(matches!( + event, + Some(InProcessServerEvent::Lagged { skipped: 3 }) + )); + + client.shutdown().await.expect("shutdown should complete"); + } } diff --git a/codex-rs/app-server-protocol/src/jsonrpc_lite.rs b/codex-rs/app-server-protocol/src/jsonrpc_lite.rs index 7d4ba63fbaf3..13d3e0cc9ac5 100644 --- a/codex-rs/app-server-protocol/src/jsonrpc_lite.rs +++ b/codex-rs/app-server-protocol/src/jsonrpc_lite.rs @@ -1,9 +1,6 @@ //! We do not do true JSON-RPC 2.0, as we neither send nor expect the //! "jsonrpc": "2.0" field. -use std::error::Error; -use std::fmt; - use codex_protocol::protocol::W3cTraceContext; use schemars::JsonSchema; use serde::Deserialize; @@ -79,11 +76,3 @@ pub struct JSONRPCErrorError { pub data: Option, pub message: String, } - -impl fmt::Display for JSONRPCErrorError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{} (code {})", self.message, self.code) - } -} - -impl Error for JSONRPCErrorError {} diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index 8d6a27732308..54c0425256c2 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -110,15 +110,6 @@ macro_rules! client_request_definitions { )* } - impl ClientRequest { - /// Returns the request id regardless of method variant. - pub fn id(&self) -> &RequestId { - match self { - $(Self::$variant { request_id, .. } => request_id,)* - } - } - } - impl crate::experimental_api::ExperimentalApi for ClientRequest { fn experimental_reason(&self) -> Option<&'static str> { match self { diff --git a/codex-rs/app-server/src/app_server_tracing.rs b/codex-rs/app-server/src/app_server_tracing.rs index 4a2de97a4a11..06b63c8a6172 100644 --- a/codex-rs/app-server/src/app_server_tracing.rs +++ b/codex-rs/app-server/src/app_server_tracing.rs @@ -4,6 +4,7 @@ use crate::transport::AppServerTransport; use codex_app_server_protocol::ClientRequest; use codex_app_server_protocol::InitializeParams; use codex_app_server_protocol::JSONRPCRequest; +use codex_app_server_protocol::RequestId; use codex_otel::set_parent_from_context; use codex_otel::set_parent_from_w3c_trace_context; use codex_otel::traceparent_context_from_env; @@ -83,7 +84,7 @@ pub(crate) fn typed_request_span( rpc.system = "jsonrpc", rpc.method = "in_process", rpc.transport = "in-process", - rpc.request_id = ?request.id(), + rpc.request_id = ?client_request_id(request), app_server.connection_id = ?connection_id, app_server.api_version = "v2", app_server.client_name = field::Empty, @@ -154,3 +155,65 @@ fn initialize_client_info_from_typed_request(request: &ClientRequest) -> Option< _ => None, } } + +pub(crate) fn client_request_id(request: &ClientRequest) -> &RequestId { + match request { + ClientRequest::Initialize { request_id, .. } + | ClientRequest::ThreadStart { request_id, .. } + | ClientRequest::ThreadResume { request_id, .. } + | ClientRequest::ThreadFork { request_id, .. } + | ClientRequest::ThreadArchive { request_id, .. } + | ClientRequest::ThreadUnsubscribe { request_id, .. } + | ClientRequest::ThreadSetName { request_id, .. } + | ClientRequest::ThreadMetadataUpdate { request_id, .. } + | ClientRequest::ThreadUnarchive { request_id, .. } + | ClientRequest::ThreadCompactStart { request_id, .. } + | ClientRequest::ThreadBackgroundTerminalsClean { request_id, .. } + | ClientRequest::ThreadRollback { request_id, .. } + | ClientRequest::ThreadList { request_id, .. } + | ClientRequest::ThreadLoadedList { request_id, .. } + | ClientRequest::ThreadRead { request_id, .. } + | ClientRequest::SkillsList { request_id, .. } + | ClientRequest::SkillsRemoteList { request_id, .. } + | ClientRequest::SkillsRemoteExport { request_id, .. } + | ClientRequest::AppsList { request_id, .. } + | ClientRequest::SkillsConfigWrite { request_id, .. } + | ClientRequest::PluginInstall { request_id, .. } + | ClientRequest::TurnStart { request_id, .. } + | ClientRequest::TurnSteer { request_id, .. } + | ClientRequest::TurnInterrupt { request_id, .. } + | ClientRequest::ThreadRealtimeStart { request_id, .. } + | ClientRequest::ThreadRealtimeAppendAudio { request_id, .. } + | ClientRequest::ThreadRealtimeAppendText { request_id, .. } + | ClientRequest::ThreadRealtimeStop { request_id, .. } + | ClientRequest::ReviewStart { request_id, .. } + | ClientRequest::ModelList { request_id, .. } + | ClientRequest::ExperimentalFeatureList { request_id, .. } + | ClientRequest::CollaborationModeList { request_id, .. } + | ClientRequest::MockExperimentalMethod { request_id, .. } + | ClientRequest::McpServerOauthLogin { request_id, .. } + | ClientRequest::McpServerRefresh { request_id, .. } + | ClientRequest::McpServerStatusList { request_id, .. } + | ClientRequest::WindowsSandboxSetupStart { request_id, .. } + | ClientRequest::LoginAccount { request_id, .. } + | ClientRequest::CancelLoginAccount { request_id, .. } + | ClientRequest::LogoutAccount { request_id, .. } + | ClientRequest::GetAccountRateLimits { request_id, .. } + | ClientRequest::FeedbackUpload { request_id, .. } + | ClientRequest::OneOffCommandExec { request_id, .. } + | ClientRequest::ConfigRead { request_id, .. } + | ClientRequest::ExternalAgentConfigDetect { request_id, .. } + | ClientRequest::ExternalAgentConfigImport { request_id, .. } + | ClientRequest::ConfigValueWrite { request_id, .. } + | ClientRequest::ConfigBatchWrite { request_id, .. } + | ClientRequest::ConfigRequirementsRead { request_id, .. } + | ClientRequest::GetAccount { request_id, .. } + | ClientRequest::GetConversationSummary { request_id, .. } + | ClientRequest::GitDiffToRemote { request_id, .. } + | ClientRequest::GetAuthStatus { request_id, .. } + | ClientRequest::FuzzyFileSearch { request_id, .. } + | ClientRequest::FuzzyFileSearchSessionStart { request_id, .. } + | ClientRequest::FuzzyFileSearchSessionUpdate { request_id, .. } + | ClientRequest::FuzzyFileSearchSessionStop { request_id, .. } => request_id, + } +} diff --git a/codex-rs/app-server/src/in_process.rs b/codex-rs/app-server/src/in_process.rs index 7686fd03edad..d51d9405ac26 100644 --- a/codex-rs/app-server/src/in_process.rs +++ b/codex-rs/app-server/src/in_process.rs @@ -325,7 +325,8 @@ fn start_uninitialized(args: InProcessStartArgs) -> InProcessClientHandle { match message { Some(InProcessClientMessage::Request { request, response_tx }) => { let request = *request; - let request_id = request.id().clone(); + let request_id = + crate::app_server_tracing::client_request_id(&request).clone(); match pending_request_responses.entry(request_id.clone()) { Entry::Vacant(entry) => { entry.insert(response_tx); diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index f7186744982b..24c9fe4f2fc6 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -307,7 +307,7 @@ impl MessageProcessor { async { let request_id = ConnectionRequestId { connection_id, - request_id: request.id().clone(), + request_id: crate::app_server_tracing::client_request_id(&request).clone(), }; tracing::trace!( ?connection_id, diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index f678ece85963..b27f5431c6e7 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -956,33 +956,16 @@ async fn await_session_configured_event( return Err("event stream ended before session configured".to_string()); }; - match &server_event { - InProcessServerEvent::ServerRequest(request) => { - handle_server_request(client, request.clone(), config, thread_id, error_seen) - .await; + match classify_bootstrap_event(server_event, thread_id, buffered_events) { + Ok(BootstrapEventAction::HandleServerRequest(request)) => { + handle_server_request(client, request, config, thread_id, error_seen).await; } - InProcessServerEvent::ServerNotification(_) => { - buffered_events.push_back(server_event); - } - InProcessServerEvent::LegacyNotification(notification) => { - let event = match legacy_notification_to_event(notification.clone()) { - Ok(event) => event, - Err(err) => { - warn!("{err}"); - buffered_events.push_back(server_event); - continue; - } - }; - - if let EventMsg::SessionConfigured(session_configured) = event.msg - && session_configured.session_id.to_string() == thread_id - { - return Ok(session_configured); - } - buffered_events.push_back(server_event); + Ok(BootstrapEventAction::SessionConfigured(session_configured)) => { + return Ok(session_configured); } - InProcessServerEvent::Lagged { .. } => { - buffered_events.push_back(server_event); + Ok(BootstrapEventAction::Continue) => {} + Err(err) => { + warn!("{err}"); } } } @@ -998,6 +981,50 @@ async fn await_session_configured_event( } } +enum BootstrapEventAction { + Continue, + HandleServerRequest(ServerRequest), + SessionConfigured(SessionConfiguredEvent), +} + +fn classify_bootstrap_event( + server_event: InProcessServerEvent, + thread_id: &str, + buffered_events: &mut VecDeque, +) -> Result { + match &server_event { + InProcessServerEvent::ServerRequest(_) => { + if let InProcessServerEvent::ServerRequest(request) = server_event { + Ok(BootstrapEventAction::HandleServerRequest(request)) + } else { + unreachable!("matched server request variant") + } + } + InProcessServerEvent::ServerNotification(_) | InProcessServerEvent::Lagged { .. } => { + buffered_events.push_back(server_event); + Ok(BootstrapEventAction::Continue) + } + InProcessServerEvent::LegacyNotification(notification) => { + let event = legacy_notification_to_event(notification.clone()); + match event { + Ok(event) => { + if let EventMsg::SessionConfigured(session_configured) = event.msg + && session_configured.session_id.to_string() == thread_id + { + return Ok(BootstrapEventAction::SessionConfigured(session_configured)); + } + buffered_events.push_back(server_event); + Ok(BootstrapEventAction::Continue) + } + Err(err) => { + buffered_events.push_back(server_event); + Err(err) + } + } + } + } +} + fn session_configured_from_thread_start_response( response: &ThreadStartResponse, ) -> Result { @@ -1598,12 +1625,14 @@ fn build_review_request(args: &ReviewArgs) -> anyhow::Result { #[cfg(test)] mod tests { use super::*; + use codex_app_server_protocol::JSONRPCNotification; use codex_otel::set_parent_from_w3c_trace_context; use opentelemetry::trace::TraceContextExt; use opentelemetry::trace::TraceId; use opentelemetry::trace::TracerProvider as _; use opentelemetry_sdk::trace::SdkTracerProvider; use pretty_assertions::assert_eq; + use serde_json::json; use tracing_opentelemetry::OpenTelemetrySpanExt; fn test_tracing_subscriber() -> impl tracing::Subscriber + Send + Sync { @@ -1772,4 +1801,71 @@ mod tests { assert_eq!(err, PromptDecodeError::InvalidUtf8 { valid_up_to: 0 }); } + + #[test] + fn classify_bootstrap_event_buffers_lagged_markers() { + let mut buffered_events = VecDeque::new(); + + let action = classify_bootstrap_event( + InProcessServerEvent::Lagged { skipped: 3 }, + "thread-1", + &mut buffered_events, + ) + .expect("lagged events should be buffered"); + + assert!(matches!(action, BootstrapEventAction::Continue)); + assert_eq!(buffered_events.len(), 1); + assert!(matches!( + buffered_events.front(), + Some(InProcessServerEvent::Lagged { skipped: 3 }) + )); + } + + #[test] + fn classify_bootstrap_event_extracts_matching_session_configured() { + let mut buffered_events = VecDeque::new(); + let thread_id = "019cbf93-9ff5-7ac0-ac93-c8a36f0c98d3"; + let session_configured = session_configured_from_thread_response( + thread_id, + Some("thread".to_string()), + None, + "gpt-5".to_string(), + "openai".to_string(), + None, + AskForApproval::Never, + codex_protocol::protocol::SandboxPolicy::DangerFullAccess, + std::env::temp_dir(), + None, + ) + .expect("session configured payload should build"); + let notification = JSONRPCNotification { + method: "codex/event/session_configured".to_string(), + params: Some(json!({ + "msg": serde_json::to_value(&session_configured) + .expect("session configured should serialize"), + })), + }; + + let action = classify_bootstrap_event( + InProcessServerEvent::LegacyNotification(notification), + thread_id, + &mut buffered_events, + ) + .expect("matching session configured should decode"); + + match action { + BootstrapEventAction::SessionConfigured(payload) => { + assert_eq!( + payload.session_id.to_string(), + session_configured.session_id.to_string() + ); + assert_eq!(payload.thread_name, session_configured.thread_name); + assert_eq!(payload.model, session_configured.model); + } + BootstrapEventAction::Continue | BootstrapEventAction::HandleServerRequest(_) => { + panic!("expected matching session configured event") + } + } + assert!(buffered_events.is_empty()); + } } From b2fdde9dd6a833bd2242b30161709f20fe404a2d Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 8 Mar 2026 15:11:20 -0600 Subject: [PATCH 04/25] docs(core): clarify in-process app-server contracts --- codex-rs/app-server-client/README.md | 22 +++++++++++++ codex-rs/app-server-client/src/lib.rs | 32 +++++++++++++++++++ codex-rs/app-server/src/app_server_tracing.rs | 10 ++++++ codex-rs/app-server/src/in_process.rs | 18 +++++++++++ codex-rs/exec/src/lib.rs | 21 ++++++++++++ 5 files changed, 103 insertions(+) diff --git a/codex-rs/app-server-client/README.md b/codex-rs/app-server-client/README.md index ad8e7b93328f..aa1c702ce96b 100644 --- a/codex-rs/app-server-client/README.md +++ b/codex-rs/app-server-client/README.md @@ -39,9 +39,31 @@ The in-process path uses typed channels: JSON serialization is still used at external transport boundaries (stdio/websocket), but the in-process hot path is typed. +Typed requests still receive app-server responses through the JSON-RPC +result envelope internally. That is intentional: the in-process path is +meant to preserve app-server semantics while removing the process +boundary, not to introduce a second response contract. + +## Bootstrap behavior + +The client facade starts an already-initialized in-process runtime, but +thread bootstrap still follows normal app-server flow: + +- caller sends `thread/start` or `thread/resume` +- app-server returns the immediate typed response +- richer session metadata may arrive later as a `SessionConfigured` + legacy event + +Surfaces such as TUI and exec may therefore need a short bootstrap +phase where they reconcile startup response data with later events. + ## Backpressure and shutdown - Queues are bounded and use `DEFAULT_IN_PROCESS_CHANNEL_CAPACITY` by default. - Full queues return explicit overload behavior instead of unbounded growth. - `shutdown()` performs a bounded graceful shutdown and then aborts if timeout is exceeded. + +If the client falls behind on event consumption, the worker emits +`InProcessServerEvent::Lagged` and may reject pending server requests so +approval flows do not hang indefinitely behind a saturated queue. diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index 29082b416d2a..d499e3a9d58e 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -46,6 +46,11 @@ use tracing::warn; const SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(5); +/// Raw app-server request result for typed in-process requests. +/// +/// Even on the in-process path, successful responses still travel back through +/// the same JSON-RPC result envelope used by socket/stdio transports because +/// `MessageProcessor` continues to produce that shape internally. pub type RequestResult = std::result::Result; /// Layered error for [`InProcessAppServerClient::request_typed`]. @@ -104,6 +109,9 @@ pub enum ClientSurface { } /// Maps facade surface identity to app-server `SessionSource`. +/// +/// `ClientSurface::Tui` intentionally maps to `SessionSource::Cli` because the +/// TUI is the interactive CLI surface from the server's perspective. pub fn session_source_for_surface(surface: ClientSurface) -> SessionSource { match surface { ClientSurface::Exec => SessionSource::Exec, @@ -152,6 +160,10 @@ pub struct InProcessClientStartArgs { impl InProcessClientStartArgs { /// Builds initialize params from surface and caller-provided metadata. + /// + /// This keeps the initialize handshake policy in one place so TUI and exec + /// do not drift on client naming, experimental opt-in, or notification + /// suppression. pub fn initialize_params(&self) -> InitializeParams { let client_name = self .client_name @@ -217,6 +229,17 @@ enum ClientCommand { }, } +/// Async facade over the in-process app-server runtime. +/// +/// This type owns a worker task that bridges between: +/// - caller-facing async `mpsc` channels used by TUI/exec +/// - [`codex_app_server::in_process::InProcessClientHandle`], which speaks to +/// the embedded `MessageProcessor` +/// +/// The facade intentionally preserves the server's request/notification/event +/// model instead of exposing direct core runtime handles. That keeps in-process +/// callers aligned with app-server behavior while still avoiding a process +/// boundary. pub struct InProcessAppServerClient { command_tx: mpsc::Sender, event_rx: mpsc::Receiver, @@ -377,6 +400,9 @@ impl InProcessAppServerClient { /// Sends a typed client request and decodes the successful response body. /// + /// This still deserializes from a JSON value produced by app-server's + /// JSON-RPC result envelope. Callers should treat `Deserialize` failures as + /// a protocol-shape mismatch rather than a transport problem. pub async fn request_typed(&self, request: ClientRequest) -> Result where T: DeserializeOwned, @@ -480,6 +506,10 @@ impl InProcessAppServerClient { } /// Returns the next in-process event, or `None` when worker exits. + /// + /// Callers are expected to drain this stream promptly. If they fall behind, + /// the worker emits [`InProcessServerEvent::Lagged`] markers and may reject + /// pending server requests rather than letting approval flows hang. pub async fn next_event(&mut self) -> Option { self.event_rx.recv().await } @@ -514,6 +544,8 @@ impl InProcessAppServerClient { } } +/// Extracts the JSON-RPC method name for diagnostics without extending the +/// protocol crate with in-process-only helpers. fn request_method_name(request: &ClientRequest) -> String { serde_json::to_value(request) .ok() diff --git a/codex-rs/app-server/src/app_server_tracing.rs b/codex-rs/app-server/src/app_server_tracing.rs index 06b63c8a6172..c55b00711bbe 100644 --- a/codex-rs/app-server/src/app_server_tracing.rs +++ b/codex-rs/app-server/src/app_server_tracing.rs @@ -1,3 +1,9 @@ +//! Tracing helpers shared by socket and in-process app-server entry points. +//! +//! The in-process path intentionally reuses the same span shape as JSON-RPC +//! transports so request telemetry stays comparable across stdio, websocket, +//! and embedded callers. + use crate::message_processor::ConnectionSessionState; use crate::outgoing_message::ConnectionId; use crate::transport::AppServerTransport; @@ -156,6 +162,10 @@ fn initialize_client_info_from_typed_request(request: &ClientRequest) -> Option< } } +/// Extracts the request id from a typed in-process request. +/// +/// This duplicates the `ClientRequest` variant match locally so the protocol +/// crate does not need an in-process-specific helper just for tracing. pub(crate) fn client_request_id(request: &ClientRequest) -> &RequestId { match request { ClientRequest::Initialize { request_id, .. } diff --git a/codex-rs/app-server/src/in_process.rs b/codex-rs/app-server/src/in_process.rs index d51d9405ac26..92f1dde0fa08 100644 --- a/codex-rs/app-server/src/in_process.rs +++ b/codex-rs/app-server/src/in_process.rs @@ -11,6 +11,12 @@ //! - interact through [`InProcessClientHandle`] request/notify/event methods //! - terminate with [`InProcessClientHandle::shutdown`] //! +//! The runtime is transport-local but not protocol-free. Incoming requests are +//! typed `ClientRequest` values, yet responses still come back through the same +//! JSON-RPC result envelope that `MessageProcessor` uses for stdio/websocket +//! transports. This keeps in-process behavior aligned with app-server rather +//! than creating a second execution contract. +//! //! Backpressure is explicit: command submission uses `try_send` and can return //! `WouldBlock`, while event fanout may drop notifications under saturation. //! Server requests are never silently abandoned: if they cannot be queued they @@ -73,6 +79,9 @@ pub const DEFAULT_IN_PROCESS_CHANNEL_CAPACITY: usize = CHANNEL_CAPACITY; type PendingClientRequestResponse = std::result::Result; /// Input needed to start an in-process app-server runtime. +/// +/// These fields mirror the pieces of ambient process state that stdio and +/// websocket transports normally assemble before `MessageProcessor` starts. #[derive(Clone)] pub struct InProcessStartArgs { /// Resolved argv0 dispatch paths used by command execution internals. @@ -98,6 +107,11 @@ pub struct InProcessStartArgs { } /// Event emitted from the app-server to the in-process client. +/// +/// The stream intentionally carries both typed app-server events and legacy +/// bridge notifications because some CLI surfaces still consume +/// `codex_protocol::Event` values today. `Lagged` is a transport health marker, +/// not an application event. #[derive(Debug, Clone)] pub enum InProcessServerEvent { /// Server request that requires client response/rejection. @@ -132,6 +146,10 @@ enum InProcessClientMessage { } /// Handle used by an in-process client to call app-server and consume events. +/// +/// This is the low-level runtime handle. Higher-level callers should usually go +/// through `codex-app-server-client`, which adds worker-task buffering, +/// request/response helpers, and surface-specific startup policy. pub struct InProcessClientHandle { client_tx: mpsc::Sender, event_rx: mpsc::Receiver, diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index b27f5431c6e7..9912f0e7bb62 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -941,6 +941,13 @@ where }) } +/// Waits for the authoritative bootstrap `SessionConfigured` event. +/// +/// `thread/start` and `thread/resume` provide enough data to synthesize a +/// session configuration, but exec prefers the streamed event when it arrives +/// in time because that payload is the closest match to main's startup data. +/// Non-bootstrap events are buffered so the main event loop can process them in +/// order once startup finishes. async fn await_session_configured_event( client: &mut InProcessAppServerClient, config: &Config, @@ -987,6 +994,14 @@ enum BootstrapEventAction { SessionConfigured(SessionConfiguredEvent), } +/// Classifies early events seen before exec has finished bootstrap. +/// +/// The bootstrap phase has two jobs: +/// - answer server requests that would otherwise stall startup +/// - capture the matching `SessionConfigured` event for the primary thread +/// +/// Everything else is buffered and replayed by the main loop so startup does +/// not reorder or discard early notifications. fn classify_bootstrap_event( server_event: InProcessServerEvent, thread_id: &str, @@ -1063,6 +1078,12 @@ fn session_configured_from_thread_resume_response( clippy::too_many_arguments, reason = "session mapping keeps explicit fields" )] +/// Synthesizes startup session metadata from `thread/start` or `thread/resume`. +/// +/// This is a compatibility bridge for the current in-process architecture. +/// Some session fields are not available synchronously from the start/resume +/// response, so callers must treat the result as a best-effort fallback until +/// a later `SessionConfigured` event proves otherwise. fn session_configured_from_thread_response( thread_id: &str, thread_name: Option, From eab69aa28265571393567b6cc32fb67154806fe8 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 8 Mar 2026 15:11:39 -0600 Subject: [PATCH 05/25] fix(exec): surface in-process lag warnings --- codex-rs/exec/src/lib.rs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 9912f0e7bb62..df92b01e3720 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -866,7 +866,12 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> { } } InProcessServerEvent::Lagged { skipped } => { - warn!("in-process app-server event stream lagged; dropped {skipped} events"); + let message = lagged_event_warning_message(skipped); + warn!("{message}"); + let _ = event_processor.process_event(Event { + id: String::new(), + msg: EventMsg::Warning(codex_protocol::protocol::WarningEvent { message }), + }); } } } @@ -1131,6 +1136,10 @@ fn normalize_legacy_notification_method(method: &str) -> &str { method.strip_prefix("codex/event/").unwrap_or(method) } +fn lagged_event_warning_message(skipped: usize) -> String { + format!("in-process app-server event stream lagged; dropped {skipped} events") +} + fn legacy_notification_to_event(notification: JSONRPCNotification) -> Result { let value = notification .params @@ -1889,4 +1898,12 @@ mod tests { } assert!(buffered_events.is_empty()); } + + #[test] + fn lagged_event_warning_message_is_explicit() { + assert_eq!( + lagged_event_warning_message(7), + "in-process app-server event stream lagged; dropped 7 events".to_string() + ); + } } From 39e0b6921ab11aa6a8e0466e4b6210c29cba7748 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 8 Mar 2026 15:11:46 -0600 Subject: [PATCH 06/25] docs(core): expand in-process architecture notes --- codex-rs/app-server-client/src/lib.rs | 24 +++++--- codex-rs/app-server/src/app_server_tracing.rs | 5 +- codex-rs/app-server/src/in_process.rs | 60 +++++++++++++------ 3 files changed, 62 insertions(+), 27 deletions(-) diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index d499e3a9d58e..a965cb20165c 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -1,15 +1,19 @@ //! Shared in-process app-server client facade for CLI surfaces. //! -//! This crate wraps `codex_app_server::in_process` behind a single async API +//! This crate wraps [`codex_app_server::in_process`] behind a single async API //! used by surfaces like TUI and exec. It centralizes: -//! - runtime startup and initialize capabilities -//! - surface-to-session-source policy -//! - request/notification dispatch -//! - server request resolution and rejection -//! - event consumption and shutdown behavior //! -//! The facade uses bounded channels between caller and worker task so overload -//! is visible instead of unbounded memory growth. +//! - Runtime startup and initialize-capabilities handshake. +//! - Surface-to-session-source policy ([`ClientSurface`] → [`SessionSource`]). +//! - Typed and raw request/notification dispatch. +//! - Server request resolution and rejection. +//! - Event consumption with backpressure signaling ([`InProcessServerEvent::Lagged`]). +//! - Bounded graceful shutdown with abort fallback. +//! +//! The facade interposes a worker task between the caller and the underlying +//! [`InProcessClientHandle`](codex_app_server::in_process::InProcessClientHandle), +//! bridging async `mpsc` channels on both sides. Queues are bounded so overload +//! surfaces as channel-full errors rather than unbounded memory growth. use std::error::Error; use std::fmt; @@ -205,6 +209,10 @@ impl InProcessClientStartArgs { } } +/// Internal command sent from public facade methods to the worker task. +/// +/// Each variant carries a oneshot sender so the caller can `await` the +/// result without holding a mutable reference to the client. enum ClientCommand { Request { request: Box, diff --git a/codex-rs/app-server/src/app_server_tracing.rs b/codex-rs/app-server/src/app_server_tracing.rs index c55b00711bbe..563769bb6447 100644 --- a/codex-rs/app-server/src/app_server_tracing.rs +++ b/codex-rs/app-server/src/app_server_tracing.rs @@ -2,7 +2,10 @@ //! //! The in-process path intentionally reuses the same span shape as JSON-RPC //! transports so request telemetry stays comparable across stdio, websocket, -//! and embedded callers. +//! and embedded callers. [`typed_request_span`] is the in-process counterpart +//! of [`request_span`] and stamps `rpc.transport` as `"in-process"` while +//! deriving client identity from the typed [`ClientRequest`] rather than +//! from a parsed JSON envelope. use crate::message_processor::ConnectionSessionState; use crate::outgoing_message::ConnectionId; diff --git a/codex-rs/app-server/src/in_process.rs b/codex-rs/app-server/src/in_process.rs index 92f1dde0fa08..229f525961a3 100644 --- a/codex-rs/app-server/src/in_process.rs +++ b/codex-rs/app-server/src/in_process.rs @@ -1,26 +1,42 @@ //! In-process app-server runtime host for local embedders. //! -//! This module runs the existing `MessageProcessor` and outbound routing logic +//! This module runs the existing [`MessageProcessor`] and outbound routing logic //! on Tokio tasks, but replaces socket/stdio transports with bounded in-memory //! channels. The intent is to preserve app-server semantics while avoiding a //! process boundary for CLI surfaces that run in the same process. //! -//! The lifecycle is: -//! - construct runtime state with [`InProcessStartArgs`] -//! - perform `initialize` + `initialized` handshake in [`start`] -//! - interact through [`InProcessClientHandle`] request/notify/event methods -//! - terminate with [`InProcessClientHandle::shutdown`] +//! # Lifecycle +//! +//! 1. Construct runtime state with [`InProcessStartArgs`]. +//! 2. Call [`start`], which performs the `initialize` / `initialized` handshake +//! internally and returns a ready-to-use [`InProcessClientHandle`]. +//! 3. Send requests via [`InProcessClientHandle::request`], notifications via +//! [`InProcessClientHandle::notify`], and consume events via +//! [`InProcessClientHandle::next_event`]. +//! 4. Terminate with [`InProcessClientHandle::shutdown`]. +//! +//! # Transport model //! //! The runtime is transport-local but not protocol-free. Incoming requests are -//! typed `ClientRequest` values, yet responses still come back through the same -//! JSON-RPC result envelope that `MessageProcessor` uses for stdio/websocket -//! transports. This keeps in-process behavior aligned with app-server rather -//! than creating a second execution contract. +//! typed [`ClientRequest`] values, yet responses still come back through the +//! same JSON-RPC result envelope that `MessageProcessor` uses for stdio and +//! websocket transports. This keeps in-process behavior aligned with +//! app-server rather than creating a second execution contract. +//! +//! # Backpressure +//! +//! Command submission uses `try_send` and can return `WouldBlock`, while event +//! fanout may drop notifications under saturation. Server requests are never +//! silently abandoned: if they cannot be queued they are failed back into +//! `MessageProcessor` with overload or internal errors so approval flows do +//! not hang indefinitely. //! -//! Backpressure is explicit: command submission uses `try_send` and can return -//! `WouldBlock`, while event fanout may drop notifications under saturation. -//! Server requests are never silently abandoned: if they cannot be queued they -//! are failed back into `MessageProcessor` with overload/internal errors. +//! # Relationship to `codex-app-server-client` +//! +//! This module provides the low-level runtime handle ([`InProcessClientHandle`]). +//! Higher-level callers (TUI, exec) should go through `codex-app-server-client`, +//! which wraps this module behind a worker task with async request/response +//! helpers, surface-specific startup policy, and bounded shutdown. use std::collections::HashMap; use std::collections::HashSet; @@ -108,10 +124,13 @@ pub struct InProcessStartArgs { /// Event emitted from the app-server to the in-process client. /// -/// The stream intentionally carries both typed app-server events and legacy -/// bridge notifications because some CLI surfaces still consume -/// `codex_protocol::Event` values today. `Lagged` is a transport health marker, -/// not an application event. +/// The stream carries three event families because CLI surfaces are mid-migration +/// from the legacy `codex_protocol::Event` model to the typed app-server +/// notification model. Once all surfaces consume only [`ServerNotification`], +/// [`LegacyNotification`](Self::LegacyNotification) can be removed. +/// +/// [`Lagged`](Self::Lagged) is a transport health marker, not an application +/// event — it signals that the consumer fell behind and some events were dropped. #[derive(Debug, Clone)] pub enum InProcessServerEvent { /// Server request that requires client response/rejection. @@ -124,6 +143,11 @@ pub enum InProcessServerEvent { Lagged { skipped: usize }, } +/// Internal message sent from [`InProcessClientHandle`] methods to the runtime task. +/// +/// Requests carry a oneshot sender for the response; notifications and server-request +/// replies are fire-and-forget from the caller's perspective (transport errors are +/// caught by `try_send` on the outer channel). enum InProcessClientMessage { Request { request: Box, From bf1cb8bb396d63d590ff69ecb5498c25454ccc7c Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 8 Mar 2026 15:11:52 -0600 Subject: [PATCH 07/25] fix(exec): avoid in-process bootstrap stall --- codex-rs/exec/src/lib.rs | 196 ++------------------------------------- 1 file changed, 6 insertions(+), 190 deletions(-) diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index df92b01e3720..7213ebb2cb16 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -84,8 +84,6 @@ use std::io::Read; use std::path::PathBuf; use supports_color::Stream; use tokio::sync::mpsc; -use tokio::time::Duration; -use tokio::time::timeout; use tracing::Instrument; use tracing::error; use tracing::field; @@ -593,24 +591,10 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> { let primary_thread_id_for_span = primary_thread_id.to_string(); let mut error_seen = false; let mut buffered_events = VecDeque::new(); - let session_configured = match await_session_configured_event( - &mut client, - &config, - &primary_thread_id_for_span, - &mut error_seen, - &mut buffered_events, - ) - .await - { - Ok(session_configured) => session_configured, - Err(err) => { - warn!( - "failed to receive session configured event for thread {}: {err}; using startup response fallback", - primary_thread_id_for_span - ); - fallback_session_configured - } - }; + // Use the start/resume response as the authoritative bootstrap payload. + // Waiting for a later streamed `SessionConfigured` event adds up to 10s of + // avoidable startup latency on the in-process path. + let session_configured = fallback_session_configured; exec_span.record("thread.id", primary_thread_id_for_span.as_str()); @@ -946,105 +930,6 @@ where }) } -/// Waits for the authoritative bootstrap `SessionConfigured` event. -/// -/// `thread/start` and `thread/resume` provide enough data to synthesize a -/// session configuration, but exec prefers the streamed event when it arrives -/// in time because that payload is the closest match to main's startup data. -/// Non-bootstrap events are buffered so the main event loop can process them in -/// order once startup finishes. -async fn await_session_configured_event( - client: &mut InProcessAppServerClient, - config: &Config, - thread_id: &str, - error_seen: &mut bool, - buffered_events: &mut VecDeque, -) -> Result { - const SESSION_CONFIGURED_TIMEOUT: Duration = Duration::from_secs(10); - let wait_result = timeout(SESSION_CONFIGURED_TIMEOUT, async { - loop { - let server_event = client.next_event().await; - let Some(server_event) = server_event else { - return Err("event stream ended before session configured".to_string()); - }; - - match classify_bootstrap_event(server_event, thread_id, buffered_events) { - Ok(BootstrapEventAction::HandleServerRequest(request)) => { - handle_server_request(client, request, config, thread_id, error_seen).await; - } - Ok(BootstrapEventAction::SessionConfigured(session_configured)) => { - return Ok(session_configured); - } - Ok(BootstrapEventAction::Continue) => {} - Err(err) => { - warn!("{err}"); - } - } - } - }) - .await; - - match wait_result { - Ok(result) => result, - Err(_) => Err(format!( - "timed out waiting for session configured event after {}s", - SESSION_CONFIGURED_TIMEOUT.as_secs() - )), - } -} - -enum BootstrapEventAction { - Continue, - HandleServerRequest(ServerRequest), - SessionConfigured(SessionConfiguredEvent), -} - -/// Classifies early events seen before exec has finished bootstrap. -/// -/// The bootstrap phase has two jobs: -/// - answer server requests that would otherwise stall startup -/// - capture the matching `SessionConfigured` event for the primary thread -/// -/// Everything else is buffered and replayed by the main loop so startup does -/// not reorder or discard early notifications. -fn classify_bootstrap_event( - server_event: InProcessServerEvent, - thread_id: &str, - buffered_events: &mut VecDeque, -) -> Result { - match &server_event { - InProcessServerEvent::ServerRequest(_) => { - if let InProcessServerEvent::ServerRequest(request) = server_event { - Ok(BootstrapEventAction::HandleServerRequest(request)) - } else { - unreachable!("matched server request variant") - } - } - InProcessServerEvent::ServerNotification(_) | InProcessServerEvent::Lagged { .. } => { - buffered_events.push_back(server_event); - Ok(BootstrapEventAction::Continue) - } - InProcessServerEvent::LegacyNotification(notification) => { - let event = legacy_notification_to_event(notification.clone()); - match event { - Ok(event) => { - if let EventMsg::SessionConfigured(session_configured) = event.msg - && session_configured.session_id.to_string() == thread_id - { - return Ok(BootstrapEventAction::SessionConfigured(session_configured)); - } - buffered_events.push_back(server_event); - Ok(BootstrapEventAction::Continue) - } - Err(err) => { - buffered_events.push_back(server_event); - Err(err) - } - } - } - } -} - fn session_configured_from_thread_start_response( response: &ThreadStartResponse, ) -> Result { @@ -1054,7 +939,7 @@ fn session_configured_from_thread_start_response( response.thread.path.clone(), response.model.clone(), response.model_provider.clone(), - response.service_tier.clone(), + response.service_tier, response.approval_policy.to_core(), response.sandbox.to_core(), response.cwd.clone(), @@ -1071,7 +956,7 @@ fn session_configured_from_thread_resume_response( response.thread.path.clone(), response.model.clone(), response.model_provider.clone(), - response.service_tier.clone(), + response.service_tier, response.approval_policy.to_core(), response.sandbox.to_core(), response.cwd.clone(), @@ -1655,14 +1540,12 @@ fn build_review_request(args: &ReviewArgs) -> anyhow::Result { #[cfg(test)] mod tests { use super::*; - use codex_app_server_protocol::JSONRPCNotification; use codex_otel::set_parent_from_w3c_trace_context; use opentelemetry::trace::TraceContextExt; use opentelemetry::trace::TraceId; use opentelemetry::trace::TracerProvider as _; use opentelemetry_sdk::trace::SdkTracerProvider; use pretty_assertions::assert_eq; - use serde_json::json; use tracing_opentelemetry::OpenTelemetrySpanExt; fn test_tracing_subscriber() -> impl tracing::Subscriber + Send + Sync { @@ -1832,73 +1715,6 @@ mod tests { assert_eq!(err, PromptDecodeError::InvalidUtf8 { valid_up_to: 0 }); } - #[test] - fn classify_bootstrap_event_buffers_lagged_markers() { - let mut buffered_events = VecDeque::new(); - - let action = classify_bootstrap_event( - InProcessServerEvent::Lagged { skipped: 3 }, - "thread-1", - &mut buffered_events, - ) - .expect("lagged events should be buffered"); - - assert!(matches!(action, BootstrapEventAction::Continue)); - assert_eq!(buffered_events.len(), 1); - assert!(matches!( - buffered_events.front(), - Some(InProcessServerEvent::Lagged { skipped: 3 }) - )); - } - - #[test] - fn classify_bootstrap_event_extracts_matching_session_configured() { - let mut buffered_events = VecDeque::new(); - let thread_id = "019cbf93-9ff5-7ac0-ac93-c8a36f0c98d3"; - let session_configured = session_configured_from_thread_response( - thread_id, - Some("thread".to_string()), - None, - "gpt-5".to_string(), - "openai".to_string(), - None, - AskForApproval::Never, - codex_protocol::protocol::SandboxPolicy::DangerFullAccess, - std::env::temp_dir(), - None, - ) - .expect("session configured payload should build"); - let notification = JSONRPCNotification { - method: "codex/event/session_configured".to_string(), - params: Some(json!({ - "msg": serde_json::to_value(&session_configured) - .expect("session configured should serialize"), - })), - }; - - let action = classify_bootstrap_event( - InProcessServerEvent::LegacyNotification(notification), - thread_id, - &mut buffered_events, - ) - .expect("matching session configured should decode"); - - match action { - BootstrapEventAction::SessionConfigured(payload) => { - assert_eq!( - payload.session_id.to_string(), - session_configured.session_id.to_string() - ); - assert_eq!(payload.thread_name, session_configured.thread_name); - assert_eq!(payload.model, session_configured.model); - } - BootstrapEventAction::Continue | BootstrapEventAction::HandleServerRequest(_) => { - panic!("expected matching session configured event") - } - } - assert!(buffered_events.is_empty()); - } - #[test] fn lagged_event_warning_message_is_explicit() { assert_eq!( From 99b1d5376dcf7ce43bb03b593166a5d54c55a424 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Fri, 6 Mar 2026 12:59:06 -0300 Subject: [PATCH 08/25] fix(app-server): handle plugin list request ids Cover `ClientRequest::PluginList` in `codex-rs/app-server/src/app_server_tracing.rs` so the tracing helper stays exhaustive as the app-server protocol evolves. This fixes the `codex-app-server` compile failure that was breaking `rust-ci` across the full matrix. The change is intentionally narrow: it updates the request-id extractor used by in-process tracing without changing protocol or runtime behavior. --- codex-rs/app-server/src/app_server_tracing.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/codex-rs/app-server/src/app_server_tracing.rs b/codex-rs/app-server/src/app_server_tracing.rs index 563769bb6447..5abe29ca45c0 100644 --- a/codex-rs/app-server/src/app_server_tracing.rs +++ b/codex-rs/app-server/src/app_server_tracing.rs @@ -191,6 +191,7 @@ pub(crate) fn client_request_id(request: &ClientRequest) -> &RequestId { | ClientRequest::SkillsRemoteExport { request_id, .. } | ClientRequest::AppsList { request_id, .. } | ClientRequest::SkillsConfigWrite { request_id, .. } + | ClientRequest::PluginList { request_id, .. } | ClientRequest::PluginInstall { request_id, .. } | ClientRequest::TurnStart { request_id, .. } | ClientRequest::TurnSteer { request_id, .. } From 3ad5c8443fe87d0c101c88b2a6145c4b59d51060 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 8 Mar 2026 15:11:59 -0600 Subject: [PATCH 09/25] fix(exec): resolve in-process protocol drift --- codex-rs/exec/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 7213ebb2cb16..b734531d04cb 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -589,7 +589,6 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> { }; let primary_thread_id_for_span = primary_thread_id.to_string(); - let mut error_seen = false; let mut buffered_events = VecDeque::new(); // Use the start/resume response as the authoritative bootstrap payload. // Waiting for a later streamed `SessionConfigured` event adds up to 10s of @@ -1144,6 +1143,7 @@ async fn handle_server_request( let response = McpServerElicitationRequestResponse { action: McpServerElicitationAction::Cancel, content: None, + meta: None, }; match serde_json::to_value(response) { Ok(value) => { From 85b2f712aeddde60c9a61dba5be032898f7e4c46 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Fri, 6 Mar 2026 14:11:55 -0300 Subject: [PATCH 10/25] fix(exec): address rust-ci clippy warnings Remove the redundant `Into::into` on `default_effort` and the needless borrows around `sandbox_policy.get()` in `codex-rs/exec/src/lib.rs`. These were the remaining `clippy` failures in the `Lint/Build` matrix after the protocol-compatibility fixes landed. The change is intentionally tiny and keeps the generated request payloads unchanged while letting `rust-ci` finish the `codex-exec` lint pass. --- codex-rs/exec/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index b734531d04cb..107ae84bb713 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -688,7 +688,7 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> { sandbox_policy: Some(default_sandbox_policy.clone().into()), model: None, service_tier: None, - effort: default_effort.map(Into::into), + effort: default_effort, summary: None, personality: None, output_schema, @@ -893,7 +893,7 @@ fn thread_start_params_from_config(config: &Config) -> ThreadStartParams { model_provider: Some(config.model_provider_id.clone()), cwd: Some(config.cwd.to_string_lossy().to_string()), approval_policy: Some(config.permissions.approval_policy.value().into()), - sandbox: sandbox_mode_from_policy(&config.permissions.sandbox_policy.get()), + sandbox: sandbox_mode_from_policy(config.permissions.sandbox_policy.get()), ephemeral: Some(config.ephemeral), ..ThreadStartParams::default() } @@ -907,7 +907,7 @@ fn thread_resume_params_from_config(config: &Config, path: Option) -> T model_provider: Some(config.model_provider_id.clone()), cwd: Some(config.cwd.to_string_lossy().to_string()), approval_policy: Some(config.permissions.approval_policy.value().into()), - sandbox: sandbox_mode_from_policy(&config.permissions.sandbox_policy.get()), + sandbox: sandbox_mode_from_policy(config.permissions.sandbox_policy.get()), ..ThreadResumeParams::default() } } From 6e61fde8958675f83b740546f597b1daaf7f3cbd Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 8 Mar 2026 15:12:06 -0600 Subject: [PATCH 11/25] fix(app-server): avoid unnecessary clone in in-process path --- codex-rs/app-server/src/in_process.rs | 58 +++++++++++++-------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/codex-rs/app-server/src/in_process.rs b/codex-rs/app-server/src/in_process.rs index 229f525961a3..3b2f6733b3b6 100644 --- a/codex-rs/app-server/src/in_process.rs +++ b/codex-rs/app-server/src/in_process.rs @@ -462,37 +462,37 @@ fn start_uninitialized(args: InProcessStartArgs) -> InProcessClientHandle { } } OutgoingMessage::Request(request) => { + // Send directly to avoid cloning; on failure the + // original value is returned inside the error. if let Err(send_error) = event_tx - .try_send(InProcessServerEvent::ServerRequest(request.clone())) + .try_send(InProcessServerEvent::ServerRequest(request)) { - match send_error { - mpsc::error::TrySendError::Full(_) => { - processor - .process_error(JSONRPCError { - id: request.id().clone(), - error: JSONRPCErrorError { - code: OVERLOADED_ERROR_CODE, - message: "in-process server request queue is full" - .to_string(), - data: None, - }, - }) - .await; - } - mpsc::error::TrySendError::Closed(_) => { - processor - .process_error(JSONRPCError { - id: request.id().clone(), - error: JSONRPCErrorError { - code: INTERNAL_ERROR_CODE, - message: "in-process server request consumer is closed" - .to_string(), - data: None, - }, - }) - .await; - } - } + let (code, message, inner) = match send_error { + mpsc::error::TrySendError::Full(inner) => ( + OVERLOADED_ERROR_CODE, + "in-process server request queue is full", + inner, + ), + mpsc::error::TrySendError::Closed(inner) => ( + INTERNAL_ERROR_CODE, + "in-process server request consumer is closed", + inner, + ), + }; + let request_id = match inner { + InProcessServerEvent::ServerRequest(req) => req.id().clone(), + _ => unreachable!("we just sent a ServerRequest variant"), + }; + processor + .process_error(JSONRPCError { + id: request_id, + error: JSONRPCErrorError { + code, + message: message.to_string(), + data: None, + }, + }) + .await; } } OutgoingMessage::AppServerNotification(notification) => { From f440091d3a3f3e155f7db51dd0a9c384141a9a12 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Sun, 8 Mar 2026 14:52:40 -0300 Subject: [PATCH 12/25] fix(app-server): cover command exec request ids in tracing Extend `client_request_id` to include the v2 `command/exec/*` request variants added upstream so the rebased branch stays exhaustive and keeps compiling. --- codex-rs/app-server/src/app_server_tracing.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/codex-rs/app-server/src/app_server_tracing.rs b/codex-rs/app-server/src/app_server_tracing.rs index 5abe29ca45c0..a4503568fd35 100644 --- a/codex-rs/app-server/src/app_server_tracing.rs +++ b/codex-rs/app-server/src/app_server_tracing.rs @@ -215,6 +215,9 @@ pub(crate) fn client_request_id(request: &ClientRequest) -> &RequestId { | ClientRequest::GetAccountRateLimits { request_id, .. } | ClientRequest::FeedbackUpload { request_id, .. } | ClientRequest::OneOffCommandExec { request_id, .. } + | ClientRequest::CommandExecWrite { request_id, .. } + | ClientRequest::CommandExecTerminate { request_id, .. } + | ClientRequest::CommandExecResize { request_id, .. } | ClientRequest::ConfigRead { request_id, .. } | ClientRequest::ExternalAgentConfigDetect { request_id, .. } | ClientRequest::ExternalAgentConfigImport { request_id, .. } From aaf337ac5924f4888b41675c504bb1d9ecc186ff Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 8 Mar 2026 15:55:37 -0600 Subject: [PATCH 13/25] Remove ClientSurface from app-server client --- codex-rs/app-server-client/README.md | 16 +++--- codex-rs/app-server-client/src/lib.rs | 78 ++++++++------------------- codex-rs/exec/src/lib.rs | 6 +-- 3 files changed, 31 insertions(+), 69 deletions(-) diff --git a/codex-rs/app-server-client/README.md b/codex-rs/app-server-client/README.md index aa1c702ce96b..8944ab4ba159 100644 --- a/codex-rs/app-server-client/README.md +++ b/codex-rs/app-server-client/README.md @@ -12,19 +12,17 @@ This crate centralizes startup and lifecycle management for an in-process - app-server bootstrap and initialize handshake - in-memory request/event transport wiring -- session source selection per client surface +- lifecycle orchestration around caller-provided startup identity - graceful shutdown behavior -## Client surfaces and session source +## Startup identity -`ClientSurface` controls which `SessionSource` is used when starting -app-server: +Callers pass both the app-server `SessionSource` and the initialize +`client_info.name` explicitly when starting the facade. -- `ClientSurface::Exec` -> `SessionSource::Exec` -- `ClientSurface::Tui` -> `SessionSource::Cli` - -This ensures thread metadata (for example in `thread/list` and `thread/read`) -matches the originating runtime. +That keeps thread metadata (for example in `thread/list` and `thread/read`) +aligned with the originating runtime without baking TUI/exec-specific policy +into the shared client layer. ## Transport model diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index a965cb20165c..c10f575e2ae8 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -4,7 +4,7 @@ //! used by surfaces like TUI and exec. It centralizes: //! //! - Runtime startup and initialize-capabilities handshake. -//! - Surface-to-session-source policy ([`ClientSurface`] → [`SessionSource`]). +//! - Typed caller-provided startup identity (`SessionSource` + client name). //! - Typed and raw request/notification dispatch. //! - Server request resolution and rejection. //! - Event consumption with backpressure signaling ([`InProcessServerEvent::Lagged`]). @@ -104,34 +104,6 @@ impl Error for TypedRequestError { } } -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub enum ClientSurface { - /// Non-interactive execution surface. - Exec, - /// Interactive terminal UI surface. - Tui, -} - -/// Maps facade surface identity to app-server `SessionSource`. -/// -/// `ClientSurface::Tui` intentionally maps to `SessionSource::Cli` because the -/// TUI is the interactive CLI surface from the server's perspective. -pub fn session_source_for_surface(surface: ClientSurface) -> SessionSource { - match surface { - ClientSurface::Exec => SessionSource::Exec, - ClientSurface::Tui => SessionSource::Cli, - } -} - -impl ClientSurface { - fn default_client_name(self) -> &'static str { - match self { - ClientSurface::Exec => "codex-exec", - ClientSurface::Tui => "codex-tui", - } - } -} - #[derive(Clone)] pub struct InProcessClientStartArgs { /// Resolved argv0 dispatch paths used by command execution internals. @@ -148,10 +120,10 @@ pub struct InProcessClientStartArgs { pub feedback: CodexFeedback, /// Startup warnings emitted after initialize succeeds. pub config_warnings: Vec, - /// Surface identity that drives session source and default client name. - pub surface: ClientSurface, - /// Optional explicit client name; falls back to surface default. - pub client_name: Option, + /// Session source recorded in app-server thread metadata. + pub session_source: SessionSource, + /// Client name reported during initialize. + pub client_name: String, /// Client version reported during initialize. pub client_version: String, /// Whether experimental APIs are requested at initialize time. @@ -163,16 +135,8 @@ pub struct InProcessClientStartArgs { } impl InProcessClientStartArgs { - /// Builds initialize params from surface and caller-provided metadata. - /// - /// This keeps the initialize handshake policy in one place so TUI and exec - /// do not drift on client naming, experimental opt-in, or notification - /// suppression. + /// Builds initialize params from caller-provided metadata. pub fn initialize_params(&self) -> InitializeParams { - let client_name = self - .client_name - .clone() - .unwrap_or_else(|| self.surface.default_client_name().to_string()); let capabilities = InitializeCapabilities { experimental_api: self.experimental_api, opt_out_notification_methods: if self.opt_out_notification_methods.is_empty() { @@ -184,7 +148,7 @@ impl InProcessClientStartArgs { InitializeParams { client_info: ClientInfo { - name: client_name, + name: self.client_name.clone(), title: None, version: self.client_version.clone(), }, @@ -202,7 +166,7 @@ impl InProcessClientStartArgs { cloud_requirements: self.cloud_requirements, feedback: self.feedback, config_warnings: self.config_warnings, - session_source: session_source_for_surface(self.surface), + session_source: self.session_source, initialize, channel_capacity: self.channel_capacity, } @@ -587,7 +551,7 @@ mod tests { } async fn start_test_client_with_capacity( - surface: ClientSurface, + session_source: SessionSource, channel_capacity: usize, ) -> InProcessAppServerClient { InProcessAppServerClient::start(InProcessClientStartArgs { @@ -598,8 +562,8 @@ mod tests { cloud_requirements: CloudRequirementsLoader::default(), feedback: CodexFeedback::new(), config_warnings: Vec::new(), - surface, - client_name: Some("codex-app-server-client-test".to_string()), + session_source, + client_name: "codex-app-server-client-test".to_string(), client_version: "0.0.0-test".to_string(), experimental_api: true, opt_out_notification_methods: Vec::new(), @@ -609,13 +573,13 @@ mod tests { .expect("in-process app-server client should start") } - async fn start_test_client(surface: ClientSurface) -> InProcessAppServerClient { - start_test_client_with_capacity(surface, DEFAULT_IN_PROCESS_CHANNEL_CAPACITY).await + async fn start_test_client(session_source: SessionSource) -> InProcessAppServerClient { + start_test_client_with_capacity(session_source, DEFAULT_IN_PROCESS_CHANNEL_CAPACITY).await } #[tokio::test] async fn typed_request_roundtrip_works() { - let client = start_test_client(ClientSurface::Exec).await; + let client = start_test_client(SessionSource::Exec).await; let _response: ConfigRequirementsReadResponse = client .request_typed(ClientRequest::ConfigRequirementsRead { request_id: RequestId::Integer(1), @@ -628,7 +592,7 @@ mod tests { #[tokio::test] async fn typed_request_reports_json_rpc_errors() { - let client = start_test_client(ClientSurface::Exec).await; + let client = start_test_client(SessionSource::Exec).await; let err = client .request_typed::(ClientRequest::ThreadRead { request_id: RequestId::Integer(99), @@ -647,12 +611,12 @@ mod tests { } #[tokio::test] - async fn surface_to_session_source_mapping_is_applied() { - for (surface, expected_source) in [ - (ClientSurface::Exec, ApiSessionSource::Exec), - (ClientSurface::Tui, ApiSessionSource::Cli), + async fn caller_provided_session_source_is_applied() { + for (session_source, expected_source) in [ + (SessionSource::Exec, ApiSessionSource::Exec), + (SessionSource::Cli, ApiSessionSource::Cli), ] { - let client = start_test_client(surface).await; + let client = start_test_client(session_source).await; let parsed: ThreadStartResponse = client .request_typed(ClientRequest::ThreadStart { request_id: RequestId::Integer(2), @@ -670,7 +634,7 @@ mod tests { #[tokio::test] async fn tiny_channel_capacity_still_supports_request_roundtrip() { - let client = start_test_client_with_capacity(ClientSurface::Exec, 1).await; + let client = start_test_client_with_capacity(SessionSource::Exec, 1).await; let _response: ConfigRequirementsReadResponse = client .request_typed(ClientRequest::ConfigRequirementsRead { request_id: RequestId::Integer(1), diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 107ae84bb713..1742c8cb7b5b 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -13,7 +13,6 @@ pub mod exec_events; pub use cli::Cli; pub use cli::Command; pub use cli::ReviewArgs; -use codex_app_server_client::ClientSurface; use codex_app_server_client::DEFAULT_IN_PROCESS_CHANNEL_CAPACITY; use codex_app_server_client::InProcessAppServerClient; use codex_app_server_client::InProcessClientStartArgs; @@ -70,6 +69,7 @@ use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ReviewRequest; use codex_protocol::protocol::ReviewTarget; use codex_protocol::protocol::SessionConfiguredEvent; +use codex_protocol::protocol::SessionSource; use codex_protocol::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_oss::ensure_oss_provider_ready; @@ -435,8 +435,8 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result cloud_requirements: run_cloud_requirements, feedback: CodexFeedback::new(), config_warnings, - surface: ClientSurface::Exec, - client_name: Some("codex-exec".to_string()), + session_source: SessionSource::Exec, + client_name: "codex-exec".to_string(), client_version: env!("CARGO_PKG_VERSION").to_string(), experimental_api: true, opt_out_notification_methods: Vec::new(), From c7461382542d0d01514a97859c5ea3eeceedbf6f Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 8 Mar 2026 16:00:59 -0600 Subject: [PATCH 14/25] Address in-process exec review feedback --- codex-rs/app-server/src/in_process.rs | 90 ++++++++++++++++++++++++--- codex-rs/exec/src/lib.rs | 6 +- 2 files changed, 87 insertions(+), 9 deletions(-) diff --git a/codex-rs/app-server/src/in_process.rs b/codex-rs/app-server/src/in_process.rs index 3b2f6733b3b6..201f2063fd82 100644 --- a/codex-rs/app-server/src/in_process.rs +++ b/codex-rs/app-server/src/in_process.rs @@ -94,6 +94,20 @@ pub const DEFAULT_IN_PROCESS_CHANNEL_CAPACITY: usize = CHANNEL_CAPACITY; type PendingClientRequestResponse = std::result::Result; +fn server_notification_requires_delivery(notification: &ServerNotification) -> bool { + matches!(notification, ServerNotification::TurnCompleted(_)) +} + +fn legacy_notification_requires_delivery(notification: &JSONRPCNotification) -> bool { + matches!( + notification + .method + .strip_prefix("codex/event/") + .unwrap_or(¬ification.method), + "task_complete" | "turn_aborted" | "shutdown_complete" + ) +} + /// Input needed to start an in-process app-server runtime. /// /// These fields mirror the pieces of ambient process state that stdio and @@ -496,8 +510,16 @@ fn start_uninitialized(args: InProcessStartArgs) -> InProcessClientHandle { } } OutgoingMessage::AppServerNotification(notification) => { - if let Err(send_error) = event_tx - .try_send(InProcessServerEvent::ServerNotification(notification)) + if server_notification_requires_delivery(¬ification) { + if event_tx + .send(InProcessServerEvent::ServerNotification(notification)) + .await + .is_err() + { + break; + } + } else if let Err(send_error) = + event_tx.try_send(InProcessServerEvent::ServerNotification(notification)) { match send_error { mpsc::error::TrySendError::Full(_) => { @@ -510,12 +532,21 @@ fn start_uninitialized(args: InProcessStartArgs) -> InProcessClientHandle { } } OutgoingMessage::Notification(notification) => { - if let Err(send_error) = event_tx.try_send( - InProcessServerEvent::LegacyNotification(JSONRPCNotification { - method: notification.method, - params: notification.params, - }), - ) { + let notification = JSONRPCNotification { + method: notification.method, + params: notification.params, + }; + if legacy_notification_requires_delivery(¬ification) { + if event_tx + .send(InProcessServerEvent::LegacyNotification(notification)) + .await + .is_err() + { + break; + } + } else if let Err(send_error) = + event_tx.try_send(InProcessServerEvent::LegacyNotification(notification)) + { match send_error { mpsc::error::TrySendError::Full(_) => { warn!("dropping in-process legacy notification (queue full)"); @@ -586,6 +617,9 @@ mod tests { use codex_app_server_protocol::SessionSource as ApiSessionSource; use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; + use codex_app_server_protocol::Turn; + use codex_app_server_protocol::TurnCompletedNotification; + use codex_app_server_protocol::TurnStatus; use codex_core::config::ConfigBuilder; use pretty_assertions::assert_eq; @@ -701,4 +735,44 @@ mod tests { .await .expect("in-process runtime should shutdown cleanly"); } + + #[test] + fn guaranteed_delivery_helpers_cover_terminal_notifications() { + assert!(server_notification_requires_delivery( + &ServerNotification::TurnCompleted(TurnCompletedNotification { + thread_id: "thread-1".to_string(), + turn: Turn { + id: "turn-1".to_string(), + items: Vec::new(), + status: TurnStatus::Completed, + error: None, + }, + }) + )); + + assert!(legacy_notification_requires_delivery( + &JSONRPCNotification { + method: "codex/event/task_complete".to_string(), + params: None, + } + )); + assert!(legacy_notification_requires_delivery( + &JSONRPCNotification { + method: "codex/event/turn_aborted".to_string(), + params: None, + } + )); + assert!(legacy_notification_requires_delivery( + &JSONRPCNotification { + method: "codex/event/shutdown_complete".to_string(), + params: None, + } + )); + assert!(!legacy_notification_requires_delivery( + &JSONRPCNotification { + method: "codex/event/item_started".to_string(), + params: None, + } + )); + } } diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 1742c8cb7b5b..51b38628c104 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -845,7 +845,11 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> { } break; } - CodexStatus::Shutdown => break, + CodexStatus::Shutdown => { + // `ShutdownComplete` does not identify which attached + // thread emitted it, so subagent shutdowns must not end + // the primary exec loop early. + } } } InProcessServerEvent::Lagged { skipped } => { From 0f784d268b44c6d9f4339c70221a06208c141a5d Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 8 Mar 2026 16:11:12 -0600 Subject: [PATCH 15/25] Move client request id access into protocol --- .../src/protocol/common.rs | 9 +++ codex-rs/app-server/src/app_server_tracing.rs | 73 +------------------ codex-rs/app-server/src/in_process.rs | 3 +- codex-rs/app-server/src/message_processor.rs | 2 +- 4 files changed, 12 insertions(+), 75 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index 54c0425256c2..7975231b87e9 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -110,6 +110,14 @@ macro_rules! client_request_definitions { )* } + impl ClientRequest { + pub fn id(&self) -> &RequestId { + match self { + $(Self::$variant { request_id, .. } => request_id,)* + } + } + } + impl crate::experimental_api::ExperimentalApi for ClientRequest { fn experimental_reason(&self) -> Option<&'static str> { match self { @@ -1136,6 +1144,7 @@ mod tests { request_id: RequestId::Integer(1), params: None, }; + assert_eq!(request.id(), &RequestId::Integer(1)); assert_eq!( json!({ "method": "account/rateLimits/read", diff --git a/codex-rs/app-server/src/app_server_tracing.rs b/codex-rs/app-server/src/app_server_tracing.rs index a4503568fd35..3bae285c0ad6 100644 --- a/codex-rs/app-server/src/app_server_tracing.rs +++ b/codex-rs/app-server/src/app_server_tracing.rs @@ -13,7 +13,6 @@ use crate::transport::AppServerTransport; use codex_app_server_protocol::ClientRequest; use codex_app_server_protocol::InitializeParams; use codex_app_server_protocol::JSONRPCRequest; -use codex_app_server_protocol::RequestId; use codex_otel::set_parent_from_context; use codex_otel::set_parent_from_w3c_trace_context; use codex_otel::traceparent_context_from_env; @@ -93,7 +92,7 @@ pub(crate) fn typed_request_span( rpc.system = "jsonrpc", rpc.method = "in_process", rpc.transport = "in-process", - rpc.request_id = ?client_request_id(request), + rpc.request_id = ?request.id(), app_server.connection_id = ?connection_id, app_server.api_version = "v2", app_server.client_name = field::Empty, @@ -164,73 +163,3 @@ fn initialize_client_info_from_typed_request(request: &ClientRequest) -> Option< _ => None, } } - -/// Extracts the request id from a typed in-process request. -/// -/// This duplicates the `ClientRequest` variant match locally so the protocol -/// crate does not need an in-process-specific helper just for tracing. -pub(crate) fn client_request_id(request: &ClientRequest) -> &RequestId { - match request { - ClientRequest::Initialize { request_id, .. } - | ClientRequest::ThreadStart { request_id, .. } - | ClientRequest::ThreadResume { request_id, .. } - | ClientRequest::ThreadFork { request_id, .. } - | ClientRequest::ThreadArchive { request_id, .. } - | ClientRequest::ThreadUnsubscribe { request_id, .. } - | ClientRequest::ThreadSetName { request_id, .. } - | ClientRequest::ThreadMetadataUpdate { request_id, .. } - | ClientRequest::ThreadUnarchive { request_id, .. } - | ClientRequest::ThreadCompactStart { request_id, .. } - | ClientRequest::ThreadBackgroundTerminalsClean { request_id, .. } - | ClientRequest::ThreadRollback { request_id, .. } - | ClientRequest::ThreadList { request_id, .. } - | ClientRequest::ThreadLoadedList { request_id, .. } - | ClientRequest::ThreadRead { request_id, .. } - | ClientRequest::SkillsList { request_id, .. } - | ClientRequest::SkillsRemoteList { request_id, .. } - | ClientRequest::SkillsRemoteExport { request_id, .. } - | ClientRequest::AppsList { request_id, .. } - | ClientRequest::SkillsConfigWrite { request_id, .. } - | ClientRequest::PluginList { request_id, .. } - | ClientRequest::PluginInstall { request_id, .. } - | ClientRequest::TurnStart { request_id, .. } - | ClientRequest::TurnSteer { request_id, .. } - | ClientRequest::TurnInterrupt { request_id, .. } - | ClientRequest::ThreadRealtimeStart { request_id, .. } - | ClientRequest::ThreadRealtimeAppendAudio { request_id, .. } - | ClientRequest::ThreadRealtimeAppendText { request_id, .. } - | ClientRequest::ThreadRealtimeStop { request_id, .. } - | ClientRequest::ReviewStart { request_id, .. } - | ClientRequest::ModelList { request_id, .. } - | ClientRequest::ExperimentalFeatureList { request_id, .. } - | ClientRequest::CollaborationModeList { request_id, .. } - | ClientRequest::MockExperimentalMethod { request_id, .. } - | ClientRequest::McpServerOauthLogin { request_id, .. } - | ClientRequest::McpServerRefresh { request_id, .. } - | ClientRequest::McpServerStatusList { request_id, .. } - | ClientRequest::WindowsSandboxSetupStart { request_id, .. } - | ClientRequest::LoginAccount { request_id, .. } - | ClientRequest::CancelLoginAccount { request_id, .. } - | ClientRequest::LogoutAccount { request_id, .. } - | ClientRequest::GetAccountRateLimits { request_id, .. } - | ClientRequest::FeedbackUpload { request_id, .. } - | ClientRequest::OneOffCommandExec { request_id, .. } - | ClientRequest::CommandExecWrite { request_id, .. } - | ClientRequest::CommandExecTerminate { request_id, .. } - | ClientRequest::CommandExecResize { request_id, .. } - | ClientRequest::ConfigRead { request_id, .. } - | ClientRequest::ExternalAgentConfigDetect { request_id, .. } - | ClientRequest::ExternalAgentConfigImport { request_id, .. } - | ClientRequest::ConfigValueWrite { request_id, .. } - | ClientRequest::ConfigBatchWrite { request_id, .. } - | ClientRequest::ConfigRequirementsRead { request_id, .. } - | ClientRequest::GetAccount { request_id, .. } - | ClientRequest::GetConversationSummary { request_id, .. } - | ClientRequest::GitDiffToRemote { request_id, .. } - | ClientRequest::GetAuthStatus { request_id, .. } - | ClientRequest::FuzzyFileSearch { request_id, .. } - | ClientRequest::FuzzyFileSearchSessionStart { request_id, .. } - | ClientRequest::FuzzyFileSearchSessionUpdate { request_id, .. } - | ClientRequest::FuzzyFileSearchSessionStop { request_id, .. } => request_id, - } -} diff --git a/codex-rs/app-server/src/in_process.rs b/codex-rs/app-server/src/in_process.rs index 201f2063fd82..5ac7ec362feb 100644 --- a/codex-rs/app-server/src/in_process.rs +++ b/codex-rs/app-server/src/in_process.rs @@ -381,8 +381,7 @@ fn start_uninitialized(args: InProcessStartArgs) -> InProcessClientHandle { match message { Some(InProcessClientMessage::Request { request, response_tx }) => { let request = *request; - let request_id = - crate::app_server_tracing::client_request_id(&request).clone(); + let request_id = request.id().clone(); match pending_request_responses.entry(request_id.clone()) { Entry::Vacant(entry) => { entry.insert(response_tx); diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 24c9fe4f2fc6..f7186744982b 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -307,7 +307,7 @@ impl MessageProcessor { async { let request_id = ConnectionRequestId { connection_id, - request_id: crate::app_server_tracing::client_request_id(&request).clone(), + request_id: request.id().clone(), }; tracing::trace!( ?connection_id, From 794087d58a90fa44f626082026521a3b6297a2a8 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 8 Mar 2026 16:15:35 -0600 Subject: [PATCH 16/25] Clarify in-process typed request decode semantics --- codex-rs/app-server-client/src/lib.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index c10f575e2ae8..21f7822bf3be 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -60,8 +60,8 @@ pub type RequestResult = std::result::Result; /// Layered error for [`InProcessAppServerClient::request_typed`]. /// /// This keeps transport failures, server-side JSON-RPC failures, and response -/// shape mismatches distinct so callers can decide whether to retry, surface a -/// server error, or treat the response as protocol drift. +/// decode failures distinct so callers can decide whether to retry, surface a +/// server error, or treat the response as an internal request/response mismatch. #[derive(Debug)] pub enum TypedRequestError { Transport { @@ -88,7 +88,7 @@ impl fmt::Display for TypedRequestError { write!(f, "{method} failed: {}", source.message) } Self::Deserialize { method, source } => { - write!(f, "{method} invalid response payload: {source}") + write!(f, "{method} response decode error: {source}") } } } @@ -373,8 +373,9 @@ impl InProcessAppServerClient { /// Sends a typed client request and decodes the successful response body. /// /// This still deserializes from a JSON value produced by app-server's - /// JSON-RPC result envelope. Callers should treat `Deserialize` failures as - /// a protocol-shape mismatch rather than a transport problem. + /// JSON-RPC result envelope. Because the caller chooses `T`, `Deserialize` + /// failures indicate an internal request/response mismatch at the call site + /// (or an in-process bug), not transport skew from an external client. pub async fn request_typed(&self, request: ClientRequest) -> Result where T: DeserializeOwned, From 9291956d3d137190f261b6ffb4aa5a742d834f42 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 8 Mar 2026 16:20:39 -0600 Subject: [PATCH 17/25] Move exec auth env policy to startup args --- codex-rs/app-server-client/src/lib.rs | 4 ++++ codex-rs/app-server/src/in_process.rs | 4 ++++ codex-rs/app-server/src/lib.rs | 1 + codex-rs/app-server/src/message_processor.rs | 3 ++- codex-rs/exec/src/lib.rs | 1 + 5 files changed, 12 insertions(+), 1 deletion(-) diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index 21f7822bf3be..368a00b9aa03 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -122,6 +122,8 @@ pub struct InProcessClientStartArgs { pub config_warnings: Vec, /// Session source recorded in app-server thread metadata. pub session_source: SessionSource, + /// Whether auth loading should honor the `CODEX_API_KEY` environment variable. + pub enable_codex_api_key_env: bool, /// Client name reported during initialize. pub client_name: String, /// Client version reported during initialize. @@ -167,6 +169,7 @@ impl InProcessClientStartArgs { feedback: self.feedback, config_warnings: self.config_warnings, session_source: self.session_source, + enable_codex_api_key_env: self.enable_codex_api_key_env, initialize, channel_capacity: self.channel_capacity, } @@ -564,6 +567,7 @@ mod tests { feedback: CodexFeedback::new(), config_warnings: Vec::new(), session_source, + enable_codex_api_key_env: false, client_name: "codex-app-server-client-test".to_string(), client_version: "0.0.0-test".to_string(), experimental_api: true, diff --git a/codex-rs/app-server/src/in_process.rs b/codex-rs/app-server/src/in_process.rs index 5ac7ec362feb..ae752ec1bfa5 100644 --- a/codex-rs/app-server/src/in_process.rs +++ b/codex-rs/app-server/src/in_process.rs @@ -130,6 +130,8 @@ pub struct InProcessStartArgs { pub config_warnings: Vec, /// Session source stamped into thread/session metadata. pub session_source: SessionSource, + /// Whether auth loading should honor the `CODEX_API_KEY` environment variable. + pub enable_codex_api_key_env: bool, /// Initialize params used for initial handshake. pub initialize: InitializeParams, /// Capacity used for all runtime queues (clamped to at least 1). @@ -367,6 +369,7 @@ fn start_uninitialized(args: InProcessStartArgs) -> InProcessClientHandle { log_db: None, config_warnings: args.config_warnings, session_source: args.session_source, + enable_codex_api_key_env: args.enable_codex_api_key_env, }); let mut thread_created_rx = processor.thread_created_receiver(); let mut session = ConnectionSessionState::default(); @@ -643,6 +646,7 @@ mod tests { feedback: CodexFeedback::new(), config_warnings: Vec::new(), session_source, + enable_codex_api_key_env: false, initialize: InitializeParams { client_info: ClientInfo { name: "codex-in-process-test".to_string(), diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index 4852216d3fad..26f90eb6168a 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -600,6 +600,7 @@ pub async fn run_main_with_transport( log_db, config_warnings, session_source: SessionSource::VSCode, + enable_codex_api_key_env: false, }); let mut thread_created_rx = processor.thread_created_receiver(); let mut running_turn_count_rx = processor.subscribe_running_assistant_turn_count(); diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index f7186744982b..7c62820d9002 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -159,6 +159,7 @@ pub(crate) struct MessageProcessorArgs { pub(crate) log_db: Option, pub(crate) config_warnings: Vec, pub(crate) session_source: SessionSource, + pub(crate) enable_codex_api_key_env: bool, } impl MessageProcessor { @@ -176,8 +177,8 @@ impl MessageProcessor { log_db, config_warnings, session_source, + enable_codex_api_key_env, } = args; - let enable_codex_api_key_env = matches!(session_source, SessionSource::Exec); let auth_manager = AuthManager::shared( config.codex_home.clone(), enable_codex_api_key_env, diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 51b38628c104..b86be6ebdcd2 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -436,6 +436,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result feedback: CodexFeedback::new(), config_warnings, session_source: SessionSource::Exec, + enable_codex_api_key_env: true, client_name: "codex-exec".to_string(), client_version: env!("CARGO_PKG_VERSION").to_string(), experimental_api: true, From d44ff4de2195e84816acc25f6da9f265e71e86a9 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 8 Mar 2026 16:31:40 -0600 Subject: [PATCH 18/25] Add cleanup TODO for synthesized session config --- codex-rs/exec/src/lib.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index b86be6ebdcd2..d457e43ba379 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -978,6 +978,10 @@ fn session_configured_from_thread_resume_response( /// Some session fields are not available synchronously from the start/resume /// response, so callers must treat the result as a best-effort fallback until /// a later `SessionConfigured` event proves otherwise. +/// TODO(architecture): stop synthesizing a partial `SessionConfiguredEvent` +/// here. Either return the authoritative session-configured payload from +/// `thread/start`/`thread/resume`, or introduce a smaller bootstrap type for +/// exec so this path cannot accidentally depend on placeholder fields. fn session_configured_from_thread_response( thread_id: &str, thread_name: Option, From 7c07f4ec8f2bc33b9a4ae45a9331236323220ea4 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 8 Mar 2026 16:34:10 -0600 Subject: [PATCH 19/25] Handle closed exec interrupt channel --- codex-rs/exec/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index d457e43ba379..df99e3ef8459 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -730,14 +730,16 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> { // Track whether a fatal error was reported by the server so we can // exit with a non-zero status for automation-friendly signaling. let mut error_seen = false; + let mut interrupt_channel_open = true; let primary_thread_id_for_requests = primary_thread_id.to_string(); loop { let server_event = if let Some(event) = buffered_events.pop_front() { Some(event) } else { tokio::select! { - maybe_interrupt = interrupt_rx.recv() => { + maybe_interrupt = interrupt_rx.recv(), if interrupt_channel_open => { if maybe_interrupt.is_none() { + interrupt_channel_open = false; continue; } if let Err(err) = send_request_with_response::( From 29b2c65c03d9b51bf36c0cd2b0dbb83bc8f8e921 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 8 Mar 2026 16:55:11 -0600 Subject: [PATCH 20/25] Keep in-process requests pumping during client waits --- codex-rs/app-server-client/src/lib.rs | 19 +- codex-rs/app-server/src/in_process.rs | 324 +++++++++++++------- codex-rs/app-server/src/outgoing_message.rs | 19 ++ 3 files changed, 243 insertions(+), 119 deletions(-) diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index 368a00b9aa03..9f85c18dede9 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -231,6 +231,7 @@ impl InProcessAppServerClient { let channel_capacity = args.channel_capacity.max(1); let mut handle = codex_app_server::in_process::start(args.into_runtime_start_args()).await?; + let request_sender = handle.sender(); let (command_tx, mut command_rx) = mpsc::channel::(channel_capacity); let (event_tx, event_rx) = mpsc::channel::(channel_capacity); @@ -242,14 +243,17 @@ impl InProcessAppServerClient { command = command_rx.recv() => { match command { Some(ClientCommand::Request { request, response_tx }) => { - let result = handle.request(*request).await; - let _ = response_tx.send(result); + let request_sender = request_sender.clone(); + tokio::spawn(async move { + let result = request_sender.request(*request).await; + let _ = response_tx.send(result); + }); } Some(ClientCommand::Notify { notification, response_tx, }) => { - let result = handle.notify(notification); + let result = request_sender.notify(notification); let _ = response_tx.send(result); } Some(ClientCommand::ResolveServerRequest { @@ -257,7 +261,8 @@ impl InProcessAppServerClient { result, response_tx, }) => { - let send_result = handle.respond_to_server_request(request_id, result); + let send_result = + request_sender.respond_to_server_request(request_id, result); let _ = response_tx.send(send_result); } Some(ClientCommand::RejectServerRequest { @@ -265,7 +270,7 @@ impl InProcessAppServerClient { error, response_tx, }) => { - let send_result = handle.fail_server_request(request_id, error); + let send_result = request_sender.fail_server_request(request_id, error); let _ = response_tx.send(send_result); } Some(ClientCommand::Shutdown { response_tx }) => { @@ -297,7 +302,7 @@ impl InProcessAppServerClient { "dropping in-process app-server event because consumer queue is full" ); if let InProcessServerEvent::ServerRequest(request) = event { - let _ = handle.fail_server_request( + let _ = request_sender.fail_server_request( request.id().clone(), JSONRPCErrorError { code: -32001, @@ -321,7 +326,7 @@ impl InProcessAppServerClient { skipped_events = skipped_events.saturating_add(1); warn!("dropping in-process app-server event because consumer queue is full"); if let InProcessServerEvent::ServerRequest(request) = event { - let _ = handle.fail_server_request( + let _ = request_sender.fail_server_request( request.id().clone(), JSONRPCErrorError { code: -32001, diff --git a/codex-rs/app-server/src/in_process.rs b/codex-rs/app-server/src/in_process.rs index ae752ec1bfa5..13c9aac352cf 100644 --- a/codex-rs/app-server/src/in_process.rs +++ b/codex-rs/app-server/src/in_process.rs @@ -67,10 +67,8 @@ use codex_app_server_protocol::ClientNotification; use codex_app_server_protocol::ClientRequest; use codex_app_server_protocol::ConfigWarningNotification; use codex_app_server_protocol::InitializeParams; -use codex_app_server_protocol::JSONRPCError; use codex_app_server_protocol::JSONRPCErrorError; use codex_app_server_protocol::JSONRPCNotification; -use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::RequestId; use codex_app_server_protocol::Result; use codex_app_server_protocol::ServerNotification; @@ -185,13 +183,75 @@ enum InProcessClientMessage { }, } +enum ProcessorCommand { + Request(Box), + Notification(ClientNotification), +} + +#[derive(Clone)] +pub struct InProcessClientSender { + client_tx: mpsc::Sender, +} + +impl InProcessClientSender { + pub async fn request(&self, request: ClientRequest) -> IoResult { + let (response_tx, response_rx) = oneshot::channel(); + self.try_send_client_message(InProcessClientMessage::Request { + request: Box::new(request), + response_tx, + })?; + response_rx.await.map_err(|err| { + IoError::new( + ErrorKind::BrokenPipe, + format!("in-process request response channel closed: {err}"), + ) + }) + } + + pub fn notify(&self, notification: ClientNotification) -> IoResult<()> { + self.try_send_client_message(InProcessClientMessage::Notification { notification }) + } + + pub fn respond_to_server_request(&self, request_id: RequestId, result: Result) -> IoResult<()> { + self.try_send_client_message(InProcessClientMessage::ServerRequestResponse { + request_id, + result, + }) + } + + pub fn fail_server_request( + &self, + request_id: RequestId, + error: JSONRPCErrorError, + ) -> IoResult<()> { + self.try_send_client_message(InProcessClientMessage::ServerRequestError { + request_id, + error, + }) + } + + fn try_send_client_message(&self, message: InProcessClientMessage) -> IoResult<()> { + match self.client_tx.try_send(message) { + Ok(()) => Ok(()), + Err(mpsc::error::TrySendError::Full(_)) => Err(IoError::new( + ErrorKind::WouldBlock, + "in-process app-server client queue is full", + )), + Err(mpsc::error::TrySendError::Closed(_)) => Err(IoError::new( + ErrorKind::BrokenPipe, + "in-process app-server runtime is closed", + )), + } + } +} + /// Handle used by an in-process client to call app-server and consume events. /// /// This is the low-level runtime handle. Higher-level callers should usually go /// through `codex-app-server-client`, which adds worker-task buffering, /// request/response helpers, and surface-specific startup policy. pub struct InProcessClientHandle { - client_tx: mpsc::Sender, + client: InProcessClientSender, event_rx: mpsc::Receiver, runtime_handle: tokio::task::JoinHandle<()>, } @@ -205,17 +265,7 @@ impl InProcessClientHandle { /// produces an `INVALID_REQUEST` response and can make request routing /// ambiguous in the caller. pub async fn request(&self, request: ClientRequest) -> IoResult { - let (response_tx, response_rx) = oneshot::channel(); - self.try_send_client_message(InProcessClientMessage::Request { - request: Box::new(request), - response_tx, - })?; - response_rx.await.map_err(|err| { - IoError::new( - ErrorKind::BrokenPipe, - format!("in-process request response channel closed: {err}"), - ) - }) + self.client.request(request).await } /// Sends a typed client notification into the in-process runtime. @@ -223,7 +273,7 @@ impl InProcessClientHandle { /// Notifications do not have an application-level response. Transport /// errors indicate queue saturation or closed runtime. pub fn notify(&self, notification: ClientNotification) -> IoResult<()> { - self.try_send_client_message(InProcessClientMessage::Notification { notification }) + self.client.notify(notification) } /// Resolves a pending [`ServerRequest`](InProcessServerEvent::ServerRequest). @@ -232,10 +282,7 @@ impl InProcessClientHandle { /// runtime event stream; sending arbitrary IDs has no effect on app-server /// state and can mask a stuck approval flow in the caller. pub fn respond_to_server_request(&self, request_id: RequestId, result: Result) -> IoResult<()> { - self.try_send_client_message(InProcessClientMessage::ServerRequestResponse { - request_id, - result, - }) + self.client.respond_to_server_request(request_id, result) } /// Rejects a pending [`ServerRequest`](InProcessServerEvent::ServerRequest). @@ -247,10 +294,7 @@ impl InProcessClientHandle { request_id: RequestId, error: JSONRPCErrorError, ) -> IoResult<()> { - self.try_send_client_message(InProcessClientMessage::ServerRequestError { - request_id, - error, - }) + self.client.fail_server_request(request_id, error) } /// Receives the next server event from the in-process runtime. @@ -270,6 +314,7 @@ impl InProcessClientHandle { let (done_tx, done_rx) = oneshot::channel(); if self + .client .client_tx .send(InProcessClientMessage::Shutdown { done_tx }) .await @@ -285,18 +330,8 @@ impl InProcessClientHandle { Ok(()) } - fn try_send_client_message(&self, message: InProcessClientMessage) -> IoResult<()> { - match self.client_tx.try_send(message) { - Ok(()) => Ok(()), - Err(mpsc::error::TrySendError::Full(_)) => Err(IoError::new( - ErrorKind::WouldBlock, - "in-process app-server client queue is full", - )), - Err(mpsc::error::TrySendError::Closed(_)) => Err(IoError::new( - ErrorKind::BrokenPipe, - "in-process app-server runtime is closed", - )), - } + pub fn sender(&self) -> InProcessClientSender { + self.client.clone() } } @@ -358,25 +393,92 @@ fn start_uninitialized(args: InProcessStartArgs) -> InProcessClientHandle { } }); - let mut processor = MessageProcessor::new(MessageProcessorArgs { - outgoing: outgoing_message_sender, - arg0_paths: args.arg0_paths, - config: args.config, - cli_overrides: args.cli_overrides, - loader_overrides: args.loader_overrides, - cloud_requirements: args.cloud_requirements, - feedback: args.feedback, - log_db: None, - config_warnings: args.config_warnings, - session_source: args.session_source, - enable_codex_api_key_env: args.enable_codex_api_key_env, + let processor_outgoing = Arc::clone(&outgoing_message_sender); + let (processor_tx, mut processor_rx) = mpsc::channel::(channel_capacity); + let mut processor_handle = tokio::spawn(async move { + let mut processor = MessageProcessor::new(MessageProcessorArgs { + outgoing: Arc::clone(&processor_outgoing), + arg0_paths: args.arg0_paths, + config: args.config, + cli_overrides: args.cli_overrides, + loader_overrides: args.loader_overrides, + cloud_requirements: args.cloud_requirements, + feedback: args.feedback, + log_db: None, + config_warnings: args.config_warnings, + session_source: args.session_source, + enable_codex_api_key_env: args.enable_codex_api_key_env, + }); + let mut thread_created_rx = processor.thread_created_receiver(); + let mut session = ConnectionSessionState::default(); + let mut listen_for_threads = true; + + loop { + tokio::select! { + command = processor_rx.recv() => { + match command { + Some(ProcessorCommand::Request(request)) => { + let was_initialized = session.initialized; + processor + .process_client_request( + IN_PROCESS_CONNECTION_ID, + *request, + &mut session, + &outbound_initialized, + ) + .await; + if let Ok(mut opted_out_notification_methods) = + outbound_opted_out_notification_methods.write() + { + *opted_out_notification_methods = + session.opted_out_notification_methods.clone(); + } else { + warn!("failed to update outbound opted-out notifications"); + } + outbound_experimental_api_enabled.store( + session.experimental_api_enabled, + Ordering::Release, + ); + if !was_initialized && session.initialized { + processor.send_initialize_notifications().await; + } + } + Some(ProcessorCommand::Notification(notification)) => { + processor.process_client_notification(notification).await; + } + None => { + break; + } + } + } + created = thread_created_rx.recv(), if listen_for_threads => { + match created { + Ok(thread_id) => { + let connection_ids = if session.initialized { + vec![IN_PROCESS_CONNECTION_ID] + } else { + Vec::::new() + }; + processor + .try_attach_thread_listener(thread_id, connection_ids) + .await; + } + Err(tokio::sync::broadcast::error::RecvError::Lagged(_)) => { + warn!("thread_created receiver lagged; skipping resync"); + } + Err(tokio::sync::broadcast::error::RecvError::Closed) => { + listen_for_threads = false; + } + } + } + } + } + + processor.connection_closed(IN_PROCESS_CONNECTION_ID).await; }); - let mut thread_created_rx = processor.thread_created_receiver(); - let mut session = ConnectionSessionState::default(); let mut pending_request_responses = HashMap::>::new(); let mut shutdown_ack = None; - let mut listen_for_threads = true; loop { tokio::select! { @@ -399,48 +501,55 @@ fn start_uninitialized(args: InProcessStartArgs) -> InProcessClientHandle { } } - let was_initialized = session.initialized; - processor - .process_client_request( - IN_PROCESS_CONNECTION_ID, - request, - &mut session, - &outbound_initialized, - ) - .await; - if let Ok(mut opted_out_notification_methods) = - outbound_opted_out_notification_methods.write() - { - *opted_out_notification_methods = - session.opted_out_notification_methods.clone(); - } else { - warn!("failed to update outbound opted-out notifications"); - } - outbound_experimental_api_enabled.store( - session.experimental_api_enabled, - Ordering::Release, - ); - if !was_initialized && session.initialized { - processor.send_initialize_notifications().await; + match processor_tx.try_send(ProcessorCommand::Request(Box::new(request))) { + Ok(()) => {} + Err(mpsc::error::TrySendError::Full(_)) => { + if let Some(response_tx) = + pending_request_responses.remove(&request_id) + { + let _ = response_tx.send(Err(JSONRPCErrorError { + code: OVERLOADED_ERROR_CODE, + message: "in-process app-server request queue is full" + .to_string(), + data: None, + })); + } + } + Err(mpsc::error::TrySendError::Closed(_)) => { + if let Some(response_tx) = + pending_request_responses.remove(&request_id) + { + let _ = response_tx.send(Err(JSONRPCErrorError { + code: INTERNAL_ERROR_CODE, + message: + "in-process app-server request processor is closed" + .to_string(), + data: None, + })); + } + break; + } } } Some(InProcessClientMessage::Notification { notification }) => { - processor.process_client_notification(notification).await; + match processor_tx.try_send(ProcessorCommand::Notification(notification)) { + Ok(()) => {} + Err(mpsc::error::TrySendError::Full(_)) => { + warn!("dropping in-process client notification (queue full)"); + } + Err(mpsc::error::TrySendError::Closed(_)) => { + break; + } + } } Some(InProcessClientMessage::ServerRequestResponse { request_id, result }) => { - processor - .process_response(JSONRPCResponse { - id: request_id, - result, - }) + outgoing_message_sender + .notify_client_response(request_id, result) .await; } Some(InProcessClientMessage::ServerRequestError { request_id, error }) => { - processor - .process_error(JSONRPCError { - error, - id: request_id, - }) + outgoing_message_sender + .notify_client_error(request_id, error) .await; } Some(InProcessClientMessage::Shutdown { done_tx }) => { @@ -499,15 +608,15 @@ fn start_uninitialized(args: InProcessStartArgs) -> InProcessClientHandle { InProcessServerEvent::ServerRequest(req) => req.id().clone(), _ => unreachable!("we just sent a ServerRequest variant"), }; - processor - .process_error(JSONRPCError { - id: request_id, - error: JSONRPCErrorError { + outgoing_message_sender + .notify_client_error( + request_id, + JSONRPCErrorError { code, message: message.to_string(), data: None, }, - }) + ) .await; } } @@ -561,30 +670,18 @@ fn start_uninitialized(args: InProcessStartArgs) -> InProcessClientHandle { } } } - created = thread_created_rx.recv(), if listen_for_threads => { - match created { - Ok(thread_id) => { - let connection_ids = if session.initialized { - vec![IN_PROCESS_CONNECTION_ID] - } else { - Vec::::new() - }; - processor - .try_attach_thread_listener(thread_id, connection_ids) - .await; - } - Err(tokio::sync::broadcast::error::RecvError::Lagged(_)) => { - warn!("thread_created receiver lagged; skipping resync"); - } - Err(tokio::sync::broadcast::error::RecvError::Closed) => { - listen_for_threads = false; - } - } - } } } - processor.connection_closed(IN_PROCESS_CONNECTION_ID).await; + drop(writer_rx); + drop(processor_tx); + outgoing_message_sender + .cancel_all_requests(Some(JSONRPCErrorError { + code: INTERNAL_ERROR_CODE, + message: "in-process app-server runtime is shutting down".to_string(), + data: None, + })) + .await; for (_, response_tx) in pending_request_responses { let _ = response_tx.send(Err(JSONRPCErrorError { code: INTERNAL_ERROR_CODE, @@ -593,7 +690,10 @@ fn start_uninitialized(args: InProcessStartArgs) -> InProcessClientHandle { })); } - drop(processor); + if let Err(_elapsed) = timeout(SHUTDOWN_TIMEOUT, &mut processor_handle).await { + processor_handle.abort(); + let _ = processor_handle.await; + } if let Err(_elapsed) = timeout(SHUTDOWN_TIMEOUT, &mut outbound_handle).await { outbound_handle.abort(); let _ = outbound_handle.await; @@ -605,7 +705,7 @@ fn start_uninitialized(args: InProcessStartArgs) -> InProcessClientHandle { }); InProcessClientHandle { - client_tx, + client: InProcessClientSender { client_tx }, event_rx, runtime_handle, } diff --git a/codex-rs/app-server/src/outgoing_message.rs b/codex-rs/app-server/src/outgoing_message.rs index bc38bd36c159..d615e515184b 100644 --- a/codex-rs/app-server/src/outgoing_message.rs +++ b/codex-rs/app-server/src/outgoing_message.rs @@ -273,6 +273,25 @@ impl OutgoingMessageSender { self.take_request_callback(id).await.is_some() } + pub(crate) async fn cancel_all_requests(&self, error: Option) { + let entries = { + let mut request_id_to_callback = self.request_id_to_callback.lock().await; + request_id_to_callback + .drain() + .map(|(_, entry)| entry) + .collect::>() + }; + + if let Some(error) = error { + for entry in entries { + if let Err(err) = entry.callback.send(Err(error.clone())) { + let request_id = entry.request.id(); + warn!("could not notify callback for {request_id:?} due to: {err:?}"); + } + } + } + } + async fn take_request_callback( &self, id: &RequestId, From c8eb062b90fe5fb3e34cffc7c791f1b6a6181b8d Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 8 Mar 2026 17:11:11 -0600 Subject: [PATCH 21/25] Document in-process event delivery flow --- codex-rs/app-server-client/src/lib.rs | 127 +++++++++++++++++++++----- codex-rs/exec/src/lib.rs | 6 ++ 2 files changed, 110 insertions(+), 23 deletions(-) diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index 9f85c18dede9..91667e34fb7c 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -57,6 +57,25 @@ const SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(5); /// `MessageProcessor` continues to produce that shape internally. pub type RequestResult = std::result::Result; +fn event_requires_delivery(event: &InProcessServerEvent) -> bool { + // These terminal events drive surface shutdown/completion state. Dropping + // them under backpressure can leave exec/TUI waiting forever even though + // the underlying turn has already ended. + match event { + InProcessServerEvent::ServerNotification( + codex_app_server_protocol::ServerNotification::TurnCompleted(_), + ) => true, + InProcessServerEvent::LegacyNotification(notification) => matches!( + notification + .method + .strip_prefix("codex/event/") + .unwrap_or(¬ification.method), + "task_complete" | "turn_aborted" | "shutdown_complete" + ), + _ => false, + } +} + /// Layered error for [`InProcessAppServerClient::request_typed`]. /// /// This keeps transport failures, server-side JSON-RPC failures, and response @@ -244,6 +263,9 @@ impl InProcessAppServerClient { match command { Some(ClientCommand::Request { request, response_tx }) => { let request_sender = request_sender.clone(); + // Request waits happen on a detached task so + // this loop can keep draining runtime events + // while the request is blocked on client input. tokio::spawn(async move { let result = request_sender.request(*request).await; let _ = response_tx.send(result); @@ -290,36 +312,65 @@ impl InProcessAppServerClient { }; if skipped_events > 0 { - match event_tx.try_send(InProcessServerEvent::Lagged { - skipped: skipped_events, - }) { - Ok(()) => { - skipped_events = 0; + if event_requires_delivery(&event) { + // Surface lag before the terminal event, but + // do not let the lag marker itself cause us to + // drop the completion/abort notification that + // the caller is blocked on. + if event_tx + .send(InProcessServerEvent::Lagged { + skipped: skipped_events, + }) + .await + .is_err() + { + event_stream_enabled = false; + continue; } - Err(mpsc::error::TrySendError::Full(_)) => { - skipped_events = skipped_events.saturating_add(1); - warn!( - "dropping in-process app-server event because consumer queue is full" - ); - if let InProcessServerEvent::ServerRequest(request) = event { - let _ = request_sender.fail_server_request( - request.id().clone(), - JSONRPCErrorError { - code: -32001, - message: "in-process app-server event queue is full".to_string(), - data: None, - }, + skipped_events = 0; + } else { + match event_tx.try_send(InProcessServerEvent::Lagged { + skipped: skipped_events, + }) { + Ok(()) => { + skipped_events = 0; + } + Err(mpsc::error::TrySendError::Full(_)) => { + skipped_events = skipped_events.saturating_add(1); + warn!( + "dropping in-process app-server event because consumer queue is full" ); + if let InProcessServerEvent::ServerRequest(request) = event { + let _ = request_sender.fail_server_request( + request.id().clone(), + JSONRPCErrorError { + code: -32001, + message: "in-process app-server event queue is full".to_string(), + data: None, + }, + ); + } + continue; + } + Err(mpsc::error::TrySendError::Closed(_)) => { + event_stream_enabled = false; + continue; } - continue; - } - Err(mpsc::error::TrySendError::Closed(_)) => { - event_stream_enabled = false; - continue; } } } + if event_requires_delivery(&event) { + // Block until the consumer catches up for + // terminal notifications; this preserves the + // completion signal even when the queue is + // otherwise saturated. + if event_tx.send(event).await.is_err() { + event_stream_enabled = false; + } + continue; + } + match event_tx.try_send(event) { Ok(()) => {} Err(mpsc::error::TrySendError::Full(event)) => { @@ -708,4 +759,34 @@ mod tests { client.shutdown().await.expect("shutdown should complete"); } + + #[test] + fn event_requires_delivery_marks_terminal_events() { + assert!(event_requires_delivery( + &InProcessServerEvent::ServerNotification( + codex_app_server_protocol::ServerNotification::TurnCompleted( + codex_app_server_protocol::TurnCompletedNotification { + thread_id: "thread".to_string(), + turn: codex_app_server_protocol::Turn { + id: "turn".to_string(), + items: Vec::new(), + status: codex_app_server_protocol::TurnStatus::Completed, + error: None, + }, + } + ) + ) + )); + assert!(event_requires_delivery( + &InProcessServerEvent::LegacyNotification( + codex_app_server_protocol::JSONRPCNotification { + method: "codex/event/turn_aborted".to_string(), + params: None, + } + ) + )); + assert!(!event_requires_delivery(&InProcessServerEvent::Lagged { + skipped: 1 + })); + } } diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index df99e3ef8459..19582c3444d4 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -798,6 +798,12 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> { if matches!(event.msg, EventMsg::SessionConfigured(_)) { continue; } + if matches!(event.msg, EventMsg::Error(_)) { + // The legacy bridge still carries fatal turn failures for + // exec. Preserve the non-zero exit behavior until this + // path is fully replaced by typed server notifications. + error_seen = true; + } match &event.msg { EventMsg::TurnComplete(payload) => { if payload.turn_id != task_id { From 23bb3258c36c91aa65b2ec25d0ce9601001d7549 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 8 Mar 2026 17:27:23 -0600 Subject: [PATCH 22/25] Preserve exec thread scoping for legacy events --- codex-rs/exec/src/lib.rs | 128 ++++++++++++++++++++++++++++----------- 1 file changed, 92 insertions(+), 36 deletions(-) diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 19582c3444d4..b4336db810bc 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -788,13 +788,20 @@ async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> { } } InProcessServerEvent::LegacyNotification(notification) => { - let event = match legacy_notification_to_event(notification) { + let decoded = match decode_legacy_notification(notification) { Ok(event) => event, Err(err) => { warn!("{err}"); continue; } }; + if decoded.conversation_id.as_deref() + != Some(primary_thread_id_for_requests.as_str()) + && decoded.conversation_id.is_some() + { + continue; + } + let event = decoded.event; if matches!(event.msg, EventMsg::SessionConfigured(_)) { continue; } @@ -1041,21 +1048,33 @@ fn lagged_event_warning_message(skipped: usize) -> String { format!("in-process app-server event stream lagged; dropped {skipped} events") } -fn legacy_notification_to_event(notification: JSONRPCNotification) -> Result { +struct DecodedLegacyNotification { + conversation_id: Option, + event: Event, +} + +fn decode_legacy_notification( + notification: JSONRPCNotification, +) -> Result { let value = notification .params .unwrap_or_else(|| serde_json::Value::Object(serde_json::Map::new())); let method = notification.method; let normalized_method = normalize_legacy_notification_method(&method).to_string(); - let serde_json::Value::Object(object) = value else { + let serde_json::Value::Object(mut object) = value else { return Err(format!( "legacy notification `{method}` params were not an object" )); }; + let conversation_id = object + .get("conversationId") + .and_then(serde_json::Value::as_str) + .map(str::to_owned); let mut event_payload = if let Some(serde_json::Value::Object(msg_payload)) = object.get("msg") { serde_json::Value::Object(msg_payload.clone()) } else { + object.remove("conversationId"); serde_json::Value::Object(object) }; let serde_json::Value::Object(ref mut object) = event_payload else { @@ -1070,10 +1089,22 @@ fn legacy_notification_to_event(notification: JSONRPCNotification) -> Result Result { + serde_json::to_value(McpServerElicitationRequestResponse { + action: McpServerElicitationAction::Cancel, + content: None, + meta: None, }) + .map_err(|err| format!("failed to encode mcp elicitation response: {err}")) } async fn request_shutdown( @@ -1139,41 +1170,26 @@ async fn handle_server_request( client: &InProcessAppServerClient, request: ServerRequest, config: &Config, - thread_id: &str, + _thread_id: &str, error_seen: &mut bool, ) { let method = server_request_method_name(&request); let handle_result = match request { - ServerRequest::McpServerElicitationRequest { request_id, params } => { - if params.thread_id != thread_id { - reject_server_request( - client, - request_id, - &method, - format!( - "request targets thread `{}`, but active thread is `{thread_id}`", - params.thread_id - ), - ) - .await - } else { - let response = McpServerElicitationRequestResponse { - action: McpServerElicitationAction::Cancel, - content: None, - meta: None, - }; - match serde_json::to_value(response) { - Ok(value) => { - resolve_server_request( - client, - request_id, - value, - "mcpServer/elicitation/request", - ) - .await - } - Err(err) => Err(format!("failed to encode mcp elicitation response: {err}")), + ServerRequest::McpServerElicitationRequest { request_id, .. } => { + // Exec auto-cancels elicitation instead of surfacing it + // interactively. Preserve that behavior for attached subagent + // threads too so we do not turn a cancel into a decline/error. + match canceled_mcp_server_elicitation_response() { + Ok(value) => { + resolve_server_request( + client, + request_id, + value, + "mcpServer/elicitation/request", + ) + .await } + Err(err) => Err(err), } } ServerRequest::ChatgptAuthTokensRefresh { request_id, params } => { @@ -1739,4 +1755,44 @@ mod tests { "in-process app-server event stream lagged; dropped 7 events".to_string() ); } + + #[test] + fn decode_legacy_notification_preserves_conversation_id() { + let decoded = decode_legacy_notification(JSONRPCNotification { + method: "codex/event/error".to_string(), + params: Some(serde_json::json!({ + "conversationId": "thread-123", + "msg": { + "message": "boom" + } + })), + }) + .expect("legacy notification should decode"); + + assert_eq!(decoded.conversation_id.as_deref(), Some("thread-123")); + assert!(matches!( + decoded.event.msg, + EventMsg::Error(codex_protocol::protocol::ErrorEvent { + message, + codex_error_info: None, + }) if message == "boom" + )); + } + + #[test] + fn canceled_mcp_server_elicitation_response_uses_cancel_action() { + let value = canceled_mcp_server_elicitation_response() + .expect("mcp elicitation cancel response should serialize"); + let response: McpServerElicitationRequestResponse = + serde_json::from_value(value).expect("cancel response should deserialize"); + + assert_eq!( + response, + McpServerElicitationRequestResponse { + action: McpServerElicitationAction::Cancel, + content: None, + meta: None, + } + ); + } } From 183e73e7eea5e737029dd074904c433560fa8670 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 8 Mar 2026 17:41:09 -0600 Subject: [PATCH 23/25] codex: address PR review feedback (#14005) --- codex-rs/app-server-protocol/src/protocol/common.rs | 13 +++++++++++++ codex-rs/app-server/src/app_server_tracing.rs | 5 +++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index 7975231b87e9..8e87aa9ecb74 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -116,6 +116,18 @@ macro_rules! client_request_definitions { $(Self::$variant { request_id, .. } => request_id,)* } } + + pub fn method(&self) -> String { + serde_json::to_value(self) + .ok() + .and_then(|value| { + value + .get("method") + .and_then(serde_json::Value::as_str) + .map(str::to_owned) + }) + .unwrap_or_else(|| "".to_string()) + } } impl crate::experimental_api::ExperimentalApi for ClientRequest { @@ -1145,6 +1157,7 @@ mod tests { params: None, }; assert_eq!(request.id(), &RequestId::Integer(1)); + assert_eq!(request.method(), "account/rateLimits/read"); assert_eq!( json!({ "method": "account/rateLimits/read", diff --git a/codex-rs/app-server/src/app_server_tracing.rs b/codex-rs/app-server/src/app_server_tracing.rs index 3bae285c0ad6..fd32be87c12d 100644 --- a/codex-rs/app-server/src/app_server_tracing.rs +++ b/codex-rs/app-server/src/app_server_tracing.rs @@ -85,12 +85,13 @@ pub(crate) fn typed_request_span( connection_id: ConnectionId, session: &ConnectionSessionState, ) -> Span { + let method = request.method(); let span = info_span!( "app_server.request", otel.kind = "server", - otel.name = "in_process", + otel.name = method, rpc.system = "jsonrpc", - rpc.method = "in_process", + rpc.method = method, rpc.transport = "in-process", rpc.request_id = ?request.id(), app_server.connection_id = ?connection_id, From b4cf3df4cfe54c6e1fde3ed9e19a8f1f4ae67326 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 8 Mar 2026 17:51:22 -0600 Subject: [PATCH 24/25] codex: address PR review feedback (#14005) --- codex-rs/app-server/src/in_process.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/codex-rs/app-server/src/in_process.rs b/codex-rs/app-server/src/in_process.rs index 13c9aac352cf..4e3572ee5dd5 100644 --- a/codex-rs/app-server/src/in_process.rs +++ b/codex-rs/app-server/src/in_process.rs @@ -682,6 +682,9 @@ fn start_uninitialized(args: InProcessStartArgs) -> InProcessClientHandle { data: None, })) .await; + // Drop the runtime's last sender before awaiting the router task so + // `outgoing_rx.recv()` can observe channel closure and exit cleanly. + drop(outgoing_message_sender); for (_, response_tx) in pending_request_responses { let _ = response_tx.send(Err(JSONRPCErrorError { code: INTERNAL_ERROR_CODE, From dce63d417f9ea443894175c8641ace41f3bb448a Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 8 Mar 2026 18:06:22 -0600 Subject: [PATCH 25/25] codex: address PR review feedback (#14005) --- codex-rs/app-server-client/src/lib.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/codex-rs/app-server-client/src/lib.rs b/codex-rs/app-server-client/src/lib.rs index 91667e34fb7c..80a328384fc0 100644 --- a/codex-rs/app-server-client/src/lib.rs +++ b/codex-rs/app-server-client/src/lib.rs @@ -551,10 +551,19 @@ impl InProcessAppServerClient { /// If graceful shutdown exceeds timeout, the worker task is aborted to /// avoid leaking background tasks in embedding callers. pub async fn shutdown(self) -> IoResult<()> { - let mut worker_handle = self.worker_handle; + let Self { + command_tx, + event_rx, + worker_handle, + } = self; + let mut worker_handle = worker_handle; + // Drop the caller-facing receiver before asking the worker to shut + // down. That unblocks any pending must-deliver `event_tx.send(..)` + // so the worker can reach `handle.shutdown()` instead of timing out + // and getting aborted with the runtime still attached. + drop(event_rx); let (response_tx, response_rx) = oneshot::channel(); - if self - .command_tx + if command_tx .send(ClientCommand::Shutdown { response_tx }) .await .is_ok()