Skip to content

Commit 0613cd2

Browse files
fix(dash): only act on key Press events in the overlay dispatch (#89)
The dash event loop dispatched each operational overlay (serve wizard, engine manager, onboarding, …) straight to its `on_key` via `if state.<overlay>.is_some()` arms that ran BEFORE the general `handle_key` — the only place filtering non-Press key events. Terminals that also emit Release/Repeat events (Windows Terminal / ConPTY under WSL, kitty keyboard protocol) thus delivered each keystroke to overlays twice. For the serve wizard's model picker this meant Enter chose+closed the picker on Press, then the Release/Repeat echo re-opened it seeded with the just-chosen model as a filter — so the model could never be selected. This hit both the bare-`rocm` launcher (focused Serve) and the full dash, since both share this loop. Extract `is_actionable_key(KeyEventKind)` (Press only), add a top-priority arm that swallows non-Press key events above every key arm so the Press-only invariant covers overlays too, and route `handle_key` through the same predicate. Regression test: `only_press_key_events_are_actionable`. Signed-off-by: Michael Roy <michael.roy@amd.com>
1 parent 3e9ec2b commit 0613cd2

1 file changed

Lines changed: 40 additions & 1 deletion

File tree

  • crates/rocm-dash-tui/src/app

crates/rocm-dash-tui/src/app/mod.rs

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1694,6 +1694,20 @@ async fn event_loop(terminal: &mut Tui, args: &ResolvedArgs) -> color_eyre::Resu
16941694
}
16951695
maybe_ev = events.next() => {
16961696
match maybe_ev {
1697+
// Only ACT on key presses. Terminals (notably Windows
1698+
// Terminal / ConPTY under WSL, and any with the kitty
1699+
// keyboard protocol) also emit Release/Repeat events; the
1700+
// general `handle_key` already drops non-Press, but the
1701+
// operational-overlay arms below dispatch straight to their
1702+
// managers and would otherwise process the SAME keystroke
1703+
// twice. That double-fire is what made Enter in the serve
1704+
// wizard's model picker re-open the picker (seeding it with
1705+
// the just-chosen model as a filter) instead of choosing.
1706+
// Swallow non-Press key events here, above every key arm, so
1707+
// the Press-only invariant holds for overlays too.
1708+
Some(Ok(CtEvent::Key(k))) if !is_actionable_key(k.kind) => {
1709+
let _ = k;
1710+
}
16971711
// The approval modal, when open, owns ALL keys with the
16981712
// highest priority (above every operational overlay and the
16991713
// general handler) so the operator's decision can't be
@@ -2679,8 +2693,21 @@ pub enum KeyAction {
26792693
OpenLogs,
26802694
}
26812695

2696+
/// Whether a crossterm key event should be acted on. Terminals emit
2697+
/// Release/Repeat events in addition to Press (notably Windows Terminal /
2698+
/// ConPTY under WSL, and any terminal advertising the kitty keyboard protocol).
2699+
///
2700+
/// The whole TUI acts on Press only. Both the general [`handle_key`] and the
2701+
/// event loop's operational-overlay dispatch share this gate — without it, a
2702+
/// single keystroke reaches an overlay's `on_key` more than once, which made
2703+
/// Enter in the serve wizard's model picker re-open the picker (seeded with the
2704+
/// just-chosen model as a filter) instead of choosing it.
2705+
const fn is_actionable_key(kind: KeyEventKind) -> bool {
2706+
matches!(kind, KeyEventKind::Press)
2707+
}
2708+
26822709
fn handle_key(k: KeyEvent, current: ActiveTab, modal: &Modal, chat: ChatKeyCtx) -> KeyAction {
2683-
if k.kind != KeyEventKind::Press {
2710+
if !is_actionable_key(k.kind) {
26842711
return KeyAction::Nothing;
26852712
}
26862713
// Chat tab key handling, placed BEFORE the global hotkey match so focused
@@ -3442,6 +3469,18 @@ mod tests {
34423469
);
34433470
}
34443471

3472+
#[test]
3473+
fn only_press_key_events_are_actionable() {
3474+
// The event loop gates overlay dispatch on this predicate so a single
3475+
// keystroke isn't processed twice by an overlay's `on_key` (Release /
3476+
// Repeat echoes on Windows Terminal / ConPTY / kitty keyboard). The
3477+
// double-fire re-opened the serve wizard's model picker on Enter instead
3478+
// of choosing — this pins Press-only routing.
3479+
assert!(is_actionable_key(KeyEventKind::Press));
3480+
assert!(!is_actionable_key(KeyEventKind::Release));
3481+
assert!(!is_actionable_key(KeyEventKind::Repeat));
3482+
}
3483+
34453484
#[test]
34463485
fn jk_arrows_and_g_drive_selection() {
34473486
assert_eq!(

0 commit comments

Comments
 (0)