fix(tui): queued-message edit shortcut unreachable in some terminals (#12240)
## Problem The TUI's "edit queued message" shortcut (Alt+Up) is either silently swallowed or recognized as another key combination by Apple Terminal, Warp, and VSCode's integrated terminal on macOS. Users in those environments see the hint but pressing the keys does nothing. ## Mental model When a model turn is in progress the user can still type follow-up messages. These are queued and displayed below the composer with a hint line showing how to pop the most recent one back into the editor. The hint text and the actual key handler must agree on which shortcut is used, and that shortcut must actually reach the TUI—i.e. it must not be intercepted by the host terminal. Three terminals are known to intercept Alt+Up: Apple Terminal (remaps it to cursor movement), Warp (consumes it for its own command palette), and VSCode (maps it to "move line up"). For these we use Shift+Left instead. <p align="center"> <img width="283" height="182" alt="image" src="https://github.com/user-attachments/assets/4a9c5d13-6e47-4157-bb41-28b4ce96a914" /> </p> | macOS Native Terminal | Warp | VSCode Terminal | |---|---|---| | <img width="1557" height="1010" alt="SCR-20260219-kigi" src="https://github.com/user-attachments/assets/f4ff52f8-119e-407b-a3f3-52f564c36d70" /> | <img width="1479" height="1261" alt="SCR-20260219-krrf" src="https://github.com/user-attachments/assets/5807d7c4-17ae-4a2b-aa27-238fd49d90fd" /> | <img width="1612" height="1312" alt="SCR-20260219-ksbz" src="https://github.com/user-attachments/assets/1cedb895-6966-4d63-ac5f-0eea0f7057e8" /> | ## Non-goals - Making the binding user-configurable at runtime (deferred to a broader keybinding-config effort). - Remapping any other shortcuts that might be terminal-specific. ## Tradeoffs - **Exhaustive match instead of a wildcard default.** The `queued_message_edit_binding_for_terminal` function explicitly lists every `TerminalName` variant. This is intentional: adding a new terminal to the enum will produce a compile error, forcing the author to decide which binding that terminal should use. - **Binding lives on `ChatWidget`, hint lives on `QueuedUserMessages`.** The key event handler that actually acts on the press is in `ChatWidget`, but the rendered hint text is inside `QueuedUserMessages`. These are kept in sync by `ChatWidget` calling `bottom_pane.set_queued_message_edit_binding(self.queued_message_edit_binding)` during construction. A mismatch would show the wrong hint but would not lose data. ## Architecture ```mermaid graph TD TI["terminal_info().name"] --> FN["queued_message_edit_binding_for_terminal(name)"] FN --> KB["KeyBinding"] KB --> CW["ChatWidget.queued_message_edit_binding<br/><i>key event matching</i>"] KB --> BP["BottomPane.set_queued_message_edit_binding()"] BP --> QUM["QueuedUserMessages.edit_binding<br/><i>rendered in hint line</i>"] subgraph "Special terminals (Shift+Left)" AT["Apple Terminal"] WT["Warp"] VS["VSCode"] end subgraph "Default (Alt+Up)" GH["Ghostty"] IT["iTerm2"] OT["Others…"] end AT --> FN WT --> FN VS --> FN GH --> FN IT --> FN OT --> FN ``` No new crates or public API surface. The only cross-crate dependency added is `codex_core::terminal::{TerminalName, terminal_info}`, which already existed for telemetry. ## Observability No new logging. Terminal detection already emits a `tracing::debug!` log line at startup with the detected terminal name, which is sufficient to diagnose binding mismatches. ## Tests - Existing `alt_up_edits_most_recent_queued_message` test is preserved and explicitly sets the Alt+Up binding to isolate from the host terminal. - New parameterized async tests verify Shift+Left works for Apple Terminal, Warp, and VSCode. - A sync unit test asserts the mapping table covers the three special terminals (Shift+Left) and that iTerm2 still gets Alt+Up. Fixes #4490
This commit is contained in:
parent
4ebdddaa34
commit
a5d0757ed1
4 changed files with 165 additions and 14 deletions
|
|
@ -287,6 +287,13 @@ impl BottomPane {
|
|||
self.request_redraw();
|
||||
}
|
||||
|
||||
/// Update the key hint shown next to queued messages so it matches the
|
||||
/// binding that `ChatWidget` actually listens for.
|
||||
pub(crate) fn set_queued_message_edit_binding(&mut self, binding: KeyBinding) {
|
||||
self.queued_user_messages.set_edit_binding(binding);
|
||||
self.request_redraw();
|
||||
}
|
||||
|
||||
pub fn status_widget(&self) -> Option<&StatusIndicatorWidget> {
|
||||
self.status.as_ref()
|
||||
}
|
||||
|
|
|
|||
|
|
@ -11,17 +11,33 @@ use crate::wrapping::RtOptions;
|
|||
use crate::wrapping::word_wrap_lines;
|
||||
|
||||
/// Widget that displays a list of user messages queued while a turn is in progress.
|
||||
///
|
||||
/// The widget shows a key hint at the bottom (e.g. "⌥ + ↑ edit") telling the
|
||||
/// user how to pop the most recent queued message back into the composer.
|
||||
/// Because some terminals intercept certain modifier-key combinations, the
|
||||
/// displayed binding is configurable via [`set_edit_binding`](Self::set_edit_binding).
|
||||
pub(crate) struct QueuedUserMessages {
|
||||
pub messages: Vec<String>,
|
||||
/// Key combination rendered in the hint line. Defaults to Alt+Up but may
|
||||
/// be overridden for terminals where that chord is unavailable.
|
||||
edit_binding: key_hint::KeyBinding,
|
||||
}
|
||||
|
||||
impl QueuedUserMessages {
|
||||
pub(crate) fn new() -> Self {
|
||||
Self {
|
||||
messages: Vec::new(),
|
||||
edit_binding: key_hint::alt(KeyCode::Up),
|
||||
}
|
||||
}
|
||||
|
||||
/// Replace the keybinding shown in the hint line at the bottom of the
|
||||
/// queued-messages list. The caller is responsible for also wiring the
|
||||
/// corresponding key event handler.
|
||||
pub(crate) fn set_edit_binding(&mut self, binding: key_hint::KeyBinding) {
|
||||
self.edit_binding = binding;
|
||||
}
|
||||
|
||||
fn as_renderable(&self, width: u16) -> Box<dyn Renderable> {
|
||||
if self.messages.is_empty() || width < 4 {
|
||||
return Box::new(());
|
||||
|
|
@ -48,7 +64,7 @@ impl QueuedUserMessages {
|
|||
lines.push(
|
||||
Line::from(vec![
|
||||
" ".into(),
|
||||
key_hint::alt(KeyCode::Up).into(),
|
||||
self.edit_binding.into(),
|
||||
" edit".into(),
|
||||
])
|
||||
.dim(),
|
||||
|
|
|
|||
|
|
@ -110,6 +110,8 @@ use codex_core::protocol::WarningEvent;
|
|||
use codex_core::protocol::WebSearchBeginEvent;
|
||||
use codex_core::protocol::WebSearchEndEvent;
|
||||
use codex_core::skills::model::SkillMetadata;
|
||||
use codex_core::terminal::TerminalName;
|
||||
use codex_core::terminal::terminal_info;
|
||||
#[cfg(target_os = "windows")]
|
||||
use codex_core::windows_sandbox::WindowsSandboxLevelExt;
|
||||
use codex_otel::OtelManager;
|
||||
|
|
@ -158,6 +160,34 @@ const PLAN_IMPLEMENTATION_NO: &str = "No, stay in Plan mode";
|
|||
const PLAN_IMPLEMENTATION_CODING_MESSAGE: &str = "Implement the plan.";
|
||||
const CONNECTORS_SELECTION_VIEW_ID: &str = "connectors-selection";
|
||||
|
||||
/// Choose the keybinding used to edit the most-recently queued message.
|
||||
///
|
||||
/// Apple Terminal, Warp, and VSCode integrated terminals intercept or silently
|
||||
/// swallow Alt+Up, so users in those environments would never be able to trigger
|
||||
/// the edit action. We fall back to Shift+Left for those terminals while
|
||||
/// keeping the more discoverable Alt+Up everywhere else.
|
||||
///
|
||||
/// The match is exhaustive so that adding a new `TerminalName` variant forces
|
||||
/// an explicit decision about which binding that terminal should use.
|
||||
fn queued_message_edit_binding_for_terminal(terminal_name: TerminalName) -> KeyBinding {
|
||||
match terminal_name {
|
||||
TerminalName::AppleTerminal | TerminalName::WarpTerminal | TerminalName::VsCode => {
|
||||
key_hint::shift(KeyCode::Left)
|
||||
}
|
||||
TerminalName::Ghostty
|
||||
| TerminalName::Iterm2
|
||||
| TerminalName::WezTerm
|
||||
| TerminalName::Kitty
|
||||
| TerminalName::Alacritty
|
||||
| TerminalName::Konsole
|
||||
| TerminalName::GnomeTerminal
|
||||
| TerminalName::Vte
|
||||
| TerminalName::WindowsTerminal
|
||||
| TerminalName::Dumb
|
||||
| TerminalName::Unknown => key_hint::alt(KeyCode::Up),
|
||||
}
|
||||
}
|
||||
|
||||
use crate::app_event::AppEvent;
|
||||
use crate::app_event::ConnectorsSnapshot;
|
||||
use crate::app_event::ExitMode;
|
||||
|
|
@ -563,6 +593,11 @@ pub(crate) struct ChatWidget {
|
|||
suppress_session_configured_redraw: bool,
|
||||
// User messages queued while a turn is in progress
|
||||
queued_user_messages: VecDeque<UserMessage>,
|
||||
/// Terminal-appropriate keybinding for popping the most-recently queued
|
||||
/// message back into the composer. Determined once at construction time via
|
||||
/// [`queued_message_edit_binding_for_terminal`] and propagated to
|
||||
/// `BottomPane` so the hint text matches the actual shortcut.
|
||||
queued_message_edit_binding: KeyBinding,
|
||||
// Pending notification to show when unfocused on next Draw
|
||||
pending_notification: Option<Notification>,
|
||||
/// When `Some`, the user has pressed a quit shortcut and the second press
|
||||
|
|
@ -2605,6 +2640,8 @@ impl ChatWidget {
|
|||
let active_cell = Some(Self::placeholder_session_header_cell(&config));
|
||||
|
||||
let current_cwd = Some(config.cwd.clone());
|
||||
let queued_message_edit_binding =
|
||||
queued_message_edit_binding_for_terminal(terminal_info().name);
|
||||
let mut widget = Self {
|
||||
app_event_tx: app_event_tx.clone(),
|
||||
frame_requester: frame_requester.clone(),
|
||||
|
|
@ -2662,6 +2699,7 @@ impl ChatWidget {
|
|||
thread_name: None,
|
||||
forked_from: None,
|
||||
queued_user_messages: VecDeque::new(),
|
||||
queued_message_edit_binding,
|
||||
show_welcome_banner: is_first_run,
|
||||
suppress_session_configured_redraw: false,
|
||||
pending_notification: None,
|
||||
|
|
@ -2702,6 +2740,9 @@ impl ChatWidget {
|
|||
widget.config.features.enabled(Feature::CollaborationModes),
|
||||
);
|
||||
widget.sync_personality_command_enabled();
|
||||
widget
|
||||
.bottom_pane
|
||||
.set_queued_message_edit_binding(widget.queued_message_edit_binding);
|
||||
#[cfg(target_os = "windows")]
|
||||
widget.bottom_pane.set_windows_degraded_sandbox_active(
|
||||
codex_core::windows_sandbox::ELEVATED_SANDBOX_NUX_ENABLED
|
||||
|
|
@ -2769,6 +2810,8 @@ impl ChatWidget {
|
|||
let active_cell = Some(Self::placeholder_session_header_cell(&config));
|
||||
let current_cwd = Some(config.cwd.clone());
|
||||
|
||||
let queued_message_edit_binding =
|
||||
queued_message_edit_binding_for_terminal(terminal_info().name);
|
||||
let mut widget = Self {
|
||||
app_event_tx: app_event_tx.clone(),
|
||||
frame_requester: frame_requester.clone(),
|
||||
|
|
@ -2830,6 +2873,7 @@ impl ChatWidget {
|
|||
plan_delta_buffer: String::new(),
|
||||
plan_item_active: false,
|
||||
queued_user_messages: VecDeque::new(),
|
||||
queued_message_edit_binding,
|
||||
show_welcome_banner: is_first_run,
|
||||
suppress_session_configured_redraw: false,
|
||||
pending_notification: None,
|
||||
|
|
@ -2866,6 +2910,9 @@ impl ChatWidget {
|
|||
widget.config.features.enabled(Feature::CollaborationModes),
|
||||
);
|
||||
widget.sync_personality_command_enabled();
|
||||
widget
|
||||
.bottom_pane
|
||||
.set_queued_message_edit_binding(widget.queued_message_edit_binding);
|
||||
|
||||
widget
|
||||
}
|
||||
|
|
@ -2922,6 +2969,8 @@ impl ChatWidget {
|
|||
settings: fallback_default,
|
||||
};
|
||||
|
||||
let queued_message_edit_binding =
|
||||
queued_message_edit_binding_for_terminal(terminal_info().name);
|
||||
let mut widget = Self {
|
||||
app_event_tx: app_event_tx.clone(),
|
||||
frame_requester: frame_requester.clone(),
|
||||
|
|
@ -2979,6 +3028,7 @@ impl ChatWidget {
|
|||
thread_name: None,
|
||||
forked_from: None,
|
||||
queued_user_messages: VecDeque::new(),
|
||||
queued_message_edit_binding,
|
||||
show_welcome_banner: false,
|
||||
suppress_session_configured_redraw: true,
|
||||
pending_notification: None,
|
||||
|
|
@ -3019,6 +3069,9 @@ impl ChatWidget {
|
|||
widget.config.features.enabled(Feature::CollaborationModes),
|
||||
);
|
||||
widget.sync_personality_command_enabled();
|
||||
widget
|
||||
.bottom_pane
|
||||
.set_queued_message_edit_binding(widget.queued_message_edit_binding);
|
||||
#[cfg(target_os = "windows")]
|
||||
widget.bottom_pane.set_windows_degraded_sandbox_active(
|
||||
codex_core::windows_sandbox::ELEVATED_SANDBOX_NUX_ENABLED
|
||||
|
|
@ -3091,6 +3144,18 @@ impl ChatWidget {
|
|||
_ => {}
|
||||
}
|
||||
|
||||
if key_event.kind == KeyEventKind::Press
|
||||
&& self.queued_message_edit_binding.is_press(key_event)
|
||||
&& !self.queued_user_messages.is_empty()
|
||||
{
|
||||
if let Some(user_message) = self.queued_user_messages.pop_back() {
|
||||
self.restore_user_message_to_composer(user_message);
|
||||
self.refresh_queued_user_messages();
|
||||
self.request_redraw();
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
match key_event {
|
||||
KeyEvent {
|
||||
code: KeyCode::BackTab,
|
||||
|
|
@ -3102,19 +3167,6 @@ impl ChatWidget {
|
|||
{
|
||||
self.cycle_collaboration_mode();
|
||||
}
|
||||
KeyEvent {
|
||||
code: KeyCode::Up,
|
||||
modifiers: KeyModifiers::ALT,
|
||||
kind: KeyEventKind::Press,
|
||||
..
|
||||
} if !self.queued_user_messages.is_empty() => {
|
||||
// Prefer the most recently queued item.
|
||||
if let Some(user_message) = self.queued_user_messages.pop_back() {
|
||||
self.restore_user_message_to_composer(user_message);
|
||||
self.refresh_queued_user_messages();
|
||||
self.request_redraw();
|
||||
}
|
||||
}
|
||||
_ => match self.bottom_pane.handle_key_event(key_event) {
|
||||
InputResult::Submitted {
|
||||
text,
|
||||
|
|
|
|||
|
|
@ -67,6 +67,7 @@ use codex_core::protocol::UndoStartedEvent;
|
|||
use codex_core::protocol::ViewImageToolCallEvent;
|
||||
use codex_core::protocol::WarningEvent;
|
||||
use codex_core::skills::model::SkillMetadata;
|
||||
use codex_core::terminal::TerminalName;
|
||||
use codex_otel::OtelManager;
|
||||
use codex_otel::RuntimeMetricsSummary;
|
||||
use codex_protocol::ThreadId;
|
||||
|
|
@ -1640,6 +1641,7 @@ async fn make_chatwidget_manual(
|
|||
frame_requester: FrameRequester::test_dummy(),
|
||||
show_welcome_banner: true,
|
||||
queued_user_messages: VecDeque::new(),
|
||||
queued_message_edit_binding: crate::key_hint::alt(KeyCode::Up),
|
||||
suppress_session_configured_redraw: false,
|
||||
pending_notification: None,
|
||||
quit_shortcut_expires_at: None,
|
||||
|
|
@ -2781,6 +2783,9 @@ async fn empty_enter_during_task_does_not_queue() {
|
|||
#[tokio::test]
|
||||
async fn alt_up_edits_most_recent_queued_message() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
chat.queued_message_edit_binding = crate::key_hint::alt(KeyCode::Up);
|
||||
chat.bottom_pane
|
||||
.set_queued_message_edit_binding(crate::key_hint::alt(KeyCode::Up));
|
||||
|
||||
// Simulate a running task so messages would normally be queued.
|
||||
chat.bottom_pane.set_task_running(true);
|
||||
|
|
@ -2808,6 +2813,77 @@ async fn alt_up_edits_most_recent_queued_message() {
|
|||
);
|
||||
}
|
||||
|
||||
async fn assert_shift_left_edits_most_recent_queued_message_for_terminal(
|
||||
terminal_name: TerminalName,
|
||||
) {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await;
|
||||
chat.queued_message_edit_binding = queued_message_edit_binding_for_terminal(terminal_name);
|
||||
chat.bottom_pane
|
||||
.set_queued_message_edit_binding(chat.queued_message_edit_binding);
|
||||
|
||||
// Simulate a running task so messages would normally be queued.
|
||||
chat.bottom_pane.set_task_running(true);
|
||||
|
||||
// Seed two queued messages.
|
||||
chat.queued_user_messages
|
||||
.push_back(UserMessage::from("first queued".to_string()));
|
||||
chat.queued_user_messages
|
||||
.push_back(UserMessage::from("second queued".to_string()));
|
||||
chat.refresh_queued_user_messages();
|
||||
|
||||
// Press Shift+Left to edit the most recent (last) queued message.
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Left, KeyModifiers::SHIFT));
|
||||
|
||||
// Composer should now contain the last queued message.
|
||||
assert_eq!(
|
||||
chat.bottom_pane.composer_text(),
|
||||
"second queued".to_string()
|
||||
);
|
||||
// And the queue should now contain only the remaining (older) item.
|
||||
assert_eq!(chat.queued_user_messages.len(), 1);
|
||||
assert_eq!(
|
||||
chat.queued_user_messages.front().unwrap().text,
|
||||
"first queued"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn shift_left_edits_most_recent_queued_message_in_apple_terminal() {
|
||||
assert_shift_left_edits_most_recent_queued_message_for_terminal(TerminalName::AppleTerminal)
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn shift_left_edits_most_recent_queued_message_in_warp_terminal() {
|
||||
assert_shift_left_edits_most_recent_queued_message_for_terminal(TerminalName::WarpTerminal)
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn shift_left_edits_most_recent_queued_message_in_vscode_terminal() {
|
||||
assert_shift_left_edits_most_recent_queued_message_for_terminal(TerminalName::VsCode).await;
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn queued_message_edit_binding_mapping_covers_special_terminals() {
|
||||
assert_eq!(
|
||||
queued_message_edit_binding_for_terminal(TerminalName::AppleTerminal),
|
||||
crate::key_hint::shift(KeyCode::Left)
|
||||
);
|
||||
assert_eq!(
|
||||
queued_message_edit_binding_for_terminal(TerminalName::WarpTerminal),
|
||||
crate::key_hint::shift(KeyCode::Left)
|
||||
);
|
||||
assert_eq!(
|
||||
queued_message_edit_binding_for_terminal(TerminalName::VsCode),
|
||||
crate::key_hint::shift(KeyCode::Left)
|
||||
);
|
||||
assert_eq!(
|
||||
queued_message_edit_binding_for_terminal(TerminalName::Iterm2),
|
||||
crate::key_hint::alt(KeyCode::Up)
|
||||
);
|
||||
}
|
||||
|
||||
/// Pressing Up to recall the most recent history entry and immediately queuing
|
||||
/// it while a task is running should always enqueue the same text, even when it
|
||||
/// is queued repeatedly.
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue