mirror of
https://github.com/openai/codex.git
synced 2026-04-24 06:35:50 +00:00
Expand exec-server unit test coverage
Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
}),
|
||||
}
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user