tui: the fast-forward spawn key is dead ("S" never matches "shift-S") #70

Closed
opened 2026-08-24 00:41:38 +01:00 by naps62-yolo · 1 comment
Collaborator

crates/maestro-tui/src/config.rs — the TUI fast-forward key does nothing.

The default binding is ("S", "FastForwardSpawn"), stored verbatim as "S". At runtime crossterm 0.28 attaches KeyModifiers::SHIFT to any uppercase character, so pressing Shift+S yields InputKey::Char { c: 'S', shift: true }, whose canonical() is "shift-S". Keymap::resolve is a plain map lookup, so it misses and nothing happens.

The TUI half of #67 is unreachable from the keyboard. The web binding is fine — KeyboardEvent.key is "N" with no separate modifier in the lookup.

Fix by binding "shift-S", or by normalising uppercase characters in canonical()/parse so the two forms agree. Whichever is chosen, a test should press the actual key rather than assert on the binding string — the existing tests passed while the feature was dead.

`crates/maestro-tui/src/config.rs` — **the TUI fast-forward key does nothing.** The default binding is `("S", "FastForwardSpawn")`, stored verbatim as `"S"`. At runtime crossterm 0.28 attaches `KeyModifiers::SHIFT` to any uppercase character, so pressing Shift+S yields `InputKey::Char { c: 'S', shift: true }`, whose `canonical()` is `"shift-S"`. `Keymap::resolve` is a plain map lookup, so it misses and nothing happens. The TUI half of #67 is unreachable from the keyboard. The web binding is fine — `KeyboardEvent.key` is `"N"` with no separate modifier in the lookup. Fix by binding `"shift-S"`, or by normalising uppercase characters in `canonical()`/`parse` so the two forms agree. Whichever is chosen, a test should press the actual key rather than assert on the binding string — the existing tests passed while the feature was dead.
naps62-yolo added this to the Spawn v2 milestone 2026-08-24 00:41:38 +01:00
naps62-yolo added the area/tuidifficulty/easytype/fix labels 2026-08-24 00:41:38 +01:00
Author
Collaborator

Out-of-scope failure found while gating this fix (unrelated to #70, not touched by this branch's diff — only crates/maestro-tui/src/{input,config}.rs changed here):

test natural_exit_of_a_pristine_worktree_removes_it ... FAILED
thread 'natural_exit_of_a_pristine_worktree_removes_it' panicked at crates/maestro-daemon/tests/session_manager.rs:843:5:
a pristine worktree leaves nothing behind

Reproduces in isolation (cargo test -p maestro-daemon --test session_manager natural_exit_of_a_pristine_worktree_removes_it -- --test-threads=1), not a concurrency flake. Leaving it for whoever owns maestro-daemon's worktree-lifecycle code — not fixing it here.

Out-of-scope failure found while gating this fix (unrelated to #70, not touched by this branch's diff — only `crates/maestro-tui/src/{input,config}.rs` changed here): ``` test natural_exit_of_a_pristine_worktree_removes_it ... FAILED thread 'natural_exit_of_a_pristine_worktree_removes_it' panicked at crates/maestro-daemon/tests/session_manager.rs:843:5: a pristine worktree leaves nothing behind ``` Reproduces in isolation (`cargo test -p maestro-daemon --test session_manager natural_exit_of_a_pristine_worktree_removes_it -- --test-threads=1`), not a concurrency flake. Leaving it for whoever owns `maestro-daemon`'s worktree-lifecycle code — not fixing it here.
This repo is archived. You cannot comment on issues.
1 Participants
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: naps62/maestro#70