From d9c80edb7346c1a3f3079c5757cb3c5564a41842 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Thu, 5 Feb 2026 20:02:31 -0800 Subject: [PATCH] test(ci): harden flaky cross-platform sandbox and pty tests --- codex-rs/exec-server/src/posix/stopwatch.rs | 9 ++- .../linux-sandbox/tests/suite/landlock.rs | 70 ++++++++++++++++--- codex-rs/mcp-server/tests/suite/codex_tool.rs | 22 +++++- codex-rs/utils/pty/src/tests.rs | 5 +- 4 files changed, 90 insertions(+), 16 deletions(-) diff --git a/codex-rs/exec-server/src/posix/stopwatch.rs b/codex-rs/exec-server/src/posix/stopwatch.rs index de29a45685..d3c0869fee 100644 --- a/codex-rs/exec-server/src/posix/stopwatch.rs +++ b/codex-rs/exec-server/src/posix/stopwatch.rs @@ -118,7 +118,6 @@ impl Stopwatch { mod tests { use super::Stopwatch; use tokio::time::Duration; - use tokio::time::Instant; use tokio::time::sleep; use tokio::time::timeout; @@ -126,9 +125,13 @@ mod tests { async fn cancellation_receiver_fires_after_limit() { let stopwatch = Stopwatch::new(Duration::from_millis(50)); let token = stopwatch.cancellation_token(); - let start = Instant::now(); + assert!( + timeout(Duration::from_millis(40), token.cancelled()) + .await + .is_err(), + "stopwatch cancellation fired too early" + ); token.cancelled().await; - assert!(start.elapsed() >= Duration::from_millis(50)); } #[tokio::test] diff --git a/codex-rs/linux-sandbox/tests/suite/landlock.rs b/codex-rs/linux-sandbox/tests/suite/landlock.rs index 75551d4d91..4d803b2297 100644 --- a/codex-rs/linux-sandbox/tests/suite/landlock.rs +++ b/codex-rs/linux-sandbox/tests/suite/landlock.rs @@ -39,7 +39,16 @@ fn create_env_from_core_vars() -> HashMap { #[expect(clippy::print_stdout)] async fn run_cmd(cmd: &[&str], writable_roots: &[PathBuf], timeout_ms: u64) { - let output = run_cmd_output(cmd, writable_roots, timeout_ms).await; + let output = match run_cmd_output(cmd, writable_roots, timeout_ms).await { + Ok(output) => output, + Err(err) if is_landlock_unavailable_error(&err) => { + println!("Skipping Landlock test because restrictions are unavailable: {err}"); + return; + } + Err(err) => { + panic!("sandbox execution failed: {err:?}"); + } + }; if output.exit_code != 0 { println!("stdout:\n{}", output.stdout.text); println!("stderr:\n{}", output.stderr.text); @@ -52,7 +61,7 @@ async fn run_cmd_output( cmd: &[&str], writable_roots: &[PathBuf], timeout_ms: u64, -) -> codex_core::exec::ExecToolCallOutput { +) -> codex_core::error::Result { let cwd = std::env::current_dir().expect("cwd should exist"); let sandbox_cwd = cwd.clone(); let params = ExecParams { @@ -89,7 +98,20 @@ async fn run_cmd_output( None, ) .await - .unwrap() +} + +fn is_landlock_unavailable_error(err: &CodexErr) -> bool { + match err { + CodexErr::Sandbox(SandboxErr::LandlockRestrict) => true, + CodexErr::Sandbox(SandboxErr::Denied { output }) => { + output.stderr.text.contains("LandlockRestrict") + || output + .stderr + .text + .contains("error applying legacy Linux sandbox restrictions") + } + _ => false, + } } #[tokio::test] @@ -98,16 +120,28 @@ async fn test_root_read() { } #[tokio::test] -#[should_panic] async fn test_root_write() { let tmpfile = NamedTempFile::new().unwrap(); let tmpfile_path = tmpfile.path().to_string_lossy(); - run_cmd( + match run_cmd_output( &["bash", "-lc", &format!("echo blah > {tmpfile_path}")], &[], SHORT_TIMEOUT_MS, ) - .await; + .await + { + Err(CodexErr::Sandbox(SandboxErr::Denied { .. })) => {} + Err(err) if is_landlock_unavailable_error(&err) => {} + Ok(output) => { + panic!( + "expected root write to be denied but command exited {}\nstdout:\n{}\nstderr:\n{}", + output.exit_code, output.stdout.text, output.stderr.text + ); + } + Err(err) => { + panic!("expected sandbox deny for root write, got: {err:?}"); + } + } } #[tokio::test] @@ -142,12 +176,17 @@ async fn test_writable_root() { #[tokio::test] async fn test_no_new_privs_is_enabled() { - let output = run_cmd_output( + let output = match run_cmd_output( &["bash", "-lc", "grep '^NoNewPrivs:' /proc/self/status"], &[], SHORT_TIMEOUT_MS, ) - .await; + .await + { + Ok(output) => output, + Err(err) if is_landlock_unavailable_error(&err) => return, + Err(err) => panic!("unexpected sandbox error: {err:?}"), + }; let line = output .stdout .text @@ -158,9 +197,20 @@ async fn test_no_new_privs_is_enabled() { } #[tokio::test] -#[should_panic(expected = "Sandbox(Timeout")] async fn test_timeout() { - run_cmd(&["sleep", "2"], &[], 50).await; + match run_cmd_output(&["sleep", "2"], &[], 50).await { + Err(CodexErr::Sandbox(SandboxErr::Timeout { .. })) => {} + Err(err) if is_landlock_unavailable_error(&err) => {} + Ok(output) => { + panic!( + "expected timeout but command exited {}\nstdout:\n{}\nstderr:\n{}", + output.exit_code, output.stdout.text, output.stderr.text + ); + } + Err(err) => { + panic!("expected timeout error, got: {err:?}"); + } + } } /// Helper that runs `cmd` under the Linux sandbox and asserts that the command diff --git a/codex-rs/mcp-server/tests/suite/codex_tool.rs b/codex-rs/mcp-server/tests/suite/codex_tool.rs index edf2f1b028..85441c8433 100644 --- a/codex-rs/mcp-server/tests/suite/codex_tool.rs +++ b/codex-rs/mcp-server/tests/suite/codex_tool.rs @@ -32,6 +32,19 @@ use mcp_test_support::format_with_current_shell; // Allow ample time on slower CI or under load to avoid flakes. const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(20); +fn find_python() -> Option { + for candidate in ["python3", "python"] { + if let Ok(output) = std::process::Command::new(candidate) + .arg("--version") + .output() + && output.status.success() + { + return Some(candidate.to_string()); + } + } + None +} + /// Test that a shell command that is not on the "trusted" list triggers an /// elicitation request to the MCP and that sending the approval runs the /// command, as expected. @@ -63,13 +76,18 @@ async fn shell_command_approval_triggers_elicitation() -> anyhow::Result<()> { .path() .join(created_filename); + let Some(python) = find_python() else { + println!("Skipping test because python is unavailable in this environment."); + return Ok(()); + }; + let shell_command = vec![ - "python3".to_string(), + python.clone(), "-c".to_string(), format!("import pathlib; pathlib.Path('{created_filename}').touch()"), ]; let expected_shell_command = format_with_current_shell(&format!( - "python3 -c \"import pathlib; pathlib.Path('{created_filename}').touch()\"" + "{python} -c \"import pathlib; pathlib.Path('{created_filename}').touch()\"" )); let McpHandle { diff --git a/codex-rs/utils/pty/src/tests.rs b/codex-rs/utils/pty/src/tests.rs index 89e72eeb4b..b801bed56a 100644 --- a/codex-rs/utils/pty/src/tests.rs +++ b/codex-rs/utils/pty/src/tests.rs @@ -72,7 +72,7 @@ async fn collect_output_until_exit( // On Windows (ConPTY in particular), it's possible to observe the exit notification // before the final bytes are drained from the PTY reader thread. Drain for a brief // "quiet" window to make output assertions deterministic. - let (quiet_ms, max_ms) = if cfg!(windows) { (200, 2_000) } else { (50, 500) }; + let (quiet_ms, max_ms) = if cfg!(windows) { (500, 3_000) } else { (50, 500) }; let quiet = tokio::time::Duration::from_millis(quiet_ms); let max_deadline = tokio::time::Instant::now() + tokio::time::Duration::from_millis(max_ms); @@ -107,6 +107,9 @@ async fn pty_python_repl_emits_output_and_exits() -> anyhow::Result<()> { writer .send(format!("print('hello from pty'){newline}").into_bytes()) .await?; + if cfg!(windows) { + tokio::time::sleep(tokio::time::Duration::from_millis(250)).await; + } writer.send(format!("exit(){newline}").into_bytes()).await?; let timeout_ms = if cfg!(windows) { 10_000 } else { 5_000 };