Skip to content

Commit ed5d656

Browse files
authored
Revert "chore: sanbox extraction" (#4626)
Reverts #4286
1 parent c43a561 commit ed5d656

File tree

14 files changed

+415
-1417
lines changed

14 files changed

+415
-1417
lines changed

codex-rs/app-server/tests/common/mcp_process.rs

Lines changed: 6 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
use std::collections::VecDeque;
21
use std::path::Path;
32
use std::process::Stdio;
43
use std::sync::atomic::AtomicI64;
@@ -48,7 +47,6 @@ pub struct McpProcess {
4847
process: Child,
4948
stdin: ChildStdin,
5049
stdout: BufReader<ChildStdout>,
51-
pending_user_messages: VecDeque<JSONRPCNotification>,
5250
}
5351

5452
impl McpProcess {
@@ -119,7 +117,6 @@ impl McpProcess {
119117
process,
120118
stdin,
121119
stdout,
122-
pending_user_messages: VecDeque::new(),
123120
})
124121
}
125122

@@ -378,9 +375,8 @@ impl McpProcess {
378375
let message = self.read_jsonrpc_message().await?;
379376

380377
match message {
381-
JSONRPCMessage::Notification(notification) => {
382-
eprintln!("notification: {notification:?}");
383-
self.enqueue_user_message(notification);
378+
JSONRPCMessage::Notification(_) => {
379+
eprintln!("notification: {message:?}");
384380
}
385381
JSONRPCMessage::Request(jsonrpc_request) => {
386382
return jsonrpc_request.try_into().with_context(
@@ -406,9 +402,8 @@ impl McpProcess {
406402
loop {
407403
let message = self.read_jsonrpc_message().await?;
408404
match message {
409-
JSONRPCMessage::Notification(notification) => {
410-
eprintln!("notification: {notification:?}");
411-
self.enqueue_user_message(notification);
405+
JSONRPCMessage::Notification(_) => {
406+
eprintln!("notification: {message:?}");
412407
}
413408
JSONRPCMessage::Request(_) => {
414409
anyhow::bail!("unexpected JSONRPCMessage::Request: {message:?}");
@@ -432,9 +427,8 @@ impl McpProcess {
432427
loop {
433428
let message = self.read_jsonrpc_message().await?;
434429
match message {
435-
JSONRPCMessage::Notification(notification) => {
436-
eprintln!("notification: {notification:?}");
437-
self.enqueue_user_message(notification);
430+
JSONRPCMessage::Notification(_) => {
431+
eprintln!("notification: {message:?}");
438432
}
439433
JSONRPCMessage::Request(_) => {
440434
anyhow::bail!("unexpected JSONRPCMessage::Request: {message:?}");
@@ -457,18 +451,13 @@ impl McpProcess {
457451
) -> anyhow::Result<JSONRPCNotification> {
458452
eprintln!("in read_stream_until_notification_message({method})");
459453

460-
if let Some(notification) = self.take_pending_notification_by_method(method) {
461-
return Ok(notification);
462-
}
463-
464454
loop {
465455
let message = self.read_jsonrpc_message().await?;
466456
match message {
467457
JSONRPCMessage::Notification(notification) => {
468458
if notification.method == method {
469459
return Ok(notification);
470460
}
471-
self.enqueue_user_message(notification);
472461
}
473462
JSONRPCMessage::Request(_) => {
474463
anyhow::bail!("unexpected JSONRPCMessage::Request: {message:?}");
@@ -482,21 +471,4 @@ impl McpProcess {
482471
}
483472
}
484473
}
485-
486-
fn take_pending_notification_by_method(&mut self, method: &str) -> Option<JSONRPCNotification> {
487-
if let Some(pos) = self
488-
.pending_user_messages
489-
.iter()
490-
.position(|notification| notification.method == method)
491-
{
492-
return self.pending_user_messages.remove(pos);
493-
}
494-
None
495-
}
496-
497-
fn enqueue_user_message(&mut self, notification: JSONRPCNotification) {
498-
if notification.method == "codex/event/user_message" {
499-
self.pending_user_messages.push_back(notification);
500-
}
501-
}
502474
}

codex-rs/app-server/tests/suite/codex_message_processor_flow.rs

Lines changed: 0 additions & 233 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ use app_test_support::to_response;
88
use codex_app_server_protocol::AddConversationListenerParams;
99
use codex_app_server_protocol::AddConversationSubscriptionResponse;
1010
use codex_app_server_protocol::ExecCommandApprovalParams;
11-
use codex_app_server_protocol::InputItem;
1211
use codex_app_server_protocol::JSONRPCNotification;
1312
use codex_app_server_protocol::JSONRPCResponse;
1413
use codex_app_server_protocol::NewConversationParams;
@@ -26,10 +25,6 @@ use codex_core::protocol::SandboxPolicy;
2625
use codex_core::protocol_config_types::ReasoningEffort;
2726
use codex_core::protocol_config_types::ReasoningSummary;
2827
use codex_core::spawn::CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR;
29-
use codex_protocol::config_types::SandboxMode;
30-
use codex_protocol::protocol::Event;
31-
use codex_protocol::protocol::EventMsg;
32-
use codex_protocol::protocol::InputMessageKind;
3328
use pretty_assertions::assert_eq;
3429
use std::env;
3530
use tempfile::TempDir;
@@ -372,234 +367,6 @@ async fn test_send_user_turn_changes_approval_policy_behavior() {
372367
}
373368

374369
// Helper: minimal config.toml pointing at mock provider.
375-
376-
#[tokio::test(flavor = "multi_thread", worker_threads = 4)]
377-
async fn test_send_user_turn_updates_sandbox_and_cwd_between_turns() {
378-
if env::var(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR).is_ok() {
379-
println!(
380-
"Skipping test because it cannot execute when network is disabled in a Codex sandbox."
381-
);
382-
return;
383-
}
384-
385-
let tmp = TempDir::new().expect("tmp dir");
386-
let codex_home = tmp.path().join("codex_home");
387-
std::fs::create_dir(&codex_home).expect("create codex home dir");
388-
let workspace_root = tmp.path().join("workspace");
389-
std::fs::create_dir(&workspace_root).expect("create workspace root");
390-
let first_cwd = workspace_root.join("turn1");
391-
let second_cwd = workspace_root.join("turn2");
392-
std::fs::create_dir(&first_cwd).expect("create first cwd");
393-
std::fs::create_dir(&second_cwd).expect("create second cwd");
394-
395-
let responses = vec![
396-
create_shell_sse_response(
397-
vec![
398-
"bash".to_string(),
399-
"-lc".to_string(),
400-
"echo first turn".to_string(),
401-
],
402-
None,
403-
Some(5000),
404-
"call-first",
405-
)
406-
.expect("create first shell response"),
407-
create_final_assistant_message_sse_response("done first")
408-
.expect("create first final assistant message"),
409-
create_shell_sse_response(
410-
vec![
411-
"bash".to_string(),
412-
"-lc".to_string(),
413-
"echo second turn".to_string(),
414-
],
415-
None,
416-
Some(5000),
417-
"call-second",
418-
)
419-
.expect("create second shell response"),
420-
create_final_assistant_message_sse_response("done second")
421-
.expect("create second final assistant message"),
422-
];
423-
let server = create_mock_chat_completions_server(responses).await;
424-
create_config_toml(&codex_home, &server.uri()).expect("write config");
425-
426-
let mut mcp = McpProcess::new(&codex_home)
427-
.await
428-
.expect("spawn mcp process");
429-
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize())
430-
.await
431-
.expect("init timeout")
432-
.expect("init failed");
433-
434-
let new_conv_id = mcp
435-
.send_new_conversation_request(NewConversationParams {
436-
cwd: Some(first_cwd.to_string_lossy().into_owned()),
437-
approval_policy: Some(AskForApproval::Never),
438-
sandbox: Some(SandboxMode::WorkspaceWrite),
439-
..Default::default()
440-
})
441-
.await
442-
.expect("send newConversation");
443-
let new_conv_resp: JSONRPCResponse = timeout(
444-
DEFAULT_READ_TIMEOUT,
445-
mcp.read_stream_until_response_message(RequestId::Integer(new_conv_id)),
446-
)
447-
.await
448-
.expect("newConversation timeout")
449-
.expect("newConversation resp");
450-
let NewConversationResponse {
451-
conversation_id,
452-
model,
453-
..
454-
} = to_response::<NewConversationResponse>(new_conv_resp)
455-
.expect("deserialize newConversation response");
456-
457-
let add_listener_id = mcp
458-
.send_add_conversation_listener_request(AddConversationListenerParams { conversation_id })
459-
.await
460-
.expect("send addConversationListener");
461-
timeout(
462-
DEFAULT_READ_TIMEOUT,
463-
mcp.read_stream_until_response_message(RequestId::Integer(add_listener_id)),
464-
)
465-
.await
466-
.expect("addConversationListener timeout")
467-
.expect("addConversationListener resp");
468-
469-
let first_turn_id = mcp
470-
.send_send_user_turn_request(SendUserTurnParams {
471-
conversation_id,
472-
items: vec![InputItem::Text {
473-
text: "first turn".to_string(),
474-
}],
475-
cwd: first_cwd.clone(),
476-
approval_policy: AskForApproval::Never,
477-
sandbox_policy: SandboxPolicy::WorkspaceWrite {
478-
writable_roots: vec![first_cwd.clone()],
479-
network_access: false,
480-
exclude_tmpdir_env_var: false,
481-
exclude_slash_tmp: false,
482-
},
483-
model: model.clone(),
484-
effort: Some(ReasoningEffort::Medium),
485-
summary: ReasoningSummary::Auto,
486-
})
487-
.await
488-
.expect("send first sendUserTurn");
489-
timeout(
490-
DEFAULT_READ_TIMEOUT,
491-
mcp.read_stream_until_response_message(RequestId::Integer(first_turn_id)),
492-
)
493-
.await
494-
.expect("sendUserTurn 1 timeout")
495-
.expect("sendUserTurn 1 resp");
496-
timeout(
497-
DEFAULT_READ_TIMEOUT,
498-
mcp.read_stream_until_notification_message("codex/event/task_complete"),
499-
)
500-
.await
501-
.expect("task_complete 1 timeout")
502-
.expect("task_complete 1 notification");
503-
504-
let second_turn_id = mcp
505-
.send_send_user_turn_request(SendUserTurnParams {
506-
conversation_id,
507-
items: vec![InputItem::Text {
508-
text: "second turn".to_string(),
509-
}],
510-
cwd: second_cwd.clone(),
511-
approval_policy: AskForApproval::Never,
512-
sandbox_policy: SandboxPolicy::DangerFullAccess,
513-
model: model.clone(),
514-
effort: Some(ReasoningEffort::Medium),
515-
summary: ReasoningSummary::Auto,
516-
})
517-
.await
518-
.expect("send second sendUserTurn");
519-
timeout(
520-
DEFAULT_READ_TIMEOUT,
521-
mcp.read_stream_until_response_message(RequestId::Integer(second_turn_id)),
522-
)
523-
.await
524-
.expect("sendUserTurn 2 timeout")
525-
.expect("sendUserTurn 2 resp");
526-
527-
let mut env_message: Option<String> = None;
528-
let second_cwd_str = second_cwd.to_string_lossy().into_owned();
529-
for _ in 0..10 {
530-
let notification = timeout(
531-
DEFAULT_READ_TIMEOUT,
532-
mcp.read_stream_until_notification_message("codex/event/user_message"),
533-
)
534-
.await
535-
.expect("user_message timeout")
536-
.expect("user_message notification");
537-
let params = notification
538-
.params
539-
.clone()
540-
.expect("user_message should include params");
541-
let event: Event = serde_json::from_value(params).expect("deserialize user_message event");
542-
if let EventMsg::UserMessage(user) = event.msg
543-
&& matches!(user.kind, Some(InputMessageKind::EnvironmentContext))
544-
&& user.message.contains(&second_cwd_str)
545-
{
546-
env_message = Some(user.message);
547-
break;
548-
}
549-
}
550-
let env_message = env_message.expect("expected environment context update");
551-
assert!(
552-
env_message.contains("<sandbox_mode>danger-full-access</sandbox_mode>"),
553-
"env context should reflect new sandbox mode: {env_message}"
554-
);
555-
assert!(
556-
env_message.contains("<network_access>enabled</network_access>"),
557-
"env context should enable network access for danger-full-access policy: {env_message}"
558-
);
559-
assert!(
560-
env_message.contains(&second_cwd_str),
561-
"env context should include updated cwd: {env_message}"
562-
);
563-
564-
let exec_begin_notification = timeout(
565-
DEFAULT_READ_TIMEOUT,
566-
mcp.read_stream_until_notification_message("codex/event/exec_command_begin"),
567-
)
568-
.await
569-
.expect("exec_command_begin timeout")
570-
.expect("exec_command_begin notification");
571-
let params = exec_begin_notification
572-
.params
573-
.clone()
574-
.expect("exec_command_begin params");
575-
let event: Event = serde_json::from_value(params).expect("deserialize exec begin event");
576-
let exec_begin = match event.msg {
577-
EventMsg::ExecCommandBegin(exec_begin) => exec_begin,
578-
other => panic!("expected ExecCommandBegin event, got {other:?}"),
579-
};
580-
assert_eq!(
581-
exec_begin.cwd, second_cwd,
582-
"exec turn should run from updated cwd"
583-
);
584-
assert_eq!(
585-
exec_begin.command,
586-
vec![
587-
"bash".to_string(),
588-
"-lc".to_string(),
589-
"echo second turn".to_string()
590-
],
591-
"exec turn should run expected command"
592-
);
593-
594-
timeout(
595-
DEFAULT_READ_TIMEOUT,
596-
mcp.read_stream_until_notification_message("codex/event/task_complete"),
597-
)
598-
.await
599-
.expect("task_complete 2 timeout")
600-
.expect("task_complete 2 notification");
601-
}
602-
603370
fn create_config_toml(codex_home: &Path, server_uri: &str) -> std::io::Result<()> {
604371
let config_toml = codex_home.join("config.toml");
605372
std::fs::write(

codex-rs/core/src/apply_patch.rs

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ pub(crate) enum InternalApplyPatchInvocation {
2727
DelegateToExec(ApplyPatchExec),
2828
}
2929

30-
#[derive(Debug)]
3130
pub(crate) struct ApplyPatchExec {
3231
pub(crate) action: ApplyPatchAction,
3332
pub(crate) user_explicitly_approved_this_action: bool,
@@ -110,28 +109,3 @@ pub(crate) fn convert_apply_patch_to_protocol(
110109
}
111110
result
112111
}
113-
114-
#[cfg(test)]
115-
mod tests {
116-
use super::*;
117-
use pretty_assertions::assert_eq;
118-
119-
use tempfile::tempdir;
120-
121-
#[test]
122-
fn convert_apply_patch_maps_add_variant() {
123-
let tmp = tempdir().expect("tmp");
124-
let p = tmp.path().join("a.txt");
125-
// Create an action with a single Add change
126-
let action = ApplyPatchAction::new_add_for_test(&p, "hello".to_string());
127-
128-
let got = convert_apply_patch_to_protocol(&action);
129-
130-
assert_eq!(
131-
got.get(&p),
132-
Some(&FileChange::Add {
133-
content: "hello".to_string()
134-
})
135-
);
136-
}
137-
}

0 commit comments

Comments
 (0)