From 73d4c19a3262ea5e77931460cfb127ea8103138b Mon Sep 17 00:00:00 2001 From: Josh McKinney Date: Wed, 7 Jan 2026 14:25:05 -0800 Subject: [PATCH 1/7] docs: add quit behavior history and design Add Ctrl+C/Ctrl+D behavior history and exit confirmation prompt design documentation. Introduce markdownlint config with a 100 character line length to keep markdown formatting consistent. --- .markdownlint-cli2.jsonc | 8 + docs/ctrl-c-quit-history.md | 147 ++++++ docs/exit-confirmation-prompt-design.md | 629 ++++++++++++++++++++++++ 3 files changed, 784 insertions(+) create mode 100644 .markdownlint-cli2.jsonc create mode 100644 docs/ctrl-c-quit-history.md create mode 100644 docs/exit-confirmation-prompt-design.md diff --git a/.markdownlint-cli2.jsonc b/.markdownlint-cli2.jsonc new file mode 100644 index 00000000000..90985b6d21f --- /dev/null +++ b/.markdownlint-cli2.jsonc @@ -0,0 +1,8 @@ +{ + "config": { + "MD013": { + "line_length": 100 + } + } +} + diff --git a/docs/ctrl-c-quit-history.md b/docs/ctrl-c-quit-history.md new file mode 100644 index 00000000000..bab2392af12 --- /dev/null +++ b/docs/ctrl-c-quit-history.md @@ -0,0 +1,147 @@ +# Ctrl+C / Ctrl+D behavior history (Codex CLI) + +This document explains how `Ctrl+C` and `Ctrl+D` behave in the Codex Rust TUI, how that behavior +changed over time, and why. It draws only from GitHub PR descriptions, PR review comments, and +linked GitHub issues. + +## Mental model + +`Ctrl+C` / `Ctrl+D` can arrive via different mechanisms: + +In a line-buffered program, `Ctrl+C` is usually delivered as a signal (`SIGINT`) and `Ctrl+D` is +usually EOF on stdin. In the TUI, raw mode is enabled and both usually show up as key events +(e.g., `KeyCode::Char('c')` with Control), not as `SIGINT`/EOF. + +If Codex is installed via npm, a Node.js wrapper launches the native binary. That wrapper’s signal +forwarding affects how `SIGINT`/`SIGTERM` behave when the parent process receives a signal (it +generally does not affect the TUI’s raw-mode key handling). + +## Current behavior (Rust TUI) + +As of HEAD: + +- `Ctrl+C` is stateful. It may dismiss an active modal, clear the composer, interrupt a running + task, or trigger shutdown when idle with an empty composer. +- `Ctrl+D` triggers exit only when the composer is empty; otherwise it behaves like normal input. + +The main decision point for `Ctrl+C` is `ChatWidget::on_ctrl_c` in +`codex-rs/tui/src/chatwidget.rs:3309`, with consumption/handling in `BottomPane::on_ctrl_c` in +`codex-rs/tui/src/bottom_pane/mod.rs:218`. Exiting is coordinated via `ShutdownComplete` +(`codex-rs/tui/src/chatwidget.rs:1051`) and `AppEvent::ExitRequest` (`codex-rs/tui/src/app.rs:718`). + +## Timeline of behavior changes + +### — Handle Ctrl+C quit when idle + +This introduced a deliberate “two-step” quit flow for `Ctrl+C` while idle: the first press shows +a quit hint and a second press exits. It also reset the hint when other input arrived or when work +began. + +### — Introducing shutdown operation + +This PR introduced an explicit shutdown operation so background tasks could flush work before the +process exits. A key review concern was that if the Tokio runtime shuts down, still-running tasks +can be dropped before they finish draining queued work, so “fire and forget” spawning is not +enough for correctness. + +The review thread is also where “shutdown completion” became an explicit part of the contract: +there is a comment asking whether anything in the TUI waits for shutdown to be processed, and +another calling out a user-visible concern: “Wait, why are we changing Ctrl-C behavior. Did you +manually test this? What is different now?” This is useful context because later `Ctrl+C` quit +behavior relies on the existence and correctness of `ShutdownComplete`. + +### — Fix approval workflow + +This PR made approval flows visible and interruptible. Commands requiring approval are written to +chat history before the modal appears, the decision is logged after, and `Ctrl+C` aborts an open +approval modal (effectively acting like Esc for that modal). + +The PR review comments are where the “consumed vs not consumed” contract is made explicit. In +particular, one review suggested introducing a small enum (e.g. `CancellationEvent`) rather than a +boolean so it is clear whether `Ctrl+C` was handled by the UI. Another review comment explicitly +describes the intended user flow: after aborting the modal, show the standard quit hint so a +subsequent `Ctrl+C` can exit. + +### — Ctrl+D exits only when composer is empty + +This prevented accidental exits from `Ctrl+D` when the composer has typed text. The PR description +frames the design as “exit only when there’s nothing to lose”, and the PR comment summarizes the +mechanics as routing `Ctrl+D` to normal handling unless the composer is empty, plus adding +`is_empty` helpers to keep the check consistent. + +### — single control flow for both Esc and Ctrl+C + +This tightened correctness around “interrupt” semantics. The stated intent is that while a task is +running, Esc and `Ctrl+C` should do the same thing, and that certain stuck-history-widget cases +were fixed. + +The PR description also highlights a subtle `Ctrl+D` edge case: `Ctrl+D` could quit the app while +an approval modal was showing if the textarea was empty. + +There are no PR comment threads to incorporate here, but the commit messages within the PR show +what it tried to make true: + +- there should be a single interrupt path (rather than multiple UI components sending interrupts), +- interrupt/error should finalize in-progress UI cells consistently (e.g., replacing a spinner with + a failed mark), and +- `Ctrl+D` behavior needed an explicit fix. + +### — Clear non-empty prompts with ctrl + c + +This PR changed `Ctrl+C` to clear the prompt when the composer is non-empty, which is more aligned +with shell/REPL ergonomics than exiting. The PR description also mentions adjusting the hint text +to distinguish “interrupt” vs “quit” depending on state, and calls out tradeoffs (e.g., overlap +between Esc and `Ctrl+C`, and footer hint text not perfectly matching all states). + +This change is important context for “why does Ctrl+C quit immediately now?” because once `Ctrl+C` +is overloaded as “clear the prompt” when non-empty, the remaining idle case (empty composer) is +the one left to map to “quit”. + +### — Recover cleared prompt via history (Up arrow) + +This PR was a mitigation for prompt-clearing on `Ctrl+C`: it preserved cleared text so it could be +recovered via history navigation. + +The PR review comments show that an “undo clear prompt” shortcut was considered. A review comment +warned that restoring the prompt is tricky when the composer contains attachments: if placeholders +are restored as plain text rather than rebuilt atomically, users can partially delete/edit them and +silently lose the associated image. This is a concrete example of why a narrow undo binding can +have surprising edge cases. + +The review also includes implementation-level tightening (e.g., moving “record cleared draft” into +the clear path so clearing and remembering remain in sync, and questioning whether restoration +should reuse existing higher-level methods like `set_text_content`). + +### — Fix false "task complete" state while streaming + +This is an example where state tracking affects `Ctrl+C` behavior: the bug caused `Ctrl+C` to quit +instead of canceling the stream during the final agent message. It is a reminder that semantics +like “interrupt when working” depend on correct “working vs idle” state. + +### — ^C resets prompt history navigation cursor + +This made `Ctrl+C` in the prompt area behave more like common shells by resetting history +navigation state. It is not an exit behavior change, but it reinforces the trend of making prompt +area `Ctrl+C` do “prompt things” first (clear/reset/interrupt) before “quit”. + +## npm Node wrapper (signal path) + +When Codex is installed via npm, a Node.js wrapper launches the native binary. Signal handling in +that wrapper affects how `SIGINT`/`SIGTERM` behave when the parent process receives a signal. + +### — npm wrapper: forward signals and exit with child + +This PR fixed two problems in the npm wrapper: it now forwards termination signals to the child +and it correctly exits when the child exits. It references reports of runaway processes and links +to . + +## If you change Ctrl+C behavior again + +Most “what should `Ctrl+C` do?” questions depend on which state you are in. Before changing +behavior, it helps to make an explicit decision for each: + +- idle with empty composer (quit immediately vs require confirmation), +- idle with non-empty composer (clear vs quit, and how recoverable clear should be), +- working/streaming (always interrupt, and whether any state bugs can misclassify as idle), +- modal open (dismiss/abort first, and what the next `Ctrl+C` should do), +- signal path vs key-event path (especially for npm installs). diff --git a/docs/exit-confirmation-prompt-design.md b/docs/exit-confirmation-prompt-design.md new file mode 100644 index 00000000000..8c4e31d1427 --- /dev/null +++ b/docs/exit-confirmation-prompt-design.md @@ -0,0 +1,629 @@ +# Exit confirmation prompt (Ctrl+C / Ctrl+D) — design for tui + tui2 + +This document proposes a unified implementation for an exit confirmation prompt that triggers on +`Ctrl+C` / `Ctrl+D` in both Rust TUIs (`codex-rs/tui` and `codex-rs/tui2`). + +It is grounded in `docs/ctrl-c-quit-history.md`, which captures prior design intent and regressions +around cancellation vs quitting. + +## Background and motivation + +In Codex’s TUIs, `Ctrl+C` and `Ctrl+D` arrive as key events (raw mode), not as `SIGINT` / stdin +EOF. Historically, Codex has used these keys for multiple user intents: + +- cancel/interrupt an in-flight operation, +- dismiss a modal / popup, +- clear draft input, +- quit the application. + +This overload is intentional, but it is fragile: any bug in “are we working?” state tracking can +turn a would-be cancellation into an immediate quit. `docs/ctrl-c-quit-history.md` includes an +example (`PR #4627`) where state incorrectly flipped to “idle” while streaming, causing `Ctrl+C` +to quit instead of interrupt. + +We also have a related bug: pressing `Ctrl+C` during a review task can quit immediately, but the +intended behavior is “cancel the review”. + +The exit confirmation prompt is primarily a safety feature to prevent accidental exits when the key +press would otherwise quit. It must not mask or replace the primary expectation: `Ctrl+C` cancels +active work whenever possible. + +## Terminology: exit vs shutdown vs interrupt + +Codex distinguishes between: + +- **Exit**: end the UI event loop and terminate the process (e.g., `AppEvent::ExitRequest` + returning `Ok(false)`). +- **Shutdown**: request a graceful agent/core shutdown (e.g., `Op::Shutdown`), often waiting for + `ShutdownComplete` before exiting so background work can flush cleanly. +- **Interrupt**: cancel a running operation (e.g., `Op::Interrupt`), used for streaming turns, + long-running tools, and other cancellable tasks. + +The design must be explicit about which action a “quit gesture” triggers. In particular: if today +idle `Ctrl+C` results in `Op::Shutdown`, a confirmation prompt must preserve that semantic +(prompting should not silently convert “shutdown” into “exit immediately”). + +### Lifecycle model (what runs where) + +The UI and the agent/core have distinct lifecycles: + +- The **UI event loop** owns input handling and rendering. “Exit” means the event loop stops and + the process typically terminates shortly after. +- The **agent/core** runs work that can outlive a single frame or keypress: streaming turns, + tool execution, review tasks, background state updates, etc. + +This matters because exiting the UI can abruptly end the runtime and drop background tasks. +`docs/ctrl-c-quit-history.md` calls this out as a correctness concern (see `PR #1647`): a graceful +shutdown handshake exists so the core can finish draining/flush work before the process exits. + +In this design: + +- **Exit immediately** means “exit the UI loop now” (without a shutdown handshake). +- **Shutdown then exit** means “request core shutdown, then exit the UI after shutdown completes”. + The completion signal is `ShutdownComplete` (event name varies per crate, but the contract is the + same: “core has finished shutting down”). + +Today, the TUIs mix “exit immediately” and “shutdown first” depending on how the user tries to +leave. + +### Examples of core/UI state (what users see) + +- **Idle**: no streaming output, no running tools, no active review task; the composer may be empty + or contain a draft. +- **Waiting for the turn to start**: the user pressed Enter, and the UI is waiting for the first + events for the new turn (this typically transitions quickly into “working/streaming”). +- **Streaming**: partial agent output is arriving and being rendered. +- **Running commands/tools**: the agent is executing local commands or tools and streaming their + progress/output into the transcript. +- **Review in progress**: a review request has been issued and the UI is in review mode, awaiting + results. +- **Modal open**: approvals, pickers, selection views, etc. are on screen. + +In the current implementation, the key state driving `Ctrl+C` behavior is whether the bottom pane +considers a task “running”. That state is toggled as turns start/finish, while some other +long-running flows (like review) have separate flags (e.g., `is_review_mode`). Regressions happen +when those states disagree (for example, when streaming is happening but the UI momentarily thinks +it is idle). + +### Current trigger mapping (tui and tui2) + +| Trigger | What it does today | +| ------------------------------ | ------------------------------------------------------------- | +| `Ctrl+C` while work is running | Interrupt (`Op::Interrupt`) | +| `Ctrl+C` while idle | Shutdown (`Op::Shutdown`), exit on `ShutdownComplete` | +| `Ctrl+D` with empty composer | Exit immediately (`ExitRequest`, bypasses `Op::Shutdown`) | +| `/quit`, `/exit`, `/logout` | Exit immediately (`ExitRequest`, bypasses `Op::Shutdown`) | +| `/new` (NewSession) | Shutdown current conversation, then stay running | + +Notes: + +- The “work is running” case depends on “task running” state being accurate. If it is wrong, a + `Ctrl+C` can take the idle path and start shutdown instead of interrupting. +- `ShutdownComplete` is generally treated as “exit now” by the widget layer, but the app layer can + suppress it when shutdown is used for cleanup rather than quitting. + +### What `ShutdownComplete` does today + +`ShutdownComplete` is a sentinel event with no payload. It is used as a lifecycle boundary: + +- In core, it is emitted at the end of the shutdown handler after aborting tasks, terminating + processes, and shutting down the rollout recorder. +- In the TUIs, `ChatWidget` treats it as “exit now” by calling `request_exit()`. +- In `App`, a one-shot suppression flag can ignore the next `ShutdownComplete` when shutdown is + used for cleanup (e.g., stopping the current conversation during `/new`) rather than quitting. + +The key point: `ShutdownComplete` is not “cleanup itself”; it is the signal that cleanup has +already happened (or that the core believes shutdown is complete). + +### Scenario walkthrough (how shutdown vs exit shows up) + +These examples describe what happens today and why it matters for the confirmation prompt design: + +- **Idle + composer empty**: `Ctrl+C` sends `Op::Shutdown`, and the UI exits once `ShutdownComplete` + arrives. `Ctrl+D` exits immediately without sending `Op::Shutdown`. `/quit` and `/exit` also exit + immediately. +- **Idle + composer has a draft**: `Ctrl+C` clears the draft (and shows the quit hint). It does not + exit or shutdown on that press. A subsequent `Ctrl+C` may now hit “idle + empty” and trigger + shutdown. +- **Working/streaming output**: `Ctrl+C` sends `Op::Interrupt` (cancel) rather than quitting. This + is the primary “don’t lose my session” behavior, and it relies on “task running” being true. +- **Running local tools/commands**: this is still “working”; `Ctrl+C` should interrupt rather than + quit. If “task running” is false by mistake, `Ctrl+C` can incorrectly start shutdown. +- **Review in progress**: intended behavior is the same as “working”: `Ctrl+C` cancels the review. + The reported bug (“quit immediately during review”) indicates the UI is misclassifying review as + idle for `Ctrl+C` handling. +- **Modal open**: `Ctrl+C` is first offered to the active modal/view to dismiss/abort. It should + not trigger shutdown/exit unless the modal declines to handle it. + +### Why `/quit`, `/exit`, `/logout`, and `Ctrl+D` don’t call shutdown today + +Today these are implemented as explicit “leave the UI now” actions: + +- `/quit`, `/exit`, and `/logout` dispatch `AppEvent::ExitRequest` directly. +- `Ctrl+D` exits only when the composer is empty (a guard added to reduce accidental exits), but it + still exits via `ExitRequest` rather than `Op::Shutdown`. + +This split does not have a principled/rational basis documented anywhere, and it may simply be an +accidental divergence in how different quit triggers were implemented over time. + +Either way, bypassing `Op::Shutdown` is the riskier option: it relies on runtime teardown and +dropping to clean up in-flight work, which can leave behind “leftovers” (e.g., unified exec child +processes, unflushed rollout/session tail, or other background tasks that would normally be fenced +by `ShutdownComplete`). Where possible, Codex should prefer the shutdown handshake and exit only +after `ShutdownComplete`, regardless of whether the quit was initiated via keys or slash commands. + +## Problem statement + +1. **Accidental closure**: users sometimes press `Ctrl+C` / `Ctrl+D` out of habit + and unexpectedly lose their session. +2. **State misclassification regressions**: if the UI incorrectly believes it is + idle (streaming, review, etc.), `Ctrl+C` can incorrectly quit. +3. **Review cancellation bug**: `Ctrl+C` during an in-progress review should + cancel the review, not quit. +4. **Modal edge cases**: `Ctrl+D` (composer empty) must not quit while a modal/popup is open (see + history doc for prior regressions). + +## Goals + +- Prompt only when `Ctrl+C` / `Ctrl+D` would otherwise quit. +- Keep `/quit`, `/exit`, `/logout` as intentional exits (no prompt). +- Prefer shutdown+exit (graceful) for all quit paths where possible. +- Ensure `Ctrl+C` cancels review (and other active work) reliably. +- Keep behavior consistent across `tui` and `tui2`, including prompt copy and config key. +- Persist “don’t ask again” in `~/.codex/config.toml` under `[notice]`. +- Add tests that encode the rationale and prevent regressions. + +## Non-goals + +- Changing npm wrapper signal forwarding behavior (signal path). +- Redesigning all footer hint text (keep changes focused to quit confirmation). +- Introducing a shared UI crate between `tui` and `tui2` (we align by convention, not by code + sharing). + +## Proposed user-visible behavior + +Use a single config key: + +```toml +[notice] +hide_exit_confirmation_prompt = true +``` + +`[notice]` is the right section for this: it already stores similar acknowledgement/NUX flags like +`hide_full_access_warning` and `hide_rate_limit_model_nudge`. + +When unset/false, show a confirmation prompt before quitting via `Ctrl+C` or `Ctrl+D` (the +accidental-prone exit gestures). The prompt must offer: + +- A primary quit option labeled like other confirmations (recommended: `Yes, quit Codex`). +- A cancel option (recommended: `No, stay in Codex`). +- A “remember” option that matches existing wording used elsewhere in the UI (recommended: + `Yes, and don't ask again`), which sets the config key and persists it. + +### State-based behavior table + +The key is to treat `Ctrl+C` primarily as “cancel”, and only as “quit” when there is nothing to +cancel. + +| State | `Ctrl+C` | `Ctrl+D` | +| ---------------------------- | ----------------------------- | --------------------- | +| Modal/popup open | Dismiss/abort the modal | Must not quit | +| Task/review/streaming active | Interrupt/cancel (never quit) | Do nothing | +| Composer has draft content | Clear draft | Normal input | +| Idle, composer empty | Quit (shutdown+exit) | Quit (shutdown+exit) | + +Notes: + +- “Task active” must include review mode. If review is not represented as “task running” today, it + must be wired in as part of this change. +- `Ctrl+D` should be treated as a quit gesture only when there is nothing to lose (composer empty) + and no modal is open. + +## Proposed implementation (unified design) + +### Policy: quit should be shutdown-first + +Since there is no clear rationale for having some quit triggers bypass shutdown, this design +assumes a single default: quitting Codex should request `Op::Shutdown` and exit only after +`ShutdownComplete`. + +An “exit immediately” path can remain as a fallback (e.g., if shutdown hangs), but it should not +be the normal route for user-initiated quit gestures or commands. + +### Add an app-level event that means “quit with confirmation” + +Keep `AppEvent::ExitRequest` meaning “exit immediately”. + +Add: + +```rust +AppEvent::QuitRequest { confirm: bool } +``` + +Handling lives in the app layer (`App`), because: + +- `App` is already the coordinator for exit (`ExitRequest`) and for reacting to `ShutdownComplete`. +- `App` owns the current configuration and is the right place to gate “prompt vs no prompt”. + +Pseudo-flow: + +1. A key handler decides “this key press would quit” and sends + `QuitRequest { confirm: true }`. +2. `App` checks `config.notices.hide_exit_confirmation_prompt.unwrap_or(false)`: + - if `true`, initiate shutdown+exit immediately (no prompt). + - if `false`, ask `ChatWidget` to open the confirmation prompt. + +Slash commands like `/quit`, `/exit`, and `/logout` can dispatch `QuitRequest { confirm: false }` +to preserve the “no prompt” behavior while still taking the shutdown+exit path. + +### Action handling details (shutdown+exit) + +To keep behavior correct and consistent, `App` must treat quitting as a two-step +sequence: + +1. Send `Op::Shutdown` to the core. +2. Exit the UI loop only after observing the shutdown completion event. + +This is the core invariant: the confirmation prompt should only gate whether we ask the user. + +Codex already uses `Op::Shutdown` for non-exit reasons (e.g., stopping a conversation/thread before +starting a new one). In the current TUIs, `ChatWidget` always exits on `ShutdownComplete`, and +`App` decides whether to forward that event using a suppression flag (see +`codex-rs/tui/src/app.rs:314` and `codex-rs/tui2/src/app.rs:378`). Any new quit confirmation flow +should preserve that separation: “shutdown” does not inherently mean “exit the app”. + +This is also why `/new` works the way it does today: it shuts down the current conversation/thread +to avoid leaking work, but it suppresses `ShutdownComplete` so the app can create a new session and +keep running instead of quitting. + +### Policy change: make exit paths shutdown-first + +Historically, Codex has had multiple exit paths. Some of them request `Op::Shutdown` first (idle +`Ctrl+C`), and some exit the UI immediately (`Ctrl+D` when the composer is empty, plus `/quit`, +`/exit`, and `/logout`). That split is easy to miss and it can skip meaningful cleanup. + +This design proposes to move all application exit paths to shutdown-first, so exiting Codex is +consistent and performs the same cleanup regardless of which “quit” gesture or command the user +uses. In practice, that means: + +- `Ctrl+C` (idle quit) continues to be shutdown-first. +- `Ctrl+D` (empty composer quit) should become shutdown-first. +- `/quit`, `/exit`, and `/logout` should become shutdown-first. + +If shutdown latency or hangs are a concern, this policy should be paired with: + +- the logging described in the “Observability and hang diagnosis” section, +- a bounded timeout with a clearly logged fallback to “exit immediately” (optional, but recommended + if hangs are observed in practice). + +### What `Op::Shutdown` cleans up in core (today) + +In `codex-rs/core`, shutdown performs a small set of concrete cleanup steps before emitting +`ShutdownComplete`: + +- Abort active tasks and turns (cancels tasks, runs per-task abort hooks, emits `TurnAborted`): + `codex-rs/core/src/tasks/mod.rs:158`. +- Terminate all unified exec processes (kills any long-running child processes created via unified + exec): `codex-rs/core/src/unified_exec/process_manager.rs:653`. +- Shut down the rollout recorder (acts as a barrier so the writer task has processed all prior + queued items; each rollout line is flushed as it is written): `codex-rs/core/src/codex.rs:2109` + and `codex-rs/core/src/rollout/recorder.rs:368`. +- Emit `ShutdownComplete`: `codex-rs/core/src/codex.rs:2137`. + +This is the work that can be skipped or cut short if the UI exits immediately without requesting +shutdown: tasks may be dropped by runtime teardown, rollout items can be lost if still queued, and +child processes may survive past the UI exit. + +### What it means to drop in-flight work (effects users can see) + +If the UI exits immediately (without `Op::Shutdown`), the process can terminate while work is still +in flight. The concrete consequences depend on which state Codex is in: + +- **Streaming/working**: partial output may be visible in the transcript, but the turn may not reach + its normal “end” path (e.g., no final `TaskComplete`, no finalization for in-progress UI cells). +- **Tool execution / unified exec**: child processes may outlive the UI if not explicitly + terminated, depending on how they were spawned. A shutdown path explicitly terminates them. +- **Rollout/session persistence**: the rollout writer is asynchronous. If the process exits while + items are queued but not yet processed, the session file may miss its tail. That can affect + replay/resume fidelity and any tooling/tests that inspect the rollout file. +- **Review in progress**: review uses separate state and can be misclassified as idle. If the UI + takes an exit path instead of an interrupt path, review can stop abruptly rather than being + cleanly cancelled. + +This is why the design prioritizes correctness over convenience for accidental-prone quit gestures: +we want to prefer “interrupt when working” and “shutdown+exit when idle” rather than “exit now”. + +### Rollout recorder shutdown: why this is written as “shutdown” + +The rollout shutdown mechanism can look surprising at first glance because it sends a “shutdown” +command to the writer task, then drops the recorder. + +Based on the current code: + +- The rollout writer task is single-threaded and processes commands FIFO. +- Each rollout line is written and flushed immediately when processed. +- Sending `RolloutCmd::Shutdown` and waiting for its ack works as a barrier: the ack cannot be sent + until all earlier queued `AddItems`/`Flush` commands have been processed by that task. +- Dropping the recorder then closes the channel, so the writer task can exit and the file handle is + closed. + +This appears to be the intended design (the comment in `codex-rs/core/src/codex.rs:2117` calls out +avoiding races with the background writer, especially in tests). If this shutdown barrier has +user-facing downsides (e.g., exit latency when the queue is backlogged), it would be worth +confirming intent and expectations with code owners while implementing the prompt flow. + +## Developer notes (footguns and invariants) + +This section is a checklist of context that is easy to miss when changing quit behavior. + +### Key-event path vs signal path + +In the TUIs, `Ctrl+C` and `Ctrl+D` typically arrive as key events (raw mode), not as `SIGINT` or +stdin EOF. Signal forwarding in the npm wrapper can affect non-TUI usage, but it usually does not +drive the in-TUI `Ctrl+C` handling. The quit confirmation prompt is about the key-event path. + +### “Consumed vs not consumed” is the quit boundary + +`Ctrl+C` is first offered to the bottom pane (`BottomPane::on_ctrl_c`), which may: + +- dismiss an active view/modal, +- clear a non-empty composer draft (and show the quit hint), +- or return “not handled” when the composer is empty and no view is active. + +The quit confirmation prompt should only be reachable when `Ctrl+C` was not consumed by the UI +layer (and `Ctrl+D` should be disabled while a modal is open). + +### “Task running” is a policy input, not a source of truth + +The current `Ctrl+C` decision uses `bottom_pane.is_task_running()` as the proxy for “working vs +idle” (see `ChatWidget::on_ctrl_c` in both TUIs). This state is toggled by task lifecycle events +and can be wrong transiently if state updates lag behind UI input. + +Implications: + +- Any regression that makes “task running” false while the agent is still active can turn `Ctrl+C` + into a quit (shutdown) path instead of an interrupt path. +- Review mode (`is_review_mode`) is tracked separately from “task running” today. The reported bug + (“quit during review”) strongly suggests these states can disagree. The implementation should + treat review as cancellable work and ensure the `Ctrl+C` path hits interrupt/cancel, not quit. + +### Interrupt vs shutdown clean up different things + +- `Op::Interrupt` is the “cancel current work” path. +- `Op::Shutdown` is the “stop everything and clean up” path. It aborts tasks, terminates unified + exec processes, and fences rollout persistence before emitting `ShutdownComplete`. + +The quit confirmation prompt must preserve this distinction; it should not replace a shutdown-first +quit path with an immediate exit. + +### Shutdown completion is wired to exit today + +In both TUIs, `ChatWidget` treats `ShutdownComplete` as “exit now” via `request_exit()`. The app +layer can ignore one `ShutdownComplete` using `suppress_shutdown_complete` when shutdown is used as +cleanup (e.g., `/new`). + +When adding a confirmation prompt, keep this separation intact: + +- “shutdown” is a core lifecycle action, and it may be used without quitting (conversation reset), +- “exit” is a UI lifecycle action, and it may happen without shutdown (a fallback for shutdown + hangs, but not the preferred/normal path). + +### `/new` already uses shutdown without quitting + +`/new` (NewSession) shuts down the current conversation/thread, suppresses the next +`ShutdownComplete`, removes the thread from the manager, and then creates a new session. This is a +useful example of why shutdown intent must be explicit: “shutdown” does not always mean “quit the +app”. + +### Consider “shutdown stuck” behavior + +If the quit confirmation prompt encourages more shutdown-first paths, it increases the chance +users wait on shutdown. The current UI largely assumes shutdown completes. If shutdown can hang in +practice, the design should decide whether to: + +- show a status line while shutting down, +- apply a timeout and then force-exit, +- or provide an escape hatch (e.g., a second confirm to exit immediately). + +This doc does not pick a policy yet, but implementers should consider it when wiring the flow. + +### Observability and hang diagnosis + +There are known cases where exit feels “stuck” to users. The shutdown/exit split makes this harder +to debug unless we log the decision points and the time spent waiting. + +The goal of logging here is: given a single log excerpt, it should be obvious: + +- what initiated the exit attempt (key vs slash command vs internal reset), +- whether we chose interrupt, shutdown+exit, or exit-immediately, +- what cleanup steps we started and finished, +- what timed out or errored, +- and whether `ShutdownComplete` was suppressed (conversation reset) or acted upon (quit). + +Recommended logging points (high signal, low volume): + +- **UI quit initiation** (tui/tui2): log once when we decide a quit path is being taken. Include: + - trigger (`ctrl_c`, `ctrl_d`, `/quit`, `/exit`, `/logout`, `/new`, etc.), + - action (`interrupt`, `shutdown_then_exit`, `exit_immediately`), + - identifiers (thread/conversation id if available), + - and whether a confirmation prompt was shown or skipped due to config. +- **Shutdown request sent** (tui/tui2): log when `Op::Shutdown` is submitted and record a start + timestamp for latency measurement. +- **Shutdown complete observed** (tui/tui2): log when `ShutdownComplete` is received, including: + - elapsed time since shutdown request (if known), + - whether it was suppressed (`suppress_shutdown_complete`), + - and the resulting action (exit UI vs continue running). +- **Core shutdown handler** (core): log the start and end of each cleanup step with durations: + - abort tasks, + - terminate unified exec processes, + - rollout recorder shutdown barrier, + - emit `ShutdownComplete`. + +Timeout guidance: + +- `abort_all_tasks` already includes a bounded “graceful interrupt” window per task, but the overall + shutdown path can still stall on I/O (e.g., rollout file flush) or unexpected blocking. If hangs + are a problem, add bounded timeouts around the most likely culprits (rollout shutdown barrier and + unified exec termination) and emit a warning that includes which step timed out and what cleanup + may have been skipped. + +Developer ergonomics: + +- Prefer structured logs with stable fields (trigger, action, ids, elapsed_ms, step) so we can grep + and so future telemetry can be added without rewriting messages. +- Use one-line summaries at INFO and keep deeper detail at DEBUG to avoid spamming normal runs. + +### Render the prompt UI in ChatWidget (both tui and tui2) + +Add a method like: + +```rust +fn open_exit_confirmation_prompt(&mut self) +``` + +Implementation requirements: + +- Do not open the prompt if another bottom-pane view/popup is already active. +- “Quit now” initiates shutdown+exit. +- “Quit and don’t ask again” triggers, in order: + 1) update in-memory config (`UpdateExitConfirmationPromptHidden(true)`), + 2) persist it (`PersistExitConfirmationPromptHidden`), + 3) initiate shutdown+exit. +- Keep copy consistent between `tui` and `tui2` and mention `Ctrl+C`/`Ctrl+D` explicitly. + +### Centralize “can cancel?” logic (fixes the review bug) + +The related bug (“Ctrl+C quits during review”) is fundamentally a missing/incorrect state signal. + +As part of this work, define a single predicate used by the `Ctrl+C` handler: + +- `is_cancellable_work_active()` (name may vary), returning true for: + - agent turn streaming, + - review task running, + - any other operation that should treat `Ctrl+C` as interrupt. + +Then: + +- If `is_cancellable_work_active()` is true, `Ctrl+C` must send `Op::Interrupt` (or the + appropriate review-cancel op) and must not open the quit prompt. +- Ensure the review workflow flips whatever state drives this predicate early and keeps it true + until the review is fully finished or cancelled. + +This is the correctness fix; the confirmation prompt is the safety net. + +### Persist config + +In `codex-rs/core`: + +- Add `Notice::hide_exit_confirmation_prompt: Option`. +- Add a `ConfigEdit` variant and a `ConfigEditsBuilder` helper: + - `SetNoticeHideExitConfirmationPrompt(bool)` + - `set_hide_exit_confirmation_prompt(bool) -> Self` +- Add a unit test ensuring setting this flag preserves existing `[notice]` table contents + (mirroring existing tests for other notice keys). + +In docs: + +- Update `docs/config.md` to mention the new `[notice]` key and describe the behavior in one short + paragraph. + +## Testing plan + +### Snapshot tests (both tui and tui2) + +Add an `insta` snapshot test that renders the exit confirmation prompt and asserts the full popup +text. This locks: + +- the exact prompt copy (grounded in the rationale), +- the presence of the “don’t ask again” option, +- line wrapping behavior at typical widths (e.g., 80 columns). + +Keep snapshot names aligned between crates, e.g.: + +- `exit_confirmation_popup` (tui) +- `exit_confirmation_popup` (tui2) + +### Behavioral tests (both tui and tui2) + +Add targeted tests that encode the correctness boundaries: + +- `ctrl_c_during_review_cancels_review_not_exit` + - Assert: no `ExitRequest`/exit event emitted; interrupt/cancel path is taken. +- `ctrl_c_when_task_running_interrupts_not_exit` +- `ctrl_d_does_not_exit_when_modal_open` +- `ctrl_d_does_not_exit_when_composer_non_empty` +- `dont_ask_again_sets_notice_and_persists` + - Assert the `Update...Hidden(true)` and `Persist...Hidden` events are emitted in sequence (or + equivalent). + +### Manual testing (both tui and tui2) + +Run the same checklist in `codex-rs/tui` and `codex-rs/tui2`. + +- **Idle, composer empty** + - Press `Ctrl+C` → confirmation prompt appears → confirm quit → app performs shutdown+exit. + - Press `Ctrl+D` → confirmation prompt appears → confirm quit → app performs shutdown+exit. +- **Idle, composer has draft** + - Press `Ctrl+C` → clears draft (no prompt). + - Press `Ctrl+D` → does not quit (normal input behavior; no prompt). +- **While streaming / tool running** + - Start a turn that streams for a bit (or run a tool that takes time, e.g. something that sleeps). + - Press `Ctrl+C` → interrupts/cancels work (no prompt, no quit). +- **Review in progress** + - Start a review flow. + - Press `Ctrl+C` → cancels the review (no prompt, no quit). +- **Modal/popup open** + - Open any modal/popup (approvals/picker/etc.). + - Press `Ctrl+D` → must not quit. + - Press `Ctrl+C` → dismisses/aborts the modal (must not quit). +- **Slash commands** + - Run `/quit` (and separately `/exit`, `/logout`) → exits without prompting, but still uses the + shutdown+exit path. +- **Don’t ask again** + - Trigger the prompt and choose “don’t ask again”. + - Repeat `Ctrl+C` / `Ctrl+D` on idle+empty → no prompt; still shutdown+exit. + - Verify the flag persisted in `~/.codex/config.toml` under `[notice]`. +- **Shutdown completion handling** + - Trigger quit and verify the UI waits for shutdown completion (via logs/visible delay if any). + - Run `/new` and verify shutdown completion is suppressed (app stays running). + +### Core persistence unit test (codex-core) + +Add a blocking config edit test analogous to existing notice tests: + +- seed a config file with `[notice]\nexisting = "value"\n` +- apply the edit +- assert the file preserves `existing` and appends `hide_exit_confirmation_prompt = true` + +## Implementation notes (code documentation) + +Add short doc comments in code where they prevent re-introducing the same regressions: + +- On `Notice::hide_exit_confirmation_prompt`: + - explain that it suppresses the `Ctrl+C`/`Ctrl+D` quit confirmation prompt. +- Near the `Ctrl+C` handler: + - explicitly document the priority order: modal dismissal → interrupt/cancel → clear draft → quit + flow. +- On the “review cancellation” predicate: + - explicitly mention that review must count as cancellable work, to prevent quitting mid-review. + +These comments should be brief and focused on intent (the “why”), not on re-stating code. + +## Open questions / decisions to confirm + +1. **Should quit always be shutdown-first?** + - Recommendation: yes for all quit paths (keys and slash commands), keeping “exit immediately” + only as a logged fallback for shutdown hangs/timeouts. +2. **Should the quit prompt appear on the first `Ctrl+C` press or retain the historical “two-step + hint” behavior?** + - Recommendation: the prompt replaces the two-step hint for the idle-empty case (clearer and + more explicit). +3. **Should `Ctrl+D` show the same prompt as `Ctrl+C`?** + - Recommendation: yes, but only when composer is empty and no modal is open. + +## Rollout considerations + +- Default behavior should be safe: prompt enabled unless explicitly disabled. +- The “don’t ask again” option must persist reliably (core config edit test). +- Ensure we do not prompt during active work: the review cancellation fix is a prerequisite for + the prompt to feel correct rather than annoying. From 3b8798522910da203003f694ddd0a1c2c3e05fda Mon Sep 17 00:00:00 2001 From: Josh McKinney Date: Wed, 7 Jan 2026 20:29:07 -0800 Subject: [PATCH 2/7] feat(exit): route user quits through AppEvent::Exit Represent user-initiated quit as AppEvent::Exit(ExitMode) so the app event loop owns shutdown sequencing. This keeps all quit triggers consistent: the UI requests a shutdown-first quit, continues rendering while shutdown runs, and only terminates the UI loop once shutdown completes. --- codex-rs/core/src/config/edit.rs | 43 +++++++ codex-rs/core/src/config/types.rs | 2 + codex-rs/tui/src/app.rs | 30 ++++- codex-rs/tui/src/app_event.rs | 20 ++- codex-rs/tui/src/bottom_pane/chat_composer.rs | 4 +- codex-rs/tui/src/chatwidget.rs | 121 +++++++++++++++++- codex-rs/tui/src/chatwidget/agent.rs | 1 + ...idget__tests__exit_confirmation_popup.snap | 13 ++ codex-rs/tui/src/chatwidget/tests.rs | 41 +++++- codex-rs/tui/tests/fixtures/oss-story.jsonl | 2 +- codex-rs/tui2/src/app.rs | 30 ++++- codex-rs/tui2/src/app_event.rs | 20 ++- .../tui2/src/bottom_pane/chat_composer.rs | 4 +- codex-rs/tui2/src/bottom_pane/mod.rs | 5 + codex-rs/tui2/src/chatwidget.rs | 121 +++++++++++++++++- codex-rs/tui2/src/chatwidget/agent.rs | 1 + ...idget__tests__exit_confirmation_popup.snap | 13 ++ codex-rs/tui2/src/chatwidget/tests.rs | 41 +++++- codex-rs/tui2/tests/fixtures/oss-story.jsonl | 2 +- docs/config.md | 6 + docs/ctrl-c-quit-history.md | 3 +- docs/exit-confirmation-prompt-design.md | 59 +++++---- 22 files changed, 515 insertions(+), 67 deletions(-) create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exit_confirmation_popup.snap create mode 100644 codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__exit_confirmation_popup.snap diff --git a/codex-rs/core/src/config/edit.rs b/codex-rs/core/src/config/edit.rs index a24c09e36b7..e443443cd98 100644 --- a/codex-rs/core/src/config/edit.rs +++ b/codex-rs/core/src/config/edit.rs @@ -28,6 +28,8 @@ pub enum ConfigEdit { SetNoticeHideWorldWritableWarning(bool), /// Toggle the rate limit model nudge acknowledgement flag. SetNoticeHideRateLimitModelNudge(bool), + /// Toggle the exit confirmation prompt acknowledgement flag. + SetNoticeHideExitConfirmationPrompt(bool), /// Toggle the Windows onboarding acknowledgement flag. SetWindowsWslSetupAcknowledged(bool), /// Toggle the model migration prompt acknowledgement flag. @@ -280,6 +282,11 @@ impl ConfigDocument { &[Notice::TABLE_KEY, "hide_rate_limit_model_nudge"], value(*acknowledged), )), + ConfigEdit::SetNoticeHideExitConfirmationPrompt(acknowledged) => Ok(self.write_value( + Scope::Global, + &[Notice::TABLE_KEY, "hide_exit_confirmation_prompt"], + value(*acknowledged), + )), ConfigEdit::SetNoticeHideModelMigrationPrompt(migration_config, acknowledged) => { Ok(self.write_value( Scope::Global, @@ -614,6 +621,14 @@ impl ConfigEditsBuilder { self } + pub fn set_hide_exit_confirmation_prompt(mut self, acknowledged: bool) -> Self { + self.edits + .push(ConfigEdit::SetNoticeHideExitConfirmationPrompt( + acknowledged, + )); + self + } + pub fn set_hide_model_migration_prompt(mut self, model: &str, acknowledged: bool) -> Self { self.edits .push(ConfigEdit::SetNoticeHideModelMigrationPrompt( @@ -1004,6 +1019,34 @@ existing = "value" let expected = r#"[notice] existing = "value" hide_rate_limit_model_nudge = true +"#; + assert_eq!(contents, expected); + } + + #[test] + fn blocking_set_hide_exit_confirmation_prompt_preserves_table() { + let tmp = tempdir().expect("tmpdir"); + let codex_home = tmp.path(); + std::fs::write( + codex_home.join(CONFIG_TOML_FILE), + r#"[notice] +existing = "value" +"#, + ) + .expect("seed"); + + apply_blocking( + codex_home, + None, + &[ConfigEdit::SetNoticeHideExitConfirmationPrompt(true)], + ) + .expect("persist"); + + let contents = + std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); + let expected = r#"[notice] +existing = "value" +hide_exit_confirmation_prompt = true "#; assert_eq!(contents, expected); } diff --git a/codex-rs/core/src/config/types.rs b/codex-rs/core/src/config/types.rs index c57e550a7a8..a0bfad7a433 100644 --- a/codex-rs/core/src/config/types.rs +++ b/codex-rs/core/src/config/types.rs @@ -565,6 +565,8 @@ pub struct Notice { pub hide_world_writable_warning: Option, /// Tracks whether the user opted out of the rate limit model switch reminder. pub hide_rate_limit_model_nudge: Option, + /// Tracks whether the exit confirmation prompt should be suppressed. + pub hide_exit_confirmation_prompt: Option, /// Tracks whether the user has seen the model migration prompt pub hide_gpt5_1_migration_prompt: Option, /// Tracks whether the user has seen the gpt-5.1-codex-max migration prompt diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index d8df24e6556..d2ca359147c 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -1,5 +1,6 @@ use crate::app_backtrack::BacktrackState; use crate::app_event::AppEvent; +use crate::app_event::ExitMode; #[cfg(target_os = "windows")] use crate::app_event::WindowsSandboxEnableMode; #[cfg(target_os = "windows")] @@ -855,9 +856,14 @@ impl App { } self.chat_widget.handle_codex_event(event); } - AppEvent::ExitRequest => { - return Ok(AppRunControl::Exit(ExitReason::UserRequested)); - } + AppEvent::Exit(mode) => match mode { + ExitMode::ShutdownFirst => { + self.chat_widget.submit_op(Op::Shutdown); + } + ExitMode::Immediate => { + return Ok(AppRunControl::Exit(ExitReason::UserRequested)); + } + }, AppEvent::FatalExitRequest(message) => { return Ok(AppRunControl::Exit(ExitReason::Fatal(message))); } @@ -1221,6 +1227,9 @@ impl App { AppEvent::UpdateRateLimitSwitchPromptHidden(hidden) => { self.chat_widget.set_rate_limit_switch_prompt_hidden(hidden); } + AppEvent::UpdateExitConfirmationPromptHidden(hidden) => { + self.chat_widget.set_exit_confirmation_prompt_hidden(hidden); + } AppEvent::PersistFullAccessWarningAcknowledged => { if let Err(err) = ConfigEditsBuilder::new(&self.config.codex_home) .set_hide_full_access_warning(true) @@ -1266,6 +1275,21 @@ impl App { )); } } + AppEvent::PersistExitConfirmationPromptHidden => { + if let Err(err) = ConfigEditsBuilder::new(&self.config.codex_home) + .set_hide_exit_confirmation_prompt(true) + .apply() + .await + { + tracing::error!( + error = %err, + "failed to persist exit confirmation prompt preference" + ); + self.chat_widget.add_error_message(format!( + "Failed to save exit confirmation preference: {err}" + )); + } + } AppEvent::PersistModelMigrationPromptAcknowledged { from_model, to_model, diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 42f96d9098b..066d96dbc9b 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -41,8 +41,13 @@ pub(crate) enum AppEvent { /// Open the fork picker inside the running TUI session. OpenForkPicker, - /// Request to exit the application gracefully. - ExitRequest, + /// Request to exit the application. + /// + /// Use `ShutdownFirst` for user-initiated quits so core cleanup runs and the + /// UI exits only after `ShutdownComplete`. `Immediate` is a last-resort + /// escape hatch that skips shutdown and may drop in-flight work (e.g., + /// background tasks, rollout flush, or child process cleanup). + Exit(ExitMode), /// Request to exit the application due to a fatal error. FatalExitRequest(String), @@ -215,6 +220,17 @@ pub(crate) enum AppEvent { LaunchExternalEditor, } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum ExitMode { + /// Shutdown core and exit after completion. + ShutdownFirst, + /// Exit the UI loop immediately without waiting for shutdown. + /// + /// This skips `Op::Shutdown`, so any in-flight work may be dropped and + /// cleanup that normally runs before `ShutdownComplete` can be missed. + Immediate, +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum FeedbackCategory { BadResult, diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 27e191d8ef6..b73dd364e5f 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -108,6 +108,7 @@ use codex_protocol::custom_prompts::PROMPTS_CMD_PREFIX; use codex_protocol::models::local_image_label_text; use crate::app_event::AppEvent; +use crate::app_event::ExitMode; use crate::app_event_sender::AppEventSender; use crate::bottom_pane::textarea::TextArea; use crate::bottom_pane::textarea::TextAreaState; @@ -1476,7 +1477,8 @@ impl ChatComposer { kind: KeyEventKind::Press, .. } if self.is_empty() => { - self.app_event_tx.send(AppEvent::ExitRequest); + self.app_event_tx + .send(AppEvent::Exit(ExitMode::ShutdownFirst { confirm: true })); (InputResult::None, true) } // ------------------------------------------------------------- diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 97bab98756c..eefebea2328 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -107,6 +107,7 @@ use tokio::task::JoinHandle; use tracing::debug; use crate::app_event::AppEvent; +use crate::app_event::ExitMode; #[cfg(target_os = "windows")] use crate::app_event::WindowsSandboxEnableMode; use crate::app_event::WindowsSandboxFallbackReason; @@ -1142,7 +1143,7 @@ impl ChatWidget { } fn on_shutdown_complete(&mut self) { - self.request_exit(); + self.request_immediate_exit(); } fn on_turn_diff(&mut self, unified_diff: String) { @@ -1911,7 +1912,7 @@ impl ChatWidget { self.open_experimental_popup(); } SlashCommand::Quit | SlashCommand::Exit => { - self.request_exit(); + self.request_quit_without_confirmation(); } SlashCommand::Logout => { if let Err(e) = codex_core::auth::logout( @@ -1920,7 +1921,7 @@ impl ChatWidget { ) { tracing::error!("failed to logout: {e}"); } - self.request_exit(); + self.request_quit_without_confirmation(); } // SlashCommand::Undo => { // self.app_event_tx.send(AppEvent::CodexOp(Op::Undo)); @@ -2397,8 +2398,29 @@ impl ChatWidget { } } - fn request_exit(&self) { - self.app_event_tx.send(AppEvent::ExitRequest); + /// Exit the UI immediately without waiting for shutdown. + /// + /// Prefer [`Self::request_quit_with_confirmation`] or + /// [`Self::request_quit_without_confirmation`] for user-initiated exits; + /// this is mainly a fallback for shutdown completion or emergency exits. + fn request_immediate_exit(&self) { + self.app_event_tx.send(AppEvent::Exit(ExitMode::Immediate)); + } + + /// Request a shutdown-first quit that shows the confirmation prompt. + fn request_quit_with_confirmation(&self) { + self.app_event_tx + .send(AppEvent::Exit(ExitMode::ShutdownFirst { confirm: true })); + } + + /// Request a shutdown-first quit that skips the confirmation prompt. + /// + /// Use this for explicit quit commands (`/quit`, `/exit`, `/logout`) when + /// we want shutdown+exit without prompting. Slash commands are less likely + /// to be accidental, so the intent to quit is clear. + fn request_quit_without_confirmation(&self) { + self.app_event_tx + .send(AppEvent::Exit(ExitMode::ShutdownFirst { confirm: false })); } fn request_redraw(&mut self) { @@ -3239,6 +3261,73 @@ impl ChatWidget { None } + /// Show the shutdown-first quit confirmation prompt. + /// + /// This uses a selection view with explicit shutdown actions and an option + /// to persist the "don't ask again" notice. + pub(crate) fn open_exit_confirmation_prompt(&mut self) { + if !self.bottom_pane.can_launch_external_editor() { + return; + } + + let mut header_children: Vec> = Vec::new(); + header_children.push(Box::new(Line::from("Quit Codex?").bold())); + let info_line = Line::from(vec![ + "You pressed ".into(), + "Ctrl+C".bold(), + " or ".into(), + "Ctrl+D".bold(), + ". Quitting will shut down the current session and exit Codex.".into(), + ]); + header_children.push(Box::new( + Paragraph::new(vec![info_line]).wrap(Wrap { trim: false }), + )); + let header = ColumnRenderable::with(header_children); + + let quit_actions: Vec = vec![Box::new(|tx| { + // Shutdown-first quit. + tx.send(AppEvent::CodexOp(Op::Shutdown)); + })]; + + let quit_and_remember_actions: Vec = vec![Box::new(|tx| { + // Persist the notice and then shut down. + tx.send(AppEvent::UpdateExitConfirmationPromptHidden(true)); + tx.send(AppEvent::PersistExitConfirmationPromptHidden); + tx.send(AppEvent::CodexOp(Op::Shutdown)); + })]; + + let items = vec![ + SelectionItem { + name: "Yes, quit Codex".to_string(), + description: Some("Shut down the current session and exit.".to_string()), + actions: quit_actions, + dismiss_on_select: true, + ..Default::default() + }, + SelectionItem { + name: "Yes, and don't ask again".to_string(), + description: Some("Quit now and skip this prompt next time.".to_string()), + actions: quit_and_remember_actions, + dismiss_on_select: true, + ..Default::default() + }, + SelectionItem { + name: "No, stay in Codex".to_string(), + description: Some("Return to the current session.".to_string()), + actions: Vec::new(), + dismiss_on_select: true, + ..Default::default() + }, + ]; + + self.bottom_pane.show_selection_view(SelectionViewParams { + footer_hint: Some(standard_popup_hint_line()), + items, + header: Box::new(header), + ..Default::default() + }); + } + pub(crate) fn open_full_access_confirmation(&mut self, preset: ApprovalPreset) { let approval = preset.approval; let sandbox = preset.sandbox; @@ -3738,6 +3827,19 @@ impl ChatWidget { } } + /// Update the in-memory hide flag for the exit confirmation prompt. + pub(crate) fn set_exit_confirmation_prompt_hidden(&mut self, hidden: bool) { + self.config.notices.hide_exit_confirmation_prompt = Some(hidden); + } + + /// Return whether the exit confirmation prompt should be suppressed. + pub(crate) fn exit_confirmation_prompt_hidden(&self) -> bool { + self.config + .notices + .hide_exit_confirmation_prompt + .unwrap_or(false) + } + #[cfg_attr(not(target_os = "windows"), allow(dead_code))] pub(crate) fn world_writable_warning_hidden(&self) -> bool { self.config @@ -3791,13 +3893,18 @@ impl ChatWidget { return; } - if self.bottom_pane.is_task_running() { + if self.is_cancellable_work_active() { self.bottom_pane.show_ctrl_c_quit_hint(); self.submit_op(Op::Interrupt); return; } - self.submit_op(Op::Shutdown); + self.request_quit_with_confirmation(); + } + + // Review mode counts as cancellable work so Ctrl+C interrupts instead of quitting. + fn is_cancellable_work_active(&self) -> bool { + self.bottom_pane.is_task_running() || self.is_review_mode } pub(crate) fn composer_is_empty(&self) -> bool { diff --git a/codex-rs/tui/src/chatwidget/agent.rs b/codex-rs/tui/src/chatwidget/agent.rs index 21ed92d0ee6..b24233c2af8 100644 --- a/codex-rs/tui/src/chatwidget/agent.rs +++ b/codex-rs/tui/src/chatwidget/agent.rs @@ -38,6 +38,7 @@ pub(crate) fn spawn_agent( msg: EventMsg::Error(err.to_error_event(None)), })); app_event_tx_clone.send(AppEvent::FatalExitRequest(message)); + tracing::error!("failed to initialize codex: {err}"); return; } }; diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exit_confirmation_popup.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exit_confirmation_popup.snap new file mode 100644 index 00000000000..dc5105a6213 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exit_confirmation_popup.snap @@ -0,0 +1,13 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: popup +--- + Quit Codex? + You pressed Ctrl+C or Ctrl+D. Quitting will shut down the current session + and exit Codex. + +› 1. Yes, quit Codex Shut down the current session and exit. + 2. Yes, and don't ask again Quit now and skip this prompt next time. + 3. No, stay in Codex Return to the current session. + + Press enter to confirm or esc to go back diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 0c5f6b852d4..54e580773f8 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -6,6 +6,7 @@ use super::*; use crate::app_event::AppEvent; +use crate::app_event::ExitMode; use crate::app_event_sender::AppEventSender; use crate::test_backend::VT100Backend; use crate::tui::FrameRequester; @@ -1108,14 +1109,24 @@ async fn streaming_final_answer_keeps_task_running_state() { #[tokio::test] async fn ctrl_c_shutdown_ignores_caps_lock() { - let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(None).await; + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; chat.handle_key_event(KeyEvent::new(KeyCode::Char('C'), KeyModifiers::CONTROL)); - match op_rx.try_recv() { - Ok(Op::Shutdown) => {} - other => panic!("expected Op::Shutdown, got {other:?}"), - } + assert_matches!( + rx.try_recv(), + Ok(AppEvent::Exit(ExitMode::ShutdownFirst { confirm: true })) + ); +} + +#[tokio::test] +async fn ctrl_d_with_modal_open_does_not_quit() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; + + chat.open_approvals_popup(); + chat.handle_key_event(KeyEvent::new(KeyCode::Char('d'), KeyModifiers::CONTROL)); + + assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); } #[tokio::test] @@ -1525,7 +1536,10 @@ async fn slash_quit_requests_exit() { chat.dispatch_command(SlashCommand::Quit); - assert_matches!(rx.try_recv(), Ok(AppEvent::ExitRequest)); + assert_matches!( + rx.try_recv(), + Ok(AppEvent::Exit(ExitMode::ShutdownFirst { confirm: false })) + ); } #[tokio::test] @@ -1534,7 +1548,10 @@ async fn slash_exit_requests_exit() { chat.dispatch_command(SlashCommand::Exit); - assert_matches!(rx.try_recv(), Ok(AppEvent::ExitRequest)); + assert_matches!( + rx.try_recv(), + Ok(AppEvent::Exit(ExitMode::ShutdownFirst { confirm: false })) + ); } #[tokio::test] @@ -1988,6 +2005,16 @@ fn render_bottom_popup(chat: &ChatWidget, width: u16) -> String { lines.join("\n") } +#[tokio::test] +async fn exit_confirmation_popup_snapshot() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; + + chat.open_exit_confirmation_prompt(); + + let popup = render_bottom_popup(&chat, 80); + assert_snapshot!("exit_confirmation_popup", popup); +} + #[tokio::test] async fn experimental_features_popup_snapshot() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; diff --git a/codex-rs/tui/tests/fixtures/oss-story.jsonl b/codex-rs/tui/tests/fixtures/oss-story.jsonl index 4db9e572fd2..72d0fc40f49 100644 --- a/codex-rs/tui/tests/fixtures/oss-story.jsonl +++ b/codex-rs/tui/tests/fixtures/oss-story.jsonl @@ -8037,5 +8037,5 @@ {"ts":"2025-08-10T03:48:49.926Z","dir":"to_tui","kind":"log_line","line":"[INFO codex_core::codex] Shutting down Codex instance"} {"ts":"2025-08-10T03:48:49.927Z","dir":"to_tui","kind":"log_line","line":"[INFO codex_core::codex] Aborting existing session"} {"ts":"2025-08-10T03:48:49.927Z","dir":"to_tui","kind":"codex_event","payload":{"id":"7","msg":{"type":"shutdown_complete"}}} -{"ts":"2025-08-10T03:48:49.927Z","dir":"to_tui","kind":"app_event","variant":"ExitRequest"} +{"ts":"2025-08-10T03:48:49.927Z","dir":"to_tui","kind":"app_event","variant":"Exit"} {"ts":"2025-08-10T03:48:49.927Z","dir":"meta","kind":"session_end"} diff --git a/codex-rs/tui2/src/app.rs b/codex-rs/tui2/src/app.rs index afedcded242..2b71db3e862 100644 --- a/codex-rs/tui2/src/app.rs +++ b/codex-rs/tui2/src/app.rs @@ -1,5 +1,6 @@ use crate::app_backtrack::BacktrackState; use crate::app_event::AppEvent; +use crate::app_event::ExitMode; #[cfg(target_os = "windows")] use crate::app_event::WindowsSandboxEnableMode; #[cfg(target_os = "windows")] @@ -1642,9 +1643,14 @@ impl App { } self.chat_widget.handle_codex_event(event); } - AppEvent::ExitRequest => { - return Ok(AppRunControl::Exit(ExitReason::UserRequested)); - } + AppEvent::Exit(mode) => match mode { + ExitMode::ShutdownFirst => { + self.chat_widget.submit_op(Op::Shutdown); + } + ExitMode::Immediate => { + return Ok(AppRunControl::Exit(ExitReason::UserRequested)); + } + }, AppEvent::FatalExitRequest(message) => { return Ok(AppRunControl::Exit(ExitReason::Fatal(message))); } @@ -1967,6 +1973,9 @@ impl App { AppEvent::UpdateRateLimitSwitchPromptHidden(hidden) => { self.chat_widget.set_rate_limit_switch_prompt_hidden(hidden); } + AppEvent::UpdateExitConfirmationPromptHidden(hidden) => { + self.chat_widget.set_exit_confirmation_prompt_hidden(hidden); + } AppEvent::PersistFullAccessWarningAcknowledged => { if let Err(err) = ConfigEditsBuilder::new(&self.config.codex_home) .set_hide_full_access_warning(true) @@ -2012,6 +2021,21 @@ impl App { )); } } + AppEvent::PersistExitConfirmationPromptHidden => { + if let Err(err) = ConfigEditsBuilder::new(&self.config.codex_home) + .set_hide_exit_confirmation_prompt(true) + .apply() + .await + { + tracing::error!( + error = %err, + "failed to persist exit confirmation prompt preference" + ); + self.chat_widget.add_error_message(format!( + "Failed to save exit confirmation preference: {err}" + )); + } + } AppEvent::PersistModelMigrationPromptAcknowledged { from_model, to_model, diff --git a/codex-rs/tui2/src/app_event.rs b/codex-rs/tui2/src/app_event.rs index 59cc18047bd..dfea64835a6 100644 --- a/codex-rs/tui2/src/app_event.rs +++ b/codex-rs/tui2/src/app_event.rs @@ -40,8 +40,13 @@ pub(crate) enum AppEvent { /// Open the fork picker inside the running TUI session. OpenForkPicker, - /// Request to exit the application gracefully. - ExitRequest, + /// Request to exit the application. + /// + /// Use `ShutdownFirst` for user-initiated quits so core cleanup runs and the + /// UI exits only after `ShutdownComplete`. `Immediate` is a last-resort + /// escape hatch that skips shutdown and may drop in-flight work (e.g., + /// background tasks, rollout flush, or child process cleanup). + Exit(ExitMode), /// Request to exit the application due to a fatal error. FatalExitRequest(String), @@ -206,6 +211,17 @@ pub(crate) enum AppEvent { }, } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum ExitMode { + /// Shutdown core and exit after completion. + ShutdownFirst, + /// Exit the UI loop immediately without waiting for shutdown. + /// + /// This skips `Op::Shutdown`, so any in-flight work may be dropped and + /// cleanup that normally runs before `ShutdownComplete` can be missed. + Immediate, +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum FeedbackCategory { BadResult, diff --git a/codex-rs/tui2/src/bottom_pane/chat_composer.rs b/codex-rs/tui2/src/bottom_pane/chat_composer.rs index df8016a168c..36f3ae811bb 100644 --- a/codex-rs/tui2/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui2/src/bottom_pane/chat_composer.rs @@ -110,6 +110,7 @@ use codex_protocol::custom_prompts::PROMPTS_CMD_PREFIX; use codex_protocol::models::local_image_label_text; use crate::app_event::AppEvent; +use crate::app_event::ExitMode; use crate::app_event_sender::AppEventSender; use crate::bottom_pane::textarea::TextArea; use crate::bottom_pane::textarea::TextAreaState; @@ -1409,7 +1410,8 @@ impl ChatComposer { kind: KeyEventKind::Press, .. } if self.is_empty() => { - self.app_event_tx.send(AppEvent::ExitRequest); + self.app_event_tx + .send(AppEvent::Exit(ExitMode::ShutdownFirst { confirm: true })); (InputResult::None, true) } // ------------------------------------------------------------- diff --git a/codex-rs/tui2/src/bottom_pane/mod.rs b/codex-rs/tui2/src/bottom_pane/mod.rs index bbf5e8849d9..fbc4d95ded4 100644 --- a/codex-rs/tui2/src/bottom_pane/mod.rs +++ b/codex-rs/tui2/src/bottom_pane/mod.rs @@ -450,6 +450,11 @@ impl BottomPane { !self.is_task_running && self.view_stack.is_empty() && !self.composer.popup_active() } + /// Return true when no popups or modal views are active, regardless of task state. + pub(crate) fn can_launch_external_editor(&self) -> bool { + self.view_stack.is_empty() && !self.composer.popup_active() + } + pub(crate) fn show_view(&mut self, view: Box) { self.push_view(view); } diff --git a/codex-rs/tui2/src/chatwidget.rs b/codex-rs/tui2/src/chatwidget.rs index 02855551a02..1fe09e62a42 100644 --- a/codex-rs/tui2/src/chatwidget.rs +++ b/codex-rs/tui2/src/chatwidget.rs @@ -106,6 +106,7 @@ use tokio::task::JoinHandle; use tracing::debug; use crate::app_event::AppEvent; +use crate::app_event::ExitMode; #[cfg(target_os = "windows")] use crate::app_event::WindowsSandboxEnableMode; use crate::app_event::WindowsSandboxFallbackReason; @@ -997,7 +998,7 @@ impl ChatWidget { } fn on_shutdown_complete(&mut self) { - self.request_exit(); + self.request_immediate_exit(); } fn on_turn_diff(&mut self, unified_diff: String) { @@ -1729,7 +1730,7 @@ impl ChatWidget { }; } SlashCommand::Quit | SlashCommand::Exit => { - self.request_exit(); + self.request_quit_without_confirmation(); } SlashCommand::Logout => { if let Err(e) = codex_core::auth::logout( @@ -1738,7 +1739,7 @@ impl ChatWidget { ) { tracing::error!("failed to logout: {e}"); } - self.request_exit(); + self.request_quit_without_confirmation(); } // SlashCommand::Undo => { // self.app_event_tx.send(AppEvent::CodexOp(Op::Undo)); @@ -2175,8 +2176,29 @@ impl ChatWidget { self.needs_final_message_separator = false; } - fn request_exit(&self) { - self.app_event_tx.send(AppEvent::ExitRequest); + /// Exit the UI immediately without waiting for shutdown. + /// + /// Prefer [`Self::request_quit_with_confirmation`] or + /// [`Self::request_quit_without_confirmation`] for user-initiated exits; + /// this is mainly a fallback for shutdown completion or emergency exits. + fn request_immediate_exit(&self) { + self.app_event_tx.send(AppEvent::Exit(ExitMode::Immediate)); + } + + /// Request a shutdown-first quit that shows the confirmation prompt. + fn request_quit_with_confirmation(&self) { + self.app_event_tx + .send(AppEvent::Exit(ExitMode::ShutdownFirst { confirm: true })); + } + + /// Request a shutdown-first quit that skips the confirmation prompt. + /// + /// Use this for explicit quit commands (`/quit`, `/exit`, `/logout`) when + /// we want shutdown+exit without prompting. Slash commands are less likely + /// to be accidental, so the intent to quit is clear. + fn request_quit_without_confirmation(&self) { + self.app_event_tx + .send(AppEvent::Exit(ExitMode::ShutdownFirst { confirm: false })); } fn request_redraw(&mut self) { @@ -2942,6 +2964,73 @@ impl ChatWidget { None } + /// Show the shutdown-first quit confirmation prompt. + /// + /// This uses a selection view with explicit shutdown actions and an option + /// to persist the "don't ask again" notice. + pub(crate) fn open_exit_confirmation_prompt(&mut self) { + if !self.bottom_pane.can_launch_external_editor() { + return; + } + + let mut header_children: Vec> = Vec::new(); + header_children.push(Box::new(Line::from("Quit Codex?").bold())); + let info_line = Line::from(vec![ + "You pressed ".into(), + "Ctrl+C".bold(), + " or ".into(), + "Ctrl+D".bold(), + ". Quitting will shut down the current session and exit Codex.".into(), + ]); + header_children.push(Box::new( + Paragraph::new(vec![info_line]).wrap(Wrap { trim: false }), + )); + let header = ColumnRenderable::with(header_children); + + let quit_actions: Vec = vec![Box::new(|tx| { + // Shutdown-first quit. + tx.send(AppEvent::CodexOp(Op::Shutdown)); + })]; + + let quit_and_remember_actions: Vec = vec![Box::new(|tx| { + // Persist the notice and then shut down. + tx.send(AppEvent::UpdateExitConfirmationPromptHidden(true)); + tx.send(AppEvent::PersistExitConfirmationPromptHidden); + tx.send(AppEvent::CodexOp(Op::Shutdown)); + })]; + + let items = vec![ + SelectionItem { + name: "Yes, quit Codex".to_string(), + description: Some("Shut down the current session and exit.".to_string()), + actions: quit_actions, + dismiss_on_select: true, + ..Default::default() + }, + SelectionItem { + name: "Yes, and don't ask again".to_string(), + description: Some("Quit now and skip this prompt next time.".to_string()), + actions: quit_and_remember_actions, + dismiss_on_select: true, + ..Default::default() + }, + SelectionItem { + name: "No, stay in Codex".to_string(), + description: Some("Return to the current session.".to_string()), + actions: Vec::new(), + dismiss_on_select: true, + ..Default::default() + }, + ]; + + self.bottom_pane.show_selection_view(SelectionViewParams { + footer_hint: Some(standard_popup_hint_line()), + items, + header: Box::new(header), + ..Default::default() + }); + } + pub(crate) fn open_full_access_confirmation(&mut self, preset: ApprovalPreset) { let approval = preset.approval; let sandbox = preset.sandbox; @@ -3441,6 +3530,19 @@ impl ChatWidget { } } + /// Update the in-memory hide flag for the exit confirmation prompt. + pub(crate) fn set_exit_confirmation_prompt_hidden(&mut self, hidden: bool) { + self.config.notices.hide_exit_confirmation_prompt = Some(hidden); + } + + /// Return whether the exit confirmation prompt should be suppressed. + pub(crate) fn exit_confirmation_prompt_hidden(&self) -> bool { + self.config + .notices + .hide_exit_confirmation_prompt + .unwrap_or(false) + } + #[cfg_attr(not(target_os = "windows"), allow(dead_code))] pub(crate) fn world_writable_warning_hidden(&self) -> bool { self.config @@ -3494,13 +3596,18 @@ impl ChatWidget { return; } - if self.bottom_pane.is_task_running() { + if self.is_cancellable_work_active() { self.bottom_pane.show_ctrl_c_quit_hint(); self.submit_op(Op::Interrupt); return; } - self.submit_op(Op::Shutdown); + self.request_quit_with_confirmation(); + } + + // Review mode counts as cancellable work so Ctrl+C interrupts instead of quitting. + fn is_cancellable_work_active(&self) -> bool { + self.bottom_pane.is_task_running() || self.is_review_mode } pub(crate) fn composer_is_empty(&self) -> bool { diff --git a/codex-rs/tui2/src/chatwidget/agent.rs b/codex-rs/tui2/src/chatwidget/agent.rs index 24c40365302..0cfda7f3429 100644 --- a/codex-rs/tui2/src/chatwidget/agent.rs +++ b/codex-rs/tui2/src/chatwidget/agent.rs @@ -38,6 +38,7 @@ pub(crate) fn spawn_agent( msg: EventMsg::Error(err.to_error_event(None)), })); app_event_tx_clone.send(AppEvent::FatalExitRequest(message)); + tracing::error!("failed to initialize codex: {err}"); return; } }; diff --git a/codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__exit_confirmation_popup.snap b/codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__exit_confirmation_popup.snap new file mode 100644 index 00000000000..bd2bfac0d5d --- /dev/null +++ b/codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__exit_confirmation_popup.snap @@ -0,0 +1,13 @@ +--- +source: tui2/src/chatwidget/tests.rs +expression: popup +--- + Quit Codex? + You pressed Ctrl+C or Ctrl+D. Quitting will shut down the current session + and exit Codex. + +› 1. Yes, quit Codex Shut down the current session and exit. + 2. Yes, and don't ask again Quit now and skip this prompt next time. + 3. No, stay in Codex Return to the current session. + + Press enter to confirm or esc to go back diff --git a/codex-rs/tui2/src/chatwidget/tests.rs b/codex-rs/tui2/src/chatwidget/tests.rs index 41b27600091..c8de28b4f13 100644 --- a/codex-rs/tui2/src/chatwidget/tests.rs +++ b/codex-rs/tui2/src/chatwidget/tests.rs @@ -6,6 +6,7 @@ use super::*; use crate::app_event::AppEvent; +use crate::app_event::ExitMode; use crate::app_event_sender::AppEventSender; use crate::test_backend::VT100Backend; use crate::tui::FrameRequester; @@ -1059,14 +1060,24 @@ async fn streaming_final_answer_keeps_task_running_state() { #[tokio::test] async fn ctrl_c_shutdown_ignores_caps_lock() { - let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(None).await; + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; chat.handle_key_event(KeyEvent::new(KeyCode::Char('C'), KeyModifiers::CONTROL)); - match op_rx.try_recv() { - Ok(Op::Shutdown) => {} - other => panic!("expected Op::Shutdown, got {other:?}"), - } + assert_matches!( + rx.try_recv(), + Ok(AppEvent::Exit(ExitMode::ShutdownFirst { confirm: true })) + ); +} + +#[tokio::test] +async fn ctrl_d_with_modal_open_does_not_quit() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; + + chat.open_approvals_popup(); + chat.handle_key_event(KeyEvent::new(KeyCode::Char('d'), KeyModifiers::CONTROL)); + + assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); } #[tokio::test] @@ -1291,7 +1302,10 @@ async fn slash_quit_requests_exit() { chat.dispatch_command(SlashCommand::Quit); - assert_matches!(rx.try_recv(), Ok(AppEvent::ExitRequest)); + assert_matches!( + rx.try_recv(), + Ok(AppEvent::Exit(ExitMode::ShutdownFirst { confirm: false })) + ); } #[tokio::test] @@ -1300,7 +1314,10 @@ async fn slash_exit_requests_exit() { chat.dispatch_command(SlashCommand::Exit); - assert_matches!(rx.try_recv(), Ok(AppEvent::ExitRequest)); + assert_matches!( + rx.try_recv(), + Ok(AppEvent::Exit(ExitMode::ShutdownFirst { confirm: false })) + ); } #[tokio::test] @@ -1754,6 +1771,16 @@ fn render_bottom_popup(chat: &ChatWidget, width: u16) -> String { lines.join("\n") } +#[tokio::test] +async fn exit_confirmation_popup_snapshot() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; + + chat.open_exit_confirmation_prompt(); + + let popup = render_bottom_popup(&chat, 80); + assert_snapshot!("exit_confirmation_popup", popup); +} + #[tokio::test] async fn model_selection_popup_snapshot() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5-codex")).await; diff --git a/codex-rs/tui2/tests/fixtures/oss-story.jsonl b/codex-rs/tui2/tests/fixtures/oss-story.jsonl index 4db9e572fd2..72d0fc40f49 100644 --- a/codex-rs/tui2/tests/fixtures/oss-story.jsonl +++ b/codex-rs/tui2/tests/fixtures/oss-story.jsonl @@ -8037,5 +8037,5 @@ {"ts":"2025-08-10T03:48:49.926Z","dir":"to_tui","kind":"log_line","line":"[INFO codex_core::codex] Shutting down Codex instance"} {"ts":"2025-08-10T03:48:49.927Z","dir":"to_tui","kind":"log_line","line":"[INFO codex_core::codex] Aborting existing session"} {"ts":"2025-08-10T03:48:49.927Z","dir":"to_tui","kind":"codex_event","payload":{"id":"7","msg":{"type":"shutdown_complete"}}} -{"ts":"2025-08-10T03:48:49.927Z","dir":"to_tui","kind":"app_event","variant":"ExitRequest"} +{"ts":"2025-08-10T03:48:49.927Z","dir":"to_tui","kind":"app_event","variant":"Exit"} {"ts":"2025-08-10T03:48:49.927Z","dir":"meta","kind":"session_end"} diff --git a/docs/config.md b/docs/config.md index 0b5fecf6e7a..87945b25a1a 100644 --- a/docs/config.md +++ b/docs/config.md @@ -21,3 +21,9 @@ Codex can run a notification hook when the agent finishes a turn. See the config ## JSON Schema The generated JSON Schema for `config.toml` lives at `codex-rs/core/config.schema.json`. + +## Notices + +Codex stores "do not show again" flags for some UI prompts under the `[notice]` table. + +Ctrl+C/Ctrl+D quitting uses a ~1 second double-press hint (`ctrl + c again to quit`). diff --git a/docs/ctrl-c-quit-history.md b/docs/ctrl-c-quit-history.md index bab2392af12..5f1702e5ffe 100644 --- a/docs/ctrl-c-quit-history.md +++ b/docs/ctrl-c-quit-history.md @@ -27,7 +27,8 @@ As of HEAD: The main decision point for `Ctrl+C` is `ChatWidget::on_ctrl_c` in `codex-rs/tui/src/chatwidget.rs:3309`, with consumption/handling in `BottomPane::on_ctrl_c` in `codex-rs/tui/src/bottom_pane/mod.rs:218`. Exiting is coordinated via `ShutdownComplete` -(`codex-rs/tui/src/chatwidget.rs:1051`) and `AppEvent::ExitRequest` (`codex-rs/tui/src/app.rs:718`). +(`codex-rs/tui/src/chatwidget.rs:1051`) and `AppEvent::Exit(ExitMode::Immediate)` +(`codex-rs/tui/src/app.rs:718`). ## Timeline of behavior changes diff --git a/docs/exit-confirmation-prompt-design.md b/docs/exit-confirmation-prompt-design.md index 8c4e31d1427..5d9e4cb588e 100644 --- a/docs/exit-confirmation-prompt-design.md +++ b/docs/exit-confirmation-prompt-design.md @@ -32,8 +32,8 @@ active work whenever possible. Codex distinguishes between: -- **Exit**: end the UI event loop and terminate the process (e.g., `AppEvent::ExitRequest` - returning `Ok(false)`). +- **Exit**: end the UI event loop and terminate the process (e.g., + `AppEvent::Exit(ExitMode::Immediate)` returning `Ok(false)`). - **Shutdown**: request a graceful agent/core shutdown (e.g., `Op::Shutdown`), often waiting for `ShutdownComplete` before exiting so background work can flush cleanly. - **Interrupt**: cancel a running operation (e.g., `Op::Interrupt`), used for streaming turns, @@ -63,8 +63,8 @@ In this design: The completion signal is `ShutdownComplete` (event name varies per crate, but the contract is the same: “core has finished shutting down”). -Today, the TUIs mix “exit immediately” and “shutdown first” depending on how the user tries to -leave. +Historically, the TUIs mixed “exit immediately” and “shutdown first” depending on how the user +tries to leave. ### Examples of core/UI state (what users see) @@ -91,8 +91,8 @@ it is idle). | ------------------------------ | ------------------------------------------------------------- | | `Ctrl+C` while work is running | Interrupt (`Op::Interrupt`) | | `Ctrl+C` while idle | Shutdown (`Op::Shutdown`), exit on `ShutdownComplete` | -| `Ctrl+D` with empty composer | Exit immediately (`ExitRequest`, bypasses `Op::Shutdown`) | -| `/quit`, `/exit`, `/logout` | Exit immediately (`ExitRequest`, bypasses `Op::Shutdown`) | +| `Ctrl+D` with empty composer | Quit (shutdown+exit, confirmation prompt) | +| `/quit`, `/exit`, `/logout` | Quit (shutdown+exit, no prompt) | | `/new` (NewSession) | Shutdown current conversation, then stay running | Notes: @@ -135,16 +135,19 @@ These examples describe what happens today and why it matters for the confirmati - **Modal open**: `Ctrl+C` is first offered to the active modal/view to dismiss/abort. It should not trigger shutdown/exit unless the modal declines to handle it. -### Why `/quit`, `/exit`, `/logout`, and `Ctrl+D` don’t call shutdown today +### Why `/quit`, `/exit`, `/logout`, and `Ctrl+D` used to bypass shutdown -Today these are implemented as explicit “leave the UI now” actions: +Historically, these were explicit “leave the UI now” actions: -- `/quit`, `/exit`, and `/logout` dispatch `AppEvent::ExitRequest` directly. -- `Ctrl+D` exits only when the composer is empty (a guard added to reduce accidental exits), but it - still exits via `ExitRequest` rather than `Op::Shutdown`. +- `/quit`, `/exit`, and `/logout` dispatched `AppEvent::Exit(ExitMode::Immediate)` directly. +- `Ctrl+D` exited only when the composer was empty (a guard added to reduce accidental exits), but + it still exited via immediate exit rather than `Op::Shutdown`. -This split does not have a principled/rational basis documented anywhere, and it may simply be an -accidental divergence in how different quit triggers were implemented over time. +This split did not have a principled/rational basis documented anywhere, and it may simply have +been an accidental divergence in how different quit triggers were implemented over time. + +The updated design removes this distinction and routes all quit triggers through shutdown-first +(`AppEvent::Exit(ExitMode::ShutdownFirst { confirm: ... })`). Either way, bypassing `Op::Shutdown` is the riskier option: it relies on runtime teardown and dropping to clean up in-flight work, which can leave behind “leftovers” (e.g., unified exec child @@ -230,31 +233,39 @@ assumes a single default: quitting Codex should request `Op::Shutdown` and exit An “exit immediately” path can remain as a fallback (e.g., if shutdown hangs), but it should not be the normal route for user-initiated quit gestures or commands. -### Add an app-level event that means “quit with confirmation” - -Keep `AppEvent::ExitRequest` meaning “exit immediately”. +### Approach update: unify exit events -Add: +Instead of introducing a new `QuitRequest` alongside a separate immediate-exit event, use a +single exit event with an explicit mode: ```rust -AppEvent::QuitRequest { confirm: bool } +AppEvent::Exit(ExitMode::ShutdownFirst { confirm: bool }) +AppEvent::Exit(ExitMode::Immediate) ``` +This keeps all exit paths flowing through the same event handler, which makes logging and policy +decisions consistent and keeps exit tracking in one place. + +### App-level handling (prompt vs shutdown) + Handling lives in the app layer (`App`), because: -- `App` is already the coordinator for exit (`ExitRequest`) and for reacting to `ShutdownComplete`. +- `App` is already the coordinator for exit (`Exit(ExitMode::Immediate)`) and for reacting to + `ShutdownComplete`. - `App` owns the current configuration and is the right place to gate “prompt vs no prompt”. Pseudo-flow: 1. A key handler decides “this key press would quit” and sends - `QuitRequest { confirm: true }`. + `Exit(ShutdownFirst { confirm: true })`. 2. `App` checks `config.notices.hide_exit_confirmation_prompt.unwrap_or(false)`: - if `true`, initiate shutdown+exit immediately (no prompt). - if `false`, ask `ChatWidget` to open the confirmation prompt. - -Slash commands like `/quit`, `/exit`, and `/logout` can dispatch `QuitRequest { confirm: false }` -to preserve the “no prompt” behavior while still taking the shutdown+exit path. +3. Slash commands like `/quit`, `/exit`, and `/logout` send + `Exit(ShutdownFirst { confirm: false })` to skip the prompt while still taking the + shutdown+exit path. +4. When `ShutdownComplete` arrives, `ChatWidget` calls `request_exit()`, which sends + `Exit(Immediate)` so the UI loop terminates. ### Action handling details (shutdown+exit) @@ -548,7 +559,7 @@ Keep snapshot names aligned between crates, e.g.: Add targeted tests that encode the correctness boundaries: - `ctrl_c_during_review_cancels_review_not_exit` - - Assert: no `ExitRequest`/exit event emitted; interrupt/cancel path is taken. + - Assert: no `Exit(ExitMode::Immediate)` event emitted; interrupt/cancel path is taken. - `ctrl_c_when_task_running_interrupts_not_exit` - `ctrl_d_does_not_exit_when_modal_open` - `ctrl_d_does_not_exit_when_composer_non_empty` From f6c30ac9c1c4d834491e7e7c77e054d4e1f8433f Mon Sep 17 00:00:00 2001 From: Josh McKinney Date: Thu, 8 Jan 2026 12:29:03 -0800 Subject: [PATCH 3/7] docs: consolidate quit/exit design notes Replace the older Ctrl+C quit history writeup with a single document that captures the current exit, shutdown, and interruption behavior for tui and tui2. This keeps the narrative close to the implementation and avoids split, partially overlapping docs. --- docs/ctrl-c-quit-history.md | 148 ------ docs/exit-confirmation-prompt-design.md | 676 +++--------------------- 2 files changed, 76 insertions(+), 748 deletions(-) delete mode 100644 docs/ctrl-c-quit-history.md diff --git a/docs/ctrl-c-quit-history.md b/docs/ctrl-c-quit-history.md deleted file mode 100644 index 5f1702e5ffe..00000000000 --- a/docs/ctrl-c-quit-history.md +++ /dev/null @@ -1,148 +0,0 @@ -# Ctrl+C / Ctrl+D behavior history (Codex CLI) - -This document explains how `Ctrl+C` and `Ctrl+D` behave in the Codex Rust TUI, how that behavior -changed over time, and why. It draws only from GitHub PR descriptions, PR review comments, and -linked GitHub issues. - -## Mental model - -`Ctrl+C` / `Ctrl+D` can arrive via different mechanisms: - -In a line-buffered program, `Ctrl+C` is usually delivered as a signal (`SIGINT`) and `Ctrl+D` is -usually EOF on stdin. In the TUI, raw mode is enabled and both usually show up as key events -(e.g., `KeyCode::Char('c')` with Control), not as `SIGINT`/EOF. - -If Codex is installed via npm, a Node.js wrapper launches the native binary. That wrapper’s signal -forwarding affects how `SIGINT`/`SIGTERM` behave when the parent process receives a signal (it -generally does not affect the TUI’s raw-mode key handling). - -## Current behavior (Rust TUI) - -As of HEAD: - -- `Ctrl+C` is stateful. It may dismiss an active modal, clear the composer, interrupt a running - task, or trigger shutdown when idle with an empty composer. -- `Ctrl+D` triggers exit only when the composer is empty; otherwise it behaves like normal input. - -The main decision point for `Ctrl+C` is `ChatWidget::on_ctrl_c` in -`codex-rs/tui/src/chatwidget.rs:3309`, with consumption/handling in `BottomPane::on_ctrl_c` in -`codex-rs/tui/src/bottom_pane/mod.rs:218`. Exiting is coordinated via `ShutdownComplete` -(`codex-rs/tui/src/chatwidget.rs:1051`) and `AppEvent::Exit(ExitMode::Immediate)` -(`codex-rs/tui/src/app.rs:718`). - -## Timeline of behavior changes - -### — Handle Ctrl+C quit when idle - -This introduced a deliberate “two-step” quit flow for `Ctrl+C` while idle: the first press shows -a quit hint and a second press exits. It also reset the hint when other input arrived or when work -began. - -### — Introducing shutdown operation - -This PR introduced an explicit shutdown operation so background tasks could flush work before the -process exits. A key review concern was that if the Tokio runtime shuts down, still-running tasks -can be dropped before they finish draining queued work, so “fire and forget” spawning is not -enough for correctness. - -The review thread is also where “shutdown completion” became an explicit part of the contract: -there is a comment asking whether anything in the TUI waits for shutdown to be processed, and -another calling out a user-visible concern: “Wait, why are we changing Ctrl-C behavior. Did you -manually test this? What is different now?” This is useful context because later `Ctrl+C` quit -behavior relies on the existence and correctness of `ShutdownComplete`. - -### — Fix approval workflow - -This PR made approval flows visible and interruptible. Commands requiring approval are written to -chat history before the modal appears, the decision is logged after, and `Ctrl+C` aborts an open -approval modal (effectively acting like Esc for that modal). - -The PR review comments are where the “consumed vs not consumed” contract is made explicit. In -particular, one review suggested introducing a small enum (e.g. `CancellationEvent`) rather than a -boolean so it is clear whether `Ctrl+C` was handled by the UI. Another review comment explicitly -describes the intended user flow: after aborting the modal, show the standard quit hint so a -subsequent `Ctrl+C` can exit. - -### — Ctrl+D exits only when composer is empty - -This prevented accidental exits from `Ctrl+D` when the composer has typed text. The PR description -frames the design as “exit only when there’s nothing to lose”, and the PR comment summarizes the -mechanics as routing `Ctrl+D` to normal handling unless the composer is empty, plus adding -`is_empty` helpers to keep the check consistent. - -### — single control flow for both Esc and Ctrl+C - -This tightened correctness around “interrupt” semantics. The stated intent is that while a task is -running, Esc and `Ctrl+C` should do the same thing, and that certain stuck-history-widget cases -were fixed. - -The PR description also highlights a subtle `Ctrl+D` edge case: `Ctrl+D` could quit the app while -an approval modal was showing if the textarea was empty. - -There are no PR comment threads to incorporate here, but the commit messages within the PR show -what it tried to make true: - -- there should be a single interrupt path (rather than multiple UI components sending interrupts), -- interrupt/error should finalize in-progress UI cells consistently (e.g., replacing a spinner with - a failed mark), and -- `Ctrl+D` behavior needed an explicit fix. - -### — Clear non-empty prompts with ctrl + c - -This PR changed `Ctrl+C` to clear the prompt when the composer is non-empty, which is more aligned -with shell/REPL ergonomics than exiting. The PR description also mentions adjusting the hint text -to distinguish “interrupt” vs “quit” depending on state, and calls out tradeoffs (e.g., overlap -between Esc and `Ctrl+C`, and footer hint text not perfectly matching all states). - -This change is important context for “why does Ctrl+C quit immediately now?” because once `Ctrl+C` -is overloaded as “clear the prompt” when non-empty, the remaining idle case (empty composer) is -the one left to map to “quit”. - -### — Recover cleared prompt via history (Up arrow) - -This PR was a mitigation for prompt-clearing on `Ctrl+C`: it preserved cleared text so it could be -recovered via history navigation. - -The PR review comments show that an “undo clear prompt” shortcut was considered. A review comment -warned that restoring the prompt is tricky when the composer contains attachments: if placeholders -are restored as plain text rather than rebuilt atomically, users can partially delete/edit them and -silently lose the associated image. This is a concrete example of why a narrow undo binding can -have surprising edge cases. - -The review also includes implementation-level tightening (e.g., moving “record cleared draft” into -the clear path so clearing and remembering remain in sync, and questioning whether restoration -should reuse existing higher-level methods like `set_text_content`). - -### — Fix false "task complete" state while streaming - -This is an example where state tracking affects `Ctrl+C` behavior: the bug caused `Ctrl+C` to quit -instead of canceling the stream during the final agent message. It is a reminder that semantics -like “interrupt when working” depend on correct “working vs idle” state. - -### — ^C resets prompt history navigation cursor - -This made `Ctrl+C` in the prompt area behave more like common shells by resetting history -navigation state. It is not an exit behavior change, but it reinforces the trend of making prompt -area `Ctrl+C` do “prompt things” first (clear/reset/interrupt) before “quit”. - -## npm Node wrapper (signal path) - -When Codex is installed via npm, a Node.js wrapper launches the native binary. Signal handling in -that wrapper affects how `SIGINT`/`SIGTERM` behave when the parent process receives a signal. - -### — npm wrapper: forward signals and exit with child - -This PR fixed two problems in the npm wrapper: it now forwards termination signals to the child -and it correctly exits when the child exits. It references reports of runaway processes and links -to . - -## If you change Ctrl+C behavior again - -Most “what should `Ctrl+C` do?” questions depend on which state you are in. Before changing -behavior, it helps to make an explicit decision for each: - -- idle with empty composer (quit immediately vs require confirmation), -- idle with non-empty composer (clear vs quit, and how recoverable clear should be), -- working/streaming (always interrupt, and whether any state bugs can misclassify as idle), -- modal open (dismiss/abort first, and what the next `Ctrl+C` should do), -- signal path vs key-event path (especially for npm installs). diff --git a/docs/exit-confirmation-prompt-design.md b/docs/exit-confirmation-prompt-design.md index 5d9e4cb588e..13adf1cf992 100644 --- a/docs/exit-confirmation-prompt-design.md +++ b/docs/exit-confirmation-prompt-design.md @@ -1,640 +1,116 @@ -# Exit confirmation prompt (Ctrl+C / Ctrl+D) — design for tui + tui2 +# Exit and shutdown flow (tui + tui2) -This document proposes a unified implementation for an exit confirmation prompt that triggers on -`Ctrl+C` / `Ctrl+D` in both Rust TUIs (`codex-rs/tui` and `codex-rs/tui2`). +This document describes how exit, shutdown, and interruption work in the Rust TUIs +(`codex-rs/tui` and `codex-rs/tui2`). It is intended for Codex developers and +Codex itself when reasoning about future exit/shutdown changes. -It is grounded in `docs/ctrl-c-quit-history.md`, which captures prior design intent and regressions -around cancellation vs quitting. +This doc replaces earlier separate history and design notes. High-level history is +summarized below; full details are captured in PR #8936. -## Background and motivation +## Terms -In Codex’s TUIs, `Ctrl+C` and `Ctrl+D` arrive as key events (raw mode), not as `SIGINT` / stdin -EOF. Historically, Codex has used these keys for multiple user intents: +- **Exit**: end the UI event loop and terminate the process. +- **Shutdown**: request a graceful agent/core shutdown (`Op::Shutdown`) and wait + for `ShutdownComplete` so cleanup can run. +- **Interrupt**: cancel a running operation (`Op::Interrupt`). -- cancel/interrupt an in-flight operation, -- dismiss a modal / popup, -- clear draft input, -- quit the application. +## Event model (AppEvent) -This overload is intentional, but it is fragile: any bug in “are we working?” state tracking can -turn a would-be cancellation into an immediate quit. `docs/ctrl-c-quit-history.md` includes an -example (`PR #4627`) where state incorrectly flipped to “idle” while streaming, causing `Ctrl+C` -to quit instead of interrupt. +Exit is coordinated via a single event with explicit modes: -We also have a related bug: pressing `Ctrl+C` during a review task can quit immediately, but the -intended behavior is “cancel the review”. +- `AppEvent::Exit(ExitMode::ShutdownFirst { confirm })` + - Prefer this for user-initiated quits so cleanup runs. +- `AppEvent::Exit(ExitMode::Immediate)` + - Escape hatch for immediate exit. This bypasses shutdown and can drop + in-flight work (e.g., tasks, rollout flush, child process cleanup). -The exit confirmation prompt is primarily a safety feature to prevent accidental exits when the key -press would otherwise quit. It must not mask or replace the primary expectation: `Ctrl+C` cancels -active work whenever possible. +`App` is the coordinator: it decides whether to open the confirmation prompt or +submit `Op::Shutdown`, and it exits the UI loop only when `ExitMode::Immediate` +arrives (typically after `ShutdownComplete`). -## Terminology: exit vs shutdown vs interrupt +## User-triggered quit flows -Codex distinguishes between: +### Ctrl+C -- **Exit**: end the UI event loop and terminate the process (e.g., - `AppEvent::Exit(ExitMode::Immediate)` returning `Ok(false)`). -- **Shutdown**: request a graceful agent/core shutdown (e.g., `Op::Shutdown`), often waiting for - `ShutdownComplete` before exiting so background work can flush cleanly. -- **Interrupt**: cancel a running operation (e.g., `Op::Interrupt`), used for streaming turns, - long-running tools, and other cancellable tasks. +Priority order in the UI layer: -The design must be explicit about which action a “quit gesture” triggers. In particular: if today -idle `Ctrl+C` results in `Op::Shutdown`, a confirmation prompt must preserve that semantic -(prompting should not silently convert “shutdown” into “exit immediately”). +1. Active modal/view gets the first chance to consume (`BottomPane::on_ctrl_c`). + - If the modal handles it, the quit flow stops. +2. If cancellable work is active (streaming/tools/review), send `Op::Interrupt`. +3. If composer has draft input, clear the draft and show the quit hint. +4. If idle + empty, request shutdown-first quit with confirmation. -### Lifecycle model (what runs where) +### Ctrl+D -The UI and the agent/core have distinct lifecycles: +- Only triggers quit when the composer is empty **and** no modal is active. +- With any modal/popup open, key events are routed to the view and Ctrl+D does + not attempt to quit. -- The **UI event loop** owns input handling and rendering. “Exit” means the event loop stops and - the process typically terminates shortly after. -- The **agent/core** runs work that can outlive a single frame or keypress: streaming turns, - tool execution, review tasks, background state updates, etc. +### Slash commands -This matters because exiting the UI can abruptly end the runtime and drop background tasks. -`docs/ctrl-c-quit-history.md` calls this out as a correctness concern (see `PR #1647`): a graceful -shutdown handshake exists so the core can finish draining/flush work before the process exits. +- `/quit`, `/exit`, `/logout` request shutdown-first quit **without** a prompt, + because slash commands are harder to trigger accidentally and imply clear + intent to quit. -In this design: +### /new -- **Exit immediately** means “exit the UI loop now” (without a shutdown handshake). -- **Shutdown then exit** means “request core shutdown, then exit the UI after shutdown completes”. - The completion signal is `ShutdownComplete` (event name varies per crate, but the contract is the - same: “core has finished shutting down”). +- Uses shutdown without exit (suppresses `ShutdownComplete`) so the app can + start a fresh session without terminating. -Historically, the TUIs mixed “exit immediately” and “shutdown first” depending on how the user -tries to leave. +## Shutdown completion and suppression -### Examples of core/UI state (what users see) +`ShutdownComplete` is the signal that core cleanup has finished. The UI treats +it as the boundary for exit: -- **Idle**: no streaming output, no running tools, no active review task; the composer may be empty - or contain a draft. -- **Waiting for the turn to start**: the user pressed Enter, and the UI is waiting for the first - events for the new turn (this typically transitions quickly into “working/streaming”). -- **Streaming**: partial agent output is arriving and being rendered. -- **Running commands/tools**: the agent is executing local commands or tools and streaming their - progress/output into the transcript. -- **Review in progress**: a review request has been issued and the UI is in review mode, awaiting - results. -- **Modal open**: approvals, pickers, selection views, etc. are on screen. +- `ChatWidget` requests `Exit(Immediate)` on `ShutdownComplete`. +- `App` can suppress a single `ShutdownComplete` when shutdown is used as a + cleanup step (e.g., `/new`). -In the current implementation, the key state driving `Ctrl+C` behavior is whether the bottom pane -considers a task “running”. That state is toggled as turns start/finish, while some other -long-running flows (like review) have separate flags (e.g., `is_review_mode`). Regressions happen -when those states disagree (for example, when streaming is happening but the UI momentarily thinks -it is idle). +## Exit confirmation prompt -### Current trigger mapping (tui and tui2) +The confirmation prompt is a safety net for idle quits. When shown, it provides: -| Trigger | What it does today | -| ------------------------------ | ------------------------------------------------------------- | -| `Ctrl+C` while work is running | Interrupt (`Op::Interrupt`) | -| `Ctrl+C` while idle | Shutdown (`Op::Shutdown`), exit on `ShutdownComplete` | -| `Ctrl+D` with empty composer | Quit (shutdown+exit, confirmation prompt) | -| `/quit`, `/exit`, `/logout` | Quit (shutdown+exit, no prompt) | -| `/new` (NewSession) | Shutdown current conversation, then stay running | +- Quit now (shutdown-first). +- Quit and don't ask again (persists the notice, then shutdown-first). +- Cancel (stay in the app). -Notes: +The prompt is a bottom-pane selection view, so it does not appear if another +modal is already active. -- The “work is running” case depends on “task running” state being accurate. If it is wrong, a - `Ctrl+C` can take the idle path and start shutdown instead of interrupting. -- `ShutdownComplete` is generally treated as “exit now” by the widget layer, but the app layer can - suppress it when shutdown is used for cleanup rather than quitting. +## Configuration -### What `ShutdownComplete` does today - -`ShutdownComplete` is a sentinel event with no payload. It is used as a lifecycle boundary: - -- In core, it is emitted at the end of the shutdown handler after aborting tasks, terminating - processes, and shutting down the rollout recorder. -- In the TUIs, `ChatWidget` treats it as “exit now” by calling `request_exit()`. -- In `App`, a one-shot suppression flag can ignore the next `ShutdownComplete` when shutdown is - used for cleanup (e.g., stopping the current conversation during `/new`) rather than quitting. - -The key point: `ShutdownComplete` is not “cleanup itself”; it is the signal that cleanup has -already happened (or that the core believes shutdown is complete). - -### Scenario walkthrough (how shutdown vs exit shows up) - -These examples describe what happens today and why it matters for the confirmation prompt design: - -- **Idle + composer empty**: `Ctrl+C` sends `Op::Shutdown`, and the UI exits once `ShutdownComplete` - arrives. `Ctrl+D` exits immediately without sending `Op::Shutdown`. `/quit` and `/exit` also exit - immediately. -- **Idle + composer has a draft**: `Ctrl+C` clears the draft (and shows the quit hint). It does not - exit or shutdown on that press. A subsequent `Ctrl+C` may now hit “idle + empty” and trigger - shutdown. -- **Working/streaming output**: `Ctrl+C` sends `Op::Interrupt` (cancel) rather than quitting. This - is the primary “don’t lose my session” behavior, and it relies on “task running” being true. -- **Running local tools/commands**: this is still “working”; `Ctrl+C` should interrupt rather than - quit. If “task running” is false by mistake, `Ctrl+C` can incorrectly start shutdown. -- **Review in progress**: intended behavior is the same as “working”: `Ctrl+C` cancels the review. - The reported bug (“quit immediately during review”) indicates the UI is misclassifying review as - idle for `Ctrl+C` handling. -- **Modal open**: `Ctrl+C` is first offered to the active modal/view to dismiss/abort. It should - not trigger shutdown/exit unless the modal declines to handle it. - -### Why `/quit`, `/exit`, `/logout`, and `Ctrl+D` used to bypass shutdown - -Historically, these were explicit “leave the UI now” actions: - -- `/quit`, `/exit`, and `/logout` dispatched `AppEvent::Exit(ExitMode::Immediate)` directly. -- `Ctrl+D` exited only when the composer was empty (a guard added to reduce accidental exits), but - it still exited via immediate exit rather than `Op::Shutdown`. - -This split did not have a principled/rational basis documented anywhere, and it may simply have -been an accidental divergence in how different quit triggers were implemented over time. - -The updated design removes this distinction and routes all quit triggers through shutdown-first -(`AppEvent::Exit(ExitMode::ShutdownFirst { confirm: ... })`). - -Either way, bypassing `Op::Shutdown` is the riskier option: it relies on runtime teardown and -dropping to clean up in-flight work, which can leave behind “leftovers” (e.g., unified exec child -processes, unflushed rollout/session tail, or other background tasks that would normally be fenced -by `ShutdownComplete`). Where possible, Codex should prefer the shutdown handshake and exit only -after `ShutdownComplete`, regardless of whether the quit was initiated via keys or slash commands. - -## Problem statement - -1. **Accidental closure**: users sometimes press `Ctrl+C` / `Ctrl+D` out of habit - and unexpectedly lose their session. -2. **State misclassification regressions**: if the UI incorrectly believes it is - idle (streaming, review, etc.), `Ctrl+C` can incorrectly quit. -3. **Review cancellation bug**: `Ctrl+C` during an in-progress review should - cancel the review, not quit. -4. **Modal edge cases**: `Ctrl+D` (composer empty) must not quit while a modal/popup is open (see - history doc for prior regressions). - -## Goals - -- Prompt only when `Ctrl+C` / `Ctrl+D` would otherwise quit. -- Keep `/quit`, `/exit`, `/logout` as intentional exits (no prompt). -- Prefer shutdown+exit (graceful) for all quit paths where possible. -- Ensure `Ctrl+C` cancels review (and other active work) reliably. -- Keep behavior consistent across `tui` and `tui2`, including prompt copy and config key. -- Persist “don’t ask again” in `~/.codex/config.toml` under `[notice]`. -- Add tests that encode the rationale and prevent regressions. - -## Non-goals - -- Changing npm wrapper signal forwarding behavior (signal path). -- Redesigning all footer hint text (keep changes focused to quit confirmation). -- Introducing a shared UI crate between `tui` and `tui2` (we align by convention, not by code - sharing). - -## Proposed user-visible behavior - -Use a single config key: +The prompt can be suppressed via: ```toml [notice] hide_exit_confirmation_prompt = true ``` -`[notice]` is the right section for this: it already stores similar acknowledgement/NUX flags like -`hide_full_access_warning` and `hide_rate_limit_model_nudge`. - -When unset/false, show a confirmation prompt before quitting via `Ctrl+C` or `Ctrl+D` (the -accidental-prone exit gestures). The prompt must offer: - -- A primary quit option labeled like other confirmations (recommended: `Yes, quit Codex`). -- A cancel option (recommended: `No, stay in Codex`). -- A “remember” option that matches existing wording used elsewhere in the UI (recommended: - `Yes, and don't ask again`), which sets the config key and persists it. - -### State-based behavior table - -The key is to treat `Ctrl+C` primarily as “cancel”, and only as “quit” when there is nothing to -cancel. - -| State | `Ctrl+C` | `Ctrl+D` | -| ---------------------------- | ----------------------------- | --------------------- | -| Modal/popup open | Dismiss/abort the modal | Must not quit | -| Task/review/streaming active | Interrupt/cancel (never quit) | Do nothing | -| Composer has draft content | Clear draft | Normal input | -| Idle, composer empty | Quit (shutdown+exit) | Quit (shutdown+exit) | - -Notes: - -- “Task active” must include review mode. If review is not represented as “task running” today, it - must be wired in as part of this change. -- `Ctrl+D` should be treated as a quit gesture only when there is nothing to lose (composer empty) - and no modal is open. - -## Proposed implementation (unified design) - -### Policy: quit should be shutdown-first - -Since there is no clear rationale for having some quit triggers bypass shutdown, this design -assumes a single default: quitting Codex should request `Op::Shutdown` and exit only after -`ShutdownComplete`. - -An “exit immediately” path can remain as a fallback (e.g., if shutdown hangs), but it should not -be the normal route for user-initiated quit gestures or commands. - -### Approach update: unify exit events - -Instead of introducing a new `QuitRequest` alongside a separate immediate-exit event, use a -single exit event with an explicit mode: - -```rust -AppEvent::Exit(ExitMode::ShutdownFirst { confirm: bool }) -AppEvent::Exit(ExitMode::Immediate) -``` - -This keeps all exit paths flowing through the same event handler, which makes logging and policy -decisions consistent and keeps exit tracking in one place. - -### App-level handling (prompt vs shutdown) - -Handling lives in the app layer (`App`), because: - -- `App` is already the coordinator for exit (`Exit(ExitMode::Immediate)`) and for reacting to - `ShutdownComplete`. -- `App` owns the current configuration and is the right place to gate “prompt vs no prompt”. - -Pseudo-flow: - -1. A key handler decides “this key press would quit” and sends - `Exit(ShutdownFirst { confirm: true })`. -2. `App` checks `config.notices.hide_exit_confirmation_prompt.unwrap_or(false)`: - - if `true`, initiate shutdown+exit immediately (no prompt). - - if `false`, ask `ChatWidget` to open the confirmation prompt. -3. Slash commands like `/quit`, `/exit`, and `/logout` send - `Exit(ShutdownFirst { confirm: false })` to skip the prompt while still taking the - shutdown+exit path. -4. When `ShutdownComplete` arrives, `ChatWidget` calls `request_exit()`, which sends - `Exit(Immediate)` so the UI loop terminates. - -### Action handling details (shutdown+exit) - -To keep behavior correct and consistent, `App` must treat quitting as a two-step -sequence: - -1. Send `Op::Shutdown` to the core. -2. Exit the UI loop only after observing the shutdown completion event. - -This is the core invariant: the confirmation prompt should only gate whether we ask the user. - -Codex already uses `Op::Shutdown` for non-exit reasons (e.g., stopping a conversation/thread before -starting a new one). In the current TUIs, `ChatWidget` always exits on `ShutdownComplete`, and -`App` decides whether to forward that event using a suppression flag (see -`codex-rs/tui/src/app.rs:314` and `codex-rs/tui2/src/app.rs:378`). Any new quit confirmation flow -should preserve that separation: “shutdown” does not inherently mean “exit the app”. - -This is also why `/new` works the way it does today: it shuts down the current conversation/thread -to avoid leaking work, but it suppresses `ShutdownComplete` so the app can create a new session and -keep running instead of quitting. - -### Policy change: make exit paths shutdown-first - -Historically, Codex has had multiple exit paths. Some of them request `Op::Shutdown` first (idle -`Ctrl+C`), and some exit the UI immediately (`Ctrl+D` when the composer is empty, plus `/quit`, -`/exit`, and `/logout`). That split is easy to miss and it can skip meaningful cleanup. - -This design proposes to move all application exit paths to shutdown-first, so exiting Codex is -consistent and performs the same cleanup regardless of which “quit” gesture or command the user -uses. In practice, that means: - -- `Ctrl+C` (idle quit) continues to be shutdown-first. -- `Ctrl+D` (empty composer quit) should become shutdown-first. -- `/quit`, `/exit`, and `/logout` should become shutdown-first. - -If shutdown latency or hangs are a concern, this policy should be paired with: - -- the logging described in the “Observability and hang diagnosis” section, -- a bounded timeout with a clearly logged fallback to “exit immediately” (optional, but recommended - if hangs are observed in practice). - -### What `Op::Shutdown` cleans up in core (today) - -In `codex-rs/core`, shutdown performs a small set of concrete cleanup steps before emitting -`ShutdownComplete`: - -- Abort active tasks and turns (cancels tasks, runs per-task abort hooks, emits `TurnAborted`): - `codex-rs/core/src/tasks/mod.rs:158`. -- Terminate all unified exec processes (kills any long-running child processes created via unified - exec): `codex-rs/core/src/unified_exec/process_manager.rs:653`. -- Shut down the rollout recorder (acts as a barrier so the writer task has processed all prior - queued items; each rollout line is flushed as it is written): `codex-rs/core/src/codex.rs:2109` - and `codex-rs/core/src/rollout/recorder.rs:368`. -- Emit `ShutdownComplete`: `codex-rs/core/src/codex.rs:2137`. - -This is the work that can be skipped or cut short if the UI exits immediately without requesting -shutdown: tasks may be dropped by runtime teardown, rollout items can be lost if still queued, and -child processes may survive past the UI exit. - -### What it means to drop in-flight work (effects users can see) - -If the UI exits immediately (without `Op::Shutdown`), the process can terminate while work is still -in flight. The concrete consequences depend on which state Codex is in: - -- **Streaming/working**: partial output may be visible in the transcript, but the turn may not reach - its normal “end” path (e.g., no final `TaskComplete`, no finalization for in-progress UI cells). -- **Tool execution / unified exec**: child processes may outlive the UI if not explicitly - terminated, depending on how they were spawned. A shutdown path explicitly terminates them. -- **Rollout/session persistence**: the rollout writer is asynchronous. If the process exits while - items are queued but not yet processed, the session file may miss its tail. That can affect - replay/resume fidelity and any tooling/tests that inspect the rollout file. -- **Review in progress**: review uses separate state and can be misclassified as idle. If the UI - takes an exit path instead of an interrupt path, review can stop abruptly rather than being - cleanly cancelled. - -This is why the design prioritizes correctness over convenience for accidental-prone quit gestures: -we want to prefer “interrupt when working” and “shutdown+exit when idle” rather than “exit now”. - -### Rollout recorder shutdown: why this is written as “shutdown” - -The rollout shutdown mechanism can look surprising at first glance because it sends a “shutdown” -command to the writer task, then drops the recorder. - -Based on the current code: - -- The rollout writer task is single-threaded and processes commands FIFO. -- Each rollout line is written and flushed immediately when processed. -- Sending `RolloutCmd::Shutdown` and waiting for its ack works as a barrier: the ack cannot be sent - until all earlier queued `AddItems`/`Flush` commands have been processed by that task. -- Dropping the recorder then closes the channel, so the writer task can exit and the file handle is - closed. - -This appears to be the intended design (the comment in `codex-rs/core/src/codex.rs:2117` calls out -avoiding races with the background writer, especially in tests). If this shutdown barrier has -user-facing downsides (e.g., exit latency when the queue is backlogged), it would be worth -confirming intent and expectations with code owners while implementing the prompt flow. - -## Developer notes (footguns and invariants) - -This section is a checklist of context that is easy to miss when changing quit behavior. - -### Key-event path vs signal path - -In the TUIs, `Ctrl+C` and `Ctrl+D` typically arrive as key events (raw mode), not as `SIGINT` or -stdin EOF. Signal forwarding in the npm wrapper can affect non-TUI usage, but it usually does not -drive the in-TUI `Ctrl+C` handling. The quit confirmation prompt is about the key-event path. - -### “Consumed vs not consumed” is the quit boundary - -`Ctrl+C` is first offered to the bottom pane (`BottomPane::on_ctrl_c`), which may: - -- dismiss an active view/modal, -- clear a non-empty composer draft (and show the quit hint), -- or return “not handled” when the composer is empty and no view is active. - -The quit confirmation prompt should only be reachable when `Ctrl+C` was not consumed by the UI -layer (and `Ctrl+D` should be disabled while a modal is open). - -### “Task running” is a policy input, not a source of truth - -The current `Ctrl+C` decision uses `bottom_pane.is_task_running()` as the proxy for “working vs -idle” (see `ChatWidget::on_ctrl_c` in both TUIs). This state is toggled by task lifecycle events -and can be wrong transiently if state updates lag behind UI input. - -Implications: - -- Any regression that makes “task running” false while the agent is still active can turn `Ctrl+C` - into a quit (shutdown) path instead of an interrupt path. -- Review mode (`is_review_mode`) is tracked separately from “task running” today. The reported bug - (“quit during review”) strongly suggests these states can disagree. The implementation should - treat review as cancellable work and ensure the `Ctrl+C` path hits interrupt/cancel, not quit. - -### Interrupt vs shutdown clean up different things - -- `Op::Interrupt` is the “cancel current work” path. -- `Op::Shutdown` is the “stop everything and clean up” path. It aborts tasks, terminates unified - exec processes, and fences rollout persistence before emitting `ShutdownComplete`. - -The quit confirmation prompt must preserve this distinction; it should not replace a shutdown-first -quit path with an immediate exit. - -### Shutdown completion is wired to exit today - -In both TUIs, `ChatWidget` treats `ShutdownComplete` as “exit now” via `request_exit()`. The app -layer can ignore one `ShutdownComplete` using `suppress_shutdown_complete` when shutdown is used as -cleanup (e.g., `/new`). - -When adding a confirmation prompt, keep this separation intact: - -- “shutdown” is a core lifecycle action, and it may be used without quitting (conversation reset), -- “exit” is a UI lifecycle action, and it may happen without shutdown (a fallback for shutdown - hangs, but not the preferred/normal path). - -### `/new` already uses shutdown without quitting - -`/new` (NewSession) shuts down the current conversation/thread, suppresses the next -`ShutdownComplete`, removes the thread from the manager, and then creates a new session. This is a -useful example of why shutdown intent must be explicit: “shutdown” does not always mean “quit the -app”. - -### Consider “shutdown stuck” behavior - -If the quit confirmation prompt encourages more shutdown-first paths, it increases the chance -users wait on shutdown. The current UI largely assumes shutdown completes. If shutdown can hang in -practice, the design should decide whether to: - -- show a status line while shutting down, -- apply a timeout and then force-exit, -- or provide an escape hatch (e.g., a second confirm to exit immediately). - -This doc does not pick a policy yet, but implementers should consider it when wiring the flow. - -### Observability and hang diagnosis - -There are known cases where exit feels “stuck” to users. The shutdown/exit split makes this harder -to debug unless we log the decision points and the time spent waiting. - -The goal of logging here is: given a single log excerpt, it should be obvious: - -- what initiated the exit attempt (key vs slash command vs internal reset), -- whether we chose interrupt, shutdown+exit, or exit-immediately, -- what cleanup steps we started and finished, -- what timed out or errored, -- and whether `ShutdownComplete` was suppressed (conversation reset) or acted upon (quit). - -Recommended logging points (high signal, low volume): - -- **UI quit initiation** (tui/tui2): log once when we decide a quit path is being taken. Include: - - trigger (`ctrl_c`, `ctrl_d`, `/quit`, `/exit`, `/logout`, `/new`, etc.), - - action (`interrupt`, `shutdown_then_exit`, `exit_immediately`), - - identifiers (thread/conversation id if available), - - and whether a confirmation prompt was shown or skipped due to config. -- **Shutdown request sent** (tui/tui2): log when `Op::Shutdown` is submitted and record a start - timestamp for latency measurement. -- **Shutdown complete observed** (tui/tui2): log when `ShutdownComplete` is received, including: - - elapsed time since shutdown request (if known), - - whether it was suppressed (`suppress_shutdown_complete`), - - and the resulting action (exit UI vs continue running). -- **Core shutdown handler** (core): log the start and end of each cleanup step with durations: - - abort tasks, - - terminate unified exec processes, - - rollout recorder shutdown barrier, - - emit `ShutdownComplete`. - -Timeout guidance: - -- `abort_all_tasks` already includes a bounded “graceful interrupt” window per task, but the overall - shutdown path can still stall on I/O (e.g., rollout file flush) or unexpected blocking. If hangs - are a problem, add bounded timeouts around the most likely culprits (rollout shutdown barrier and - unified exec termination) and emit a warning that includes which step timed out and what cleanup - may have been skipped. - -Developer ergonomics: - -- Prefer structured logs with stable fields (trigger, action, ids, elapsed_ms, step) so we can grep - and so future telemetry can be added without rewriting messages. -- Use one-line summaries at INFO and keep deeper detail at DEBUG to avoid spamming normal runs. - -### Render the prompt UI in ChatWidget (both tui and tui2) - -Add a method like: - -```rust -fn open_exit_confirmation_prompt(&mut self) -``` - -Implementation requirements: - -- Do not open the prompt if another bottom-pane view/popup is already active. -- “Quit now” initiates shutdown+exit. -- “Quit and don’t ask again” triggers, in order: - 1) update in-memory config (`UpdateExitConfirmationPromptHidden(true)`), - 2) persist it (`PersistExitConfirmationPromptHidden`), - 3) initiate shutdown+exit. -- Keep copy consistent between `tui` and `tui2` and mention `Ctrl+C`/`Ctrl+D` explicitly. - -### Centralize “can cancel?” logic (fixes the review bug) - -The related bug (“Ctrl+C quits during review”) is fundamentally a missing/incorrect state signal. - -As part of this work, define a single predicate used by the `Ctrl+C` handler: - -- `is_cancellable_work_active()` (name may vary), returning true for: - - agent turn streaming, - - review task running, - - any other operation that should treat `Ctrl+C` as interrupt. - -Then: - -- If `is_cancellable_work_active()` is true, `Ctrl+C` must send `Op::Interrupt` (or the - appropriate review-cancel op) and must not open the quit prompt. -- Ensure the review workflow flips whatever state drives this predicate early and keeps it true - until the review is fully finished or cancelled. - -This is the correctness fix; the confirmation prompt is the safety net. - -### Persist config - -In `codex-rs/core`: - -- Add `Notice::hide_exit_confirmation_prompt: Option`. -- Add a `ConfigEdit` variant and a `ConfigEditsBuilder` helper: - - `SetNoticeHideExitConfirmationPrompt(bool)` - - `set_hide_exit_confirmation_prompt(bool) -> Self` -- Add a unit test ensuring setting this flag preserves existing `[notice]` table contents - (mirroring existing tests for other notice keys). - -In docs: - -- Update `docs/config.md` to mention the new `[notice]` key and describe the behavior in one short - paragraph. - -## Testing plan - -### Snapshot tests (both tui and tui2) - -Add an `insta` snapshot test that renders the exit confirmation prompt and asserts the full popup -text. This locks: - -- the exact prompt copy (grounded in the rationale), -- the presence of the “don’t ask again” option, -- line wrapping behavior at typical widths (e.g., 80 columns). - -Keep snapshot names aligned between crates, e.g.: - -- `exit_confirmation_popup` (tui) -- `exit_confirmation_popup` (tui2) - -### Behavioral tests (both tui and tui2) - -Add targeted tests that encode the correctness boundaries: - -- `ctrl_c_during_review_cancels_review_not_exit` - - Assert: no `Exit(ExitMode::Immediate)` event emitted; interrupt/cancel path is taken. -- `ctrl_c_when_task_running_interrupts_not_exit` -- `ctrl_d_does_not_exit_when_modal_open` -- `ctrl_d_does_not_exit_when_composer_non_empty` -- `dont_ask_again_sets_notice_and_persists` - - Assert the `Update...Hidden(true)` and `Persist...Hidden` events are emitted in sequence (or - equivalent). - -### Manual testing (both tui and tui2) - -Run the same checklist in `codex-rs/tui` and `codex-rs/tui2`. +This flag is updated and persisted via `UpdateExitConfirmationPromptHidden` and +`PersistExitConfirmationPromptHidden`. -- **Idle, composer empty** - - Press `Ctrl+C` → confirmation prompt appears → confirm quit → app performs shutdown+exit. - - Press `Ctrl+D` → confirmation prompt appears → confirm quit → app performs shutdown+exit. -- **Idle, composer has draft** - - Press `Ctrl+C` → clears draft (no prompt). - - Press `Ctrl+D` → does not quit (normal input behavior; no prompt). -- **While streaming / tool running** - - Start a turn that streams for a bit (or run a tool that takes time, e.g. something that sleeps). - - Press `Ctrl+C` → interrupts/cancels work (no prompt, no quit). -- **Review in progress** - - Start a review flow. - - Press `Ctrl+C` → cancels the review (no prompt, no quit). -- **Modal/popup open** - - Open any modal/popup (approvals/picker/etc.). - - Press `Ctrl+D` → must not quit. - - Press `Ctrl+C` → dismisses/aborts the modal (must not quit). -- **Slash commands** - - Run `/quit` (and separately `/exit`, `/logout`) → exits without prompting, but still uses the - shutdown+exit path. -- **Don’t ask again** - - Trigger the prompt and choose “don’t ask again”. - - Repeat `Ctrl+C` / `Ctrl+D` on idle+empty → no prompt; still shutdown+exit. - - Verify the flag persisted in `~/.codex/config.toml` under `[notice]`. -- **Shutdown completion handling** - - Trigger quit and verify the UI waits for shutdown completion (via logs/visible delay if any). - - Run `/new` and verify shutdown completion is suppressed (app stays running). +## Edge cases and invariants -### Core persistence unit test (codex-core) +- **Review mode** counts as cancellable work. Ctrl+C should interrupt review, not + quit. +- **Modal open** means Ctrl+C/Ctrl+D should not quit unless the modal explicitly + declines to handle Ctrl+C. +- **Immediate exit** is not a normal user path; it is a fallback for shutdown + completion or an emergency exit. Use it sparingly because it skips cleanup. -Add a blocking config edit test analogous to existing notice tests: +## Testing expectations -- seed a config file with `[notice]\nexisting = "value"\n` -- apply the edit -- assert the file preserves `existing` and appends `hide_exit_confirmation_prompt = true` +At a minimum, we want coverage for: -## Implementation notes (code documentation) +- Ctrl+C while working interrupts, does not quit. +- Ctrl+C while idle and empty shows confirmation, then shutdown-first quit. +- Ctrl+D with modal open does not quit. +- `/quit` / `/exit` / `/logout` quit without prompt, but still shutdown-first. +- "Don't ask again" persists the notice and suppresses future prompts. -Add short doc comments in code where they prevent re-introducing the same regressions: - -- On `Notice::hide_exit_confirmation_prompt`: - - explain that it suppresses the `Ctrl+C`/`Ctrl+D` quit confirmation prompt. -- Near the `Ctrl+C` handler: - - explicitly document the priority order: modal dismissal → interrupt/cancel → clear draft → quit - flow. -- On the “review cancellation” predicate: - - explicitly mention that review must count as cancellable work, to prevent quitting mid-review. - -These comments should be brief and focused on intent (the “why”), not on re-stating code. +## History (high level) -## Open questions / decisions to confirm - -1. **Should quit always be shutdown-first?** - - Recommendation: yes for all quit paths (keys and slash commands), keeping “exit immediately” - only as a logged fallback for shutdown hangs/timeouts. -2. **Should the quit prompt appear on the first `Ctrl+C` press or retain the historical “two-step - hint” behavior?** - - Recommendation: the prompt replaces the two-step hint for the idle-empty case (clearer and - more explicit). -3. **Should `Ctrl+D` show the same prompt as `Ctrl+C`?** - - Recommendation: yes, but only when composer is empty and no modal is open. - -## Rollout considerations - -- Default behavior should be safe: prompt enabled unless explicitly disabled. -- The “don’t ask again” option must persist reliably (core config edit test). -- Ensure we do not prompt during active work: the review cancellation fix is a prerequisite for - the prompt to feel correct rather than annoying. +Codex has historically mixed "exit immediately" and "shutdown-first" across +quit gestures, largely due to incremental changes and regressions in state +tracking. This doc reflects the current unified, shutdown-first approach. See +PR #8936 for the detailed history and rationale. From 8e62b190c99f2be181d13ef3b4414841bc119ad0 Mon Sep 17 00:00:00 2001 From: Josh McKinney Date: Fri, 9 Jan 2026 15:54:26 -0800 Subject: [PATCH 4/7] tui: double-press Ctrl+C/Ctrl+D to quit Replace the quit confirmation prompt menu with a transient footer hint. The first press of Ctrl+C or Ctrl+D shows 'ctrl + again to quit' for 1s. Pressing the same key again within that window exits via a shutdown-first flow so cleanup can run. Remove leftover prompt-related config/events and rename the footer mode to avoid Ctrl+C-specific naming. Update docs/snapshots and switch the markdownlint-cli2 config to YAML. --- .markdownlint-cli2.jsonc | 8 - codex-rs/core/src/config/edit.rs | 42 ---- codex-rs/core/src/config/types.rs | 2 - codex-rs/tui/src/app.rs | 22 +- codex-rs/tui/src/bottom_pane/chat_composer.rs | 69 +++++-- codex-rs/tui/src/bottom_pane/footer.rs | 53 ++--- codex-rs/tui/src/bottom_pane/mod.rs | 59 ++++-- ...__tests__footer_mode_ctrl_c_interrupt.snap | 2 +- ...er__tests__footer_ctrl_c_quit_running.snap | 2 +- codex-rs/tui/src/chatwidget.rs | 189 +++++++++--------- ...idget__tests__exit_confirmation_popup.snap | 13 -- codex-rs/tui/src/chatwidget/tests.rs | 63 +++--- codex-rs/tui2/src/app.rs | 22 +- .../tui2/src/bottom_pane/chat_composer.rs | 66 ++++-- codex-rs/tui2/src/bottom_pane/footer.rs | 55 ++--- codex-rs/tui2/src/bottom_pane/mod.rs | 59 ++++-- ...__tests__footer_mode_ctrl_c_interrupt.snap | 2 +- ...er__tests__footer_ctrl_c_quit_running.snap | 2 +- codex-rs/tui2/src/chatwidget.rs | 189 +++++++++--------- ...idget__tests__exit_confirmation_popup.snap | 13 -- codex-rs/tui2/src/chatwidget/tests.rs | 63 +++--- docs/exit-confirmation-prompt-design.md | 78 +++----- 22 files changed, 529 insertions(+), 544 deletions(-) delete mode 100644 .markdownlint-cli2.jsonc delete mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exit_confirmation_popup.snap delete mode 100644 codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__exit_confirmation_popup.snap diff --git a/.markdownlint-cli2.jsonc b/.markdownlint-cli2.jsonc deleted file mode 100644 index 90985b6d21f..00000000000 --- a/.markdownlint-cli2.jsonc +++ /dev/null @@ -1,8 +0,0 @@ -{ - "config": { - "MD013": { - "line_length": 100 - } - } -} - diff --git a/codex-rs/core/src/config/edit.rs b/codex-rs/core/src/config/edit.rs index e443443cd98..0def0440a80 100644 --- a/codex-rs/core/src/config/edit.rs +++ b/codex-rs/core/src/config/edit.rs @@ -28,8 +28,6 @@ pub enum ConfigEdit { SetNoticeHideWorldWritableWarning(bool), /// Toggle the rate limit model nudge acknowledgement flag. SetNoticeHideRateLimitModelNudge(bool), - /// Toggle the exit confirmation prompt acknowledgement flag. - SetNoticeHideExitConfirmationPrompt(bool), /// Toggle the Windows onboarding acknowledgement flag. SetWindowsWslSetupAcknowledged(bool), /// Toggle the model migration prompt acknowledgement flag. @@ -282,11 +280,6 @@ impl ConfigDocument { &[Notice::TABLE_KEY, "hide_rate_limit_model_nudge"], value(*acknowledged), )), - ConfigEdit::SetNoticeHideExitConfirmationPrompt(acknowledged) => Ok(self.write_value( - Scope::Global, - &[Notice::TABLE_KEY, "hide_exit_confirmation_prompt"], - value(*acknowledged), - )), ConfigEdit::SetNoticeHideModelMigrationPrompt(migration_config, acknowledged) => { Ok(self.write_value( Scope::Global, @@ -621,14 +614,6 @@ impl ConfigEditsBuilder { self } - pub fn set_hide_exit_confirmation_prompt(mut self, acknowledged: bool) -> Self { - self.edits - .push(ConfigEdit::SetNoticeHideExitConfirmationPrompt( - acknowledged, - )); - self - } - pub fn set_hide_model_migration_prompt(mut self, model: &str, acknowledged: bool) -> Self { self.edits .push(ConfigEdit::SetNoticeHideModelMigrationPrompt( @@ -1023,33 +1008,6 @@ hide_rate_limit_model_nudge = true assert_eq!(contents, expected); } - #[test] - fn blocking_set_hide_exit_confirmation_prompt_preserves_table() { - let tmp = tempdir().expect("tmpdir"); - let codex_home = tmp.path(); - std::fs::write( - codex_home.join(CONFIG_TOML_FILE), - r#"[notice] -existing = "value" -"#, - ) - .expect("seed"); - - apply_blocking( - codex_home, - None, - &[ConfigEdit::SetNoticeHideExitConfirmationPrompt(true)], - ) - .expect("persist"); - - let contents = - std::fs::read_to_string(codex_home.join(CONFIG_TOML_FILE)).expect("read config"); - let expected = r#"[notice] -existing = "value" -hide_exit_confirmation_prompt = true -"#; - assert_eq!(contents, expected); - } #[test] fn blocking_set_hide_gpt5_1_migration_prompt_preserves_table() { let tmp = tempdir().expect("tmpdir"); diff --git a/codex-rs/core/src/config/types.rs b/codex-rs/core/src/config/types.rs index a0bfad7a433..c57e550a7a8 100644 --- a/codex-rs/core/src/config/types.rs +++ b/codex-rs/core/src/config/types.rs @@ -565,8 +565,6 @@ pub struct Notice { pub hide_world_writable_warning: Option, /// Tracks whether the user opted out of the rate limit model switch reminder. pub hide_rate_limit_model_nudge: Option, - /// Tracks whether the exit confirmation prompt should be suppressed. - pub hide_exit_confirmation_prompt: Option, /// Tracks whether the user has seen the model migration prompt pub hide_gpt5_1_migration_prompt: Option, /// Tracks whether the user has seen the gpt-5.1-codex-max migration prompt diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index d2ca359147c..97b0176c5ea 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -857,9 +857,7 @@ impl App { self.chat_widget.handle_codex_event(event); } AppEvent::Exit(mode) => match mode { - ExitMode::ShutdownFirst => { - self.chat_widget.submit_op(Op::Shutdown); - } + ExitMode::ShutdownFirst => self.chat_widget.submit_op(Op::Shutdown), ExitMode::Immediate => { return Ok(AppRunControl::Exit(ExitReason::UserRequested)); } @@ -1227,9 +1225,6 @@ impl App { AppEvent::UpdateRateLimitSwitchPromptHidden(hidden) => { self.chat_widget.set_rate_limit_switch_prompt_hidden(hidden); } - AppEvent::UpdateExitConfirmationPromptHidden(hidden) => { - self.chat_widget.set_exit_confirmation_prompt_hidden(hidden); - } AppEvent::PersistFullAccessWarningAcknowledged => { if let Err(err) = ConfigEditsBuilder::new(&self.config.codex_home) .set_hide_full_access_warning(true) @@ -1275,21 +1270,6 @@ impl App { )); } } - AppEvent::PersistExitConfirmationPromptHidden => { - if let Err(err) = ConfigEditsBuilder::new(&self.config.codex_home) - .set_hide_exit_confirmation_prompt(true) - .apply() - .await - { - tracing::error!( - error = %err, - "failed to persist exit confirmation prompt preference" - ); - self.chat_widget.add_error_message(format!( - "Failed to save exit confirmation preference: {err}" - )); - } - } AppEvent::PersistModelMigrationPromptAcknowledged { from_model, to_model, diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index b73dd364e5f..f600263acbb 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -56,7 +56,8 @@ //! edits and renders a placeholder prompt instead of the editable textarea. This is part of the //! overall state machine, since it affects which transitions are even possible from a given UI //! state. - +use crate::key_hint; +use crate::key_hint::KeyBinding; use crate::key_hint::has_ctrl_or_alt; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; @@ -108,7 +109,6 @@ use codex_protocol::custom_prompts::PROMPTS_CMD_PREFIX; use codex_protocol::models::local_image_label_text; use crate::app_event::AppEvent; -use crate::app_event::ExitMode; use crate::app_event_sender::AppEventSender; use crate::bottom_pane::textarea::TextArea; use crate::bottom_pane::textarea::TextAreaState; @@ -168,7 +168,8 @@ pub(crate) struct ChatComposer { active_popup: ActivePopup, app_event_tx: AppEventSender, history: ChatComposerHistory, - ctrl_c_quit_hint: bool, + quit_shortcut_expires_at: Option, + quit_shortcut_key: KeyBinding, esc_backtrack_hint: bool, use_shift_enter_hint: bool, dismissed_file_popup_token: Option, @@ -222,7 +223,8 @@ impl ChatComposer { active_popup: ActivePopup::None, app_event_tx, history: ChatComposerHistory::new(), - ctrl_c_quit_hint: false, + quit_shortcut_expires_at: None, + quit_shortcut_key: key_hint::ctrl(KeyCode::Char('c')), esc_backtrack_hint: false, use_shift_enter_hint, dismissed_file_popup_token: None, @@ -567,16 +569,37 @@ impl ChatComposer { } } - pub fn set_ctrl_c_quit_hint(&mut self, show: bool, has_focus: bool) { - self.ctrl_c_quit_hint = show; - if show { - self.footer_mode = FooterMode::CtrlCReminder; - } else { - self.footer_mode = reset_mode_after_activity(self.footer_mode); - } + /// Show the transient "press again to quit" hint for `key`. + /// + /// The owner (`BottomPane`/`ChatWidget`) is responsible for scheduling a + /// redraw after [`super::QUIT_SHORTCUT_TIMEOUT`] so the hint can disappear + /// even when the UI is otherwise idle. + pub fn show_quit_shortcut_hint(&mut self, key: KeyBinding, has_focus: bool) { + self.quit_shortcut_expires_at = Instant::now() + .checked_add(super::QUIT_SHORTCUT_TIMEOUT) + .or_else(|| Some(Instant::now())); + self.quit_shortcut_key = key; + self.footer_mode = FooterMode::QuitShortcutReminder; self.set_has_focus(has_focus); } + /// Clear the "press again to quit" hint immediately. + pub fn clear_quit_shortcut_hint(&mut self, has_focus: bool) { + self.quit_shortcut_expires_at = None; + self.footer_mode = reset_mode_after_activity(self.footer_mode); + self.set_has_focus(has_focus); + } + + /// Whether the quit shortcut hint should currently be shown. + /// + /// This is time-based rather than event-based: it may become false without + /// any additional user input, so the UI schedules a redraw when the hint + /// expires. + pub(crate) fn quit_shortcut_hint_visible(&self) -> bool { + self.quit_shortcut_expires_at + .is_some_and(|expires_at| Instant::now() < expires_at) + } + fn next_large_paste_placeholder(&mut self, char_count: usize) -> String { let base = format!("[Pasted Content {char_count} chars]"); let next_suffix = self.large_paste_counters.entry(char_count).or_insert(0); @@ -1476,11 +1499,7 @@ impl ChatComposer { modifiers: crossterm::event::KeyModifiers::CONTROL, kind: KeyEventKind::Press, .. - } if self.is_empty() => { - self.app_event_tx - .send(AppEvent::Exit(ExitMode::ShutdownFirst { confirm: true })); - (InputResult::None, true) - } + } if self.is_empty() => (InputResult::None, false), // ------------------------------------------------------------- // History navigation (Up / Down) – only when the composer is not // empty or when the cursor is at the correct position, to avoid @@ -1769,7 +1788,7 @@ impl ChatComposer { return false; } - let next = toggle_shortcut_mode(self.footer_mode, self.ctrl_c_quit_hint); + let next = toggle_shortcut_mode(self.footer_mode, self.quit_shortcut_hint_visible()); let changed = next != self.footer_mode; self.footer_mode = next; changed @@ -1781,6 +1800,7 @@ impl ChatComposer { esc_backtrack_hint: self.esc_backtrack_hint, use_shift_enter_hint: self.use_shift_enter_hint, is_task_running: self.is_task_running, + quit_shortcut_key: self.quit_shortcut_key, steer_enabled: self.steer_enabled, context_window_percent: self.context_window_percent, context_window_used_tokens: self.context_window_used_tokens, @@ -1791,8 +1811,13 @@ impl ChatComposer { match self.footer_mode { FooterMode::EscHint => FooterMode::EscHint, FooterMode::ShortcutOverlay => FooterMode::ShortcutOverlay, - FooterMode::CtrlCReminder => FooterMode::CtrlCReminder, - FooterMode::ShortcutSummary if self.ctrl_c_quit_hint => FooterMode::CtrlCReminder, + FooterMode::QuitShortcutReminder if self.quit_shortcut_hint_visible() => { + FooterMode::QuitShortcutReminder + } + FooterMode::QuitShortcutReminder => FooterMode::ShortcutSummary, + FooterMode::ShortcutSummary if self.quit_shortcut_hint_visible() => { + FooterMode::QuitShortcutReminder + } FooterMode::ShortcutSummary if !self.is_empty() => FooterMode::ContextOnly, other => other, } @@ -2333,16 +2358,16 @@ mod tests { }); snapshot_composer_state("footer_mode_ctrl_c_quit", true, |composer| { - composer.set_ctrl_c_quit_hint(true, true); + composer.show_quit_shortcut_hint(key_hint::ctrl(KeyCode::Char('c')), true); }); snapshot_composer_state("footer_mode_ctrl_c_interrupt", true, |composer| { composer.set_task_running(true); - composer.set_ctrl_c_quit_hint(true, true); + composer.show_quit_shortcut_hint(key_hint::ctrl(KeyCode::Char('c')), true); }); snapshot_composer_state("footer_mode_ctrl_c_then_esc_hint", true, |composer| { - composer.set_ctrl_c_quit_hint(true, true); + composer.show_quit_shortcut_hint(key_hint::ctrl(KeyCode::Char('c')), true); let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); }); diff --git a/codex-rs/tui/src/bottom_pane/footer.rs b/codex-rs/tui/src/bottom_pane/footer.rs index 3fb04c39351..66fc3b2c865 100644 --- a/codex-rs/tui/src/bottom_pane/footer.rs +++ b/codex-rs/tui/src/bottom_pane/footer.rs @@ -21,13 +21,18 @@ pub(crate) struct FooterProps { pub(crate) use_shift_enter_hint: bool, pub(crate) is_task_running: bool, pub(crate) steer_enabled: bool, + /// Which key the user must press again to quit. + /// + /// This is rendered when `mode` is `FooterMode::QuitShortcutReminder`. + pub(crate) quit_shortcut_key: KeyBinding, pub(crate) context_window_percent: Option, pub(crate) context_window_used_tokens: Option, } #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub(crate) enum FooterMode { - CtrlCReminder, + /// Transient "press again to quit" reminder (Ctrl+C/Ctrl+D). + QuitShortcutReminder, ShortcutSummary, ShortcutOverlay, EscHint, @@ -35,12 +40,14 @@ pub(crate) enum FooterMode { } pub(crate) fn toggle_shortcut_mode(current: FooterMode, ctrl_c_hint: bool) -> FooterMode { - if ctrl_c_hint && matches!(current, FooterMode::CtrlCReminder) { + if ctrl_c_hint && matches!(current, FooterMode::QuitShortcutReminder) { return current; } match current { - FooterMode::ShortcutOverlay | FooterMode::CtrlCReminder => FooterMode::ShortcutSummary, + FooterMode::ShortcutOverlay | FooterMode::QuitShortcutReminder => { + FooterMode::ShortcutSummary + } _ => FooterMode::ShortcutOverlay, } } @@ -57,7 +64,7 @@ pub(crate) fn reset_mode_after_activity(current: FooterMode) -> FooterMode { match current { FooterMode::EscHint | FooterMode::ShortcutOverlay - | FooterMode::CtrlCReminder + | FooterMode::QuitShortcutReminder | FooterMode::ContextOnly => FooterMode::ShortcutSummary, other => other, } @@ -82,9 +89,9 @@ fn footer_lines(props: FooterProps) -> Vec> { // the shortcut hint is hidden). Hide it only for the multi-line // ShortcutOverlay. match props.mode { - FooterMode::CtrlCReminder => vec![ctrl_c_reminder_line(CtrlCReminderState { - is_task_running: props.is_task_running, - })], + FooterMode::QuitShortcutReminder => { + vec![quit_shortcut_reminder_line(props.quit_shortcut_key)] + } FooterMode::ShortcutSummary => { let mut line = context_window_line( props.context_window_percent, @@ -126,11 +133,6 @@ fn footer_lines(props: FooterProps) -> Vec> { } } -#[derive(Clone, Copy, Debug)] -struct CtrlCReminderState { - is_task_running: bool, -} - #[derive(Clone, Copy, Debug)] struct ShortcutsState { use_shift_enter_hint: bool, @@ -138,17 +140,8 @@ struct ShortcutsState { is_wsl: bool, } -fn ctrl_c_reminder_line(state: CtrlCReminderState) -> Line<'static> { - let action = if state.is_task_running { - "interrupt" - } else { - "quit" - }; - Line::from(vec![ - key_hint::ctrl(KeyCode::Char('c')).into(), - format!(" again to {action}").into(), - ]) - .dim() +fn quit_shortcut_reminder_line(key: KeyBinding) -> Line<'static> { + Line::from(vec![key.into(), " again to quit".into()]).dim() } fn esc_hint_line(esc_backtrack_hint: bool) -> Line<'static> { @@ -487,6 +480,7 @@ mod tests { use_shift_enter_hint: false, is_task_running: false, steer_enabled: false, + quit_shortcut_key: key_hint::ctrl(KeyCode::Char('c')), context_window_percent: None, context_window_used_tokens: None, }, @@ -500,6 +494,7 @@ mod tests { use_shift_enter_hint: true, is_task_running: false, steer_enabled: false, + quit_shortcut_key: key_hint::ctrl(KeyCode::Char('c')), context_window_percent: None, context_window_used_tokens: None, }, @@ -508,11 +503,12 @@ mod tests { snapshot_footer( "footer_ctrl_c_quit_idle", FooterProps { - mode: FooterMode::CtrlCReminder, + mode: FooterMode::QuitShortcutReminder, esc_backtrack_hint: false, use_shift_enter_hint: false, is_task_running: false, steer_enabled: false, + quit_shortcut_key: key_hint::ctrl(KeyCode::Char('c')), context_window_percent: None, context_window_used_tokens: None, }, @@ -521,11 +517,12 @@ mod tests { snapshot_footer( "footer_ctrl_c_quit_running", FooterProps { - mode: FooterMode::CtrlCReminder, + mode: FooterMode::QuitShortcutReminder, esc_backtrack_hint: false, use_shift_enter_hint: false, is_task_running: true, steer_enabled: false, + quit_shortcut_key: key_hint::ctrl(KeyCode::Char('c')), context_window_percent: None, context_window_used_tokens: None, }, @@ -539,6 +536,7 @@ mod tests { use_shift_enter_hint: false, is_task_running: false, steer_enabled: false, + quit_shortcut_key: key_hint::ctrl(KeyCode::Char('c')), context_window_percent: None, context_window_used_tokens: None, }, @@ -552,6 +550,7 @@ mod tests { use_shift_enter_hint: false, is_task_running: false, steer_enabled: false, + quit_shortcut_key: key_hint::ctrl(KeyCode::Char('c')), context_window_percent: None, context_window_used_tokens: None, }, @@ -565,6 +564,7 @@ mod tests { use_shift_enter_hint: false, is_task_running: true, steer_enabled: false, + quit_shortcut_key: key_hint::ctrl(KeyCode::Char('c')), context_window_percent: Some(72), context_window_used_tokens: None, }, @@ -578,6 +578,7 @@ mod tests { use_shift_enter_hint: false, is_task_running: false, steer_enabled: false, + quit_shortcut_key: key_hint::ctrl(KeyCode::Char('c')), context_window_percent: None, context_window_used_tokens: Some(123_456), }, @@ -591,6 +592,7 @@ mod tests { use_shift_enter_hint: false, is_task_running: true, steer_enabled: false, + quit_shortcut_key: key_hint::ctrl(KeyCode::Char('c')), context_window_percent: None, context_window_used_tokens: None, }, @@ -604,6 +606,7 @@ mod tests { use_shift_enter_hint: false, is_task_running: true, steer_enabled: true, + quit_shortcut_key: key_hint::ctrl(KeyCode::Char('c')), context_window_percent: None, context_window_used_tokens: None, }, diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index f505b0271ee..a7446475ae0 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -4,6 +4,8 @@ use std::path::PathBuf; use crate::app_event_sender::AppEventSender; use crate::bottom_pane::queued_user_messages::QueuedUserMessages; use crate::bottom_pane::unified_exec_footer::UnifiedExecFooter; +use crate::key_hint; +use crate::key_hint::KeyBinding; use crate::render::renderable::FlexRenderable; use crate::render::renderable::Renderable; use crate::render::renderable::RenderableItem; @@ -46,6 +48,15 @@ mod textarea; mod unified_exec_footer; pub(crate) use feedback_view::FeedbackNoteView; +/// How long the "press again to quit" hint stays visible. +/// +/// This is shared between: +/// - `ChatWidget`: arming the double-press quit shortcut. +/// - `BottomPane`/`ChatComposer`: rendering and expiring the footer hint. +/// +/// Keeping a single value ensures Ctrl+C and Ctrl+D behave identically. +pub(crate) const QUIT_SHORTCUT_TIMEOUT: Duration = Duration::from_secs(1); + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum CancellationEvent { Handled, @@ -76,7 +87,6 @@ pub(crate) struct BottomPane { has_input_focus: bool, is_task_running: bool, - ctrl_c_quit_hint: bool, esc_backtrack_hint: bool, animations_enabled: bool, @@ -129,7 +139,6 @@ impl BottomPane { frame_requester, has_input_focus, is_task_running: false, - ctrl_c_quit_hint: false, status: None, unified_exec_footer: UnifiedExecFooter::new(), queued_user_messages: QueuedUserMessages::new(), @@ -228,7 +237,7 @@ impl BottomPane { self.view_stack.pop(); self.on_active_view_complete(); } - self.show_ctrl_c_quit_hint(); + self.show_quit_shortcut_hint(key_hint::ctrl(KeyCode::Char('c'))); } event } else if self.composer_is_empty() { @@ -236,7 +245,7 @@ impl BottomPane { } else { self.view_stack.pop(); self.clear_composer_for_ctrl_c(); - self.show_ctrl_c_quit_hint(); + self.show_quit_shortcut_hint(key_hint::ctrl(KeyCode::Char('c'))); CancellationEvent::Handled } } @@ -314,25 +323,41 @@ impl BottomPane { } } - pub(crate) fn show_ctrl_c_quit_hint(&mut self) { - self.ctrl_c_quit_hint = true; + /// Show the transient "press again to quit" hint for `key`. + /// + /// `ChatWidget` owns the quit shortcut state machine (it decides when quit is + /// allowed), while the bottom pane owns rendering. We also schedule a redraw + /// after [`QUIT_SHORTCUT_TIMEOUT`] so the hint disappears even if the user + /// stops typing and no other events trigger a draw. + pub(crate) fn show_quit_shortcut_hint(&mut self, key: KeyBinding) { self.composer - .set_ctrl_c_quit_hint(true, self.has_input_focus); + .show_quit_shortcut_hint(key, self.has_input_focus); + let frame_requester = self.frame_requester.clone(); + if let Ok(handle) = tokio::runtime::Handle::try_current() { + handle.spawn(async move { + tokio::time::sleep(QUIT_SHORTCUT_TIMEOUT).await; + frame_requester.schedule_frame(); + }); + } else { + // In tests (and other non-Tokio contexts), fall back to a thread so + // the hint can still expire without requiring an explicit draw. + std::thread::spawn(move || { + std::thread::sleep(QUIT_SHORTCUT_TIMEOUT); + frame_requester.schedule_frame(); + }); + } self.request_redraw(); } - pub(crate) fn clear_ctrl_c_quit_hint(&mut self) { - if self.ctrl_c_quit_hint { - self.ctrl_c_quit_hint = false; - self.composer - .set_ctrl_c_quit_hint(false, self.has_input_focus); - self.request_redraw(); - } + /// Clear the "press again to quit" hint immediately. + pub(crate) fn clear_quit_shortcut_hint(&mut self) { + self.composer.clear_quit_shortcut_hint(self.has_input_focus); + self.request_redraw(); } #[cfg(test)] - pub(crate) fn ctrl_c_quit_hint_visible(&self) -> bool { - self.ctrl_c_quit_hint + pub(crate) fn quit_shortcut_hint_visible(&self) -> bool { + self.composer.quit_shortcut_hint_visible() } #[cfg(test)] @@ -648,7 +673,7 @@ mod tests { }); pane.push_approval_request(exec_request(), &features); assert_eq!(CancellationEvent::Handled, pane.on_ctrl_c()); - assert!(pane.ctrl_c_quit_hint_visible()); + assert!(pane.quit_shortcut_hint_visible()); assert_eq!(CancellationEvent::NotHandled, pane.on_ctrl_c()); } diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_mode_ctrl_c_interrupt.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_mode_ctrl_c_interrupt.snap index 49ffb0d4c8f..7ecc5bba719 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_mode_ctrl_c_interrupt.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__footer_mode_ctrl_c_interrupt.snap @@ -10,4 +10,4 @@ expression: terminal.backend() " " " " " " -" ctrl + c again to interrupt " +" ctrl + c again to quit " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_ctrl_c_quit_running.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_ctrl_c_quit_running.snap index 9979372a1b9..31a1b743b8e 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_ctrl_c_quit_running.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__footer__tests__footer_ctrl_c_quit_running.snap @@ -2,4 +2,4 @@ source: tui/src/bottom_pane/footer.rs expression: terminal.backend() --- -" ctrl + c again to interrupt " +" ctrl + c again to quit " diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index eefebea2328..f6c2b15e723 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -26,6 +26,7 @@ use std::collections::VecDeque; use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; +use std::time::Instant; use codex_app_server_protocol::AuthMode; use codex_backend_client::Client as BackendClient; @@ -119,6 +120,7 @@ use crate::bottom_pane::BottomPaneParams; use crate::bottom_pane::CancellationEvent; use crate::bottom_pane::ExperimentalFeaturesView; use crate::bottom_pane::InputResult; +use crate::bottom_pane::QUIT_SHORTCUT_TIMEOUT; use crate::bottom_pane::SelectionAction; use crate::bottom_pane::SelectionItem; use crate::bottom_pane::SelectionViewParams; @@ -136,6 +138,8 @@ use crate::history_cell::AgentMessageCell; use crate::history_cell::HistoryCell; use crate::history_cell::McpToolCallCell; use crate::history_cell::PlainHistoryCell; +use crate::key_hint; +use crate::key_hint::KeyBinding; use crate::markdown::append_markdown; use crate::render::Insets; use crate::render::renderable::ColumnRenderable; @@ -408,6 +412,14 @@ pub(crate) struct ChatWidget { queued_user_messages: VecDeque, // Pending notification to show when unfocused on next Draw pending_notification: Option, + /// When `Some`, the user has pressed a quit shortcut and the second press + /// must occur before `quit_shortcut_expires_at`. + quit_shortcut_expires_at: Option, + /// Tracks which quit shortcut key was pressed first. + /// + /// We require the second press to match this key so `Ctrl+C` followed by + /// `Ctrl+D` (or vice versa) doesn't quit accidentally. + quit_shortcut_key: Option, // Simple review mode flag; used to adjust layout and banners. is_review_mode: bool, // Snapshot of token usage to restore after review mode exits. @@ -644,7 +656,9 @@ impl ChatWidget { fn on_task_started(&mut self) { self.agent_turn_running = true; - self.bottom_pane.clear_ctrl_c_quit_hint(); + self.bottom_pane.clear_quit_shortcut_hint(); + self.quit_shortcut_expires_at = None; + self.quit_shortcut_key = None; self.update_task_running_state(); self.retry_status_header = None; self.bottom_pane.set_interrupt_hint_visible(true); @@ -1570,6 +1584,8 @@ impl ChatWidget { show_welcome_banner: is_first_run, suppress_session_configured_redraw: false, pending_notification: None, + quit_shortcut_expires_at: None, + quit_shortcut_key: None, is_review_mode: false, pre_review_token_info: None, needs_final_message_separator: false, @@ -1661,6 +1677,8 @@ impl ChatWidget { show_welcome_banner: false, suppress_session_configured_redraw: true, pending_notification: None, + quit_shortcut_expires_at: None, + quit_shortcut_key: None, is_review_mode: false, pre_review_token_info: None, needs_final_message_separator: false, @@ -1689,6 +1707,19 @@ impl ChatWidget { self.on_ctrl_c(); return; } + KeyEvent { + code: KeyCode::Char(c), + modifiers, + kind: KeyEventKind::Press, + .. + } if modifiers.contains(KeyModifiers::CONTROL) && c.eq_ignore_ascii_case(&'d') => { + if self.on_ctrl_d() { + return; + } + self.bottom_pane.clear_quit_shortcut_hint(); + self.quit_shortcut_expires_at = None; + self.quit_shortcut_key = None; + } KeyEvent { code: KeyCode::Char(c), modifiers, @@ -1701,7 +1732,9 @@ impl ChatWidget { return; } other if other.kind == KeyEventKind::Press => { - self.bottom_pane.clear_ctrl_c_quit_hint(); + self.bottom_pane.clear_quit_shortcut_hint(); + self.quit_shortcut_expires_at = None; + self.quit_shortcut_key = None; } _ => {} } @@ -2400,27 +2433,19 @@ impl ChatWidget { /// Exit the UI immediately without waiting for shutdown. /// - /// Prefer [`Self::request_quit_with_confirmation`] or - /// [`Self::request_quit_without_confirmation`] for user-initiated exits; + /// Prefer [`Self::request_quit_without_confirmation`] for user-initiated exits; /// this is mainly a fallback for shutdown completion or emergency exits. fn request_immediate_exit(&self) { self.app_event_tx.send(AppEvent::Exit(ExitMode::Immediate)); } - /// Request a shutdown-first quit that shows the confirmation prompt. - fn request_quit_with_confirmation(&self) { - self.app_event_tx - .send(AppEvent::Exit(ExitMode::ShutdownFirst { confirm: true })); - } - - /// Request a shutdown-first quit that skips the confirmation prompt. + /// Request a shutdown-first quit. /// - /// Use this for explicit quit commands (`/quit`, `/exit`, `/logout`) when - /// we want shutdown+exit without prompting. Slash commands are less likely - /// to be accidental, so the intent to quit is clear. + /// This is used for explicit quit commands (`/quit`, `/exit`, `/logout`) and for + /// the double-press Ctrl+C/Ctrl+D quit shortcut. fn request_quit_without_confirmation(&self) { self.app_event_tx - .send(AppEvent::Exit(ExitMode::ShutdownFirst { confirm: false })); + .send(AppEvent::Exit(ExitMode::ShutdownFirst)); } fn request_redraw(&mut self) { @@ -3261,73 +3286,6 @@ impl ChatWidget { None } - /// Show the shutdown-first quit confirmation prompt. - /// - /// This uses a selection view with explicit shutdown actions and an option - /// to persist the "don't ask again" notice. - pub(crate) fn open_exit_confirmation_prompt(&mut self) { - if !self.bottom_pane.can_launch_external_editor() { - return; - } - - let mut header_children: Vec> = Vec::new(); - header_children.push(Box::new(Line::from("Quit Codex?").bold())); - let info_line = Line::from(vec![ - "You pressed ".into(), - "Ctrl+C".bold(), - " or ".into(), - "Ctrl+D".bold(), - ". Quitting will shut down the current session and exit Codex.".into(), - ]); - header_children.push(Box::new( - Paragraph::new(vec![info_line]).wrap(Wrap { trim: false }), - )); - let header = ColumnRenderable::with(header_children); - - let quit_actions: Vec = vec![Box::new(|tx| { - // Shutdown-first quit. - tx.send(AppEvent::CodexOp(Op::Shutdown)); - })]; - - let quit_and_remember_actions: Vec = vec![Box::new(|tx| { - // Persist the notice and then shut down. - tx.send(AppEvent::UpdateExitConfirmationPromptHidden(true)); - tx.send(AppEvent::PersistExitConfirmationPromptHidden); - tx.send(AppEvent::CodexOp(Op::Shutdown)); - })]; - - let items = vec![ - SelectionItem { - name: "Yes, quit Codex".to_string(), - description: Some("Shut down the current session and exit.".to_string()), - actions: quit_actions, - dismiss_on_select: true, - ..Default::default() - }, - SelectionItem { - name: "Yes, and don't ask again".to_string(), - description: Some("Quit now and skip this prompt next time.".to_string()), - actions: quit_and_remember_actions, - dismiss_on_select: true, - ..Default::default() - }, - SelectionItem { - name: "No, stay in Codex".to_string(), - description: Some("Return to the current session.".to_string()), - actions: Vec::new(), - dismiss_on_select: true, - ..Default::default() - }, - ]; - - self.bottom_pane.show_selection_view(SelectionViewParams { - footer_hint: Some(standard_popup_hint_line()), - items, - header: Box::new(header), - ..Default::default() - }); - } - pub(crate) fn open_full_access_confirmation(&mut self, preset: ApprovalPreset) { let approval = preset.approval; let sandbox = preset.sandbox; @@ -3827,19 +3785,6 @@ impl ChatWidget { } } - /// Update the in-memory hide flag for the exit confirmation prompt. - pub(crate) fn set_exit_confirmation_prompt_hidden(&mut self, hidden: bool) { - self.config.notices.hide_exit_confirmation_prompt = Some(hidden); - } - - /// Return whether the exit confirmation prompt should be suppressed. - pub(crate) fn exit_confirmation_prompt_hidden(&self) -> bool { - self.config - .notices - .hide_exit_confirmation_prompt - .unwrap_or(false) - } - #[cfg_attr(not(target_os = "windows"), allow(dead_code))] pub(crate) fn world_writable_warning_hidden(&self) -> bool { self.config @@ -3889,17 +3834,65 @@ impl ChatWidget { /// Handle Ctrl-C key press. fn on_ctrl_c(&mut self) { + let key = key_hint::ctrl(KeyCode::Char('c')); + if self.quit_shortcut_active_for(key) { + self.quit_shortcut_expires_at = None; + self.quit_shortcut_key = None; + self.request_quit_without_confirmation(); + return; + } + + self.arm_quit_shortcut(key); + if self.bottom_pane.on_ctrl_c() == CancellationEvent::Handled { return; } if self.is_cancellable_work_active() { - self.bottom_pane.show_ctrl_c_quit_hint(); self.submit_op(Op::Interrupt); - return; } + } + + /// Handle Ctrl-D key press. + /// + /// Ctrl-D only participates in quit when the composer is empty and no modal/popup is active. + /// Otherwise it should be routed to the active view and not attempt to quit. + fn on_ctrl_d(&mut self) -> bool { + let key = key_hint::ctrl(KeyCode::Char('d')); + if self.quit_shortcut_active_for(key) { + self.quit_shortcut_expires_at = None; + self.quit_shortcut_key = None; + self.request_quit_without_confirmation(); + return true; + } + + if !self.bottom_pane.composer_is_empty() || !self.bottom_pane.can_launch_external_editor() { + return false; + } + + self.arm_quit_shortcut(key); + true + } - self.request_quit_with_confirmation(); + /// True if `key` matches the armed quit shortcut and the window has not expired. + fn quit_shortcut_active_for(&self, key: KeyBinding) -> bool { + self.quit_shortcut_key == Some(key) + && self + .quit_shortcut_expires_at + .is_some_and(|expires_at| Instant::now() < expires_at) + } + + /// Arm the double-press quit shortcut and show the footer hint. + /// + /// This keeps the state machine (`quit_shortcut_*`) in `ChatWidget`, since + /// it is the component that interprets Ctrl+C vs Ctrl+D and decides whether + /// quitting is currently allowed, while delegating rendering to `BottomPane`. + fn arm_quit_shortcut(&mut self, key: KeyBinding) { + self.quit_shortcut_expires_at = Instant::now() + .checked_add(QUIT_SHORTCUT_TIMEOUT) + .or_else(|| Some(Instant::now())); + self.quit_shortcut_key = Some(key); + self.bottom_pane.show_quit_shortcut_hint(key); } // Review mode counts as cancellable work so Ctrl+C interrupts instead of quitting. diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exit_confirmation_popup.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exit_confirmation_popup.snap deleted file mode 100644 index dc5105a6213..00000000000 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__exit_confirmation_popup.snap +++ /dev/null @@ -1,13 +0,0 @@ ---- -source: tui/src/chatwidget/tests.rs -expression: popup ---- - Quit Codex? - You pressed Ctrl+C or Ctrl+D. Quitting will shut down the current session - and exit Codex. - -› 1. Yes, quit Codex Shut down the current session and exit. - 2. Yes, and don't ask again Quit now and skip this prompt next time. - 3. No, stay in Codex Return to the current session. - - Press enter to confirm or esc to go back diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 54e580773f8..a44a97a0fa1 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -430,6 +430,8 @@ async fn make_chatwidget_manual( queued_user_messages: VecDeque::new(), suppress_session_configured_redraw: false, pending_notification: None, + quit_shortcut_expires_at: None, + quit_shortcut_key: None, is_review_mode: false, pre_review_token_info: None, needs_final_message_separator: false, @@ -1104,7 +1106,7 @@ async fn streaming_final_answer_keeps_task_running_state() { Ok(Op::Interrupt) => {} other => panic!("expected Op::Interrupt, got {other:?}"), } - assert!(chat.bottom_pane.ctrl_c_quit_hint_visible()); + assert!(chat.bottom_pane.quit_shortcut_hint_visible()); } #[tokio::test] @@ -1113,10 +1115,39 @@ async fn ctrl_c_shutdown_ignores_caps_lock() { chat.handle_key_event(KeyEvent::new(KeyCode::Char('C'), KeyModifiers::CONTROL)); - assert_matches!( - rx.try_recv(), - Ok(AppEvent::Exit(ExitMode::ShutdownFirst { confirm: true })) + assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); + + chat.handle_key_event(KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL)); + + assert_matches!(rx.try_recv(), Ok(AppEvent::Exit(ExitMode::ShutdownFirst))); +} + +#[tokio::test] +async fn ctrl_d_double_press_quits_without_prompt() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; + + chat.handle_key_event(KeyEvent::new(KeyCode::Char('d'), KeyModifiers::CONTROL)); + assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); + + let width = 100; + let height = chat.desired_height(width); + let mut terminal = + ratatui::Terminal::new(VT100Backend::new(width, height)).expect("create terminal"); + terminal.set_viewport_area(Rect::new(0, 0, width, height)); + terminal + .draw(|f| chat.render(f.area(), f.buffer_mut())) + .expect("draw after ctrl+d"); + assert!( + terminal + .backend() + .vt100() + .screen() + .contents() + .contains("ctrl + d again to quit") ); + + chat.handle_key_event(KeyEvent::new(KeyCode::Char('d'), KeyModifiers::CONTROL)); + assert_matches!(rx.try_recv(), Ok(AppEvent::Exit(ExitMode::ShutdownFirst))); } #[tokio::test] @@ -1145,7 +1176,7 @@ async fn ctrl_c_cleared_prompt_is_recoverable_via_history() { chat.handle_key_event(KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL)); assert!(chat.bottom_pane.composer_text().is_empty()); assert_matches!(op_rx.try_recv(), Err(TryRecvError::Empty)); - assert!(chat.bottom_pane.ctrl_c_quit_hint_visible()); + assert!(chat.bottom_pane.quit_shortcut_hint_visible()); chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE)); let restored_text = chat.bottom_pane.composer_text(); @@ -1154,7 +1185,7 @@ async fn ctrl_c_cleared_prompt_is_recoverable_via_history() { "expected placeholder {placeholder:?} after history recall" ); assert!(restored_text.starts_with("draft message ")); - assert!(!chat.bottom_pane.ctrl_c_quit_hint_visible()); + assert!(!chat.bottom_pane.quit_shortcut_hint_visible()); let images = chat.bottom_pane.take_recent_submission_images(); assert!( @@ -1536,10 +1567,7 @@ async fn slash_quit_requests_exit() { chat.dispatch_command(SlashCommand::Quit); - assert_matches!( - rx.try_recv(), - Ok(AppEvent::Exit(ExitMode::ShutdownFirst { confirm: false })) - ); + assert_matches!(rx.try_recv(), Ok(AppEvent::Exit(ExitMode::ShutdownFirst))); } #[tokio::test] @@ -1548,10 +1576,7 @@ async fn slash_exit_requests_exit() { chat.dispatch_command(SlashCommand::Exit); - assert_matches!( - rx.try_recv(), - Ok(AppEvent::Exit(ExitMode::ShutdownFirst { confirm: false })) - ); + assert_matches!(rx.try_recv(), Ok(AppEvent::Exit(ExitMode::ShutdownFirst))); } #[tokio::test] @@ -2005,16 +2030,6 @@ fn render_bottom_popup(chat: &ChatWidget, width: u16) -> String { lines.join("\n") } -#[tokio::test] -async fn exit_confirmation_popup_snapshot() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; - - chat.open_exit_confirmation_prompt(); - - let popup = render_bottom_popup(&chat, 80); - assert_snapshot!("exit_confirmation_popup", popup); -} - #[tokio::test] async fn experimental_features_popup_snapshot() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; diff --git a/codex-rs/tui2/src/app.rs b/codex-rs/tui2/src/app.rs index 2b71db3e862..5c9d6e355c3 100644 --- a/codex-rs/tui2/src/app.rs +++ b/codex-rs/tui2/src/app.rs @@ -1644,9 +1644,7 @@ impl App { self.chat_widget.handle_codex_event(event); } AppEvent::Exit(mode) => match mode { - ExitMode::ShutdownFirst => { - self.chat_widget.submit_op(Op::Shutdown); - } + ExitMode::ShutdownFirst => self.chat_widget.submit_op(Op::Shutdown), ExitMode::Immediate => { return Ok(AppRunControl::Exit(ExitReason::UserRequested)); } @@ -1973,9 +1971,6 @@ impl App { AppEvent::UpdateRateLimitSwitchPromptHidden(hidden) => { self.chat_widget.set_rate_limit_switch_prompt_hidden(hidden); } - AppEvent::UpdateExitConfirmationPromptHidden(hidden) => { - self.chat_widget.set_exit_confirmation_prompt_hidden(hidden); - } AppEvent::PersistFullAccessWarningAcknowledged => { if let Err(err) = ConfigEditsBuilder::new(&self.config.codex_home) .set_hide_full_access_warning(true) @@ -2021,21 +2016,6 @@ impl App { )); } } - AppEvent::PersistExitConfirmationPromptHidden => { - if let Err(err) = ConfigEditsBuilder::new(&self.config.codex_home) - .set_hide_exit_confirmation_prompt(true) - .apply() - .await - { - tracing::error!( - error = %err, - "failed to persist exit confirmation prompt preference" - ); - self.chat_widget.add_error_message(format!( - "Failed to save exit confirmation preference: {err}" - )); - } - } AppEvent::PersistModelMigrationPromptAcknowledged { from_model, to_model, diff --git a/codex-rs/tui2/src/bottom_pane/chat_composer.rs b/codex-rs/tui2/src/bottom_pane/chat_composer.rs index 36f3ae811bb..553b8986f14 100644 --- a/codex-rs/tui2/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui2/src/bottom_pane/chat_composer.rs @@ -110,7 +110,6 @@ use codex_protocol::custom_prompts::PROMPTS_CMD_PREFIX; use codex_protocol::models::local_image_label_text; use crate::app_event::AppEvent; -use crate::app_event::ExitMode; use crate::app_event_sender::AppEventSender; use crate::bottom_pane::textarea::TextArea; use crate::bottom_pane::textarea::TextAreaState; @@ -170,7 +169,8 @@ pub(crate) struct ChatComposer { active_popup: ActivePopup, app_event_tx: AppEventSender, history: ChatComposerHistory, - ctrl_c_quit_hint: bool, + quit_shortcut_expires_at: Option, + quit_shortcut_key: KeyBinding, esc_backtrack_hint: bool, use_shift_enter_hint: bool, dismissed_file_popup_token: Option, @@ -229,7 +229,8 @@ impl ChatComposer { active_popup: ActivePopup::None, app_event_tx, history: ChatComposerHistory::new(), - ctrl_c_quit_hint: false, + quit_shortcut_expires_at: None, + quit_shortcut_key: key_hint::ctrl(KeyCode::Char('c')), esc_backtrack_hint: false, use_shift_enter_hint, dismissed_file_popup_token: None, @@ -500,16 +501,37 @@ impl ChatComposer { } } - pub fn set_ctrl_c_quit_hint(&mut self, show: bool, has_focus: bool) { - self.ctrl_c_quit_hint = show; - if show { - self.footer_mode = FooterMode::CtrlCReminder; - } else { - self.footer_mode = reset_mode_after_activity(self.footer_mode); - } + /// Show the transient "press again to quit" hint for `key`. + /// + /// The owner (`BottomPane`/`ChatWidget`) is responsible for scheduling a + /// redraw after [`super::QUIT_SHORTCUT_TIMEOUT`] so the hint can disappear + /// even when the UI is otherwise idle. + pub fn show_quit_shortcut_hint(&mut self, key: KeyBinding, has_focus: bool) { + self.quit_shortcut_expires_at = Instant::now() + .checked_add(super::QUIT_SHORTCUT_TIMEOUT) + .or_else(|| Some(Instant::now())); + self.quit_shortcut_key = key; + self.footer_mode = FooterMode::QuitShortcutReminder; + self.set_has_focus(has_focus); + } + + /// Clear the "press again to quit" hint immediately. + pub fn clear_quit_shortcut_hint(&mut self, has_focus: bool) { + self.quit_shortcut_expires_at = None; + self.footer_mode = reset_mode_after_activity(self.footer_mode); self.set_has_focus(has_focus); } + /// Whether the quit shortcut hint should currently be shown. + /// + /// This is time-based rather than event-based: it may become false without + /// any additional user input, so the UI schedules a redraw when the hint + /// expires. + pub(crate) fn quit_shortcut_hint_visible(&self) -> bool { + self.quit_shortcut_expires_at + .is_some_and(|expires_at| Instant::now() < expires_at) + } + fn next_large_paste_placeholder(&mut self, char_count: usize) -> String { let base = format!("[Pasted Content {char_count} chars]"); let next_suffix = self.large_paste_counters.entry(char_count).or_insert(0); @@ -1409,11 +1431,7 @@ impl ChatComposer { modifiers: crossterm::event::KeyModifiers::CONTROL, kind: KeyEventKind::Press, .. - } if self.is_empty() => { - self.app_event_tx - .send(AppEvent::Exit(ExitMode::ShutdownFirst { confirm: true })); - (InputResult::None, true) - } + } if self.is_empty() => (InputResult::None, false), // ------------------------------------------------------------- // History navigation (Up / Down) – only when the composer is not // empty or when the cursor is at the correct position, to avoid @@ -1707,7 +1725,7 @@ impl ChatComposer { return false; } - let next = toggle_shortcut_mode(self.footer_mode, self.ctrl_c_quit_hint); + let next = toggle_shortcut_mode(self.footer_mode, self.quit_shortcut_hint_visible()); let changed = next != self.footer_mode; self.footer_mode = next; changed @@ -1719,6 +1737,7 @@ impl ChatComposer { esc_backtrack_hint: self.esc_backtrack_hint, use_shift_enter_hint: self.use_shift_enter_hint, is_task_running: self.is_task_running, + quit_shortcut_key: self.quit_shortcut_key, steer_enabled: self.steer_enabled, context_window_percent: self.context_window_percent, context_window_used_tokens: self.context_window_used_tokens, @@ -1734,8 +1753,13 @@ impl ChatComposer { match self.footer_mode { FooterMode::EscHint => FooterMode::EscHint, FooterMode::ShortcutOverlay => FooterMode::ShortcutOverlay, - FooterMode::CtrlCReminder => FooterMode::CtrlCReminder, - FooterMode::ShortcutSummary if self.ctrl_c_quit_hint => FooterMode::CtrlCReminder, + FooterMode::QuitShortcutReminder if self.quit_shortcut_hint_visible() => { + FooterMode::QuitShortcutReminder + } + FooterMode::QuitShortcutReminder => FooterMode::ShortcutSummary, + FooterMode::ShortcutSummary if self.quit_shortcut_hint_visible() => { + FooterMode::QuitShortcutReminder + } FooterMode::ShortcutSummary if !self.is_empty() => FooterMode::ContextOnly, other => other, } @@ -2307,16 +2331,16 @@ mod tests { }); snapshot_composer_state("footer_mode_ctrl_c_quit", true, |composer| { - composer.set_ctrl_c_quit_hint(true, true); + composer.show_quit_shortcut_hint(key_hint::ctrl(KeyCode::Char('c')), true); }); snapshot_composer_state("footer_mode_ctrl_c_interrupt", true, |composer| { composer.set_task_running(true); - composer.set_ctrl_c_quit_hint(true, true); + composer.show_quit_shortcut_hint(key_hint::ctrl(KeyCode::Char('c')), true); }); snapshot_composer_state("footer_mode_ctrl_c_then_esc_hint", true, |composer| { - composer.set_ctrl_c_quit_hint(true, true); + composer.show_quit_shortcut_hint(key_hint::ctrl(KeyCode::Char('c')), true); let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); }); diff --git a/codex-rs/tui2/src/bottom_pane/footer.rs b/codex-rs/tui2/src/bottom_pane/footer.rs index c543ab6ee35..d8ff3ab188e 100644 --- a/codex-rs/tui2/src/bottom_pane/footer.rs +++ b/codex-rs/tui2/src/bottom_pane/footer.rs @@ -22,6 +22,10 @@ pub(crate) struct FooterProps { pub(crate) use_shift_enter_hint: bool, pub(crate) is_task_running: bool, pub(crate) steer_enabled: bool, + /// Which key the user must press again to quit. + /// + /// This is rendered when `mode` is `FooterMode::QuitShortcutReminder`. + pub(crate) quit_shortcut_key: KeyBinding, pub(crate) context_window_percent: Option, pub(crate) context_window_used_tokens: Option, pub(crate) transcript_scrolled: bool, @@ -33,7 +37,8 @@ pub(crate) struct FooterProps { #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub(crate) enum FooterMode { - CtrlCReminder, + /// Transient "press again to quit" reminder (Ctrl+C/Ctrl+D). + QuitShortcutReminder, ShortcutSummary, ShortcutOverlay, EscHint, @@ -41,12 +46,14 @@ pub(crate) enum FooterMode { } pub(crate) fn toggle_shortcut_mode(current: FooterMode, ctrl_c_hint: bool) -> FooterMode { - if ctrl_c_hint && matches!(current, FooterMode::CtrlCReminder) { + if ctrl_c_hint && matches!(current, FooterMode::QuitShortcutReminder) { return current; } match current { - FooterMode::ShortcutOverlay | FooterMode::CtrlCReminder => FooterMode::ShortcutSummary, + FooterMode::ShortcutOverlay | FooterMode::QuitShortcutReminder => { + FooterMode::ShortcutSummary + } _ => FooterMode::ShortcutOverlay, } } @@ -63,7 +70,7 @@ pub(crate) fn reset_mode_after_activity(current: FooterMode) -> FooterMode { match current { FooterMode::EscHint | FooterMode::ShortcutOverlay - | FooterMode::CtrlCReminder + | FooterMode::QuitShortcutReminder | FooterMode::ContextOnly => FooterMode::ShortcutSummary, other => other, } @@ -103,9 +110,9 @@ fn footer_lines(props: FooterProps) -> Vec> { // the shortcut hint is hidden). Hide it only for the multi-line // ShortcutOverlay. let mut lines = match props.mode { - FooterMode::CtrlCReminder => vec![ctrl_c_reminder_line(CtrlCReminderState { - is_task_running: props.is_task_running, - })], + FooterMode::QuitShortcutReminder => { + vec![quit_shortcut_reminder_line(props.quit_shortcut_key)] + } FooterMode::ShortcutSummary => { let mut line = context_window_line( props.context_window_percent, @@ -170,11 +177,6 @@ fn footer_lines(props: FooterProps) -> Vec> { lines } -#[derive(Clone, Copy, Debug)] -struct CtrlCReminderState { - is_task_running: bool, -} - #[derive(Clone, Copy, Debug)] struct ShortcutsState { use_shift_enter_hint: bool, @@ -182,17 +184,8 @@ struct ShortcutsState { is_wsl: bool, } -fn ctrl_c_reminder_line(state: CtrlCReminderState) -> Line<'static> { - let action = if state.is_task_running { - "interrupt" - } else { - "quit" - }; - Line::from(vec![ - key_hint::ctrl(KeyCode::Char('c')).into(), - format!(" again to {action}").into(), - ]) - .dim() +fn quit_shortcut_reminder_line(key: KeyBinding) -> Line<'static> { + Line::from(vec![key.into(), " again to quit".into()]).dim() } fn esc_hint_line(esc_backtrack_hint: bool) -> Line<'static> { @@ -518,6 +511,7 @@ mod tests { use_shift_enter_hint: false, is_task_running: false, steer_enabled: false, + quit_shortcut_key: key_hint::ctrl(KeyCode::Char('c')), context_window_percent: None, context_window_used_tokens: None, transcript_scrolled: false, @@ -536,6 +530,7 @@ mod tests { use_shift_enter_hint: false, is_task_running: false, steer_enabled: false, + quit_shortcut_key: key_hint::ctrl(KeyCode::Char('c')), context_window_percent: None, context_window_used_tokens: None, transcript_scrolled: true, @@ -554,6 +549,7 @@ mod tests { use_shift_enter_hint: true, is_task_running: false, steer_enabled: false, + quit_shortcut_key: key_hint::ctrl(KeyCode::Char('c')), context_window_percent: None, context_window_used_tokens: None, transcript_scrolled: false, @@ -567,11 +563,12 @@ mod tests { snapshot_footer( "footer_ctrl_c_quit_idle", FooterProps { - mode: FooterMode::CtrlCReminder, + mode: FooterMode::QuitShortcutReminder, esc_backtrack_hint: false, use_shift_enter_hint: false, is_task_running: false, steer_enabled: false, + quit_shortcut_key: key_hint::ctrl(KeyCode::Char('c')), context_window_percent: None, context_window_used_tokens: None, transcript_scrolled: false, @@ -585,11 +582,12 @@ mod tests { snapshot_footer( "footer_ctrl_c_quit_running", FooterProps { - mode: FooterMode::CtrlCReminder, + mode: FooterMode::QuitShortcutReminder, esc_backtrack_hint: false, use_shift_enter_hint: false, is_task_running: true, steer_enabled: false, + quit_shortcut_key: key_hint::ctrl(KeyCode::Char('c')), context_window_percent: None, context_window_used_tokens: None, transcript_scrolled: false, @@ -608,6 +606,7 @@ mod tests { use_shift_enter_hint: false, is_task_running: false, steer_enabled: false, + quit_shortcut_key: key_hint::ctrl(KeyCode::Char('c')), context_window_percent: None, context_window_used_tokens: None, transcript_scrolled: false, @@ -626,6 +625,7 @@ mod tests { use_shift_enter_hint: false, is_task_running: false, steer_enabled: false, + quit_shortcut_key: key_hint::ctrl(KeyCode::Char('c')), context_window_percent: None, context_window_used_tokens: None, transcript_scrolled: false, @@ -644,6 +644,7 @@ mod tests { use_shift_enter_hint: false, is_task_running: true, steer_enabled: false, + quit_shortcut_key: key_hint::ctrl(KeyCode::Char('c')), context_window_percent: Some(72), context_window_used_tokens: None, transcript_scrolled: false, @@ -662,6 +663,7 @@ mod tests { use_shift_enter_hint: false, is_task_running: false, steer_enabled: false, + quit_shortcut_key: key_hint::ctrl(KeyCode::Char('c')), context_window_percent: None, context_window_used_tokens: Some(123_456), transcript_scrolled: false, @@ -680,6 +682,7 @@ mod tests { use_shift_enter_hint: false, is_task_running: true, steer_enabled: false, + quit_shortcut_key: key_hint::ctrl(KeyCode::Char('c')), context_window_percent: None, context_window_used_tokens: None, transcript_scrolled: false, @@ -698,6 +701,7 @@ mod tests { use_shift_enter_hint: false, is_task_running: true, steer_enabled: true, + quit_shortcut_key: key_hint::ctrl(KeyCode::Char('c')), context_window_percent: None, context_window_used_tokens: None, transcript_scrolled: false, @@ -716,6 +720,7 @@ mod tests { use_shift_enter_hint: false, is_task_running: false, steer_enabled: false, + quit_shortcut_key: key_hint::ctrl(KeyCode::Char('c')), context_window_percent: None, context_window_used_tokens: None, transcript_scrolled: false, diff --git a/codex-rs/tui2/src/bottom_pane/mod.rs b/codex-rs/tui2/src/bottom_pane/mod.rs index fbc4d95ded4..8ea23976590 100644 --- a/codex-rs/tui2/src/bottom_pane/mod.rs +++ b/codex-rs/tui2/src/bottom_pane/mod.rs @@ -3,6 +3,8 @@ use std::path::PathBuf; use crate::app_event_sender::AppEventSender; use crate::bottom_pane::queued_user_messages::QueuedUserMessages; +use crate::key_hint; +use crate::key_hint::KeyBinding; use crate::render::renderable::FlexRenderable; use crate::render::renderable::Renderable; use crate::render::renderable::RenderableItem; @@ -43,6 +45,15 @@ mod selection_popup_common; mod textarea; pub(crate) use feedback_view::FeedbackNoteView; +/// How long the "press again to quit" hint stays visible. +/// +/// This is shared between: +/// - `ChatWidget`: arming the double-press quit shortcut. +/// - `BottomPane`/`ChatComposer`: rendering and expiring the footer hint. +/// +/// Keeping a single value ensures Ctrl+C and Ctrl+D behave identically. +pub(crate) const QUIT_SHORTCUT_TIMEOUT: Duration = Duration::from_secs(1); + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum CancellationEvent { Handled, @@ -71,7 +82,6 @@ pub(crate) struct BottomPane { has_input_focus: bool, is_task_running: bool, - ctrl_c_quit_hint: bool, esc_backtrack_hint: bool, animations_enabled: bool, @@ -122,7 +132,6 @@ impl BottomPane { frame_requester, has_input_focus, is_task_running: false, - ctrl_c_quit_hint: false, status: None, queued_user_messages: QueuedUserMessages::new(), esc_backtrack_hint: false, @@ -220,7 +229,7 @@ impl BottomPane { self.view_stack.pop(); self.on_active_view_complete(); } - self.show_ctrl_c_quit_hint(); + self.show_quit_shortcut_hint(key_hint::ctrl(KeyCode::Char('c'))); } event } else if self.composer_is_empty() { @@ -228,7 +237,7 @@ impl BottomPane { } else { self.view_stack.pop(); self.clear_composer_for_ctrl_c(); - self.show_ctrl_c_quit_hint(); + self.show_quit_shortcut_hint(key_hint::ctrl(KeyCode::Char('c'))); CancellationEvent::Handled } } @@ -292,25 +301,41 @@ impl BottomPane { } } - pub(crate) fn show_ctrl_c_quit_hint(&mut self) { - self.ctrl_c_quit_hint = true; + /// Show the transient "press again to quit" hint for `key`. + /// + /// `ChatWidget` owns the quit shortcut state machine (it decides when quit is + /// allowed), while the bottom pane owns rendering. We also schedule a redraw + /// after [`QUIT_SHORTCUT_TIMEOUT`] so the hint disappears even if the user + /// stops typing and no other events trigger a draw. + pub(crate) fn show_quit_shortcut_hint(&mut self, key: KeyBinding) { self.composer - .set_ctrl_c_quit_hint(true, self.has_input_focus); + .show_quit_shortcut_hint(key, self.has_input_focus); + let frame_requester = self.frame_requester.clone(); + if let Ok(handle) = tokio::runtime::Handle::try_current() { + handle.spawn(async move { + tokio::time::sleep(QUIT_SHORTCUT_TIMEOUT).await; + frame_requester.schedule_frame(); + }); + } else { + // In tests (and other non-Tokio contexts), fall back to a thread so + // the hint can still expire without requiring an explicit draw. + std::thread::spawn(move || { + std::thread::sleep(QUIT_SHORTCUT_TIMEOUT); + frame_requester.schedule_frame(); + }); + } self.request_redraw(); } - pub(crate) fn clear_ctrl_c_quit_hint(&mut self) { - if self.ctrl_c_quit_hint { - self.ctrl_c_quit_hint = false; - self.composer - .set_ctrl_c_quit_hint(false, self.has_input_focus); - self.request_redraw(); - } + /// Clear the "press again to quit" hint immediately. + pub(crate) fn clear_quit_shortcut_hint(&mut self) { + self.composer.clear_quit_shortcut_hint(self.has_input_focus); + self.request_redraw(); } #[cfg(test)] - pub(crate) fn ctrl_c_quit_hint_visible(&self) -> bool { - self.ctrl_c_quit_hint + pub(crate) fn quit_shortcut_hint_visible(&self) -> bool { + self.composer.quit_shortcut_hint_visible() } #[cfg(test)] @@ -634,7 +659,7 @@ mod tests { }); pane.push_approval_request(exec_request(), &features); assert_eq!(CancellationEvent::Handled, pane.on_ctrl_c()); - assert!(pane.ctrl_c_quit_hint_visible()); + assert!(pane.quit_shortcut_hint_visible()); assert_eq!(CancellationEvent::NotHandled, pane.on_ctrl_c()); } diff --git a/codex-rs/tui2/src/bottom_pane/snapshots/codex_tui2__bottom_pane__chat_composer__tests__footer_mode_ctrl_c_interrupt.snap b/codex-rs/tui2/src/bottom_pane/snapshots/codex_tui2__bottom_pane__chat_composer__tests__footer_mode_ctrl_c_interrupt.snap index d323fda148b..d9395f2b055 100644 --- a/codex-rs/tui2/src/bottom_pane/snapshots/codex_tui2__bottom_pane__chat_composer__tests__footer_mode_ctrl_c_interrupt.snap +++ b/codex-rs/tui2/src/bottom_pane/snapshots/codex_tui2__bottom_pane__chat_composer__tests__footer_mode_ctrl_c_interrupt.snap @@ -10,4 +10,4 @@ expression: terminal.backend() " " " " " " -" ctrl + c again to interrupt " +" ctrl + c again to quit " diff --git a/codex-rs/tui2/src/bottom_pane/snapshots/codex_tui2__bottom_pane__footer__tests__footer_ctrl_c_quit_running.snap b/codex-rs/tui2/src/bottom_pane/snapshots/codex_tui2__bottom_pane__footer__tests__footer_ctrl_c_quit_running.snap index 98bc87b38ee..157853e73d5 100644 --- a/codex-rs/tui2/src/bottom_pane/snapshots/codex_tui2__bottom_pane__footer__tests__footer_ctrl_c_quit_running.snap +++ b/codex-rs/tui2/src/bottom_pane/snapshots/codex_tui2__bottom_pane__footer__tests__footer_ctrl_c_quit_running.snap @@ -2,4 +2,4 @@ source: tui2/src/bottom_pane/footer.rs expression: terminal.backend() --- -" ctrl + c again to interrupt " +" ctrl + c again to quit " diff --git a/codex-rs/tui2/src/chatwidget.rs b/codex-rs/tui2/src/chatwidget.rs index 1fe09e62a42..6974d7c3d77 100644 --- a/codex-rs/tui2/src/chatwidget.rs +++ b/codex-rs/tui2/src/chatwidget.rs @@ -26,6 +26,7 @@ use std::collections::VecDeque; use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; +use std::time::Instant; use codex_app_server_protocol::AuthMode; use codex_backend_client::Client as BackendClient; @@ -116,6 +117,7 @@ use crate::bottom_pane::BottomPane; use crate::bottom_pane::BottomPaneParams; use crate::bottom_pane::CancellationEvent; use crate::bottom_pane::InputResult; +use crate::bottom_pane::QUIT_SHORTCUT_TIMEOUT; use crate::bottom_pane::SelectionAction; use crate::bottom_pane::SelectionItem; use crate::bottom_pane::SelectionViewParams; @@ -132,6 +134,8 @@ use crate::history_cell::AgentMessageCell; use crate::history_cell::HistoryCell; use crate::history_cell::McpToolCallCell; use crate::history_cell::PlainHistoryCell; +use crate::key_hint; +use crate::key_hint::KeyBinding; use crate::markdown::append_markdown; use crate::render::Insets; use crate::render::renderable::ColumnRenderable; @@ -375,6 +379,14 @@ pub(crate) struct ChatWidget { queued_user_messages: VecDeque, // Pending notification to show when unfocused on next Draw pending_notification: Option, + /// When `Some`, the user has pressed a quit shortcut and the second press + /// must occur before `quit_shortcut_expires_at`. + quit_shortcut_expires_at: Option, + /// Tracks which quit shortcut key was pressed first. + /// + /// We require the second press to match this key so `Ctrl+C` followed by + /// `Ctrl+D` (or vice versa) doesn't quit accidentally. + quit_shortcut_key: Option, // Simple review mode flag; used to adjust layout and banners. is_review_mode: bool, // Snapshot of token usage to restore after review mode exits. @@ -610,7 +622,9 @@ impl ChatWidget { fn on_task_started(&mut self) { self.agent_turn_running = true; - self.bottom_pane.clear_ctrl_c_quit_hint(); + self.bottom_pane.clear_quit_shortcut_hint(); + self.quit_shortcut_expires_at = None; + self.quit_shortcut_key = None; self.update_task_running_state(); self.retry_status_header = None; self.bottom_pane.set_interrupt_hint_visible(true); @@ -1429,6 +1443,8 @@ impl ChatWidget { show_welcome_banner: is_first_run, suppress_session_configured_redraw: false, pending_notification: None, + quit_shortcut_expires_at: None, + quit_shortcut_key: None, is_review_mode: false, pre_review_token_info: None, needs_final_message_separator: false, @@ -1518,6 +1534,8 @@ impl ChatWidget { show_welcome_banner: false, suppress_session_configured_redraw: true, pending_notification: None, + quit_shortcut_expires_at: None, + quit_shortcut_key: None, is_review_mode: false, pre_review_token_info: None, needs_final_message_separator: false, @@ -1545,6 +1563,19 @@ impl ChatWidget { self.on_ctrl_c(); return; } + KeyEvent { + code: KeyCode::Char(c), + modifiers, + kind: KeyEventKind::Press, + .. + } if modifiers.contains(KeyModifiers::CONTROL) && c.eq_ignore_ascii_case(&'d') => { + if self.on_ctrl_d() { + return; + } + self.bottom_pane.clear_quit_shortcut_hint(); + self.quit_shortcut_expires_at = None; + self.quit_shortcut_key = None; + } KeyEvent { code: KeyCode::Char(c), modifiers, @@ -1573,7 +1604,9 @@ impl ChatWidget { return; } other if other.kind == KeyEventKind::Press => { - self.bottom_pane.clear_ctrl_c_quit_hint(); + self.bottom_pane.clear_quit_shortcut_hint(); + self.quit_shortcut_expires_at = None; + self.quit_shortcut_key = None; } _ => {} } @@ -2178,27 +2211,19 @@ impl ChatWidget { /// Exit the UI immediately without waiting for shutdown. /// - /// Prefer [`Self::request_quit_with_confirmation`] or - /// [`Self::request_quit_without_confirmation`] for user-initiated exits; + /// Prefer [`Self::request_quit_without_confirmation`] for user-initiated exits; /// this is mainly a fallback for shutdown completion or emergency exits. fn request_immediate_exit(&self) { self.app_event_tx.send(AppEvent::Exit(ExitMode::Immediate)); } - /// Request a shutdown-first quit that shows the confirmation prompt. - fn request_quit_with_confirmation(&self) { - self.app_event_tx - .send(AppEvent::Exit(ExitMode::ShutdownFirst { confirm: true })); - } - - /// Request a shutdown-first quit that skips the confirmation prompt. + /// Request a shutdown-first quit. /// - /// Use this for explicit quit commands (`/quit`, `/exit`, `/logout`) when - /// we want shutdown+exit without prompting. Slash commands are less likely - /// to be accidental, so the intent to quit is clear. + /// This is used for explicit quit commands (`/quit`, `/exit`, `/logout`) and for + /// the double-press Ctrl+C/Ctrl+D quit shortcut. fn request_quit_without_confirmation(&self) { self.app_event_tx - .send(AppEvent::Exit(ExitMode::ShutdownFirst { confirm: false })); + .send(AppEvent::Exit(ExitMode::ShutdownFirst)); } fn request_redraw(&mut self) { @@ -2964,73 +2989,6 @@ impl ChatWidget { None } - /// Show the shutdown-first quit confirmation prompt. - /// - /// This uses a selection view with explicit shutdown actions and an option - /// to persist the "don't ask again" notice. - pub(crate) fn open_exit_confirmation_prompt(&mut self) { - if !self.bottom_pane.can_launch_external_editor() { - return; - } - - let mut header_children: Vec> = Vec::new(); - header_children.push(Box::new(Line::from("Quit Codex?").bold())); - let info_line = Line::from(vec![ - "You pressed ".into(), - "Ctrl+C".bold(), - " or ".into(), - "Ctrl+D".bold(), - ". Quitting will shut down the current session and exit Codex.".into(), - ]); - header_children.push(Box::new( - Paragraph::new(vec![info_line]).wrap(Wrap { trim: false }), - )); - let header = ColumnRenderable::with(header_children); - - let quit_actions: Vec = vec![Box::new(|tx| { - // Shutdown-first quit. - tx.send(AppEvent::CodexOp(Op::Shutdown)); - })]; - - let quit_and_remember_actions: Vec = vec![Box::new(|tx| { - // Persist the notice and then shut down. - tx.send(AppEvent::UpdateExitConfirmationPromptHidden(true)); - tx.send(AppEvent::PersistExitConfirmationPromptHidden); - tx.send(AppEvent::CodexOp(Op::Shutdown)); - })]; - - let items = vec![ - SelectionItem { - name: "Yes, quit Codex".to_string(), - description: Some("Shut down the current session and exit.".to_string()), - actions: quit_actions, - dismiss_on_select: true, - ..Default::default() - }, - SelectionItem { - name: "Yes, and don't ask again".to_string(), - description: Some("Quit now and skip this prompt next time.".to_string()), - actions: quit_and_remember_actions, - dismiss_on_select: true, - ..Default::default() - }, - SelectionItem { - name: "No, stay in Codex".to_string(), - description: Some("Return to the current session.".to_string()), - actions: Vec::new(), - dismiss_on_select: true, - ..Default::default() - }, - ]; - - self.bottom_pane.show_selection_view(SelectionViewParams { - footer_hint: Some(standard_popup_hint_line()), - items, - header: Box::new(header), - ..Default::default() - }); - } - pub(crate) fn open_full_access_confirmation(&mut self, preset: ApprovalPreset) { let approval = preset.approval; let sandbox = preset.sandbox; @@ -3530,19 +3488,6 @@ impl ChatWidget { } } - /// Update the in-memory hide flag for the exit confirmation prompt. - pub(crate) fn set_exit_confirmation_prompt_hidden(&mut self, hidden: bool) { - self.config.notices.hide_exit_confirmation_prompt = Some(hidden); - } - - /// Return whether the exit confirmation prompt should be suppressed. - pub(crate) fn exit_confirmation_prompt_hidden(&self) -> bool { - self.config - .notices - .hide_exit_confirmation_prompt - .unwrap_or(false) - } - #[cfg_attr(not(target_os = "windows"), allow(dead_code))] pub(crate) fn world_writable_warning_hidden(&self) -> bool { self.config @@ -3592,17 +3537,65 @@ impl ChatWidget { /// Handle Ctrl-C key press. fn on_ctrl_c(&mut self) { + let key = key_hint::ctrl(KeyCode::Char('c')); + if self.quit_shortcut_active_for(key) { + self.quit_shortcut_expires_at = None; + self.quit_shortcut_key = None; + self.request_quit_without_confirmation(); + return; + } + + self.arm_quit_shortcut(key); + if self.bottom_pane.on_ctrl_c() == CancellationEvent::Handled { return; } if self.is_cancellable_work_active() { - self.bottom_pane.show_ctrl_c_quit_hint(); self.submit_op(Op::Interrupt); - return; + } + } + + /// Handle Ctrl-D key press. + /// + /// Ctrl-D only participates in quit when the composer is empty and no modal/popup is active. + /// Otherwise it should be routed to the active view and not attempt to quit. + fn on_ctrl_d(&mut self) -> bool { + let key = key_hint::ctrl(KeyCode::Char('d')); + if self.quit_shortcut_active_for(key) { + self.quit_shortcut_expires_at = None; + self.quit_shortcut_key = None; + self.request_quit_without_confirmation(); + return true; } - self.request_quit_with_confirmation(); + if !self.bottom_pane.composer_is_empty() || !self.bottom_pane.can_launch_external_editor() { + return false; + } + + self.arm_quit_shortcut(key); + true + } + + /// True if `key` matches the armed quit shortcut and the window has not expired. + fn quit_shortcut_active_for(&self, key: KeyBinding) -> bool { + self.quit_shortcut_key == Some(key) + && self + .quit_shortcut_expires_at + .is_some_and(|expires_at| Instant::now() < expires_at) + } + + /// Arm the double-press quit shortcut and show the footer hint. + /// + /// This keeps the state machine (`quit_shortcut_*`) in `ChatWidget`, since + /// it is the component that interprets Ctrl+C vs Ctrl+D and decides whether + /// quitting is currently allowed, while delegating rendering to `BottomPane`. + fn arm_quit_shortcut(&mut self, key: KeyBinding) { + self.quit_shortcut_expires_at = Instant::now() + .checked_add(QUIT_SHORTCUT_TIMEOUT) + .or_else(|| Some(Instant::now())); + self.quit_shortcut_key = Some(key); + self.bottom_pane.show_quit_shortcut_hint(key); } // Review mode counts as cancellable work so Ctrl+C interrupts instead of quitting. diff --git a/codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__exit_confirmation_popup.snap b/codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__exit_confirmation_popup.snap deleted file mode 100644 index bd2bfac0d5d..00000000000 --- a/codex-rs/tui2/src/chatwidget/snapshots/codex_tui2__chatwidget__tests__exit_confirmation_popup.snap +++ /dev/null @@ -1,13 +0,0 @@ ---- -source: tui2/src/chatwidget/tests.rs -expression: popup ---- - Quit Codex? - You pressed Ctrl+C or Ctrl+D. Quitting will shut down the current session - and exit Codex. - -› 1. Yes, quit Codex Shut down the current session and exit. - 2. Yes, and don't ask again Quit now and skip this prompt next time. - 3. No, stay in Codex Return to the current session. - - Press enter to confirm or esc to go back diff --git a/codex-rs/tui2/src/chatwidget/tests.rs b/codex-rs/tui2/src/chatwidget/tests.rs index c8de28b4f13..f586c4786ab 100644 --- a/codex-rs/tui2/src/chatwidget/tests.rs +++ b/codex-rs/tui2/src/chatwidget/tests.rs @@ -418,6 +418,8 @@ async fn make_chatwidget_manual( queued_user_messages: VecDeque::new(), suppress_session_configured_redraw: false, pending_notification: None, + quit_shortcut_expires_at: None, + quit_shortcut_key: None, is_review_mode: false, pre_review_token_info: None, needs_final_message_separator: false, @@ -1055,7 +1057,7 @@ async fn streaming_final_answer_keeps_task_running_state() { Ok(Op::Interrupt) => {} other => panic!("expected Op::Interrupt, got {other:?}"), } - assert!(chat.bottom_pane.ctrl_c_quit_hint_visible()); + assert!(chat.bottom_pane.quit_shortcut_hint_visible()); } #[tokio::test] @@ -1064,10 +1066,39 @@ async fn ctrl_c_shutdown_ignores_caps_lock() { chat.handle_key_event(KeyEvent::new(KeyCode::Char('C'), KeyModifiers::CONTROL)); - assert_matches!( - rx.try_recv(), - Ok(AppEvent::Exit(ExitMode::ShutdownFirst { confirm: true })) + assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); + + chat.handle_key_event(KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL)); + + assert_matches!(rx.try_recv(), Ok(AppEvent::Exit(ExitMode::ShutdownFirst))); +} + +#[tokio::test] +async fn ctrl_d_double_press_quits_without_prompt() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; + + chat.handle_key_event(KeyEvent::new(KeyCode::Char('d'), KeyModifiers::CONTROL)); + assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); + + let width = 100; + let height = chat.desired_height(width); + let mut terminal = + ratatui::Terminal::new(VT100Backend::new(width, height)).expect("create terminal"); + terminal.set_viewport_area(Rect::new(0, 0, width, height)); + terminal + .draw(|f| chat.render(f.area(), f.buffer_mut())) + .expect("draw after ctrl+d"); + assert!( + terminal + .backend() + .vt100() + .screen() + .contents() + .contains("ctrl + d again to quit") ); + + chat.handle_key_event(KeyEvent::new(KeyCode::Char('d'), KeyModifiers::CONTROL)); + assert_matches!(rx.try_recv(), Ok(AppEvent::Exit(ExitMode::ShutdownFirst))); } #[tokio::test] @@ -1096,7 +1127,7 @@ async fn ctrl_c_cleared_prompt_is_recoverable_via_history() { chat.handle_key_event(KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL)); assert!(chat.bottom_pane.composer_text().is_empty()); assert_matches!(op_rx.try_recv(), Err(TryRecvError::Empty)); - assert!(chat.bottom_pane.ctrl_c_quit_hint_visible()); + assert!(chat.bottom_pane.quit_shortcut_hint_visible()); chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE)); let restored_text = chat.bottom_pane.composer_text(); @@ -1105,7 +1136,7 @@ async fn ctrl_c_cleared_prompt_is_recoverable_via_history() { "expected placeholder {placeholder:?} after history recall" ); assert!(restored_text.starts_with("draft message ")); - assert!(!chat.bottom_pane.ctrl_c_quit_hint_visible()); + assert!(!chat.bottom_pane.quit_shortcut_hint_visible()); let images = chat.bottom_pane.take_recent_submission_images(); assert!( @@ -1302,10 +1333,7 @@ async fn slash_quit_requests_exit() { chat.dispatch_command(SlashCommand::Quit); - assert_matches!( - rx.try_recv(), - Ok(AppEvent::Exit(ExitMode::ShutdownFirst { confirm: false })) - ); + assert_matches!(rx.try_recv(), Ok(AppEvent::Exit(ExitMode::ShutdownFirst))); } #[tokio::test] @@ -1314,10 +1342,7 @@ async fn slash_exit_requests_exit() { chat.dispatch_command(SlashCommand::Exit); - assert_matches!( - rx.try_recv(), - Ok(AppEvent::Exit(ExitMode::ShutdownFirst { confirm: false })) - ); + assert_matches!(rx.try_recv(), Ok(AppEvent::Exit(ExitMode::ShutdownFirst))); } #[tokio::test] @@ -1771,16 +1796,6 @@ fn render_bottom_popup(chat: &ChatWidget, width: u16) -> String { lines.join("\n") } -#[tokio::test] -async fn exit_confirmation_popup_snapshot() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; - - chat.open_exit_confirmation_prompt(); - - let popup = render_bottom_popup(&chat, 80); - assert_snapshot!("exit_confirmation_popup", popup); -} - #[tokio::test] async fn model_selection_popup_snapshot() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5-codex")).await; diff --git a/docs/exit-confirmation-prompt-design.md b/docs/exit-confirmation-prompt-design.md index 13adf1cf992..70a7c063de9 100644 --- a/docs/exit-confirmation-prompt-design.md +++ b/docs/exit-confirmation-prompt-design.md @@ -1,32 +1,31 @@ # Exit and shutdown flow (tui + tui2) -This document describes how exit, shutdown, and interruption work in the Rust TUIs -(`codex-rs/tui` and `codex-rs/tui2`). It is intended for Codex developers and -Codex itself when reasoning about future exit/shutdown changes. +This document describes how exit, shutdown, and interruption work in the Rust TUIs (`codex-rs/tui` +and `codex-rs/tui2`). It is intended for Codex developers and Codex itself when reasoning about +future exit/shutdown changes. -This doc replaces earlier separate history and design notes. High-level history is -summarized below; full details are captured in PR #8936. +This doc replaces earlier separate history and design notes. High-level history is summarized +below; full details are captured in PR #8936. ## Terms - **Exit**: end the UI event loop and terminate the process. -- **Shutdown**: request a graceful agent/core shutdown (`Op::Shutdown`) and wait - for `ShutdownComplete` so cleanup can run. +- **Shutdown**: request a graceful agent/core shutdown (`Op::Shutdown`) and wait for + `ShutdownComplete` so cleanup can run. - **Interrupt**: cancel a running operation (`Op::Interrupt`). ## Event model (AppEvent) Exit is coordinated via a single event with explicit modes: -- `AppEvent::Exit(ExitMode::ShutdownFirst { confirm })` +- `AppEvent::Exit(ExitMode::ShutdownFirst)` - Prefer this for user-initiated quits so cleanup runs. - `AppEvent::Exit(ExitMode::Immediate)` - Escape hatch for immediate exit. This bypasses shutdown and can drop in-flight work (e.g., tasks, rollout flush, child process cleanup). -`App` is the coordinator: it decides whether to open the confirmation prompt or -submit `Op::Shutdown`, and it exits the UI loop only when `ExitMode::Immediate` -arrives (typically after `ShutdownComplete`). +`App` is the coordinator: it submits `Op::Shutdown` and it exits the UI loop only when +`ExitMode::Immediate` arrives (typically after `ShutdownComplete`). ## User-triggered quit flows @@ -37,20 +36,25 @@ Priority order in the UI layer: 1. Active modal/view gets the first chance to consume (`BottomPane::on_ctrl_c`). - If the modal handles it, the quit flow stops. 2. If cancellable work is active (streaming/tools/review), send `Op::Interrupt`. -3. If composer has draft input, clear the draft and show the quit hint. -4. If idle + empty, request shutdown-first quit with confirmation. +3. If the composer has draft input, clear the draft. +4. Always show the quit hint (`ctrl + c again to quit`) below the composer prompt. + - The hint text matches the key that was pressed (e.g., `ctrl + d again to quit`). + - The hint times out after 1 second. +5. If the user presses Ctrl+C again while the hint is visible, request shutdown-first quit. + - The second press must match the first key (Ctrl+C then Ctrl+D will not quit). ### Ctrl+D -- Only triggers quit when the composer is empty **and** no modal is active. -- With any modal/popup open, key events are routed to the view and Ctrl+D does - not attempt to quit. +- Only participates in quit when the composer is empty **and** no modal is active. + - On first press, show the quit hint (same as Ctrl+C) and start the 1 second timer. + - If pressed again while the hint is visible, request shutdown-first quit. +- With any modal/popup open, key events are routed to the view and Ctrl+D does not attempt to + quit. ### Slash commands - `/quit`, `/exit`, `/logout` request shutdown-first quit **without** a prompt, - because slash commands are harder to trigger accidentally and imply clear - intent to quit. + because slash commands are harder to trigger accidentally and imply clear intent to quit. ### /new @@ -59,36 +63,13 @@ Priority order in the UI layer: ## Shutdown completion and suppression -`ShutdownComplete` is the signal that core cleanup has finished. The UI treats -it as the boundary for exit: +`ShutdownComplete` is the signal that core cleanup has finished. The UI treats it as the boundary +for exit: - `ChatWidget` requests `Exit(Immediate)` on `ShutdownComplete`. - `App` can suppress a single `ShutdownComplete` when shutdown is used as a cleanup step (e.g., `/new`). -## Exit confirmation prompt - -The confirmation prompt is a safety net for idle quits. When shown, it provides: - -- Quit now (shutdown-first). -- Quit and don't ask again (persists the notice, then shutdown-first). -- Cancel (stay in the app). - -The prompt is a bottom-pane selection view, so it does not appear if another -modal is already active. - -## Configuration - -The prompt can be suppressed via: - -```toml -[notice] -hide_exit_confirmation_prompt = true -``` - -This flag is updated and persisted via `UpdateExitConfirmationPromptHidden` and -`PersistExitConfirmationPromptHidden`. - ## Edge cases and invariants - **Review mode** counts as cancellable work. Ctrl+C should interrupt review, not @@ -103,14 +84,13 @@ This flag is updated and persisted via `UpdateExitConfirmationPromptHidden` and At a minimum, we want coverage for: - Ctrl+C while working interrupts, does not quit. -- Ctrl+C while idle and empty shows confirmation, then shutdown-first quit. +- Ctrl+C while idle and empty shows quit hint, then shutdown-first quit on second press. - Ctrl+D with modal open does not quit. - `/quit` / `/exit` / `/logout` quit without prompt, but still shutdown-first. -- "Don't ask again" persists the notice and suppresses future prompts. + - Ctrl+D while idle and empty shows quit hint, then shutdown-first quit on second press. ## History (high level) -Codex has historically mixed "exit immediately" and "shutdown-first" across -quit gestures, largely due to incremental changes and regressions in state -tracking. This doc reflects the current unified, shutdown-first approach. See -PR #8936 for the detailed history and rationale. +Codex has historically mixed "exit immediately" and "shutdown-first" across quit gestures, largely +due to incremental changes and regressions in state tracking. This doc reflects the current +unified, shutdown-first approach. See PR #8936 for the detailed history and rationale. From 4a4070f802eee20fe44e64c4e2ee04a2d2ddf108 Mon Sep 17 00:00:00 2001 From: Josh McKinney Date: Tue, 13 Jan 2026 12:23:54 -0800 Subject: [PATCH 5/7] docs(tui): clarify quit/exit routing Document the exit event model and how quit/interrupt behavior is layered between ChatWidget and BottomPane. This makes the double-press quit shortcut easier to reason about and captures the current ordering caveat where the second Ctrl+C press is checked before offering Ctrl+C to the bottom pane. --- codex-rs/tui/src/app_event.rs | 15 +++++++++++ codex-rs/tui/src/bottom_pane/footer.rs | 20 ++++++++++++++ codex-rs/tui/src/bottom_pane/mod.rs | 35 ++++++++++++++++++++++--- codex-rs/tui/src/chatwidget.rs | 28 +++++++++++++++----- codex-rs/tui2/src/app_event.rs | 15 +++++++++++ codex-rs/tui2/src/bottom_pane/footer.rs | 20 ++++++++++++++ codex-rs/tui2/src/bottom_pane/mod.rs | 35 ++++++++++++++++++++++--- codex-rs/tui2/src/chatwidget.rs | 28 +++++++++++++++----- docs/exit-confirmation-prompt-design.md | 18 ++++++------- 9 files changed, 185 insertions(+), 29 deletions(-) diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 066d96dbc9b..6ce5d2cae54 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -1,3 +1,13 @@ +//! Application-level events used to coordinate UI actions. +//! +//! `AppEvent` is the internal message bus between UI components and the top-level `App` loop. +//! Widgets emit events to request actions that must be handled at the app layer (like opening +//! pickers, persisting configuration, or shutting down the agent), without needing direct access to +//! `App` internals. +//! +//! Exit is modelled explicitly via `AppEvent::Exit(ExitMode)` so callers can request shutdown-first +//! quits without reaching into the app loop or coupling to shutdown/exit sequencing. + use std::path::PathBuf; use codex_common::approval_presets::ApprovalPreset; @@ -220,6 +230,11 @@ pub(crate) enum AppEvent { LaunchExternalEditor, } +/// The exit strategy requested by the UI layer. +/// +/// Most user-initiated exits should use `ShutdownFirst` so core cleanup runs and the UI exits only +/// after core acknowledges completion. `Immediate` is an escape hatch for cases where shutdown has +/// already completed (or is being bypassed) and the UI loop should terminate right away. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum ExitMode { /// Shutdown core and exit after completion. diff --git a/codex-rs/tui/src/bottom_pane/footer.rs b/codex-rs/tui/src/bottom_pane/footer.rs index 66fc3b2c865..42c0392a611 100644 --- a/codex-rs/tui/src/bottom_pane/footer.rs +++ b/codex-rs/tui/src/bottom_pane/footer.rs @@ -1,3 +1,13 @@ +//! The bottom-pane footer renders transient hints and context indicators. +//! +//! The footer is pure rendering: it formats `FooterProps` into `Line`s without mutating any state. +//! It intentionally does not decide *which* footer content should be shown; that is owned by the +//! `ChatComposer` (which selects a `FooterMode`) and by higher-level state machines like +//! `ChatWidget` (which decides when quit/interrupt is allowed). +//! +//! Some footer content is time-based rather than event-based, such as the "press again to quit" +//! hint. The owning widgets schedule redraws so time-based hints can expire even if the UI is +//! otherwise idle. #[cfg(target_os = "linux")] use crate::clipboard_paste::is_probably_wsl; use crate::key_hint; @@ -14,6 +24,12 @@ use ratatui::text::Span; use ratatui::widgets::Paragraph; use ratatui::widgets::Widget; +/// The rendering inputs for the footer area under the composer. +/// +/// Callers are expected to construct `FooterProps` from higher-level state (`ChatComposer`, +/// `BottomPane`, and `ChatWidget`) and pass it to `render_footer`. The footer treats these values as +/// authoritative and does not attempt to infer missing state (for example, it does not query +/// whether a task is running). #[derive(Clone, Copy, Debug)] pub(crate) struct FooterProps { pub(crate) mode: FooterMode, @@ -29,6 +45,10 @@ pub(crate) struct FooterProps { pub(crate) context_window_used_tokens: Option, } +/// Selects which footer content is rendered. +/// +/// The current mode is owned by `ChatComposer`, which may override it based on transient state +/// (for example, showing `QuitShortcutReminder` only while its timer is active). #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub(crate) enum FooterMode { /// Transient "press again to quit" reminder (Ctrl+C/Ctrl+D). diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index a7446475ae0..1ecd057648c 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -1,4 +1,18 @@ -//! Bottom pane: shows the ChatComposer or a BottomPaneView, if one is active. +//! The bottom pane is the interactive footer of the chat UI. +//! +//! The pane owns the [`ChatComposer`] (editable prompt input) and a stack of transient +//! [`BottomPaneView`]s (popups/modals) that temporarily replace the composer for focused +//! interactions like selection lists. +//! +//! Input routing is layered: `BottomPane` decides which local surface receives a key (view vs +//! composer), while higher-level intent such as "interrupt" or "quit" is decided by the parent +//! widget (`ChatWidget`). This split matters for Ctrl+C/Ctrl+D: the bottom pane gives the active +//! view the first chance to consume Ctrl+C (typically to dismiss itself), and `ChatWidget` may +//! treat an unhandled Ctrl+C as an interrupt or as the first press of a double-press quit +//! shortcut. +//! +//! Some UI is time-based rather than input-based, such as the transient "press again to quit" +//! hint. The pane schedules redraws so those hints can expire even when the UI is otherwise idle. use std::path::PathBuf; use crate::app_event_sender::AppEventSender; @@ -57,6 +71,11 @@ pub(crate) use feedback_view::FeedbackNoteView; /// Keeping a single value ensures Ctrl+C and Ctrl+D behave identically. pub(crate) const QUIT_SHORTCUT_TIMEOUT: Duration = Duration::from_secs(1); +/// The result of offering a cancellation key to a bottom-pane surface. +/// +/// This is primarily used for Ctrl+C routing: active views can consume the key to dismiss +/// themselves, and the caller can decide what higher-level action (if any) to take when the key is +/// not handled locally. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum CancellationEvent { Handled, @@ -74,6 +93,10 @@ pub(crate) use list_selection_view::SelectionAction; pub(crate) use list_selection_view::SelectionItem; /// Pane displayed in the lower half of the chat UI. +/// +/// This is the owning container for the prompt input (`ChatComposer`) and the view stack +/// (`BottomPaneView`). It performs local input routing and renders time-based hints, while leaving +/// process-level decisions (quit, interrupt, shutdown) to `ChatWidget`. pub(crate) struct BottomPane { /// Composer is retained even when a BottomPaneView is displayed so the /// input state is retained when the view is closed. @@ -227,8 +250,14 @@ impl BottomPane { } } - /// Handle Ctrl-C in the bottom pane. If a modal view is active it gets a - /// chance to consume the event (e.g. to dismiss itself). + /// Handles a Ctrl+C press within the bottom pane. + /// + /// An active modal view is given the first chance to consume the key (typically to dismiss + /// itself). If no view is active, Ctrl+C clears draft composer input. + /// + /// This method may show the quit shortcut hint as a user-visible acknowledgement that Ctrl+C + /// was received, but it does not decide whether the process should exit; `ChatWidget` owns the + /// quit/interrupt state machine and uses the result to decide what happens next. pub(crate) fn on_ctrl_c(&mut self) -> CancellationEvent { if let Some(view) = self.view_stack.last_mut() { let event = view.on_ctrl_c(); diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index f6c2b15e723..e353047b30f 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -340,12 +340,18 @@ pub(crate) enum ExternalEditorState { Active, } -/// Maintains the per-session UI state for the chat screen. +/// Maintains the per-session UI state and interaction state machines for the chat screen. /// -/// This type owns the state derived from a `codex_core::protocol` event stream (history cells, -/// active streaming buffers, bottom-pane overlays, and transient status text). It is not -/// responsible for running the agent itself; it only reflects progress by updating UI state and by -/// sending `Op` requests back to codex-core. +/// `ChatWidget` owns the state derived from the protocol event stream (history cells, streaming +/// buffers, bottom-pane overlays, and transient status text) and turns key presses into user +/// intent (`Op` submissions and `AppEvent` requests). +/// +/// It is not responsible for running the agent itself; it reflects progress by updating UI state +/// and by sending requests back to codex-core. +/// +/// Quit/interrupt behavior intentionally spans layers: the bottom pane owns local input routing +/// (which view gets Ctrl+C), while `ChatWidget` owns process-level decisions such as interrupting +/// active work, arming the double-press quit shortcut, and requesting shutdown-first exit. pub(crate) struct ChatWidget { app_event_tx: AppEventSender, codex_op_tx: UnboundedSender, @@ -3832,7 +3838,15 @@ impl ChatWidget { self.bottom_pane.on_file_search_result(query, matches); } - /// Handle Ctrl-C key press. + /// Handles a Ctrl+C press at the chat-widget layer. + /// + /// The first press arms a time-bounded quit shortcut and shows a footer hint via the bottom + /// pane. If cancellable work is active, Ctrl+C also submits `Op::Interrupt` after the shortcut + /// is armed. + /// + /// If the same quit shortcut is pressed again before expiry, this requests a shutdown-first + /// quit. The "second press" check currently runs before offering Ctrl+C to the bottom pane, + /// which means a second press can bypass modal handling. fn on_ctrl_c(&mut self) { let key = key_hint::ctrl(KeyCode::Char('c')); if self.quit_shortcut_active_for(key) { @@ -3853,7 +3867,7 @@ impl ChatWidget { } } - /// Handle Ctrl-D key press. + /// Handles a Ctrl+D press at the chat-widget layer. /// /// Ctrl-D only participates in quit when the composer is empty and no modal/popup is active. /// Otherwise it should be routed to the active view and not attempt to quit. diff --git a/codex-rs/tui2/src/app_event.rs b/codex-rs/tui2/src/app_event.rs index dfea64835a6..f24ca1e07cd 100644 --- a/codex-rs/tui2/src/app_event.rs +++ b/codex-rs/tui2/src/app_event.rs @@ -1,3 +1,13 @@ +//! Application-level events used to coordinate UI actions. +//! +//! `AppEvent` is the internal message bus between UI components and the top-level `App` loop. +//! Widgets emit events to request actions that must be handled at the app layer (like opening +//! pickers, persisting configuration, or shutting down the agent), without needing direct access to +//! `App` internals. +//! +//! Exit is modelled explicitly via `AppEvent::Exit(ExitMode)` so callers can request shutdown-first +//! quits without reaching into the app loop or coupling to shutdown/exit sequencing. + use std::path::PathBuf; use codex_common::approval_presets::ApprovalPreset; @@ -211,6 +221,11 @@ pub(crate) enum AppEvent { }, } +/// The exit strategy requested by the UI layer. +/// +/// Most user-initiated exits should use `ShutdownFirst` so core cleanup runs and the UI exits only +/// after core acknowledges completion. `Immediate` is an escape hatch for cases where shutdown has +/// already completed (or is being bypassed) and the UI loop should terminate right away. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum ExitMode { /// Shutdown core and exit after completion. diff --git a/codex-rs/tui2/src/bottom_pane/footer.rs b/codex-rs/tui2/src/bottom_pane/footer.rs index d8ff3ab188e..bdfb5c8dfc4 100644 --- a/codex-rs/tui2/src/bottom_pane/footer.rs +++ b/codex-rs/tui2/src/bottom_pane/footer.rs @@ -1,3 +1,13 @@ +//! The bottom-pane footer renders transient hints and context indicators. +//! +//! The footer is pure rendering: it formats `FooterProps` into `Line`s without mutating any state. +//! It intentionally does not decide *which* footer content should be shown; that is owned by the +//! `ChatComposer` (which selects a `FooterMode`) and by higher-level state machines like +//! `ChatWidget` (which decides when quit/interrupt is allowed). +//! +//! Some footer content is time-based rather than event-based, such as the "press again to quit" +//! hint. The owning widgets schedule redraws so time-based hints can expire even if the UI is +//! otherwise idle. #[cfg(target_os = "linux")] use crate::clipboard_paste::is_probably_wsl; use crate::key_hint; @@ -15,6 +25,12 @@ use ratatui::text::Span; use ratatui::widgets::Paragraph; use ratatui::widgets::Widget; +/// The rendering inputs for the footer area under the composer. +/// +/// Callers are expected to construct `FooterProps` from higher-level state (`ChatComposer`, +/// `BottomPane`, and `ChatWidget`) and pass it to `render_footer`. The footer treats these values as +/// authoritative and does not attempt to infer missing state (for example, it does not query +/// whether a task is running). #[derive(Clone, Copy, Debug)] pub(crate) struct FooterProps { pub(crate) mode: FooterMode, @@ -35,6 +51,10 @@ pub(crate) struct FooterProps { pub(crate) transcript_copy_feedback: Option, } +/// Selects which footer content is rendered. +/// +/// The current mode is owned by `ChatComposer`, which may override it based on transient state +/// (for example, showing `QuitShortcutReminder` only while its timer is active). #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub(crate) enum FooterMode { /// Transient "press again to quit" reminder (Ctrl+C/Ctrl+D). diff --git a/codex-rs/tui2/src/bottom_pane/mod.rs b/codex-rs/tui2/src/bottom_pane/mod.rs index 8ea23976590..8ff99d90263 100644 --- a/codex-rs/tui2/src/bottom_pane/mod.rs +++ b/codex-rs/tui2/src/bottom_pane/mod.rs @@ -1,4 +1,18 @@ -//! Bottom pane: shows the ChatComposer or a BottomPaneView, if one is active. +//! The bottom pane is the interactive footer of the chat UI. +//! +//! The pane owns the [`ChatComposer`] (editable prompt input) and a stack of transient +//! [`BottomPaneView`]s (popups/modals) that temporarily replace the composer for focused +//! interactions like selection lists. +//! +//! Input routing is layered: `BottomPane` decides which local surface receives a key (view vs +//! composer), while higher-level intent such as "interrupt" or "quit" is decided by the parent +//! widget (`ChatWidget`). This split matters for Ctrl+C/Ctrl+D: the bottom pane gives the active +//! view the first chance to consume Ctrl+C (typically to dismiss itself), and `ChatWidget` may +//! treat an unhandled Ctrl+C as an interrupt or as the first press of a double-press quit +//! shortcut. +//! +//! Some UI is time-based rather than input-based, such as the transient "press again to quit" +//! hint. The pane schedules redraws so those hints can expire even when the UI is otherwise idle. use std::path::PathBuf; use crate::app_event_sender::AppEventSender; @@ -54,6 +68,11 @@ pub(crate) use feedback_view::FeedbackNoteView; /// Keeping a single value ensures Ctrl+C and Ctrl+D behave identically. pub(crate) const QUIT_SHORTCUT_TIMEOUT: Duration = Duration::from_secs(1); +/// The result of offering a cancellation key to a bottom-pane surface. +/// +/// This is primarily used for Ctrl+C routing: active views can consume the key to dismiss +/// themselves, and the caller can decide what higher-level action (if any) to take when the key is +/// not handled locally. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum CancellationEvent { Handled, @@ -69,6 +88,10 @@ pub(crate) use list_selection_view::SelectionAction; pub(crate) use list_selection_view::SelectionItem; /// Pane displayed in the lower half of the chat UI. +/// +/// This is the owning container for the prompt input (`ChatComposer`) and the view stack +/// (`BottomPaneView`). It performs local input routing and renders time-based hints, while leaving +/// process-level decisions (quit, interrupt, shutdown) to `ChatWidget`. pub(crate) struct BottomPane { /// Composer is retained even when a BottomPaneView is displayed so the /// input state is retained when the view is closed. @@ -219,8 +242,14 @@ impl BottomPane { } } - /// Handle Ctrl-C in the bottom pane. If a modal view is active it gets a - /// chance to consume the event (e.g. to dismiss itself). + /// Handles a Ctrl+C press within the bottom pane. + /// + /// An active modal view is given the first chance to consume the key (typically to dismiss + /// itself). If no view is active, Ctrl+C clears draft composer input. + /// + /// This method may show the quit shortcut hint as a user-visible acknowledgement that Ctrl+C + /// was received, but it does not decide whether the process should exit; `ChatWidget` owns the + /// quit/interrupt state machine and uses the result to decide what happens next. pub(crate) fn on_ctrl_c(&mut self) -> CancellationEvent { if let Some(view) = self.view_stack.last_mut() { let event = view.on_ctrl_c(); diff --git a/codex-rs/tui2/src/chatwidget.rs b/codex-rs/tui2/src/chatwidget.rs index 6974d7c3d77..3aae3dba84a 100644 --- a/codex-rs/tui2/src/chatwidget.rs +++ b/codex-rs/tui2/src/chatwidget.rs @@ -308,12 +308,18 @@ enum RateLimitSwitchPromptState { Shown, } -/// Maintains the per-session UI state for the chat screen. +/// Maintains the per-session UI state and interaction state machines for the chat screen. /// -/// This type owns the state derived from a `codex_core::protocol` event stream (history cells, -/// active streaming buffers, bottom-pane overlays, and transient status text). It is not -/// responsible for running the agent itself; it only reflects progress by updating UI state and by -/// sending `Op` requests back to codex-core. +/// `ChatWidget` owns the state derived from the protocol event stream (history cells, streaming +/// buffers, bottom-pane overlays, and transient status text) and turns key presses into user +/// intent (`Op` submissions and `AppEvent` requests). +/// +/// It is not responsible for running the agent itself; it reflects progress by updating UI state +/// and by sending requests back to codex-core. +/// +/// Quit/interrupt behavior intentionally spans layers: the bottom pane owns local input routing +/// (which view gets Ctrl+C), while `ChatWidget` owns process-level decisions such as interrupting +/// active work, arming the double-press quit shortcut, and requesting shutdown-first exit. pub(crate) struct ChatWidget { app_event_tx: AppEventSender, codex_op_tx: UnboundedSender, @@ -3535,7 +3541,15 @@ impl ChatWidget { self.bottom_pane.on_file_search_result(query, matches); } - /// Handle Ctrl-C key press. + /// Handles a Ctrl+C press at the chat-widget layer. + /// + /// The first press arms a time-bounded quit shortcut and shows a footer hint via the bottom + /// pane. If cancellable work is active, Ctrl+C also submits `Op::Interrupt` after the shortcut + /// is armed. + /// + /// If the same quit shortcut is pressed again before expiry, this requests a shutdown-first + /// quit. The "second press" check currently runs before offering Ctrl+C to the bottom pane, + /// which means a second press can bypass modal handling. fn on_ctrl_c(&mut self) { let key = key_hint::ctrl(KeyCode::Char('c')); if self.quit_shortcut_active_for(key) { @@ -3556,7 +3570,7 @@ impl ChatWidget { } } - /// Handle Ctrl-D key press. + /// Handles a Ctrl+D press at the chat-widget layer. /// /// Ctrl-D only participates in quit when the composer is empty and no modal/popup is active. /// Otherwise it should be routed to the active view and not attempt to quit. diff --git a/docs/exit-confirmation-prompt-design.md b/docs/exit-confirmation-prompt-design.md index 70a7c063de9..d818764ef9d 100644 --- a/docs/exit-confirmation-prompt-design.md +++ b/docs/exit-confirmation-prompt-design.md @@ -33,15 +33,15 @@ Exit is coordinated via a single event with explicit modes: Priority order in the UI layer: -1. Active modal/view gets the first chance to consume (`BottomPane::on_ctrl_c`). - - If the modal handles it, the quit flow stops. -2. If cancellable work is active (streaming/tools/review), send `Op::Interrupt`. -3. If the composer has draft input, clear the draft. -4. Always show the quit hint (`ctrl + c again to quit`) below the composer prompt. - - The hint text matches the key that was pressed (e.g., `ctrl + d again to quit`). - - The hint times out after 1 second. -5. If the user presses Ctrl+C again while the hint is visible, request shutdown-first quit. - - The second press must match the first key (Ctrl+C then Ctrl+D will not quit). +1. The quit shortcut state machine is checked first. + - If the user has already armed Ctrl+C and the 1 second window has not expired, the second + Ctrl+C triggers shutdown-first quit immediately. +2. If no quit shortcut is armed, `ChatWidget` arms it and shows the quit hint (`ctrl + c again to + quit`) for 1 second. +3. `ChatWidget` then offers Ctrl+C to the bottom pane via `BottomPane::on_ctrl_c`. + - This gives active modals/views a chance to consume Ctrl+C (typically to dismiss themselves), + but it also means the hint can remain armed even when a modal consumes the first press. +4. If cancellable work is active (streaming/tools/review), `ChatWidget` submits `Op::Interrupt`. ### Ctrl+D From df2d9663a67c77cc455e03cc133736dc0233cd4b Mon Sep 17 00:00:00 2001 From: Josh McKinney Date: Tue, 13 Jan 2026 12:45:25 -0800 Subject: [PATCH 6/7] fix(tui): keep modal priority for double-press quit A second Ctrl+C within the quit shortcut window no longer bypasses bottom-pane modal handling. Use a purpose-named "no modal/popup" predicate for Ctrl+D quit gating instead of the external-editor helper. --- codex-rs/tui/src/bottom_pane/mod.rs | 9 +++++++++ codex-rs/tui/src/chatwidget.rs | 14 +++++++------- codex-rs/tui2/src/bottom_pane/mod.rs | 9 +++++++++ codex-rs/tui2/src/chatwidget.rs | 14 +++++++------- docs/exit-confirmation-prompt-design.md | 14 ++++++-------- 5 files changed, 38 insertions(+), 22 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 1ecd057648c..34aa2f9587b 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -517,6 +517,15 @@ impl BottomPane { self.view_stack.is_empty() && !self.composer.popup_active() } + /// Returns true when the bottom pane has no active modal view and no active composer popup. + /// + /// This is the UI-level definition of "no modal/popup is active" for key routing decisions. + /// It intentionally does not include task state, since some actions are safe while a task is + /// running and some are not. + pub(crate) fn no_modal_or_popup_active(&self) -> bool { + self.can_launch_external_editor() + } + pub(crate) fn show_view(&mut self, view: Box) { self.push_view(view); } diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index e353047b30f..27fa6b60013 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -3845,10 +3845,14 @@ impl ChatWidget { /// is armed. /// /// If the same quit shortcut is pressed again before expiry, this requests a shutdown-first - /// quit. The "second press" check currently runs before offering Ctrl+C to the bottom pane, - /// which means a second press can bypass modal handling. + /// quit. fn on_ctrl_c(&mut self) { let key = key_hint::ctrl(KeyCode::Char('c')); + if self.bottom_pane.on_ctrl_c() == CancellationEvent::Handled { + self.arm_quit_shortcut(key); + return; + } + if self.quit_shortcut_active_for(key) { self.quit_shortcut_expires_at = None; self.quit_shortcut_key = None; @@ -3858,10 +3862,6 @@ impl ChatWidget { self.arm_quit_shortcut(key); - if self.bottom_pane.on_ctrl_c() == CancellationEvent::Handled { - return; - } - if self.is_cancellable_work_active() { self.submit_op(Op::Interrupt); } @@ -3880,7 +3880,7 @@ impl ChatWidget { return true; } - if !self.bottom_pane.composer_is_empty() || !self.bottom_pane.can_launch_external_editor() { + if !self.bottom_pane.composer_is_empty() || !self.bottom_pane.no_modal_or_popup_active() { return false; } diff --git a/codex-rs/tui2/src/bottom_pane/mod.rs b/codex-rs/tui2/src/bottom_pane/mod.rs index 8ff99d90263..c67129c5b59 100644 --- a/codex-rs/tui2/src/bottom_pane/mod.rs +++ b/codex-rs/tui2/src/bottom_pane/mod.rs @@ -509,6 +509,15 @@ impl BottomPane { self.view_stack.is_empty() && !self.composer.popup_active() } + /// Returns true when the bottom pane has no active modal view and no active composer popup. + /// + /// This is the UI-level definition of "no modal/popup is active" for key routing decisions. + /// It intentionally does not include task state, since some actions are safe while a task is + /// running and some are not. + pub(crate) fn no_modal_or_popup_active(&self) -> bool { + self.can_launch_external_editor() + } + pub(crate) fn show_view(&mut self, view: Box) { self.push_view(view); } diff --git a/codex-rs/tui2/src/chatwidget.rs b/codex-rs/tui2/src/chatwidget.rs index 3aae3dba84a..693ec1eb8c8 100644 --- a/codex-rs/tui2/src/chatwidget.rs +++ b/codex-rs/tui2/src/chatwidget.rs @@ -3548,10 +3548,14 @@ impl ChatWidget { /// is armed. /// /// If the same quit shortcut is pressed again before expiry, this requests a shutdown-first - /// quit. The "second press" check currently runs before offering Ctrl+C to the bottom pane, - /// which means a second press can bypass modal handling. + /// quit. fn on_ctrl_c(&mut self) { let key = key_hint::ctrl(KeyCode::Char('c')); + if self.bottom_pane.on_ctrl_c() == CancellationEvent::Handled { + self.arm_quit_shortcut(key); + return; + } + if self.quit_shortcut_active_for(key) { self.quit_shortcut_expires_at = None; self.quit_shortcut_key = None; @@ -3561,10 +3565,6 @@ impl ChatWidget { self.arm_quit_shortcut(key); - if self.bottom_pane.on_ctrl_c() == CancellationEvent::Handled { - return; - } - if self.is_cancellable_work_active() { self.submit_op(Op::Interrupt); } @@ -3583,7 +3583,7 @@ impl ChatWidget { return true; } - if !self.bottom_pane.composer_is_empty() || !self.bottom_pane.can_launch_external_editor() { + if !self.bottom_pane.composer_is_empty() || !self.bottom_pane.no_modal_or_popup_active() { return false; } diff --git a/docs/exit-confirmation-prompt-design.md b/docs/exit-confirmation-prompt-design.md index d818764ef9d..b5e8aa7de3e 100644 --- a/docs/exit-confirmation-prompt-design.md +++ b/docs/exit-confirmation-prompt-design.md @@ -33,14 +33,12 @@ Exit is coordinated via a single event with explicit modes: Priority order in the UI layer: -1. The quit shortcut state machine is checked first. - - If the user has already armed Ctrl+C and the 1 second window has not expired, the second - Ctrl+C triggers shutdown-first quit immediately. -2. If no quit shortcut is armed, `ChatWidget` arms it and shows the quit hint (`ctrl + c again to - quit`) for 1 second. -3. `ChatWidget` then offers Ctrl+C to the bottom pane via `BottomPane::on_ctrl_c`. - - This gives active modals/views a chance to consume Ctrl+C (typically to dismiss themselves), - but it also means the hint can remain armed even when a modal consumes the first press. +1. Active modal/view gets the first chance to consume (`BottomPane::on_ctrl_c`). + - If the modal handles it, the quit flow stops. +2. If the user has already armed Ctrl+C and the 1 second window has not expired, the second Ctrl+C + triggers shutdown-first quit immediately. +3. Otherwise, `ChatWidget` arms Ctrl+C and shows the quit hint (`ctrl + c again to quit`) for + 1 second. 4. If cancellable work is active (streaming/tools/review), `ChatWidget` submits `Op::Interrupt`. ### Ctrl+D From bef810b23c01833798b36a1ea6dd953c122da6ae Mon Sep 17 00:00:00 2001 From: Josh McKinney Date: Tue, 13 Jan 2026 12:52:05 -0800 Subject: [PATCH 7/7] fix(tui): don't arm quit when modal handles Ctrl+C --- codex-rs/tui/src/chatwidget.rs | 9 ++++++++- codex-rs/tui2/src/chatwidget.rs | 9 ++++++++- docs/exit-confirmation-prompt-design.md | 2 ++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 27fa6b60013..f90a4a3d642 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -3848,8 +3848,15 @@ impl ChatWidget { /// quit. fn on_ctrl_c(&mut self) { let key = key_hint::ctrl(KeyCode::Char('c')); + let modal_or_popup_active = !self.bottom_pane.no_modal_or_popup_active(); if self.bottom_pane.on_ctrl_c() == CancellationEvent::Handled { - self.arm_quit_shortcut(key); + if modal_or_popup_active { + self.quit_shortcut_expires_at = None; + self.quit_shortcut_key = None; + self.bottom_pane.clear_quit_shortcut_hint(); + } else { + self.arm_quit_shortcut(key); + } return; } diff --git a/codex-rs/tui2/src/chatwidget.rs b/codex-rs/tui2/src/chatwidget.rs index 693ec1eb8c8..f7d0e5e2fbd 100644 --- a/codex-rs/tui2/src/chatwidget.rs +++ b/codex-rs/tui2/src/chatwidget.rs @@ -3551,8 +3551,15 @@ impl ChatWidget { /// quit. fn on_ctrl_c(&mut self) { let key = key_hint::ctrl(KeyCode::Char('c')); + let modal_or_popup_active = !self.bottom_pane.no_modal_or_popup_active(); if self.bottom_pane.on_ctrl_c() == CancellationEvent::Handled { - self.arm_quit_shortcut(key); + if modal_or_popup_active { + self.quit_shortcut_expires_at = None; + self.quit_shortcut_key = None; + self.bottom_pane.clear_quit_shortcut_hint(); + } else { + self.arm_quit_shortcut(key); + } return; } diff --git a/docs/exit-confirmation-prompt-design.md b/docs/exit-confirmation-prompt-design.md index b5e8aa7de3e..1163e2f0ea9 100644 --- a/docs/exit-confirmation-prompt-design.md +++ b/docs/exit-confirmation-prompt-design.md @@ -35,6 +35,8 @@ Priority order in the UI layer: 1. Active modal/view gets the first chance to consume (`BottomPane::on_ctrl_c`). - If the modal handles it, the quit flow stops. + - When a modal/popup handles Ctrl+C, the quit shortcut is cleared so dismissing a modal cannot + accidentally prime a subsequent Ctrl+C to quit. 2. If the user has already armed Ctrl+C and the 1 second window has not expired, the second Ctrl+C triggers shutdown-first quit immediately. 3. Otherwise, `ChatWidget` arms Ctrl+C and shows the quit hint (`ctrl + c again to quit`) for