openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-09-05 15:18:41 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(tui): preserve shell display semantics

Browse Source
This commit is contained in:
Felipe Coury
2026-06-05 14:13:58 -03:00
parent a5f85d7905
commit eb1551879b
5 changed files with 94 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

50
codex-rs/shell-command/src/bash.rs
View File

@@ -120,7 +120,8 @@ fn is_recognised_bash_lc_invocation(shell: &str, flag: &str) -> bool {
/// Like `extract_bash_command`, but also recognises argv where the inner
/// script was flattened past `[shell, flag, script]` (e.g. the protocol
/// payload re-split an unquoted form into `[shell, flag, word, word, ...]`).
/// Returns an owned script, shlex-joined across the tail.
/// Returns an owned script reconstructed across the tail. Ordinary arguments
/// are shell-quoted, while standalone shell operator tokens remain operators.
///
/// Caveat: this assumes every token after the flag is script text, so it is
/// deliberately wrong for POSIX `sh -c <script> <arg0> <arg1>` where the tail
@@ -140,7 +141,32 @@ pub fn extract_bash_command_joined(command: &[String]) -> Option<(String, String
if rest.len() < 2 || !is_recognised_bash_lc_invocation(shell, flag) {
return None;
}
Some((shell.clone(), crate::parse_command::shlex_join(rest)))
Some((shell.clone(), join_flattened_shell_script(rest)))
}
fn join_flattened_shell_script(tokens: &[String]) -> String {
let mut script = String::new();
for token in tokens {
if !script.is_empty() {
script.push(' ');
}
if is_shell_operator_token(token) {
script.push_str(token);
} else {
let Ok(quoted) = shlex::try_quote(token) else {
return "<command included NUL byte>".to_string();
};
script.push_str(&quoted);
}
}
script
}
fn is_shell_operator_token(token: &str) -> bool {
token.chars().any(|ch| "<>|&;()".contains(ch))
&& token
.chars()
.all(|ch| ch.is_ascii_digit() || "<>|&;()".contains(ch))
}
/// Returns the sequence of plain commands within a `bash -lc "..."` or
@@ -380,6 +406,26 @@ mod tests {
assert_eq!(script, "touch /tmp/foo");
}
#[test]
fn extract_bash_command_joined_preserves_shell_operators() {
let (_, script) = extract_bash_command_joined(&[
"zsh".to_string(),
"-lc".to_string(),
"rg".to_string(),
"foo bar".to_string(),
"|".to_string(),
"head".to_string(),
">".to_string(),
"/tmp/out".to_string(),
"2>&1".to_string(),
";".to_string(),
"echo".to_string(),
"done".to_string(),
])
.expect("known shell + -lc + tail should match");
assert_eq!(script, "rg 'foo bar' | head > /tmp/out 2>&1 ; echo done");
}
#[test]
fn extract_bash_command_joined_accepts_absolute_shell_path() {
let (shell, script) = extract_bash_command_joined(&[

2
codex-rs/tui/src/chatwidget.rs
View File

@@ -298,7 +298,7 @@ use crate::exec_cell::CommandOutput;
use crate::exec_cell::ExecCell;
use crate::exec_cell::new_active_exec_command;
use crate::exec_command::split_command_string;
use crate::exec_command::strip_bash_lc_and_escape;
use crate::exec_command::strip_shell_wrapper_for_display;
use crate::get_git_diff::get_git_diff;
use crate::history_cell;
use crate::history_cell::HistoryCell;

2
codex-rs/tui/src/chatwidget/command_lifecycle.rs
View File

@@ -170,7 +170,7 @@ impl ChatWidget {
) {
let key = process_id.unwrap_or(call_id).to_string();
let command = split_command_string(command);
let command_display = strip_bash_lc_and_escape(&command);
let command_display = strip_shell_wrapper_for_display(&command);
if let Some(existing) = self
.unified_exec_processes
.iter_mut()

36
codex-rs/tui/src/exec_cell/render.rs
View File

@@ -3,7 +3,7 @@ use std::time::Instant;
use super::model::CommandOutput;
use super::model::ExecCall;
use super::model::ExecCell;
use crate::exec_command::strip_bash_lc_and_escape;
use crate::exec_command::strip_shell_wrapper_for_display;
use crate::history_cell::HistoryCell;
use crate::history_cell::plain_lines;
use crate::motion::MotionMode;
@@ -18,7 +18,6 @@ use crate::wrapping::adaptive_wrap_lines;
use codex_ansi_escape::ansi_escape_line;
use codex_app_server_protocol::CommandExecutionSource as ExecCommandSource;
use codex_protocol::parse_command::ParsedCommand;
use codex_shell_command::bash::extract_bash_command;
use codex_utils_elapsed::format_duration;
use itertools::Itertools;
use ratatui::prelude::*;
@@ -65,17 +64,7 @@ pub(crate) fn new_active_exec_command(
}
fn format_unified_exec_interaction(command: &[String], input: Option<&str>) -> String {
// Strip the shell wrapper (canonical 3-element shape, or flattened argv)
// so the "Waited for `…`" line shows the inner script, not `zsh -lc …`.
let command_display = if let Some((_, script)) = extract_bash_command(command) {
script.to_string()
} else if let Some((_, script)) =
codex_shell_command::bash::extract_bash_command_joined(command)
{
script
} else {
command.join(" ")
};
let command_display = strip_shell_wrapper_for_display(command);
match input {
Some(data) if !data.is_empty() => {
let preview = summarize_interaction_input(data);
@@ -213,7 +202,7 @@ impl HistoryCell for ExecCell {
if i > 0 {
lines.push("".into());
}
let script = strip_bash_lc_and_escape(&call.command);
let script = strip_shell_wrapper_for_display(&call.command);
let highlighted_script = highlight_bash_to_lines(&script);
let cmd_display = adaptive_wrap_lines(
&highlighted_script,
@@ -400,7 +389,7 @@ impl ExecCell {
let cmd_display = if call.is_unified_exec_interaction() {
format_unified_exec_interaction(&call.command, call.interaction_input.as_deref())
} else {
strip_bash_lc_and_escape(&call.command)
strip_shell_wrapper_for_display(&call.command)
};
let highlighted_lines = highlight_bash_to_lines(&cmd_display);
@@ -744,6 +733,23 @@ mod tests {
);
}
#[test]
fn flattened_shell_wrapper_preserves_operators_snapshot() {
let command = [
"/bin/zsh".to_string(),
"-lc".to_string(),
"rg".to_string(),
"foo".to_string(),
"|".to_string(),
"head".to_string(),
];
insta::assert_snapshot!(
format_unified_exec_interaction(&command, /*input*/ None),
@"Waited for `rg foo | head`"
);
}
#[test]
fn user_shell_output_is_limited_by_screen_lines() {
let long_url_like = format!(

27
codex-rs/tui/src/exec_command.rs
View File

@@ -13,8 +13,13 @@ pub(crate) fn strip_bash_lc_and_escape(command: &[String]) -> String {
if let Some((_, script)) = extract_shell_command(command) {
return script.to_string();
}
// Relaxed fallback for flattened argv, so every display-only wrapper-strip
// path agrees. Approval matching in command_canonicalization stays strict.
escape_command(command)
}
pub(crate) fn strip_shell_wrapper_for_display(command: &[String]) -> String {
if let Some((_, script)) = extract_shell_command(command) {
return script.to_string();
}
if let Some((_, script)) = codex_shell_command::bash::extract_bash_command_joined(command) {
return script;
}
@@ -88,14 +93,28 @@ mod tests {
let cmdline = strip_bash_lc_and_escape(&args);
assert_eq!(cmdline, "echo hello");
// Test a wrapper whose inner command was flattened across argv.
// Approval displays must preserve positional arguments after the
// script rather than treating them as more script text.
let args = vec![
"bash".into(),
"-c".into(),
"rm -rf \"$1\"".into(),
"sh".into(),
"/tmp/target".into(),
];
let cmdline = strip_bash_lc_and_escape(&args);
assert_eq!(cmdline, "bash -c 'rm -rf \"$1\"' sh /tmp/target");
}
#[test]
fn strip_shell_wrapper_for_display_handles_flattened_argv() {
let args = vec![
"/bin/zsh".into(),
"-lc".into(),
"python3".into(),
"build.py".into(),
];
let cmdline = strip_bash_lc_and_escape(&args);
let cmdline = strip_shell_wrapper_for_display(&args);
assert_eq!(cmdline, "python3 build.py");
}