Skip to content

Commit f7c6737

Browse files
committed
refactor: keep direct Noise client independently landable
Co-authored-by: Codex noreply@openai.com
1 parent c11f490 commit f7c6737

8 files changed

Lines changed: 57 additions & 170 deletions

File tree

codex-rs/core-api/src/lib.rs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,7 @@ pub use codex_core::resolve_installation_id;
4545
pub use codex_core::skills::SkillsManager;
4646
pub use codex_core::thread_store_from_config;
4747
pub use codex_exec_server::EnvironmentManager;
48-
pub use codex_exec_server::ExecServerError;
4948
pub use codex_exec_server::ExecServerRuntimePaths;
50-
pub use codex_exec_server::NoiseChannelIdentity;
51-
pub use codex_exec_server::NoiseChannelPublicKey;
52-
pub use codex_exec_server::NoiseRendezvousConnectArgs;
53-
pub use codex_exec_server::NoiseRendezvousConnectBundle;
54-
pub use codex_exec_server::NoiseRendezvousConnectProvider;
55-
pub use codex_exec_server::SharedNoiseRendezvousConnectProvider;
5649
pub use codex_extension_api::empty_extension_registry;
5750
pub use codex_features::Feature;
5851
pub use codex_features::Features;

codex-rs/exec-server/src/client.rs

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@ use crate::ProcessId;
2323
use crate::client_api::ExecServerClientConnectOptions;
2424
use crate::client_api::ExecServerTransportParams;
2525
use crate::client_api::HttpClient;
26-
use crate::client_api::NoiseRendezvousConnectArgs;
27-
use crate::client_api::NoiseRendezvousConnectBundle;
2826
use crate::client_api::RemoteExecServerConnectArgs;
2927
use crate::client_api::StdioExecServerConnectArgs;
3028
use crate::connection::JsonRpcConnection;
@@ -117,16 +115,6 @@ impl From<RemoteExecServerConnectArgs> for ExecServerClientConnectOptions {
117115
}
118116
}
119117

