fix(tui/auto-drive): tighten approval UI and esc handling

This commit is contained in:
James Peter
2025-10-11 08:14:06 +10:00
parent 2c1d011649
commit 0c66e42e2a
3 changed files with 246 additions and 53 deletions
+14
View File
@@ -93,6 +93,20 @@ The command execution flow in Codex follows an event-driven pattern:
This architecture separates concerns between execution logic (core), UI state management (chatwidget), and rendering (history_cell).
### Auto Drive Escape Handling
- All Auto Drive escape routing lives in `code-rs/tui/src/chatwidget.rs`. The
`ChatWidget::auto_should_handle_global_esc` helper decides whether the global
Esc handler in `app.rs` should defer to Auto Drive, and
`ChatWidget::handle_key_event` owns the actual stop / pause behaviour. When
you need to tweak Esc semantics, update those two locations together.
- The approval pane must *never* swallow Esc. `code-rs/tui/src/bottom_pane/auto_coordinator_view.rs`
intentionally lets Esc (and the other approval shortcuts) bubble back to the
chat widget; keep this contract intact when editing the view layer.
- Avoid adding additional Esc handlers elsewhere for Auto Drive flows. Doing
so breaks the modal-first ordering in `app.rs` and prevents users from
reliably stopping a run.
## Writing New UI Regression Tests
- Start with `make_chatwidget_manual()` (or `make_chatwidget_manual_with_sender()`) to build a `ChatWidget` in isolation with in-memory channels.
@@ -35,6 +35,8 @@ pub(crate) struct AutoActiveViewModel {
pub goal: Option<String>,
pub status_lines: Vec<String>,
pub cli_prompt: Option<String>,
pub cli_context: Option<String>,
pub show_composer: bool,
pub awaiting_submission: bool,
pub waiting_for_response: bool,
pub countdown: Option<CountdownState>,
@@ -99,7 +101,11 @@ impl AutoCoordinatorView {
// with the final render width, subtract those 3 exterior columns before
// delegating to `ChatComposer::desired_height`.
let composer_width = width.saturating_sub(3);
let composer_height = composer.desired_height(composer_width);
let composer_height = if model.show_composer {
composer.desired_height(composer_width)
} else {
0
};
self.estimated_height_active(width, &ctx, model, composer_height)
}
@@ -152,6 +158,22 @@ impl AutoCoordinatorView {
return true;
}
let awaiting_without_input = matches!(
&self.model,
AutoCoordinatorViewModel::Active(model)
if model.awaiting_submission && !model.show_composer
);
if awaiting_without_input {
// Allow approval keys to bubble so ChatWidget handles them.
let allow_passthrough = matches!(
key_event.code,
KeyCode::Esc | KeyCode::Enter | KeyCode::Char(' ') | KeyCode::Char('e') | KeyCode::Char('E')
);
if !allow_passthrough {
return true;
}
}
matches!(key_event.code, KeyCode::Up | KeyCode::Down)
}
@@ -227,20 +249,104 @@ impl AutoCoordinatorView {
lines
}
fn cli_prompt_lines(&self, model: &AutoActiveViewModel, style: Style) -> Option<Vec<Line<'static>>> {
model.cli_prompt.as_ref().map(|prompt| {
prompt
.lines()
.map(|line| Line::from(Span::styled(line.trim_end().to_string(), style)))
.collect()
})
fn cli_prompt_lines(&self, model: &AutoActiveViewModel) -> Option<Vec<Line<'static>>> {
let prompt = model
.cli_prompt
.as_ref()
.map(|value| value.trim_end())
.filter(|value| !value.is_empty());
let context = model
.cli_context
.as_ref()
.map(|value| value.trim())
.filter(|value| !value.is_empty());
if prompt.is_none() && context.is_none() {
return None;
}
let header_style = Style::default()
.fg(colors::text())
.add_modifier(Modifier::BOLD);
let context_label_style = Style::default()
.fg(colors::text_dim())
.add_modifier(Modifier::BOLD);
let context_body_style = Style::default()
.fg(colors::text_dim())
.add_modifier(Modifier::ITALIC);
let prompt_label_style = Style::default()
.fg(colors::info())
.add_modifier(Modifier::BOLD);
let prompt_body_style = Style::default().fg(colors::text());
let mut lines: Vec<Line<'static>> = Vec::new();
lines.push(Line::from(vec![
Span::raw(" "),
Span::styled("Auto Drive will send:", header_style),
]));
if let Some(value) = context {
lines.push(Line::from(vec![
Span::raw(" "),
Span::styled("Preface:", context_label_style),
]));
for line in value.lines() {
let trimmed = line.trim_end();
if trimmed.is_empty() {
lines.push(Line::default());
} else {
lines.push(Line::from(vec![
Span::raw(" "),
Span::styled(trimmed.to_string(), context_body_style),
]));
}
}
}
if context.is_some() && prompt.is_some() {
lines.push(Line::default());
}
if let Some(value) = prompt {
lines.push(Line::from(vec![
Span::raw(" "),
Span::styled("Prompt:", prompt_label_style),
]));
for line in value.lines() {
let trimmed = line.trim_end();
if trimmed.is_empty() {
lines.push(Line::default());
} else {
lines.push(Line::from(vec![
Span::raw(" "),
Span::styled(trimmed.to_string(), prompt_body_style),
]));
}
}
} else {
lines.push(Line::from(vec![
Span::raw(" "),
Span::styled("Prompt:", prompt_label_style),
]));
lines.push(Line::from(vec![
Span::raw(" "),
Span::styled(
"(Coordinator did not supply a prompt)".to_string(),
prompt_body_style.add_modifier(Modifier::ITALIC),
),
]));
}
Some(lines)
}
fn manual_hint_line(&self, ctx: &VariantContext) -> Option<Line<'static>> {
ctx.manual_hint.as_ref().map(|hint| {
Line::from(Span::styled(
hint.clone(),
Style::default().fg(colors::warning()),
Style::default()
.fg(colors::info())
.add_modifier(Modifier::ITALIC),
))
})
}
@@ -376,12 +482,19 @@ impl AutoCoordinatorView {
let mut total = 0usize;
if model.awaiting_submission {
if let Some(prompt) = &model.cli_prompt {
total += Self::wrap_count(prompt, inner_width).max(1);
if let Some(prompt_lines) = self.cli_prompt_lines(model) {
let prompt_height = Self::lines_height(&prompt_lines, inner_width) as usize;
total += prompt_height;
if prompt_height > 0 && ctx.button.is_some() {
total += 1; // spacer before button
}
}
if ctx.button.is_some() {
total += button_height;
}
if ctx.manual_hint.is_some() {
total += hint_height.max(1);
}
if ctrl_height > 0 {
total += 1; // spacer before ctrl hint
total += ctrl_height.max(1);
@@ -438,14 +551,22 @@ impl AutoCoordinatorView {
let mut after_lines: Vec<Line<'static>> = Vec::new();
if model.awaiting_submission {
if let Some(prompt_lines) =
self.cli_prompt_lines(model, Style::default().fg(colors::text()))
{
top_lines.extend(prompt_lines);
if let Some(mut prompt_lines) = self.cli_prompt_lines(model) {
top_lines.append(&mut prompt_lines);
}
if let Some(button_block) = self.button_block_lines(&ctx) {
after_lines.extend(button_block);
if !top_lines.is_empty() {
top_lines.push(Line::default());
}
top_lines.extend(button_block);
}
if let Some(hint_line) = self.manual_hint_line(&ctx) {
if !after_lines.is_empty() {
after_lines.push(Line::default());
}
after_lines.push(hint_line);
}
if let Some(ctrl_hint_line) = self.ctrl_hint_line(&ctx) {
@@ -506,13 +627,16 @@ impl AutoCoordinatorView {
// columns wider than the tight composer rectangle (to account for the
// outer horizontal padding applied by the BottomPane). Reconstruct that
// width so height estimation matches render-time wrapping exactly.
let measurement_width = inner.width.saturating_add(2);
let mut desired_block = composer.desired_height(measurement_width);
if desired_block < Self::MIN_COMPOSER_VIEWPORT {
desired_block = Self::MIN_COMPOSER_VIEWPORT;
}
let mut composer_block: u16 = desired_block;
let mut composer_block: u16 = if model.show_composer {
let measurement_width = inner.width.saturating_add(2);
let mut desired_block = composer.desired_height(measurement_width);
if desired_block < Self::MIN_COMPOSER_VIEWPORT {
desired_block = Self::MIN_COMPOSER_VIEWPORT;
}
desired_block
} else {
0
};
let total_needed = top_height as usize
+ after_height as usize
@@ -536,23 +660,27 @@ impl AutoCoordinatorView {
deficit -= reduce_summary;
}
if deficit > 0 {
if deficit > 0 && model.show_composer {
let reducible = composer_block.saturating_sub(Self::MIN_COMPOSER_VIEWPORT);
let reduce_composer = usize::from(reducible).min(deficit);
composer_block = composer_block.saturating_sub(reduce_composer as u16);
}
}
let max_space_for_composer = inner
.height
.saturating_sub(top_height)
.saturating_sub(after_height)
.saturating_sub(summary_height);
let composer_height = if model.show_composer {
let max_space_for_composer = inner
.height
.saturating_sub(top_height)
.saturating_sub(after_height)
.saturating_sub(summary_height);
let composer_height = if max_space_for_composer == 0 {
1
if max_space_for_composer == 0 {
1
} else {
composer_block.min(max_space_for_composer).max(1)
}
} else {
composer_block.min(max_space_for_composer).max(1)
0
};
let mut cursor_y = inner.y;
+72 -21
View File
@@ -4486,14 +4486,19 @@ impl ChatWidget<'_> {
if matches!(key_event.kind, KeyEventKind::Press | KeyEventKind::Repeat)
&& matches!(key_event.code, crossterm::event::KeyCode::Esc)
&& self.auto_state.active
{
if self.auto_state.countdown_active() {
self.auto_pause_for_manual_edit();
if self.auto_state.active {
if self.auto_state.countdown_active() {
self.auto_pause_for_manual_edit();
return;
}
if self.auto_state.awaiting_submission {
self.auto_stop(Some("Auto Drive stopped during approval.".to_string()));
} else {
self.auto_stop(Some("Auto Drive stopped by user.".to_string()));
}
return;
}
self.auto_stop(Some("Auto Drive stopped by user.".to_string()));
return;
}
if matches!(key_event.kind, KeyEventKind::Press | KeyEventKind::Repeat)
@@ -4520,6 +4525,12 @@ impl ChatWidget<'_> {
self.auto_submit_prompt();
return;
}
crossterm::event::KeyCode::Char('e') | crossterm::event::KeyCode::Char('E')
if key_event.modifiers.is_empty() =>
{
self.auto_pause_for_manual_edit();
return;
}
_ => {}
}
}
@@ -11985,6 +11996,8 @@ fi\n\
goal: None,
status_lines,
cli_prompt: None,
cli_context: None,
show_composer: true,
awaiting_submission: false,
waiting_for_response: false,
countdown: None,
@@ -12891,18 +12904,29 @@ fi\n\
if !self.auto_state.active || !self.auto_state.awaiting_submission {
return;
}
let Some(prompt) = self.auto_state.current_cli_prompt.clone() else {
return;
};
let full_prompt = self.auto_build_cli_message(&prompt);
let prompt_text = self
.auto_state
.current_cli_prompt
.clone()
.unwrap_or_default();
let mut full_prompt = self.auto_build_cli_message(&prompt_text);
if full_prompt.trim().is_empty() {
full_prompt = self
.auto_state
.current_cli_context
.clone()
.unwrap_or_default();
}
self.auto_state.paused_for_manual_edit = true;
self.auto_state.resume_after_manual_submit = true;
self.auto_state.countdown_id = self.auto_state.countdown_id.wrapping_add(1);
self.auto_state.reset_countdown();
self.clear_composer();
self.insert_str(&full_prompt);
if !full_prompt.is_empty() {
self.insert_str(&full_prompt);
}
self.bottom_pane.ensure_input_focus();
self.bottom_pane.set_task_running(true);
self.bottom_pane
@@ -13217,6 +13241,8 @@ fi\n\
goal: summary.goal.clone(),
status_lines,
cli_prompt: None,
cli_context: None,
show_composer: true,
awaiting_submission: false,
waiting_for_response: false,
countdown: None,
@@ -13333,8 +13359,14 @@ fi\n\
.current_cli_prompt
.clone()
.filter(|p| !p.trim().is_empty());
let cli_context = self
.auto_state
.current_cli_context
.clone()
.filter(|value| !value.trim().is_empty());
let countdown_limit = self.auto_state.countdown_seconds();
let countdown_active = self.auto_state.countdown_active();
let countdown = if self.auto_state.awaiting_submission {
match countdown_limit {
Some(limit) if limit > 0 => Some(CountdownState {
@@ -13347,14 +13379,10 @@ fi\n\
};
let button = if self.auto_state.awaiting_submission {
let label = if self.auto_state.countdown_active() {
format!("Continue ({}s)", self.auto_state.seconds_remaining)
let label = if countdown_active {
format!("Send prompt ({}s)", self.auto_state.seconds_remaining)
} else {
match countdown_limit {
Some(0) => "Continue now".to_string(),
Some(_) => "Continue now".to_string(),
None => "Continue".to_string(),
}
"Send prompt".to_string()
};
Some(AutoCoordinatorButton {
label,
@@ -13366,10 +13394,28 @@ fi\n\
None
};
let manual_hint = None;
let show_composer = !self.auto_state.awaiting_submission || self.auto_state.paused_for_manual_edit;
let manual_hint = if self.auto_state.awaiting_submission {
if self.auto_state.paused_for_manual_edit {
Some("Edit the prompt, then press Enter to continue.".to_string())
} else if countdown_active {
Some("Enter to send now • Esc/E to edit prompt".to_string())
} else {
Some("Enter to send • E to edit prompt • Esc to cancel Auto Drive".to_string())
}
} else {
None
};
let ctrl_switch_hint = if self.auto_state.awaiting_submission {
"Esc to cancel".to_string()
if countdown_active {
"Esc to edit prompt".to_string()
} else if self.auto_state.paused_for_manual_edit {
"Esc to cancel".to_string()
} else {
"E edit prompt • Esc cancel Auto Drive".to_string()
}
} else if self.auto_state.waiting_for_response {
String::new()
} else {
@@ -13394,6 +13440,8 @@ fi\n\
elapsed: None,
progress_past: self.auto_state.current_progress_past.clone(),
progress_current: self.auto_state.current_progress_current.clone(),
cli_context,
show_composer,
});
self
@@ -15755,8 +15803,11 @@ fi\n\
/// routing through the generic interrupt logic.
pub(crate) fn auto_should_handle_global_esc(&self) -> bool {
self.auto_state.active
&& self.auto_state.awaiting_submission
&& self.auto_state.paused_for_manual_edit
&& (
self.auto_state.awaiting_submission
|| self.auto_state.waiting_for_response
|| self.auto_state.coordinator_waiting
)
}
pub(crate) fn auto_manual_entry_active(&self) -> bool {