Commit Graph

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"
    />
  • 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"
    />
  • 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.
  • 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]) {
    ```