From 6667992753c3995fb591356ce8784624c6d4bd05 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sun, 15 Feb 2026 14:20:50 -0800 Subject: [PATCH] Harden keybinding schemas and align tests with server defaults - make resolved keybinding/shortcut schemas strict to reject unknown fields - export server `DEFAULT_KEYBINDINGS` as source of truth and reuse it in ws tests - simplify keybinding test fixtures and update docs to reference server defaults --- KEYBINDINGS.md | 13 +- apps/server/src/keybindings.ts | 2 +- apps/server/src/wsServer.test.ts | 64 ++++----- apps/web/src/keybindings.test.ts | 143 ++++++--------------- packages/contracts/src/keybindings.test.ts | 18 ++- packages/contracts/src/keybindings.ts | 4 +- 6 files changed, 82 insertions(+), 162 deletions(-) diff --git a/KEYBINDINGS.md b/KEYBINDINGS.md index 8128159f1175c15ef235fed11fbce74f84241b6c..9cac38b68b4a0cefd2180642a4269ed1b82860c0 100644 --- a/KEYBINDINGS.md +++ b/KEYBINDINGS.md @@ -21,11 +21,8 @@ The file must be a JSON array of rules: ```json [ - { "key": "mod+j", "command": "terminal.toggle" }, - { "key": "mod+d", "command": "terminal.split", "when": "terminalFocus" }, - { "key": "mod+shift+d", "command": "terminal.new", "when": "terminalFocus" }, - { "key": "mod+shift+o", "command": "chat.new" }, - { "key": "mod+o", "command": "editor.openFavorite" } + { "key": "mod+g", "command": "terminal.toggle" }, + { "key": "mod+shift+g", "command": "terminal.new", "when": "terminalFocus" } ] ``` @@ -90,11 +87,7 @@ Unknown condition keys evaluate to `false`. Built-in defaults are resolved on the server and sent to the web client via `server.getConfig`: -- `terminal.toggle`: `mod+j` -- `terminal.split`: `mod+d` when `terminalFocus` -- `terminal.new`: `mod+shift+d` when `terminalFocus` -- `chat.new`: `mod+shift+o` -- `editor.openFavorite`: `mod+o` +- Source of truth: [`DEFAULT_KEYBINDINGS` in `apps/server/src/keybindings.ts`](apps/server/src/keybindings.ts) If you define any rules for a command, the default rule for that command is removed on the server. diff --git a/apps/server/src/keybindings.ts b/apps/server/src/keybindings.ts index 1c9324e7699b804fd725f0a82b1e47ef746be250..3a92a4fed13922670c15023e95bb00570c8bcaaa 100644 --- a/apps/server/src/keybindings.ts +++ b/apps/server/src/keybindings.ts @@ -24,7 +24,7 @@ type WhenToken = | { type: "lparen" } | { type: "rparen" }; -const DEFAULT_KEYBINDINGS: KeybindingsConfig = [ +export const DEFAULT_KEYBINDINGS: ReadonlyArray = [ { key: "mod+j", command: "terminal.toggle" }, { key: "mod+d", command: "terminal.split", when: "terminalFocus" }, { key: "mod+shift+d", command: "terminal.new", when: "terminalFocus" }, diff --git a/apps/server/src/wsServer.test.ts b/apps/server/src/wsServer.test.ts index d4bc7d993d61d8c965b3f1490b9097acd379d1e5..2d6418fdbf16f8ab1b2d037efa035d8f62cb9bfb 100644 --- a/apps/server/src/wsServer.test.ts +++ b/apps/server/src/wsServer.test.ts @@ -17,7 +17,7 @@ import { type WsResponse, } from "@t3tools/contracts"; import { ProjectRegistry } from "./projectRegistry"; -import { compileResolvedKeybindingRule } from "./keybindings"; +import { compileResolvedKeybindingRule, DEFAULT_KEYBINDINGS } from "./keybindings"; import type { TerminalClearInput, TerminalCloseInput, @@ -207,6 +207,20 @@ function compileKeybindings(bindings: KeybindingsConfig): ResolvedKeybindingsCon return resolved; } +const DEFAULT_RESOLVED_KEYBINDINGS = compileKeybindings([...DEFAULT_KEYBINDINGS]); + +function mergeWithDefaultsForTest(custom: KeybindingsConfig): ResolvedKeybindingsConfig { + if (custom.length === 0) { + return DEFAULT_RESOLVED_KEYBINDINGS; + } + + const overriddenCommands = new Set(custom.map((binding) => binding.command)); + const retainedDefaults = DEFAULT_KEYBINDINGS.filter( + (binding) => !overriddenCommands.has(binding.command), + ); + return compileKeybindings([...retainedDefaults, ...custom].slice(-256)); +} + describe("WebSocket Server", () => { let server: ReturnType | null = null; const connections: WebSocket[] = []; @@ -324,13 +338,7 @@ describe("WebSocket Server", () => { expect(response.error).toBeUndefined(); expect(response.result).toEqual({ cwd: "/my/workspace", - keybindings: compileKeybindings([ - { key: "mod+j", command: "terminal.toggle" }, - { key: "mod+d", command: "terminal.split", when: "terminalFocus" }, - { key: "mod+shift+d", command: "terminal.new", when: "terminalFocus" }, - { key: "mod+shift+o", command: "chat.new" }, - { key: "mod+o", command: "editor.openFavorite" }, - ]), + keybindings: DEFAULT_RESOLVED_KEYBINDINGS, }); }); @@ -363,9 +371,7 @@ describe("WebSocket Server", () => { expect(response.error).toBeUndefined(); expect(response.result).toEqual({ cwd: "/my/workspace", - keybindings: compileKeybindings([ - { key: "mod+shift+o", command: "chat.new" }, - { key: "mod+o", command: "editor.openFavorite" }, + keybindings: mergeWithDefaultsForTest([ { key: "cmd+j", command: "terminal.toggle" }, { key: "mod+d", command: "terminal.split", when: "terminalFocus" }, { key: "mod+shift+d", command: "terminal.new", when: "terminalFocus" }, @@ -402,11 +408,7 @@ describe("WebSocket Server", () => { expect(response.error).toBeUndefined(); expect(response.result).toEqual({ cwd: "/my/workspace", - keybindings: compileKeybindings([ - { key: "mod+d", command: "terminal.split", when: "terminalFocus" }, - { key: "mod+shift+d", command: "terminal.new", when: "terminalFocus" }, - { key: "mod+shift+o", command: "chat.new" }, - { key: "mod+o", command: "editor.openFavorite" }, + keybindings: mergeWithDefaultsForTest([ { key: "mod+j", command: "terminal.toggle" }, ]), }); @@ -443,13 +445,7 @@ describe("WebSocket Server", () => { expect(response.error).toBeUndefined(); expect(response.result).toEqual({ cwd: "/my/workspace", - keybindings: compileKeybindings([ - { key: "mod+j", command: "terminal.toggle" }, - { key: "mod+d", command: "terminal.split", when: "terminalFocus" }, - { key: "mod+shift+d", command: "terminal.new", when: "terminalFocus" }, - { key: "mod+shift+o", command: "chat.new" }, - { key: "mod+o", command: "editor.openFavorite" }, - ]), + keybindings: DEFAULT_RESOLVED_KEYBINDINGS, }); expect( warnSpy.mock.calls.some(([message]) => @@ -483,11 +479,7 @@ describe("WebSocket Server", () => { expect(firstResponse.error).toBeUndefined(); expect(firstResponse.result).toEqual({ cwd: "/my/workspace", - keybindings: compileKeybindings([ - { key: "mod+d", command: "terminal.split", when: "terminalFocus" }, - { key: "mod+shift+d", command: "terminal.new", when: "terminalFocus" }, - { key: "mod+shift+o", command: "chat.new" }, - { key: "mod+o", command: "editor.openFavorite" }, + keybindings: mergeWithDefaultsForTest([ { key: "cmd+j", command: "terminal.toggle" }, ]), }); @@ -529,13 +521,7 @@ describe("WebSocket Server", () => { expect(response.error).toBeUndefined(); expect(response.result).toEqual({ cwd: "/my/workspace", - keybindings: compileKeybindings([ - { key: "mod+j", command: "terminal.toggle" }, - { key: "mod+d", command: "terminal.split", when: "terminalFocus" }, - { key: "mod+shift+d", command: "terminal.new", when: "terminalFocus" }, - { key: "mod+shift+o", command: "chat.new" }, - { key: "mod+o", command: "editor.openFavorite" }, - ]), + keybindings: DEFAULT_RESOLVED_KEYBINDINGS, }); expect( warnSpy.mock.calls.some(([message]) => @@ -566,13 +552,7 @@ describe("WebSocket Server", () => { expect(response.error).toBeUndefined(); expect(response.result).toEqual({ cwd: "/my/workspace", - keybindings: compileKeybindings([ - { key: "mod+j", command: "terminal.toggle" }, - { key: "mod+d", command: "terminal.split", when: "terminalFocus" }, - { key: "mod+shift+d", command: "terminal.new", when: "terminalFocus" }, - { key: "mod+shift+o", command: "chat.new" }, - { key: "mod+o", command: "editor.openFavorite" }, - ]), + keybindings: DEFAULT_RESOLVED_KEYBINDINGS, }); expect( warnSpy.mock.calls.some(([message]) => diff --git a/apps/web/src/keybindings.test.ts b/apps/web/src/keybindings.test.ts index 43f38637c2fe7da4fbf724a6681df580124853ed..622f2dea0783313619f8b0d8ef294572fac840f2 100644 --- a/apps/web/src/keybindings.test.ts +++ b/apps/web/src/keybindings.test.ts @@ -29,75 +29,21 @@ function event(overrides: Partial = {}): ShortcutEventLike { }; } -function parseShortcut(value: string): KeybindingShortcut | null { - const rawTokens = value.toLowerCase().split("+").map((token) => token.trim()); - const tokens = [...rawTokens]; - let trailingEmptyCount = 0; - while (tokens[tokens.length - 1] === "") { - trailingEmptyCount += 1; - tokens.pop(); - } - if (trailingEmptyCount > 0) { - tokens.push("+"); - } - if (tokens.some((token) => token.length === 0)) { - return null; - } - if (tokens.length === 0) return null; - - let key: string | null = null; - let metaKey = false; - let ctrlKey = false; - let shiftKey = false; - let altKey = false; - let modKey = false; - - for (const token of tokens) { - switch (token) { - case "cmd": - case "meta": - metaKey = true; - break; - case "ctrl": - case "control": - ctrlKey = true; - break; - case "shift": - shiftKey = true; - break; - case "alt": - case "option": - altKey = true; - break; - case "mod": - modKey = true; - break; - default: { - if (key !== null) return null; - key = token; - } - } - } - - if (key === null) return null; +function modShortcut( + key: string, + overrides: Partial> = {}, +): KeybindingShortcut { return { key, - metaKey, - ctrlKey, - shiftKey, - altKey, - modKey, + metaKey: false, + ctrlKey: false, + shiftKey: false, + altKey: false, + modKey: true, + ...overrides, }; } -function shortcut(value: string): KeybindingShortcut { - const parsed = parseShortcut(value); - if (!parsed) { - throw new Error(`invalid shortcut in test fixture: ${value}`); - } - return parsed; -} - function whenIdentifier(name: string): KeybindingWhenNode { return { type: "identifier", name }; } @@ -111,48 +57,33 @@ function whenAnd(left: KeybindingWhenNode, right: KeybindingWhenNode): Keybindin } interface TestBinding { - key: string; + shortcut: KeybindingShortcut; command: KeybindingCommand; whenAst?: KeybindingWhenNode; } function compile(bindings: TestBinding[]): ResolvedKeybindingsConfig { - const resolved: ResolvedKeybindingsConfig = []; - for (const binding of bindings) { - const parsedShortcut = parseShortcut(binding.key); - if (!parsedShortcut) { - throw new Error(`invalid shortcut in test fixture: ${binding.key}`); - } - if (binding.whenAst) { - resolved.push({ - command: binding.command, - shortcut: parsedShortcut, - whenAst: binding.whenAst, - }); - continue; - } - resolved.push({ - command: binding.command, - shortcut: parsedShortcut, - }); - } - return resolved; + return bindings.map((binding) => ({ + command: binding.command, + shortcut: binding.shortcut, + ...(binding.whenAst ? { whenAst: binding.whenAst } : {}), + })); } const DEFAULT_BINDINGS = compile([ - { key: "mod+j", command: "terminal.toggle" }, + { shortcut: modShortcut("j"), command: "terminal.toggle" }, { - key: "mod+d", + shortcut: modShortcut("d"), command: "terminal.split", whenAst: whenIdentifier("terminalFocus"), }, { - key: "mod+shift+d", + shortcut: modShortcut("d", { shiftKey: true }), command: "terminal.new", whenAst: whenIdentifier("terminalFocus"), }, - { key: "mod+shift+o", command: "chat.new" }, - { key: "mod+o", command: "editor.openFavorite" }, + { shortcut: modShortcut("o", { shiftKey: true }), command: "chat.new" }, + { shortcut: modShortcut("o"), command: "editor.openFavorite" }, ]); describe("isTerminalToggleShortcut", () => { @@ -203,16 +134,16 @@ describe("split/new terminal shortcuts", () => { it("supports when expressions", () => { const keybindings = compile([ { - key: "mod+\\", + shortcut: modShortcut("\\"), command: "terminal.split", whenAst: whenAnd(whenIdentifier("terminalOpen"), whenNot(whenIdentifier("terminalFocus"))), }, { - key: "mod+shift+n", + shortcut: modShortcut("n", { shiftKey: true }), command: "terminal.new", whenAst: whenAnd(whenIdentifier("terminalOpen"), whenNot(whenIdentifier("terminalFocus"))), }, - { key: "mod+j", command: "terminal.toggle" }, + { shortcut: modShortcut("j"), command: "terminal.toggle" }, ]); assert.isTrue( isTerminalSplitShortcut(event({ key: "\\", ctrlKey: true }), keybindings, { @@ -236,8 +167,8 @@ describe("split/new terminal shortcuts", () => { it("supports when boolean literals", () => { const keybindings = compile([ - { key: "mod+n", command: "terminal.new", whenAst: whenIdentifier("true") }, - { key: "mod+m", command: "terminal.new", whenAst: whenIdentifier("false") }, + { shortcut: modShortcut("n"), command: "terminal.new", whenAst: whenIdentifier("true") }, + { shortcut: modShortcut("m"), command: "terminal.new", whenAst: whenIdentifier("false") }, ]); assert.isTrue( @@ -257,12 +188,12 @@ describe("shortcutLabelForCommand", () => { it("returns the most recent binding label", () => { const bindings = compile([ { - key: "mod+\\", + shortcut: modShortcut("\\"), command: "terminal.split", whenAst: whenIdentifier("terminalFocus"), }, { - key: "mod+shift+\\", + shortcut: modShortcut("\\", { shiftKey: true }), command: "terminal.split", whenAst: whenNot(whenIdentifier("terminalFocus")), }, @@ -310,9 +241,9 @@ describe("chat/editor shortcuts", () => { describe("cross-command precedence", () => { it("uses when + order so a later focused rule overrides a global rule", () => { const keybindings = compile([ - { key: "mod+n", command: "chat.new" }, + { shortcut: modShortcut("n"), command: "chat.new" }, { - key: "mod+n", + shortcut: modShortcut("n"), command: "terminal.new", whenAst: whenIdentifier("terminalFocus"), }, @@ -347,11 +278,11 @@ describe("cross-command precedence", () => { it("still lets a later global rule win when both rules match", () => { const keybindings = compile([ { - key: "mod+n", + shortcut: modShortcut("n"), command: "terminal.new", whenAst: whenIdentifier("terminalFocus"), }, - { key: "mod+n", command: "chat.new" }, + { shortcut: modShortcut("n"), command: "chat.new" }, ]); assert.isFalse( @@ -371,16 +302,16 @@ describe("cross-command precedence", () => { describe("formatShortcutLabel", () => { it("formats labels for macOS", () => { - assert.strictEqual(formatShortcutLabel(shortcut("mod+shift+d"), "MacIntel"), "⇧⌘D"); + assert.strictEqual(formatShortcutLabel(modShortcut("d", { shiftKey: true }), "MacIntel"), "⇧⌘D"); }); it("formats labels for non-macOS", () => { - assert.strictEqual(formatShortcutLabel(shortcut("mod+shift+d"), "Linux"), "Ctrl+Shift+D"); + assert.strictEqual(formatShortcutLabel(modShortcut("d", { shiftKey: true }), "Linux"), "Ctrl+Shift+D"); }); it("formats labels for plus key", () => { - assert.strictEqual(formatShortcutLabel(shortcut("mod++"), "MacIntel"), "⌘+"); - assert.strictEqual(formatShortcutLabel(shortcut("mod++"), "Linux"), "Ctrl++"); + assert.strictEqual(formatShortcutLabel(modShortcut("+"), "MacIntel"), "⌘+"); + assert.strictEqual(formatShortcutLabel(modShortcut("+"), "Linux"), "Ctrl++"); }); }); @@ -397,7 +328,7 @@ describe("isTerminalClearShortcut", () => { describe("plus key parsing", () => { it("matches the plus key shortcut", () => { - const plusBindings = compile([{ key: "mod++", command: "terminal.toggle" }]); + const plusBindings = compile([{ shortcut: modShortcut("+"), command: "terminal.toggle" }]); assert.isTrue( isTerminalToggleShortcut(event({ key: "+", metaKey: true }), plusBindings, { platform: "MacIntel", diff --git a/packages/contracts/src/keybindings.test.ts b/packages/contracts/src/keybindings.test.ts index 3261d63408ac3b0982dcab215f573541c67e8f0d..5e49945268216181de3f1b1fe4bcd868afb754cb 100644 --- a/packages/contracts/src/keybindings.test.ts +++ b/packages/contracts/src/keybindings.test.ts @@ -61,7 +61,6 @@ describe("keybindings contracts", () => { it("parses resolved keybindings arrays", () => { const parsed = resolvedKeybindingsConfigSchema.parse([ { - key: "mod+j", command: "terminal.toggle", shortcut: { key: "j", @@ -75,4 +74,21 @@ describe("keybindings contracts", () => { ]); assert.lengthOf(parsed, 1); }); + + it("rejects unknown fields in resolved keybinding rules", () => { + assert.throws(() => + resolvedKeybindingRuleSchema.parse({ + command: "terminal.toggle", + shortcut: { + key: "j", + metaKey: false, + ctrlKey: false, + shiftKey: false, + altKey: false, + modKey: true, + }, + key: "mod+j", + }), + ); + }); }); diff --git a/packages/contracts/src/keybindings.ts b/packages/contracts/src/keybindings.ts index a17c695bc661e236ab0c3c609f0e5a298a8bb54b..921fcbd1dd2bf6921117bebb42c18b33531ef7f0 100644 --- a/packages/contracts/src/keybindings.ts +++ b/packages/contracts/src/keybindings.ts @@ -26,7 +26,7 @@ export const keybindingShortcutSchema = z.object({ shiftKey: z.boolean(), altKey: z.boolean(), modKey: z.boolean(), -}); +}).strict(); export type KeybindingWhenNode = | { type: "identifier"; name: string } @@ -61,7 +61,7 @@ export const resolvedKeybindingRuleSchema = z.object({ command: keybindingCommandSchema, shortcut: keybindingShortcutSchema, whenAst: keybindingWhenNodeSchema.optional(), -}); +}).strict(); export const resolvedKeybindingsConfigSchema = z.array(resolvedKeybindingRuleSchema).max(256); -- 2.54.0