Skip to content

fix: avoid false hotkey conflicts after platform normalization - #347

Open
CialloKing wants to merge 2 commits into
floatboatai:mainfrom
CialloKing:codex/fix-hotkey-platform-conflicts
Open

CialloKing wants to merge 2 commits into
floatboatai:mainfrom
CialloKing:codex/fix-hotkey-platform-conflicts

Conversation

@CialloKing

@CialloKing CialloKing commented Sep 17, 2026 •

Copy link
Copy Markdown

Summary

Fix false hotkey conflicts when multiple bindings of one command resolve to the same physical shortcut on the current platform.

Motivation

A command with both Mod+K and Ctrl+K currently conflicts with itself on Windows/Linux. On macOS, the same happens with Mod+K and Meta+K. The command is then never dispatched, and conflict diagnostics can repeat the same command ID.

Semantic normalization correctly preserves Mod for portable settings. However, candidate collection also needs to deduplicate bindings after platform resolution, separately for each command.

  • Issue / roadmap: N/A; focused bug fix.
  • OpenSpec: no new capability or API. Restores the existing Mod normalization and conflict arbitration contract in openspec/specs/plugin-commands-events/spec.md.

Changes

  • packages/plugin-runtime: track resolved shortcuts per command in candidatesByHotkey(). Both conflict queries and keyboard dispatch use the corrected candidate set.
  • Extend the existing hotkey tests to cover default/custom bindings across Windows, Linux and macOS, one execution per key event, preservation of portable preferences, and genuine conflicts between different commands with unique diagnostic IDs.

Testing

Local environment: Windows, Node 24.16.0, pnpm 9.15.4.

  • Regression demonstrated before the fix: 5 relevant cases failed, including repeated IDs in conflict diagnostics.
  • pnpm exec vitest run packages/plugin-runtime/test/hotkey-scope.test.ts: 11/11 pass after the fix.
  • pnpm typecheck
  • pnpm check:api
  • pnpm build
  • pnpm build:electron-demo
  • Upstream CI: run 35222521354 reports action_required; no verification jobs have started.
  • Full test suite green: 892/902 pass. The same 10 failures in apps/electron-demo/test/plugin-host-broker.test.ts occur before and after the fix: 9 require Windows symlink privileges; 1 assumes O_NONBLOCK exists. Before the fix, the 5 hotkey regressions also fail (887/902 pass).
  • Electron multi-window smoke: the existing runner fails before launching Electron because its spawnSync("pnpm", ...) bundling step returns a null status on this Windows environment. Electron's binary download also timed out; no native Electron acceptance is claimed.
  • Manual UI / screenshots: N/A; no visual changes. Platform dispatch is verified with DOM keyboard events in Vitest/jsdom, not physical macOS/Linux hardware.

Contribution Notes

  • CLA signed: license/cla passed; CLA Assistant confirms all committers have signed.
  • Functional code is not primarily generated by AI: this statement is intentionally not affirmed. Codex generated the four-line fix and regression tests, performed the investigation, and ran validation. This submission is for a recruitment exercise that permits AI tools; AI involvement is disclosed for maintainer assessment under GOVERNANCE.md section 6.2.
  • No new dependencies.
  • No build artifacts, secrets, environment files or personal vault data in the diff.
  • Conventional Commits PR title and focused editor-runtime scope.
  • Public API / README changes, table widget rules and new-capability proposal: N/A.

@CLAassistant

CLAassistant commented Sep 17, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants