mirror of
https://github.com/openai/codex.git
synced 2026-05-01 03:42:05 +03:00
Remove stdio transport from exec server (#15119)
Summary - delete the deprecated stdio transport plumbing from the exec server stack - add a basic `exec_server()` harness plus test utilities to start a server, send requests, and await events - refresh exec-server dependencies, configs, and documentation to reflect the new flow Testing - Not run (not requested) --------- Co-authored-by: starr-openai <starr@openai.com> Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -1,20 +1,18 @@
|
||||
use clap::Parser;
|
||||
use codex_exec_server::ExecServerTransport;
|
||||
|
||||
#[derive(Debug, Parser)]
|
||||
struct ExecServerArgs {
|
||||
/// Transport endpoint URL. Supported values: `ws://IP:PORT` (default),
|
||||
/// `stdio://`.
|
||||
/// Transport endpoint URL. Supported values: `ws://IP:PORT` (default).
|
||||
#[arg(
|
||||
long = "listen",
|
||||
value_name = "URL",
|
||||
default_value = ExecServerTransport::DEFAULT_LISTEN_URL
|
||||
default_value = codex_exec_server::DEFAULT_LISTEN_URL
|
||||
)]
|
||||
listen: ExecServerTransport,
|
||||
listen: String,
|
||||
}
|
||||
|
||||
#[tokio::main]
|
||||
async fn main() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
|
||||
let args = ExecServerArgs::parse();
|
||||
codex_exec_server::run_main_with_transport(args.listen).await
|
||||
codex_exec_server::run_main_with_listen_url(&args.listen).await
|
||||
}
|
||||
|
||||
@@ -1,8 +1,6 @@
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
|
||||
use tokio::io::AsyncRead;
|
||||
use tokio::io::AsyncWrite;
|
||||
use tokio::time::timeout;
|
||||
use tokio_tungstenite::connect_async;
|
||||
use tracing::warn;
|
||||
@@ -136,22 +134,6 @@ impl ExecServerClient {
|
||||
Ok(client)
|
||||
}
|
||||
|
||||
pub async fn connect_stdio<R, W>(
|
||||
stdin: W,
|
||||
stdout: R,
|
||||
options: ExecServerClientConnectOptions,
|
||||
) -> Result<Self, ExecServerError>
|
||||
where
|
||||
R: AsyncRead + Unpin + Send + 'static,
|
||||
W: AsyncWrite + Unpin + Send + 'static,
|
||||
{
|
||||
Self::connect(
|
||||
JsonRpcConnection::from_stdio(stdout, stdin, "exec-server stdio".to_string()),
|
||||
options,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
pub async fn connect_websocket(
|
||||
args: RemoteExecServerConnectArgs,
|
||||
) -> Result<Self, ExecServerError> {
|
||||
|
||||
@@ -1,16 +1,21 @@
|
||||
use codex_app_server_protocol::JSONRPCMessage;
|
||||
use futures::SinkExt;
|
||||
use futures::StreamExt;
|
||||
use tokio::io::AsyncBufReadExt;
|
||||
use tokio::io::AsyncRead;
|
||||
use tokio::io::AsyncWrite;
|
||||
use tokio::io::AsyncWriteExt;
|
||||
use tokio::io::BufReader;
|
||||
use tokio::io::BufWriter;
|
||||
use tokio::sync::mpsc;
|
||||
use tokio_tungstenite::WebSocketStream;
|
||||
use tokio_tungstenite::tungstenite::Message;
|
||||
|
||||
#[cfg(test)]
|
||||
use tokio::io::AsyncBufReadExt;
|
||||
#[cfg(test)]
|
||||
use tokio::io::AsyncWriteExt;
|
||||
#[cfg(test)]
|
||||
use tokio::io::BufReader;
|
||||
#[cfg(test)]
|
||||
use tokio::io::BufWriter;
|
||||
|
||||
pub(crate) const CHANNEL_CAPACITY: usize = 128;
|
||||
|
||||
#[derive(Debug)]
|
||||
@@ -27,6 +32,7 @@ pub(crate) struct JsonRpcConnection {
|
||||
}
|
||||
|
||||
impl JsonRpcConnection {
|
||||
#[cfg(test)]
|
||||
pub(crate) fn from_stdio<R, W>(reader: R, writer: W, connection_label: String) -> Self
|
||||
where
|
||||
R: AsyncRead + Unpin + Send + 'static,
|
||||
@@ -256,6 +262,7 @@ async fn send_malformed_message(
|
||||
.await;
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
async fn write_jsonrpc_line_message<W>(
|
||||
writer: &mut BufWriter<W>,
|
||||
message: &JSONRPCMessage,
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
mod client;
|
||||
mod client_api;
|
||||
mod connection;
|
||||
mod local;
|
||||
mod protocol;
|
||||
mod rpc;
|
||||
mod server;
|
||||
@@ -10,12 +9,9 @@ pub use client::ExecServerClient;
|
||||
pub use client::ExecServerError;
|
||||
pub use client_api::ExecServerClientConnectOptions;
|
||||
pub use client_api::RemoteExecServerConnectArgs;
|
||||
pub use local::ExecServerLaunchCommand;
|
||||
pub use local::SpawnedExecServer;
|
||||
pub use local::spawn_local_exec_server;
|
||||
pub use protocol::InitializeParams;
|
||||
pub use protocol::InitializeResponse;
|
||||
pub use server::ExecServerTransport;
|
||||
pub use server::ExecServerTransportParseError;
|
||||
pub use server::DEFAULT_LISTEN_URL;
|
||||
pub use server::ExecServerListenUrlParseError;
|
||||
pub use server::run_main;
|
||||
pub use server::run_main_with_transport;
|
||||
pub use server::run_main_with_listen_url;
|
||||
|
||||
@@ -1,71 +0,0 @@
|
||||
use std::path::PathBuf;
|
||||
use std::process::Stdio;
|
||||
use std::sync::Mutex as StdMutex;
|
||||
|
||||
use tokio::process::Child;
|
||||
use tokio::process::Command;
|
||||
|
||||
use crate::client::ExecServerClient;
|
||||
use crate::client::ExecServerError;
|
||||
use crate::client_api::ExecServerClientConnectOptions;
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct ExecServerLaunchCommand {
|
||||
pub program: PathBuf,
|
||||
pub args: Vec<String>,
|
||||
}
|
||||
|
||||
pub struct SpawnedExecServer {
|
||||
client: ExecServerClient,
|
||||
child: StdMutex<Option<Child>>,
|
||||
}
|
||||
|
||||
impl SpawnedExecServer {
|
||||
pub fn client(&self) -> &ExecServerClient {
|
||||
&self.client
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for SpawnedExecServer {
|
||||
fn drop(&mut self) {
|
||||
if let Ok(mut child_guard) = self.child.lock()
|
||||
&& let Some(child) = child_guard.as_mut()
|
||||
{
|
||||
let _ = child.start_kill();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub async fn spawn_local_exec_server(
|
||||
command: ExecServerLaunchCommand,
|
||||
options: ExecServerClientConnectOptions,
|
||||
) -> Result<SpawnedExecServer, ExecServerError> {
|
||||
let mut child = Command::new(&command.program);
|
||||
child.args(&command.args);
|
||||
child.args(["--listen", "stdio://"]);
|
||||
child.stdin(Stdio::piped());
|
||||
child.stdout(Stdio::piped());
|
||||
child.stderr(Stdio::inherit());
|
||||
child.kill_on_drop(true);
|
||||
|
||||
let mut child = child.spawn().map_err(ExecServerError::Spawn)?;
|
||||
let stdin = child.stdin.take().ok_or_else(|| {
|
||||
ExecServerError::Protocol("exec-server stdin was not captured".to_string())
|
||||
})?;
|
||||
let stdout = child.stdout.take().ok_or_else(|| {
|
||||
ExecServerError::Protocol("exec-server stdout was not captured".to_string())
|
||||
})?;
|
||||
|
||||
let client = match ExecServerClient::connect_stdio(stdin, stdout, options).await {
|
||||
Ok(client) => client,
|
||||
Err(err) => {
|
||||
let _ = child.start_kill();
|
||||
return Err(err);
|
||||
}
|
||||
};
|
||||
|
||||
Ok(SpawnedExecServer {
|
||||
client,
|
||||
child: StdMutex::new(Some(child)),
|
||||
})
|
||||
}
|
||||
@@ -4,15 +4,15 @@ mod processor;
|
||||
mod transport;
|
||||
|
||||
pub(crate) use handler::ExecServerHandler;
|
||||
pub use transport::ExecServerTransport;
|
||||
pub use transport::ExecServerTransportParseError;
|
||||
pub use transport::DEFAULT_LISTEN_URL;
|
||||
pub use transport::ExecServerListenUrlParseError;
|
||||
|
||||
pub async fn run_main() -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
|
||||
run_main_with_transport(ExecServerTransport::Stdio).await
|
||||
run_main_with_listen_url(DEFAULT_LISTEN_URL).await
|
||||
}
|
||||
|
||||
pub async fn run_main_with_transport(
|
||||
transport: ExecServerTransport,
|
||||
pub async fn run_main_with_listen_url(
|
||||
listen_url: &str,
|
||||
) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
|
||||
transport::run_transport(transport).await
|
||||
transport::run_transport(listen_url).await
|
||||
}
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
use std::net::SocketAddr;
|
||||
use std::str::FromStr;
|
||||
|
||||
use tokio::net::TcpListener;
|
||||
use tokio_tungstenite::accept_async;
|
||||
@@ -8,26 +7,22 @@ use tracing::warn;
|
||||
use crate::connection::JsonRpcConnection;
|
||||
use crate::server::processor::run_connection;
|
||||
|
||||
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
|
||||
pub enum ExecServerTransport {
|
||||
Stdio,
|
||||
WebSocket { bind_address: SocketAddr },
|
||||
}
|
||||
pub const DEFAULT_LISTEN_URL: &str = "ws://127.0.0.1:0";
|
||||
|
||||
#[derive(Debug, Clone, Eq, PartialEq)]
|
||||
pub enum ExecServerTransportParseError {
|
||||
pub enum ExecServerListenUrlParseError {
|
||||
UnsupportedListenUrl(String),
|
||||
InvalidWebSocketListenUrl(String),
|
||||
}
|
||||
|
||||
impl std::fmt::Display for ExecServerTransportParseError {
|
||||
impl std::fmt::Display for ExecServerListenUrlParseError {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
match self {
|
||||
ExecServerTransportParseError::UnsupportedListenUrl(listen_url) => write!(
|
||||
ExecServerListenUrlParseError::UnsupportedListenUrl(listen_url) => write!(
|
||||
f,
|
||||
"unsupported --listen URL `{listen_url}`; expected `stdio://` or `ws://IP:PORT`"
|
||||
"unsupported --listen URL `{listen_url}`; expected `ws://IP:PORT`"
|
||||
),
|
||||
ExecServerTransportParseError::InvalidWebSocketListenUrl(listen_url) => write!(
|
||||
ExecServerListenUrlParseError::InvalidWebSocketListenUrl(listen_url) => write!(
|
||||
f,
|
||||
"invalid websocket --listen URL `{listen_url}`; expected `ws://IP:PORT`"
|
||||
),
|
||||
@@ -35,54 +30,27 @@ impl std::fmt::Display for ExecServerTransportParseError {
|
||||
}
|
||||
}
|
||||
|
||||
impl std::error::Error for ExecServerTransportParseError {}
|
||||
impl std::error::Error for ExecServerListenUrlParseError {}
|
||||
|
||||
impl ExecServerTransport {
|
||||
pub const DEFAULT_LISTEN_URL: &str = "ws://127.0.0.1:0";
|
||||
|
||||
pub fn from_listen_url(listen_url: &str) -> Result<Self, ExecServerTransportParseError> {
|
||||
if listen_url == "stdio://" {
|
||||
return Ok(Self::Stdio);
|
||||
}
|
||||
|
||||
if let Some(socket_addr) = listen_url.strip_prefix("ws://") {
|
||||
let bind_address = socket_addr.parse::<SocketAddr>().map_err(|_| {
|
||||
ExecServerTransportParseError::InvalidWebSocketListenUrl(listen_url.to_string())
|
||||
})?;
|
||||
return Ok(Self::WebSocket { bind_address });
|
||||
}
|
||||
|
||||
Err(ExecServerTransportParseError::UnsupportedListenUrl(
|
||||
listen_url.to_string(),
|
||||
))
|
||||
pub(crate) fn parse_listen_url(
|
||||
listen_url: &str,
|
||||
) -> Result<SocketAddr, ExecServerListenUrlParseError> {
|
||||
if let Some(socket_addr) = listen_url.strip_prefix("ws://") {
|
||||
return socket_addr.parse::<SocketAddr>().map_err(|_| {
|
||||
ExecServerListenUrlParseError::InvalidWebSocketListenUrl(listen_url.to_string())
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
impl FromStr for ExecServerTransport {
|
||||
type Err = ExecServerTransportParseError;
|
||||
|
||||
fn from_str(s: &str) -> Result<Self, Self::Err> {
|
||||
Self::from_listen_url(s)
|
||||
}
|
||||
Err(ExecServerListenUrlParseError::UnsupportedListenUrl(
|
||||
listen_url.to_string(),
|
||||
))
|
||||
}
|
||||
|
||||
pub(crate) async fn run_transport(
|
||||
transport: ExecServerTransport,
|
||||
listen_url: &str,
|
||||
) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
|
||||
match transport {
|
||||
ExecServerTransport::Stdio => {
|
||||
run_connection(JsonRpcConnection::from_stdio(
|
||||
tokio::io::stdin(),
|
||||
tokio::io::stdout(),
|
||||
"exec-server stdio".to_string(),
|
||||
))
|
||||
.await;
|
||||
Ok(())
|
||||
}
|
||||
ExecServerTransport::WebSocket { bind_address } => {
|
||||
run_websocket_listener(bind_address).await
|
||||
}
|
||||
}
|
||||
let bind_address = parse_listen_url(listen_url)?;
|
||||
run_websocket_listener(bind_address).await
|
||||
}
|
||||
|
||||
async fn run_websocket_listener(
|
||||
|
||||
@@ -1,41 +1,31 @@
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
use super::ExecServerTransport;
|
||||
use super::DEFAULT_LISTEN_URL;
|
||||
use super::parse_listen_url;
|
||||
|
||||
#[test]
|
||||
fn exec_server_transport_parses_default_websocket_listen_url() {
|
||||
let transport = ExecServerTransport::from_listen_url(ExecServerTransport::DEFAULT_LISTEN_URL)
|
||||
.expect("default listen URL should parse");
|
||||
fn parse_listen_url_accepts_default_websocket_url() {
|
||||
let bind_address =
|
||||
parse_listen_url(DEFAULT_LISTEN_URL).expect("default listen URL should parse");
|
||||
assert_eq!(
|
||||
transport,
|
||||
ExecServerTransport::WebSocket {
|
||||
bind_address: "127.0.0.1:0".parse().expect("valid socket address"),
|
||||
}
|
||||
bind_address,
|
||||
"127.0.0.1:0".parse().expect("valid socket address")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_server_transport_parses_stdio_listen_url() {
|
||||
let transport =
|
||||
ExecServerTransport::from_listen_url("stdio://").expect("stdio listen URL should parse");
|
||||
assert_eq!(transport, ExecServerTransport::Stdio);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_server_transport_parses_websocket_listen_url() {
|
||||
let transport = ExecServerTransport::from_listen_url("ws://127.0.0.1:1234")
|
||||
.expect("websocket listen URL should parse");
|
||||
fn parse_listen_url_accepts_websocket_url() {
|
||||
let bind_address =
|
||||
parse_listen_url("ws://127.0.0.1:1234").expect("websocket listen URL should parse");
|
||||
assert_eq!(
|
||||
transport,
|
||||
ExecServerTransport::WebSocket {
|
||||
bind_address: "127.0.0.1:1234".parse().expect("valid socket address"),
|
||||
}
|
||||
bind_address,
|
||||
"127.0.0.1:1234".parse().expect("valid socket address")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_server_transport_rejects_invalid_websocket_listen_url() {
|
||||
let err = ExecServerTransport::from_listen_url("ws://localhost:1234")
|
||||
fn parse_listen_url_rejects_invalid_websocket_url() {
|
||||
let err = parse_listen_url("ws://localhost:1234")
|
||||
.expect_err("hostname bind address should be rejected");
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
@@ -44,11 +34,11 @@ fn exec_server_transport_rejects_invalid_websocket_listen_url() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_server_transport_rejects_unsupported_listen_url() {
|
||||
let err = ExecServerTransport::from_listen_url("http://127.0.0.1:1234")
|
||||
.expect_err("unsupported scheme should fail");
|
||||
fn parse_listen_url_rejects_unsupported_url() {
|
||||
let err =
|
||||
parse_listen_url("http://127.0.0.1:1234").expect_err("unsupported scheme should fail");
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
"unsupported --listen URL `http://127.0.0.1:1234`; expected `stdio://` or `ws://IP:PORT`"
|
||||
"unsupported --listen URL `http://127.0.0.1:1234`; expected `ws://IP:PORT`"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user