mirror of
https://github.com/openai/codex.git
synced 2026-10-20 00:18:49 +00:00
Upgrade rmcp to 3.5.1 and harden lifecycle tests (#52972)
## What changed - Update the pinned `rmcp` dependency and Cargo/Bazel lockfiles, and migrate MCP handlers and fixtures to `ClientConfig` and `ServerConfig`. - Stop requiring responses for cancelled elicitation requests in cancellation tests while retaining checks for cleanup and continued connection usability. - Handle client disconnects during WebSocket writes in the TUI test server, use an asynchronous HTTP mock for loopback proxy testing, and improve remote executor test diagnostics and the Windows subprocess fixture. - Use `invalid_prompt` for the memory worker failure fixture. ## Testing Add a regression test covering transport EOF during both form elicitation and user verification. Keep the client alive while checking that pending response channels close, the timeout pause clears, and the service exits with `QuitReason::Closed`. GitOrigin-RevId: c0166b83fab3b59d84a5207c6630cb6adcf3b230
This commit is contained in:
committed by
copyberry
parent
34f33e4ada
commit
de8fab6d7a
Generated
+2
-2
File diff suppressed because one or more lines are too long
Generated
+4
-4
@@ -13098,8 +13098,8 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "rmcp"
|
||||
version = "3.3.0"
|
||||
source = "git+https://github.com/modelcontextprotocol/rust-sdk?rev=3e636cab26c013eca5131103c03d20237f12c4df#3e636cab26c013eca5131103c03d20237f12c4df"
|
||||
version = "3.5.1"
|
||||
source = "git+https://github.com/modelcontextprotocol/rust-sdk?rev=4048ac40e958fb1a7d614617a2ae8d26666ee2fa#4048ac40e958fb1a7d614617a2ae8d26666ee2fa"
|
||||
dependencies = [
|
||||
"async-trait",
|
||||
"base64 0.23.0",
|
||||
@@ -13133,8 +13133,8 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "rmcp-macros"
|
||||
version = "3.3.0"
|
||||
source = "git+https://github.com/modelcontextprotocol/rust-sdk?rev=3e636cab26c013eca5131103c03d20237f12c4df#3e636cab26c013eca5131103c03d20237f12c4df"
|
||||
version = "3.5.1"
|
||||
source = "git+https://github.com/modelcontextprotocol/rust-sdk?rev=4048ac40e958fb1a7d614617a2ae8d26666ee2fa#4048ac40e958fb1a7d614617a2ae8d26666ee2fa"
|
||||
dependencies = [
|
||||
"darling 0.24.0",
|
||||
"proc-macro2",
|
||||
|
||||
+1
-2
@@ -447,8 +447,7 @@ regex-automata = { version = "0.4.13", default-features = false, features = ["st
|
||||
regex-lite = "0.1.8"
|
||||
regex-syntax = "0.8.8"
|
||||
reqwest = { version = "0.12", features = ["cookies"] }
|
||||
# rmcp-v3.3.0 includes enterprise authorization helpers (rust-sdk#1234).
|
||||
rmcp = { git = "https://github.com/modelcontextprotocol/rust-sdk", rev = "3e636cab26c013eca5131103c03d20237f12c4df", version = "=3.3.0", default-features = false }
|
||||
rmcp = { git = "https://github.com/modelcontextprotocol/rust-sdk", rev = "4048ac40e958fb1a7d614617a2ae8d26666ee2fa", version = "=3.5.1", default-features = false }
|
||||
runfiles = { git = "https://github.com/dzbarsky/rules_rust", rev = "b56cbaa8465e74127f1ea216f813cd377295ad81" }
|
||||
rustix = { version = "1.1.4", features = ["net"] }
|
||||
rustls = { version = "0.23.45", default-features = false, features = [
|
||||
|
||||
@@ -36,7 +36,7 @@ use rmcp::model::CallToolResult;
|
||||
use rmcp::model::ContentBlock;
|
||||
use rmcp::model::ListToolsResult;
|
||||
use rmcp::model::ServerCapabilities;
|
||||
use rmcp::model::ServerInfo;
|
||||
use rmcp::model::ServerConfig;
|
||||
use rmcp::model::Tool;
|
||||
use rmcp::transport::StreamableHttpServerConfig;
|
||||
use rmcp::transport::StreamableHttpService;
|
||||
@@ -461,8 +461,8 @@ struct InstalledAppsMcpServer {
|
||||
}
|
||||
|
||||
impl ServerHandler for InstalledAppsMcpServer {
|
||||
fn get_info(&self) -> ServerInfo {
|
||||
ServerInfo::new(ServerCapabilities::builder().enable_tools().build())
|
||||
fn get_info(&self) -> ServerConfig {
|
||||
ServerConfig::new(ServerCapabilities::builder().enable_tools().build())
|
||||
}
|
||||
|
||||
async fn call_tool(
|
||||
|
||||
@@ -43,7 +43,7 @@ use rmcp::model::JsonObject;
|
||||
use rmcp::model::ListToolsResult;
|
||||
use rmcp::model::MetaObject;
|
||||
use rmcp::model::ServerCapabilities;
|
||||
use rmcp::model::ServerInfo;
|
||||
use rmcp::model::ServerConfig;
|
||||
use rmcp::model::Tool;
|
||||
use rmcp::model::ToolAnnotations;
|
||||
use rmcp::transport::StreamableHttpServerConfig;
|
||||
@@ -1497,8 +1497,8 @@ impl AppsServerControl {
|
||||
}
|
||||
|
||||
impl ServerHandler for AppListMcpServer {
|
||||
fn get_info(&self) -> ServerInfo {
|
||||
ServerInfo::new(ServerCapabilities::builder().enable_tools().build())
|
||||
fn get_info(&self) -> ServerConfig {
|
||||
ServerConfig::new(ServerCapabilities::builder().enable_tools().build())
|
||||
}
|
||||
|
||||
fn list_tools(
|
||||
|
||||
@@ -37,7 +37,7 @@ use rmcp::model::CallToolResult;
|
||||
use rmcp::model::JsonObject;
|
||||
use rmcp::model::ListToolsResult;
|
||||
use rmcp::model::ServerCapabilities;
|
||||
use rmcp::model::ServerInfo;
|
||||
use rmcp::model::ServerConfig;
|
||||
use rmcp::model::Tool;
|
||||
use rmcp::model::ToolAnnotations;
|
||||
use rmcp::service::RequestContext;
|
||||
@@ -1507,8 +1507,8 @@ startup_timeout_sec = 10
|
||||
struct ExecutorHttpMcpServer;
|
||||
|
||||
impl ServerHandler for ExecutorHttpMcpServer {
|
||||
fn get_info(&self) -> ServerInfo {
|
||||
ServerInfo::new(ServerCapabilities::builder().enable_tools().build())
|
||||
fn get_info(&self) -> ServerConfig {
|
||||
ServerConfig::new(ServerCapabilities::builder().enable_tools().build())
|
||||
}
|
||||
|
||||
async fn list_tools(
|
||||
|
||||
@@ -105,7 +105,7 @@ use rmcp::model::ReadResourceResult;
|
||||
use rmcp::model::Resource;
|
||||
use rmcp::model::ResourceContents;
|
||||
use rmcp::model::ServerCapabilities;
|
||||
use rmcp::model::ServerInfo;
|
||||
use rmcp::model::ServerConfig;
|
||||
use rmcp::service::RequestContext;
|
||||
use rmcp::service::RoleServer;
|
||||
use rmcp::transport::StreamableHttpServerConfig;
|
||||
@@ -1376,8 +1376,8 @@ struct ResourceAppsMcpServer {
|
||||
}
|
||||
|
||||
impl ServerHandler for ResourceAppsMcpServer {
|
||||
fn get_info(&self) -> ServerInfo {
|
||||
let mut info = ServerInfo::new(ServerCapabilities::builder().enable_resources().build())
|
||||
fn get_info(&self) -> ServerConfig {
|
||||
let mut info = ServerConfig::new(ServerCapabilities::builder().enable_resources().build())
|
||||
.with_protocol_version(ProtocolVersion::V_2025_06_18);
|
||||
if self.calls.tools_enabled.load(Ordering::Relaxed) {
|
||||
info.capabilities.tools = Some(Default::default());
|
||||
|
||||
@@ -64,7 +64,7 @@ use rmcp::model::MetaObject;
|
||||
use rmcp::model::PrimitiveSchemaDefinition;
|
||||
use rmcp::model::RequestMetaObject;
|
||||
use rmcp::model::ServerCapabilities;
|
||||
use rmcp::model::ServerInfo;
|
||||
use rmcp::model::ServerConfig;
|
||||
use rmcp::model::ServerRequest as McpServerRequest;
|
||||
use rmcp::model::Tool;
|
||||
use rmcp::model::ToolAnnotations;
|
||||
@@ -1048,8 +1048,8 @@ impl ServerHandler for ElicitationAppsMcpServer {
|
||||
Ok(self.get_info())
|
||||
}
|
||||
|
||||
fn get_info(&self) -> ServerInfo {
|
||||
ServerInfo::new(ServerCapabilities::builder().enable_tools().build())
|
||||
fn get_info(&self) -> ServerConfig {
|
||||
ServerConfig::new(ServerCapabilities::builder().enable_tools().build())
|
||||
.with_protocol_version(rmcp::model::ProtocolVersion::V_2025_06_18)
|
||||
}
|
||||
|
||||
|
||||
@@ -47,7 +47,7 @@ use rmcp::model::ListResourcesResult;
|
||||
use rmcp::model::ListToolsResult;
|
||||
use rmcp::model::PaginatedRequestParams;
|
||||
use rmcp::model::ServerCapabilities;
|
||||
use rmcp::model::ServerInfo;
|
||||
use rmcp::model::ServerConfig;
|
||||
use rmcp::model::Tool;
|
||||
use rmcp::model::ToolAnnotations;
|
||||
use rmcp::service::RequestContext;
|
||||
@@ -1229,7 +1229,7 @@ struct McpStatusServer {
|
||||
}
|
||||
|
||||
impl ServerHandler for McpStatusServer {
|
||||
fn get_info(&self) -> ServerInfo {
|
||||
fn get_info(&self) -> ServerConfig {
|
||||
let mut capabilities = ServerCapabilities::builder().enable_tools().build();
|
||||
capabilities.extensions = Some(BTreeMap::from([(
|
||||
"openai/settings".to_string(),
|
||||
@@ -1238,7 +1238,7 @@ impl ServerHandler for McpStatusServer {
|
||||
)
|
||||
.expect("settings capability is a JSON object"),
|
||||
)]));
|
||||
ServerInfo::new(capabilities).with_server_info(
|
||||
ServerConfig::new(capabilities).with_server_info(
|
||||
Implementation::new("lookup-server", "1.0.0").with_title("Lookup Server"),
|
||||
)
|
||||
}
|
||||
@@ -1274,8 +1274,8 @@ struct SlowInventoryServer {
|
||||
}
|
||||
|
||||
impl ServerHandler for SlowInventoryServer {
|
||||
fn get_info(&self) -> ServerInfo {
|
||||
ServerInfo::new(
|
||||
fn get_info(&self) -> ServerConfig {
|
||||
ServerConfig::new(
|
||||
ServerCapabilities::builder()
|
||||
.enable_tools()
|
||||
.enable_resources()
|
||||
|
||||
@@ -66,7 +66,7 @@ use rmcp::model::PrimitiveSchemaDefinition;
|
||||
use rmcp::model::ProtocolVersion;
|
||||
use rmcp::model::RequestMetaObject;
|
||||
use rmcp::model::ServerCapabilities;
|
||||
use rmcp::model::ServerInfo;
|
||||
use rmcp::model::ServerConfig;
|
||||
use rmcp::model::Tool;
|
||||
use rmcp::model::ToolAnnotations;
|
||||
use rmcp::service::RequestContext;
|
||||
@@ -1285,8 +1285,8 @@ impl ServerHandler for ToolAppsMcpServer {
|
||||
Ok(self.get_info())
|
||||
}
|
||||
|
||||
fn get_info(&self) -> ServerInfo {
|
||||
ServerInfo::new(ServerCapabilities::builder().enable_tools().build())
|
||||
fn get_info(&self) -> ServerConfig {
|
||||
ServerConfig::new(ServerCapabilities::builder().enable_tools().build())
|
||||
}
|
||||
|
||||
async fn list_tools(
|
||||
|
||||
@@ -53,7 +53,7 @@ use rmcp::model::JsonObject;
|
||||
use rmcp::model::ListToolsResult;
|
||||
use rmcp::model::MetaObject;
|
||||
use rmcp::model::ServerCapabilities;
|
||||
use rmcp::model::ServerInfo;
|
||||
use rmcp::model::ServerConfig;
|
||||
use rmcp::model::Tool;
|
||||
use rmcp::model::ToolAnnotations;
|
||||
use rmcp::transport::StreamableHttpServerConfig;
|
||||
@@ -2632,8 +2632,8 @@ struct PluginInstallMcpServer {
|
||||
}
|
||||
|
||||
impl ServerHandler for PluginInstallMcpServer {
|
||||
fn get_info(&self) -> ServerInfo {
|
||||
ServerInfo::new(ServerCapabilities::builder().enable_tools().build())
|
||||
fn get_info(&self) -> ServerConfig {
|
||||
ServerConfig::new(ServerCapabilities::builder().enable_tools().build())
|
||||
}
|
||||
|
||||
fn list_tools(
|
||||
|
||||
@@ -33,7 +33,7 @@ use rmcp::model::CallToolResult;
|
||||
use rmcp::model::ContentBlock;
|
||||
use rmcp::model::CustomRequest;
|
||||
use rmcp::model::ServerCapabilities;
|
||||
use rmcp::model::ServerInfo;
|
||||
use rmcp::model::ServerConfig;
|
||||
use rmcp::model::ServerRequest;
|
||||
use rmcp::service::RequestContext;
|
||||
use rmcp::service::RoleServer;
|
||||
@@ -73,8 +73,8 @@ impl McpServerContributor<Config> for HostedVerificationServer {
|
||||
}
|
||||
|
||||
impl ServerHandler for VerificationServer {
|
||||
fn get_info(&self) -> ServerInfo {
|
||||
ServerInfo::new(ServerCapabilities::builder().enable_tools().build())
|
||||
fn get_info(&self) -> ServerConfig {
|
||||
ServerConfig::new(ServerCapabilities::builder().enable_tools().build())
|
||||
}
|
||||
|
||||
async fn call_tool(
|
||||
|
||||
@@ -281,7 +281,7 @@ metrics_exporter = {{ otlp-http = {{ endpoint = "{collector_url}/v1/metrics", pr
|
||||
"2",
|
||||
])
|
||||
.stdin(Stdio::piped())
|
||||
.stderr(Stdio::piped())
|
||||
.stderr(Stdio::inherit())
|
||||
.kill_on_drop(true);
|
||||
|
||||
let mut child = command.spawn()?;
|
||||
@@ -290,7 +290,9 @@ metrics_exporter = {{ otlp-http = {{ endpoint = "{collector_url}/v1/metrics", pr
|
||||
.take()
|
||||
.ok_or_else(|| anyhow::anyhow!("remote exec-server stdin was not piped"))?;
|
||||
|
||||
let environment_websocket = accept_parent_lifetime_websocket(&listener, TEST_TIMEOUT).await?;
|
||||
let environment_websocket = accept_parent_lifetime_websocket(&listener, TEST_TIMEOUT)
|
||||
.await
|
||||
.context("remote executor failed to attach")?;
|
||||
// Remote startup must capture the version before registration, not on the first initialize.
|
||||
std::fs::write(&manifest, r#"{"version":"9.9.9"}"#)?;
|
||||
let executor_public_key = registered_parent_lifetime_executor_public_key(®istry).await?;
|
||||
@@ -311,7 +313,9 @@ metrics_exporter = {{ otlp-http = {{ endpoint = "{collector_url}/v1/metrics", pr
|
||||
};
|
||||
let client_task =
|
||||
tokio::spawn(async move { ExecServerClient::connect_noise_rendezvous(harness_args).await });
|
||||
let harness_websocket = accept_parent_lifetime_websocket(&listener, TEST_TIMEOUT).await?;
|
||||
let harness_websocket = accept_parent_lifetime_websocket(&listener, TEST_TIMEOUT)
|
||||
.await
|
||||
.context("harness failed to attach")?;
|
||||
let relay_task = tokio::spawn(proxy_parent_lifetime_relay(
|
||||
environment_websocket,
|
||||
harness_websocket,
|
||||
@@ -342,9 +346,12 @@ metrics_exporter = {{ otlp-http = {{ endpoint = "{collector_url}/v1/metrics", pr
|
||||
|
||||
#[cfg(windows)]
|
||||
let argv = vec![
|
||||
"cmd.exe",
|
||||
"/C",
|
||||
"if defined CODEX_EXEC_SERVER_EXIT_ON_STDIN_CLOSE (exit /b 1) else ping -n 61 127.0.0.1",
|
||||
"powershell.exe",
|
||||
"-NoLogo",
|
||||
"-NoProfile",
|
||||
"-NonInteractive",
|
||||
"-Command",
|
||||
"if (Test-Path Env:CODEX_EXEC_SERVER_EXIT_ON_STDIN_CLOSE) { exit 1 }; Start-Sleep -Seconds 60",
|
||||
];
|
||||
#[cfg(not(windows))]
|
||||
let argv = vec![
|
||||
@@ -385,15 +392,10 @@ metrics_exporter = {{ otlp-http = {{ endpoint = "{collector_url}/v1/metrics", pr
|
||||
|
||||
drop(stdin);
|
||||
|
||||
let output = tokio::time::timeout(TEST_TIMEOUT, child.wait_with_output())
|
||||
let status = tokio::time::timeout(TEST_TIMEOUT, child.wait())
|
||||
.await
|
||||
.map_err(|_| anyhow::anyhow!("remote exec-server did not exit after stdin closed"))??;
|
||||
anyhow::ensure!(
|
||||
output.status.success(),
|
||||
"remote exec-server exited with {}; stderr: {}",
|
||||
output.status,
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
anyhow::ensure!(status.success(), "remote exec-server exited with {status}");
|
||||
|
||||
relay_task.abort();
|
||||
let _ = relay_task.await;
|
||||
|
||||
@@ -1,11 +1,14 @@
|
||||
#![cfg(target_os = "linux")]
|
||||
|
||||
use std::net::TcpListener;
|
||||
use std::time::Duration;
|
||||
use std::time::Instant;
|
||||
|
||||
use anyhow::Result;
|
||||
use tempfile::TempDir;
|
||||
use wiremock::Mock;
|
||||
use wiremock::MockServer;
|
||||
use wiremock::ResponseTemplate;
|
||||
use wiremock::matchers::method;
|
||||
use wiremock::matchers::path;
|
||||
|
||||
const BWRAP_UNAVAILABLE_ERR: &str = "bubblewrap is unavailable";
|
||||
|
||||
@@ -71,50 +74,17 @@ mode = "full"
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sandbox_with_network_proxy_allows_explicit_loopback_access() -> Result<()> {
|
||||
#[tokio::test]
|
||||
async fn sandbox_with_network_proxy_allows_explicit_loopback_access() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let listener = TcpListener::bind("127.0.0.2:0")?;
|
||||
let port = listener.local_addr()?.port();
|
||||
listener.set_nonblocking(true)?;
|
||||
let server = std::thread::spawn(move || -> std::io::Result<()> {
|
||||
let deadline = Instant::now() + Duration::from_secs(5);
|
||||
loop {
|
||||
match listener.accept() {
|
||||
Ok((mut stream, _)) => {
|
||||
// Closing with unread request bytes can reset the connection and yield a 502.
|
||||
stream.set_nonblocking(false)?;
|
||||
stream.set_read_timeout(Some(Duration::from_secs(5)))?;
|
||||
let mut request = Vec::new();
|
||||
let mut buffer = [0_u8; 1024];
|
||||
while !request.windows(4).any(|window| window == b"\r\n\r\n") {
|
||||
let bytes_read = std::io::Read::read(&mut stream, &mut buffer)?;
|
||||
if bytes_read == 0 || request.len() + bytes_read > 64 * 1024 {
|
||||
return Err(std::io::Error::other(
|
||||
"incomplete or oversized loopback request headers",
|
||||
));
|
||||
}
|
||||
request.extend_from_slice(&buffer[..bytes_read]);
|
||||
}
|
||||
std::io::Write::write_all(
|
||||
&mut stream,
|
||||
b"HTTP/1.1 204 No Content\r\nContent-Length: 0\r\n\r\n",
|
||||
)?;
|
||||
return Ok(());
|
||||
}
|
||||
Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => {
|
||||
if Instant::now() >= deadline {
|
||||
return Err(std::io::Error::new(
|
||||
std::io::ErrorKind::TimedOut,
|
||||
"timed out waiting for allowlisted loopback request",
|
||||
));
|
||||
}
|
||||
std::thread::sleep(Duration::from_millis(10));
|
||||
}
|
||||
Err(err) => return Err(err),
|
||||
}
|
||||
}
|
||||
});
|
||||
let server = MockServer::builder().listener(listener).start().await;
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/"))
|
||||
.respond_with(ResponseTemplate::new(204))
|
||||
.mount(&server)
|
||||
.await;
|
||||
std::fs::write(
|
||||
codex_home.path().join("config.toml"),
|
||||
r#"
|
||||
@@ -138,7 +108,7 @@ allow_local_binding = false
|
||||
)?;
|
||||
|
||||
let url = format!("http://127.0.0.2:{port}/");
|
||||
let output = std::process::Command::new(codex_utils_cargo_bin::cargo_bin("codex")?)
|
||||
let output = tokio::process::Command::new(codex_utils_cargo_bin::cargo_bin("codex")?)
|
||||
.env("CODEX_HOME", codex_home.path())
|
||||
.args([
|
||||
"sandbox",
|
||||
@@ -155,7 +125,8 @@ allow_local_binding = false
|
||||
"4",
|
||||
url.as_str(),
|
||||
])
|
||||
.output()?;
|
||||
.output()
|
||||
.await?;
|
||||
|
||||
let stderr = String::from_utf8_lossy(&output.stderr);
|
||||
if stderr.contains(BWRAP_UNAVAILABLE_ERR) {
|
||||
@@ -170,7 +141,5 @@ allow_local_binding = false
|
||||
String::from_utf8_lossy(&output.stdout),
|
||||
stderr,
|
||||
);
|
||||
server.join().expect("loopback server panicked")?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -80,7 +80,7 @@ use rmcp::model::NumberOrString;
|
||||
use rmcp::model::PaginatedRequestParams;
|
||||
use rmcp::model::ProtocolVersion;
|
||||
use rmcp::model::ServerCapabilities;
|
||||
use rmcp::model::ServerInfo;
|
||||
use rmcp::model::ServerConfig;
|
||||
use rmcp::model::Tool;
|
||||
use rmcp::service::RequestContext;
|
||||
use std::collections::HashMap;
|
||||
@@ -290,8 +290,8 @@ struct RefreshTestTransportFactory {
|
||||
}
|
||||
|
||||
impl ServerHandler for RefreshTestTransportFactory {
|
||||
fn get_info(&self) -> ServerInfo {
|
||||
ServerInfo::new(ServerCapabilities::builder().enable_tools().build())
|
||||
fn get_info(&self) -> ServerConfig {
|
||||
ServerConfig::new(ServerCapabilities::builder().enable_tools().build())
|
||||
}
|
||||
|
||||
async fn list_tools(
|
||||
@@ -338,8 +338,8 @@ struct MutableToolsServer {
|
||||
}
|
||||
|
||||
impl ServerHandler for MutableToolsServer {
|
||||
fn get_info(&self) -> ServerInfo {
|
||||
ServerInfo::new(ServerCapabilities::builder().enable_tools().build())
|
||||
fn get_info(&self) -> ServerConfig {
|
||||
ServerConfig::new(ServerCapabilities::builder().enable_tools().build())
|
||||
}
|
||||
|
||||
async fn list_tools(
|
||||
|
||||
@@ -143,7 +143,7 @@ async fn memories_startup_fails_consolidation_when_worker_creates_extension_syml
|
||||
ev_assistant_message("msg-phase2-complete", "phase2 complete"),
|
||||
ev_completed("resp-phase2-complete"),
|
||||
]),
|
||||
sse_failed("resp-phase2-failed", "server_error", "worker failed"),
|
||||
sse_failed("resp-phase2-failed", "invalid_prompt", "worker failed"),
|
||||
];
|
||||
|
||||
for response in responses {
|
||||
|
||||
@@ -11,7 +11,7 @@ use rmcp::model::JsonObject;
|
||||
use rmcp::model::ListToolsResult;
|
||||
use rmcp::model::PaginatedRequestParams;
|
||||
use rmcp::model::ServerCapabilities;
|
||||
use rmcp::model::ServerInfo;
|
||||
use rmcp::model::ServerConfig;
|
||||
use rmcp::model::Tool;
|
||||
use serde::Deserialize;
|
||||
use serde_json::json;
|
||||
@@ -78,8 +78,8 @@ struct EchoArgs {
|
||||
}
|
||||
|
||||
impl ServerHandler for TestToolServer {
|
||||
fn get_info(&self) -> ServerInfo {
|
||||
ServerInfo::new(
|
||||
fn get_info(&self) -> ServerConfig {
|
||||
ServerConfig::new(
|
||||
ServerCapabilities::builder()
|
||||
.enable_tools()
|
||||
.enable_tool_list_changed()
|
||||
|
||||
@@ -28,7 +28,7 @@ use rmcp::model::Resource;
|
||||
use rmcp::model::ResourceContents;
|
||||
use rmcp::model::ResourceTemplate;
|
||||
use rmcp::model::ServerCapabilities;
|
||||
use rmcp::model::ServerInfo;
|
||||
use rmcp::model::ServerConfig;
|
||||
use rmcp::model::Tool;
|
||||
use rmcp::model::ToolAnnotations;
|
||||
use serde::Deserialize;
|
||||
@@ -527,7 +527,7 @@ impl ServerHandler for TestToolServer {
|
||||
Ok(self.get_info())
|
||||
}
|
||||
|
||||
fn get_info(&self) -> ServerInfo {
|
||||
fn get_info(&self) -> ServerConfig {
|
||||
let mut capabilities = ServerCapabilities::builder()
|
||||
.enable_tools()
|
||||
.enable_tool_list_changed()
|
||||
@@ -538,7 +538,7 @@ impl ServerHandler for TestToolServer {
|
||||
JsonObject::new(),
|
||||
)]));
|
||||
|
||||
let server_info = ServerInfo::new(capabilities);
|
||||
let server_info = ServerConfig::new(capabilities);
|
||||
let server_info = match dynamic_server_process_label() {
|
||||
Some(process_label) => server_info
|
||||
.with_server_info(
|
||||
|
||||
@@ -40,7 +40,7 @@ use rmcp::model::Resource;
|
||||
use rmcp::model::ResourceContents;
|
||||
use rmcp::model::ResourceTemplate;
|
||||
use rmcp::model::ServerCapabilities;
|
||||
use rmcp::model::ServerInfo;
|
||||
use rmcp::model::ServerConfig;
|
||||
use rmcp::model::Tool;
|
||||
use rmcp::model::ToolAnnotations;
|
||||
use rmcp::transport::StreamableHttpServerConfig;
|
||||
@@ -251,8 +251,8 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
|
||||
}
|
||||
|
||||
impl ServerHandler for TestToolServer {
|
||||
fn get_info(&self) -> ServerInfo {
|
||||
ServerInfo::new(
|
||||
fn get_info(&self) -> ServerConfig {
|
||||
ServerConfig::new(
|
||||
ServerCapabilities::builder()
|
||||
.enable_tools()
|
||||
.enable_tool_list_changed()
|
||||
|
||||
@@ -7,7 +7,7 @@ use std::sync::PoisonError;
|
||||
use codex_protocol::mcp::OPENAI_ELICITATION_EXTENSION_ID;
|
||||
|
||||
use rmcp::RoleClient;
|
||||
use rmcp::model::ClientInfo;
|
||||
use rmcp::model::ClientConfig;
|
||||
use rmcp::model::ClientResult;
|
||||
use rmcp::model::CustomRequest;
|
||||
use rmcp::model::CustomResult;
|
||||
@@ -95,7 +95,7 @@ impl Drop for PendingVerification {
|
||||
|
||||
impl ElicitationClientService {
|
||||
pub(crate) fn new(
|
||||
client_info: ClientInfo,
|
||||
client_info: ClientConfig,
|
||||
send_elicitation: SendElicitation,
|
||||
pause_state: ElicitationPauseState,
|
||||
) -> Self {
|
||||
@@ -336,7 +336,7 @@ impl Service<RoleClient> for ElicitationClientService {
|
||||
.await
|
||||
}
|
||||
|
||||
fn get_info(&self) -> ClientInfo {
|
||||
fn get_info(&self) -> ClientConfig {
|
||||
<LoggingClientHandler as Service<RoleClient>>::get_info(&self.handler)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3,7 +3,7 @@ use std::sync::Arc;
|
||||
use rmcp::ClientHandler;
|
||||
use rmcp::RoleClient;
|
||||
use rmcp::model::CancelledNotificationParam;
|
||||
use rmcp::model::ClientInfo;
|
||||
use rmcp::model::ClientConfig;
|
||||
use rmcp::model::ElicitRequestParams;
|
||||
use rmcp::model::ElicitResult;
|
||||
#[allow(deprecated)]
|
||||
@@ -24,12 +24,12 @@ use crate::rmcp_client::SendElicitation;
|
||||
|
||||
#[derive(Clone)]
|
||||
pub(crate) struct LoggingClientHandler {
|
||||
client_info: ClientInfo,
|
||||
client_info: ClientConfig,
|
||||
send_elicitation: Arc<SendElicitation>,
|
||||
}
|
||||
|
||||
impl LoggingClientHandler {
|
||||
pub(crate) fn new(client_info: ClientInfo, send_elicitation: SendElicitation) -> Self {
|
||||
pub(crate) fn new(client_info: ClientConfig, send_elicitation: SendElicitation) -> Self {
|
||||
Self {
|
||||
client_info,
|
||||
send_elicitation: Arc::new(send_elicitation),
|
||||
@@ -91,7 +91,7 @@ impl ClientHandler for LoggingClientHandler {
|
||||
info!("MCP server prompt list changed");
|
||||
}
|
||||
|
||||
fn get_info(&self) -> ClientInfo {
|
||||
fn get_info(&self) -> ClientConfig {
|
||||
self.client_info.clone()
|
||||
}
|
||||
|
||||
|
||||
@@ -6,7 +6,7 @@ use crate::tool_input::call_tool;
|
||||
use codex_protocol::mcp::OPENAI_ELICITATION_EXTENSION_ID;
|
||||
use rmcp::RoleServer;
|
||||
use rmcp::model::CallToolRequestParams;
|
||||
use rmcp::model::ClientInfo;
|
||||
use rmcp::model::ClientConfig;
|
||||
use rmcp::model::ClientJsonRpcMessage;
|
||||
use rmcp::model::CustomResult;
|
||||
use rmcp::model::ServerJsonRpcMessage;
|
||||
@@ -28,7 +28,7 @@ async fn connection_closure_releases_pending_inputs_and_timeout_pause() -> anyho
|
||||
let pause_state = ElicitationPauseState::new();
|
||||
let mut paused = pause_state.subscribe();
|
||||
let (route_tx, mut route_rx) = mpsc::unbounded_channel();
|
||||
let mut info = ClientInfo::default();
|
||||
let mut info = ClientConfig::default();
|
||||
info.capabilities.extensions = Some(
|
||||
[(
|
||||
OPENAI_ELICITATION_EXTENSION_ID.into(),
|
||||
|
||||
@@ -13,7 +13,7 @@ use rmcp::ServerHandler;
|
||||
use rmcp::ServiceExt;
|
||||
use rmcp::model::CancelledNotification;
|
||||
use rmcp::model::CancelledNotificationParam;
|
||||
use rmcp::model::ClientInfo;
|
||||
use rmcp::model::ClientConfig;
|
||||
use rmcp::model::ClientJsonRpcMessage;
|
||||
use rmcp::model::CustomRequest;
|
||||
use rmcp::model::ElicitationAction;
|
||||
@@ -23,6 +23,7 @@ use rmcp::model::ServerJsonRpcMessage;
|
||||
use rmcp::model::ServerNotification;
|
||||
use rmcp::model::ServerRequest;
|
||||
use rmcp::service::PeerRequestOptions;
|
||||
use rmcp::service::QuitReason;
|
||||
use rmcp::service::RunningService;
|
||||
use rmcp::service::ServerInitializeError;
|
||||
use rmcp::service::serve_directly;
|
||||
@@ -82,7 +83,7 @@ async fn recovered_connections_accept_elicitations_with_previously_cancelled_ids
|
||||
.await?;
|
||||
client
|
||||
.initialize(
|
||||
ClientInfo::default().with_protocol_version(ProtocolVersion::V_2025_06_18),
|
||||
ClientConfig::default().with_protocol_version(ProtocolVersion::V_2025_06_18),
|
||||
Some(Duration::from_secs(/*secs*/ 5)),
|
||||
Box::new(|_, _| {
|
||||
Box::pin(async {
|
||||
@@ -171,7 +172,7 @@ async fn ordinary_elicitations_release_pending_responses_on_cancellation() -> an
|
||||
let mut paused = pause_state.subscribe();
|
||||
let (route_tx, mut route_rx) = mpsc::unbounded_channel();
|
||||
let service = ElicitationClientService::new(
|
||||
ClientInfo::default(),
|
||||
ClientConfig::default(),
|
||||
Box::new(move |_, _| {
|
||||
let (response_tx, response_rx) = oneshot::channel();
|
||||
route_tx.send(response_tx).expect("observe elicitation");
|
||||
@@ -211,14 +212,6 @@ async fn ordinary_elicitations_release_pending_responses_on_cancellation() -> an
|
||||
paused.wait_for(|paused| !*paused),
|
||||
)
|
||||
.await??;
|
||||
let response = timeout(Duration::from_secs(/*secs*/ 5), server.receive())
|
||||
.await?
|
||||
.expect("cancelled elicitation returned a response");
|
||||
assert_eq!(
|
||||
serde_json::to_value(response)?,
|
||||
json!({"jsonrpc": "2.0", "id": 1, "result": {"action": "cancel"}}),
|
||||
);
|
||||
|
||||
server
|
||||
.send(ServerJsonRpcMessage::request(request, RequestId::Number(2)))
|
||||
.await?;
|
||||
@@ -239,12 +232,88 @@ async fn ordinary_elicitations_release_pending_responses_on_cancellation() -> an
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(start_paused = true)]
|
||||
async fn transport_eof_releases_pending_elicitations_without_dropping_client() -> anyhow::Result<()>
|
||||
{
|
||||
for (method, params) in [
|
||||
(
|
||||
"elicitation/create",
|
||||
json!({
|
||||
"mode": "form",
|
||||
"message": "Confirm",
|
||||
"requestedSchema": {"type": "object", "properties": {}},
|
||||
}),
|
||||
),
|
||||
(
|
||||
"openai/elicitation/create",
|
||||
json!({
|
||||
"mode": "openai/userVerification",
|
||||
"title": "Approve",
|
||||
"description": "",
|
||||
"challenge": "AQID",
|
||||
}),
|
||||
),
|
||||
] {
|
||||
let pause_state = ElicitationPauseState::new();
|
||||
let mut paused = pause_state.subscribe();
|
||||
let (route_tx, mut route_rx) = mpsc::unbounded_channel();
|
||||
let mut client_info = ClientConfig::default();
|
||||
client_info.capabilities.extensions = Some(
|
||||
[(
|
||||
OPENAI_ELICITATION_EXTENSION_ID.to_string(),
|
||||
serde_json::Map::from_iter([("userVerification".to_string(), json!({}))]),
|
||||
)]
|
||||
.into_iter()
|
||||
.collect(),
|
||||
);
|
||||
let service = ElicitationClientService::new(
|
||||
client_info,
|
||||
Box::new(move |_, _| {
|
||||
let (response_tx, response_rx) = oneshot::channel();
|
||||
route_tx
|
||||
.send(response_tx)
|
||||
.expect("observe pending elicitation");
|
||||
Box::pin(async move { Ok(response_rx.await?) })
|
||||
}),
|
||||
pause_state,
|
||||
);
|
||||
let (client_transport, server_transport) = tokio::io::duplex(/*max_buf_size*/ 4096);
|
||||
let client = serve_directly(service, client_transport, /*peer_info*/ None);
|
||||
let mut server = IntoTransport::<RoleServer, _, _>::into_transport(server_transport);
|
||||
// A server can request approval without an outgoing tools/call to detect EOF.
|
||||
server
|
||||
.send(serde_json::from_value::<ServerJsonRpcMessage>(json!({
|
||||
"jsonrpc": "2.0", "id": 1, "method": method, "params": params,
|
||||
}))?)
|
||||
.await?;
|
||||
let mut response_tx = timeout(Duration::from_secs(/*secs*/ 5), route_rx.recv())
|
||||
.await?
|
||||
.expect("elicitation reached the UI");
|
||||
assert!(*paused.borrow());
|
||||
|
||||
drop(server);
|
||||
|
||||
// RMCP allows five seconds to drain responses after EOF. Retain the client
|
||||
// throughout cleanup: dropping it would cancel the handler and hide the bug.
|
||||
timeout(Duration::from_secs(/*secs*/ 7), response_tx.closed()).await?;
|
||||
timeout(
|
||||
Duration::from_secs(/*secs*/ 1),
|
||||
paused.wait_for(|paused| !*paused),
|
||||
)
|
||||
.await??;
|
||||
// Only consume the retained client after observing handler cleanup.
|
||||
let reason = timeout(Duration::from_secs(/*secs*/ 1), client.waiting()).await??;
|
||||
assert!(matches!(reason, QuitReason::Closed));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn user_verification_service_cancellation_drops_pending_response() -> anyhow::Result<()> {
|
||||
let pause_state = ElicitationPauseState::new();
|
||||
let mut paused = pause_state.subscribe();
|
||||
let (route_tx, mut route_rx) = mpsc::unbounded_channel();
|
||||
let mut client_info = ClientInfo::default();
|
||||
let mut client_info = ClientConfig::default();
|
||||
client_info.capabilities.extensions = Some(
|
||||
[(
|
||||
OPENAI_ELICITATION_EXTENSION_ID.to_string(),
|
||||
@@ -303,7 +372,7 @@ async fn cancelling_one_verification_leaves_the_mcp_connection_and_other_request
|
||||
let pause_state = ElicitationPauseState::new();
|
||||
let mut paused = pause_state.subscribe();
|
||||
let (route_tx, mut route_rx) = mpsc::unbounded_channel();
|
||||
let mut client_info = ClientInfo::default();
|
||||
let mut client_info = ClientConfig::default();
|
||||
client_info.capabilities.extensions = Some(
|
||||
[(
|
||||
OPENAI_ELICITATION_EXTENSION_ID.to_string(),
|
||||
@@ -362,17 +431,6 @@ async fn cancelling_one_verification_leaves_the_mcp_connection_and_other_request
|
||||
timeout(Duration::from_secs(/*secs*/ 5), first.closed()).await?;
|
||||
assert!(!second.is_closed());
|
||||
assert!(*paused.borrow());
|
||||
let cancel = timeout(Duration::from_secs(/*secs*/ 5), server.receive())
|
||||
.await?
|
||||
.expect("cancelled verification still sends a response");
|
||||
let ClientJsonRpcMessage::Response(cancel) = cancel else {
|
||||
anyhow::bail!("expected a cancellation response");
|
||||
};
|
||||
assert_eq!(cancel.id, first_id);
|
||||
assert_eq!(
|
||||
serde_json::to_value(cancel.result)?,
|
||||
json!({"action": "cancel", "_meta": {"openai/userVerificationReason": "interrupted"}})
|
||||
);
|
||||
|
||||
for request_id in [first_id, RequestId::Number(999)] {
|
||||
server
|
||||
|
||||
@@ -11,7 +11,7 @@ use std::time::Duration;
|
||||
use tokio::time::timeout;
|
||||
|
||||
fn service() -> ElicitationClientService {
|
||||
let mut info = ClientInfo::default();
|
||||
let mut info = ClientConfig::default();
|
||||
info.capabilities.extensions = Some(
|
||||
[(
|
||||
OPENAI_ELICITATION_EXTENSION_ID.into(),
|
||||
|
||||
@@ -39,11 +39,26 @@ use tokio::net::TcpListener;
|
||||
use tokio::sync::oneshot;
|
||||
use tokio::task::JoinHandle;
|
||||
use tokio_tungstenite::accept_async;
|
||||
use tokio_tungstenite::tungstenite::Error as WebSocketError;
|
||||
use tokio_tungstenite::tungstenite::Message;
|
||||
use tokio_tungstenite::tungstenite::error::ProtocolError;
|
||||
|
||||
pub(super) type RecordedRequests = Arc<Mutex<Vec<JSONRPCRequest>>>;
|
||||
pub(super) type RecordingAppServer = (AppServerSession, RecordedRequests, JoinHandle<Result<()>>);
|
||||
|
||||
fn is_recording_client_disconnect(error: &WebSocketError) -> bool {
|
||||
match error {
|
||||
WebSocketError::Protocol(ProtocolError::ResetWithoutClosingHandshake) => true,
|
||||
WebSocketError::Io(error) => matches!(
|
||||
error.kind(),
|
||||
std::io::ErrorKind::BrokenPipe
|
||||
| std::io::ErrorKind::ConnectionReset
|
||||
| std::io::ErrorKind::ConnectionAborted
|
||||
),
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn daybreak_command_persists_and_confirms_each_selection() -> Result<()> {
|
||||
use codex_protocol::openai_models::ModelAccessPrograms;
|
||||
@@ -495,20 +510,19 @@ pub(super) async fn start_recording_app_server_with_realtime_speech(
|
||||
if let codex_app_server_client::InProcessServerEvent::ServerNotification(notification) = event
|
||||
&& matches!(*notification, ServerNotification::ThreadSettingsUpdated(_))
|
||||
{
|
||||
websocket.send(Message::Text(serde_json::to_string(¬ification)?.into())).await?;
|
||||
let result = websocket.send(Message::Text(serde_json::to_string(¬ification)?.into())).await;
|
||||
if result.as_ref().is_err_and(is_recording_client_disconnect) {
|
||||
break;
|
||||
}
|
||||
result?;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
};
|
||||
let Some(frame) = frame else { break };
|
||||
// The client can close with an unread settings notification during shutdown.
|
||||
match &frame {
|
||||
Err(tokio_tungstenite::tungstenite::Error::Protocol(
|
||||
tokio_tungstenite::tungstenite::error::ProtocolError::ResetWithoutClosingHandshake,
|
||||
)) => break,
|
||||
Err(tokio_tungstenite::tungstenite::Error::Io(error))
|
||||
if matches!(error.kind(), std::io::ErrorKind::BrokenPipe | std::io::ErrorKind::ConnectionReset | std::io::ErrorKind::ConnectionAborted) => break,
|
||||
_ => {}
|
||||
// The client can close with requests or notifications pending during shutdown.
|
||||
if frame.as_ref().is_err_and(is_recording_client_disconnect) {
|
||||
break;
|
||||
}
|
||||
let Message::Text(text) = frame? else {
|
||||
continue;
|
||||
@@ -516,7 +530,7 @@ pub(super) async fn start_recording_app_server_with_realtime_speech(
|
||||
let message = serde_json::from_str::<JSONRPCMessage>(&text)?;
|
||||
match message {
|
||||
JSONRPCMessage::Request(request) if request.method == "initialize" => {
|
||||
websocket
|
||||
let result = websocket
|
||||
.send(Message::Text(
|
||||
serde_json::to_string(&JSONRPCMessage::Response(JSONRPCResponse {
|
||||
id: request.id,
|
||||
@@ -527,7 +541,11 @@ pub(super) async fn start_recording_app_server_with_realtime_speech(
|
||||
}))?
|
||||
.into(),
|
||||
))
|
||||
.await?;
|
||||
.await;
|
||||
if result.as_ref().is_err_and(is_recording_client_disconnect) {
|
||||
break;
|
||||
}
|
||||
result?;
|
||||
}
|
||||
JSONRPCMessage::Request(request) => {
|
||||
request_sink
|
||||
@@ -803,9 +821,13 @@ pub(super) async fn start_recording_app_server_with_realtime_speech(
|
||||
}
|
||||
}
|
||||
};
|
||||
websocket
|
||||
let result = websocket
|
||||
.send(Message::Text(serde_json::to_string(&response)?.into()))
|
||||
.await?;
|
||||
.await;
|
||||
if result.as_ref().is_err_and(is_recording_client_disconnect) {
|
||||
break;
|
||||
}
|
||||
result?;
|
||||
}
|
||||
JSONRPCMessage::Notification(notification)
|
||||
if notification.method == "initialized" => {}
|
||||
|
||||
@@ -31,7 +31,7 @@ use rmcp::model::JsonObject;
|
||||
use rmcp::model::ListToolsResult;
|
||||
use rmcp::model::PaginatedRequestParams;
|
||||
use rmcp::model::ServerCapabilities;
|
||||
use rmcp::model::ServerInfo;
|
||||
use rmcp::model::ServerConfig;
|
||||
use rmcp::model::Tool;
|
||||
use rmcp::model::ToolAnnotations;
|
||||
use rmcp::service::RequestContext;
|
||||
@@ -246,8 +246,8 @@ struct DynamicToolMcpHandler {
|
||||
}
|
||||
|
||||
impl ServerHandler for DynamicToolMcpHandler {
|
||||
fn get_info(&self) -> ServerInfo {
|
||||
ServerInfo::new(ServerCapabilities::builder().enable_tools().build())
|
||||
fn get_info(&self) -> ServerConfig {
|
||||
ServerConfig::new(ServerCapabilities::builder().enable_tools().build())
|
||||
}
|
||||
|
||||
async fn list_tools(
|
||||
|
||||
Reference in New Issue
Block a user