mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
714151eb4ead7f6e29ada12402efbb009e9bae76
2 Commits
-
tui: double-press Ctrl+C/Ctrl+D to quit (#8936)
## Problem Codex’s TUI quit behavior has historically been easy to trigger accidentally and hard to reason about. - `Ctrl+C`/`Ctrl+D` could terminate the UI immediately, which is a common key to press while trying to dismiss a modal, cancel a command, or recover from a stuck state. - “Quit” and “shutdown” were not consistently separated, so some exit paths could bypass the shutdown/cleanup work that should run before the process terminates. This PR makes quitting both safer (harder to do by accident) and more uniform across quit gestures, while keeping the shutdown-first semantics explicit. ## Mental model After this change, the system treats quitting as a UI request that is coordinated by the app layer. - The UI requests exit via `AppEvent::Exit(ExitMode)`. - `ExitMode::ShutdownFirst` is the normal user path: the app triggers `Op::Shutdown`, continues rendering while shutdown runs, and only ends the UI loop once shutdown has completed. - `ExitMode::Immediate` exists as an escape hatch (and as the post-shutdown “now actually exit” signal); it bypasses cleanup and should not be the default for user-triggered quits. User-facing quit gestures are intentionally “two-step” for safety: - `Ctrl+C` and `Ctrl+D` no longer exit immediately. - The first press arms a 1-second window and shows a footer hint (“ctrl + <key> again to quit”). - Pressing the same key again within the window requests a shutdown-first quit; otherwise the hint expires and the next press starts a fresh window. Key routing remains modal-first: - A modal/popup gets first chance to consume `Ctrl+C`. - If a modal handles `Ctrl+C`, any armed quit shortcut is cleared so dismissing a modal cannot prime a subsequent `Ctrl+C` to quit. - `Ctrl+D` only participates in quitting when the composer is empty and no modal/popup is active. The design doc `docs/exit-confirmation-prompt-design.md` captures the intended routing and the invariants the UI should maintain. ## Non-goals - This does not attempt to redesign modal UX or make modals uniformly dismissible via `Ctrl+C`. It only ensures modals get priority and that quit arming does not leak across modal handling. - This does not introduce a persistent confirmation prompt/menu for quitting; the goal is to keep the exit gesture lightweight and consistent. - This does not change the semantics of core shutdown itself; it changes how the UI requests and sequences it. ## Tradeoffs - Quitting via `Ctrl+C`/`Ctrl+D` now requires a deliberate second keypress, which adds friction for users who relied on the old “instant quit” behavior. - The UI now maintains a small time-bounded state machine for the armed shortcut, which increases complexity and introduces timing-dependent behavior. This design was chosen over alternatives (a modal confirmation prompt or a long-lived “are you sure” state) because it provides an explicit safety barrier while keeping the flow fast and keyboard-native. ## Architecture - `ChatWidget` owns the quit-shortcut state machine and decides when a quit gesture is allowed (idle vs cancellable work, composer state, etc.). - `BottomPane` owns rendering and local input routing for modals/popups. It is responsible for consuming cancellation keys when a view is active and for showing/expiring the footer hint. - `App` owns shutdown sequencing: translating `AppEvent::Exit(ShutdownFirst)` into `Op::Shutdown` and only terminating the UI loop when exit is safe. This keeps “what should happen” decisions (quit vs interrupt vs ignore) in the chat/widget layer, while keeping “how it looks and which view gets the key” in the bottom-pane layer. ## Observability You can tell this is working by running the TUIs and exercising the quit gestures: - While idle: pressing `Ctrl+C` (or `Ctrl+D` with an empty composer and no modal) shows a footer hint for ~1 second; pressing again within that window exits via shutdown-first. - While streaming/tools/review are active: `Ctrl+C` interrupts work rather than quitting. - With a modal/popup open: `Ctrl+C` dismisses/handles the modal (if it chooses to) and does not arm a quit shortcut; a subsequent quick `Ctrl+C` should not quit unless the user re-arms it. Failure modes are visible as: - Quits that happen immediately (no hint window) from `Ctrl+C`/`Ctrl+D`. - Quits that occur while a modal is open and consuming `Ctrl+C`. - UI termination before shutdown completes (cleanup skipped). ## Tests - Updated/added unit and snapshot coverage in `codex-tui` and `codex-tui2` to validate: - The quit hint appears and expires on the expected key. - Double-press within the window triggers a shutdown-first quit request. - Modal-first routing prevents quit bypass and clears any armed shortcut when a modal consumes `Ctrl+C`. These tests focus on the UI-level invariants and rendered output; they do not attempt to validate real terminal key-repeat timing or end-to-end process shutdown behavior. --- Screenshot: <img width="912" height="740" alt="Screenshot 2026-01-13 at 1 05 28 PM" src="https://github.com/user-attachments/assets/18f3d22e-2557-47f2-a369-ae7a9531f29f" />Josh McKinney ·
2026-01-14 17:42:52 +00:00 -
Sync tui2 with tui and keep dual-run glue (#7965)
- Copy latest tui sources into tui2 - Restore notifications, tests, and styles - Keep codex-tui interop conversions and snapshots The expected changes that are necessary to make this work are still in place: diff -ru codex-rs/tui codex-rs/tui2 --exclude='*.snap' --exclude='*.snap.new' ```diff diff -ru --ex codex-rs/tui/Cargo.toml codex-rs/tui2/Cargo.toml --- codex-rs/tui/Cargo.toml 2025-12-12 16:39:12 +++ codex-rs/tui2/Cargo.toml 2025-12-12 17:31:01 @@ -1,15 +1,15 @@ [package] -name = "codex-tui" +name = "codex-tui2" version.workspace = true edition.workspace = true license.workspace = true [[bin]] -name = "codex-tui" +name = "codex-tui2" path = "src/main.rs" [lib] -name = "codex_tui" +name = "codex_tui2" path = "src/lib.rs" [features] @@ -42,6 +42,7 @@ codex-login = { workspace = true } codex-protocol = { workspace = true } codex-utils-absolute-path = { workspace = true } +codex-tui = { workspace = true } color-eyre = { workspace = true } crossterm = { workspace = true, features = ["bracketed-paste", "event-stream"] } derive_more = { workspace = true, features = ["is_variant"] } diff -ru --ex codex-rs/tui/src/app.rs codex-rs/tui2/src/app.rs --- codex-rs/tui/src/app.rs 2025-12-12 16:39:05 +++ codex-rs/tui2/src/app.rs 2025-12-12 17:30:36 @@ -69,6 +69,16 @@ pub update_action: Option<UpdateAction>, } +impl From<AppExitInfo> for codex_tui::AppExitInfo { + fn from(info: AppExitInfo) -> Self { + codex_tui::AppExitInfo { + token_usage: info.token_usage, + conversation_id: info.conversation_id, + update_action: info.update_action.map(Into::into), + } + } +} + fn session_summary( token_usage: TokenUsage, conversation_id: Option<ConversationId>, Only in codex-rs/tui/src/bin: md-events.rs Only in codex-rs/tui2/src/bin: md-events2.rs diff -ru --ex codex-rs/tui/src/cli.rs codex-rs/tui2/src/cli.rs --- codex-rs/tui/src/cli.rs 2025-11-19 13:40:42 +++ codex-rs/tui2/src/cli.rs 2025-12-12 17:30:43 @@ -88,3 +88,28 @@ #[clap(skip)] pub config_overrides: CliConfigOverrides, } + +impl From<codex_tui::Cli> for Cli { + fn from(cli: codex_tui::Cli) -> Self { + Self { + prompt: cli.prompt, + images: cli.images, + resume_picker: cli.resume_picker, + resume_last: cli.resume_last, + resume_session_id: cli.resume_session_id, + resume_show_all: cli.resume_show_all, + model: cli.model, + oss: cli.oss, + oss_provider: cli.oss_provider, + config_profile: cli.config_profile, + sandbox_mode: cli.sandbox_mode, + approval_policy: cli.approval_policy, + full_auto: cli.full_auto, + dangerously_bypass_approvals_and_sandbox: cli.dangerously_bypass_approvals_and_sandbox, + cwd: cli.cwd, + web_search: cli.web_search, + add_dir: cli.add_dir, + config_overrides: cli.config_overrides, + } + } +} diff -ru --ex codex-rs/tui/src/main.rs codex-rs/tui2/src/main.rs --- codex-rs/tui/src/main.rs 2025-12-12 16:39:05 +++ codex-rs/tui2/src/main.rs 2025-12-12 16:39:06 @@ -1,8 +1,8 @@ use clap::Parser; use codex_arg0::arg0_dispatch_or_else; use codex_common::CliConfigOverrides; -use codex_tui::Cli; -use codex_tui::run_main; +use codex_tui2::Cli; +use codex_tui2::run_main; #[derive(Parser, Debug)] struct TopCli { diff -ru --ex codex-rs/tui/src/update_action.rs codex-rs/tui2/src/update_action.rs --- codex-rs/tui/src/update_action.rs 2025-11-19 11:11:47 +++ codex-rs/tui2/src/update_action.rs 2025-12-12 17:30:48 @@ -9,6 +9,20 @@ BrewUpgrade, } +impl From<UpdateAction> for codex_tui::update_action::UpdateAction { + fn from(action: UpdateAction) -> Self { + match action { + UpdateAction::NpmGlobalLatest => { + codex_tui::update_action::UpdateAction::NpmGlobalLatest + } + UpdateAction::BunGlobalLatest => { + codex_tui::update_action::UpdateAction::BunGlobalLatest + } + UpdateAction::BrewUpgrade => codex_tui::update_action::UpdateAction::BrewUpgrade, + } + } +} + impl UpdateAction { /// Returns the list of command-line arguments for invoking the update. pub fn command_args(self) -> (&'static str, &'static [&'static str]) { ```Josh McKinney ·
2025-12-12 20:46:18 -08:00