Fix macOS editing shortcuts dropped in non-KKP alt-screen TUIs

[warp-agent-staging]

Description

macOS line-editing shortcuts (cmd-left, cmd-right, cmd-backspace, cmd-delete, alt-left, alt-right, alt-backspace) were silently dropped inside non-KKP alt-screen TUIs (e.g. fzf, less) because seven bindings in app/src/terminal/view/init.rs were gated on id!("LongRunningCommand") alone. TerminalView::keymap_context explicitly suppresses LongRunningCommand while the alt screen is active, so the predicate could never match.

Other bindings in the same file (e.g. backward_tabulation for shift-tab) already use id!("LongRunningCommand") | id!("AltScreen"). This PR applies the same widening to the seven editing bindings (word left/right, home/end, delete word left, delete to line start/end).

The underlying action handler (control_sequence_on_terminal) already gates on the block's actual long-running state independent of the keymap context flag, so no other logic changes were required.

Kitty keyboard protocol carve-out

Widening on AltScreen alone would have regressed #14012. AltScreenElement::dispatch_event only falls back to the KKP-aware encoder (KeystrokeWithDetails::to_escape_sequence) when no keybinding claimed the keystroke, so an unconditional AltScreen match would hijack these chords inside KKP-negotiating TUIs (Claude Code, opencode, …) and replace CSI 1;3D / CSI 127;3u with legacy control characters.

The seven predicates are therefore consolidated into a single shared executing_command_editing_context() helper — so they cannot drift from one another — with the KKP exclusion applied to the alt-screen arm:

id!("Terminal")
    & !id!("IMEOpen")
    & (id!("LongRunningCommand") | (id!("AltScreen") & !id!(KEYBOARD_PROTOCOL_ENABLED_KEY)))

What this deliberately does not change: the LongRunningCommand arm is left untouched. The sibling shift-enter binding applies !id!(KEYBOARD_PROTOCOL_ENABLED_KEY) to its whole predicate; doing the same here would also change today's normal-screen behavior for KKP sessions with a running command, which nobody reported, is outside the scope of #14835, and is not covered by any test in this PR. Flagging it explicitly in case a reviewer would prefer that broader change as a follow-up.

Note the widening applies on Linux/Windows too (ctrl-left / ctrl-right / ctrl-backspace), since these are the same binding entries with per-platform keystrokes. That makes alt-screen behavior consistent with the normal-screen behavior those platforms already have.

Linked Issue

• The linked issue is factory-auto-implement.
• No UI changes; screenshots/video not applicable.

Fixes #14835

Testing

Two regression tests in app/src/terminal/view_tests.rs, both simulating a long-running block that swaps to the alt screen:

• alt_screen_editing_shortcuts_are_dispatched_without_kkp — no KKP negotiated: asserts the keymap context has AltScreen but not LongRunningCommand, the keystroke is handled, and ESC b reaches the PTY. Fails without the AltScreen widening.
• alt_screen_editing_shortcuts_are_not_dispatched_with_kkp — KKP negotiated after the screen swap: asserts the keymap context has both AltScreen and KeyboardProtocolEnabled, the keystroke is not claimed by a binding, and no legacy control sequence is written, i.e. it falls through to the Encode Cmd/Option modifiers for editing keys under the Kitty keyboard protocol #14012 encoder. Verified this test fails (handled == true) when the !id!(KEYBOARD_PROTOCOL_ENABLED_KEY) exclusion is removed, and passes with it.

Commands run in this sandbox (Linux), all against the current branch head:

• cargo test -p warp --lib alt_screen_editing_shortcuts — 2 passed, 0 failed.
• cargo test -p warp --lib terminal::view::tests — 149 passed, 5 failed. The 5 failures (test_insert, test_insert_into_input, test_reinput_blocks, test_viewport_iter_most_recent_at_bottom, test_scroll_position_doesnt_change_when_block_finished) are pre-existing parallel-execution flakes: each passes when run individually with --exact --test-threads=1, and the Review agent independently reproduced the identical failures on unmodified master.
• ./script/format --check — clean.
• ./script/check_no_inline_test_modules — clean.
• cargo clippy -p warp --lib --tests --no-deps -- -D warnings — exit 0, no warnings.

Not run in this sandbox, and therefore not verified here:

