fix(terminal-input): toggle Rich input panel on Ctrl+G (#9916)#10706
Conversation
The Ctrl+G shortcut opened the CLI agent Rich input panel but did not hide it on a second press. Although PR warpdotdev#10030 added an EditorView-scoped close case after warpdotdev#9286, the predicate still failed to match in a real scenario: when the CLI agent block has transitioned out of `LongRunningCommand` (e.g., the agent paused waiting for input) and focus is not on the embedded editor (e.g., the user clicked into the block list). In that state, neither existing predicate branch matched and the keystroke was silently dropped. Make the binding a true toggle by: 1. Mirroring `CLI_AGENT_RICH_INPUT_OPEN` onto `TerminalView`'s keymap context whenever rich input is open. The flag was previously only set on the terminal input's `EditorView` keymap context. 2. Adding a third context branch to the Ctrl+G binding: `Terminal & !IMEOpen & CLI_AGENT_RICH_INPUT_OPEN`. With (1), this matches anywhere in the terminal subtree once rich input is open, regardless of focus or active-block state. The existing toggle handler in `TerminalAction::ToggleCLIAgentRichInput` already routes to `close_cli_agent_rich_input_and_disable_auto_toggle` when a session is active, so this is purely a predicate-coverage fix and reuses the existing close path used by the "Hide Rich input" button. Adds two regression tests: - `ctrl_g_closes_cli_agent_rich_input_from_terminal_context` verifies the close path fires when only the terminal view is in the responder chain (no editor focus). - `ctrl_g_toggles_cli_agent_rich_input_from_terminal_context` verifies open->close->open->close behaves as a true toggle from that context.
github-actions
Bot
added
the
external-contributor
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR mirrors the CLI agent rich-input-open state onto the terminal keymap context, adds a terminal-level Ctrl+G close predicate, and adds regression coverage for closing from terminal context. I did not find code-level correctness or security issues in the changed lines.
Concerns
- Manual testing is required for user-visible behavior changes that can be manually tested. The PR description lists the Linux/macOS and Windows manual checks as incomplete and does not include screenshots or a screen recording that show Ctrl+G opening and closing Rich input end to end, or justify why manual testing is not possible.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I reviewed this pull request and requested human review from: Comment Powered by Oz |
oz-for-oss
Bot
dismissed
their stale review
Oz no longer requests changes for this pull request after the latest automated review.
There was a problem hiding this comment.
Overview
This PR mirrors the CLI agent rich-input-open state onto the terminal keymap context and extends the Ctrl+G binding so the rich input panel can close from terminal focus as well as the embedded editor. The added regression tests cover dispatching Ctrl+G with only the terminal view in the responder chain and repeated close behavior after reopening.
Concerns
- No blocking correctness or security concerns found in the annotated diff.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
oz-for-oss
Bot
requested review from
a team and
kevinchevalier
and removed request for
a team
kevinchevalier
left a comment
kevinchevalier
left a comment
There was a problem hiding this comment.
Thanks for the fix!
2 participants
Closes #9916
Summary
Resolves #9916. Ctrl+G now toggles the CLI agent Rich input panel in both directions instead of only opening it.
Root cause
The Ctrl+G binding in
app/src/terminal/view/init.rshad two context-predicate branches:Terminal & (LongRunningCommand | AltScreen) & CLI_AGENT_FOOTER_ENABLED & CLI_AGENT_RICH_INPUT_CHIP_ENABLED— the open path.EditorView & !IMEOpen & CLI_AGENT_RICH_INPUT_OPEN— added in PR fix: close cli rich input with ctrl-g from editor #10030 to close from the focused editor (Can't exit Rich text prompt with Ctrl + g #9286).The
CLI_AGENT_RICH_INPUT_OPENflag was set only on the terminal input'sEditorViewkeymap context (inapp/src/terminal/input.rs). Predicates are evaluated per-view against that view's own context — they are not merged across the responder chain (seeMatcher::push_keystrokeincrates/warpui_core/src/keymap/matcher.rs).That left a gap: when the CLI agent block transitions out of
LongRunningCommand(e.g., the agent paused waiting for user input) AND focus is not on the embedded editor (e.g., the user clicked into the block list, or a child view of the terminal is focused), neither branch matched and the Ctrl+G keystroke was silently dropped. The "Hide Rich input" footer button kept working because it emitsUseAgentToolbarEvent::HideRichInputdirectly without going through the keybinding.Fix
A two-part predicate-coverage fix that reuses the existing toggle action and close path (no behavior change in the action handler):
app/src/terminal/view.rs— mirrorflags::CLI_AGENT_RICH_INPUT_OPENontoTerminalView'skeymap_contextwheneverCLIAgentSessionsModel::is_input_open(view_id)is true. The flag was previously only set on the editor's context modifier.app/src/terminal/view/init.rs— add a third branch to the binding predicate:With (1), this matches anywhere in the terminal subtree once rich input is open, independent of which descendant has focus and independent of
LongRunningCommand/AltScreen/ chip-visibility state.The existing
TerminalAction::ToggleCLIAgentRichInputhandler atapp/src/terminal/view.rs:26164already routes toclose_cli_agent_rich_input_and_disable_auto_togglewhen a session is active, so this is purely a predicate-coverage fix that reuses the same close path used by the "Hide Rich input" footer button.Test plan
ctrl_g_closes_cli_agent_rich_input_from_terminal_context— dispatches Ctrl+G with only the terminal view in the responder chain and asserts the session closes. This case is exactly what previously failed.ctrl_g_toggles_cli_agent_rich_input_from_terminal_context— exercises open → close → open → close from the terminal context, verifying true toggle behavior.ctrl_g_closes_cli_agent_rich_input_when_editor_is_focused(for Can't exit Rich text prompt with Ctrl + g #9286) still covers the EditorView path.Notes
cargo check -p warprequires the Xcode Metal toolchain (not just Command Line Tools) on macOS, so I could not run the test suite in this environment. The changes are deliberately small, idiomatic, and pattern-match the existing predicate /keymap_contextstyle in the file. CI will exercise full compilation and the new tests.TerminalAction::ToggleCLIAgentRichInput, which is idempotent w.r.t. session state and bails early if the rich-input session is already in the target state.Cargo.lock, CI, or unrelated files were touched.Manual testing note
End-to-end manual verification on macOS requires the Xcode
metalshader toolchain, which is not available in my contribution environment (Command Line Tools only —crates/warpui/build.rspanics on missingmetal). I verified the change via:cargo check/cargo teston the affected crate(s) — all green (see PR body for specific counts).Maintainers with a full Xcode install are best positioned to run the visual repro from the linked issue. Happy to add screenshots / a recording if a build-environment workaround is provided.