Skip to content

fix(keymap): unblock ~46 integration tests by retiring ctrl-b binding on linux/windows#165

Merged
BunsDev merged 1 commit into
mainfrom
fix/keymap-binding-validation
May 31, 2026
Merged

fix(keymap): unblock ~46 integration tests by retiring ctrl-b binding on linux/windows#165
BunsDev merged 1 commit into
mainfrom
fix/keymap-binding-validation

Conversation

@BunsDev
Copy link
Copy Markdown
Member

@BunsDev BunsDev commented May 31, 2026

Summary

ctrl-b is the ASCII STX control character. The Workspace ToggleWarpDrive action was registering ctrl-b as its Linux/Windows binding, which failed is_binding_pty_compliant validation at boot. KeymapMatcher::validate_all_bindings then panicked at crates/warpui_core/src/keymap/matcher.rs:197 (Bindings failed validation), taking out ~50+ integration tests that boot the harness — every ui_tests::* test and most shell-integration tests.

Fix

One-line change in app/src/workspace/mod.rs:707: switch ctrl-bctrl-shift-B, matching the cmd_or_ctrl_shift convention for Mac-only single-letter shortcuts that collide with PTY control characters elsewhere.

Verification

Local run (macOS, same panic surface as Linux/Windows since the binding registers on both):

Step Result
cargo check -p warp-app ✅ passes
cargo test -p integration --test integration 211 passed, 8 failed, 63 ignored (was ~50+ tests panicking at the keymap validator before this fix)

The 8 remaining failures are unrelated:

  • 5 SSH harness tests (test_ctrl_c, test_legacy_ssh_into_{bash,zsh}, test_ssh_into_{sh,ash})
  • 3 vertical context-menu / SSH override tests (test_vertical_{pane,tab}_context_menu_copies_metadata, test_ssh_with_shell_override)

Those are separate root causes — should be cut as follow-up PRs.

Note

This PR is independent of #164. #164 fixes 8 unrelated unit-test failures (auth_manager, blocklist, etc.). Both can land in either order against main.

Test plan

  • CI Warp CI on Linux/Windows/macOS — Run X tests jobs should drop from ~50+ failing to ≤8 failing.

`ctrl-b` is the ASCII STX control character. Registering it on
Linux/Windows for the Workspace `ToggleWarpDrive` action failed
`is_binding_pty_compliant` validation at boot — `KeymapMatcher` then
panicked at `crates/warpui_core/src/keymap/matcher.rs:197` with
`Bindings failed validation`, taking out ~50+ integration tests
(`ui_tests::*`, shell-integration suites) that boot the test harness.

Switch to `ctrl-shift-B`, matching the `cmd_or_ctrl_shift` convention
for single-letter shortcuts that would otherwise collide with PTY
control characters on non-Mac platforms.

Verification:
- `cargo check -p warp-app` — passes
- `cargo test -p integration --test integration` — 211 passed, 8
  failed (all SSH / ctrl-c / vertical-context-menu — separate root
  causes, not keymap-related), 63 ignored. Was ~50+ tests panicking
  at the keymap validator before the fix.
@BunsDev BunsDev marked this pull request as ready for review May 31, 2026 02:17
Copilot AI review requested due to automatic review settings May 31, 2026 02:17
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@BunsDev
Copy link
Copy Markdown
Member Author

BunsDev commented May 31, 2026

Self-review notes (can't formally approve own PR):

Verified locally:

  • Grepped the tree for ctrl-shift-B / ctrl-shift-b — no other literal binding uses it, so no static conflict.
  • cmd_or_ctrl_shift is the established convention (FocusInput, CommandPalette, NewTab, Find, etc. in app/src/util/bindings.rs).
  • The fix is the right Linux/Windows-only escape from the STX PTY-compliance trap; Mac stays on cmd-b correctly.

Nit / heads-up (non-blocking):
CustomAction::ToggleBookmarkBlock is bound to cmd_or_ctrl_shift("b") at app/src/util/bindings.rs:361, which also resolves to ctrl-shift-B on Linux/Windows. It lives in the Terminal context with BlockSelectionCardinality != None, while ToggleWarpDrive is in Workspace — so the matcher should disambiguate via context specificity, but worth eyeballing the in-progress integration tests once they finish to confirm neither shadows the other when a terminal block is selected.

The load-bearing concern is the boot-time panic taking out ~46 tests; this fix resolves it cleanly. The 8 unrelated SSH/context-menu failures should get a tracking issue if they don't already have one.

@BunsDev BunsDev merged commit def938b into main May 31, 2026
31 of 35 checks passed
@BunsDev BunsDev deleted the fix/keymap-binding-validation branch May 31, 2026 03:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants