Compare commits

...

3 Commits

Author SHA1 Message Date
Eason Goodale
2358af7803 fix approvals 2025-07-26 12:15:05 -07:00
Eason Goodale
6a64165349 fmt 2025-07-26 11:48:20 -07:00
Eason Goodale
306ff95020 fix invisible approvals 2025-07-26 11:48:10 -07:00
2 changed files with 178 additions and 4 deletions

View File

@@ -3,10 +3,12 @@ use codex_core::config::Config;
use codex_core::protocol::AgentMessageDeltaEvent;
use codex_core::protocol::AgentMessageEvent;
use codex_core::protocol::AgentReasoningDeltaEvent;
use codex_core::protocol::ApplyPatchApprovalRequestEvent;
use codex_core::protocol::BackgroundEventEvent;
use codex_core::protocol::ErrorEvent;
use codex_core::protocol::Event;
use codex_core::protocol::EventMsg;
use codex_core::protocol::ExecApprovalRequestEvent;
use codex_core::protocol::ExecCommandBeginEvent;
use codex_core::protocol::ExecCommandEndEvent;
use codex_core::protocol::FileChange;
@@ -474,11 +476,45 @@ impl EventProcessor for EventProcessorWithHumanOutput {
println!("{}", line.style(self.dimmed));
}
}
EventMsg::ExecApprovalRequest(_) => {
// Should we exit?
EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent {
command,
cwd,
reason,
..
}) => {
ts_println!(
self,
"{} {} in {}",
"approval required for".style(self.magenta),
escape_command(&command).style(self.bold),
cwd.to_string_lossy(),
);
if let Some(r) = reason {
ts_println!(self, "{r}");
}
return CodexStatus::InitiateShutdown;
}
EventMsg::ApplyPatchApprovalRequest(_) => {
// Should we exit?
EventMsg::ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent {
changes,
reason,
..
}) => {
ts_println!(
self,
"{}:",
"approval required for apply_patch".style(self.magenta),
);
for (path, change) in changes.iter() {
println!(
" {} {}",
format_file_change(change).style(self.cyan),
path.to_string_lossy(),
);
}
if let Some(r) = reason {
ts_println!(self, "{r}");
}
return CodexStatus::InitiateShutdown;
}
EventMsg::AgentReasoning(agent_reasoning_event) => {
if self.show_agent_reasoning {
@@ -538,3 +574,42 @@ fn format_file_change(change: &FileChange) -> &'static str {
} => "M",
}
}
#[cfg(test)]
mod tests {
use super::*;
use codex_core::config::Config;
use codex_core::config::ConfigOverrides;
use codex_core::config::ConfigToml;
use codex_core::protocol::Event;
use codex_core::protocol::EventMsg;
use codex_core::protocol::ExecApprovalRequestEvent;
fn test_config() -> Config {
Config::load_from_base_config_with_overrides(
ConfigToml::default(),
ConfigOverrides::default(),
std::env::temp_dir(),
)
.unwrap_or_else(|e| panic!("failed to load test configuration: {e}"))
}
#[test]
fn exec_approval_request_displays_command() {
let config = test_config();
let mut processor = EventProcessorWithHumanOutput::create_with_ansi(false, &config, None);
let event = Event {
id: "1".into(),
msg: EventMsg::ExecApprovalRequest(ExecApprovalRequestEvent {
call_id: "c1".into(),
command: vec!["rm".into(), "-rf".into(), "/".into()],
cwd: PathBuf::from("/tmp"),
reason: None,
}),
};
let status = processor.process_event(event);
assert!(matches!(status, CodexStatus::InitiateShutdown));
}
}

View File

@@ -368,3 +368,102 @@ impl WidgetRef for &UserApprovalWidget<'_> {
Widget::render(List::new(lines), response_chunk, buf);
}
}
#[cfg(test)]
mod tests {
use super::*;
use crate::app_event::AppEvent;
use crate::app_event_sender::AppEventSender;
use ratatui::buffer::Buffer;
use ratatui::layout::Rect;
use ratatui::widgets::WidgetRef;
use std::path::PathBuf;
use std::sync::mpsc::channel;
#[test]
fn exec_command_is_visible_in_small_viewport() {
let long_reason = "This is a very long explanatory reason that would normally occupy many lines in the confirmation prompt. \
It should not cause the actual command or the response options to be scrolled out of the visible area.";
let (tx, _rx) = channel::<AppEvent>();
let app_tx = AppEventSender::new(tx);
let cwd = PathBuf::from("/home/alice/project");
let command = vec![
"bash".to_string(),
"-lc".to_string(),
"echo 123 && printf 'hello'".to_string(),
];
let widget = UserApprovalWidget::new(
ApprovalRequest::Exec {
id: "test-id".to_string(),
command: command.clone(),
cwd: cwd.clone(),
reason: Some(long_reason.to_string()),
},
app_tx,
);
let area = Rect::new(0, 0, 50, 12);
let mut buf = Buffer::empty(area);
(&widget).render_ref(area, &mut buf);
let mut rendered = String::new();
for y in 0..area.height {
for x in 0..area.width {
let cell = &buf[(x, y)];
rendered.push(cell.symbol().chars().next().unwrap_or('\0'));
}
rendered.push('\n');
}
assert!(
rendered.contains("echo 123 && printf 'hello'"),
"rendered buffer did not contain the command.\n--- buffer ---\n{rendered}\n----------------"
);
assert!(rendered.contains("Yes (y)"));
}
#[test]
fn all_options_visible_in_reasonable_viewport() {
let (tx, _rx) = channel::<AppEvent>();
let app_tx = AppEventSender::new(tx);
let widget = UserApprovalWidget::new(
ApprovalRequest::Exec {
id: "test-id".to_string(),
command: vec![
"bash".into(),
"-lc".into(),
"echo 123 && printf 'hello'".into(),
],
cwd: PathBuf::from("/home/alice/project"),
reason: Some("short reason".into()),
},
app_tx,
);
// Use a generous area to avoid truncation of either the prompt or the options.
let area = Rect::new(0, 0, 100, 30);
let mut buf = Buffer::empty(area);
(&widget).render_ref(area, &mut buf);
let mut rendered = String::new();
for y in 0..area.height {
for x in 0..area.width {
let cell = &buf[(x, y)];
rendered.push(cell.symbol().chars().next().unwrap_or('\0'));
}
rendered.push('\n');
}
for opt in super::SELECT_OPTIONS {
assert!(
rendered.contains(opt.label),
"expected option label to be visible: {}\n--- buffer ---\n{rendered}\n----------------",
opt.label
);
}
}
}