valknar/llmx
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

tui: fix approval dialog for large commands (#3087)

Browse Source 
#### Summary
- Emit a “Proposed Command” history cell when an ExecApprovalRequest
arrives (parity with proposed patches).
- Simplify the approval dialog: show only the reason/instructions; move
the command preview into history.
- Make approval/abort decision history concise:
  - Single line snippet; if multiline, show first line + " ...".
  - Truncate to 80 graphemes with ellipsis for very long commands.

#### Details
- History
- Add `new_proposed_command` to render a header and indented command
preview.
  - Use shared `prefix_lines` helper for first/subsequent line prefixes.
- Approval UI
- `UserApprovalWidget` no longer renders the command in the modal; shows
optional `reason` text only.
  - Decision history renders an inline, dimmed snippet per rules above.
- Tests (snapshot-based)
  - Proposed/decision flow for short command.
  - Proposed multi-line + aborted decision snippet with “ ...”.
  - Very long one-line command -> truncated snippet with “…”.
  - Updated existing exec approval snapshots and test reasons.

<img width="1053" height="704" alt="Screenshot 2025-09-03 at 11 57
35 AM"
src="https://github.com/user-attachments/assets/9ed4c316-9daf-4ac1-80ff-7ae1f481dd10"
/>

after approving:

<img width="1053" height="704" alt="Screenshot 2025-09-03 at 11 58
18 AM"
src="https://github.com/user-attachments/assets/a44e243f-eb9d-42ea-87f4-171b3fb481e7"
/>

rejection:

<img width="1053" height="207" alt="Screenshot 2025-09-03 at 11 58
45 AM"
src="https://github.com/user-attachments/assets/a022664b-ae0e-4b70-a388-509208707934"
/>

big command:


https://github.com/user-attachments/assets/2dd976e5-799f-4af7-9682-a046e66cc494
This commit is contained in:
Jeremy Rose
2025-09-04 16:54:53 -07:00
committed by GitHub
parent 907d3dd348
commit 742feaf40f
13 changed files with 230 additions and 116 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -524,6 +524,8 @@ impl ChatWidget {
pub(crate) fn handle_exec_approval_now(&mut self, id: String, ev: ExecApprovalRequestEvent) {
self.flush_answer_stream_with_separator();
// Emit the proposed command into history (like proposed patches)
self.add_to_history(history_cell::new_proposed_command(&ev.command));
let request = ApprovalRequest::Exec {
id,

4
codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec.snap
View File

@@ -4,9 +4,7 @@ assertion_line: 728
expression: terminal.backend()
---
" "
"? Codex wants to run echo hello world "
" "
"Model wants to run a command "
"this is a test reason such as one that would be produced by the model "
" "
"▌Allow command? "
"▌ Yes Always No, provide feedback "

2
codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__approval_modal_exec_no_reason.snap
View File

@@ -3,8 +3,6 @@ source: tui/src/chatwidget/tests.rs
expression: terminal.backend()
---
" "
"? Codex wants to run echo hello world "
" "
"▌Allow command? "
"▌ Yes Always No, provide feedback "
"▌ Approve and run the command "

5
codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exec_approval_history_decision_aborted_long.snap Normal file
View File

@@ -0,0 +1,5 @@
---
source: tui/src/chatwidget/tests.rs
expression: lines_to_single_string(&aborted_long)
---
✗ You canceled the request to run echo aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...

6
codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exec_approval_history_decision_aborted_multiline.snap Normal file
View File

@@ -0,0 +1,6 @@
---
source: tui/src/chatwidget/tests.rs
expression: lines_to_single_string(&aborted_multi)
---
✗ You canceled the request to run echo line1 ...

6
codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exec_approval_history_decision_approved_short.snap Normal file
View File

@@ -0,0 +1,6 @@
---
source: tui/src/chatwidget/tests.rs
expression: lines_to_single_string(&decision)
---
✔ You approved codex to run echo hello world this time

7
codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exec_approval_history_proposed_multiline.snap Normal file
View File

@@ -0,0 +1,7 @@
---
source: tui/src/chatwidget/tests.rs
expression: lines_to_single_string(&proposed_multi)
---
• Proposed Command
└ echo line1
echo line2

6
codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exec_approval_history_proposed_short.snap Normal file
View File

@@ -0,0 +1,6 @@
---
source: tui/src/chatwidget/tests.rs
expression: lines_to_single_string(&proposed)
---
• Proposed Command
└ echo hello world

4
codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_and_approval_modal.snap
View File

@@ -4,9 +4,7 @@ assertion_line: 921
expression: terminal.backend()
---
" "
"? Codex wants to run echo 'hello world' "
" "
"Codex wants to run a command "
"this is a test reason such as one that would be produced by the model "
" "
"▌Allow command? "
"▌ Yes Always No, provide feedback "

106
codex-rs/tui/src/chatwidget/tests.rs
View File

@@ -265,6 +265,104 @@ fn lines_to_single_string(lines: &[ratatui::text::Line<'static>]) -> String {
s
}
// (removed experimental resize snapshot test)
#[test]
fn exec_approval_emits_proposed_command_and_decision_history() {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual();
// Trigger an exec approval request with a short, single-line command
let ev = ExecApprovalRequestEvent {
call_id: "call-short".into(),
command: vec!["bash".into(), "-lc".into(), "echo hello world".into()],
cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
reason: Some(
"this is a test reason such as one that would be produced by the model".into(),
),
};
chat.handle_codex_event(Event {
id: "sub-short".into(),
msg: EventMsg::ExecApprovalRequest(ev),
});
// Snapshot the Proposed Command cell emitted into history
let proposed = drain_insert_history(&mut rx)
.pop()
.expect("expected proposed command cell");
assert_snapshot!(
"exec_approval_history_proposed_short",
lines_to_single_string(&proposed)
);
// Approve via keyboard and verify a concise decision history line is added
chat.handle_key_event(KeyEvent::new(KeyCode::Char('y'), KeyModifiers::NONE));
let decision = drain_insert_history(&mut rx)
.pop()
.expect("expected decision cell in history");
assert_snapshot!(
"exec_approval_history_decision_approved_short",
lines_to_single_string(&decision)
);
}
#[test]
fn exec_approval_decision_truncates_multiline_and_long_commands() {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual();
// Multiline command: should render proposed command fully in history with prefixes
let ev_multi = ExecApprovalRequestEvent {
call_id: "call-multi".into(),
command: vec!["bash".into(), "-lc".into(), "echo line1\necho line2".into()],
cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
reason: Some(
"this is a test reason such as one that would be produced by the model".into(),
),
};
chat.handle_codex_event(Event {
id: "sub-multi".into(),
msg: EventMsg::ExecApprovalRequest(ev_multi),
});
let proposed_multi = drain_insert_history(&mut rx)
.pop()
.expect("expected proposed multiline command cell");
assert_snapshot!(
"exec_approval_history_proposed_multiline",
lines_to_single_string(&proposed_multi)
);
// Deny via keyboard; decision snippet should be single-line and elided with " ..."
chat.handle_key_event(KeyEvent::new(KeyCode::Char('n'), KeyModifiers::NONE));
let aborted_multi = drain_insert_history(&mut rx)
.pop()
.expect("expected aborted decision cell (multiline)");
assert_snapshot!(
"exec_approval_history_decision_aborted_multiline",
lines_to_single_string(&aborted_multi)
);
// Very long single-line command: decision snippet should be truncated <= 80 chars with trailing ...
let long = format!("echo {}", "a".repeat(200));
let ev_long = ExecApprovalRequestEvent {
call_id: "call-long".into(),
command: vec!["bash".into(), "-lc".into(), long.clone()],
cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
reason: None,
};
chat.handle_codex_event(Event {
id: "sub-long".into(),
msg: EventMsg::ExecApprovalRequest(ev_long),
});
drain_insert_history(&mut rx); // proposed cell not needed for this assertion
chat.handle_key_event(KeyEvent::new(KeyCode::Char('n'), KeyModifiers::NONE));
let aborted_long = drain_insert_history(&mut rx)
.pop()
.expect("expected aborted decision cell (long)");
assert_snapshot!(
"exec_approval_history_decision_aborted_long",
lines_to_single_string(&aborted_long)
);
}
// --- Small helpers to tersely drive exec begin/end and snapshot active cell ---
fn begin_exec(chat: &mut ChatWidget, call_id: &str, raw_cmd: &str) {
// Build the full command vec and parse it using core's parser,
@@ -714,7 +812,9 @@ fn approval_modal_exec_snapshot() {
call_id: "call-approve-cmd".into(),
command: vec!["bash".into(), "-lc".into(), "echo hello world".into()],
cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
reason: Some("Model wants to run a command".into()),
reason: Some(
"this is a test reason such as one that would be produced by the model".into(),
),
};
chat.handle_codex_event(Event {
id: "sub-approve".into(),
@@ -907,7 +1007,9 @@ fn status_widget_and_approval_modal_snapshot() {
call_id: "call-approve-exec".into(),
command: vec!["echo".into(), "hello world".into()],
cwd: std::path::PathBuf::from("/tmp"),
reason: Some("Codex wants to run a command".into()),
reason: Some(
"this is a test reason such as one that would be produced by the model".into(),
),
};
chat.handle_codex_event(Event {
id: "sub-approve-exec".into(),

49
codex-rs/tui/src/history_cell.rs
View File

@@ -2,6 +2,7 @@ use crate::diff_render::create_diff_summary;
use crate::exec_command::relativize_to_home;
use crate::exec_command::strip_bash_lc_and_escape;
use crate::markdown::append_markdown;
use crate::render::line_utils::prefix_lines;
use crate::slash_command::SlashCommand;
use crate::text_formatting::format_and_truncate_tool_result;
use base64::Engine;
@@ -1140,33 +1141,9 @@ impl HistoryCell for PlanUpdateCell {
.into_iter()
.map(|s| s.to_string().set_style(step_style).into())
.collect();
prefix_lines(step_text, &box_str.into(), &" ".into())
prefix_lines(step_text, box_str.into(), " ".into())
};
fn prefix_lines(
lines: Vec<Line<'static>>,
initial_prefix: &Span<'static>,
subsequent_prefix: &Span<'static>,
) -> Vec<Line<'static>> {
lines
.into_iter()
.enumerate()
.map(|(i, l)| {
Line::from(
[
vec![if i == 0 {
initial_prefix.clone()
} else {
subsequent_prefix.clone()
}],
l.spans,
]
.concat(),
)
})
.collect()
}
let mut lines: Vec<Line<'static>> = vec![];
lines.push(vec!["• ".into(), "Updated Plan".bold()].into());
@@ -1187,7 +1164,7 @@ impl HistoryCell for PlanUpdateCell {
indented_lines.extend(render_step(status, step));
}
}
lines.extend(prefix_lines(indented_lines, &" └ ".into(), &" ".into()));
lines.extend(prefix_lines(indented_lines, " └ ".into(), " ".into()));
lines
}
@@ -1231,6 +1208,26 @@ pub(crate) fn new_patch_apply_failure(stderr: String) -> PlainHistoryCell {
PlainHistoryCell { lines }
}
/// Create a new history cell for a proposed command approval.
/// Renders a header and the command preview similar to how proposed patches
/// show a header and summary.
pub(crate) fn new_proposed_command(command: &[String]) -> PlainHistoryCell {
let cmd = strip_bash_lc_and_escape(command);
let mut lines: Vec<Line<'static>> = Vec::new();
lines.push(Line::from(vec!["• ".into(), "Proposed Command".bold()]));
let cmd_lines: Vec<Line<'static>> = cmd
.lines()
.map(|part| Line::from(part.to_string()))
.collect();
let initial_prefix: Span<'static> = " └ ".dim();
let subsequent_prefix: Span<'static> = " ".into();
lines.extend(prefix_lines(cmd_lines, initial_prefix, subsequent_prefix));
PlainHistoryCell { lines }
}
pub(crate) fn new_reasoning_block(
full_reasoning_buffer: String,
config: &Config,

23
codex-rs/tui/src/render/line_utils.rs
View File

@@ -34,3 +34,26 @@ pub fn is_blank_line_spaces_only(line: &Line<'_>) -> bool {
.iter()
.all(|s| s.content.is_empty() || s.content.chars().all(|c| c == ' '))
}
/// Prefix each line with `initial_prefix` for the first line and
/// `subsequent_prefix` for following lines. Returns a new Vec of owned lines.
pub fn prefix_lines(
lines: Vec<Line<'static>>,
initial_prefix: Span<'static>,
subsequent_prefix: Span<'static>,
) -> Vec<Line<'static>> {
lines
.into_iter()
.enumerate()
.map(|(i, l)| {
let mut spans = Vec::with_capacity(l.spans.len() + 1);
spans.push(if i == 0 {
initial_prefix.clone()
} else {
subsequent_prefix.clone()
});
spans.extend(l.spans);
Line::from(spans)
})
.collect()
}

126
codex-rs/tui/src/user_approval_widget.rs
View File

@@ -30,6 +30,7 @@ use crate::app_event::AppEvent;
use crate::app_event_sender::AppEventSender;
use crate::exec_command::strip_bash_lc_and_escape;
use crate::history_cell;
use crate::text_formatting::truncate_text;
/// Request coming from the agent that needs user approval.
pub(crate) enum ApprovalRequest {
@@ -110,45 +111,11 @@ pub(crate) struct UserApprovalWidget {
done: bool,
}
fn to_command_display<'a>(
first_line: Vec<Span<'a>>,
cmd: String,
last_line: Vec<Span<'a>>,
) -> Vec<Line<'a>> {
let command_lines: Vec<Span> = cmd.lines().map(|line| line.to_string().dim()).collect();
let mut lines: Vec<Line<'a>> = vec![];
let mut first_line = first_line.clone();
if command_lines.len() == 1 {
first_line.push(command_lines[0].clone());
first_line.extend(last_line);
} else {
for line in command_lines {
lines.push(vec![" ".into(), line].into());
}
let last_line = last_line.clone();
lines.push(Line::from(last_line));
}
lines.insert(0, Line::from(first_line));
lines
}
impl UserApprovalWidget {
pub(crate) fn new(approval_request: ApprovalRequest, app_event_tx: AppEventSender) -> Self {
let confirmation_prompt = match &approval_request {
ApprovalRequest::Exec {
command, reason, ..
} => {
let cmd = strip_bash_lc_and_escape(command);
let mut contents: Vec<Line> = to_command_display(
vec!["? ".fg(Color::Cyan), "Codex wants to run ".bold()],
cmd,
vec![],
);
contents.push(Line::from(""));
ApprovalRequest::Exec { reason, .. } => {
let mut contents: Vec<Line> = vec![];
if let Some(reason) = reason {
contents.push(Line::from(reason.clone().italic()));
contents.push(Line::from(""));
@@ -258,62 +225,61 @@ impl UserApprovalWidget {
fn send_decision_with_feedback(&mut self, decision: ReviewDecision, feedback: String) {
match &self.approval_request {
ApprovalRequest::Exec { command, .. } => {
let cmd = strip_bash_lc_and_escape(command);
// TODO: move this rendering into history_cell.
let mut lines: Vec<Line<'static>> = vec![];
let full_cmd = strip_bash_lc_and_escape(command);
// Construct a concise, single-line summary of the command:
// - If multi-line, take the first line and append " ...".
// - Truncate to 80 graphemes.
let mut snippet = match full_cmd.split_once('\n') {
Some((first, _)) => format!("{first} ..."),
None => full_cmd.clone(),
};
// Enforce the 80 character length limit.
snippet = truncate_text(&snippet, 80);
// Result line based on decision.
let mut result_spans: Vec<Span<'static>> = Vec::new();
match decision {
ReviewDecision::Approved => {
lines.extend(to_command_display(
vec![
"✔ ".fg(Color::Green),
"You ".into(),
"approved".bold(),
" codex to run ".into(),
],
cmd,
vec![" this time".bold()],
));
result_spans.extend(vec![
"✔ ".fg(Color::Green),
"You ".into(),
"approved".bold(),
" codex to run ".into(),
snippet.clone().dim(),
" this time".bold(),
]);
}
ReviewDecision::ApprovedForSession => {
lines.extend(to_command_display(
vec![
"✔ ".fg(Color::Green),
"You ".into(),
"approved".bold(),
" codex to run ".into(),
],
cmd,
vec![" every time this session".bold()],
));
result_spans.extend(vec![
"✔ ".fg(Color::Green),
"You ".into(),
"approved".bold(),
" codex to run ".into(),
snippet.clone().dim(),
" every time this session".bold(),
]);
}
ReviewDecision::Denied => {
lines.extend(to_command_display(
vec![
"✗ ".fg(Color::Red),
"You ".into(),
"did not approve".bold(),
" codex to run ".into(),
],
cmd,
vec![],
));
result_spans.extend(vec![
"✗ ".fg(Color::Red),
"You ".into(),
"did not approve".bold(),
" codex to run ".into(),
snippet.clone().dim(),
]);
}
ReviewDecision::Abort => {
lines.extend(to_command_display(
vec![
"✗ ".fg(Color::Red),
"You ".into(),
"canceled".bold(),
" the request to run ".into(),
],
cmd,
vec![],
));
result_spans.extend(vec![
"✗ ".fg(Color::Red),
"You ".into(),
"canceled".bold(),
" the request to run ".into(),
snippet.clone().dim(),
]);
}
}
let mut lines: Vec<Line<'static>> = vec![Line::from(result_spans)];
if !feedback.trim().is_empty() {
lines.push(Line::from("feedback:"));
for l in feedback.lines() {