120-
impl From<NoiseRendezvousConnectArgs> for ExecServerClientConnectOptions {
121-
fn from(value: NoiseRendezvousConnectArgs) -> Self {
122-
Self {
123-
client_name: value.client_name,
124-
initialize_timeout: value.initialize_timeout,
125-
resume_session_id: value.resume_session_id,
126-
}
127-
}
128-
}
129-
130118
impl From<StdioExecServerConnectArgs> for ExecServerClientConnectOptions {
131119
fn from(value: StdioExecServerConnectArgs) -> Self {
132120
Self {
@@ -149,23 +137,6 @@ impl RemoteExecServerConnectArgs {
149137
}
150138
}
151139

152-
impl NoiseRendezvousConnectArgs {
153-
pub fn new(
154-
bundle: NoiseRendezvousConnectBundle,
155-
harness_identity: crate::NoiseChannelIdentity,
156-
client_name: String,
157-
) -> Self {
158-
Self {
159-
bundle,
160-
harness_identity,
161-
client_name,
162-
connect_timeout: CONNECT_TIMEOUT,
163-
initialize_timeout: INITIALIZE_TIMEOUT,
164-
resume_session_id: None,
165-
}
166-
}
167-
}
168-
169140
pub(crate) struct SessionState {
170141
wake_tx: watch::Sender<u64>,
171142
events: ExecProcessEventLog,
@@ -260,7 +231,6 @@ impl LazyRemoteExecServerClient {
260231
if matches!(
261232
&self.transport_params,
262233
ExecServerTransportParams::WebSocketUrl { .. }
263-
| ExecServerTransportParams::NoiseRendezvous { .. }
264234
) =>
265235
{
266236
ExecServerClient::connect_for_transport(self.transport_params.clone()).await?

codex-rs/exec-server/src/client_api.rs

Lines changed: 11 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
use std::collections::HashMap;
22
use std::path::PathBuf;
3-
use std::sync::Arc;
43
use std::time::Duration;
54

65
use futures::future::BoxFuture;
@@ -34,6 +33,11 @@ pub struct RemoteExecServerConnectArgs {
3433
}
3534

3635
/// Registry-authorized material for one Noise rendezvous connection attempt.
36+
///
37+
/// Treat this as an atomic, single-use bundle. The URL authorization, executor
38+
/// registration, pinned executor key, and harness-key authorization describe one
39+
/// physical connection attempt and must not be mixed with values from another
40+
/// registry response.
3741
pub struct NoiseRendezvousConnectBundle {
3842
pub websocket_url: String,
3943
pub environment_id: String,
@@ -58,6 +62,10 @@ impl std::fmt::Debug for NoiseRendezvousConnectBundle {
5862
}
5963

6064
/// Connection arguments for an authenticated Noise rendezvous exec-server.
65+
///
66+
/// `harness_identity` identifies the logical harness endpoint and may be reused
67+
/// across reconnects. In contrast, callers must supply a fresh
68+
/// [`NoiseRendezvousConnectBundle`] for each physical connection attempt.
6169
pub struct NoiseRendezvousConnectArgs {
6270
pub bundle: NoiseRendezvousConnectBundle,
6371
pub harness_identity: NoiseChannelIdentity,
@@ -80,20 +88,6 @@ impl std::fmt::Debug for NoiseRendezvousConnectArgs {
8088
}
8189
}
8290

83-
/// Supplies fresh registry-authorized material for Noise rendezvous connections.
84-
///
85-
/// Implementations must preserve one endpoint-local harness identity while
86-
/// refreshing short-lived registry material for every physical connection attempt.
87-
pub trait NoiseRendezvousConnectProvider: Send + Sync {
88-
/// Environment ID this provider is authorized to connect to.
89-
fn environment_id(&self) -> &str;
90-
91-
/// Returns a fresh atomic bundle for one physical connection attempt.
92-
fn connect_args(&self) -> BoxFuture<'_, Result<NoiseRendezvousConnectArgs, ExecServerError>>;
93-
}
94-
95-
pub type SharedNoiseRendezvousConnectProvider = Arc<dyn NoiseRendezvousConnectProvider>;
96-
9791
/// Stdio connection arguments for a command-backed exec-server.
9892
#[derive(Debug, Clone, PartialEq, Eq)]
9993
pub(crate) struct StdioExecServerConnectArgs {
@@ -113,52 +107,20 @@ pub(crate) struct StdioExecServerCommand {
113107
}
114108

115109
/// Parameters used to connect to a remote exec-server environment.
116-
#[derive(Clone)]
110+
#[derive(Debug, Clone, PartialEq, Eq)]
117111
pub(crate) enum ExecServerTransportParams {
118112
WebSocketUrl {
119113
websocket_url: String,
120114
connect_timeout: Duration,
121115
initialize_timeout: Duration,
122116
},
123-
NoiseRendezvous {
124-
provider: SharedNoiseRendezvousConnectProvider,
125-
},
126117
#[allow(dead_code)]
127118
StdioCommand {
128119
command: StdioExecServerCommand,
129120
initialize_timeout: Duration,
130121
},
131122
}
132123

133-
impl std::fmt::Debug for ExecServerTransportParams {
134-
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
135-
match self {
136-
Self::WebSocketUrl {
137-
websocket_url,
138-
connect_timeout,
139-
initialize_timeout,
140-
} => f
141-
.debug_struct("WebSocketUrl")
142-
.field("websocket_url", websocket_url)
143-
.field("connect_timeout", connect_timeout)
144-
.field("initialize_timeout", initialize_timeout)
145-
.finish(),
146-
Self::NoiseRendezvous { provider } => f
147-
.debug_struct("NoiseRendezvous")
148-
.field("environment_id", &provider.environment_id())
149-
.finish(),
150-
Self::StdioCommand {
151-
command,
152-
initialize_timeout,
153-
} => f
154-
.debug_struct("StdioCommand")
155-
.field("command", command)
156-
.field("initialize_timeout", initialize_timeout)
157-
.finish(),
158-
}
159-
}
160-
}
161-
162124
impl ExecServerTransportParams {
163125
pub(crate) fn websocket_url(websocket_url: String) -> Self {
164126
Self::WebSocketUrl {
@@ -169,6 +131,7 @@ impl ExecServerTransportParams {
169131
}
170132
}
171133

134+
/// Removes URL query and fragment data before a rendezvous URL reaches logs or errors.
172135
pub(crate) fn redacted_websocket_url(websocket_url: &str) -> String {
173136
match url::Url::parse(websocket_url) {
174137
Ok(mut url) => {

codex-rs/exec-server/src/client_transport.rs

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,15 +44,6 @@ impl ExecServerClient {
4444
})
4545
.await
4646
}
47-
crate::client_api::ExecServerTransportParams::NoiseRendezvous { provider } => {
48-
let args = provider.connect_args().await?;
49-
if args.bundle.environment_id != provider.environment_id() {
50-
return Err(ExecServerError::Protocol(
51-
"Noise rendezvous provider returned a different environment id".to_string(),
52-
));
53-
}
54-
Self::connect_noise_rendezvous(args).await
55-
}
5647
crate::client_api::ExecServerTransportParams::StdioCommand {
5748
command,
5849
initialize_timeout,
@@ -94,6 +85,12 @@ impl ExecServerClient {
9485
Self::connect(connection, args.into()).await
9586
}
9687

88+
/// Connects to one exec-server through an authenticated, encrypted rendezvous stream.
89+
///
90+
/// This method pins the executor's Noise public key from `args`, completes
91+
/// the encrypted channel before starting JSON-RPC, and uses the rendezvous
92+
/// websocket only as a ciphertext transport. Callers are responsible for
93+
/// obtaining a fresh atomic connect bundle for every physical connection.
9794
pub async fn connect_noise_rendezvous(
9895
args: NoiseRendezvousConnectArgs,
9996
) -> Result<Self, ExecServerError> {

codex-rs/exec-server/src/environment.rs

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ use crate::ExecServerError;
66
use crate::ExecServerRuntimePaths;
77
use crate::ExecutorFileSystem;
88
use crate::HttpClient;
9-
use crate::SharedNoiseRendezvousConnectProvider;
109
use crate::client::LazyRemoteExecServerClient;
1110
use crate::client::http_client::ReqwestHttpClient;
1211
use crate::client_api::ExecServerTransportParams;
@@ -277,34 +276,6 @@ impl EnvironmentManager {
277276
.insert(environment_id, Arc::new(environment));
278277
Ok(())
279278
}
280-
281-
/// Adds or replaces a named remote environment that connects through an
282-
/// authenticated, end-to-end encrypted rendezvous stream.
283-
pub fn upsert_noise_environment(
284-
&self,
285-
environment_id: String,
286-
provider: SharedNoiseRendezvousConnectProvider,
287-
) -> Result<(), ExecServerError> {
288-
if environment_id.is_empty() {
289-
return Err(ExecServerError::Protocol(
290-
"environment id cannot be empty".to_string(),
291-
));
292-
}
293-
if environment_id != provider.environment_id() {
294-
return Err(ExecServerError::Protocol(
295-
"Noise environment id does not match connection provider".to_string(),
296-
));
297-
}
298-
let environment = Environment::remote_with_transport(
299-
ExecServerTransportParams::NoiseRendezvous { provider },
300-
self.local_runtime_paths.clone(),
301-
);
302-
self.environments
303-
.write()
304-
.unwrap_or_else(std::sync::PoisonError::into_inner)
305-
.insert(environment_id, Arc::new(environment));
306-
Ok(())
307-
}
308279
}
309280

310281
/// Concrete execution/filesystem environment selected for a session.
@@ -411,7 +382,6 @@ impl Environment {
411382
websocket_url: exec_server_url,
412383
..
413384
} => Some(exec_server_url.clone()),
414-
ExecServerTransportParams::NoiseRendezvous { .. } => None,
415385
ExecServerTransportParams::StdioCommand { .. } => None,
416386
};
417387
let client = LazyRemoteExecServerClient::new(remote_transport.clone());

codex-rs/exec-server/src/environment_toml.rs

Lines changed: 27 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ struct EnvironmentToml {
4848
initialize_timeout_sec: Option<Duration>,
4949
}
5050

51-
#[derive(Clone, Debug)]
51+
#[derive(Clone, Debug, PartialEq, Eq)]
5252
struct TomlEnvironmentProvider {
5353
default: EnvironmentDefault,
5454
include_local: bool,
@@ -577,26 +577,18 @@ mod tests {
577577
)
578578
.expect("provider");
579579

580-
let ExecServerTransportParams::StdioCommand {
581-
command,
582-
initialize_timeout,
583-
} = &provider.environments[0].1
584-
else {
585-
panic!("expected stdio transport");
586-
};
587580
assert_eq!(
588-
command,
589-
&StdioExecServerCommand {
590-
program: "ssh".to_string(),
591-
args: Vec::new(),
592-
env: HashMap::new(),
593-
cwd: Some(config_dir.path().join("workspace")),
581+
provider.environments[0].1,
582+
ExecServerTransportParams::StdioCommand {
583+
command: StdioExecServerCommand {
584+
program: "ssh".to_string(),
585+
args: Vec::new(),
586+
env: HashMap::new(),
587+
cwd: Some(config_dir.path().join("workspace")),
588+
},
589+
initialize_timeout: DEFAULT_REMOTE_EXEC_SERVER_INITIALIZE_TIMEOUT,
594590
}
595591
);
596-
assert_eq!(
597-
*initialize_timeout,
598-
DEFAULT_REMOTE_EXEC_SERVER_INITIALIZE_TIMEOUT
599-
);
600592
}
601593

602594
#[test]
@@ -622,35 +614,26 @@ mod tests {
622614
})
623615
.expect("provider");
624616

625-
let ExecServerTransportParams::WebSocketUrl {
626-
websocket_url,
627-
connect_timeout,
628-
initialize_timeout,
629-
} = &provider.environments[0].1
630-
else {
631-
panic!("expected websocket transport");
632-
};
633-
assert_eq!(websocket_url, "ws://127.0.0.1:8765");
634-
assert_eq!(*connect_timeout, Duration::from_secs(12));
635-
assert_eq!(*initialize_timeout, Duration::from_secs(34));
636-
637-
let ExecServerTransportParams::StdioCommand {
638-
command,
639-
initialize_timeout,
640-
} = &provider.environments[1].1
641-
else {
642-
panic!("expected stdio transport");
643-
};
644617
assert_eq!(
645-
command,
646-
&StdioExecServerCommand {
647-
program: "ssh".to_string(),
648-
args: Vec::new(),
649-
env: HashMap::new(),
650-
cwd: None,
618+
provider.environments[0].1,
619+
ExecServerTransportParams::WebSocketUrl {
620+
websocket_url: "ws://127.0.0.1:8765".to_string(),
621+
connect_timeout: Duration::from_secs(12),
622+
initialize_timeout: Duration::from_secs(34),
623+
}
624+
);
625+
assert_eq!(
626+
provider.environments[1].1,
627+
ExecServerTransportParams::StdioCommand {
628+
command: StdioExecServerCommand {
629+
program: "ssh".to_string(),
630+
args: Vec::new(),
631+
env: HashMap::new(),
632+
cwd: None,
633+
},
634+
initialize_timeout: Duration::from_secs(56),
651635
}
652636
);
653-
assert_eq!(*initialize_timeout, Duration::from_secs(56));
654637
}
655638

656639
#[test]

codex-rs/exec-server/src/lib.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,7 @@ pub use client_api::ExecServerClientConnectOptions;
3535
pub use client_api::HttpClient;
3636
pub use client_api::NoiseRendezvousConnectArgs;
3737
pub use client_api::NoiseRendezvousConnectBundle;
38-
pub use client_api::NoiseRendezvousConnectProvider;
3938
pub use client_api::RemoteExecServerConnectArgs;
40-
pub use client_api::SharedNoiseRendezvousConnectProvider;
4139
pub use codex_file_system::CopyOptions;
4240
pub use codex_file_system::CreateDirectoryOptions;
4341
pub use codex_file_system::ExecutorFileSystem;

codex-rs/exec-server/src/noise_relay/mod.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,23 @@ mod harness;
22
mod message_framing;
33
mod ordered_ciphertext;
44

5+
use tokio_tungstenite::tungstenite::protocol::WebSocketConfig;
6+
57
use crate::ExecServerError;
68

79
pub(crate) use harness::noise_harness_connection_from_websocket;
810

11+
// This bounds allocation in tungstenite before protobuf and Noise record
12+
// validation run. It comfortably fits one maximum Noise record plus metadata.
13+
const MAX_NOISE_RELAY_WEBSOCKET_MESSAGE_SIZE: usize = 256 * 1024;
14+
15+
/// Return the websocket limits required by every Noise relay endpoint.
16+
pub(crate) fn noise_relay_websocket_config() -> WebSocketConfig {
17+
WebSocketConfig::default()
18+
.max_frame_size(Some(MAX_NOISE_RELAY_WEBSOCKET_MESSAGE_SIZE))
19+
.max_message_size(Some(MAX_NOISE_RELAY_WEBSOCKET_MESSAGE_SIZE))
20+
}
21+
922
fn take_next_sequence(next_seq: &mut u32) -> Result<u32, ExecServerError> {
1023
// Never wrap: relay sequence is the explicit ordering key for an implicit
1124
// Noise nonce. Reusing zero after u32::MAX would be ambiguous and unsafe.

0 commit comments

Comments
 (0)