• The full workspace suite / ./script/presubmit (cargo nextest is not installed, and a full-workspace build exceeds this environment's memory limit — the warp lib target had to be built with -j 1 and debuginfo disabled to avoid an OOM kill). CI should cover it.

• Manual verification against a real fzf / less / Claude Code session on macOS: this is a headless Linux sandbox with no macOS host and no GUI, so the behavior is covered by the unit tests above rather than by hand.

• I have manually tested my changes locally with ./script/run (not available in this sandboxed environment; verified via unit tests instead)

Agent Mode

• Warp Agent Mode - This PR was created via Warp's AI Agent Mode

Conversation: https://staging.warp.dev/conversation/dbb3fb93-67af-46ca-b1a4-e430c26cf1b8
Run: https://oz.staging.warp.dev/runs/019fdeac-bfd5-7efc-93e1-17b1eeef7296

This PR was generated with Oz.

[warp-agent-staging]

Review: Fix macOS editing shortcuts dropped in non-KKP alt-screen TUIs (#14835)

Verdict: Approve

Scope

Diff against merge-base (720871f) touches exactly two files (+120/-7):

• app/src/terminal/view/init.rs
• app/src/terminal/view_tests.rs

No unrelated changes. (A raw diff against current master tip also shows churn in unrelated billing/workspace/graphql files, but that's just master having advanced past the PR's base commit — not part of this PR.)

Correctness

• The fix widens the context predicate on exactly the seven bindings named in the issue (word-left/right, home/end, delete-word-left, delete-line-start/end) from id!("LongRunningCommand") to id!("LongRunningCommand") | id!("AltScreen"), matching the existing pattern already used by terminal:backward_tabulation (shift-tab) in the same file.
• Verified in app/src/terminal/view.rs that the actual PTY write for these bindings is gated by control_sequence_on_terminal, which checks self.is_long_running() — a runtime check on the active block's actual long-running state (block_list().active_block().is_active_and_long_running() && !model.is_read_only()), fully independent of the keymap-context flags being loosened here. So broadening the dispatch predicate cannot cause the control sequence to reach the PTY when the block isn't genuinely long-running; it only unblocks dispatch while genuinely long-running and on the alt screen, which is exactly the bug scenario (non-KKP alt-screen TUIs like fzf/less where LongRunningCommand is suppressed).
• Confirmed other bindings still gated on LongRunningCommand alone (e.g. terminal:warpify_subshell, ctrl-i) are intentionally out of scope — they're tied to subshell-banner bootstrapping, not general alt-screen TUI editing, so leaving them untouched is correct restraint rather than an oversight.

Test coverage

alt_screen_editing_shortcuts_are_dispatched_without_kkp (added in view_tests.rs) is a solid regression test:

• Asserts the binding does not fire before a long-running block exists (baseline/no false positive).
• Simulates a long-running block that swaps to the alt screen without KKP negotiation, and asserts AltScreen is present and LongRunningCommand is absent from the keymap context (validates the actual bug precondition).
• Asserts the binding does fire afterward and that the exact WORD_LEFT escape sequence reaches the PTY.

This directly exercises the reported bug and would fail on master prior to this fix.

Verification notes

I was unable to complete an independent cargo test run for this PR in this sandbox — build was OOM-killed (SIGKILL) while linking the warp test binary due to the sandbox's memory limit, unrelated to this PR's contents. The Implement agent's reported results (test fails without the fix / passes with it; cargo fmt --check and cargo clippy clean) were verified indirectly via manual source review as described above, which is consistent with correctness.

Draft PR left open for human merge; not merging.

[warp-agent-staging]

Verdict: Request changes (blocking regression risk)

Correctness vs. #14835

The fix correctly identifies the root cause: LongRunningCommand is suppressed while AltScreen is active (app/src/terminal/view.rs:28380-28383), so the seven line-editing bindings gated on LongRunningCommand alone could never fire inside an alt-screen TUI. Widening the predicate to LongRunningCommand | AltScreen is the right general shape and matches the existing backward_tabulation (shift-tab) binding. I verified all seven bindings (word-left/right, home/end, delete-word-left, delete-line-start/end) were updated consistently — confirmed by reading the diff and grepping for AltScreen in init.rs.

Blocking issue: likely reintroduces the #14012 KKP regression for alt-screen apps

AltScreenElement::dispatch_event (app/src/terminal/alt_screen/alt_screen_element.rs:856-865) only falls back to KeystrokeWithDetails::to_escape_sequence (the Kitty-keyboard-protocol-aware encoder added in #14012) when no registered keybinding action already matched the keystroke. Since the widened predicate now matches purely on AltScreen regardless of whether the Kitty keyboard protocol has been negotiated, these seven bindings will now unconditionally intercept alt-left/right/backspace and cmd-left/right/backspace/delete inside any alt-screen app — including KKP-negotiating TUIs like Claude Code / opencode, which is exactly the set of apps and exact set of key combos #14012 fixed (see its table: Cmd+Backspace -> CSI 127;9u, Option+← -> CSI 1;3D, etc.). With this change, those chords will instead be hijacked to emit the legacy readline sequences (ESC b, ^A, ^U, ^W, ...) inside a KKP session, silently breaking word-navigation/deletion editing in Claude Code/opencode again, just via a different code path than #14012 fixed.

The codebase already has precedent for handling this correctly: the sibling shift-enter binding in the same file (app/src/terminal/view/init.rs:117-124) uses the identical (LongRunningCommand | AltScreen) predicate but additionally excludes !id!(KEYBOARD_PROTOCOL_ENABLED_KEY) so that KKP-negotiating sessions fall through to the raw encoder instead of being intercepted. The seven bindings touched by this PR should very likely get the same exclusion, e.g.:

.with_context_predicate(
    id!("Terminal") & !id!("IMEOpen") & (id!("LongRunningCommand") | id!("AltScreen")) & !id!(KEYBOARD_PROTOCOL_ENABLED_KEY),
),

(Note backward_tabulation has the same gap, but shift-tab isn't part of #14012's fix set, so it's lower risk there.)

Test gap

The new regression test (alt_screen_editing_shortcuts_are_dispatched_without_kkp) is well-targeted for the non-KKP case, but as its name signals, it does not cover the KKP case at all. A companion test that enables TermMode::KEYBOARD_PROTOCOL (+ the disambiguate/report-all flag) on the alt-screen model and asserts the CSI-u sequence still reaches the PTY (rather than the readline control sequence) would have caught the regression above and should be added before merge.

Independent verification performed

• Rebuilt the branch and ran cargo test -p warp --lib terminal::view::tests: the new test passes; 5 tests fail (test_insert, test_insert_into_input, test_reinput_blocks, test_scroll_position_doesnt_change_when_block_finished, test_viewport_iter_most_recent_at_bottom).
• Confirmed the flakiness claim independently: with the two changed files reverted to origin/master, running the same suite under --test-threads=4 reproduces the identical 5 failures with identical assertion messages, and each of the 5 tests passes when run individually in its own process. This confirms the failures are pre-existing test-isolation flakes under parallel execution, not caused by this PR.
• cargo fmt -p warp -- --check and cargo clippy -p warp --lib --no-deps -- -D warnings: both pass cleanly, no new warnings.
• CI (gh pr checks): only cheap checks ran (CodeQL, CLA, external-contributor label) and passed; the full Warp CI matrix (build/test/clippy) shows as skipped, which is expected while the PR remains draft.
• No references to any repository other than warpdotdev/warp, and no scope creep — the diff is limited to the two intended files.

Summary

Correct diagnosis and mostly correct fix for the reported non-KKP bug, but the predicate widening is very likely too broad and creates a credible regression for KKP-negotiating alt-screen apps (the exact scenario #14012 fixed). Please add the !id!(KEYBOARD_PROTOCOL_ENABLED_KEY) exclusion to the seven bindings and a regression test covering the KKP + alt-screen case before this merges. Leaving as draft for a human to review/merge.

[warp-agent-staging]

Blocking: this widened predicate (and the other 6 identical ones below) matches whenever AltScreen is active, regardless of whether the Kitty keyboard protocol is negotiated. AltScreenElement::dispatch_event only falls back to the KKP-aware KeystrokeWithDetails::to_escape_sequence encoder (from #14012) when no keybinding action already matched, so this binding will now hijack alt-left/cmd-left etc. inside KKP-negotiating alt-screen apps (Claude Code, opencode) and send the legacy ESC b-style sequence instead of the correct CSI-u sequence. The sibling shift-enter binding at lines 117-124 handles this exact situation by additionally excluding !id!(KEYBOARD_PROTOCOL_ENABLED_KEY) — these 7 bindings should likely do the same.

[warp-agent-staging]

This regression test only covers the non-KKP path (as its name states). Consider adding a companion test that negotiates the Kitty keyboard protocol (TermMode::KEYBOARD_PROTOCOL + disambiguate/report-all flag) on the alt-screen model and asserts the CSI-u sequence still reaches the PTY instead of the readline control sequence — this would catch the KKP regression described in the top-level review comment.

[warp-agent-staging]

Re-review of commit 031d35a: Verdict = Approve (advisory)

Verified the delta independently (rebuilt from scratch in a fresh sandbox, same protoc/CARGO_BUILD_JOBS=2 workaround as before).

Blocking finding resolved

The new executing_command_editing_context() helper adds & !id!(KEYBOARD_PROTOCOL_ENABLED_KEY) to the AltScreen arm, matching the mechanism (not literally the same predicate shape, see residual risk #1 below) used by shift-enter. I independently reproduced the fix's necessity: reverting the exclusion locally makes the new alt_screen_editing_shortcuts_are_not_dispatched_with_kkp test fail with ctrl-left must fall through to the Kitty keyboard protocol encoder, not be claimed by the legacy editing binding, and restoring it makes both tests pass again. This confirms the test is a real guard against the regression I flagged, not a tautology.

Test/CI verification performed

• cargo test -p warp --lib alt_screen_editing_shortcuts: 2 passed (both the non-KKP and new KKP-guard tests).
• cargo test -p warp --lib terminal::view::tests (--test-threads=4): 149 passed, same 5 pre-existing flakes (test_insert, test_insert_into_input, test_reinput_blocks, test_scroll_position_doesnt_change_when_block_finished, test_viewport_iter_most_recent_at_bottom) with identical assertion messages to what I reproduced on unmodified master in the prior review round — consistent with "pre-existing, unrelated to this change."
• cargo fmt -p warp -- --check: passes.
• cargo clippy -p warp --lib --tests --no-deps -- -D warnings: passes, no warnings.

Verdict on the three self-reported residual risks

1. KKP exclusion scoped to the alt-screen arm only (LongRunningCommand arm unchanged): Agree this is correct scope containment, not an oversight. Before this PR the predicate was id!("LongRunningCommand") alone, with no KKP awareness at all — so a long-running command negotiating KKP on the normal screen was already subject to being intercepted by these bindings prior to this change. This PR doesn't touch or worsen that pre-existing behavior; broadening the guard to the LongRunningCommand arm too would be a reasonable follow-up (for full consistency with shift-enter, which excludes KKP unconditionally across both arms) but is legitimately out of scope for #14835, which is specifically about the alt-screen suppression gap. Non-blocking.
2. Widening also reaches Linux/Windows ctrl-left/ctrl-right/ctrl-backspace: This is correct and desirable, not a risk to gate. The root cause (LongRunningCommand suppressed while AltScreen is active) is platform-agnostic — app/src/terminal/view.rs's context-building logic and AltScreenElement::dispatch_event's KKP fallback are shared across platforms, so the identical dropped-keystroke bug affects ctrl-left/etc. in alt-screen fzf/less on Linux/Windows for the same reason it affects alt-left/cmd-left on macOS. Gating the fix to macOS-only would leave a known-identical bug unfixed on other platforms for no clear benefit. No action needed.
3. Unit-test-only coverage, no real-TUI end-to-end run: Reasonable and consistent with how #14012 was primarily validated. Full end-to-end verification (real fzf/less, and a real KKP app such as Claude Code or opencode, on macOS and Linux) isn't practical in this sandboxed review pipeline, but would be valuable for a human to spot-check before or shortly after merge given the subtlety of the interaction being fixed. Flagging as a suggestion for the human merger, not a blocker.

Summary

The regression I flagged in the previous round is fixed and the fix is verified to actually guard the regression (not just cosmetically present). No new blocking issues found in the delta. Approve, pending the usual human review before merge — PR left as a draft, not merged, not marked ready, no auto-merge touched.