test(windows): 增加低层热键钩子回归#44
Merged
appergb merged 1 commit intoApr 30, 2026
Merged
Conversation
Reviewer's GuideAdds an automated Windows smoke test for the native low-level keyboard hook and introduces a guarded code path to accept synthetic hotkey events plus additional logging for trigger edges. Sequence diagram for the Windows low-level hotkey smoke test reaching CoordinatorsequenceDiagram
actor Tester
participant SmokeScript as WindowsHotkeyOSHookSmokePs1
participant OpenLessExe as OpenLessExe
participant WindowsOS
participant LowLevelHook as WindowsLowLevelHook
participant HotkeyModule as HotkeyHandler
participant Coordinator
participant LogFile as OpenLessLog
Tester->>SmokeScript: Run windows-hotkey-os-hook-smoke.ps1
SmokeScript->>SmokeScript: Set env OPENLESS_SHOW_MAIN_ON_START=1
SmokeScript->>SmokeScript: Set env OPENLESS_ACCEPT_SYNTHETIC_HOTKEY_EVENTS=1
SmokeScript->>OpenLessExe: Start-Process openless.exe
OpenLessExe->>WindowsOS: Register low-level keyboard hook
WindowsOS->>LowLevelHook: Install hook callback
LowLevelHook->>LogFile: Write "hotkey listener installed" / "Windows low-level keyboard hook"
SmokeScript->>WindowsOS: Start notepad.exe
WindowsOS->>SmokeScript: Return notepad process with window handle
SmokeScript->>WindowsOS: Focus notepad window
loop Up to 3 attempts
SmokeScript->>WindowsOS: keybd_event(RControl, down)
WindowsOS->>LowLevelHook: Deliver KBDLLHOOKSTRUCT (vk=RControl, injected)
LowLevelHook->>HotkeyModule: accept_injected_events() (reads env)
HotkeyModule-->>LowLevelHook: true when env OPENLESS_ACCEPT_SYNTHETIC_HOTKEY_EVENTS=1
LowLevelHook->>HotkeyModule: dispatch_keyboard_event(vk=RControl, WM_KEYDOWN)
HotkeyModule->>LogFile: Write "[hotkey] Windows trigger pressed vk=163"
HotkeyModule->>Coordinator: Send HotkeyEvent.Pressed
Coordinator->>LogFile: Write "[coord] hotkey pressed"
SmokeScript->>LogFile: Poll for "[hotkey] Windows trigger pressed"
SmokeScript->>WindowsOS: keybd_event(RControl, up)
WindowsOS->>LowLevelHook: Deliver KBDLLHOOKSTRUCT (vk=RControl, injected)
LowLevelHook->>HotkeyModule: dispatch_keyboard_event(vk=RControl, WM_KEYUP)
HotkeyModule->>LogFile: Write "[hotkey] Windows trigger released vk=163"
end
SmokeScript->>LogFile: Poll for "[coord] hotkey pressed"
SmokeScript-->>Tester: Report "Windows low-level hook observed vk=163 and reached Coordinator"
File-Level Changes
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 found 2 issues, and left some high level feedback:
- The
accept_injected_events()check is evaluated on every hook callback; consider caching the env var in a static/once-initialized flag so the low-level keyboard hook hot path avoids repeated environment lookups. - Logging trigger press/release at
infolevel on every key edge may be noisy in normal usage; consider switching these todebugor gating them behind a dedicated debug env flag. - In
windows-hotkey-os-hook-smoke.ps1,Wait-LogPatternrepeatedly reads the entire log withGet-Content -Raw; for large logs this can be slow, so consider limiting reads (e.g.,-Tailor incremental reads) to reduce overhead while polling.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `accept_injected_events()` check is evaluated on every hook callback; consider caching the env var in a static/once-initialized flag so the low-level keyboard hook hot path avoids repeated environment lookups.
- Logging trigger press/release at `info` level on every key edge may be noisy in normal usage; consider switching these to `debug` or gating them behind a dedicated debug env flag.
- In `windows-hotkey-os-hook-smoke.ps1`, `Wait-LogPattern` repeatedly reads the entire log with `Get-Content -Raw`; for large logs this can be slow, so consider limiting reads (e.g., `-Tail` or incremental reads) to reduce overhead while polling.
## Individual Comments
### Comment 1
<location path="openless-all/app/src-tauri/src/hotkey.rs" line_range="535-538" />
<code_context>
if let Some(ctx) = callback_context() {
let keyboard = *(lparam.0 as *const KBDLLHOOKSTRUCT);
- if keyboard.flags.0 & LLKHF_INJECTED == 0 {
+ if keyboard.flags.0 & LLKHF_INJECTED == 0 || accept_injected_events() {
dispatch_keyboard_event(ctx, keyboard.vkCode, wparam.0);
}
</code_context>
<issue_to_address>
**suggestion (performance):** Consider caching the environment flag instead of re-reading it for every keyboard event.
This hook runs on every keyboard event, so repeatedly calling `accept_injected_events()` (and thus `std::env::var`) adds avoidable overhead to a hot path. Consider resolving the env var once (e.g., into a `static OnceLock<bool>` / `LazyLock<bool>` or an `AtomicBool`) and reading a cached bool here instead.
Suggested implementation:
```rust
const VK_RWIN: u32 = 0x5C;
const LLKHF_INJECTED: u32 = 0x0000_0010;
const ACCEPT_INJECTED_ENV: &str = "OPENLESS_ACCEPT_SYNTHETIC_HOTKEY_EVENTS";
/// Cached value of whether we should accept synthetic/injected keyboard events.
///
/// This is initialized once from the `OPENLESS_ACCEPT_SYNTHETIC_HOTKEY_EVENTS`
/// environment variable the first time `accept_injected_events()` is called,
/// and then read cheaply on every subsequent keyboard hook invocation.
static ACCEPT_INJECTED_EVENTS: std::sync::OnceLock<bool> = std::sync::OnceLock::new();
static HOOK_CONTEXT: AtomicPtr<CallbackContext> = AtomicPtr::new(std::ptr::null_mut());
```
```rust
fn accept_injected_events() -> bool {
// Initialize once from the environment and reuse the cached value.
*ACCEPT_INJECTED_EVENTS.get_or_init(|| {
std::env::var(ACCEPT_INJECTED_ENV)
.map(|v| v == "1" || v.eq_ignore_ascii_case("true"))
.unwrap_or(false)
})
}
```
1. Ensure there is an existing `fn accept_injected_events() -> bool` definition matching the `SEARCH` block; if the current implementation differs, adjust the `SEARCH` pattern accordingly when applying the replacement.
2. If you already import `OnceLock` elsewhere (e.g., `use std::sync::OnceLock;`), you can simplify the static type from `std::sync::OnceLock<bool>` to just `OnceLock<bool>` and rely on the existing import. If you do not, you may add `use std::sync::OnceLock;` at the top and update the static accordingly.
</issue_to_address>
### Comment 2
<location path="openless-all/app/scripts/windows-hotkey-os-hook-smoke.ps1" line_range="58-63" />
<code_context>
+ return $false
+}
+
+function Send-KeyEdge($Vk, $KeyUp) {
+ $flags = [OpenLessInput]::KEYEVENTF_EXTENDEDKEY
+ if ($KeyUp) {
+ $flags = $flags -bor [OpenLessInput]::KEYEVENTF_KEYUP
+ }
+ [OpenLessInput]::keybd_event([byte]$Vk, 0x1D, $flags, [UIntPtr]::Zero)
+}
+
</code_context>
<issue_to_address>
**issue (bug_risk):** `Send-KeyEdge` uses a fixed scan code that may not match the provided virtual key.
The helper always uses scan code `0x1D` (Control) when calling `keybd_event`, which only matches the default `$Vk = 0xA3`. If callers pass a different virtual key, the scan code/virtual key pair becomes inconsistent. Consider deriving the scan code from the virtual key (e.g., via `MapVirtualKey`) or exposing the scan code as a parameter so they always match.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+535
to
538
| if keyboard.flags.0 & LLKHF_INJECTED == 0 || accept_injected_events() { | ||
| dispatch_keyboard_event(ctx, keyboard.vkCode, wparam.0); | ||
| } | ||
| } |
There was a problem hiding this comment.
suggestion (performance): Consider caching the environment flag instead of re-reading it for every keyboard event.
This hook runs on every keyboard event, so repeatedly calling accept_injected_events() (and thus std::env::var) adds avoidable overhead to a hot path. Consider resolving the env var once (e.g., into a static OnceLock<bool> / LazyLock<bool> or an AtomicBool) and reading a cached bool here instead.
Suggested implementation:
const VK_RWIN: u32 = 0x5C;
const LLKHF_INJECTED: u32 = 0x0000_0010;
const ACCEPT_INJECTED_ENV: &str = "OPENLESS_ACCEPT_SYNTHETIC_HOTKEY_EVENTS";
/// Cached value of whether we should accept synthetic/injected keyboard events.
///
/// This is initialized once from the `OPENLESS_ACCEPT_SYNTHETIC_HOTKEY_EVENTS`
/// environment variable the first time `accept_injected_events()` is called,
/// and then read cheaply on every subsequent keyboard hook invocation.
static ACCEPT_INJECTED_EVENTS: std::sync::OnceLock<bool> = std::sync::OnceLock::new();
static HOOK_CONTEXT: AtomicPtr<CallbackContext> = AtomicPtr::new(std::ptr::null_mut());fn accept_injected_events() -> bool {
// Initialize once from the environment and reuse the cached value.
*ACCEPT_INJECTED_EVENTS.get_or_init(|| {
std::env::var(ACCEPT_INJECTED_ENV)
.map(|v| v == "1" || v.eq_ignore_ascii_case("true"))
.unwrap_or(false)
})
}- Ensure there is an existing
fn accept_injected_events() -> booldefinition matching theSEARCHblock; if the current implementation differs, adjust theSEARCHpattern accordingly when applying the replacement. - If you already import
OnceLockelsewhere (e.g.,use std::sync::OnceLock;), you can simplify the static type fromstd::sync::OnceLock<bool>to justOnceLock<bool>and rely on the existing import. If you do not, you may adduse std::sync::OnceLock;at the top and update the static accordingly.
Comment on lines
+58
to
+63
| function Send-KeyEdge($Vk, $KeyUp) { | ||
| $flags = [OpenLessInput]::KEYEVENTF_EXTENDEDKEY | ||
| if ($KeyUp) { | ||
| $flags = $flags -bor [OpenLessInput]::KEYEVENTF_KEYUP | ||
| } | ||
| [OpenLessInput]::keybd_event([byte]$Vk, 0x1D, $flags, [UIntPtr]::Zero) |
There was a problem hiding this comment.
issue (bug_risk): Send-KeyEdge uses a fixed scan code that may not match the provided virtual key.
The helper always uses scan code 0x1D (Control) when calling keybd_event, which only matches the default $Vk = 0xA3. If callers pass a different virtual key, the scan code/virtual key pair becomes inconsistent. Consider deriving the scan code from the virtual key (e.g., via MapVirtualKey) or exposing the scan code as a parameter so they always match.
Cooper-X-Oak
force-pushed
the
codex/windows-hotkey-hook-pr
branch
from
6298bb2 to
3a13e56
Compare
Contributor
Author
|
自审结论：通过，作为 fork/dev 已验证批次拆出的最小 upstream 维护项。 核对结果：
|
7 tasks
appergb
pushed a commit
that referenced
this pull request
Apr 30, 2026
包含本轮所有合并： - Codex 终审两条 HIGH (cancel race) 修复 (PR #79) - 6 个 Cooper-X-Oak/Codex bot PRs 自动合并 (#44 #49 #53 #68 #72 #73) - 2 个有冲突 PR 本地 rebase 后合并 (#66 cancel + 空转写并存 / #67 Windows docs) - README 破图修复 (PR #80) - workflow-scope 受限的 #48 + #75 由用户在 GitHub UI 直接合并 3 处版本字段同步：package.json + tauri.conf.json + Cargo.toml
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.
摘要
关联 issue：无。本 PR 是 Windows 热键核心回归门禁，不混入 ASR / 插入 / 权限修复。
本 PR 只解决一个目标：给 Windows 原生低层键盘 hook 增加可自动回归的 OS 触发门禁，避免 modifier-only 全局快捷键退化时只能靠人工按键发现。
fork/dev 先行验证
b9ee8b8。修复 / 新增 / 改进
windows-hotkey-os-hook-smoke.ps1，启动 release exe 后用右 Control 合成事件验证 Windows low-level hook 能到达 Coordinator。OPENLESS_ACCEPT_SYNTHETIC_HOTKEY_EVENTS=1时接受合成事件，供 smoke 脚本使用。兼容
测试计划
openless-all/app/scripts/windows-build-gnu.ps1openless-all/app/scripts/windows-hotkey-os-hook-smoke.ps1Windows low-level hook observed vk=163 and reached Coordinator.%LOCALAPPDATA%\OpenLess\Logs\openless.log