github/codex
4
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-20 00:18:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

Keep Codex visible during external editor handoff (#48211)

Browse Source 
## 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
This commit is contained in:
Eric Traut
2026-09-25 20:46:05 +00:00
committed by copyberry
parent f5ffa46959
commit d5355e95ef
11 changed files with 278 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/tui/src/app/input.rs
+1 -1
View File
@@ -261,7 +261,7 @@ impl App {
let config = self.chat_widget.config_ref();
let file_system_policy = config.permissions.file_system_sandbox_policy();
let editor_result = tui
.with_restored(|| async {
.with_restored(tui::TerminalHandoff::KeepScreen, || async {
external_editor::run_editor(
&seed,
&editor_cmd,
codex-rs/tui/src/custom_terminal.rs
+1 -1
View File
@@ -272,7 +272,7 @@ where
}
/// Gets the previous buffer as a reference.
fn previous_buffer(&self) -> &Buffer {
pub(crate) fn previous_buffer(&self) -> &Buffer {
&self.buffers[1 - self.current]
}
codex-rs/tui/src/daemon_recovery.rs
+1 -1
View File
@@ -94,7 +94,7 @@ pub(super) async fn check(
issue.reason
))),
(Some(1), Some(features)) if managed_daemon => {
tui.with_restored(|| async {
tui.with_restored(crate::tui::TerminalHandoff::Restore, || async {
crossterm::terminal::disable_raw_mode()?;
codex_app_server_daemon::restart_with_features(features)
.await
codex-rs/tui/src/startup_orchestration.rs
+4 -4
View File
@@ -388,7 +388,7 @@ pub(super) async fn run_main_inner(
startup_draft.flush_pending_events().await?;
startup_draft
.tui_mut()
.with_restored(|| {
.with_restored(crate::tui::TerminalHandoff::Restore, || {
oss_selection::select_oss_provider(lmstudio_status, ollama_status)
})
.await?
@@ -526,7 +526,7 @@ pub(super) async fn run_main_inner(
startup_draft.flush_pending_events().await?;
let output = startup_draft
.tui_mut()
.with_restored(|| async {
.with_restored(crate::tui::TerminalHandoff::Restore, || async {
// Package installation may print progress; keep ordinary Ctrl+C handling.
crossterm::terminal::disable_raw_mode()?;
let result = codex_app_server_daemon::start_with_features(&daemon_features).await;
@@ -602,7 +602,7 @@ pub(super) async fn run_main_inner(
startup_draft.flush_pending_events().await?;
startup_draft
.tui_mut()
.with_restored(|| async {
.with_restored(crate::tui::TerminalHandoff::Restore, || async {
#[allow(clippy::print_stderr)]
{
eprintln!("Could not create otel exporter: {e}");
@@ -815,7 +815,7 @@ pub(super) async fn run_main_inner(
startup_draft.flush_pending_events().await?;
startup_draft
.tui_mut()
.with_restored(|| async {
.with_restored(crate::tui::TerminalHandoff::Restore, || async {
// Provider setup may print progress or block in an external downloader.
// Restore ordinary signal handling so Ctrl+C can interrupt that process.
crossterm::terminal::disable_raw_mode()?;
codex-rs/tui/src/tui.rs
+78 -13
View File
@@ -315,13 +315,30 @@ enum KeyboardRestore {
ResetAfterExit,
}
#[derive(Clone, Copy)]
pub(crate) enum TerminalHandoff {
Restore,
KeepScreen,
}
fn restore_common(
raw_mode_restore: RawModeRestore,
keyboard_restore: KeyboardRestore,
handoff: TerminalHandoff,
) -> Result<()> {
let mut first_error = ensure_virtual_terminal_processing().err();
if let Err(err) = ALTERNATE_SCREEN.restore(&mut stdout(), keyboard_restore) {
let screen_result = match handoff {
TerminalHandoff::KeepScreen if ALTERNATE_SCREEN.is_active() => {
let result = ALTERNATE_SCREEN.release_input(&mut stdout());
let _ = execute!(stdout(), keyboard_modes::DisableModifyOtherKeys);
result
}
TerminalHandoff::Restore | TerminalHandoff::KeepScreen => {
ALTERNATE_SCREEN.restore(&mut stdout(), keyboard_restore)
}
};
if let Err(err) = screen_result {
first_error.get_or_insert(err);
}
@@ -355,7 +372,11 @@ fn restore_common(
/// Inverse of `set_modes`.
#[cfg(unix)]
pub fn restore() -> Result<()> {
restore_common(RawModeRestore::Disable, KeyboardRestore::PopStack)
restore_common(
RawModeRestore::Disable,
KeyboardRestore::PopStack,
TerminalHandoff::Restore,
)
}
/// 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<()> {
/// Uses a stronger keyboard reset than `restore` so the parent shell recovers even if a
/// terminal missed the stack pop that normally pairs with [`set_modes`].
pub fn restore_after_exit() -> Result<()> {
let mut first_error =
restore_common(RawModeRestore::Disable, KeyboardRestore::ResetAfterExit).err();
let mut first_error = restore_common(
RawModeRestore::Disable,
KeyboardRestore::ResetAfterExit,
TerminalHandoff::Restore,
)
.err();
if let Err(err) = terminal_stderr::finish() {
first_error.get_or_insert(err);
}
@@ -388,8 +413,8 @@ pub fn restore_after_exit() -> Result<()> {
}
/// Restore the terminal to its original state, but keep raw mode enabled.
pub fn restore_keep_raw() -> Result<()> {
restore_common(RawModeRestore::Keep, KeyboardRestore::PopStack)
fn restore_keep_raw(handoff: TerminalHandoff) -> Result<()> {
restore_common(RawModeRestore::Keep, KeyboardRestore::PopStack, handoff)
}
/// Flush the underlying stdin buffer to clear any input that may be buffered at the terminal level.
@@ -887,7 +912,7 @@ impl Tui {
/// This pauses crossterm's stdin polling by dropping the underlying event stream, restores
/// terminal modes and stderr while keeping raw mode enabled, then re-applies Codex TUI modes
/// and stderr suppression before resuming events.
pub async fn with_restored<R, F, Fut>(&mut self, f: F) -> R
pub(crate) async fn with_restored<R, F, Fut>(&mut self, handoff: TerminalHandoff, f: F) -> R
where
F: FnOnce() -> Fut,
Fut: Future<Output = R>,
@@ -895,13 +920,47 @@ impl Tui {
// Pause crossterm events to avoid stdin conflicts with external program `f`.
self.pause_events();
// Leave alt screen if active to avoid conflicts with external program `f`.
// Restore both screens' input modes before giving the terminal to an editor. Repaint the
// last frame on the alternate screen so separate-window editors leave Codex visible.
let was_alt_screen = self.is_alt_screen_active();
if was_alt_screen {
let _ = self.leave_alt_screen_for_handoff();
}
if let Err(err) = restore_keep_raw() {
let visible_frame = (was_alt_screen && matches!(handoff, TerminalHandoff::KeepScreen))
.then(|| self.terminal.previous_buffer().clone());
let restore_result = if let Some(visible_frame) = visible_frame {
stdout()
.sync_update(|_| {
let leave_result = self.leave_alt_screen_for_handoff();
let main_result = restore_keep_raw(TerminalHandoff::Restore);
let enter_result = self.enter_alt_screen();
if !self.is_alt_screen_active() {
return leave_result.and(main_result).and(enter_result);
}
let draw_result = if enter_result.is_ok() {
self.terminal.draw(|frame| {
let area = frame.area().intersection(visible_frame.area);
for y in area.top()..area.bottom() {
for x in area.left()..area.right() {
frame.buffer_mut()[(x, y)] = visible_frame[(x, y)].clone();
}
}
})
} else {
Ok(())
};
let input_result = restore_keep_raw(TerminalHandoff::KeepScreen);
leave_result
.and(main_result)
.and(enter_result)
.and(draw_result)
.and(input_result)
})
.and_then(std::convert::identity)
} else {
if was_alt_screen {
let _ = self.leave_alt_screen_for_handoff();
}
restore_keep_raw(TerminalHandoff::Restore)
};
if let Err(err) = restore_result {
tracing::warn!("failed to restore terminal modes before external program: {err}");
}
if let Err(err) = terminal_stderr::pause() {
@@ -910,6 +969,12 @@ impl Tui {
let output = f().await;
if was_alt_screen && matches!(handoff, TerminalHandoff::KeepScreen) {
// The editor may already have left the alternate screen. Return to the main screen
// before reapplying its keyboard mode and entering the alternate screen again.
let _ = self.leave_alt_screen_for_handoff();
}
if let Err(err) = terminal_stderr::resume() {
tracing::warn!("failed to suppress terminal stderr after external program: {err}");
}
codex-rs/tui/src/tui/alternate_screen.rs
+27 -13
View File
@@ -1,9 +1,9 @@
//! Pair alternate-screen transitions with that screen's independent keyboard-mode stack.
//!
//! Push only on entry and pop before leaving; the main screen keeps its own TUI mode until
//! terminal handoff. Transcript surfaces retain pointer reporting across overlays and disable it
//! before yielding to the shell. Promoting an overlay must not push another keyboard frame.
//! Refresh tmux's input policy on entry and apply it to every mouse-capture request.
//! Push only on entry and pop before leaving or yielding input to an editor. The main screen
//! keeps its own TUI mode until the handoff returns to it. Transcript surfaces retain pointer
//! reporting across overlays and disable it before yielding. Promoting an overlay must not push
//! another keyboard frame. Refresh tmux's input policy on entry for mouse-capture requests.
use std::io::Result;
use std::io::Write;
@@ -32,6 +32,7 @@ pub(super) static ALTERNATE_SCREEN: AlternateScreen = AlternateScreen {
mouse_active: AtomicBool::new(/*v*/ false),
mouse_capture_disabled: AtomicBool::new(/*v*/ false),
input_configured: AtomicBool::new(/*v*/ false),
keyboard_active: AtomicBool::new(/*v*/ false),
};
#[derive(Default)]
@@ -42,6 +43,8 @@ pub(super) struct AlternateScreen {
mouse_capture_disabled: AtomicBool,
// A cleanup/setup error must not make the next identical request look already applied.
input_configured: AtomicBool,
// An editor can take over the screen after Codex has popped its keyboard mode.
keyboard_active: AtomicBool,
}
/// Report pointer motion so the owned transcript can update its return-to-bottom hover state.
@@ -74,6 +77,10 @@ impl AlternateScreen {
self.active.store(/*val*/ true, Ordering::Relaxed);
writer.flush()?;
let mouse_capture = keyboard_modes::enable_keyboard_enhancement(writer);
self.keyboard_active.store(
!cfg!(windows) && !keyboard_modes::keyboard_enhancement_disabled(),
Ordering::Relaxed,
);
self.mouse_capture_disabled.store(
mouse_capture == MouseCapture::DisabledByTmux,
Ordering::Relaxed,
@@ -127,29 +134,36 @@ impl AlternateScreen {
Ok(())
}
pub(super) fn leave(&self, writer: &mut impl Write) -> Result<()> {
/// Release input modes without changing the screen, so an editor can take it over.
pub(super) fn release_input(&self, writer: &mut impl Write) -> Result<()> {
self.input_configured
.store(/*val*/ false, Ordering::Relaxed);
let mouse_result = self.disable_mouse(writer);
// Crossterm never pushes a keyboard stack on native Windows: its input-record API
// already reports enhanced keys, and its push/pop commands return Unsupported.
let keyboard_result = if cfg!(windows) || keyboard_modes::keyboard_enhancement_disabled() {
Ok(())
} else {
let keyboard_result = if self.keyboard_active.load(Ordering::Relaxed) {
// modifyOtherKeys is not stacked per screen; keep the main screen's fallback enabled
// until terminal handoff restores its keyboard modes too.
execute!(writer, PopKeyboardEnhancementFlags)
let result = execute!(writer, PopKeyboardEnhancementFlags);
if result.is_ok() {
self.keyboard_active.store(/*val*/ false, Ordering::Relaxed);
}
result
} else {
Ok(())
};
let scroll_result = execute!(writer, DisableAlternateScroll);
mouse_result.and(keyboard_result).and(scroll_result)
}
pub(super) fn leave(&self, writer: &mut impl Write) -> Result<()> {
let input_result = self.release_input(writer);
// A failed earlier cleanup write must not prevent the actual screen transition.
let screen_result = execute!(writer, LeaveAlternateScreen);
if screen_result.is_ok() {
self.active.store(/*val*/ false, Ordering::Relaxed);
}
mouse_result
.and(keyboard_result)
.and(scroll_result)
.and(screen_result)
input_result.and(screen_result)
}
pub(super) fn restore(
codex-rs/tui/src/tui/alternate_screen_tests.rs
+39
View File
@@ -92,6 +92,45 @@ impl KeyboardScreens {
}
}
#[test]
fn editor_handoff_balances_keyboard_stacks_if_editor_leaves_the_screen() {
if keyboard_modes::keyboard_enhancement_disabled() {
return;
}
for editor_leaves in [false, true] {
let screen = AlternateScreen::default();
let mut output = Vec::new();
let mut keyboard = KeyboardScreens::default();
execute!(
output,
PushKeyboardEnhancementFlags(KeyboardEnhancementFlags::DISAMBIGUATE_ESCAPE_CODES)
)
.unwrap();
keyboard_modes::enable_keyboard_enhancement(&mut output);
screen.enter(&mut output, /*capture_mouse*/ true).unwrap();
screen.leave(&mut output).unwrap();
screen
.restore(&mut output, KeyboardRestore::PopStack)
.unwrap();
screen.enter(&mut output, /*capture_mouse*/ true).unwrap();
screen.release_input(&mut output).unwrap();
keyboard.process(&std::mem::take(&mut output));
assert_eq!(keyboard.state(), (true, &[1][..], &[][..]));
if editor_leaves {
execute!(output, LeaveAlternateScreen).unwrap();
}
screen.leave(&mut output).unwrap();
keyboard_modes::enable_keyboard_enhancement(&mut output);
screen.enter(&mut output, /*capture_mouse*/ true).unwrap();
screen
.restore(&mut output, KeyboardRestore::PopStack)
.unwrap();
keyboard.process(&output);
assert_eq!(keyboard.state(), (false, &[1][..], &[][..]));
}
}
// Capture-specific tests must not inherit the tmux session running the test binary.
fn enter_with_mouse_enabled(
screen: &AlternateScreen,
codex-rs/tui/src/tui/keyboard_modes.rs
+1 -1
View File
@@ -337,7 +337,7 @@ impl Command for EnableModifyOtherKeys {
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
struct DisableModifyOtherKeys;
pub(super) struct DisableModifyOtherKeys;
impl Command for DisableModifyOtherKeys {
fn write_ansi(&self, f: &mut impl fmt::Write) -> fmt::Result {
codex-rs/tui/tests/suite/external_editor_tests.rs
+105
View File
@@ -0,0 +1,105 @@
//! Exercise editor handoff through a terminal, including editors that switch screens.
use super::PtyCodex;
use super::write_test_config;
use anyhow::Result;
use anyhow::ensure;
use std::fs;
use std::os::unix::fs::PermissionsExt;
use std::time::Duration;
use std::time::Instant;
#[test]
fn external_editors_keep_the_screen_and_return_the_draft() -> Result<()> {
let repo_root = codex_utils_cargo_bin::repo_root()?;
for (editor_output, fullscreen) in [
("", true),
("printf '\\033[?1049h\\033[2J\\033[Hterminal editor'", true),
("printf '\\033[2J\\033[Hterminal editor'", true),
("printf '\\033[?1049l\\033[2J\\033[Hterminal editor'", true),
("", false),
] {
let home = tempfile::tempdir()?;
write_test_config(home.path(), &repo_root)?;
let editor = home.path().join("editor.sh");
fs::write(
&editor,
format!(
"#!/bin/sh\n{editor_output}\n: > \"$0.ready\"\nIFS= read -r ignored\n\
{exit_screen}\nprintf 'edited draft' > \"$1\"\n",
exit_screen = if editor_output.contains("1049h") {
"printf '\\033[?1049l'"
} else {
""
},
),
)?;
fs::set_permissions(&editor, fs::Permissions::from_mode(0o755))?;
let args = if fullscreen {
vec!["-c", "sandbox_mode=\"read-only\""]
} else {
vec![
"-c",
"sandbox_mode=\"read-only\"",
"-c",
"tui.fullscreen_transcript=false",
]
};
let codex = codex_utils_cargo_bin::cargo_bin("codex-tui")
.or_else(|_| codex_utils_cargo_bin::cargo_bin("codex"))?;
let mut terminal = PtyCodex::start_binary(&codex, &repo_root, home, &args, Some(&editor))?;
terminal.wait_for_startup()?;
terminal.wait_for_screen("GPT-5.6-Terra")?;
terminal.write_input(b"initial draft")?;
terminal.wait_for_screen("initial draft")?;
terminal.write_input(b"\x07")?;
let deadline = Instant::now() + Duration::from_secs(/*secs*/ 10);
while !editor.with_extension("sh.ready").exists() && Instant::now() < deadline {
terminal.read_output(Duration::from_millis(/*millis*/ 50))?;
terminal.ensure_running()?;
}
ensure!(
editor.with_extension("sh.ready").exists(),
"editor did not start"
);
let mut visible = None;
if editor_output.is_empty() {
terminal.wait_for_screen("Save and close external editor to continue.")?;
ensure!(terminal.screen_contains("initial draft"));
if fullscreen {
ensure!(terminal.screen_contains("OpenAI Codex"));
ensure!(terminal.screen_contains("GPT-5.6-Terra"));
}
ensure!(
terminal.parser.screen().mouse_protocol_mode() == vt100::MouseProtocolMode::None
);
if fullscreen {
visible = Some(
terminal
.screen_contents()
.lines()
.filter(|line| {
line.contains("initial draft")
|| line.contains("Save and close external editor to continue.")
})
.collect::<Vec<_>>()
.join("\n"),
);
}
} else {
terminal.wait_for_screen("terminal editor")?;
}
terminal.write_input(b"\n")?;
terminal.wait_for_screen("edited draft")?;
ensure!(terminal.parser.screen().alternate_screen() == fullscreen);
terminal.write_input(b"!")?;
terminal.wait_for_screen("edited draft!")?;
if let Some(visible) = visible {
insta::assert_snapshot!("external_editor_visible_handoff", visible);
}
}
Ok(())
}
codex-rs/tui/tests/suite/focus_palette.rs
+14 -3
View File
@@ -22,6 +22,8 @@ const STARTUP_TIMEOUT: Duration = Duration::from_secs(/*secs*/ 30);
const FOCUS_INPUT_TIMEOUT: Duration = Duration::from_secs(/*secs*/ 5);
const FOCUS_PROBE_INPUT: &str = "focus-palette-24527";
#[path = "external_editor_tests.rs"]
mod external_editor;
#[path = "tui_mode_picker_tests.rs"]
mod tui_mode_picker;
@@ -291,7 +293,9 @@ impl PtyCodex {
) -> Result<Self> {
let codex = codex_utils_cargo_bin::cargo_bin("codex-tui")
.or_else(|_| codex_utils_cargo_bin::cargo_bin("codex"))?;
Self::start_binary(&codex, repo_root, codex_home, extra_args)
Self::start_binary(
&codex, repo_root, codex_home, extra_args, /*editor*/ None,
)
}
/// Include the CLI dispatch futures when testing production stack headroom.
@@ -302,7 +306,9 @@ impl PtyCodex {
) -> Result<Self> {
let codex = codex_utils_cargo_bin::cargo_bin("codex")
.context("build codex-cli and set CARGO_BIN_EXE_codex to its executable")?;
Self::start_binary(&codex, repo_root, codex_home, extra_args)
Self::start_binary(
&codex, repo_root, codex_home, extra_args, /*editor*/ None,
)
}
fn start_binary(
@@ -310,6 +316,7 @@ impl PtyCodex {
repo_root: &Path,
codex_home: TempDir,
extra_args: &[&str],
editor: Option<&Path>,
) -> Result<Self> {
let mut master_fd = -1;
let mut slave_fd = -1;
@@ -342,7 +349,11 @@ impl PtyCodex {
let stdin = slave.try_clone().context("clone pseudo-terminal stdin")?;
let stdout = slave.try_clone().context("clone pseudo-terminal stdout")?;
let child = Command::new(codex)
let mut command = Command::new(codex);
if let Some(editor) = editor {
command.env("VISUAL", editor);
}
let child = command
.args(extra_args)
.arg("-C")
.arg(repo_root)
codex-rs/tui/tests/suite/snapshots/all__suite__focus_palette__external_editor__external_editor_visible_handoff.snap
+7
View File
@@ -0,0 +1,7 @@
---
source: tui/tests/suite/external_editor_tests.rs
assertion_line: 89
expression: visible
---
› initial draft
Save and close external editor to continue.