From 25481592c2b76b09d6bd694a8cc62a9bcc859172 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Tue, 17 Mar 2026 04:12:02 +0000 Subject: [PATCH] Expand exec-server unit test coverage Co-authored-by: Codex --- codex-rs/exec-server/src/client.rs | 158 +++++++++++++++++++++ codex-rs/exec-server/src/server/handler.rs | 107 ++++++++++++++ codex-rs/exec-server/src/server/routing.rs | 42 ++++++ 3 files changed, 307 insertions(+) diff --git a/codex-rs/exec-server/src/client.rs b/codex-rs/exec-server/src/client.rs index ea08a6cb96..9271778921 100644 --- a/codex-rs/exec-server/src/client.rs +++ b/codex-rs/exec-server/src/client.rs @@ -1001,4 +1001,162 @@ mod tests { assert!(!process.has_exited(), "terminate should not imply exit"); assert_eq!(process.exit_code(), None); } + + #[tokio::test] + async fn start_process_rejects_mismatched_process_ids_and_cleans_up_state() { + let (client_stdin, server_reader) = tokio::io::duplex(4096); + let (mut server_writer, client_stdout) = tokio::io::duplex(4096); + + tokio::spawn(async move { + let mut lines = BufReader::new(server_reader).lines(); + + let initialize = read_jsonrpc_line(&mut lines).await; + let JSONRPCMessage::Request(initialize_request) = initialize else { + panic!("expected initialize request"); + }; + write_jsonrpc_line( + &mut server_writer, + JSONRPCMessage::Response(JSONRPCResponse { + id: initialize_request.id, + result: serde_json::json!({ "protocolVersion": PROTOCOL_VERSION }), + }), + ) + .await; + + let initialized = read_jsonrpc_line(&mut lines).await; + let JSONRPCMessage::Notification(notification) = initialized else { + panic!("expected initialized notification"); + }; + assert_eq!(notification.method, INITIALIZED_METHOD); + + let exec_request = read_jsonrpc_line(&mut lines).await; + let JSONRPCMessage::Request(JSONRPCRequest { id, method, .. }) = exec_request else { + panic!("expected exec request"); + }; + assert_eq!(method, EXEC_METHOD); + write_jsonrpc_line( + &mut server_writer, + JSONRPCMessage::Response(JSONRPCResponse { + id, + result: serde_json::json!({ "processId": "other-proc" }), + }), + ) + .await; + }); + + let client = match ExecServerClient::connect_stdio( + client_stdin, + client_stdout, + test_options(), + ) + .await + { + Ok(client) => client, + Err(err) => panic!("failed to connect test client: {err}"), + }; + + let result = client + .start_process(ExecParams { + process_id: "proc-1".to_string(), + argv: vec!["bash".to_string(), "-lc".to_string(), "true".to_string()], + cwd: std::env::current_dir().unwrap_or_else(|err| panic!("missing cwd: {err}")), + env: HashMap::new(), + tty: true, + arg0: None, + }) + .await; + + match result { + Err(ExecServerError::Protocol(message)) => { + assert_eq!( + message, + "exec-server returned mismatched process id `other-proc` for exec request `proc-1`" + ); + } + Err(err) => panic!("unexpected start_process failure: {err}"), + Ok(_) => panic!("expected protocol failure"), + } + + assert!( + client.inner.processes.lock().await.is_empty(), + "mismatched responses should not leave registered process state behind" + ); + } + + #[tokio::test] + async fn transport_shutdown_marks_processes_exited_without_exit_codes() { + let (client_stdin, server_reader) = tokio::io::duplex(4096); + let (mut server_writer, client_stdout) = tokio::io::duplex(4096); + + tokio::spawn(async move { + let mut lines = BufReader::new(server_reader).lines(); + + let initialize = read_jsonrpc_line(&mut lines).await; + let JSONRPCMessage::Request(initialize_request) = initialize else { + panic!("expected initialize request"); + }; + write_jsonrpc_line( + &mut server_writer, + JSONRPCMessage::Response(JSONRPCResponse { + id: initialize_request.id, + result: serde_json::json!({ "protocolVersion": PROTOCOL_VERSION }), + }), + ) + .await; + + let initialized = read_jsonrpc_line(&mut lines).await; + let JSONRPCMessage::Notification(notification) = initialized else { + panic!("expected initialized notification"); + }; + assert_eq!(notification.method, INITIALIZED_METHOD); + + let exec_request = read_jsonrpc_line(&mut lines).await; + let JSONRPCMessage::Request(JSONRPCRequest { id, method, .. }) = exec_request else { + panic!("expected exec request"); + }; + assert_eq!(method, EXEC_METHOD); + write_jsonrpc_line( + &mut server_writer, + JSONRPCMessage::Response(JSONRPCResponse { + id, + result: serde_json::json!({ "processId": "proc-1" }), + }), + ) + .await; + drop(server_writer); + }); + + let client = match ExecServerClient::connect_stdio( + client_stdin, + client_stdout, + test_options(), + ) + .await + { + Ok(client) => client, + Err(err) => panic!("failed to connect test client: {err}"), + }; + + let process = match client + .start_process(ExecParams { + process_id: "proc-1".to_string(), + argv: vec!["bash".to_string(), "-lc".to_string(), "true".to_string()], + cwd: std::env::current_dir().unwrap_or_else(|err| panic!("missing cwd: {err}")), + env: HashMap::new(), + tty: true, + arg0: None, + }) + .await + { + Ok(process) => process, + Err(err) => panic!("failed to start process: {err}"), + }; + + tokio::time::sleep(Duration::from_millis(50)).await; + assert!( + process.has_exited(), + "transport shutdown should mark processes exited" + ); + assert_eq!(process.exit_code(), None); + } } diff --git a/codex-rs/exec-server/src/server/handler.rs b/codex-rs/exec-server/src/server/handler.rs index 9f49b528ae..1664632f89 100644 --- a/codex-rs/exec-server/src/server/handler.rs +++ b/codex-rs/exec-server/src/server/handler.rs @@ -357,6 +357,7 @@ mod tests { use crate::protocol::InitializeParams; use crate::protocol::InitializeResponse; use crate::protocol::PROTOCOL_VERSION; + use crate::protocol::TerminateResponse; use crate::protocol::WriteParams; use crate::server::routing::ExecServerClientNotification; use crate::server::routing::ExecServerInboundMessage; @@ -720,4 +721,110 @@ mod tests { handler.shutdown().await; } + + #[tokio::test] + async fn writes_to_unknown_processes_are_rejected() { + let (outgoing_tx, mut outgoing_rx) = tokio::sync::mpsc::channel(2); + let mut handler = ExecServerHandler::new(outgoing_tx); + + if let Err(err) = handler + .handle_message(ExecServerInboundMessage::Request( + ExecServerRequest::Initialize { + request_id: RequestId::Integer(1), + params: InitializeParams { + client_name: "test".to_string(), + }, + }, + )) + .await + { + panic!("initialize should succeed: {err}"); + } + let _ = recv_outbound(&mut outgoing_rx).await; + if let Err(err) = handler + .handle_message(ExecServerInboundMessage::Notification( + ExecServerClientNotification::Initialized, + )) + .await + { + panic!("initialized should succeed: {err}"); + } + + if let Err(err) = handler + .handle_message(ExecServerInboundMessage::Request( + ExecServerRequest::Write { + request_id: RequestId::Integer(2), + params: WriteParams { + process_id: "missing".to_string(), + chunk: b"hello\n".to_vec().into(), + }, + }, + )) + .await + { + panic!("write should not fail the handler: {err}"); + } + + let ExecServerOutboundMessage::Error { request_id, error } = + recv_outbound(&mut outgoing_rx).await + else { + panic!("expected unknown-process error"); + }; + assert_eq!(request_id, RequestId::Integer(2)); + assert_eq!(error.code, -32600); + assert_eq!(error.message, "unknown process id missing"); + } + + #[tokio::test] + async fn terminate_unknown_processes_report_running_false() { + let (outgoing_tx, mut outgoing_rx) = tokio::sync::mpsc::channel(2); + let mut handler = ExecServerHandler::new(outgoing_tx); + + if let Err(err) = handler + .handle_message(ExecServerInboundMessage::Request( + ExecServerRequest::Initialize { + request_id: RequestId::Integer(1), + params: InitializeParams { + client_name: "test".to_string(), + }, + }, + )) + .await + { + panic!("initialize should succeed: {err}"); + } + let _ = recv_outbound(&mut outgoing_rx).await; + if let Err(err) = handler + .handle_message(ExecServerInboundMessage::Notification( + ExecServerClientNotification::Initialized, + )) + .await + { + panic!("initialized should succeed: {err}"); + } + + if let Err(err) = handler + .handle_message(ExecServerInboundMessage::Request( + ExecServerRequest::Terminate { + request_id: RequestId::Integer(2), + params: crate::protocol::TerminateParams { + process_id: "missing".to_string(), + }, + }, + )) + .await + { + panic!("terminate should not fail the handler: {err}"); + } + + assert_eq!( + recv_outbound(&mut outgoing_rx).await, + ExecServerOutboundMessage::Response { + request_id: RequestId::Integer(2), + response: ExecServerResponseMessage::Terminate(TerminateResponse { + running: false, + }), + } + ); + } } diff --git a/codex-rs/exec-server/src/server/routing.rs b/codex-rs/exec-server/src/server/routing.rs index aba9c61c93..7090a742b3 100644 --- a/codex-rs/exec-server/src/server/routing.rs +++ b/codex-rs/exec-server/src/server/routing.rs @@ -255,6 +255,7 @@ mod tests { use codex_app_server_protocol::JSONRPCMessage; use codex_app_server_protocol::JSONRPCNotification; use codex_app_server_protocol::JSONRPCRequest; + use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::RequestId; #[test] @@ -397,4 +398,45 @@ mod tests { } ); } + + #[test] + fn unknown_request_methods_return_immediate_invalid_request_errors() { + let routed = route_jsonrpc_message(JSONRPCMessage::Request(JSONRPCRequest { + id: RequestId::Integer(5), + method: "process/unknown".to_string(), + params: Some(json!({})), + trace: None, + })) + .expect("unknown request should still route"); + + assert_eq!( + routed, + RoutedExecServerMessage::ImmediateOutbound(ExecServerOutboundMessage::Error { + request_id: RequestId::Integer(5), + error: super::invalid_request("unknown method: process/unknown".to_string()), + }) + ); + } + + #[test] + fn unexpected_client_notifications_are_rejected() { + let err = route_jsonrpc_message(JSONRPCMessage::Notification(JSONRPCNotification { + method: "process/outputDelta".to_string(), + params: Some(json!({})), + })) + .expect_err("unexpected client notification should fail"); + + assert_eq!(err, "unexpected notification method: process/outputDelta"); + } + + #[test] + fn unexpected_client_responses_are_rejected() { + let err = route_jsonrpc_message(JSONRPCMessage::Response(JSONRPCResponse { + id: RequestId::Integer(6), + result: json!({}), + })) + .expect_err("unexpected client response should fail"); + + assert_eq!(err, "unexpected client response for request id Integer(6)"); + } }