From f1cd76c17af2fa9496729ef08056ae69e1f27742 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sun, 15 Feb 2026 11:30:27 -0800 Subject: [PATCH] Resolve terminal keybindings on server before sending config - Move default keybinding resolution/override merging into `server.getConfig` - Return server-resolved keybindings to the web client instead of resolving in UI - Update docs and tests to reflect server-side defaults, overrides, and truncation behavior --- KEYBINDINGS.md | 13 +++- apps/server/src/wsServer.test.ts | 24 ++++++-- apps/server/src/wsServer.ts | 53 ++++++++++++---- apps/web/src/components/ChatView.tsx | 11 ++-- apps/web/src/keybindings.test.ts | 90 ++++++++-------------------- apps/web/src/keybindings.ts | 51 ++-------------- 6 files changed, 108 insertions(+), 134 deletions(-) diff --git a/KEYBINDINGS.md b/KEYBINDINGS.md index ae319a08c0d423abf1a1da23f5d881f68e3bd3ed..bec5d27dc22a301e0ef54c8c1c601fb71dae21d1 100644 --- a/KEYBINDINGS.md +++ b/KEYBINDINGS.md @@ -4,6 +4,15 @@ T3 Code reads keybindings from: - `~/.t3/keybindings.json` +Schema source of truth: + +- `packages/contracts/src/keybindings.ts` +- `packages/contracts/src/server.ts` + +Server-side default resolution/merging: + +- `apps/server/src/wsServer.ts` + The file must be a JSON array of rules: ```json @@ -71,13 +80,13 @@ Unknown condition keys evaluate to `false`. ## Defaults -Built-in defaults: +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` -If you define any rules for a command, the default rule for that command is removed. +If you define any rules for a command, the default rule for that command is removed on the server. ## Precedence diff --git a/apps/server/src/wsServer.test.ts b/apps/server/src/wsServer.test.ts index 288de25288a889a92e1e840915f58af25a29de09..0b4726c51b59bdb6472403d4abcac3ce1ecb0bfa 100644 --- a/apps/server/src/wsServer.test.ts +++ b/apps/server/src/wsServer.test.ts @@ -309,7 +309,11 @@ describe("WebSocket Server", () => { expect(response.error).toBeUndefined(); expect(response.result).toEqual({ cwd: "/my/workspace", - keybindings: [], + keybindings: [ + { key: "mod+j", command: "terminal.toggle" }, + { key: "mod+d", command: "terminal.split", when: "terminalFocus" }, + { key: "mod+shift+d", command: "terminal.new", when: "terminalFocus" }, + ], }); }); @@ -379,7 +383,11 @@ describe("WebSocket Server", () => { expect(response.error).toBeUndefined(); expect(response.result).toEqual({ cwd: "/my/workspace", - keybindings: [{ key: "mod+j", command: "terminal.toggle" }], + keybindings: [ + { key: "mod+d", command: "terminal.split", when: "terminalFocus" }, + { key: "mod+shift+d", command: "terminal.new", when: "terminalFocus" }, + { key: "mod+j", command: "terminal.toggle" }, + ], }); expect( warnSpy.mock.calls.some(([message]) => @@ -414,7 +422,11 @@ describe("WebSocket Server", () => { expect(response.error).toBeUndefined(); expect(response.result).toEqual({ cwd: "/my/workspace", - keybindings: [], + keybindings: [ + { key: "mod+j", command: "terminal.toggle" }, + { key: "mod+d", command: "terminal.split", when: "terminalFocus" }, + { key: "mod+shift+d", command: "terminal.new", when: "terminalFocus" }, + ], }); expect( warnSpy.mock.calls.some(([message]) => @@ -445,7 +457,11 @@ describe("WebSocket Server", () => { expect(response.error).toBeUndefined(); expect(response.result).toEqual({ cwd: "/my/workspace", - keybindings: [], + keybindings: [ + { key: "mod+j", command: "terminal.toggle" }, + { key: "mod+d", command: "terminal.split", when: "terminalFocus" }, + { key: "mod+shift+d", command: "terminal.new", when: "terminalFocus" }, + ], }); expect( warnSpy.mock.calls.some(([message]) => diff --git a/apps/server/src/wsServer.ts b/apps/server/src/wsServer.ts index ddd4d53e765e4ce8aece7c43b3177796443d0172..640b3af76c5b5fda0e5c2725946eb94e04f33bff 100644 --- a/apps/server/src/wsServer.ts +++ b/apps/server/src/wsServer.ts @@ -70,6 +70,31 @@ function parseBooleanEnv(value: string | undefined): boolean | undefined { return undefined; } +const DEFAULT_KEYBINDINGS: KeybindingsConfig = [ + { key: "mod+j", command: "terminal.toggle" }, + { key: "mod+d", command: "terminal.split", when: "terminalFocus" }, + { key: "mod+shift+d", command: "terminal.new", when: "terminalFocus" }, +]; + +function mergeWithDefaultKeybindings(custom: KeybindingsConfig): KeybindingsConfig { + if (custom.length === 0) { + return [...DEFAULT_KEYBINDINGS]; + } + + const overriddenCommands = new Set(custom.map((binding) => binding.command)); + const retainedDefaults = DEFAULT_KEYBINDINGS.filter( + (binding) => !overriddenCommands.has(binding.command), + ); + const merged = [...retainedDefaults, ...custom]; + + if (merged.length <= 256) { + return merged; + } + + // Keep the latest rules when the config exceeds max size; later rules have higher precedence. + return merged.slice(-256); +} + function readKeybindingsConfig(logger: ReturnType): KeybindingsConfig { const configPath = path.join(os.homedir(), ".t3", "keybindings.json"); try { @@ -79,7 +104,7 @@ function readKeybindingsConfig(logger: ReturnType): Keybind logger.warn("ignoring keybindings config with unsupported format; expected array", { path: configPath, }); - return []; + return [...DEFAULT_KEYBINDINGS]; } const sanitized: KeybindingsConfig = []; @@ -88,13 +113,6 @@ function readKeybindingsConfig(logger: ReturnType): Keybind const result = keybindingRuleSchema.safeParse(entry); if (result.success) { sanitized.push(result.data); - if (sanitized.length >= 256) { - logger.warn("truncating keybindings config to max entries", { - path: configPath, - maxEntries: 256, - }); - break; - } continue; } invalidEntries += 1; @@ -106,17 +124,30 @@ function readKeybindingsConfig(logger: ReturnType): Keybind totalEntries: parsed.length, }); } - return sanitized; + const mergedBeforeCap = [ + ...DEFAULT_KEYBINDINGS.filter( + (binding) => !new Set(sanitized.map((entry) => entry.command)).has(binding.command), + ), + ...sanitized, + ]; + const merged = mergeWithDefaultKeybindings(sanitized); + if (mergedBeforeCap.length > 256) { + logger.warn("truncating merged keybindings config to max entries", { + path: configPath, + maxEntries: 256, + }); + } + return merged; } catch (error) { if ((error as NodeJS.ErrnoException)?.code === "ENOENT") { - return []; + return [...DEFAULT_KEYBINDINGS]; } logger.warn("ignoring malformed keybindings config", { path: configPath, error: error instanceof Error ? error.message : String(error), }); } - return []; + return [...DEFAULT_KEYBINDINGS]; } export function createServer(options: ServerOptions) { diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index 35b99b011f143bd666da24d0e2c770f583681efd..30a039048733d95fd49f0991b5808c2636d22778 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -52,11 +52,10 @@ import type { ChatImageAttachment } from "../types"; import BranchToolbar from "./BranchToolbar"; import GitActionsControl from "./GitActionsControl"; import { - DEFAULT_TERMINAL_KEYBINDINGS, isTerminalNewShortcut, isTerminalSplitShortcut, isTerminalToggleShortcut, - resolveTerminalKeybindings, + type ResolvedTerminalKeybindings, shortcutLabelForCommand, } from "../keybindings"; import ChatMarkdown from "./ChatMarkdown"; @@ -152,7 +151,7 @@ export default function ChatView() { const [expandedWorkGroups, setExpandedWorkGroups] = useState>({}); const [nowTick, setNowTick] = useState(() => Date.now()); const [terminalFocusRequestId, setTerminalFocusRequestId] = useState(0); - const [terminalKeybindings, setTerminalKeybindings] = useState(DEFAULT_TERMINAL_KEYBINDINGS); + const [terminalKeybindings, setTerminalKeybindings] = useState([]); const messagesScrollRef = useRef(null); const messagesEndRef = useRef(null); const shouldAutoScrollRef = useRef(true); @@ -282,7 +281,7 @@ export default function ChatView() { useEffect(() => { if (!api || !api.server || typeof api.server.getConfig !== "function") { - setTerminalKeybindings(DEFAULT_TERMINAL_KEYBINDINGS); + setTerminalKeybindings([]); return; } @@ -292,11 +291,11 @@ export default function ChatView() { .getConfig() .then((config) => { if (disposed) return; - setTerminalKeybindings(resolveTerminalKeybindings(config.keybindings)); + setTerminalKeybindings(config.keybindings); }) .catch(() => { if (disposed) return; - setTerminalKeybindings(DEFAULT_TERMINAL_KEYBINDINGS); + setTerminalKeybindings([]); }); return () => { diff --git a/apps/web/src/keybindings.test.ts b/apps/web/src/keybindings.test.ts index f71cb8f09b42a4c203bf962cf37518bbd3fada2e..e6d42778c1428441d8b170112309c52141c83fe3 100644 --- a/apps/web/src/keybindings.test.ts +++ b/apps/web/src/keybindings.test.ts @@ -1,13 +1,12 @@ import { describe, expect, it } from "vitest"; +import type { KeybindingsConfig } from "@t3tools/contracts"; import { - DEFAULT_TERMINAL_KEYBINDINGS, formatShortcutLabel, isTerminalClearShortcut, isTerminalNewShortcut, isTerminalSplitShortcut, isTerminalToggleShortcut, - resolveTerminalKeybindings, shortcutLabelForCommand, type ShortcutEventLike, } from "./keybindings"; @@ -23,65 +22,55 @@ function event(overrides: Partial = {}): ShortcutEventLike { }; } +const DEFAULT_BINDINGS: KeybindingsConfig = [ + { key: "mod+j", command: "terminal.toggle" }, + { key: "mod+d", command: "terminal.split", when: "terminalFocus" }, + { key: "mod+shift+d", command: "terminal.new", when: "terminalFocus" }, +]; + describe("isTerminalToggleShortcut", () => { it("matches Cmd+J on macOS", () => { - expect( - isTerminalToggleShortcut(event({ metaKey: true }), DEFAULT_TERMINAL_KEYBINDINGS, { - platform: "MacIntel", - }), - ).toBe(true); + expect(isTerminalToggleShortcut(event({ metaKey: true }), DEFAULT_BINDINGS, { platform: "MacIntel" })).toBe(true); }); it("matches Ctrl+J on non-macOS", () => { - expect( - isTerminalToggleShortcut(event({ ctrlKey: true }), DEFAULT_TERMINAL_KEYBINDINGS, { - platform: "Win32", - }), - ).toBe(true); + expect(isTerminalToggleShortcut(event({ ctrlKey: true }), DEFAULT_BINDINGS, { platform: "Win32" })).toBe(true); }); }); describe("split/new terminal shortcuts", () => { it("requires terminalFocus for default split/new bindings", () => { expect( - isTerminalSplitShortcut(event({ key: "d", metaKey: true }), DEFAULT_TERMINAL_KEYBINDINGS, { + isTerminalSplitShortcut(event({ key: "d", metaKey: true }), DEFAULT_BINDINGS, { platform: "MacIntel", context: { terminalFocus: false }, }), ).toBe(false); expect( - isTerminalNewShortcut( - event({ key: "d", ctrlKey: true, shiftKey: true }), - DEFAULT_TERMINAL_KEYBINDINGS, - { - platform: "Linux", - context: { terminalFocus: false }, - }, - ), + isTerminalNewShortcut(event({ key: "d", ctrlKey: true, shiftKey: true }), DEFAULT_BINDINGS, { + platform: "Linux", + context: { terminalFocus: false }, + }), ).toBe(false); }); it("matches split/new when terminalFocus is true", () => { expect( - isTerminalSplitShortcut(event({ key: "d", metaKey: true }), DEFAULT_TERMINAL_KEYBINDINGS, { + isTerminalSplitShortcut(event({ key: "d", metaKey: true }), DEFAULT_BINDINGS, { platform: "MacIntel", context: { terminalFocus: true }, }), ).toBe(true); expect( - isTerminalNewShortcut( - event({ key: "d", ctrlKey: true, shiftKey: true }), - DEFAULT_TERMINAL_KEYBINDINGS, - { - platform: "Linux", - context: { terminalFocus: true }, - }, - ), + isTerminalNewShortcut(event({ key: "d", ctrlKey: true, shiftKey: true }), DEFAULT_BINDINGS, { + platform: "Linux", + context: { terminalFocus: true }, + }), ).toBe(true); }); - it("supports configured overrides with when expressions", () => { - const keybindings = resolveTerminalKeybindings([ + it("supports when expressions", () => { + const keybindings: KeybindingsConfig = [ { key: "mod+\\", command: "terminal.split", @@ -92,7 +81,8 @@ describe("split/new terminal shortcuts", () => { command: "terminal.new", when: "terminalOpen && !terminalFocus", }, - ]); + { key: "mod+j", command: "terminal.toggle" }, + ]; expect( isTerminalSplitShortcut(event({ key: "\\", ctrlKey: true }), keybindings, { platform: "Win32", @@ -114,41 +104,13 @@ describe("split/new terminal shortcuts", () => { }); }); -describe("resolveTerminalKeybindings", () => { - it("uses defaults when config is empty", () => { - expect(resolveTerminalKeybindings(undefined)).toEqual(DEFAULT_TERMINAL_KEYBINDINGS); - }); - - it("replaces defaults for commands configured by user", () => { - const resolved = resolveTerminalKeybindings([ - { key: "mod+\\", command: "terminal.split", when: "terminalFocus" }, - ]); - - expect(resolved).toEqual([ - { key: "mod+j", command: "terminal.toggle" }, - { key: "mod+shift+d", command: "terminal.new", when: "terminalFocus" }, - { key: "mod+\\", command: "terminal.split", when: "terminalFocus" }, - ]); - }); - - it("ignores invalid configured values", () => { - expect( - resolveTerminalKeybindings([ - { key: " ", command: "terminal.toggle" }, - { key: "mod+d+f", command: "terminal.split" }, - ]), - ).toEqual(DEFAULT_TERMINAL_KEYBINDINGS); - }); -}); - describe("shortcutLabelForCommand", () => { it("returns the most recent binding label", () => { - const resolved = resolveTerminalKeybindings([ + const bindings: KeybindingsConfig = [ { key: "mod+\\", command: "terminal.split", when: "terminalFocus" }, { key: "mod+shift+\\", command: "terminal.split", when: "!terminalFocus" }, - ]); - - expect(shortcutLabelForCommand(resolved, "terminal.split", "Linux")).toBe("Ctrl+Shift+\\"); + ]; + expect(shortcutLabelForCommand(bindings, "terminal.split", "Linux")).toBe("Ctrl+Shift+\\"); }); }); diff --git a/apps/web/src/keybindings.ts b/apps/web/src/keybindings.ts index 1a6d7b4886abfb1e036418a2251dab8347146b50..cea96b5a4ecb7b0b10d327be9f0f8c0ed2ca40ef 100644 --- a/apps/web/src/keybindings.ts +++ b/apps/web/src/keybindings.ts @@ -1,4 +1,4 @@ -import type { KeybindingCommand, KeybindingRule, KeybindingsConfig } from "@t3tools/contracts"; +import type { KeybindingCommand, KeybindingsConfig } from "@t3tools/contracts"; export interface ShortcutEventLike { key: string; @@ -14,15 +14,8 @@ export interface ShortcutMatchContext { [key: string]: boolean; } -export type ResolvedTerminalKeybinding = KeybindingRule; export type ResolvedTerminalKeybindings = KeybindingsConfig; -export const DEFAULT_TERMINAL_KEYBINDINGS: ResolvedTerminalKeybindings = [ - { key: "mod+j", command: "terminal.toggle" }, - { key: "mod+d", command: "terminal.split", when: "terminalFocus" }, - { key: "mod+shift+d", command: "terminal.new", when: "terminalFocus" }, -]; - interface ParsedShortcut { key: string; metaKey: boolean; @@ -306,21 +299,6 @@ function resolveContext(options: ShortcutMatchOptions | undefined): ShortcutMatc }; } -function normalizeConfiguredKeybinding( - keybinding: KeybindingRule, -): ResolvedTerminalKeybinding | null { - const key = keybinding.key.trim(); - if (key.length === 0) return null; - if (!parseShortcutValue(key)) return null; - - const when = keybinding.when?.trim(); - return { - key, - command: keybinding.command, - ...(when ? { when } : {}), - }; -} - function matchesCommandShortcut( event: ShortcutEventLike, keybindings: ResolvedTerminalKeybindings, @@ -340,27 +318,6 @@ function matchesCommandShortcut( return false; } -export function resolveTerminalKeybindings( - input: KeybindingsConfig | null | undefined, -): ResolvedTerminalKeybindings { - if (!input || input.length === 0) { - return [...DEFAULT_TERMINAL_KEYBINDINGS]; - } - - const normalizedInput = input - .map((binding) => normalizeConfiguredKeybinding(binding)) - .filter((binding): binding is ResolvedTerminalKeybinding => binding !== null); - if (normalizedInput.length === 0) { - return [...DEFAULT_TERMINAL_KEYBINDINGS]; - } - - const overriddenCommands = new Set(normalizedInput.map((binding) => binding.command)); - const retainedDefaults = DEFAULT_TERMINAL_KEYBINDINGS.filter( - (binding) => !overriddenCommands.has(binding.command), - ); - return [...retainedDefaults, ...normalizedInput]; -} - function formatShortcutKeyLabel(key: string): string { if (key === " ") return "Space"; if (key.length === 1) return key.toUpperCase(); @@ -413,7 +370,7 @@ export function shortcutLabelForCommand( export function isTerminalToggleShortcut( event: ShortcutEventLike, - keybindings: ResolvedTerminalKeybindings = DEFAULT_TERMINAL_KEYBINDINGS, + keybindings: ResolvedTerminalKeybindings, options?: ShortcutMatchOptions, ): boolean { return matchesCommandShortcut(event, keybindings, "terminal.toggle", options); @@ -421,7 +378,7 @@ export function isTerminalToggleShortcut( export function isTerminalSplitShortcut( event: ShortcutEventLike, - keybindings: ResolvedTerminalKeybindings = DEFAULT_TERMINAL_KEYBINDINGS, + keybindings: ResolvedTerminalKeybindings, options?: ShortcutMatchOptions, ): boolean { return matchesCommandShortcut(event, keybindings, "terminal.split", options); @@ -429,7 +386,7 @@ export function isTerminalSplitShortcut( export function isTerminalNewShortcut( event: ShortcutEventLike, - keybindings: ResolvedTerminalKeybindings = DEFAULT_TERMINAL_KEYBINDINGS, + keybindings: ResolvedTerminalKeybindings, options?: ShortcutMatchOptions, ): boolean { return matchesCommandShortcut(event, keybindings, "terminal.new", options); -- 2.54.0