fix(hotkey): 收口桌面热键适配边界#38
Merged
Merged
Conversation
Issue Open-Less#36 exposed that the desktop hotkey path was still mixing platform hooks, UI copy, and coordinator state. This change keeps the existing macOS EventTap path, introduces an explicit Windows low-level keyboard adapter for modifier-only triggers, and makes the frontend render hotkey choices and status from capability/status data instead of OS string branching. Constraint: Must preserve the existing macOS CGEventTap implementation Constraint: Windows modifier-only triggers must not be silently downgraded to registered shortcuts Rejected: Keep rdev as the only non-macOS path | cannot reliably model right-side modifier-only behavior on Windows Rejected: Patch UI strings only | would leave root-cause visibility and adapter boundaries coupled Confidence: medium Scope-risk: moderate Reversibility: clean Directive: Keep platform hook specifics inside the hotkey adapter layer and feed UI from capability/status APIs rather than OS hardcoding Tested: npm run build; cargo check Not-tested: Windows native hook runtime on real hardware; cross-target cargo check blocked by missing x86_64-w64-mingw32-gcc in this environment Related: Open-Less#36
Reviewer's GuideRefactors global hotkey handling behind a new HotkeyAdapter abstraction with per-platform implementations (macOS EventTap, Windows low-level keyboard hook, Linux/other via rdev), exposes HotkeyCapability/HotkeyStatus/HotkeyInstallError to the frontend via IPC, and updates all UI hotkey copy and settings to consume backend-reported capability and current binding instead of OS string checks, including changing Windows defaults to right Control hold-to-talk. Sequence diagram for get_hotkey_capability IPC flowsequenceDiagram
actor User
participant ReactUI as ReactComponent
participant Ctx as HotkeySettingsContext
participant IPC as ipc_getHotkeyCapability
participant Tauri as TauriCommands
participant Coord as Coordinator
participant Monitor as HotkeyMonitor
participant Types as HotkeyCapability_current
User->>ReactUI: Open Settings / Onboarding / Overview
ReactUI->>Ctx: useHotkeySettings()
Ctx->>Ctx: refresh()
Ctx->>IPC: getHotkeyCapability()
IPC->>Tauri: invoke get_hotkey_capability
Tauri->>Coord: get_hotkey_capability()
Coord->>Monitor: HotkeyMonitor::capability()
Monitor->>Types: HotkeyCapability::current()
Types-->>Monitor: HotkeyCapability
Monitor-->>Coord: HotkeyCapability
Coord-->>Tauri: HotkeyCapability
Tauri-->>IPC: HotkeyCapability
IPC-->>Ctx: HotkeyCapability
Ctx-->>ReactUI: { prefs, hotkey, capability }
ReactUI->>ReactUI: Render triggers, labels,
ReactUI->>ReactUI: permission hints based on capability
Class diagram for Rust hotkey adapter and capability typesclassDiagram
class HotkeyTrigger {
+RightOption
+LeftOption
+RightControl
+LeftControl
+RightCommand
+Fn
+RightAlt
+display_name() String
}
class HotkeyMode {
<<enum>>
+Toggle
+Hold
}
class HotkeyAdapterKind {
<<enum>>
+MacEventTap
+WindowsLowLevel
+Rdev
+display_name() String
}
class HotkeyBinding {
+HotkeyTrigger trigger
+HotkeyMode mode
+default() HotkeyBinding
}
class HotkeyCapability {
+HotkeyAdapterKind adapter
+Vec~HotkeyTrigger~ available_triggers
+bool requires_accessibility_permission
+bool supports_modifier_only_trigger
+bool supports_side_specific_modifiers
+bool explicit_fallback_available
+Option~String~ status_hint
+current() HotkeyCapability
}
class HotkeyInstallError {
+String code
+String message
+fmt(f) fmt::Result
}
class HotkeyStatusState {
<<enum>>
+Starting
+Installed
+Failed
}
class HotkeyStatus {
+HotkeyAdapterKind adapter
+HotkeyStatusState state
+Option~String~ message
+Option~HotkeyInstallError~ last_error
+default() HotkeyStatus
}
class HotkeyEvent {
<<enum>>
+Pressed
+Released
+Cancelled
}
class HotkeyAdapter {
<<trait>>
+kind() HotkeyAdapterKind
+update_binding(binding HotkeyBinding) void
+shutdown() void
}
class Shared {
+RwLock~HotkeyBinding~ binding
+AtomicBool trigger_held
}
class HotkeyMonitor {
-Box~dyn HotkeyAdapter~ adapter
+start(binding HotkeyBinding, tx Sender~HotkeyEvent~) Result~HotkeyMonitor, HotkeyInstallError~
+update_binding(binding HotkeyBinding) void
+kind() HotkeyAdapterKind
+capability() HotkeyCapability
}
class MacHotkeyAdapter {
-Arc~Shared~ shared
+kind() HotkeyAdapterKind
+update_binding(binding HotkeyBinding) void
}
class WindowsHotkeyAdapter {
-Arc~Shared~ shared
-u32 thread_id
+kind() HotkeyAdapterKind
+update_binding(binding HotkeyBinding) void
+shutdown() void
}
class RdevHotkeyAdapter {
-Arc~Shared~ shared
+kind() HotkeyAdapterKind
+update_binding(binding HotkeyBinding) void
}
class ListenerThread~T~ {
+Arc~Shared~ shared
+T startup
}
HotkeyBinding --> HotkeyTrigger
HotkeyBinding --> HotkeyMode
HotkeyCapability --> HotkeyAdapterKind
HotkeyCapability --> HotkeyTrigger
HotkeyStatus --> HotkeyAdapterKind
HotkeyStatus --> HotkeyStatusState
HotkeyStatus --> HotkeyInstallError
HotkeyMonitor o--> HotkeyAdapter
HotkeyMonitor --> HotkeyCapability
MacHotkeyAdapter ..|> HotkeyAdapter
WindowsHotkeyAdapter ..|> HotkeyAdapter
RdevHotkeyAdapter ..|> HotkeyAdapter
MacHotkeyAdapter --> Shared
WindowsHotkeyAdapter --> Shared
RdevHotkeyAdapter --> Shared
ListenerThread~T~ --> Shared
class Coordinator {
+hotkey_status() HotkeyStatus
+hotkey_capability() HotkeyCapability
+stop_hotkey_listener() void
}
Coordinator --> HotkeyMonitor
Coordinator --> HotkeyStatus
Coordinator --> HotkeyCapability
Class diagram for frontend hotkey types and HotkeySettingsContextclassDiagram
class HotkeyTrigger {
<<type alias>>
+rightOption
+leftOption
+rightControl
+leftControl
+rightCommand
+fn
+rightAlt
}
class HotkeyAdapterKind {
<<union>>
+'macEventTap'
+'windowsLowLevel'
+'rdev'
}
class HotkeyBinding {
+HotkeyTrigger trigger
+HotkeyMode mode
}
class HotkeyMode {
<<union>>
+'toggle'
+'hold'
}
class HotkeyCapability {
+HotkeyAdapterKind adapter
+HotkeyTrigger[] availableTriggers
+boolean requiresAccessibilityPermission
+boolean supportsModifierOnlyTrigger
+boolean supportsSideSpecificModifiers
+boolean explicitFallbackAvailable
+string statusHint
}
class HotkeyInstallError {
+string code
+string message
}
class HotkeyStatusState {
<<union>>
+'starting'
+'installed'
+'failed'
}
class HotkeyStatus {
+HotkeyAdapterKind adapter
+HotkeyStatusState state
+string message
+HotkeyInstallError lastError
}
class UserPreferences {
+HotkeyBinding hotkey
+string defaultMode
+string[] enabledModes
+boolean launchAtLogin
+boolean showCapsule
+string activeLlmProvider
+string polishMode
}
class HotkeySettingsContextValue {
+UserPreferences prefs
+HotkeyBinding hotkey
+HotkeyCapability capability
+boolean loading
+refresh() Promise~void~
+updatePrefs(next UserPreferences) Promise~void~
}
class HotkeySettingsProvider {
+children ReactNode
}
class useHotkeySettings {
+useHotkeySettings() HotkeySettingsContextValue
}
class hotkey_ts {
+HOTKEY_TRIGGER_LABEL Record~HotkeyTrigger, string~
+getHotkeyTriggerLabel(trigger HotkeyTrigger) string
+getHotkeyStartStopLabel(binding HotkeyBinding) string
+getHotkeyUsageHint(binding HotkeyBinding) string
}
class ipc_ts {
+getSettings() Promise~UserPreferences~
+setSettings(next UserPreferences) Promise~void~
+getHotkeyCapability() Promise~HotkeyCapability~
+getHotkeyStatus() Promise~HotkeyStatus~
}
HotkeySettingsContextValue --> UserPreferences
HotkeySettingsContextValue --> HotkeyBinding
HotkeySettingsContextValue --> HotkeyCapability
HotkeySettingsProvider --> HotkeySettingsContextValue
useHotkeySettings --> HotkeySettingsContextValue
HotkeyCapability --> HotkeyAdapterKind
HotkeyCapability --> HotkeyTrigger
HotkeyStatus --> HotkeyAdapterKind
HotkeyStatus --> HotkeyStatusState
HotkeyStatus --> HotkeyInstallError
UserPreferences --> HotkeyBinding
HotkeySettingsProvider ..> ipc_ts : uses
HotkeySettingsProvider ..> HotkeySettingsContextValue : provides
hotkey_ts --> HotkeyBinding
hotkey_ts --> HotkeyTrigger
class SettingsPage {
+RecordingSection
+ShortcutsSection
+PermissionsSection
}
class OverviewPage
class HistoryPage
class FloatingShell
class Onboarding
SettingsPage ..> useHotkeySettings : uses
OverviewPage ..> useHotkeySettings : uses
HistoryPage ..> useHotkeySettings : uses
FloatingShell ..> useHotkeySettings : uses
Onboarding ..> useHotkeySettings : uses
SettingsPage ..> hotkey_ts : uses
OverviewPage ..> hotkey_ts : uses
HistoryPage ..> hotkey_ts : uses
FloatingShell ..> hotkey_ts : uses
Onboarding ..> hotkey_ts : uses
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- On the Windows adapter, the mapping in
trigger_to_vk_code(e.g.LeftOption→VK_RMENU,Fn→VK_RCONTROL) is a bit surprising—consider either aligning these more closely with their physical keys or adding a brief comment explaining the rationale so future changes don’t accidentally break the intended semantics. - Several frontend sections independently call
getSettings/getHotkeyCapabilityand render separate "加载中…" states (RecordingSection, ShortcutsSection, PermissionsSection, Overview, History, FloatingShell); consider centralizing hotkey capability and binding in a shared context or hook to avoid duplicated IPC calls and slightly smoother initial rendering.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- On the Windows adapter, the mapping in `trigger_to_vk_code` (e.g. `LeftOption` → `VK_RMENU`, `Fn` → `VK_RCONTROL`) is a bit surprising—consider either aligning these more closely with their physical keys or adding a brief comment explaining the rationale so future changes don’t accidentally break the intended semantics.
- Several frontend sections independently call `getSettings`/`getHotkeyCapability` and render separate "加载中…" states (RecordingSection, ShortcutsSection, PermissionsSection, Overview, History, FloatingShell); consider centralizing hotkey capability and binding in a shared context or hook to avoid duplicated IPC calls and slightly smoother initial rendering.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Windows cross-target cargo check exposed two target-specific mismatches in the new low-level keyboard hook path: the hook struct import needed to match the windows crate module layout used here, and the injected-event flag access needed the wrapper-field form expected by that type. I also split the default hotkey binding cfg blocks so the Windows default stays explicit without leaving a cross-target unreachable branch behind. Constraint: Must preserve the new native Windows low-level hook path added for issue 36 Rejected: Skip Windows-target verification | would leave the affected platform path unproven Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep Windows hook imports and flag access aligned with this repo's windows crate APIs Tested: cargo check; cargo check --target x86_64-pc-windows-gnu Not-tested: Windows real hardware runtime behavior
The Windows adapter now documents why a few cross-platform trigger aliases map to the same modifier virtual key, so follow-up edits do not accidentally undo issue-36 semantics. On the frontend, hotkey settings and capability now load through one shared provider instead of repeated per-view IPC fetches, which cuts redundant loading branches across onboarding, shell, overview, history, and the hotkey-related settings sections. Constraint: Keep the issue-36 behavior intact while making the review follow-ups minimal Rejected: Rework the full settings data flow | broader than the review comments required Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep shared hotkey state limited to binding and capability unless another cross-view settings dependency is proven Tested: npm run build; cargo check; cargo check --target x86_64-pc-windows-gnu Not-tested: Real Windows runtime hotkey behavior and interactive UI smoke test
Collaborator
Author
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The three platform-specific
start_adapterimplementations (macOS,windows,rdev) share a lot of boilerplate (allocatingShared, spawning a thread, status channel + timeout); consider extracting a small helper to centralize this pattern so adapter-specific code only needs to define the listen loop. - The Windows low-level hook thread runs a
GetMessageWloop without any explicit shutdown signal; it may be worth adding a termination mechanism (e.g., posting a quit message from the coordinator) so the hook can be cleanly torn down on app shutdown or adapter reinitialization.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The three platform-specific `start_adapter` implementations (`macOS`, `windows`, `rdev`) share a lot of boilerplate (allocating `Shared`, spawning a thread, status channel + timeout); consider extracting a small helper to centralize this pattern so adapter-specific code only needs to define the listen loop.
- The Windows low-level hook thread runs a `GetMessageW` loop without any explicit shutdown signal; it may be worth adding a termination mechanism (e.g., posting a quit message from the coordinator) so the hook can be cleanly torn down on app shutdown or adapter reinitialization.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
The three platform adapters were all repeating the same startup scaffolding for shared state, thread launch, and readiness reporting, so that boilerplate now lives in one helper and each adapter keeps only its platform-specific listen loop. The Windows low-level hook also now carries an explicit shutdown path by posting WM_QUIT to the hook thread when the monitor is dropped during app exit, which keeps hook teardown aligned with coordinator lifecycle changes. Constraint: Keep issue-36 hotkey behavior unchanged while tightening adapter lifecycle handling Rejected: Leave Windows hook teardown implicit | would keep GetMessageW loop lifetime coupled to process exit only Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep adapter startup centralized only for shared scaffolding; platform event semantics should stay in their own modules Tested: cargo fmt --all; cargo check; cargo check --target x86_64-pc-windows-gnu Not-tested: Real Windows runtime shutdown and hook reinitialization behavior
Collaborator
Author
|
@sourcery-ai review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
摘要
Fixes #36。
本 PR 解决桌面热键路径中平台 Hook、UI 文案和 Coordinator 状态混在一起的问题。
本次改动将热键能力收口到统一的后端适配层，保留现有 macOS CGEventTap 实现，并为 Windows 新增显式的低级键盘 Hook 适配器，用于支持右侧修饰键和 modifier-only 按住说话触发。前端不再根据 OS 字符串硬编码热键选项和状态，而是统一根据后端返回的 capability、status 和当前 binding 渲染。
修复 / 新增 / 改进
新增统一热键适配边界：
HotkeyAdapterHotkeyCapabilityHotkeyStatusHotkeyInstallError保留现有 macOS CGEventTap 热键实现，不改动既有 macOS EventTap 路径。
Windows 热键实现不再走泛化的
rdev分支，改为显式的WH_KEYBOARD_LLlow-level keyboard hook 适配器。Windows modifier-only 触发不再被静默降级为普通 registered shortcut。
Coordinator 只消费统一的热键边沿事件，不再接触具体平台 Hook 实现细节。
前端热键展示统一走
lib/hotkey.ts，不再使用类似os === 'win' ? ... : ...的 OS 字符串分支。UI 热键选项、当前状态和文案改为基于后端 capability/status/current binding 渲染。
Windows 默认热键调整为右 Control 按住说话。
补充平台适配层文档：
docs/platform-adapter-architecture.md兼容
不包含：
对现有用户 / 本地环境 / 构建流程的影响：
测试计划
命令：
npm run build结果：通过
证据路径：本地构建输出
命令：
cargo check结果：通过
证据路径：本地检查输出
Windows native hook runtime on real hardware
cross-target cargo check
主要改动文件
docs/platform-adapter-architecture.mdopenless-all/app/src-tauri/src/hotkey.rsopenless-all/app/src-tauri/src/types.rsopenless-all/app/src-tauri/src/coordinator.rsopenless-all/app/src-tauri/src/commands.rsopenless-all/app/src-tauri/src/lib.rsopenless-all/app/src/lib/types.tsopenless-all/app/src/lib/ipc.tsopenless-all/app/src/lib/hotkey.tsopenless-all/app/src/pages/Settings.tsxopenless-all/app/src/pages/Overview.tsxopenless-all/app/src/pages/History.tsxopenless-all/app/src/components/FloatingShell.tsxopenless-all/app/src/components/Onboarding.tsxSummary by Sourcery
Introduce a unified hotkey adapter boundary across platforms and update UI to consume backend-reported hotkey capability and status instead of OS-specific logic.
New Features:
Bug Fixes:
Enhancements:
Summary by Sourcery
Unify global hotkey handling behind a platform-specific adapter boundary and update both backend and frontend to consume explicit hotkey capabilities and status, including a new native Windows low-level keyboard hook and adjusted defaults.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: