From 2576fadc742cc0030800214be55c5c7833521679 Mon Sep 17 00:00:00 2001 From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Date: Sun, 3 Aug 2025 11:51:33 -0700 Subject: [PATCH 1/2] shimmer on working (#1807) change the animation on "working" to be a text shimmer https://github.com/user-attachments/assets/f64529eb-1c64-493a-8d97-0f68b964bdd0 --- codex-rs/Cargo.lock | 16 +++++ codex-rs/tui/Cargo.toml | 1 + codex-rs/tui/src/status_indicator_widget.rs | 73 +++++++++++++-------- 3 files changed, 61 insertions(+), 29 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index e1a0e162dc..7d4e41d0b1 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -869,6 +869,7 @@ dependencies = [ "shlex", "strum 0.27.2", "strum_macros 0.27.2", + "supports-color", "textwrap 0.16.2", "tokio", "tracing", @@ -2337,6 +2338,12 @@ dependencies = [ "windows-sys 0.59.0", ] +[[package]] +name = "is_ci" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7655c9839580ee829dfacba1d1278c2b7883e50a277ff7541299489d6bdfdc45" + [[package]] name = "is_terminal_polyfill" version = "1.70.1" @@ -4378,6 +4385,15 @@ version = "2.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "13c2bddecc57b384dee18652358fb23172facb8a2c51ccc10d74c157bdea3292" +[[package]] +name = "supports-color" +version = "3.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c64fc7232dd8d2e4ac5ce4ef302b1d81e0b80d055b9d77c7c4f51f6aa4c867d6" +dependencies = [ + "is_ci", +] + [[package]] name = "syn" version = "1.0.109" diff --git a/codex-rs/tui/Cargo.toml b/codex-rs/tui/Cargo.toml index 823fd1428e..a571b32c8d 100644 --- a/codex-rs/tui/Cargo.toml +++ b/codex-rs/tui/Cargo.toml @@ -48,6 +48,7 @@ serde_json = { version = "1", features = ["preserve_order"] } shlex = "1.3.0" strum = "0.27.2" strum_macros = "0.27.2" +supports-color = "3.0.2" textwrap = "0.16.2" tokio = { version = "1", features = [ "io-std", diff --git a/codex-rs/tui/src/status_indicator_widget.rs b/codex-rs/tui/src/status_indicator_widget.rs index 7e6d267481..aa18ac6fa5 100644 --- a/codex-rs/tui/src/status_indicator_widget.rs +++ b/codex-rs/tui/src/status_indicator_widget.rs @@ -57,7 +57,7 @@ impl StatusIndicatorWidget { thread::spawn(move || { let mut counter = 0usize; while running_clone.load(Ordering::Relaxed) { - std::thread::sleep(Duration::from_millis(200)); + std::thread::sleep(Duration::from_millis(100)); counter = counter.wrapping_add(1); frame_idx_clone.store(counter, Ordering::Relaxed); app_event_tx_clone.send(AppEvent::RequestRedraw); @@ -98,46 +98,51 @@ impl WidgetRef for StatusIndicatorWidget { .borders(Borders::LEFT) .border_type(BorderType::QuadrantOutside) .border_style(widget_style.dim()); - // Animated 3‑dot pattern inside brackets. The *active* dot is bold - // white, the others are dim. - const DOT_COUNT: usize = 3; let idx = self.frame_idx.load(std::sync::atomic::Ordering::Relaxed); - let phase = idx % (DOT_COUNT * 2 - 2); - let active = if phase < DOT_COUNT { - phase - } else { - (DOT_COUNT * 2 - 2) - phase - }; + let header_text = "Working"; + let header_chars: Vec = header_text.chars().collect(); + + let padding = 4usize; // virtual padding around the word for smoother loop + let period = header_chars.len() + padding * 2; + let pos = idx % period; + + let has_true_color = supports_color::on_cached(supports_color::Stream::Stdout) + .map(|level| level.has_16m) + .unwrap_or(false); + + // Width of the bright band (in characters). + let band_half_width = 2.0; let mut header_spans: Vec> = Vec::new(); + for (i, ch) in header_chars.iter().enumerate() { + let i_pos = i as isize + padding as isize; + let pos = pos as isize; + let dist = (i_pos - pos).abs() as f32; - header_spans.push(Span::styled( - "Working ", - Style::default() - .fg(Color::White) - .add_modifier(Modifier::BOLD), - )); + let t = if dist <= band_half_width { + let x = std::f32::consts::PI * (dist / band_half_width); + 0.5 * (1.0 + x.cos()) + } else { + 0.0 + }; - header_spans.push(Span::styled( - "[", - Style::default() - .fg(Color::White) - .add_modifier(Modifier::BOLD), - )); - - for i in 0..DOT_COUNT { - let style = if i == active { + let brightness = 0.4 + 0.6 * t; + let level = (brightness * 255.0).clamp(0.0, 255.0) as u8; + let style = if has_true_color { Style::default() - .fg(Color::White) + .fg(Color::Rgb(level, level, level)) .add_modifier(Modifier::BOLD) } else { - Style::default().dim() + // Bold makes dark gray and gray look the same, so don't use it + // when true color is not supported. + Style::default().fg(color_for_level(level)) }; - header_spans.push(Span::styled(".", style)); + + header_spans.push(Span::styled(ch.to_string(), style)); } header_spans.push(Span::styled( - "] ", + " ", Style::default() .fg(Color::White) .add_modifier(Modifier::BOLD), @@ -189,3 +194,13 @@ impl WidgetRef for StatusIndicatorWidget { paragraph.render_ref(area, buf); } } + +fn color_for_level(level: u8) -> Color { + if level < 128 { + Color::DarkGray + } else if level < 192 { + Color::Gray + } else { + Color::White + } +} From e3565a3f438c30c9d36412d2817346c7accd487c Mon Sep 17 00:00:00 2001 From: Dylan Date: Sun, 3 Aug 2025 13:05:48 -0700 Subject: [PATCH 2/2] [sandbox] Filter out certain non-sandbox errors (#1804) ## Summary Users frequently complain about re-approving commands that have failed for non-sandbox reasons. We can't diagnose with complete accuracy which errors happened because of a sandbox failure, but we can start to eliminate some common simple cases. This PR captures the most common case I've seen, which is a `command not found` error. ## Testing - [x] Added unit tests - [x] Ran a few cases locally --- codex-rs/core/src/exec.rs | 26 +++++++++++--- codex-rs/core/src/lib.rs | 3 +- codex-rs/core/tests/exec.rs | 69 +++++++++++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+), 6 deletions(-) create mode 100644 codex-rs/core/tests/exec.rs diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 5301f0220d..dce02cc5e2 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -140,11 +140,7 @@ pub async fn process_exec_tool_call( let exit_code = raw_output.exit_status.code().unwrap_or(-1); - // NOTE(ragona): This is much less restrictive than the previous check. If we exec - // a command, and it returns anything other than success, we assume that it may have - // been a sandboxing error and allow the user to retry. (The user of course may choose - // not to retry, or in a non-interactive mode, would automatically reject the approval.) - if exit_code != 0 && sandbox_type != SandboxType::None { + if exit_code != 0 && is_likely_sandbox_denied(sandbox_type, exit_code) { return Err(CodexErr::Sandbox(SandboxErr::Denied( exit_code, stdout, stderr, ))); @@ -223,6 +219,26 @@ fn create_linux_sandbox_command_args( linux_cmd } +/// We don't have a fully deterministic way to tell if our command failed +/// because of the sandbox - a command in the user's zshrc file might hit an +/// error, but the command itself might fail or succeed for other reasons. +/// For now, we conservatively check for 'command not found' (exit code 127), +/// and can add additional cases as necessary. +fn is_likely_sandbox_denied(sandbox_type: SandboxType, exit_code: i32) -> bool { + if sandbox_type == SandboxType::None { + return false; + } + + // Quick rejects: well-known non-sandbox shell exit codes + // 127: command not found, 2: misuse of shell builtins + if exit_code == 127 { + return false; + } + + // For all other cases, we assume the sandbox is the cause + true +} + #[derive(Debug)] pub struct RawExecToolCallOutput { pub exit_status: ExitStatus, diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index a33e185afb..80f9014954 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -38,7 +38,7 @@ pub mod plan_tool; mod project_doc; pub mod protocol; mod rollout; -mod safety; +pub(crate) mod safety; pub mod seatbelt; pub mod shell; pub mod spawn; @@ -47,3 +47,4 @@ pub mod util; pub use apply_patch::CODEX_APPLY_PATCH_ARG1; pub use client_common::model_supports_reasoning_summaries; +pub use safety::get_platform_sandbox; diff --git a/codex-rs/core/tests/exec.rs b/codex-rs/core/tests/exec.rs new file mode 100644 index 0000000000..da169296ed --- /dev/null +++ b/codex-rs/core/tests/exec.rs @@ -0,0 +1,69 @@ +#![cfg(target_os = "macos")] +#![expect(clippy::expect_used)] + +use std::collections::HashMap; +use std::sync::Arc; + +use codex_core::exec::ExecParams; +use codex_core::exec::SandboxType; +use codex_core::exec::process_exec_tool_call; +use codex_core::protocol::SandboxPolicy; +use codex_core::spawn::CODEX_SANDBOX_ENV_VAR; +use tempfile::TempDir; +use tokio::sync::Notify; + +use codex_core::get_platform_sandbox; + +async fn run_test_cmd(tmp: TempDir, cmd: Vec<&str>, should_be_ok: bool) { + if std::env::var(CODEX_SANDBOX_ENV_VAR) == Ok("seatbelt".to_string()) { + eprintln!("{CODEX_SANDBOX_ENV_VAR} is set to 'seatbelt', skipping test."); + return; + } + + let sandbox_type = get_platform_sandbox().expect("should be able to get sandbox type"); + assert_eq!(sandbox_type, SandboxType::MacosSeatbelt); + + let params = ExecParams { + command: cmd.iter().map(|s| s.to_string()).collect(), + cwd: tmp.path().to_path_buf(), + timeout_ms: Some(1000), + env: HashMap::new(), + }; + + let ctrl_c = Arc::new(Notify::new()); + let policy = SandboxPolicy::new_read_only_policy(); + + let result = process_exec_tool_call(params, sandbox_type, ctrl_c, &policy, &None, None).await; + + assert!(result.is_ok() == should_be_ok); +} + +/// Command succeeds with exit code 0 normally +#[tokio::test] +async fn exit_code_0_succeeds() { + let tmp = TempDir::new().expect("should be able to create temp dir"); + let cmd = vec!["echo", "hello"]; + + run_test_cmd(tmp, cmd, true).await +} + +/// Command not found returns exit code 127, this is not considered a sandbox error +#[tokio::test] +async fn exit_command_not_found_is_ok() { + let tmp = TempDir::new().expect("should be able to create temp dir"); + let cmd = vec!["/bin/bash", "-c", "nonexistent_command_12345"]; + run_test_cmd(tmp, cmd, true).await +} + +/// Writing a file fails and should be considered a sandbox error +#[tokio::test] +async fn write_file_fails_as_sandbox_error() { + let tmp = TempDir::new().expect("should be able to create temp dir"); + let path = tmp.path().join("test.txt"); + let cmd = vec![ + "/user/bin/touch", + path.to_str().expect("should be able to get path"), + ]; + + run_test_cmd(tmp, cmd, false).await; +}