fix(keyboard): shortcuts, run focus-changing actions on key release

Close tab and tab switching move keyboard focus to another session before
the key that fired them is released. On Linux the Flutter input source
routes map-mode sessions through the Rust matcher and legacy-mode sessions
through the Dart matcher, and neither knows the other's fired keys, so
after such an action the repeats and the release of the held key reached
the next session's remote, and the key stayed recorded as fired in the
matcher that saw the press.

Routing Linux legacy mode through the Rust FFI path instead is not a drop-in
change: the raw key handler builds its event with no unicode, so Rust's
legacy mode would fall back to US key names and lose the layout-aware
characters the Dart path sends today, and it would take modifiers from the
X11 key state instead of Flutter's.

Run these three actions when the fired key is released. The matcher that
consumed the press then also sees the repeats and the release, the remote
modifiers are still released on the press, and a session that closes drops
the action it owed. Both matchers keep the same list of actions.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ra4my61t8q1FN5n59wB16D

flutter/lib/common/widgets/keyboard_shortcuts/shortcut_constants.dart

@@ -17,6 +17,18 @@ const kShortcutActionRestartRemote = 'restart_remote';
 const kShortcutActionResetCanvas = 'reset_canvas';
 const kShortcutActionSwitchTabNext = 'switch_tab_next';
 const kShortcutActionSwitchTabPrev = 'switch_tab_prev';
+
+/// Actions that move keyboard focus to another session. They run when the
+/// key that fired them is released, not when it is pressed, so the matcher
+/// that consumed the press also sees its repeats and its release. On Linux a
+/// legacy-mode session matches in Dart while a map-mode session matches in
+/// Rust, and neither knows the other's fired keys. Mirrors `runs_on_release`
+/// in `src/keyboard/shortcuts.rs`.
+const kShortcutActionsRunOnKeyUp = {
+  kShortcutActionCloseTab,
+  kShortcutActionSwitchTabNext,
+  kShortcutActionSwitchTabPrev,
+};
 const kShortcutActionToggleMute = 'toggle_mute';
 const kShortcutActionPinToolbar = 'pin_toolbar';
 const kShortcutActionViewModeOriginal = 'view_mode_original';

flutter/lib/models/local_flutter_shortcuts.dart

@@ -3,6 +3,8 @@ import 'dart:async';
 import 'package:flutter/foundation.dart';
 import 'package:flutter/services.dart';
 
