-
Notifications
You must be signed in to change notification settings - Fork 7.2k
tui: double-press Ctrl+C/Ctrl+D to quit #8936
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
ef650fc to
1ce1aa3
Compare
01f8853 to
bac7f58
Compare
28f6f6d to
15ca2f5
Compare
|
@codex review |
jif-oai
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think @nornagon-openai review would be good here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 15ca2f5258
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
d417acd to
01d8970
Compare
|
Added a bit more docs and fixed something that that doc review caught.
|
01d8970 to
0046d89
Compare
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.
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.
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.
Replace the quit confirmation prompt menu with a transient footer hint. The first press of Ctrl+C or Ctrl+D shows 'ctrl + <key> 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.
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.
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.
0046d89 to
bef810b
Compare
Problem
Codex’s TUI quit behavior has historically been easy to trigger accidentally and hard to reason
about.
Ctrl+C/Ctrl+Dcould terminate the UI immediately, which is a common key to press while tryingto dismiss a modal, cancel a command, or recover from a stuck state.
shutdown/cleanup work that should run before the process terminates.
This PR makes quitting both safer (harder to do by accident) and more uniform across quit
gestures, while keeping the shutdown-first semantics explicit.
Mental model
After this change, the system treats quitting as a UI request that is coordinated by the app
layer.
AppEvent::Exit(ExitMode).ExitMode::ShutdownFirstis the normal user path: the app triggersOp::Shutdown, continuesrendering while shutdown runs, and only ends the UI loop once shutdown has completed.
ExitMode::Immediateexists as an escape hatch (and as the post-shutdown “now actually exit”signal); it bypasses cleanup and should not be the default for user-triggered quits.
User-facing quit gestures are intentionally “two-step” for safety:
Ctrl+CandCtrl+Dno longer exit immediately.hint expires and the next press starts a fresh window.
Key routing remains modal-first:
Ctrl+C.Ctrl+C, any armed quit shortcut is cleared so dismissing a modal cannotprime a subsequent
Ctrl+Cto quit.Ctrl+Donly participates in quitting when the composer is empty and no modal/popup is active.The design doc
docs/exit-confirmation-prompt-design.mdcaptures the intended routing and theinvariants the UI should maintain.
Non-goals
Ctrl+C.It only ensures modals get priority and that quit arming does not leak across modal handling.
the exit gesture lightweight and consistent.
sequences it.
Tradeoffs
Ctrl+C/Ctrl+Dnow requires a deliberate second keypress, which adds friction forusers who relied on the old “instant quit” behavior.
complexity and introduces timing-dependent behavior.
This design was chosen over alternatives (a modal confirmation prompt or a long-lived “are you
sure” state) because it provides an explicit safety barrier while keeping the flow fast and
keyboard-native.
Architecture
ChatWidgetowns the quit-shortcut state machine and decides when a quit gesture is allowed(idle vs cancellable work, composer state, etc.).
BottomPaneowns rendering and local input routing for modals/popups. It is responsible forconsuming cancellation keys when a view is active and for showing/expiring the footer hint.
Appowns shutdown sequencing: translatingAppEvent::Exit(ShutdownFirst)intoOp::Shutdownand only terminating the UI loop when exit is safe.
This keeps “what should happen” decisions (quit vs interrupt vs ignore) in the chat/widget layer,
while keeping “how it looks and which view gets the key” in the bottom-pane layer.
Observability
You can tell this is working by running the TUIs and exercising the quit gestures:
Ctrl+C(orCtrl+Dwith an empty composer and no modal) shows a footerhint for ~1 second; pressing again within that window exits via shutdown-first.
Ctrl+Cinterrupts work rather than quitting.Ctrl+Cdismisses/handles the modal (if it chooses to) and does notarm a quit shortcut; a subsequent quick
Ctrl+Cshould not quit unless the user re-arms it.Failure modes are visible as:
Ctrl+C/Ctrl+D.Ctrl+C.Tests
codex-tuiandcodex-tui2to validate:Ctrl+C.These tests focus on the UI-level invariants and rendered output; they do not attempt to validate
real terminal key-repeat timing or end-to-end process shutdown behavior.
Screenshot:
