feat(settings): configurable keyboard shortcuts with a Shortcuts section

[omiinaya]

What changed

Adds a new Settings → Shortcuts section that lets you rebind every hotkey the app ships with. The combos live in a single shared registry (src/shared/shortcuts.ts) that seeds Settings.shortcuts and drives both the section's capture rows and the ShortcutsPanel (⌘/) badge rows — so a rebind applies immediately and the reference panel always reflects it.

Rebindable actions: command palette (⌘K), settings (⌘,), shortcuts panel (⌘/), undo/redo, canvas toggles, new terminal/agent, close node, markdown view, find, commit, copy.

Scope of the change (19 files, +858 / −202):

• src/shared/shortcuts.ts (+shortcuts.test.ts) — the registry of defaults and the both-branch cross-platform proof.
• New ShortcutsSection.tsx (+ test). Extracts the Speech section's capture field into a shared control src/renderer/components/settings/ShortcutCaptureField.tsx with allowChord/defaultValue knobs, so dictation (hold-to-talk) and plain hotkeys share one control. Conflict detection flags a combo mapped to two actions inline.
• Wiring: every dispatch site now reads settings.shortcuts through the existing matchesShortcut engine instead of hardcoded key checks — Canvas.tsx (chords), main-process ⌘M/⌘W intercepts in src/main/index.ts, TerminalNode.tsx (⌘F), SourceControlPanel.tsx (commit).
• src/shared/types.ts — Settings.shortcuts field, seeded in DEFAULT_SETTINGS from the registry.
• src/core/settings-store.ts — deep-merges .shortcuts exactly like .speech, so an existing settings.json without the new key keeps the shipped defaults (new actions just appear).
• New section registration: nav.ts, SettingsIcons.tsx, SettingsPage.tsx, nav.test.ts.

The fix / design

The existing shared shortcut engine already abstracts platform variance via an isMac flag — Cmd renders and matches as ⌘/metaKey on macOS and Ctrl/ctrlKey on Windows/Linux, so one stored combo behaves and displays correctly on every OS. This is the same mechanism the already-shipped dictation chord uses. No new OS-specific code paths were added; each dispatch just passes its isMac through the engine.

Dictionary-style conflict detection: if a user binds a combo that already maps to another action, both rows show an inline conflict flag rather than silently overwriting.

Measured

• typecheck: clean on both tsconfig.node.json and tsconfig.web.json.
• Full suite green on the rebased tree: 5079 passed / 12 skipped / 0 failed (the 12 skips are the docker-only SSH tests).
• New tests:
  • shortcuts.test.ts round-trips every shipped default through parse → format → match → capture on both the macOS and Windows/Linux branches (pins punctuation combos like Cmd+,).
  • ShortcutsSection.test.tsx component test (jsdom).
  • test/server/shortcuts-e2e.test.ts — a server-level E2E driving a settings round-trip through the real HTTP + WS-RPC stack, including a legacy settings.json migration case.
• npm run build (electron-vite): ✓.
• Deployed to a running Server Edition and the served bundle manually verified to carry the new section.

Not in scope

