fix: Ctrl+C on selected block clears selection and returns focus to prompt - #14380
warp-agent-staging[bot] wants to merge 2 commits into
Conversation
…ocus to prompt On the idle path (no foreground process), pressing Ctrl+C on a whole-block selection left the block highlighted and focus remained on the terminal grid instead of the input box when FeatureFlag::AgentView was enabled. Root cause: skipped clearing when AgentView is enabled (blocks are preserved as attachable AI context). After returned without clearing, saw the selection still present and focused the terminal grid instead of the input box. Fix: In the non-Windows , call and directly, bypassing the AgentView guard. This overrides the block-preservation behaviour for the Ctrl+C gesture specifically, matching the product decision that Ctrl+C acts as a consistent 'dismiss and return to prompt' gesture (equivalent to Escape / FocusInputAndClearSelection). The SIGINT path (foreground process running) is unchanged: SIGINT is still sent to the PTY after the selection is cleared. Fixes: #13480 Linear: APP-5007 Co-Authored-By: Warp <agent@warp.dev>
|
@warp-dev-github-integration[bot] 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 |
There was a problem hiding this comment.
Overview
This PR updates the non-Windows Ctrl+C path so block selections are cleared directly before focus is redetermined, and adds regression coverage for idle and running-process paths.
Concerns
- For this user-facing Ctrl+C/focus behavior change, please include screenshots or a short screen recording demonstrating it working end to end. The PR currently says visual proof will be attached later, so the required evidence is still missing.
- No approved spec context was provided, so there was no implementation/spec drift to flag.
- No security issues were found in the changed code.
Verdict
Found: 0 critical, 1 important, 1 suggestion
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
There was a problem hiding this comment.
Overview
Narrow, well-targeted fix: the non-Windows ctrl_c_internal now clears block/text selection directly so redetermine_global_focus falls through to focus_input_box. I verified this locally — ctrl_c_with_selected_block_clears_selection_on_idle_path fails on the pre-fix code and passes with the fix, ctrl_c_with_selected_block_and_running_process_clears_selection_and_sends_sigint passes both before and after (confirming the SIGINT path is genuinely unchanged), and all 7 terminal::view::tests::ctrl_c* tests pass on the PR head. CI is green so far (0 failing; the test jobs were still running). The attached video proof does demonstrate both acceptance criteria on the idle path — the block loses its teal highlight and a blinking cursor appears in the input. One scoping issue and one carried-forward test gap keep me from approving.
Concerns
The fix widens the bypass further than the PR describes. clear_selections_when_shell_mode_without_focusing_input was guarded by two conditions — !is_ai_input_enabled() and !FeatureFlag::AgentView.is_enabled() — and the new code bypasses both, not just the AgentView one. has_copiable_block_selection is deliberately false in AI-input mode ("we don't want to copy blocks in AI input mode because those are context blocks"), but has_block_list_selection is not, so a plain text selection in AI-input mode now enters this branch and drops the user's selected context blocks. That directly contradicts the PR body's "No change to cmd-c / copy() behavior or text-selection semantics", and no test covers it. Either restore the is_ai_input_enabled() guard or make the widening explicit in the comment/PR body and cover it with a test. See the inline comment.
Neither regression test asserts the symptom that was actually reported. Both assert that selected_blocks is empty, which is the precondition for the fix, not its outcome; the user-visible bug is that focus does not return to the prompt. redetermine_global_focus runs inside handle_action, so a focus assertion is reachable from these tests and would protect the fix against a future focus-policy change that leaves selection-clearing intact. This repeats the still-open suggestion on app/src/terminal/view_tests.rs:8358 from the earlier review — I have not filed a duplicate inline comment.
Minor evidence note: the "after Ctrl+C" still is captioned as showing the blinking cursor, but no cursor is visible in that frame (it appears to have been captured on the blink-off phase) and the prompt row is visually identical to the "before" still. The video artifact does show the cursor, so the proof stands, but the screenshot caption overstates what that image demonstrates.
Verdict
Found: 0 critical, 2 important, 1 suggestion
Prior concerns still outstanding: the missing-visual-proof concern from the earlier review is resolved (proof is now attached and validated against the acceptance criteria); the focus-assertion suggestion remains open.
Request changes (posted as a comment review: GitHub blocks a formal "request changes" from the PR author's identity, which the factory bot shares)
Review run
https://oz.staging.warp.dev/runs/019fa60a-7f40-7ad9-b8c7-91f4ecda79d0
…roper selection API Address review feedback on PR #14380: 1. Scope leak: restore the is_ai_input_enabled() guard in ctrl_c_internal (non-Windows). The original change removed both the AgentView AND the AI-input-mode guards. The AI-input guard must stay: when AI input is enabled, has_block_list_selection (unlike has_copiable_block_selection) can be true, and clearing selected_blocks in that state would silently drop blocks the user staged as AI context. The fix now clears only when idle-path bug (GH-13480) is fixed in shell mode. 2. Focus assertion: add a post-Ctrl+C assertion that view.input.as_ref(ctx).editor().is_focused(ctx) is true. This directly tests the reported symptom ('ctrl+c does not set focus on prompt') and guards against a future redetermine_global_focus regression. 3. Real selection entry point: use view.reset_selection_to_single_block() in both tests instead of mutating view.selected_blocks directly. The real path calls change_block_selections(), which fires AI-context side effects that mirror what a click-select in production produces. Co-Authored-By: Warp <agent@warp.dev>
There was a problem hiding this comment.
Overview
Rework cycle 1 addresses all three findings, and I verified each against the new head 028ee82 rather than taking the summary on faith. The is_ai_input_enabled() guard is restored so only the AgentView preservation is overridden, both tests now go through reset_selection_to_single_block(), and the idle-path test asserts the reported symptom directly. CI is now fully green on this head — 21 passing, 6 skipped, nothing failing or pending, including the Linux, macOS and Windows test jobs. The PR description was rewritten into an accurate current-state summary of the guard. Approving.
The strongest evidence: I reverted the fix, deleted the selection assertion, and re-ran the idle-path test in isolation. It failed precisely on input editor must be focused after Ctrl+C on idle path, so the new focus assertion genuinely pins the reported bug rather than passing trivially. With the fix in place all 7 terminal::view::tests::ctrl_c* tests pass, and ctrl_c_with_selected_block_and_running_process_clears_selection_and_sends_sigint continues to pass both with and without the fix, confirming the SIGINT path is still untouched.
Concerns
One non-blocking gap: nothing pins the AI-input-mode branch in a test. That is acceptable here because restoring the guard returns that path to its pre-PR behavior, so there is no new delta to protect — but a small test that enables AI input, selects a block, and asserts the selection survives Ctrl+C would make the guard's intent explicit and stop a future refactor from quietly deleting it. Worth doing opportunistically, not a reason to hold this PR.
Note on evidence: the attached screenshots and video were captured on the previous head. The idle-path behavior they demonstrate is unchanged by this rework (that session had AI input disabled, so the restored guard does not apply), so the proof remains valid for the current head and I did not ask for a recapture.
Verdict
Found: 0 critical, 0 important, 1 suggestion
Prior concerns: none remain. All three findings from the previous pass are fixed and their threads are resolved.
Approve (posted as a comment review: GitHub blocks a formal approval from the PR author's identity, which the factory bot shares — this is an accepted verdict, but a human approval is still required to merge)
Review run
https://oz.staging.warp.dev/runs/019fa60a-7f40-7ad9-b8c7-91f4ecda79d0
Summary
Fixes a bug where pressing Ctrl+C on a whole-block selection (completed block highlighted) with no foreground process running left the block selected and focus on the terminal grid instead of returning to the prompt. This affects all users on macOS/Linux when
FeatureFlag::AgentViewis enabled.Root cause:
ctrl_c_internal(non-Windows) delegated toclear_selections_when_shell_mode_without_focusing_input, which skipped clearing when eitheris_ai_input_enabled()ORFeatureFlag::AgentViewwas enabled. In practice,AgentViewis broadly enabled, so the guard always triggered and blocks were never cleared. After returning without clearing,redetermine_global_focussaw the block still selected and focused the terminal grid instead of the input box.Fix: In the non-Windows
ctrl_c_internal, callclear_selected_blocks+clear_selected_textinside an!is_ai_input_enabled()guard — keeping the AI-input-mode protection (blocks staged as AI context are preserved when AI input is active) while removing the AgentView guard. This makes Ctrl+C act as a consistent "dismiss and return to prompt" gesture in shell mode, regardless of whether AgentView is enabled.What is unchanged:
has_block_list_selectionselections (in-block text selections staged as AI context) are not cleared — the AI-input guard is keptcopy()behaviorTrade-off: In shell mode, Ctrl+C on a selected block dismisses the selection even when
AgentViewis enabled. This is intentional: Ctrl+C is a "dismiss" gesture, and the Escape key already provides the same "clear selection + return to prompt" behavior. Reviewers can push back if this trade-off is unwanted.Linked Issue
Closes #13480
Linear: https://linear.app/warpdotdev/issue/APP-5007/verify-gh-13480-ctrlc-does-not-return-focus-to-the-prompt-when-a-block
Testing
Added two regression tests in
app/src/terminal/view_tests.rs:ctrl_c_with_selected_block_clears_selection_on_idle_path— asserts selection is cleared, no ETX written to PTY, AND the input editor is focused (the literal reported symptom)ctrl_c_with_selected_block_and_running_process_clears_selection_and_sends_sigint— asserts selection is cleared AND ETX sent to PTY (SIGINT path unchanged)Both tests use
reset_selection_to_single_block()(the real selection entry point) rather than direct field mutation, so AI-context side effects run and the precondition mirrors production.All 142
terminal::view::testspass../script/runScreenshots / Videos
Computer-use screenshots
View screenshot: Before Ctrl+C: the completed "ls" block is selected (teal whole-block highlight with action icons), input prompt has no cursor.
View screenshot: After Ctrl+C: the "ls" block is no longer highlighted (deselected) and the input prompt shows the blinking cursor, indicating focus returned to the input.
Computer-use video recordings
View video recording: Running a command in Warp to create a completed block, selecting the block, then pressing Ctrl+C to confirm the selection clears and focus returns to the input prompt.
Agent Mode
CHANGELOG-BUG-FIX: Ctrl+C on a selected block now correctly clears the selection and returns focus to the input prompt
Originating thread: https://warpdev.slack.com/archives/C0BDQDW8V5E/p1785193867728139
Conversation: https://staging.warp.dev/conversation/d41fa921-3900-4ec6-83b6-23ee17f73464
Run: https://oz.staging.warp.dev/runs/019fa5eb-c46e-7a5f-ae26-9a46d8e83f5d
This PR was generated with Oz.