Skip to content

Commit db31b25

Browse files
etraut-openaicopyberry
etraut-openai
authored and
copyberry
committed
Keep Codex visible during external editor handoff
## Why Opening a separate-window editor from the fullscreen TUI leaves the alternate screen, hiding the draft and the instruction to save and close the editor. ## What changed Preserve and repaint the last Codex frame when handing the terminal to an external editor, while releasing keyboard and mouse input modes. Track whether the alternate screen's keyboard mode is active so cleanup does not pop it twice if the editor switches screens. Restore TUI modes and the alternate screen when the editor returns. ## Testing Add terminal integration coverage for separate-window editors, terminal editors that switch or reuse screens, and inline mode. Verify the visible draft and handoff hint, disabled mouse reporting, edited draft recovery, and continued typing. Add a unit test for balanced keyboard stacks whether or not the editor leaves the alternate screen. GitOrigin-RevId: c2ff1046a933f5f9e3168663e2c326999845c578
1 parent f5ffa46 commit db31b25

11 files changed

Lines changed: 278 additions & 37 deletions

‎codex-rs/tui/src/app/input.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ impl App {
261261
let config = self.chat_widget.config_ref();
262262
let file_system_policy = config.permissions.file_system_sandbox_policy();
263263
let editor_result = tui
264-
.with_restored(|| async {
264+
.with_restored(tui::TerminalHandoff::KeepScreen, || async {
265265
external_editor::run_editor(
266266
&seed,
267267
&editor_cmd,

‎codex-rs/tui/src/custom_terminal.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ where
272272
}
273273

274274
/// Gets the previous buffer as a reference.
275-
fn previous_buffer(&self) -> &Buffer {
275+
pub(crate) fn previous_buffer(&self) -> &Buffer {
276276
&self.buffers[1 - self.current]
277277
}
278278

‎codex-rs/tui/src/daemon_recovery.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ pub(super) async fn check(
9494
issue.reason
9595
))),
9696
(Some(1), Some(features)) if managed_daemon => {
97-
tui.with_restored(|| async {
97+
tui.with_restored(crate::tui::TerminalHandoff::Restore, || async {
9898
crossterm::terminal::disable_raw_mode()?;
9999
codex_app_server_daemon::restart_with_features(features)
100100
.await

‎codex-rs/tui/src/startup_orchestration.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,7 @@ pub(super) async fn run_main_inner(
388388
startup_draft.flush_pending_events().await?;
389389
startup_draft
390390
.tui_mut()
391-
.with_restored(|| {
391+
.with_restored(crate::tui::TerminalHandoff::Restore, || {
392392
oss_selection::select_oss_provider(lmstudio_status, ollama_status)
393393
})
394394
.await?
@@ -526,7 +526,7 @@ pub(super) async fn run_main_inner(
526526
startup_draft.flush_pending_events().await?;
527527
let output = startup_draft
528528
.tui_mut()
529-
.with_restored(|| async {
529+
.with_restored(crate::tui::TerminalHandoff::Restore, || async {
530530
// Package installation may print progress; keep ordinary Ctrl+C handling.
531531
crossterm::terminal::disable_raw_mode()?;
532532
let result = codex_app_server_daemon::start_with_features(&daemon_features).await;
@@ -602,7 +602,7 @@ pub(super) async fn run_main_inner(
602602
startup_draft.flush_pending_events().await?;
603603
startup_draft
604604
.tui_mut()
605-
.with_restored(|| async {
605+
.with_restored(crate::tui::TerminalHandoff::Restore, || async {
606606
#[allow(clippy::print_stderr)]
607607
{
608608
eprintln!("Could not create otel exporter: {e}");
@@ -815,7 +815,7 @@ pub(super) async fn run_main_inner(
815815
startup_draft.flush_pending_events().await?;
816816
startup_draft
817817
.tui_mut()
818-
.with_restored(|| async {
818+
.with_restored(crate::tui::TerminalHandoff::Restore, || async {
819819
// Provider setup may print progress or block in an external downloader.
820820
// Restore ordinary signal handling so Ctrl+C can interrupt that process.
821821
crossterm::terminal::disable_raw_mode()?;

‎codex-rs/tui/src/tui.rs‎

Lines changed: 78 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -315,13 +315,30 @@ enum KeyboardRestore {
315315
ResetAfterExit,
316316
}
317317

318+
#[derive(Clone, Copy)]
319+
pub(crate) enum TerminalHandoff {
320+
Restore,
321+
KeepScreen,
322+
}
323+
318324
fn restore_common(
319325
raw_mode_restore: RawModeRestore,
320326
keyboard_restore: KeyboardRestore,
327+
handoff: TerminalHandoff,
321328
) -> Result<()> {
322329
let mut first_error = ensure_virtual_terminal_processing().err();
323330

324-
if let Err(err) = ALTERNATE_SCREEN.restore(&mut stdout(), keyboard_restore) {
331+
let screen_result = match handoff {
332+
TerminalHandoff::KeepScreen if ALTERNATE_SCREEN.is_active() => {
333+
let result = ALTERNATE_SCREEN.release_input(&mut stdout());
334+
let _ = execute!(stdout(), keyboard_modes::DisableModifyOtherKeys);
335+
result
336+
}
337+
TerminalHandoff::Restore | TerminalHandoff::KeepScreen => {
338+
ALTERNATE_SCREEN.restore(&mut stdout(), keyboard_restore)
339+
}
340+
};
341+
if let Err(err) = screen_result {
325342
first_error.get_or_insert(err);
326343
}
327344

@@ -355,7 +372,11 @@ fn restore_common(
355372
/// Inverse of `set_modes`.
356373
#[cfg(unix)]
357374
pub fn restore() -> Result<()> {
358-
restore_common(RawModeRestore::Disable, KeyboardRestore::PopStack)
375+
restore_common(
376+
RawModeRestore::Disable,
377+
KeyboardRestore::PopStack,
378+
TerminalHandoff::Restore,
379+
)
359380
}
360381

361382
/// Force crossterm's cached raw-mode state back in sync with the terminal after `fg`.
@@ -375,8 +396,12 @@ pub(super) fn reapply_raw_mode_after_resume() -> Result<()> {
375396
/// Uses a stronger keyboard reset than `restore` so the parent shell recovers even if a
376397
/// terminal missed the stack pop that normally pairs with [`set_modes`].
377398
pub fn restore_after_exit() -> Result<()> {
378-
let mut first_error =
379-
restore_common(RawModeRestore::Disable, KeyboardRestore::ResetAfterExit).err();
399+
let mut first_error = restore_common(
400+
RawModeRestore::Disable,
401+
KeyboardRestore::ResetAfterExit,
402+
TerminalHandoff::Restore,
403+
)
404+
.err();
380405
if let Err(err) = terminal_stderr::finish() {
381406
first_error.get_or_insert(err);
382407
}
@@ -388,8 +413,8 @@ pub fn restore_after_exit() -> Result<()> {
388413
}
389414

390415
/// Restore the terminal to its original state, but keep raw mode enabled.
391-
pub fn restore_keep_raw() -> Result<()> {
392-
restore_common(RawModeRestore::Keep, KeyboardRestore::PopStack)
416+
fn restore_keep_raw(handoff: TerminalHandoff) -> Result<()> {
417+
restore_common(RawModeRestore::Keep, KeyboardRestore::PopStack, handoff)
393418
}
394419

395420
/// Flush the underlying stdin buffer to clear any input that may be buffered at the terminal level.
@@ -887,21 +912,55 @@ impl Tui {
887912
/// This pauses crossterm's stdin polling by dropping the underlying event stream, restores
888913
/// terminal modes and stderr while keeping raw mode enabled, then re-applies Codex TUI modes
889914
/// and stderr suppression before resuming events.
890-
pub async fn with_restored<R, F, Fut>(&mut self, f: F) -> R
915+
pub(crate) async fn with_restored<R, F, Fut>(&mut self, handoff: TerminalHandoff, f: F) -> R
891916
where
892917
F: FnOnce() -> Fut,
893918
Fut: Future<Output = R>,
894919
{
895920
// Pause crossterm events to avoid stdin conflicts with external program `f`.
896921
self.pause_events();
897922

898-
// Leave alt screen if active to avoid conflicts with external program `f`.
923+
// Restore both screens' input modes before giving the terminal to an editor. Repaint the
924+
// last frame on the alternate screen so separate-window editors leave Codex visible.
899925
let was_alt_screen = self.is_alt_screen_active();
900-
if was_alt_screen {
901-
let _ = self.leave_alt_screen_for_handoff();
902-
}
903-
904-
if let Err(err) = restore_keep_raw() {
926+
let visible_frame = (was_alt_screen && matches!(handoff, TerminalHandoff::KeepScreen))
927+
.then(|| self.terminal.previous_buffer().clone());
928+
let restore_result = if let Some(visible_frame) = visible_frame {
929+
stdout()
930+
.sync_update(|_| {
931+
let leave_result = self.leave_alt_screen_for_handoff();
932+
let main_result = restore_keep_raw(TerminalHandoff::Restore);
933+
let enter_result = self.enter_alt_screen();
934+
if !self.is_alt_screen_active() {
935+
return leave_result.and(main_result).and(enter_result);
936+
}
937+
let draw_result = if enter_result.is_ok() {
938+
self.terminal.draw(|frame| {
939+
let area = frame.area().intersection(visible_frame.area);
940+
for y in area.top()..area.bottom() {
941+
for x in area.left()..area.right() {
942+
frame.buffer_mut()[(x, y)] = visible_frame[(x, y)].clone();
943+
}
944+
}
945+
})
946+
} else {
947+
Ok(())
948+
};
949+
let input_result = restore_keep_raw(TerminalHandoff::KeepScreen);
950+
leave_result
951+
.and(main_result)
952+
.and(enter_result)
953+
.and(draw_result)
954+
.and(input_result)
955+
})
956+
.and_then(std::convert::identity)
957+
} else {
958+
if was_alt_screen {
959+
let _ = self.leave_alt_screen_for_handoff();
960+
}
961+
restore_keep_raw(TerminalHandoff::Restore)
962+
};
963+
if let Err(err) = restore_result {
905964
tracing::warn!("failed to restore terminal modes before external program: {err}");
906965
}
907966
if let Err(err) = terminal_stderr::pause() {
@@ -910,6 +969,12 @@ impl Tui {
910969

911970
let output = f().await;
912971

972+
if was_alt_screen && matches!(handoff, TerminalHandoff::KeepScreen) {
973+
// The editor may already have left the alternate screen. Return to the main screen
974+
// before reapplying its keyboard mode and entering the alternate screen again.
975+
let _ = self.leave_alt_screen_for_handoff();
976+
}
977+
913978
if let Err(err) = terminal_stderr::resume() {
914979
tracing::warn!("failed to suppress terminal stderr after external program: {err}");
915980
}

‎codex-rs/tui/src/tui/alternate_screen.rs‎

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
//! Pair alternate-screen transitions with that screen's independent keyboard-mode stack.
22
//!
3-
//! Push only on entry and pop before leaving; the main screen keeps its own TUI mode until
4-
//! terminal handoff. Transcript surfaces retain pointer reporting across overlays and disable it
5-
//! before yielding to the shell. Promoting an overlay must not push another keyboard frame.
6-
//! Refresh tmux's input policy on entry and apply it to every mouse-capture request.
3+
//! Push only on entry and pop before leaving or yielding input to an editor. The main screen
4+
//! keeps its own TUI mode until the handoff returns to it. Transcript surfaces retain pointer
5+
//! reporting across overlays and disable it before yielding. Promoting an overlay must not push
6+
//! another keyboard frame. Refresh tmux's input policy on entry for mouse-capture requests.
77
88
use std::io::Result;
99
use std::io::Write;
@@ -32,6 +32,7 @@ pub(super) static ALTERNATE_SCREEN: AlternateScreen = AlternateScreen {
3232
mouse_active: AtomicBool::new(/*v*/ false),
3333
mouse_capture_disabled: AtomicBool::new(/*v*/ false),
3434
input_configured: AtomicBool::new(/*v*/ false),
35+
keyboard_active: AtomicBool::new(/*v*/ false),
3536
};
3637

3738
#[derive(Default)]
@@ -42,6 +43,8 @@ pub(super) struct AlternateScreen {
4243
mouse_capture_disabled: AtomicBool,
4344
// A cleanup/setup error must not make the next identical request look already applied.
4445
input_configured: AtomicBool,
46+
// An editor can take over the screen after Codex has popped its keyboard mode.
47+
keyboard_active: AtomicBool,
4548
}
4649

4750
/// Report pointer motion so the owned transcript can update its return-to-bottom hover state.
@@ -74,6 +77,10 @@ impl AlternateScreen {
7477
self.active.store(/*val*/ true, Ordering::Relaxed);
7578
writer.flush()?;
7679
let mouse_capture = keyboard_modes::enable_keyboard_enhancement(writer);
80+
self.keyboard_active.store(
81+
!cfg!(windows) && !keyboard_modes::keyboard_enhancement_disabled(),
82+
Ordering::Relaxed,
83+
);
7784
self.mouse_capture_disabled.store(
7885
mouse_capture == MouseCapture::DisabledByTmux,
7986
Ordering::Relaxed,
@@ -127,29 +134,36 @@ impl AlternateScreen {
127134
Ok(())
128135
}
129136

130-
pub(super) fn leave(&self, writer: &mut impl Write) -> Result<()> {
137+
/// Release input modes without changing the screen, so an editor can take it over.
138+
pub(super) fn release_input(&self, writer: &mut impl Write) -> Result<()> {
131139
self.input_configured
132140
.store(/*val*/ false, Ordering::Relaxed);
133141
let mouse_result = self.disable_mouse(writer);
134142
// Crossterm never pushes a keyboard stack on native Windows: its input-record API
135143
// already reports enhanced keys, and its push/pop commands return Unsupported.
136-
let keyboard_result = if cfg!(windows) || keyboard_modes::keyboard_enhancement_disabled() {
137-
Ok(())
138-
} else {
144+
let keyboard_result = if self.keyboard_active.load(Ordering::Relaxed) {
139145
// modifyOtherKeys is not stacked per screen; keep the main screen's fallback enabled
140146
// until terminal handoff restores its keyboard modes too.
141-
execute!(writer, PopKeyboardEnhancementFlags)
147+
let result = execute!(writer, PopKeyboardEnhancementFlags);
148+
if result.is_ok() {
149+
self.keyboard_active.store(/*val*/ false, Ordering::Relaxed);
150+
}
151+
result
152+
} else {
153+
Ok(())
142154
};
143155
let scroll_result = execute!(writer, DisableAlternateScroll);
156+
mouse_result.and(keyboard_result).and(scroll_result)
157+
}
158+
159+
pub(super) fn leave(&self, writer: &mut impl Write) -> Result<()> {
160+
let input_result = self.release_input(writer);
144161
// A failed earlier cleanup write must not prevent the actual screen transition.
145162
let screen_result = execute!(writer, LeaveAlternateScreen);
146163
if screen_result.is_ok() {
147164
self.active.store(/*val*/ false, Ordering::Relaxed);
148165
}
149-
mouse_result
150-
.and(keyboard_result)
151-
.and(scroll_result)
152-
.and(screen_result)
166+
input_result.and(screen_result)
153167
}
154168

155169
pub(super) fn restore(

‎codex-rs/tui/src/tui/alternate_screen_tests.rs‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,45 @@ impl KeyboardScreens {
9292
}
9393
}
9494

95+
#[test]
96+
fn editor_handoff_balances_keyboard_stacks_if_editor_leaves_the_screen() {
97+
if keyboard_modes::keyboard_enhancement_disabled() {
98+
return;
99+
}
100+
for editor_leaves in [false, true] {
101+
let screen = AlternateScreen::default();
102+
let mut output = Vec::new();
103+
let mut keyboard = KeyboardScreens::default();
104+
execute!(
105+
output,
106+
PushKeyboardEnhancementFlags(KeyboardEnhancementFlags::DISAMBIGUATE_ESCAPE_CODES)
107+
)
108+
.unwrap();
109+
keyboard_modes::enable_keyboard_enhancement(&mut output);
110+
screen.enter(&mut output, /*capture_mouse*/ true).unwrap();
111+
screen.leave(&mut output).unwrap();
112+
screen
113+
.restore(&mut output, KeyboardRestore::PopStack)
114+
.unwrap();
115+
screen.enter(&mut output, /*capture_mouse*/ true).unwrap();
116+
screen.release_input(&mut output).unwrap();
117+
keyboard.process(&std::mem::take(&mut output));
118+
assert_eq!(keyboard.state(), (true, &[1][..], &[][..]));
119+
120+
if editor_leaves {
121+
execute!(output, LeaveAlternateScreen).unwrap();
122+
}
123+
screen.leave(&mut output).unwrap();
124+
keyboard_modes::enable_keyboard_enhancement(&mut output);
125+
screen.enter(&mut output, /*capture_mouse*/ true).unwrap();
126+
screen
127+
.restore(&mut output, KeyboardRestore::PopStack)
128+
.unwrap();
129+
keyboard.process(&output);
130+
assert_eq!(keyboard.state(), (false, &[1][..], &[][..]));
131+
}
132+
}
133+
95134
// Capture-specific tests must not inherit the tmux session running the test binary.
96135
fn enter_with_mouse_enabled(
97136
screen: &AlternateScreen,

‎codex-rs/tui/src/tui/keyboard_modes.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ impl Command for EnableModifyOtherKeys {
337337
}
338338

339339
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
340-
struct DisableModifyOtherKeys;
340+
pub(super) struct DisableModifyOtherKeys;
341341

342342
impl Command for DisableModifyOtherKeys {
343343
fn write_ansi(&self, f: &mut impl fmt::Write) -> fmt::Result {

0 commit comments

Comments
 (0)