From 1d0c1a68dfa27a7982607de72e055b3ade5b0253 Mon Sep 17 00:00:00 2001 From: haritabh-z01 Date: Sun, 28 Dec 2025 01:04:29 +0530 Subject: [PATCH] chore: adjust shortcut validation rules --- .../desktop/src/components/shortcut-input.tsx | 24 +- .../src/main/managers/shortcut-manager.ts | 48 ++ .../main/pages/settings/shortcuts/index.tsx | 23 +- .../shared/OnboardingShortcutInput.tsx | 6 + apps/desktop/src/trpc/routers/settings.ts | 59 +-- apps/desktop/src/utils/shortcut-validation.ts | 415 ++++++++++++++++++ .../Sources/SwiftHelper/ShortcutManager.swift | 73 ++- .../Sources/SwiftHelper/main.swift | 11 + .../windows-helper/src/ShortcutManager.cs | 72 ++- .../windows-helper/src/ShortcutMonitor.cs | 16 + 10 files changed, 660 insertions(+), 87 deletions(-) create mode 100644 apps/desktop/src/utils/shortcut-validation.ts diff --git a/apps/desktop/src/components/shortcut-input.tsx b/apps/desktop/src/components/shortcut-input.tsx index 06b5ae7..3078cb7 100644 --- a/apps/desktop/src/components/shortcut-input.tsx +++ b/apps/desktop/src/components/shortcut-input.tsx @@ -21,7 +21,10 @@ type ValidationResult = { error?: string; }; -function validateShortcut(keys: string[]): ValidationResult { +/** + * Basic format validation only - business logic validation happens on backend + */ +function validateShortcutFormat(keys: string[]): ValidationResult { if (keys.length === 0) { return { valid: false, error: "No keys detected" }; } @@ -29,24 +32,18 @@ function validateShortcut(keys: string[]): ValidationResult { if (keys.length > MAX_KEY_COMBINATION_LENGTH) { return { valid: false, - error: `Maximum ${MAX_KEY_COMBINATION_LENGTH} keys allowed`, + error: `Too many keys - use ${MAX_KEY_COMBINATION_LENGTH} or fewer`, }; } const modifierKeys = keys.filter((key) => MODIFIER_KEYS.includes(key)); const regularKeys = keys.filter((key) => !MODIFIER_KEYS.includes(key)); - // Require at least one modifier key - if (modifierKeys.length === 0) { - return { - valid: false, - error: - "At least one modifier key (Cmd, Win, Ctrl, Alt, Shift, Fn, etc) is required", - }; - } - // Return array format: modifiers first, then regular keys - return { valid: true, shortcut: [...modifierKeys, ...regularKeys] }; + return { + valid: true, + shortcut: [...modifierKeys, ...regularKeys], + }; } function RecordingDisplay({ @@ -152,9 +149,10 @@ export function ShortcutInput({ // When any key is released, validate the combination if (previousKeys.length > 0 && keys.length < previousKeys.length) { - const result = validateShortcut(previousKeys); + const result = validateShortcutFormat(previousKeys); if (result.valid && result.shortcut) { + // Basic format is valid - let parent handle backend validation onChange(result.shortcut); } else { toast.error(result.error || "Invalid key combination"); diff --git a/apps/desktop/src/main/managers/shortcut-manager.ts b/apps/desktop/src/main/managers/shortcut-manager.ts index 1addc3f..6d970ea 100644 --- a/apps/desktop/src/main/managers/shortcut-manager.ts +++ b/apps/desktop/src/main/managers/shortcut-manager.ts @@ -6,6 +6,11 @@ import { getKeyNameFromPayload } from "@/utils/keycode-map"; import { isWindows } from "@/utils/platform"; import { KeyEventPayload, HelperEvent } from "@amical/types"; import { logger } from "@/main/logger"; +import { + validateShortcutComprehensive, + type ShortcutType, + type ValidationResult, +} from "@/utils/shortcut-validation"; const log = logger.main; @@ -78,6 +83,49 @@ export class ShortcutManager extends EventEmitter { this.syncShortcutsToNative(); // fire-and-forget } + /** + * Set a shortcut with full validation. + * Validates, persists, updates internal state, and syncs to native. + */ + async setShortcut( + type: ShortcutType, + keys: string[], + ): Promise { + // Get the other shortcut for cross-validation + const otherShortcut = + type === "pushToTalk" + ? this.shortcuts.toggleRecording + : this.shortcuts.pushToTalk; + + // Validate the shortcut + const result = validateShortcutComprehensive({ + currentShortcut: keys, + otherShortcut, + shortcutType: type, + platform: process.platform, + }); + + if (!result.valid) { + return result; + } + + // Persist to settings + const updatedShortcuts = { + ...this.shortcuts, + [type]: keys, + }; + await this.settingsService.setShortcuts(updatedShortcuts); + + // Update internal state + this.shortcuts = updatedShortcuts; + log.info("Shortcut updated", { type, keys }); + + // Sync to native helper + await this.syncShortcutsToNative(); + + return result; + } + setIsRecordingShortcut(isRecording: boolean) { this.isRecordingShortcut = isRecording; log.info("Shortcut recording state changed", { isRecording }); diff --git a/apps/desktop/src/renderer/main/pages/settings/shortcuts/index.tsx b/apps/desktop/src/renderer/main/pages/settings/shortcuts/index.tsx index 2fa89c8..927cdf2 100644 --- a/apps/desktop/src/renderer/main/pages/settings/shortcuts/index.tsx +++ b/apps/desktop/src/renderer/main/pages/settings/shortcuts/index.tsx @@ -20,17 +20,24 @@ export function ShortcutsSettingsPage() { const utils = api.useUtils(); const setShortcutMutation = api.settings.setShortcut.useMutation({ - onSuccess: (_data, variables) => { + onSuccess: (data, variables) => { utils.settings.getShortcuts.invalidate(); - toast.success( - variables.type === "pushToTalk" - ? "Push to talk shortcut updated" - : "Toggle Recording shortcut updated", - ); + + // Show warning if there is one + if (data.warning) { + toast.warning(data.warning); + } else { + toast.success( + variables.type === "pushToTalk" + ? "Push to talk shortcut updated" + : "Toggle Recording shortcut updated", + ); + } }, onError: (error) => { - console.error("Failed to save shortcut:", error); - toast.error("Failed to save shortcut. Please try again."); + toast.error(error.message); + // Revert to saved value + utils.settings.getShortcuts.invalidate(); }, }); diff --git a/apps/desktop/src/renderer/onboarding/components/shared/OnboardingShortcutInput.tsx b/apps/desktop/src/renderer/onboarding/components/shared/OnboardingShortcutInput.tsx index 98a6615..afd7118 100644 --- a/apps/desktop/src/renderer/onboarding/components/shared/OnboardingShortcutInput.tsx +++ b/apps/desktop/src/renderer/onboarding/components/shared/OnboardingShortcutInput.tsx @@ -2,6 +2,7 @@ import { useState, useEffect } from "react"; import { Label } from "@/components/ui/label"; import { ShortcutInput } from "@/components/shortcut-input"; import { api } from "@/trpc/react"; +import { toast } from "sonner"; /** * Push to Talk shortcut input for onboarding @@ -17,6 +18,11 @@ export function OnboardingShortcutInput() { onSuccess: () => { utils.settings.getShortcuts.invalidate(); }, + onError: (error) => { + toast.error(error.message); + // Revert to saved value + utils.settings.getShortcuts.invalidate(); + }, }); // Load current shortcut diff --git a/apps/desktop/src/trpc/routers/settings.ts b/apps/desktop/src/trpc/routers/settings.ts index 5db791d..6e70429 100644 --- a/apps/desktop/src/trpc/routers/settings.ts +++ b/apps/desktop/src/trpc/routers/settings.ts @@ -1,4 +1,5 @@ import { observable } from "@trpc/server/observable"; +import { TRPCError } from "@trpc/server"; import { z } from "zod"; import { app } from "electron"; import path from "node:path"; @@ -163,45 +164,27 @@ export const settingsRouter = createRouter({ setShortcut: procedure .input(SetShortcutSchema) .mutation(async ({ input, ctx }) => { - try { - const settingsService = - ctx.serviceManager.getService("settingsService"); - if (!settingsService) { - throw new Error("SettingsService not available"); - } - - // Get current shortcuts and update the specific one - const currentShortcuts = await settingsService.getShortcuts(); - const updatedShortcuts = { - ...currentShortcuts, - [input.type]: input.shortcut, - }; - - await settingsService.setShortcuts(updatedShortcuts); - - const logger = ctx.serviceManager.getLogger(); - if (logger) { - logger?.main.info("Shortcut updated", input); - } - - // Notify shortcut manager to reload shortcuts - const shortcutManager = - ctx.serviceManager.getService("shortcutManager"); - if (shortcutManager) { - await shortcutManager.reloadShortcuts(); - if (logger) { - logger.main.info("Shortcut manager notified of shortcut change"); - } - } - - return true; - } catch (error) { - const logger = ctx.serviceManager.getLogger(); - if (logger) { - logger.main.error("Error setting shortcut:", error); - } - throw error; + const shortcutManager = ctx.serviceManager.getService("shortcutManager"); + if (!shortcutManager) { + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: "ShortcutManager not available", + }); } + + const result = await shortcutManager.setShortcut( + input.type, + input.shortcut, + ); + + if (!result.valid) { + throw new TRPCError({ + code: "BAD_REQUEST", + message: result.error || "Invalid shortcut", + }); + } + + return { success: true, warning: result.warning }; }), // Set shortcut recording state diff --git a/apps/desktop/src/utils/shortcut-validation.ts b/apps/desktop/src/utils/shortcut-validation.ts new file mode 100644 index 0000000..91903c6 --- /dev/null +++ b/apps/desktop/src/utils/shortcut-validation.ts @@ -0,0 +1,415 @@ +/** + * Shortcut validation utilities + * Provides comprehensive validation for keyboard shortcuts + */ + +export type ShortcutType = "pushToTalk" | "toggleRecording"; + +export interface ValidationContext { + currentShortcut: string[]; + otherShortcut: string[]; + shortcutType: ShortcutType; + platform: NodeJS.Platform; +} + +export interface ValidationResult { + valid: boolean; + error?: string; + warning?: string; +} + +// Maximum number of keys allowed in a shortcut +const MAX_KEY_COMBINATION_LENGTH = 4; + +// Keys considered modifiers +const MODIFIER_KEYS = ["Cmd", "Win", "Ctrl", "Alt", "Shift", "Fn"]; + +// Left/right modifier pairs (for duplicate modifier detection) +const MODIFIER_PAIRS: [string, string][] = [ + ["LShift", "RShift"], + ["LCtrl", "RCtrl"], + ["LAlt", "RAlt"], + ["LCmd", "RCmd"], + ["LWin", "RWin"], +]; + +// Keys that are valid on their own (not alphanumeric) +const SPECIAL_KEYS = [ + "Space", + "Tab", + "Enter", + "Escape", + "Delete", + "Backspace", + "Up", + "Down", + "Left", + "Right", + "Home", + "End", + "PageUp", + "PageDown", + "Insert", + "F1", + "F2", + "F3", + "F4", + "F5", + "F6", + "F7", + "F8", + "F9", + "F10", + "F11", + "F12", + "F13", + "F14", + "F15", + "F16", + "F17", + "F18", + "F19", + "F20", + "F21", + "F22", + "F23", + "F24", +]; + +// macOS reserved shortcuts +const RESERVED_SHORTCUTS_MACOS: string[][] = [ + // Clipboard + ["Cmd", "C"], + ["Cmd", "V"], + ["Cmd", "X"], + ["Cmd", "Z"], + ["Cmd", "Shift", "Z"], + // Window/App management + ["Cmd", "Q"], + ["Cmd", "W"], + ["Cmd", "M"], + ["Cmd", "H"], + ["Cmd", "Tab"], + ["Cmd", "Space"], + // Screenshots + ["Cmd", "Shift", "3"], + ["Cmd", "Shift", "4"], + ["Cmd", "Shift", "5"], + // Mission Control + ["Ctrl", "Up"], + ["Ctrl", "Down"], + ["Ctrl", "Left"], + ["Ctrl", "Right"], + // File operations + ["Cmd", "N"], + ["Cmd", "O"], + ["Cmd", "S"], + ["Cmd", "P"], + // Edit + ["Cmd", "A"], + ["Cmd", "F"], + ["Cmd", "G"], + ["Cmd", "Shift", "G"], + ["Cmd", "R"], + // Text formatting + ["Cmd", "B"], + ["Cmd", "I"], + ["Cmd", "U"], + // Navigation + ["Cmd", "Left"], + ["Cmd", "Right"], + ["Cmd", "Up"], + ["Cmd", "Down"], + // Selection + ["Cmd", "Shift", "Left"], + ["Cmd", "Shift", "Right"], + ["Cmd", "Shift", "Up"], + ["Cmd", "Shift", "Down"], + // System + ["Cmd", "Alt", "Escape"], + // Delete + ["Cmd", "Backspace"], + ["Alt", "Backspace"], + ["Alt", "Delete"], + // Tabs + ["Cmd", "T"], + ["Cmd", "Shift", "T"], + // Zoom + ["Cmd", "="], + ["Cmd", "-"], + // Other common + ["Cmd", ","], +]; + +// Windows reserved shortcuts +const RESERVED_SHORTCUTS_WINDOWS: string[][] = [ + // Clipboard + ["Ctrl", "C"], + ["Ctrl", "V"], + ["Ctrl", "X"], + ["Ctrl", "Z"], + ["Ctrl", "Y"], + // Window/App management + ["Alt", "Tab"], + ["Alt", "F4"], + ["F11"], + // File operations + ["Ctrl", "N"], + ["Ctrl", "O"], + ["Ctrl", "S"], + ["Ctrl", "P"], + ["Ctrl", "T"], + ["Ctrl", "W"], + // Edit + ["Ctrl", "A"], + ["Ctrl", "F"], + ["Ctrl", "G"], + ["Ctrl", "R"], + ["F5"], + // Text formatting + ["Ctrl", "B"], + ["Ctrl", "I"], + ["Ctrl", "U"], + // Navigation + ["Home"], + ["End"], + ["Ctrl", "Home"], + ["Ctrl", "End"], + ["Alt", "Left"], + ["Alt", "Right"], + // Selection + ["Shift", "Home"], + ["Shift", "End"], + ["Ctrl", "Shift", "Home"], + ["Ctrl", "Shift", "End"], + // System + ["Ctrl", "Alt", "Delete"], + ["Ctrl", "Shift", "Escape"], + // Windows key shortcuts + ["Win", "E"], + ["Win", "R"], + ["Win", "L"], + ["Win", "D"], + ["Win", "Tab"], + ["Win", "I"], + ["Win", "S"], + ["Win", "X"], + ["Win", "P"], + ["Win", "Up"], + ["Win", "Down"], + ["Win", "Q"], + // Delete + ["Ctrl", "Backspace"], + ["Ctrl", "Delete"], + // Tabs + ["Ctrl", "Shift", "T"], + // Zoom + ["Ctrl", "="], + ["Ctrl", "-"], + // Other + ["Ctrl", "K"], +]; + +/** + * Helper function to compare two sorted arrays + */ +function arraysEqual(a: string[], b: string[]): boolean { + if (a.length !== b.length) return false; + return a.every((val, idx) => val.toUpperCase() === b[idx].toUpperCase()); +} + +/** + * Normalize and sort keys for comparison + */ +function normalizeKeys(keys: string[]): string[] { + return keys.map((k) => k.toUpperCase()).sort(); +} + +/** + * Check if the shortcut has too many keys + */ +export function checkMaxKeysLength(keys: string[]): ValidationResult { + if (keys.length === 0) { + return { valid: false, error: "No keys detected" }; + } + if (keys.length > MAX_KEY_COMBINATION_LENGTH) { + return { + valid: false, + error: `Too many keys - use ${MAX_KEY_COMBINATION_LENGTH} or fewer`, + }; + } + return { valid: true }; +} + +/** + * Check if the shortcut is already assigned to another action + */ +export function checkDuplicateShortcut( + currentKeys: string[], + otherKeys: string[], +): ValidationResult { + if (otherKeys.length === 0) return { valid: true }; + + const currentNormalized = normalizeKeys(currentKeys); + const otherNormalized = normalizeKeys(otherKeys); + + if (arraysEqual(currentNormalized, otherNormalized)) { + return { + valid: false, + error: "Shortcut already assigned to another action", + }; + } + return { valid: true }; +} + +/** + * Check if the shortcut conflicts with a system shortcut + */ +export function checkReservedShortcut( + keys: string[], + platform: NodeJS.Platform, +): ValidationResult { + const reserved = + platform === "darwin" + ? RESERVED_SHORTCUTS_MACOS + : RESERVED_SHORTCUTS_WINDOWS; + + const normalizedKeys = normalizeKeys(keys); + + for (const reservedShortcut of reserved) { + const normalizedReserved = normalizeKeys(reservedShortcut); + if (arraysEqual(normalizedKeys, normalizedReserved)) { + const displayShortcut = keys.join("+"); + return { + valid: false, + error: `${displayShortcut} conflicts with a system shortcut`, + }; + } + } + return { valid: true }; +} + +/** + * Check if all keys are alphanumeric (letters, digits, punctuation only) + * Without a modifier, such shortcuts are not valid + */ +export function checkAlphanumericOnly(keys: string[]): ValidationResult { + // Check if any key is a modifier + const hasModifier = keys.some((key) => MODIFIER_KEYS.includes(key)); + if (hasModifier) { + return { valid: true }; + } + + // Check if any key is a special key (Space, F1-F24, navigation, etc.) + const hasSpecialKey = keys.some((key) => SPECIAL_KEYS.includes(key)); + if (hasSpecialKey) { + return { valid: true }; + } + + // All keys are alphanumeric - need a modifier + return { + valid: false, + error: "Add a modifier key like Cmd, Ctrl, or Fn", + }; +} + +/** + * Check for duplicate left/right modifier pairs (Windows only) + * macOS can't distinguish left/right modifiers via its event system + */ +export function checkDuplicateModifierPairs( + keys: string[], + platform: NodeJS.Platform, +): ValidationResult { + // Only applies to Windows + if (platform === "darwin") { + return { valid: true }; + } + + for (const [left, right] of MODIFIER_PAIRS) { + if (keys.includes(left) && keys.includes(right)) { + // Extract base modifier name (remove L/R prefix) + const baseName = left.substring(1); + return { + valid: false, + error: `Can't use both left and right ${baseName} together`, + }; + } + } + return { valid: true }; +} + +/** + * Check if toggle shortcut is a subset of PTT shortcut (soft warning) + * Only warns when setting toggleRecording + */ +export function checkSubsetConflict( + currentKeys: string[], + otherKeys: string[], + shortcutType: ShortcutType, +): ValidationResult { + // Only warn when setting toggleRecording + if (shortcutType !== "toggleRecording") return { valid: true }; + if (otherKeys.length === 0 || currentKeys.length === 0) + return { valid: true }; + + const toggleNormalized = normalizeKeys(currentKeys); + const pttNormalized = normalizeKeys(otherKeys); + + // Check if toggle shortcut is a subset of PTT shortcut + const isSubset = toggleNormalized.every((key) => + pttNormalized.some((pttKey) => pttKey === key), + ); + + if (isSubset && toggleNormalized.length < pttNormalized.length) { + return { + valid: true, // Still valid, just warning + warning: + "This overlaps with your Push-to-talk shortcut and may cause issues", + }; + } + + return { valid: true }; +} + +/** + * Run all validation checks in order + * Returns first error found, or warning if all pass + */ +export function validateShortcutComprehensive( + context: ValidationContext, +): ValidationResult { + const { currentShortcut, otherShortcut, shortcutType, platform } = context; + + // 1. Max keys length check + const maxKeysCheck = checkMaxKeysLength(currentShortcut); + if (!maxKeysCheck.valid) return maxKeysCheck; + + // 2. Duplicate shortcut check + const duplicateCheck = checkDuplicateShortcut(currentShortcut, otherShortcut); + if (!duplicateCheck.valid) return duplicateCheck; + + // 3. Reserved shortcut check + const reservedCheck = checkReservedShortcut(currentShortcut, platform); + if (!reservedCheck.valid) return reservedCheck; + + // 4. Alphanumeric-only check + const alphaCheck = checkAlphanumericOnly(currentShortcut); + if (!alphaCheck.valid) return alphaCheck; + + // 5. Duplicate modifier pair check (Windows only) + const pairCheck = checkDuplicateModifierPairs(currentShortcut, platform); + if (!pairCheck.valid) return pairCheck; + + // 6. Subset conflict check (soft warning - returns valid:true with warning) + const subsetCheck = checkSubsetConflict( + currentShortcut, + otherShortcut, + shortcutType, + ); + + return { + valid: true, + warning: subsetCheck.warning, + }; +} diff --git a/packages/native-helpers/swift-helper/Sources/SwiftHelper/ShortcutManager.swift b/packages/native-helpers/swift-helper/Sources/SwiftHelper/ShortcutManager.swift index 794c05a..af14e14 100644 --- a/packages/native-helpers/swift-helper/Sources/SwiftHelper/ShortcutManager.swift +++ b/packages/native-helpers/swift-helper/Sources/SwiftHelper/ShortcutManager.swift @@ -34,6 +34,23 @@ class ShortcutManager { // ============================================================================ private var fnKeyDown: Bool = false + // ============================================================================ + // Non-Modifier Key State Tracking + // ============================================================================ + // We track currently pressed non-modifier keys across keyDown/keyUp events. + // This is necessary for multi-key shortcuts like Shift+A+B where we need to + // know that 'A' is still held when 'B' is pressed. + // + // WARNING: pressedRegularKeys can get stuck if keyUp events are missed + // (e.g., event tap disabled by timeout, sleep/wake cycles, accessibility + // permission changes). This will cause shortcuts to stop matching because + // activeKeys retains extra keys. Consider clearing this state on: + // - flagsChanged showing all modifiers released + // - App/tap re-initialization + // - Sleep/wake notifications + // ============================================================================ + private var pressedRegularKeys = Set() + private let lock = NSLock() private let dateFormatter: DateFormatter @@ -67,6 +84,22 @@ class ShortcutManager { fnKeyDown = isDown } + /// Add a regular (non-modifier) key to the tracked set + /// Called from event tap callback on keyDown events + func addRegularKey(_ key: String) { + lock.lock() + defer { lock.unlock() } + pressedRegularKeys.insert(key) + } + + /// Remove a regular (non-modifier) key from the tracked set + /// Called from event tap callback on keyUp events + func removeRegularKey(_ key: String) { + lock.lock() + defer { lock.unlock() } + pressedRegularKeys.remove(key) + } + /// Check if this key event should be consumed (prevent default behavior) /// Called from event tap callback for keyDown/keyUp events only func shouldConsumeKey(keyCode: Int, modifiers: ModifierState) -> Bool { @@ -78,24 +111,34 @@ class ShortcutManager { return false } - // Build set of currently active keys (modifiers + this regular key) - // Note: We use tracked fnKeyDown instead of modifiers.fn because macOS - // can report unreliable Fn flag on keyDown events (especially on MacBooks) - var activeKeys = Set() - if fnKeyDown { activeKeys.insert("Fn") } - if modifiers.cmd { activeKeys.insert("Cmd") } - if modifiers.ctrl { activeKeys.insert("Ctrl") } - if modifiers.alt { activeKeys.insert("Alt") } - if modifiers.shift { activeKeys.insert("Shift") } - - // Add the regular key being pressed - if let keyName = keyCodeToName(keyCode) { - activeKeys.insert(keyName) + // If we can't map this key, don't consume it - prevents unmapped keys + // (like PageUp, Home) from being incorrectly consumed when a modifier is held + guard let currentKeyName = keyCodeToName(keyCode) else { + return false } - // PTT: subset match (all PTT keys pressed, possibly with extras) + // Build set of currently active modifier keys + // Note: We use tracked fnKeyDown instead of modifiers.fn because macOS + // can report unreliable Fn flag on keyDown events (especially on MacBooks) + var activeModifiers = Set() + if fnKeyDown { activeModifiers.insert("Fn") } + if modifiers.cmd { activeModifiers.insert("Cmd") } + if modifiers.ctrl { activeModifiers.insert("Ctrl") } + if modifiers.alt { activeModifiers.insert("Alt") } + if modifiers.shift { activeModifiers.insert("Shift") } + + // Build full set of active keys (modifiers + tracked regular keys + current key) + var activeKeys = activeModifiers + activeKeys.formUnion(pressedRegularKeys) + activeKeys.insert(currentKeyName) + + // PTT: consume if building toward the shortcut + // - At least one modifier from the shortcut must be held (signals intent) + // - All currently pressed keys must be part of the shortcut (activeKeys ⊆ pttKeys) let pttKeys = Set(pushToTalkKeys) - let pttMatch = !pttKeys.isEmpty && pttKeys.isSubset(of: activeKeys) + let pttModifiers = pttKeys.intersection(["Fn", "Cmd", "Ctrl", "Alt", "Shift"]) + let hasRequiredModifier = !pttModifiers.isEmpty && !pttModifiers.isDisjoint(with: activeModifiers) + let pttMatch = !pttKeys.isEmpty && hasRequiredModifier && activeKeys.isSubset(of: pttKeys) // Toggle: exact match (only these keys pressed) let toggleKeys = Set(toggleRecordingKeys) diff --git a/packages/native-helpers/swift-helper/Sources/SwiftHelper/main.swift b/packages/native-helpers/swift-helper/Sources/SwiftHelper/main.swift index 07c567b..214a4b3 100644 --- a/packages/native-helpers/swift-helper/Sources/SwiftHelper/main.swift +++ b/packages/native-helpers/swift-helper/Sources/SwiftHelper/main.swift @@ -72,6 +72,17 @@ func eventTapCallback( shift: event.flags.contains(.maskShift) ) + // Track regular key state for multi-key shortcuts + // We need to track which non-modifier keys are held down so that + // shortcuts like Shift+A+B can work properly + if let keyName = keyCodeToName(Int(keyCode)) { + if type == .keyDown { + ShortcutManager.shared.addRegularKey(keyName) + } else { + ShortcutManager.shared.removeRegularKey(keyName) + } + } + if ShortcutManager.shared.shouldConsumeKey(keyCode: Int(keyCode), modifiers: modifiers) { // CONSUME - prevent default behavior (e.g., cursor movement for arrow keys) return nil diff --git a/packages/native-helpers/windows-helper/src/ShortcutManager.cs b/packages/native-helpers/windows-helper/src/ShortcutManager.cs index 6f8242c..123eafd 100644 --- a/packages/native-helpers/windows-helper/src/ShortcutManager.cs +++ b/packages/native-helpers/windows-helper/src/ShortcutManager.cs @@ -30,6 +30,16 @@ namespace WindowsHelper private string[] _pushToTalkKeys = Array.Empty(); private string[] _toggleRecordingKeys = Array.Empty(); + // Track currently pressed non-modifier keys across keyDown/keyUp events. + // This is necessary for multi-key shortcuts like Shift+A+B where we need to + // know that 'A' is still held when 'B' is pressed. + // + // WARNING: _pressedRegularKeys can get stuck if keyUp events are missed + // (e.g., hook restarts, sleep/wake cycles). This will cause shortcuts to + // stop matching because activeKeys retains extra keys. Consider clearing + // this state on app re-initialization or power management events. + private readonly HashSet _pressedRegularKeys = new(); + private ShortcutManager() { } private void LogToStderr(string message) @@ -53,6 +63,30 @@ namespace WindowsHelper } } + /// + /// Add a regular (non-modifier) key to the tracked set. + /// Called from ShortcutMonitor hook callback on keyDown events. + /// + public void AddRegularKey(string key) + { + lock (_lock) + { + _pressedRegularKeys.Add(key); + } + } + + /// + /// Remove a regular (non-modifier) key from the tracked set. + /// Called from ShortcutMonitor hook callback on keyUp events. + /// + public void RemoveRegularKey(string key) + { + lock (_lock) + { + _pressedRegularKeys.Remove(key); + } + } + /// /// Check if this key event should be consumed (prevent default behavior). /// Called from ShortcutMonitor hook callback for keyDown/keyUp events only. @@ -67,23 +101,35 @@ namespace WindowsHelper return false; } - // Build set of currently active keys (modifiers + this regular key) - var activeKeys = new HashSet(); - if (modifiers.Win) activeKeys.Add("Win"); - if (modifiers.Ctrl) activeKeys.Add("Ctrl"); - if (modifiers.Alt) activeKeys.Add("Alt"); - if (modifiers.Shift) activeKeys.Add("Shift"); - - // Add the regular key being pressed - var keyName = VirtualKeyMap.GetKeyName(vkCode); - if (keyName != null) + // If we can't map this key, don't consume it - prevents unmapped keys + // (like PageUp, Home) from being incorrectly consumed when a modifier is held + var currentKeyName = VirtualKeyMap.GetKeyName(vkCode); + if (currentKeyName == null) { - activeKeys.Add(keyName); + return false; } - // PTT: subset match (all PTT keys pressed, possibly with extras) + // Build set of currently active modifier keys + var activeModifiers = new HashSet(); + if (modifiers.Win) activeModifiers.Add("Win"); + if (modifiers.Ctrl) activeModifiers.Add("Ctrl"); + if (modifiers.Alt) activeModifiers.Add("Alt"); + if (modifiers.Shift) activeModifiers.Add("Shift"); + + // Build full set of active keys (modifiers + tracked regular keys + current key) + var activeKeys = new HashSet(activeModifiers); + activeKeys.UnionWith(_pressedRegularKeys); + activeKeys.Add(currentKeyName); + + // PTT: consume if building toward the shortcut + // - At least one modifier from the shortcut must be held (signals intent) + // - All currently pressed keys must be part of the shortcut (activeKeys ⊆ pttKeys) var pttKeys = new HashSet(_pushToTalkKeys); - var pttMatch = pttKeys.Count > 0 && pttKeys.IsSubsetOf(activeKeys); + var modifierKeys = new HashSet { "Win", "Ctrl", "Alt", "Shift" }; + var pttModifiers = new HashSet(pttKeys); + pttModifiers.IntersectWith(modifierKeys); + var hasRequiredModifier = pttModifiers.Count > 0 && pttModifiers.Overlaps(activeModifiers); + var pttMatch = pttKeys.Count > 0 && hasRequiredModifier && activeKeys.IsSubsetOf(pttKeys); // Toggle: exact match (only these keys pressed) var toggleKeys = new HashSet(_toggleRecordingKeys); diff --git a/packages/native-helpers/windows-helper/src/ShortcutMonitor.cs b/packages/native-helpers/windows-helper/src/ShortcutMonitor.cs index b9138c6..9939efd 100644 --- a/packages/native-helpers/windows-helper/src/ShortcutMonitor.cs +++ b/packages/native-helpers/windows-helper/src/ShortcutMonitor.cs @@ -329,6 +329,22 @@ namespace WindowsHelper // Send regular key event KeyEventOccurred?.Invoke(this, keyEvent); + // Track regular key state for multi-key shortcuts + // We need to track which non-modifier keys are held down so that + // shortcuts like Shift+A+B can work properly + var keyName = VirtualKeyMap.GetKeyName((int)kbStruct.vkCode); + if (keyName != null) + { + if (isKeyDown) + { + ShortcutManager.Instance.AddRegularKey(keyName); + } + else + { + ShortcutManager.Instance.RemoveRegularKey(keyName); + } + } + // Check if this key event should be consumed (prevent default behavior) // Only for regular key events, not modifiers var modifierState = new ModifierState