+import '../common/widgets/keyboard_shortcuts/shortcut_constants.dart';
+
 class LocalFlutterShortcutDispatcher {
   final void Function(String) onTriggered;
   // Keys whose press fired a shortcut and whose release has not arrived yet.
@@ -11,22 +13,30 @@ class LocalFlutterShortcutDispatcher {
   // it is never dropped with a session. A key whose release is missed heals
   // itself: its next press is consumed as a repeat and that release removes it.
   static final _firedKeys = <PhysicalKeyboardKey>{};
+  // Actions waiting for the release of the key that fired them, with the
+  // session that fired them. See [kShortcutActionsRunOnKeyUp].
+  static final _keyUpActions =
+      <PhysicalKeyboardKey, (LocalFlutterShortcutDispatcher, String)>{};
   final _releasedModifiers = <PhysicalKeyboardKey>{};
   bool _viewOnlyShortcutPending = false;
   int _generation = 0;
 
   LocalFlutterShortcutDispatcher({required this.onTriggered});
 
-  /// Drops this session's pending action and modifier replay. Fired keys
+  /// Drops this session's pending actions and modifier replay. Fired keys
   /// stay owned until their release, whichever session receives it.
   void clear() {
+    _keyUpActions.removeWhere((_, pending) => identical(pending.$1, this));
     _releasedModifiers.clear();
     _viewOnlyShortcutPending = false;
     _generation++;
   }
 
   @visibleForTesting
-  static void resetFiredKeys() => _firedKeys.clear();
+  static void resetFiredKeys() {
+    _firedKeys.clear();
+    _keyUpActions.clear();
+  }
 
   void recordReleasedModifiers(Iterable<PhysicalKeyboardKey> keys) {
     _releasedModifiers.addAll(keys);
@@ -106,23 +116,39 @@ class LocalFlutterShortcutDispatcher {
     String? Function()? match,
     Future<void> Function()? releaseModifiers,
   }) {
-    if (up) return _firedKeys.remove(key);
+    if (up) {
+      if (!_firedKeys.remove(key)) return false;
+      final pending = _keyUpActions.remove(key);
+      if (pending != null) {
+        final (owner, action) = pending;
+        unawaited(owner._trigger(action, null));
+      }
+      return true;
+    }
     if (_firedKeys.contains(key)) return true;
     if (!down) return false;
     final action = match?.call();
     if (action == null) return false;
     _firedKeys.add(key);
     if (viewOnly) _viewOnlyShortcutPending = true;
-    unawaited(_trigger(action, releaseModifiers));
+    final runOnKeyUp = kShortcutActionsRunOnKeyUp.contains(action);
+    if (runOnKeyUp) {
+      _keyUpActions[key] = (this, action);
+    } else {
+      _keyUpActions.remove(key);
+    }
+    unawaited(_trigger(runOnKeyUp ? null : action, releaseModifiers));
     return true;
   }
 
+  /// Releases the remote modifiers, then runs [action] unless it is null or
+  /// this session was cleared meanwhile.
   Future<void> _trigger(
-      String action, Future<void> Function()? releaseModifiers) async {
+      String? action, Future<void> Function()? releaseModifiers) async {
     final generation = _generation;
     try {
       if (releaseModifiers != null) await releaseModifiers();
-      if (generation == _generation) onTriggered(action);
+      if (action != null && generation == _generation) onTriggered(action);
     } catch (e, st) {
       debugPrint('Local shortcut failed for $action: $e\n$st');
     }

flutter/test/local_flutter_shortcuts_test.dart

@@ -2,6 +2,7 @@ import 'dart:async';
 
 import 'package:flutter/services.dart';
 import 'package:flutter_test/flutter_test.dart';
+import 'package:flutter_hbb/common/widgets/keyboard_shortcuts/shortcut_constants.dart';
 import 'package:flutter_hbb/models/local_flutter_shortcuts.dart';
 
 LocalFlutterShortcutDispatcher dispatcherFor(
@@ -224,6 +225,47 @@ void main() {
     expect(actions, isEmpty);
   });
 
+  test('focus-changing actions run when the key is released', () async {
+    // Close tab moves focus before the key is released; the release may then
+    // land in a session whose keys go through the other matcher.
+    var releases = 0;
+    final actions = <String>[];
+    final dispatcher = _ShortcutHarness(
+      match: (_) => kShortcutActionCloseTab,
+      releaseModifiers: () async => releases++,
+      onTriggered: actions.add,
+    );
+
+    expect(dispatcher.tryDispatch(down(PhysicalKeyboardKey.keyW)), isTrue);
+    expect(dispatcher.tryDispatch(repeat(PhysicalKeyboardKey.keyW)), isTrue);
+    await Future<void>.value();
+    expect(releases, 1, reason: 'remote modifiers are released on the press');
+    expect(actions, isEmpty, reason: 'the action waits for the release');
+
+    final other = dispatcherFor(actions.add);
+    expect(other.tryDispatch(up(PhysicalKeyboardKey.keyW)), isTrue);
+    await Future<void>.value();
+    expect(actions, [kShortcutActionCloseTab]);
+    expect(other.tryDispatch(up(PhysicalKeyboardKey.keyW)), isFalse);
+  });
+
+  test('closing the session drops its action pending on key release',
+      () async {
+    final actions = <String>[];
+    final dispatcher = _ShortcutHarness(
+      match: (_) => kShortcutActionSwitchTabNext,
+      releaseModifiers: () async {},
+      onTriggered: actions.add,
+    );
+
+    expect(dispatcher.tryDispatch(down(PhysicalKeyboardKey.keyW)), isTrue);
+    dispatcher.clear();
+    expect(dispatcher.tryDispatch(up(PhysicalKeyboardKey.keyW)), isTrue,
+        reason: 'the key stays owned');
+    await Future<void>.value();
+    expect(actions, isEmpty);
+  });
+
   test('raw legacy shortcuts own repeats and release after modifiers change',
       () async {
     addTearDown(RawKeyboard.instance.clearKeysPressed);

src/keyboard/shortcuts.rs

@@ -328,15 +328,35 @@ lazy_static::lazy_static! {
     /// removes it.
     static ref FIRED_KEYS: std::sync::Mutex<std::collections::HashSet<rdev::Key>> =
         Default::default();
+    /// Actions waiting for the release of the key that fired them, with the
+    /// session that fired them. See `runs_on_release`.
+    static ref RELEASE_ACTIONS: std::sync::Mutex<
+        std::collections::HashMap<rdev::Key, (hbb_common::SessionID, String)>,
+    > = Default::default();
     static ref RELEASED_MODIFIERS: std::sync::Mutex<
         std::collections::HashMap<hbb_common::SessionID, std::collections::HashMap<rdev::Key, rdev::Event>>,
     > = Default::default();
 }
 
-/// Forget the modifiers a session released on its remote. Fired keys are
-/// physical and stay owned until their release, whichever session gets it.
+/// Actions that move keyboard focus to another session. They run when the
+/// key that fired them is released, not when it is pressed, so the matcher
+/// that consumed the press also sees its repeats and its release. The next
+/// session may route its keys through the other matcher (Flutter's legacy
+/// path on Linux), which never saw the press. Mirrors
+/// `kShortcutActionsRunOnKeyUp` in `shortcut_constants.dart`.
+pub fn runs_on_release(action_id: &str) -> bool {
+    matches!(
+        action_id,
+        action_id::CLOSE_TAB | action_id::SWITCH_TAB_NEXT | action_id::SWITCH_TAB_PREV
+    )
+}
+
+/// Forget the modifiers a session released on its remote and the actions it
+/// still owes on key release. Fired keys are physical and stay owned until
+/// their release, whichever session gets it.
 #[cfg(feature = "flutter")]
 pub fn clear_session_state(session_id: &hbb_common::SessionID) {
+    RELEASE_ACTIONS.lock().unwrap().retain(|_, (sid, _)| sid != session_id);
     let held = RELEASED_MODIFIERS.lock().unwrap().remove(session_id);
     if let Some(held) = held {
         {
@@ -361,6 +381,7 @@ pub fn enter_view_only(session_id: &hbb_common::SessionID) {
     // The Flutter input source stops routing key events here, so the release
     // of a fired key can no longer reach this matcher.
     FIRED_KEYS.lock().unwrap().clear();
+    RELEASE_ACTIONS.lock().unwrap().clear();
     clear_session_state(session_id);
 }
 
@@ -419,7 +440,18 @@ pub fn try_dispatch(
         let mut fired = FIRED_KEYS.lock().unwrap();
         match event.event_type {
             EventType::KeyPress(k) if fired.contains(&k) => return true,
-            EventType::KeyRelease(k) if fired.remove(&k) => return true,
+            EventType::KeyRelease(k) if fired.remove(&k) => {
+                drop(fired);
+                let pending = RELEASE_ACTIONS.lock().unwrap().remove(&k);
+                if let Some((sid, action_id)) = pending {
+                    crate::flutter::push_session_event(
+                        &sid,
+                        "shortcut_triggered",
+                        vec![("action", &action_id)],
+                    );
+                }
+                return true;
+            }
             _ => {}
         }
     }
@@ -430,6 +462,12 @@ pub fn try_dispatch(
     release_remote_keys(sid, keyboard_mode, &peer(), &send);
     if let EventType::KeyPress(k) = event.event_type {
         FIRED_KEYS.lock().unwrap().insert(k);
+        let mut pending = RELEASE_ACTIONS.lock().unwrap();
+        if runs_on_release(&action_id) {
+            pending.insert(k, (*sid, action_id));
+            return true;
+        }
+        pending.remove(&k);
     }
     crate::flutter::push_session_event(sid, "shortcut_triggered", vec![("action", &action_id)]);
     true
@@ -1199,6 +1237,48 @@ mod tests {
         release_chord(chord);
     }
 
+    /// Close tab and tab switching move focus away before the key is
+    /// released; they run on the release so the same matcher sees the whole
+    /// press.
+    #[cfg(feature = "flutter")]
+    #[test]
+    fn focus_changing_actions_run_on_release() {
+        use rdev::Key;
+
+        let _guard = CACHE_TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner());
+        let chord = enable_defaults_and_hold_chord();
+        let prefix = || vec![Modifier::Primary, Modifier::Alt, Modifier::Shift];
+        *CACHE.write().unwrap() = Arc::new(Bindings {
+            enabled: true,
+            pass_through: false,
+            bindings: vec![
+                Binding { action: action_id::CLOSE_TAB.into(), mods: prefix(), key: "w".into() },
+                Binding { action: action_id::SCREENSHOT.into(), mods: prefix(), key: "p".into() },
+            ],
+        });
+        let dispatch = |sid: &hbb_common::SessionID, e: &rdev::Event| {
+            try_dispatch(Some(sid), e, "map", || "windows".into(), |_| {})
+        };
+        let pending_for = |k: Key| RELEASE_ACTIONS.lock().unwrap().get(&k).cloned();
+
+        assert!(dispatch(&SID_A, &make_press(Key::KeyW)));
+        assert_eq!(pending_for(Key::KeyW), Some((SID_A, action_id::CLOSE_TAB.to_owned())));
+        assert!(dispatch(&SID_A, &make_press(Key::KeyW)), "repeat is consumed");
+        assert_eq!(pending_for(Key::KeyW), Some((SID_A, action_id::CLOSE_TAB.to_owned())));
+        assert!(dispatch(&SID_B, &make_release(Key::KeyW)), "release is consumed wherever it lands");
+        assert_eq!(pending_for(Key::KeyW), None, "the action ran on the release");
+
+        assert!(dispatch(&SID_A, &make_press(Key::KeyP)));
+        assert_eq!(pending_for(Key::KeyP), None, "other actions still run on the press");
+        assert!(dispatch(&SID_A, &make_release(Key::KeyP)));
+
+        assert!(dispatch(&SID_A, &make_press(Key::KeyW)));
+        clear_session_state(&SID_A);
+        assert_eq!(pending_for(Key::KeyW), None, "a closed session owes nothing");
+        assert!(dispatch(&SID_A, &make_release(Key::KeyW)), "but its key stays owned");
+        release_chord(chord);
+    }
+
     #[cfg(feature = "flutter")]
     const SID_A: hbb_common::SessionID = hbb_common::SessionID::from_u128(0xA);
     #[cfg(feature = "flutter")]
@@ -1212,6 +1292,7 @@ mod tests {
     #[cfg(feature = "flutter")]
     fn reset_fired_keys() {
         FIRED_KEYS.lock().unwrap().clear();
+        RELEASE_ACTIONS.lock().unwrap().clear();
     }
 
     #[cfg(feature = "flutter")]