fix(tui): queued-message edit shortcut unreachable in some terminals#12240
fix(tui): queued-message edit shortcut unreachable in some terminals#12240etraut-openai merged 1 commit intoopenai:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #4490 where the "edit queued message" keyboard shortcut (Alt+Up) is intercepted by certain terminals (Apple Terminal, Warp, VSCode) on macOS, preventing users from editing queued messages during a model turn. The solution implements terminal-specific key binding selection, using Shift+Left for problematic terminals while keeping Alt+Up for others.
Changes:
- Introduced
queued_message_edit_binding_for_terminal()function that maps terminal types to appropriate key bindings using exhaustive matching - Modified
ChatWidgetconstructors to detect terminal type and set the appropriate binding at initialization - Updated key event handling to use dynamic binding instead of hard-coded Alt+Up
- Added
set_queued_message_edit_binding()method toBottomPaneandQueuedUserMessagesto synchronize displayed hints with actual bindings - Added comprehensive test coverage for terminal-specific bindings
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
codex-rs/tui/src/chatwidget.rs |
Added terminal detection function, updated all three constructors to initialize binding, replaced hard-coded Alt+Up handler with dynamic binding check |
codex-rs/tui/src/bottom_pane/mod.rs |
Added set_queued_message_edit_binding() method to propagate binding to queued messages widget |
codex-rs/tui/src/bottom_pane/queued_user_messages.rs |
Made edit binding configurable, updated widget to use dynamic binding in hint display |
codex-rs/tui/src/chatwidget/tests.rs |
Updated existing test to explicitly set Alt+Up binding, added new tests for Shift+Left behavior in problematic terminals, added unit test verifying binding mappings |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
joshka-oai
left a comment
There was a problem hiding this comment.
LGTM - on VSCode I see Opt+Up gets interpreted as Ctrl+Up btw. I'm unsure if this is default or something configured. So we could keep things there and just detect that instead. To test, gh clone crossterm-rs/crossterm then run cargo run --example event-read
I rolled my own tiny crate using crossterm for it. The only small difference is that it also allows toggling kitty support on and off: https://github.com/fcoury/ktt |
|
Thoughts about this PR as an alternative solution? I only tested on mac terminal and mac vscode terminal, but for me option+up gets encoded as ctrl+up so just making those keybindings accept either option+ or ctrl+ seems to work well (but maybe this wouldn't work for other terminals? Not sure if that alt/option -> ctrl remapping is just on mac) |
|
@joshka-oai pointed out we should probably be more explicit about keybindings on different terminals, as this PR accomplishes Although personally I would prefer the mac / mac-vscode keybindings to still be option+backspace / option+delete / option+up (which are seemingly mapped to ctrl+backspace / ctrl+delete / ctrl+up for some reason?) if reasonable |
@charley-oai I totally agree with your opinion here. I evaluated Ctrl+Up as the first option. The only reason I gave up was that on the bundled macOS Terminal.app, it still is recognized as only Up by crossterm:
Vs. Ghostty for instance and others where it is properly recognized.
That's the only reason I went with Shift+Left -- it was recognized on the three places where we needed this contingency. |


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.
Non-goals
Tradeoffs
Exhaustive match instead of a wildcard default. The
queued_message_edit_binding_for_terminalfunction explicitly lists everyTerminalNamevariant. 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 onQueuedUserMessages. The key event handler that actually acts on the press is inChatWidget, but the rendered hint text is insideQueuedUserMessages. These are kept in sync byChatWidgetcallingbottom_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
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 --> FNNo 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
alt_up_edits_most_recent_queued_messagetest is preserved and explicitly sets the Alt+Up binding to isolate from the host terminal.Fixes #4490