mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
1fa8350ae79c37da62cd2f752a27cd67bc15cf4e
4 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 -
Improve handling of config and rules errors for app server clients (#9182)
When an invalid config.toml key or value is detected, the CLI currently just quits. This leaves the VSCE in a dead state. This PR changes the behavior to not quit and bubble up the config error to users to make it actionable. It also surfaces errors related to "rules" parsing. This allows us to surface these errors to users in the VSCE, like this: <img width="342" height="129" alt="Screenshot 2026-01-13 at 4 29 22 PM" src="https://github.com/user-attachments/assets/a79ffbe7-7604-400c-a304-c5165b6eebc4" /> <img width="346" height="244" alt="Screenshot 2026-01-13 at 4 45 06 PM" src="https://github.com/user-attachments/assets/de874f7c-16a2-4a95-8c6d-15f10482e67b" />
Eric Traut ·
2026-01-13 17:57:09 -08:00 -
fix: integration test for #9011 (#9166)
Adds an integration test for the new behavior introduced in https://github.com/openai/codex/pull/9011. The work to create the test setup was substantial enough that I thought it merited a separate PR. This integration test spawns `codex` in TUI mode, which requires spawning a PTY to run successfully, so I had to introduce quite a bit of scaffolding in `run_codex_cli()`. I was surprised to discover that we have not done this in our codebase before, so perhaps this should get moved to a common location so it can be reused. The test itself verifies that a malformed `rules` in `$CODEX_HOME` prints a human-readable error message and exits nonzero.
Michael Bolin ·
2026-01-13 23:39:34 +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