From 92d947d1e0b5fcb0e9ab95f0f296f7a493cff6c4 Mon Sep 17 00:00:00 2001 From: Thibault Sottiaux Date: Mon, 5 Jan 2026 02:22:59 -0800 Subject: [PATCH 1/6] core: respect originator override in tests --- codex-rs/core/tests/suite/client.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index a22027f99fb..ccafba535ee 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -43,6 +43,7 @@ use core_test_support::test_codex::test_codex; use core_test_support::wait_for_event; use dunce::canonicalize as normalize_path; use futures::StreamExt; +use pretty_assertions::assert_eq; use serde_json::json; use std::io::Write; use std::sync::Arc; @@ -384,7 +385,10 @@ async fn includes_conversation_id_and_model_headers_in_request() { request_conversation_id.to_str().unwrap(), conversation_id.to_string() ); - assert_eq!(request_originator.to_str().unwrap(), "codex_cli_rs"); + let expected_originator = + std::env::var(codex_core::default_client::CODEX_INTERNAL_ORIGINATOR_OVERRIDE_ENV_VAR) + .unwrap_or_else(|_| codex_core::default_client::DEFAULT_ORIGINATOR.to_string()); + assert_eq!(request_originator.to_str().unwrap(), expected_originator); assert_eq!( request_authorization.to_str().unwrap(), "Bearer Test API Key" @@ -509,7 +513,10 @@ async fn chatgpt_auth_sends_correct_request() { request_conversation_id.to_str().unwrap(), conversation_id.to_string() ); - assert_eq!(request_originator.to_str().unwrap(), "codex_cli_rs"); + let expected_originator = + std::env::var(codex_core::default_client::CODEX_INTERNAL_ORIGINATOR_OVERRIDE_ENV_VAR) + .unwrap_or_else(|_| codex_core::default_client::DEFAULT_ORIGINATOR.to_string()); + assert_eq!(request_originator.to_str().unwrap(), expected_originator); assert_eq!( request_authorization.to_str().unwrap(), "Bearer Access Token" From 26af5ce80607c27ace84565e98b7292f3617afc4 Mon Sep 17 00:00:00 2001 From: Thibault Sottiaux Date: Mon, 5 Jan 2026 02:23:05 -0800 Subject: [PATCH 2/6] tui: truncate exec approval prefix labels --- .../tui/src/bottom_pane/approval_overlay.rs | 3 +- ...modal_exec_multiline_prefix_truncates.snap | 14 +++++++ codex-rs/tui/src/chatwidget/tests.rs | 41 +++++++++++++++++++ codex-rs/tui/src/exec_command.rs | 27 ++++++++++++ .../tui2/src/bottom_pane/approval_overlay.rs | 3 +- ...modal_exec_multiline_prefix_truncates.snap | 14 +++++++ codex-rs/tui2/src/chatwidget/tests.rs | 39 ++++++++++++++++++ codex-rs/tui2/src/exec_command.rs | 27 ++++++++++++ 8 files changed, 166 insertions(+), 2 deletions(-) create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_multiline_prefix_truncates.snap create mode 100644 codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__approval_modal_exec_multiline_prefix_truncates.snap diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index d42861eb1d5..bb8bb355641 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -9,6 +9,7 @@ use crate::bottom_pane::list_selection_view::ListSelectionView; use crate::bottom_pane::list_selection_view::SelectionItem; use crate::bottom_pane::list_selection_view::SelectionViewParams; use crate::diff_render::DiffSummary; +use crate::exec_command::render_for_approval_prefix_label; use crate::exec_command::strip_bash_lc_and_escape; use crate::history_cell; use crate::key_hint; @@ -462,7 +463,7 @@ fn exec_options( proposed_execpolicy_amendment .filter(|_| features.enabled(Feature::ExecPolicy)) .map(|prefix| { - let rendered_prefix = strip_bash_lc_and_escape(prefix.command()); + let rendered_prefix = render_for_approval_prefix_label(prefix.command()); ApprovalOption { label: format!( "Yes, and don't ask again for commands that start with `{rendered_prefix}`" diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_multiline_prefix_truncates.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_multiline_prefix_truncates.snap new file mode 100644 index 00000000000..7a8840719c8 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_multiline_prefix_truncates.snap @@ -0,0 +1,14 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: contents +--- + Would you like to run the following command? + + $ echo hello world + +› 1. Yes, proceed (y) + 2. Yes, and don't ask again for commands that start with `python - <<'PY' + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx... (truncated)` (p) + 3. No, and tell Codex what to do differently (esc) + + Press enter to confirm or esc to cancel diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index a0ff8d42e9d..1015094369b 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -2465,6 +2465,47 @@ async fn approval_modal_exec_without_reason_snapshot() -> anyhow::Result<()> { Ok(()) } +// Snapshot test: approval modal with a proposed execpolicy prefix that is long enough +// to trigger truncation in the selection label. +#[tokio::test] +async fn approval_modal_exec_multiline_prefix_truncates_snapshot() -> anyhow::Result<()> { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; + chat.config.approval_policy.set(AskForApproval::OnRequest)?; + + let long = format!("python - <<'PY'\n{}\nPY\n", "x".repeat(500)); + let ev = ExecApprovalRequestEvent { + call_id: "call-approve-cmd-multiline-trunc".into(), + turn_id: "turn-approve-cmd-multiline-trunc".into(), + command: vec!["bash".into(), "-lc".into(), "echo hello world".into()], + cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), + reason: None, + proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ + "bash".into(), + "-lc".into(), + long, + ])), + parsed_cmd: vec![], + }; + chat.handle_codex_event(Event { + id: "sub-approve-multiline-trunc".into(), + msg: EventMsg::ExecApprovalRequest(ev), + }); + + let width = 100; + let height = chat.desired_height(width); + let mut terminal = + ratatui::Terminal::new(VT100Backend::new(width, height)).expect("create terminal"); + terminal.set_viewport_area(Rect::new(0, 0, width, height)); + terminal + .draw(|f| chat.render(f.area(), f.buffer_mut())) + .expect("draw approval modal (multiline prefix truncation)"); + let contents = terminal.backend().vt100().screen().contents(); + assert!(contents.contains("(truncated)")); + assert_snapshot!("approval_modal_exec_multiline_prefix_truncates", contents); + + Ok(()) +} + // Snapshot test: patch approval modal #[tokio::test] async fn approval_modal_patch_snapshot() -> anyhow::Result<()> { diff --git a/codex-rs/tui/src/exec_command.rs b/codex-rs/tui/src/exec_command.rs index 8ce6c2632e4..ba7e9cf733b 100644 --- a/codex-rs/tui/src/exec_command.rs +++ b/codex-rs/tui/src/exec_command.rs @@ -5,6 +5,9 @@ use codex_core::parse_command::extract_shell_command; use dirs::home_dir; use shlex::try_join; +const APPROVAL_LABEL_MAX_LEN: usize = 80; +const APPROVAL_LABEL_TRUNCATED_SUFFIX: &str = "... (truncated)"; + pub(crate) fn escape_command(command: &[String]) -> String { try_join(command.iter().map(String::as_str)).unwrap_or_else(|_| command.join(" ")) } @@ -16,6 +19,21 @@ pub(crate) fn strip_bash_lc_and_escape(command: &[String]) -> String { escape_command(command) } +pub(crate) fn render_for_approval_prefix_label(command: &[String]) -> String { + let rendered = strip_bash_lc_and_escape(command); + + // Approval choices are rendered inline in a list; ensure we never introduce + // multi-line labels due to heredocs or embedded newlines. + let collapsed = rendered.split_whitespace().collect::>().join(" "); + let mut chars = collapsed.chars(); + let prefix: String = chars.by_ref().take(APPROVAL_LABEL_MAX_LEN).collect(); + if chars.next().is_some() { + format!("{}{APPROVAL_LABEL_TRUNCATED_SUFFIX}", prefix.trim_end()) + } else { + prefix + } +} + /// If `path` is absolute and inside $HOME, return the part *after* the home /// directory; otherwise, return the path as-is. Note if `path` is the homedir, /// this will return and empty path. @@ -67,4 +85,13 @@ mod tests { let cmdline = strip_bash_lc_and_escape(&args); assert_eq!(cmdline, "echo hello"); } + + #[test] + fn approval_prefix_label_is_single_line_and_truncated() { + let long = format!("python - <<'PY'\n{}\nPY\n", "x".repeat(500)); + let args = vec!["bash".into(), "-lc".into(), long]; + let label = render_for_approval_prefix_label(&args); + assert!(!label.contains('\n')); + assert!(label.ends_with(APPROVAL_LABEL_TRUNCATED_SUFFIX)); + } } diff --git a/codex-rs/tui2/src/bottom_pane/approval_overlay.rs b/codex-rs/tui2/src/bottom_pane/approval_overlay.rs index d42861eb1d5..bb8bb355641 100644 --- a/codex-rs/tui2/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui2/src/bottom_pane/approval_overlay.rs @@ -9,6 +9,7 @@ use crate::bottom_pane::list_selection_view::ListSelectionView; use crate::bottom_pane::list_selection_view::SelectionItem; use crate::bottom_pane::list_selection_view::SelectionViewParams; use crate::diff_render::DiffSummary; +use crate::exec_command::render_for_approval_prefix_label; use crate::exec_command::strip_bash_lc_and_escape; use crate::history_cell; use crate::key_hint; @@ -462,7 +463,7 @@ fn exec_options( proposed_execpolicy_amendment .filter(|_| features.enabled(Feature::ExecPolicy)) .map(|prefix| { - let rendered_prefix = strip_bash_lc_and_escape(prefix.command()); + let rendered_prefix = render_for_approval_prefix_label(prefix.command()); ApprovalOption { label: format!( "Yes, and don't ask again for commands that start with `{rendered_prefix}`" diff --git a/codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__approval_modal_exec_multiline_prefix_truncates.snap b/codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__approval_modal_exec_multiline_prefix_truncates.snap new file mode 100644 index 00000000000..01a3fe71c0d --- /dev/null +++ b/codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__approval_modal_exec_multiline_prefix_truncates.snap @@ -0,0 +1,14 @@ +--- +source: tui2/src/chatwidget/tests.rs +expression: contents +--- + Would you like to run the following command? + + $ echo hello world + +› 1. Yes, proceed (y) + 2. Yes, and don't ask again for commands that start with `python - <<'PY' + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx... (truncated)` (p) + 3. No, and tell Codex what to do differently (esc) + + Press enter to confirm or esc to cancel diff --git a/codex-rs/tui2/src/chatwidget/tests.rs b/codex-rs/tui2/src/chatwidget/tests.rs index 8b216812dfd..fa7d134fdb9 100644 --- a/codex-rs/tui2/src/chatwidget/tests.rs +++ b/codex-rs/tui2/src/chatwidget/tests.rs @@ -2123,6 +2123,45 @@ async fn approval_modal_exec_without_reason_snapshot() { ); } +// Snapshot test: approval modal with a proposed execpolicy prefix that is long enough +// to trigger truncation in the selection label. +#[tokio::test] +async fn approval_modal_exec_multiline_prefix_truncates_snapshot() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; + chat.config.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); + + let long = format!("python - <<'PY'\n{}\nPY\n", "x".repeat(500)); + let ev = ExecApprovalRequestEvent { + call_id: "call-approve-cmd-multiline-trunc".into(), + turn_id: "turn-approve-cmd-multiline-trunc".into(), + command: vec!["bash".into(), "-lc".into(), "echo hello world".into()], + cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), + reason: None, + proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ + "bash".into(), + "-lc".into(), + long, + ])), + parsed_cmd: vec![], + }; + chat.handle_codex_event(Event { + id: "sub-approve-multiline-trunc".into(), + msg: EventMsg::ExecApprovalRequest(ev), + }); + + let width = 100; + let height = chat.desired_height(width); + let mut terminal = + ratatui::Terminal::new(VT100Backend::new(width, height)).expect("create terminal"); + terminal.set_viewport_area(Rect::new(0, 0, width, height)); + terminal + .draw(|f| chat.render(f.area(), f.buffer_mut())) + .expect("draw approval modal (multiline prefix truncation)"); + let contents = terminal.backend().vt100().screen().contents(); + assert!(contents.contains("(truncated)")); + assert_snapshot!("approval_modal_exec_multiline_prefix_truncates", contents); +} + // Snapshot test: patch approval modal #[tokio::test] async fn approval_modal_patch_snapshot() { diff --git a/codex-rs/tui2/src/exec_command.rs b/codex-rs/tui2/src/exec_command.rs index 8ce6c2632e4..ba7e9cf733b 100644 --- a/codex-rs/tui2/src/exec_command.rs +++ b/codex-rs/tui2/src/exec_command.rs @@ -5,6 +5,9 @@ use codex_core::parse_command::extract_shell_command; use dirs::home_dir; use shlex::try_join; +const APPROVAL_LABEL_MAX_LEN: usize = 80; +const APPROVAL_LABEL_TRUNCATED_SUFFIX: &str = "... (truncated)"; + pub(crate) fn escape_command(command: &[String]) -> String { try_join(command.iter().map(String::as_str)).unwrap_or_else(|_| command.join(" ")) } @@ -16,6 +19,21 @@ pub(crate) fn strip_bash_lc_and_escape(command: &[String]) -> String { escape_command(command) } +pub(crate) fn render_for_approval_prefix_label(command: &[String]) -> String { + let rendered = strip_bash_lc_and_escape(command); + + // Approval choices are rendered inline in a list; ensure we never introduce + // multi-line labels due to heredocs or embedded newlines. + let collapsed = rendered.split_whitespace().collect::>().join(" "); + let mut chars = collapsed.chars(); + let prefix: String = chars.by_ref().take(APPROVAL_LABEL_MAX_LEN).collect(); + if chars.next().is_some() { + format!("{}{APPROVAL_LABEL_TRUNCATED_SUFFIX}", prefix.trim_end()) + } else { + prefix + } +} + /// If `path` is absolute and inside $HOME, return the part *after* the home /// directory; otherwise, return the path as-is. Note if `path` is the homedir, /// this will return and empty path. @@ -67,4 +85,13 @@ mod tests { let cmdline = strip_bash_lc_and_escape(&args); assert_eq!(cmdline, "echo hello"); } + + #[test] + fn approval_prefix_label_is_single_line_and_truncated() { + let long = format!("python - <<'PY'\n{}\nPY\n", "x".repeat(500)); + let args = vec!["bash".into(), "-lc".into(), long]; + let label = render_for_approval_prefix_label(&args); + assert!(!label.contains('\n')); + assert!(label.ends_with(APPROVAL_LABEL_TRUNCATED_SUFFIX)); + } } From cc952253c1bf9f959b544b823d07ae55de607710 Mon Sep 17 00:00:00 2001 From: Thibault Sottiaux Date: Mon, 5 Jan 2026 15:41:35 -0800 Subject: [PATCH 3/6] Update client.rs --- codex-rs/core/tests/suite/client.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index ccafba535ee..c8cc3d2b982 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -385,10 +385,7 @@ async fn includes_conversation_id_and_model_headers_in_request() { request_conversation_id.to_str().unwrap(), conversation_id.to_string() ); - let expected_originator = - std::env::var(codex_core::default_client::CODEX_INTERNAL_ORIGINATOR_OVERRIDE_ENV_VAR) - .unwrap_or_else(|_| codex_core::default_client::DEFAULT_ORIGINATOR.to_string()); - assert_eq!(request_originator.to_str().unwrap(), expected_originator); + assert_eq!(request_originator.to_str().unwrap(), "codex_cli_rs"); assert_eq!( request_authorization.to_str().unwrap(), "Bearer Test API Key" @@ -513,10 +510,7 @@ async fn chatgpt_auth_sends_correct_request() { request_conversation_id.to_str().unwrap(), conversation_id.to_string() ); - let expected_originator = - std::env::var(codex_core::default_client::CODEX_INTERNAL_ORIGINATOR_OVERRIDE_ENV_VAR) - .unwrap_or_else(|_| codex_core::default_client::DEFAULT_ORIGINATOR.to_string()); - assert_eq!(request_originator.to_str().unwrap(), expected_originator); + assert_eq!(request_originator.to_str().unwrap(), "codex_cli_rs"); assert_eq!( request_authorization.to_str().unwrap(), "Bearer Access Token" From d61bb3feee9b8b5a7e96fca745af42a3cc8a5cb7 Mon Sep 17 00:00:00 2001 From: Thibault Sottiaux Date: Mon, 5 Jan 2026 15:42:34 -0800 Subject: [PATCH 4/6] Update client.rs --- codex-rs/core/tests/suite/client.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index c8cc3d2b982..a22027f99fb 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -43,7 +43,6 @@ use core_test_support::test_codex::test_codex; use core_test_support::wait_for_event; use dunce::canonicalize as normalize_path; use futures::StreamExt; -use pretty_assertions::assert_eq; use serde_json::json; use std::io::Write; use std::sync::Arc; From 3ab4d977dd76d814cb1e43207a9239c3aac67a24 Mon Sep 17 00:00:00 2001 From: Thibault Sottiaux Date: Tue, 6 Jan 2026 05:16:57 -0800 Subject: [PATCH 5/6] tui: hide multiline execpolicy prefix option --- .../tui/src/bottom_pane/approval_overlay.rs | 13 +++++---- ...l_exec_multiline_prefix_no_execpolicy.snap | 12 +++++++++ ...modal_exec_multiline_prefix_truncates.snap | 14 ---------- codex-rs/tui/src/chatwidget/tests.rs | 16 ++++++----- codex-rs/tui/src/exec_command.rs | 27 ------------------- .../tui2/src/bottom_pane/approval_overlay.rs | 13 +++++---- ...l_exec_multiline_prefix_no_execpolicy.snap | 12 +++++++++ ...modal_exec_multiline_prefix_truncates.snap | 14 ---------- codex-rs/tui2/src/chatwidget/tests.rs | 15 ++++++----- codex-rs/tui2/src/exec_command.rs | 27 ------------------- 10 files changed, 59 insertions(+), 104 deletions(-) create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_multiline_prefix_no_execpolicy.snap delete mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_multiline_prefix_truncates.snap create mode 100644 codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__approval_modal_exec_multiline_prefix_no_execpolicy.snap delete mode 100644 codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__approval_modal_exec_multiline_prefix_truncates.snap diff --git a/codex-rs/tui/src/bottom_pane/approval_overlay.rs b/codex-rs/tui/src/bottom_pane/approval_overlay.rs index bb8bb355641..0923d41b5fb 100644 --- a/codex-rs/tui/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui/src/bottom_pane/approval_overlay.rs @@ -9,7 +9,6 @@ use crate::bottom_pane::list_selection_view::ListSelectionView; use crate::bottom_pane::list_selection_view::SelectionItem; use crate::bottom_pane::list_selection_view::SelectionViewParams; use crate::diff_render::DiffSummary; -use crate::exec_command::render_for_approval_prefix_label; use crate::exec_command::strip_bash_lc_and_escape; use crate::history_cell; use crate::key_hint; @@ -462,9 +461,13 @@ fn exec_options( .chain( proposed_execpolicy_amendment .filter(|_| features.enabled(Feature::ExecPolicy)) - .map(|prefix| { - let rendered_prefix = render_for_approval_prefix_label(prefix.command()); - ApprovalOption { + .and_then(|prefix| { + let rendered_prefix = strip_bash_lc_and_escape(prefix.command()); + if rendered_prefix.contains('\n') || rendered_prefix.contains('\r') { + return None; + } + + Some(ApprovalOption { label: format!( "Yes, and don't ask again for commands that start with `{rendered_prefix}`" ), @@ -475,7 +478,7 @@ fn exec_options( ), display_shortcut: None, additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))], - } + }) }), ) .chain([ApprovalOption { diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_multiline_prefix_no_execpolicy.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_multiline_prefix_no_execpolicy.snap new file mode 100644 index 00000000000..2604519ec8d --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_multiline_prefix_no_execpolicy.snap @@ -0,0 +1,12 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: contents +--- + Would you like to run the following command? + + $ echo hello world + +› 1. Yes, proceed (y) + 2. No, and tell Codex what to do differently (esc) + + Press enter to confirm or esc to cancel diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_multiline_prefix_truncates.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_multiline_prefix_truncates.snap deleted file mode 100644 index 7a8840719c8..00000000000 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_multiline_prefix_truncates.snap +++ /dev/null @@ -1,14 +0,0 @@ ---- -source: tui/src/chatwidget/tests.rs -expression: contents ---- - Would you like to run the following command? - - $ echo hello world - -› 1. Yes, proceed (y) - 2. Yes, and don't ask again for commands that start with `python - <<'PY' - xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx... (truncated)` (p) - 3. No, and tell Codex what to do differently (esc) - - Press enter to confirm or esc to cancel diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 1015094369b..1398b6a26ec 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -2465,10 +2465,11 @@ async fn approval_modal_exec_without_reason_snapshot() -> anyhow::Result<()> { Ok(()) } -// Snapshot test: approval modal with a proposed execpolicy prefix that is long enough -// to trigger truncation in the selection label. +// Snapshot test: approval modal with a proposed execpolicy prefix that is multi-line; +// we should not offer adding it to execpolicy. #[tokio::test] -async fn approval_modal_exec_multiline_prefix_truncates_snapshot() -> anyhow::Result<()> { +async fn approval_modal_exec_multiline_prefix_hides_execpolicy_option_snapshot() +-> anyhow::Result<()> { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; chat.config.approval_policy.set(AskForApproval::OnRequest)?; @@ -2498,10 +2499,13 @@ async fn approval_modal_exec_multiline_prefix_truncates_snapshot() -> anyhow::Re terminal.set_viewport_area(Rect::new(0, 0, width, height)); terminal .draw(|f| chat.render(f.area(), f.buffer_mut())) - .expect("draw approval modal (multiline prefix truncation)"); + .expect("draw approval modal (multiline prefix)"); let contents = terminal.backend().vt100().screen().contents(); - assert!(contents.contains("(truncated)")); - assert_snapshot!("approval_modal_exec_multiline_prefix_truncates", contents); + assert!(!contents.contains("don't ask again")); + assert_snapshot!( + "approval_modal_exec_multiline_prefix_no_execpolicy", + contents + ); Ok(()) } diff --git a/codex-rs/tui/src/exec_command.rs b/codex-rs/tui/src/exec_command.rs index ba7e9cf733b..8ce6c2632e4 100644 --- a/codex-rs/tui/src/exec_command.rs +++ b/codex-rs/tui/src/exec_command.rs @@ -5,9 +5,6 @@ use codex_core::parse_command::extract_shell_command; use dirs::home_dir; use shlex::try_join; -const APPROVAL_LABEL_MAX_LEN: usize = 80; -const APPROVAL_LABEL_TRUNCATED_SUFFIX: &str = "... (truncated)"; - pub(crate) fn escape_command(command: &[String]) -> String { try_join(command.iter().map(String::as_str)).unwrap_or_else(|_| command.join(" ")) } @@ -19,21 +16,6 @@ pub(crate) fn strip_bash_lc_and_escape(command: &[String]) -> String { escape_command(command) } -pub(crate) fn render_for_approval_prefix_label(command: &[String]) -> String { - let rendered = strip_bash_lc_and_escape(command); - - // Approval choices are rendered inline in a list; ensure we never introduce - // multi-line labels due to heredocs or embedded newlines. - let collapsed = rendered.split_whitespace().collect::>().join(" "); - let mut chars = collapsed.chars(); - let prefix: String = chars.by_ref().take(APPROVAL_LABEL_MAX_LEN).collect(); - if chars.next().is_some() { - format!("{}{APPROVAL_LABEL_TRUNCATED_SUFFIX}", prefix.trim_end()) - } else { - prefix - } -} - /// If `path` is absolute and inside $HOME, return the part *after* the home /// directory; otherwise, return the path as-is. Note if `path` is the homedir, /// this will return and empty path. @@ -85,13 +67,4 @@ mod tests { let cmdline = strip_bash_lc_and_escape(&args); assert_eq!(cmdline, "echo hello"); } - - #[test] - fn approval_prefix_label_is_single_line_and_truncated() { - let long = format!("python - <<'PY'\n{}\nPY\n", "x".repeat(500)); - let args = vec!["bash".into(), "-lc".into(), long]; - let label = render_for_approval_prefix_label(&args); - assert!(!label.contains('\n')); - assert!(label.ends_with(APPROVAL_LABEL_TRUNCATED_SUFFIX)); - } } diff --git a/codex-rs/tui2/src/bottom_pane/approval_overlay.rs b/codex-rs/tui2/src/bottom_pane/approval_overlay.rs index bb8bb355641..0923d41b5fb 100644 --- a/codex-rs/tui2/src/bottom_pane/approval_overlay.rs +++ b/codex-rs/tui2/src/bottom_pane/approval_overlay.rs @@ -9,7 +9,6 @@ use crate::bottom_pane::list_selection_view::ListSelectionView; use crate::bottom_pane::list_selection_view::SelectionItem; use crate::bottom_pane::list_selection_view::SelectionViewParams; use crate::diff_render::DiffSummary; -use crate::exec_command::render_for_approval_prefix_label; use crate::exec_command::strip_bash_lc_and_escape; use crate::history_cell; use crate::key_hint; @@ -462,9 +461,13 @@ fn exec_options( .chain( proposed_execpolicy_amendment .filter(|_| features.enabled(Feature::ExecPolicy)) - .map(|prefix| { - let rendered_prefix = render_for_approval_prefix_label(prefix.command()); - ApprovalOption { + .and_then(|prefix| { + let rendered_prefix = strip_bash_lc_and_escape(prefix.command()); + if rendered_prefix.contains('\n') || rendered_prefix.contains('\r') { + return None; + } + + Some(ApprovalOption { label: format!( "Yes, and don't ask again for commands that start with `{rendered_prefix}`" ), @@ -475,7 +478,7 @@ fn exec_options( ), display_shortcut: None, additional_shortcuts: vec![key_hint::plain(KeyCode::Char('p'))], - } + }) }), ) .chain([ApprovalOption { diff --git a/codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__approval_modal_exec_multiline_prefix_no_execpolicy.snap b/codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__approval_modal_exec_multiline_prefix_no_execpolicy.snap new file mode 100644 index 00000000000..5df124ccf5c --- /dev/null +++ b/codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__approval_modal_exec_multiline_prefix_no_execpolicy.snap @@ -0,0 +1,12 @@ +--- +source: tui2/src/chatwidget/tests.rs +expression: contents +--- + Would you like to run the following command? + + $ echo hello world + +› 1. Yes, proceed (y) + 2. No, and tell Codex what to do differently (esc) + + Press enter to confirm or esc to cancel diff --git a/codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__approval_modal_exec_multiline_prefix_truncates.snap b/codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__approval_modal_exec_multiline_prefix_truncates.snap deleted file mode 100644 index 01a3fe71c0d..00000000000 --- a/codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__approval_modal_exec_multiline_prefix_truncates.snap +++ /dev/null @@ -1,14 +0,0 @@ ---- -source: tui2/src/chatwidget/tests.rs -expression: contents ---- - Would you like to run the following command? - - $ echo hello world - -› 1. Yes, proceed (y) - 2. Yes, and don't ask again for commands that start with `python - <<'PY' - xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx... (truncated)` (p) - 3. No, and tell Codex what to do differently (esc) - - Press enter to confirm or esc to cancel diff --git a/codex-rs/tui2/src/chatwidget/tests.rs b/codex-rs/tui2/src/chatwidget/tests.rs index fa7d134fdb9..e6ae14509a0 100644 --- a/codex-rs/tui2/src/chatwidget/tests.rs +++ b/codex-rs/tui2/src/chatwidget/tests.rs @@ -2123,10 +2123,10 @@ async fn approval_modal_exec_without_reason_snapshot() { ); } -// Snapshot test: approval modal with a proposed execpolicy prefix that is long enough -// to trigger truncation in the selection label. +// Snapshot test: approval modal with a proposed execpolicy prefix that is multi-line; +// we should not offer adding it to execpolicy. #[tokio::test] -async fn approval_modal_exec_multiline_prefix_truncates_snapshot() { +async fn approval_modal_exec_multiline_prefix_hides_execpolicy_option_snapshot() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; chat.config.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); @@ -2156,10 +2156,13 @@ async fn approval_modal_exec_multiline_prefix_truncates_snapshot() { terminal.set_viewport_area(Rect::new(0, 0, width, height)); terminal .draw(|f| chat.render(f.area(), f.buffer_mut())) - .expect("draw approval modal (multiline prefix truncation)"); + .expect("draw approval modal (multiline prefix)"); let contents = terminal.backend().vt100().screen().contents(); - assert!(contents.contains("(truncated)")); - assert_snapshot!("approval_modal_exec_multiline_prefix_truncates", contents); + assert!(!contents.contains("don't ask again")); + assert_snapshot!( + "approval_modal_exec_multiline_prefix_no_execpolicy", + contents + ); } // Snapshot test: patch approval modal diff --git a/codex-rs/tui2/src/exec_command.rs b/codex-rs/tui2/src/exec_command.rs index ba7e9cf733b..8ce6c2632e4 100644 --- a/codex-rs/tui2/src/exec_command.rs +++ b/codex-rs/tui2/src/exec_command.rs @@ -5,9 +5,6 @@ use codex_core::parse_command::extract_shell_command; use dirs::home_dir; use shlex::try_join; -const APPROVAL_LABEL_MAX_LEN: usize = 80; -const APPROVAL_LABEL_TRUNCATED_SUFFIX: &str = "... (truncated)"; - pub(crate) fn escape_command(command: &[String]) -> String { try_join(command.iter().map(String::as_str)).unwrap_or_else(|_| command.join(" ")) } @@ -19,21 +16,6 @@ pub(crate) fn strip_bash_lc_and_escape(command: &[String]) -> String { escape_command(command) } -pub(crate) fn render_for_approval_prefix_label(command: &[String]) -> String { - let rendered = strip_bash_lc_and_escape(command); - - // Approval choices are rendered inline in a list; ensure we never introduce - // multi-line labels due to heredocs or embedded newlines. - let collapsed = rendered.split_whitespace().collect::>().join(" "); - let mut chars = collapsed.chars(); - let prefix: String = chars.by_ref().take(APPROVAL_LABEL_MAX_LEN).collect(); - if chars.next().is_some() { - format!("{}{APPROVAL_LABEL_TRUNCATED_SUFFIX}", prefix.trim_end()) - } else { - prefix - } -} - /// If `path` is absolute and inside $HOME, return the part *after* the home /// directory; otherwise, return the path as-is. Note if `path` is the homedir, /// this will return and empty path. @@ -85,13 +67,4 @@ mod tests { let cmdline = strip_bash_lc_and_escape(&args); assert_eq!(cmdline, "echo hello"); } - - #[test] - fn approval_prefix_label_is_single_line_and_truncated() { - let long = format!("python - <<'PY'\n{}\nPY\n", "x".repeat(500)); - let args = vec!["bash".into(), "-lc".into(), long]; - let label = render_for_approval_prefix_label(&args); - assert!(!label.contains('\n')); - assert!(label.ends_with(APPROVAL_LABEL_TRUNCATED_SUFFIX)); - } } From 62b810bf35704ab18066d7dfbd8eafa2e802f9d5 Mon Sep 17 00:00:00 2001 From: Thibault Sottiaux Date: Tue, 6 Jan 2026 05:29:33 -0800 Subject: [PATCH 6/6] tui: align multiline execpolicy snapshot --- ...val_modal_exec_multiline_prefix_no_execpolicy.snap | 6 +++++- codex-rs/tui/src/chatwidget/tests.rs | 11 ++++------- ...val_modal_exec_multiline_prefix_no_execpolicy.snap | 6 +++++- codex-rs/tui2/src/chatwidget/tests.rs | 11 ++++------- 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_multiline_prefix_no_execpolicy.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_multiline_prefix_no_execpolicy.snap index 2604519ec8d..3c256fe9231 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_multiline_prefix_no_execpolicy.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_multiline_prefix_no_execpolicy.snap @@ -2,9 +2,13 @@ source: tui/src/chatwidget/tests.rs expression: contents --- + + Would you like to run the following command? - $ echo hello world + $ python - <<'PY' + print('hello') + PY › 1. Yes, proceed (y) 2. No, and tell Codex what to do differently (esc) diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 1398b6a26ec..d3aee88432b 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -2473,18 +2473,15 @@ async fn approval_modal_exec_multiline_prefix_hides_execpolicy_option_snapshot() let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; chat.config.approval_policy.set(AskForApproval::OnRequest)?; - let long = format!("python - <<'PY'\n{}\nPY\n", "x".repeat(500)); + let script = "python - <<'PY'\nprint('hello')\nPY".to_string(); + let command = vec!["bash".into(), "-lc".into(), script]; let ev = ExecApprovalRequestEvent { call_id: "call-approve-cmd-multiline-trunc".into(), turn_id: "turn-approve-cmd-multiline-trunc".into(), - command: vec!["bash".into(), "-lc".into(), "echo hello world".into()], + command: command.clone(), cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), reason: None, - proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ - "bash".into(), - "-lc".into(), - long, - ])), + proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)), parsed_cmd: vec![], }; chat.handle_codex_event(Event { diff --git a/codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__approval_modal_exec_multiline_prefix_no_execpolicy.snap b/codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__approval_modal_exec_multiline_prefix_no_execpolicy.snap index 5df124ccf5c..962a38ebde6 100644 --- a/codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__approval_modal_exec_multiline_prefix_no_execpolicy.snap +++ b/codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__approval_modal_exec_multiline_prefix_no_execpolicy.snap @@ -2,9 +2,13 @@ source: tui2/src/chatwidget/tests.rs expression: contents --- + + Would you like to run the following command? - $ echo hello world + $ python - <<'PY' + print('hello') + PY › 1. Yes, proceed (y) 2. No, and tell Codex what to do differently (esc) diff --git a/codex-rs/tui2/src/chatwidget/tests.rs b/codex-rs/tui2/src/chatwidget/tests.rs index e6ae14509a0..df41f0dc696 100644 --- a/codex-rs/tui2/src/chatwidget/tests.rs +++ b/codex-rs/tui2/src/chatwidget/tests.rs @@ -2130,18 +2130,15 @@ async fn approval_modal_exec_multiline_prefix_hides_execpolicy_option_snapshot() let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; chat.config.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); - let long = format!("python - <<'PY'\n{}\nPY\n", "x".repeat(500)); + let script = "python - <<'PY'\nprint('hello')\nPY".to_string(); + let command = vec!["bash".into(), "-lc".into(), script]; let ev = ExecApprovalRequestEvent { call_id: "call-approve-cmd-multiline-trunc".into(), turn_id: "turn-approve-cmd-multiline-trunc".into(), - command: vec!["bash".into(), "-lc".into(), "echo hello world".into()], + command: command.clone(), cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")), reason: None, - proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(vec![ - "bash".into(), - "-lc".into(), - long, - ])), + proposed_execpolicy_amendment: Some(ExecPolicyAmendment::new(command)), parsed_cmd: vec![], }; chat.handle_codex_event(Event {