• Mouse gestures remain fixed (not hotkeys).
• Dictation stays in its existing Speech section (it was already configurable; this only extracts the shared capture control, preserving its hold-to-talk shape).
• Live runtime verification was performed on Linux only (host + the repo's CI run ubuntu-latest). The both-branch unit test pins the cross-platform logic on macOS and Windows/Linux branches, but true Windows/macOS runtime coverage is left for a future CI matrix — ci.yml currently has no Windows/macOS test job.

Testing

npm run typecheck · npx vitest (5079 passed / 12 skipped / 0 failed) · npm run build, plus a manual pass on a running Server Edition via the browser.

References

• The platform-abstracted chord engine (matchesShortcut / formatShortcut) predates this PR and is the same one the dictation (Speech) setting uses.

[eneskirca]

Thanks for this — the registry shape is good, the capture field correctly mandates a primary modifier, and findShortcutConflicts is real (I broke it to return [] and the tests caught it).

I have to flag one thing up front, because it is the reason this cannot land as-is and it is not visible as a merge conflict.

Blocker: after merging main, the 0 key stops working app-wide

src/main/index.ts merges cleanly, typecheck passes, npm run build passes, and the full suite passes — on a tree where you cannot type 0 anywhere in the app.

PR #193 landed a main-process accelerator intercept whose branch reads:

else if (input.code === 'Digit0' && !input.shift && !input.alt)

That branch is only ⌘0 rather than bare 0 because of the handler's opening early return on main:

if (input.type !== 'keyDown' || !(input.meta || input.control)) return

This PR rewrites that line to if (input.type !== 'keyDown') return (reasonably — matchesShortcut carries its own modifier checks). Git merges your opening line with #193's trailing branch and produces a handler where a plain 0 reaches the Digit0 branch and gets an unconditional event.preventDefault().

Verified by transcribing the merged handler and feeding it a bare 0:

• origin/main → { prevented: false, sent: null }
• merged with this PR → { prevented: true, sent: 'zoomActualSize' } (on macOS and on Linux/Windows)

before-input-event has no focus context, so this applies while typing into a terminal, into Monaco, into any input. Only Shift+0 and Alt+0 survive.

#193's own guard test does not catch it: menu-accelerator-intercepts.test.ts asserts on source strings, and every asserted string is still present, so it passes green on the broken tree.

Also verified broken

• ⌘C copy is dead code. In Canvas.tsx the new } else if (matchesShortcut(e, shortcuts.copySelection, isMac)) { was inserted between the pre-existing ⌘C condition and its body. The old branch is now empty (comment only) and swallows ⌘C, so copy-selection and copy-nodes-as-file-references are unreachable at their default binding. The wiring test asserts the matchesShortcut call is present — and passes, because presence is not reachability.
• ShortcutsPanel.tsx conflict silently un-advertises feat(canvas): ⌘0 zooms to 100%, Shift+1 fits the view #193. Resolving to your side (the only sensible resolution) drops the ⌘0 Zoom to 100% and ⇧1 Fit view rows. The section's own description says "Every hotkey in the app, with the key it is bound to" — which then isn't true, since neither zoom action is in SHORTCUT_DEFS.
• Off macOS, Cmd is Ctrl. Rebinding any main-intercepted action to Cmd+C / Cmd+D / Cmd+A makes the main process preventDefault() that key in every terminal with no focus guard — SIGINT gone app-wide. Recoverable via mouse, so not a hard lock, but the UI gives no warning.

Two tests that pass on broken source

I mutation-tested the suite; these two survived:

1. settings-store.ts deep-merge. Deleting merged.shortcuts = {...} fails nothing — shortcuts-e2e.test.ts seeds a settings file with no shortcuts key, which the plain {...DEFAULT_SETTINGS, ...saved} spread already handles. The line only matters for a partial saved map, and the real failure mode there is parseShortcut(undefined).split throwing on every keystroke. Please add that case.
2. shortcuts-dispatch-wiring.test.ts. Its comment says a closure-captured copy "would serve the OLD combo forever", but it only regex-counts occurrences of the live read. Hoisting one read out of onKey into the effect body keeps every asserted string, keeps the count, passes all 4 tests — and silently breaks live rebinding. Assert the read is inside the handler.

Requested changes

1. Add an explicit input.meta || input.control check to the Digit0 branch in src/main/index.ts — do not rely on an early return this PR removes.
2. Add a behavioural regression test asserting a bare 0 is untouched.
3. Fix the unreachable copySelection branch in Canvas.tsx (delete the old condition, keep the matchesShortcut one).
4. Resolve ShortcutsPanel.tsx without dropping feat(canvas): ⌘0 zooms to 100%, Shift+1 fits the view #193's ⌘0 / ⇧1 rows.
5. Decide whether zoomTo100 / fitAll join the registry, or soften the "every hotkey" copy.
6. Make shortcuts-dispatch-wiring.test.ts pin the read inside the handler.
7. Cover the partial-shortcuts merge case in settings-store.ts.
8. Guard the main-process intercept against terminal control keys (refuse a reserved set, or warn in the capture field).
9. Extend conflict detection past the registry — collisions with settings.speech.shortcut, the zoom chords, and the ⌘1-9 project jump are silently accepted today.
10. CONTRIBUTING.md asks for new chords to be reasoned about across the shells; this touches none of the docs.

The textual conflicts are small (src/shared/types.ts and ShortcutsPanel.tsx, one hunk each). The dangerous file is the one that does not conflict, which is why this needs item 1 called out explicitly rather than just "please rebase".

Happy to re-review once these are in.

[eneskirca]

Following up on item 1 specifically, because it is the one that a rebase makes worse.

Symptom after rebasing onto current main: typing 0 stops working anywhere in the desktop window. The character is swallowed and the canvas snaps to 100% zoom — in terminals, commit messages, settings fields, everywhere. (Server Edition is unaffected: no main process.)

Why. main now carries #193 (⌘0 → zoom 100%). Its main-process intercept, src/main/index.ts:486 on main, deliberately has no modifier test of its own:

} else if (input.code === 'Digit0' && !input.shift && !input.alt) {

because the guard at the top of the same handler (src/main/index.ts:476 on main) already required one:

if (input.type !== 'keyDown' || !(input.meta || input.control)) return

This PR replaces that with if (input.type !== 'keyDown') return (src/main/index.ts:478 on your branch) and moves the modifier decision into each branch's matchesShortcut. That is the right call for the configurable branches — but Digit0 is not configurable and is left holding nothing.

This is invisible as a merge conflict. Merging main into this branch conflicts only in ShortcutsPanel.tsx and src/shared/types.ts; src/main/index.ts auto-merges clean, because the two edits touch different lines. So it will come back on every rebase unless it is handled deliberately — that is the whole reason for this separate comment.

Measured on the merged tree, by lifting the real before-input-event callback out of index.ts and running it against { key: '0', code: 'Digit0' }, no modifiers:

{ prevented: true, sent: ['app:zoom-actual-size'] }

The same test on main alone: 5/5 green.

Fix — one line, the Digit0 branch in src/main/index.ts:

-    } else if (input.code === 'Digit0' && !input.shift && !input.alt) {
+    } else if ((input.meta || input.control) && input.code === 'Digit0' && !input.shift && !input.alt) {

That is zoomShortcutChord's own primary = metaKey || ctrlKey, restored on the branch that lost it. It stays hardcoded on purpose and does not fight this PR's design: the zoom chords are not in SHORTCUT_DEFS, because they match on e.code for AZERTY layouts and the registry does not model that — so there is no binding to respect. Keeping it on one line also keeps #193's existing guard test green.

Our half of this. #193's menu-accelerator-intercepts.test.ts asserts on source strings (MAIN_SRC.toContain(...)). It could not see this — and the tell is that it goes red on the corrected line while it was green on the broken tree. That is on us, not on you. We are landing a behavioural replacement on main that executes the real handler and asserts a bare 0 is not swallowed, so it is in the tree before you rebase and this shows up as a red test instead of a silent regression. Nothing for you to do there.

[eneskirca]

Heads up on a main change that touches this branch: #201 replaces src/main/menu-accelerator-intercepts.test.ts (which asserted on the source text of index.ts) with a behavioural test, and extracts the before-input-event decision into a pure src/main/keydown-intercept.ts. The old test was green on the merged tree even with the bare 0 key being swallowed, because the strings it matched were all still there — the new one presses keys and asserts what happened.

Practically, this means your rebase will show a red test instead of a silent breakage: keydown-intercept.test.ts presses 0, m and w bare and expects them to reach the page, so moving the modifier check out of the shared guard fails loudly unless every branch has one. The one-line fix already discussed for your Digit0 branch is unchanged; the handler body just moves to the new module, and index.ts becomes a single installKeydownIntercepts(win) call, so the rebase there should be smaller than it was.

Sorry for the extra churn — this was our own test debt from #193, not anything wrong with your branch.