Skip to content

fix(shortcuts): reject conflicting individual resets - #808

Merged
vastsa merged 1 commit into
vastsa:mainfrom
yuxino:codex/fix-shortcut-reset-conflict
Sep 21, 2026
Merged

vastsa merged 1 commit into
vastsa:mainfrom
yuxino:codex/fix-shortcut-reset-conflict

Conversation

@yuxino

@yuxino yuxino commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Disable New task, assign Cmd+N to Search, then restore New task's default: both actions end up on Cmd+N, which opens New task instead of Search.

Individual resets now use the existing conflict check. An occupied default shows an inline error and leaves both mappings unchanged. Ordinary resets and Restore defaults still work.

shortcut-reset-before-after-english-hd.mp4

Validated with a failing-before/passing-after component regression and real desktop interaction. The settings spec and E2E-072 are updated in both languages.

Validation details
  • Candidate: d432ef47d90c9603aee44349a4a05e87a02bf57e; base: 4413e25cb2d9620a570e042d5353960aff9ab696.
  • macOS arm64, Node 24.13.0, Electron 43.6.0.
  • pnpm test:e2e:shortcut-settings: mounted production component, macOS/Windows/Linux binding rules on macOS; disable, reassign, conflicting reset, recovery, and global reset.
  • Shared shortcut tests and related desktop tests passed.
  • pnpm build:js, desktop typecheck, pnpm lint, pnpm docs:check, and pnpm check:pr-base passed.
  • Native before/after recording covers conflict rejection and Search still opening with Cmd+N. Windows/Linux native acceptance was not run. No persisted format or default binding changes.

Check the effective binding before saving any individual shortcut change.
Restoring a default must preserve both mappings when another action uses it.

Cover disable, reassign, reset, recovery and global reset with the mounted
production component, and clarify the existing conflict-safe contract.
@vastsa
vastsa merged commit 51b74c9 into vastsa:main Sep 21, 2026
4 checks passed
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