fix(keybindings): migrate to namespaced ids closes #2391

packages/coding-agent/src/core/extensions/index.ts

@@ -24,9 +24,9 @@ export type {
 	// Re-exports
 	AgentToolResult,
 	AgentToolUpdateCallback,
-	// App keybindings (for custom editors)
-	AppAction,
 	AppendEntryHandler,
+	// App keybindings (for custom editors)
+	AppKeybinding,
 	// Events - Tool (ToolCallEvent types)
 	BashToolCallEvent,
 	BashToolResultEvent,

packages/coding-agent/src/core/extensions/runner.ts

@@ -7,7 +7,7 @@ import type { ImageContent, Model } from "@mariozechner/pi-ai";
 import type { KeyId } from "@mariozechner/pi-tui";
 import { type Theme, theme } from "../../modes/interactive/theme/theme.js";
 import type { ResourceDiagnostic } from "../diagnostics.js";
-import type { KeyAction, KeybindingsConfig } from "../keybindings.js";
+import type { KeybindingsConfig } from "../keybindings.js";
 import type { ModelRegistry } from "../model-registry.js";
 import type { SessionManager } from "../session-manager.js";
 import type {
@@ -51,40 +51,41 @@ import type {
 	UserBashEventResult,
 } from "./types.js";
 
-// Keybindings for these actions cannot be overridden by extensions
-const RESERVED_ACTIONS_FOR_EXTENSION_CONFLICTS: ReadonlyArray<KeyAction> = [
-	"interrupt",
-	"clear",
-	"exit",
-	"suspend",
-	"cycleThinkingLevel",
-	"cycleModelForward",
-	"cycleModelBackward",
-	"selectModel",
-	"expandTools",
-	"toggleThinking",
-	"externalEditor",
-	"followUp",
-	"submit",
-	"selectConfirm",
-	"selectCancel",
-	"copy",
-	"deleteToLineEnd",
-];
+// Extension shortcuts compete with canonical keybinding ids from keybindings.json.
+// Only editor-global shortcuts are reserved here. Picker-specific bindings are not.
+const RESERVED_KEYBINDINGS_FOR_EXTENSION_CONFLICTS = [
+	"app.interrupt",
+	"app.clear",
+	"app.exit",
+	"app.suspend",
+	"app.thinking.cycle",
+	"app.model.cycleForward",
+	"app.model.cycleBackward",
+	"app.model.select",
+	"app.tools.expand",
+	"app.thinking.toggle",
+	"app.editor.external",
+	"app.message.followUp",
+	"tui.input.submit",
+	"tui.select.confirm",
+	"tui.select.cancel",
+	"tui.input.copy",
+	"tui.editor.deleteToLineEnd",
+] as const;
 
-type BuiltInKeyBindings = Partial<Record<KeyId, { action: KeyAction; restrictOverride: boolean }>>;
+type BuiltInKeyBindings = Partial<Record<KeyId, { keybinding: string; restrictOverride: boolean }>>;
 
-const buildBuiltinKeybindings = (effectiveKeybindings: Required<KeybindingsConfig>): BuiltInKeyBindings => {
+const buildBuiltinKeybindings = (resolvedKeybindings: KeybindingsConfig): BuiltInKeyBindings => {
 	const builtinKeybindings = {} as BuiltInKeyBindings;
-	for (const [action, keys] of Object.entries(effectiveKeybindings)) {
-		const keyAction = action as KeyAction;
+	for (const [keybinding, keys] of Object.entries(resolvedKeybindings)) {
+		if (keys === undefined) continue;
 		const keyList = Array.isArray(keys) ? keys : [keys];
-		const restrictOverride = RESERVED_ACTIONS_FOR_EXTENSION_CONFLICTS.includes(keyAction);
+		const restrictOverride = (RESERVED_KEYBINDINGS_FOR_EXTENSION_CONFLICTS as readonly string[]).includes(keybinding);
 		for (const key of keyList) {
 			const normalizedKey = key.toLowerCase() as KeyId;
 			builtinKeybindings[normalizedKey] = {
-				action: keyAction,
-				restrictOverride: restrictOverride,
+				keybinding,
+				restrictOverride,
 			};
 		}
 	}
@@ -386,9 +387,9 @@ export class ExtensionRunner {
 		return new Map(this.runtime.flagValues);
 	}
 
-	getShortcuts(effectiveKeybindings: Required<KeybindingsConfig>): Map<KeyId, ExtensionShortcut> {
+	getShortcuts(resolvedKeybindings: KeybindingsConfig): Map<KeyId, ExtensionShortcut> {
 		this.shortcutDiagnostics = [];
-		const builtinKeybindings = buildBuiltinKeybindings(effectiveKeybindings);
+		const builtinKeybindings = buildBuiltinKeybindings(resolvedKeybindings);
 		const extensionShortcuts = new Map<KeyId, ExtensionShortcut>();
 
 		const addDiagnostic = (message: string, extensionPath: string) => {
@@ -413,7 +414,7 @@ export class ExtensionRunner {
 
 				if (builtInKeybinding?.restrictOverride === false) {
 					addDiagnostic(
-						`Extension shortcut conflict: '${key}' is built-in shortcut for ${builtInKeybinding.action} and ${shortcut.extensionPath}. Using ${shortcut.extensionPath}.`,
+						`Extension shortcut conflict: '${key}' is built-in shortcut for ${builtInKeybinding.keybinding} and ${shortcut.extensionPath}. Using ${shortcut.extensionPath}.`,
 						shortcut.extensionPath,
 					);
 				}

packages/coding-agent/src/core/extensions/types.ts

@@ -74,7 +74,7 @@ import type {
 
 export type { ExecOptions, ExecResult } from "../exec.js";
 export type { AgentToolResult, AgentToolUpdateCallback };
-export type { AppAction, KeybindingsManager } from "../keybindings.js";
+export type { AppKeybinding, KeybindingsManager } from "../keybindings.js";
 
 // ============================================================================
 // UI Context

packages/coding-agent/src/core/keybindings.ts

@@ -1,220 +1,302 @@
 import {
-	DEFAULT_EDITOR_KEYBINDINGS,
-	type EditorAction,
-	type EditorKeybindingsConfig,
-	EditorKeybindingsManager,
+	type Keybinding,
+	type KeybindingDefinitions,
+	type KeybindingsConfig,
 	type KeyId,
-	matchesKey,
-	setEditorKeybindings,
+	TUI_KEYBINDINGS,
+	KeybindingsManager as TuiKeybindingsManager,
 } from "@mariozechner/pi-tui";
-import { existsSync, readFileSync } from "fs";
+import { existsSync, readFileSync, writeFileSync } from "fs";
 import { join } from "path";
 import { getAgentDir } from "../config.js";
 
-/**
- * Application-level actions (coding agent specific).
- */
-export type AppAction =
-	| "interrupt"
-	| "clear"
-	| "exit"
-	| "suspend"
-	| "cycleThinkingLevel"
-	| "cycleModelForward"
-	| "cycleModelBackward"
-	| "selectModel"
-	| "expandTools"
-	| "toggleThinking"
-	| "toggleSessionNamedFilter"
-	| "externalEditor"
-	| "followUp"
-	| "dequeue"
-	| "pasteImage"
-	| "newSession"
-	| "tree"
-	| "fork"
-	| "resume";
-
-/**
- * All configurable actions.
- */
-export type KeyAction = AppAction | EditorAction;
-
-/**
- * Full keybindings configuration (app + editor actions).
- */
-export type KeybindingsConfig = {
-	[K in KeyAction]?: KeyId | KeyId[];
-};
-
-/**
- * Default application keybindings.
- */
-export const DEFAULT_APP_KEYBINDINGS: Record<AppAction, KeyId | KeyId[]> = {
-	interrupt: "escape",
-	clear: "ctrl+c",
-	exit: "ctrl+d",
-	suspend: "ctrl+z",
-	cycleThinkingLevel: "shift+tab",
-	cycleModelForward: "ctrl+p",
-	cycleModelBackward: "shift+ctrl+p",
-	selectModel: "ctrl+l",
-	expandTools: "ctrl+o",
-	toggleThinking: "ctrl+t",
-	toggleSessionNamedFilter: "ctrl+n",
-	externalEditor: "ctrl+g",
-	followUp: "alt+enter",
-	dequeue: "alt+up",
-	pasteImage: process.platform === "win32" ? "alt+v" : "ctrl+v",
-	newSession: [],
-	tree: [],
-	fork: [],
-	resume: [],
-};
-
-/**
- * All default keybindings (app + editor).
- */
-export const DEFAULT_KEYBINDINGS: Required<KeybindingsConfig> = {
-	...DEFAULT_EDITOR_KEYBINDINGS,
-	...DEFAULT_APP_KEYBINDINGS,
-};
-
-// App actions list for type checking
-const APP_ACTIONS: AppAction[] = [
-	"interrupt",
-	"clear",
-	"exit",
-	"suspend",
-	"cycleThinkingLevel",
-	"cycleModelForward",
-	"cycleModelBackward",
-	"selectModel",
-	"expandTools",
-	"toggleThinking",
-	"toggleSessionNamedFilter",
-	"externalEditor",
-	"followUp",
-	"dequeue",
-	"pasteImage",
-	"newSession",
-	"tree",
-	"fork",
-	"resume",
-];
-
-function isAppAction(action: string): action is AppAction {
-	return APP_ACTIONS.includes(action as AppAction);
+export interface AppKeybindings {
+	"app.interrupt": true;
+	"app.clear": true;
+	"app.exit": true;
+	"app.suspend": true;
+	"app.thinking.cycle": true;
+	"app.model.cycleForward": true;
+	"app.model.cycleBackward": true;
+	"app.model.select": true;
+	"app.tools.expand": true;
+	"app.thinking.toggle": true;
+	"app.session.toggleNamedFilter": true;
+	"app.editor.external": true;
+	"app.message.followUp": true;
+	"app.message.dequeue": true;
+	"app.clipboard.pasteImage": true;
+	"app.session.new": true;
+	"app.session.tree": true;
+	"app.session.fork": true;
+	"app.session.resume": true;
+	"app.tree.foldOrUp": true;
+	"app.tree.unfoldOrDown": true;
+	"app.session.togglePath": true;
+	"app.session.toggleSort": true;
+	"app.session.rename": true;
+	"app.session.delete": true;
+	"app.session.deleteNoninvasive": true;
 }
 
-/**
- * Manages all keybindings (app + editor).
- */
-export class KeybindingsManager {
-	private config: KeybindingsConfig;
-	private configPath: string | undefined;
-	private appActionToKeys: Map<AppAction, KeyId[]>;
+export type AppKeybinding = keyof AppKeybindings;
 
-	private constructor(config: KeybindingsConfig, configPath?: string) {
-		this.config = config;
-		this.configPath = configPath;
-		this.appActionToKeys = new Map();
-		this.buildMaps();
+declare module "@mariozechner/pi-tui" {
+	interface Keybindings extends AppKeybindings {}
+}
+
+export const KEYBINDINGS = {
+	...TUI_KEYBINDINGS,
+	"app.interrupt": { defaultKeys: "escape", description: "Cancel or abort" },
+	"app.clear": { defaultKeys: "ctrl+c", description: "Clear editor" },
+	"app.exit": { defaultKeys: "ctrl+d", description: "Exit when editor is empty" },
+	"app.suspend": { defaultKeys: "ctrl+z", description: "Suspend to background" },
+	"app.thinking.cycle": {
+		defaultKeys: "shift+tab",
+		description: "Cycle thinking level",
+	},
+	"app.model.cycleForward": {
+		defaultKeys: "ctrl+p",
+		description: "Cycle to next model",
+	},
+	"app.model.cycleBackward": {
+		defaultKeys: "shift+ctrl+p",
+		description: "Cycle to previous model",
+	},
+	"app.model.select": { defaultKeys: "ctrl+l", description: "Open model selector" },
+	"app.tools.expand": { defaultKeys: "ctrl+o", description: "Toggle tool output" },
+	"app.thinking.toggle": {
+		defaultKeys: "ctrl+t",
+		description: "Toggle thinking blocks",
+	},
+	"app.session.toggleNamedFilter": {
+		defaultKeys: "ctrl+n",
+		description: "Toggle named session filter",
+	},
+	"app.editor.external": {
+		defaultKeys: "ctrl+g",
+		description: "Open external editor",
+	},
+	"app.message.followUp": {
+		defaultKeys: "alt+enter",
+		description: "Queue follow-up message",
+	},
+	"app.message.dequeue": {
+		defaultKeys: "alt+up",
+		description: "Restore queued messages",
+	},
+	"app.clipboard.pasteImage": {
+		defaultKeys: process.platform === "win32" ? "alt+v" : "ctrl+v",
+		description: "Paste image from clipboard",
+	},
+	"app.session.new": { defaultKeys: [], description: "Start a new session" },
+	"app.session.tree": { defaultKeys: [], description: "Open session tree" },
+	"app.session.fork": { defaultKeys: [], description: "Fork current session" },
+	"app.session.resume": { defaultKeys: [], description: "Resume a session" },
+	"app.tree.foldOrUp": {
+		defaultKeys: ["ctrl+left", "alt+left"],
+		description: "Fold tree branch or move up",
+	},
+	"app.tree.unfoldOrDown": {
+		defaultKeys: ["ctrl+right", "alt+right"],
+		description: "Unfold tree branch or move down",
+	},
+	"app.session.togglePath": {
+		defaultKeys: "ctrl+p",
+		description: "Toggle session path display",
+	},
+	"app.session.toggleSort": {
+		defaultKeys: "ctrl+s",
+		description: "Toggle session sort mode",
+	},
+	"app.session.rename": {
+		defaultKeys: "ctrl+r",
+		description: "Rename session",
+	},
+	"app.session.delete": {
+		defaultKeys: "ctrl+d",
+		description: "Delete session",
+	},
+	"app.session.deleteNoninvasive": {
+		defaultKeys: "ctrl+backspace",
+		description: "Delete session when query is empty",
+	},
+} as const satisfies KeybindingDefinitions;
+
+const KEYBINDING_NAME_MIGRATIONS = {
+	cursorUp: "tui.editor.cursorUp",
+	cursorDown: "tui.editor.cursorDown",
+	cursorLeft: "tui.editor.cursorLeft",
+	cursorRight: "tui.editor.cursorRight",
+	cursorWordLeft: "tui.editor.cursorWordLeft",
+	cursorWordRight: "tui.editor.cursorWordRight",
+	cursorLineStart: "tui.editor.cursorLineStart",
+	cursorLineEnd: "tui.editor.cursorLineEnd",
+	jumpForward: "tui.editor.jumpForward",
+	jumpBackward: "tui.editor.jumpBackward",
+	pageUp: "tui.editor.pageUp",
+	pageDown: "tui.editor.pageDown",
+	deleteCharBackward: "tui.editor.deleteCharBackward",
+	deleteCharForward: "tui.editor.deleteCharForward",
+	deleteWordBackward: "tui.editor.deleteWordBackward",
+	deleteWordForward: "tui.editor.deleteWordForward",
+	deleteToLineStart: "tui.editor.deleteToLineStart",
+	deleteToLineEnd: "tui.editor.deleteToLineEnd",
+	yank: "tui.editor.yank",
+	yankPop: "tui.editor.yankPop",
+	undo: "tui.editor.undo",
+	newLine: "tui.input.newLine",
+	submit: "tui.input.submit",
+	tab: "tui.input.tab",
+	copy: "tui.input.copy",
+	selectUp: "tui.select.up",
+	selectDown: "tui.select.down",
+	selectPageUp: "tui.select.pageUp",
+	selectPageDown: "tui.select.pageDown",
+	selectConfirm: "tui.select.confirm",
+	selectCancel: "tui.select.cancel",
+	interrupt: "app.interrupt",
+	clear: "app.clear",
+	exit: "app.exit",
+	suspend: "app.suspend",
+	cycleThinkingLevel: "app.thinking.cycle",
+	cycleModelForward: "app.model.cycleForward",
+	cycleModelBackward: "app.model.cycleBackward",
+	selectModel: "app.model.select",
+	expandTools: "app.tools.expand",
+	toggleThinking: "app.thinking.toggle",
+	toggleSessionNamedFilter: "app.session.toggleNamedFilter",
+	externalEditor: "app.editor.external",
+	followUp: "app.message.followUp",
+	dequeue: "app.message.dequeue",
+	pasteImage: "app.clipboard.pasteImage",
+	newSession: "app.session.new",
+	tree: "app.session.tree",
+	fork: "app.session.fork",
+	resume: "app.session.resume",
+	treeFoldOrUp: "app.tree.foldOrUp",
+	treeUnfoldOrDown: "app.tree.unfoldOrDown",
+	toggleSessionPath: "app.session.togglePath",
+	toggleSessionSort: "app.session.toggleSort",
+	renameSession: "app.session.rename",
+	deleteSession: "app.session.delete",
+	deleteSessionNoninvasive: "app.session.deleteNoninvasive",
+} as const satisfies Record<string, Keybinding>;
+
+function isRecord(value: unknown): value is Record<string, unknown> {
+	return typeof value === "object" && value !== null && !Array.isArray(value);
+}
+
+function isLegacyKeybindingName(key: string): key is keyof typeof KEYBINDING_NAME_MIGRATIONS {
+	return key in KEYBINDING_NAME_MIGRATIONS;
+}
+
+function toKeybindingsConfig(value: unknown): KeybindingsConfig {
+	if (!isRecord(value)) return {};
+
+	const config: KeybindingsConfig = {};
+	for (const [key, binding] of Object.entries(value)) {
+		if (typeof binding === "string") {
+			config[key] = binding as KeyId;
+			continue;
+		}
+		if (Array.isArray(binding) && binding.every((entry) => typeof entry === "string")) {
+			config[key] = binding as KeyId[];
+		}
+	}
+	return config;
+}
+
+function migrateKeybindingNames(rawConfig: Record<string, unknown>): {
+	config: Record<string, unknown>;
+	migrated: boolean;
+} {
+	const config: Record<string, unknown> = {};
+	let migrated = false;
+
+	for (const [key, value] of Object.entries(rawConfig)) {
+		const nextKey = isLegacyKeybindingName(key) ? KEYBINDING_NAME_MIGRATIONS[key] : key;
+		if (nextKey !== key) {
+			migrated = true;
+		}
+		if (key !== nextKey && Object.hasOwn(rawConfig, nextKey)) {
+			migrated = true;
+			continue;
+		}
+		config[nextKey] = value;
+	}
+
+	return { config: orderKeybindingsConfig(config), migrated };
+}
+
+function orderKeybindingsConfig(config: Record<string, unknown>): Record<string, unknown> {
+	const ordered: Record<string, unknown> = {};
+	for (const keybinding of Object.keys(KEYBINDINGS)) {
+		if (Object.hasOwn(config, keybinding)) {
+			ordered[keybinding] = config[keybinding];
+		}
+	}
+
+	const extras = Object.keys(config)
+		.filter((key) => !Object.hasOwn(ordered, key))
+		.sort();
+	for (const key of extras) {
+		ordered[key] = config[key];
+	}
+
+	return ordered;
+}
+
+function loadRawConfig(path: string): Record<string, unknown> | undefined {
+	if (!existsSync(path)) return undefined;
+	try {
+		const parsed = JSON.parse(readFileSync(path, "utf-8")) as unknown;
+		return isRecord(parsed) ? parsed : undefined;
+	} catch {
+		return undefined;
+	}
+}
+
+export function migrateKeybindingsConfigFile(agentDir: string = getAgentDir()): boolean {
+	const configPath = join(agentDir, "keybindings.json");
+	const rawConfig = loadRawConfig(configPath);
+	if (!rawConfig) return false;
+
+	const { config, migrated } = migrateKeybindingNames(rawConfig);
+	if (!migrated) return false;
+
+	writeFileSync(configPath, `${JSON.stringify(config, null, 2)}\n`, "utf-8");
+	return true;
+}
+
+export class KeybindingsManager extends TuiKeybindingsManager {
+	private configPath: string | undefined;
+
+	constructor(userBindings: KeybindingsConfig = {}, configPath?: string) {
+		super(KEYBINDINGS, userBindings);
+		this.configPath = configPath;
 	}
 
-	/**
-	 * Create from config file and set up editor keybindings.
-	 */
 	static create(agentDir: string = getAgentDir()): KeybindingsManager {
 		const configPath = join(agentDir, "keybindings.json");
-		const config = KeybindingsManager.loadFromFile(configPath);
-		const manager = new KeybindingsManager(config, configPath);
-		manager.applyEditorKeybindings();
-		return manager;
-	}
-
-	/**
-	 * Create in-memory.
-	 */
-	static inMemory(config: KeybindingsConfig = {}): KeybindingsManager {
-		return new KeybindingsManager(config);
+		const userBindings = KeybindingsManager.loadFromFile(configPath);
+		return new KeybindingsManager(userBindings, configPath);
 	}
 
 	reload(): void {
 		if (!this.configPath) return;
-		this.config = KeybindingsManager.loadFromFile(this.configPath);
-		this.buildMaps();
-		this.applyEditorKeybindings();
+		this.setUserBindings(KeybindingsManager.loadFromFile(this.configPath));
+	}
+
+	getEffectiveConfig(): KeybindingsConfig {
+		return this.getResolvedBindings();
 	}
 
 	private static loadFromFile(path: string): KeybindingsConfig {
-		if (!existsSync(path)) return {};
-		try {
-			return JSON.parse(readFileSync(path, "utf-8"));
-		} catch {
-			return {};
-		}
-	}
-
-	private buildMaps(): void {
-		this.appActionToKeys.clear();
-
-		// Set defaults for app actions
-		for (const [action, keys] of Object.entries(DEFAULT_APP_KEYBINDINGS)) {
-			const keyArray = Array.isArray(keys) ? keys : [keys];
-			this.appActionToKeys.set(action as AppAction, [...keyArray]);
-		}
-
-		// Override with user config (app actions only)
-		for (const [action, keys] of Object.entries(this.config)) {
-			if (keys === undefined || !isAppAction(action)) continue;
-			const keyArray = Array.isArray(keys) ? keys : [keys];
-			this.appActionToKeys.set(action, keyArray);
-		}
-	}
-
-	private applyEditorKeybindings(): void {
-		const editorConfig: EditorKeybindingsConfig = {};
-		for (const [action, keys] of Object.entries(this.config)) {
-			if (!isAppAction(action) || action === "expandTools") {
-				editorConfig[action as EditorAction] = keys;
-			}
-		}
-		setEditorKeybindings(new EditorKeybindingsManager(editorConfig));
-	}
-
-	/**
-	 * Check if input matches an app action.
-	 */
-	matches(data: string, action: AppAction): boolean {
-		const keys = this.appActionToKeys.get(action);
-		if (!keys) return false;
-		for (const key of keys) {
-			if (matchesKey(data, key)) return true;
-		}
-		return false;
-	}
-
-	/**
-	 * Get keys bound to an app action.
-	 */
-	getKeys(action: AppAction): KeyId[] {
-		return this.appActionToKeys.get(action) ?? [];
-	}
-
-	/**
-	 * Get the full effective config.
-	 */
-	getEffectiveConfig(): Required<KeybindingsConfig> {
-		const result = { ...DEFAULT_KEYBINDINGS };
-		for (const [action, keys] of Object.entries(this.config)) {
-			if (keys !== undefined) {
-				(result as KeybindingsConfig)[action as KeyAction] = keys;
-			}
-		}
-		return result;
+		const rawConfig = loadRawConfig(path);
+		if (!rawConfig) return {};
+		return toKeybindingsConfig(migrateKeybindingNames(rawConfig).config);
 	}
 }
 
-// Re-export for convenience
-export type { EditorAction, KeyId };
+export type { Keybinding, KeyId, KeybindingsConfig };