Respect reserved keybindings when registering extensions

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

@@ -6,6 +6,7 @@ import type { AgentMessage } from "@mariozechner/pi-agent-core";
 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 { KeyAction, KeybindingsConfig } from "../keybindings.js";
 import type { ModelRegistry } from "../model-registry.js";
 import type { SessionManager } from "../session-manager.js";
 import type {
@@ -42,6 +43,44 @@ import type {
 	UserBashEventResult,
 } from "./types.js";
 
+// These keybindings 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",
+	"deleteToLineEnd",
+];
+
+type BuiltInKeyBindings = Partial<Record<KeyId, { action: KeyAction; restrictOverride: boolean }>>;
+
+const buildBuiltinKeybindings = (effectiveKeybindings: Required<KeybindingsConfig>): BuiltInKeyBindings => {
+	const builtinKeybindings = {} as BuiltInKeyBindings;
+	for (const [action, keys] of Object.entries(effectiveKeybindings)) {
+		const keyAction = action as KeyAction;
+		const keyList = Array.isArray(keys) ? keys : [keys];
+		const restrictOverride = RESERVED_ACTIONS_FOR_EXTENSION_CONFLICTS.includes(keyAction);
+		for (const key of keyList) {
+			builtinKeybindings[key] = {
+				action: keyAction,
+				restrictOverride: restrictOverride,
+			};
+		}
+	}
+	return builtinKeybindings;
+};
+
 /** Combined result from all before_agent_start handlers */
 interface BeforeAgentStartCombinedResult {
 	messages?: NonNullable<BeforeAgentStartEventResult["message"]>[];
@@ -224,46 +263,37 @@ export class ExtensionRunner {
 		this.runtime.flagValues.set(name, value);
 	}
 
-	private static readonly RESERVED_SHORTCUTS = new Set([
-		"ctrl+c",
-		"ctrl+d",
-		"ctrl+z",
-		"ctrl+k",
-		"ctrl+p",
-		"ctrl+l",
-		"ctrl+o",
-		"ctrl+t",
-		"ctrl+g",
-		"shift+tab",
-		"shift+ctrl+p",
-		"alt+enter",
-		"escape",
-		"enter",
-	]);
-
-	getShortcuts(): Map<KeyId, ExtensionShortcut> {
-		const allShortcuts = new Map<KeyId, ExtensionShortcut>();
+	getShortcuts(effectiveKeybindings: Required<KeybindingsConfig>): Map<KeyId, ExtensionShortcut> {
+		const builtinKeybindings = buildBuiltinKeybindings(effectiveKeybindings);
+		const extensionShortcuts = new Map<KeyId, ExtensionShortcut>();
 		for (const ext of this.extensions) {
 			for (const [key, shortcut] of ext.shortcuts) {
 				const normalizedKey = key.toLowerCase() as KeyId;
 
-				if (ExtensionRunner.RESERVED_SHORTCUTS.has(normalizedKey)) {
+				const builtInKeybinding = builtinKeybindings[normalizedKey];
+				if (builtInKeybinding?.restrictOverride === true) {
 					console.warn(
 						`Extension shortcut '${key}' from ${shortcut.extensionPath} conflicts with built-in shortcut. Skipping.`,
 					);
 					continue;
 				}
 
-				const existing = allShortcuts.get(normalizedKey);
-				if (existing) {
+				if (builtInKeybinding?.restrictOverride === false) {
 					console.warn(
-						`Extension shortcut conflict: '${key}' registered by both ${existing.extensionPath} and ${shortcut.extensionPath}. Using ${shortcut.extensionPath}.`,
+						`Extension shortcut conflict: '${key}' is built-in shortcut for ${builtInKeybinding.action} and ${shortcut.extensionPath}. Using ${shortcut.extensionPath}.`,
 					);
 				}
-				allShortcuts.set(normalizedKey, shortcut);
+
+				const existingExtensionShortcut = extensionShortcuts.get(normalizedKey);
+				if (existingExtensionShortcut) {
+					console.warn(
+						`Extension shortcut conflict: '${key}' registered by both ${existingExtensionShortcut.extensionPath} and ${shortcut.extensionPath}. Using ${shortcut.extensionPath}.`,
+					);
+				}
+				extensionShortcuts.set(normalizedKey, shortcut);
 			}
 		}
-		return allShortcuts;
+		return extensionShortcuts;
 	}
 
 	onError(listener: ExtensionErrorListener): () => void {

packages/coding-agent/src/modes/interactive/interactive-mode.ts

@@ -814,7 +814,7 @@ export class InteractiveMode {
 	 * Set up keyboard shortcuts registered by extensions.
 	 */
 	private setupExtensionShortcuts(extensionRunner: ExtensionRunner): void {
-		const shortcuts = extensionRunner.getShortcuts();
+		const shortcuts = extensionRunner.getShortcuts(this.keybindings.getEffectiveConfig());
 		if (shortcuts.size === 0) return;
 
 		// Create a context for shortcut handlers
@@ -3505,7 +3505,7 @@ export class InteractiveMode {
 		// Add extension-registered shortcuts
 		const extensionRunner = this.session.extensionRunner;
 		if (extensionRunner) {
-			const shortcuts = extensionRunner.getShortcuts();
+			const shortcuts = extensionRunner.getShortcuts(this.keybindings.getEffectiveConfig());
 			if (shortcuts.size > 0) {
 				hotkeys += `
 **Extensions**

packages/coding-agent/test/extensions-runner.test.ts

@@ -9,6 +9,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import { AuthStorage } from "../src/core/auth-storage.js";
 import { discoverAndLoadExtensions } from "../src/core/extensions/loader.js";
 import { ExtensionRunner } from "../src/core/extensions/runner.js";
+import { DEFAULT_KEYBINDINGS, type KeyId } from "../src/core/keybindings.js";
 import { ModelRegistry } from "../src/core/model-registry.js";
 import { SessionManager } from "../src/core/session-manager.js";
 
@@ -47,7 +48,7 @@ describe("ExtensionRunner", () => {
 
 			const result = await discoverAndLoadExtensions([], tempDir, tempDir);
 			const runner = new ExtensionRunner(result.extensions, result.runtime, tempDir, sessionManager, modelRegistry);
-			const shortcuts = runner.getShortcuts();
+			const shortcuts = runner.getShortcuts(DEFAULT_KEYBINDINGS);
 
 			expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("conflicts with built-in"));
 			expect(shortcuts.has("ctrl+c")).toBe(false);
@@ -55,6 +56,125 @@ describe("ExtensionRunner", () => {
 			warnSpy.mockRestore();
 		});
 
+		it("allows a shortcut when the reserved set no longer contains the default key", async () => {
+			const extCode = `
+				export default function(pi) {
+					pi.registerShortcut("ctrl+p", {
+						description: "Uses freed default",
+						handler: async () => {},
+					});
+				}
+			`;
+			fs.writeFileSync(path.join(extensionsDir, "rebinding.ts"), extCode);
+
+			const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
+
+			const result = await discoverAndLoadExtensions([], tempDir, tempDir);
+			const runner = new ExtensionRunner(result.extensions, result.runtime, tempDir, sessionManager, modelRegistry);
+			const keybindings = { ...DEFAULT_KEYBINDINGS, cycleModelForward: "ctrl+n" as KeyId };
+			const shortcuts = runner.getShortcuts(keybindings);
+
+			expect(shortcuts.has("ctrl+p")).toBe(true);
+			expect(warnSpy).not.toHaveBeenCalledWith(expect.stringContaining("conflicts with built-in"));
+
+			warnSpy.mockRestore();
+		});
+
+		it("warns but allows when extension uses non-reserved built-in shortcut", async () => {
+			const extCode = `
+				export default function(pi) {
+					pi.registerShortcut("ctrl+v", {
+						description: "Overrides non-reserved",
+						handler: async () => {},
+					});
+				}
+			`;
+			fs.writeFileSync(path.join(extensionsDir, "non-reserved.ts"), extCode);
+
+			const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
+
+			const result = await discoverAndLoadExtensions([], tempDir, tempDir);
+			const runner = new ExtensionRunner(result.extensions, result.runtime, tempDir, sessionManager, modelRegistry);
+			const shortcuts = runner.getShortcuts(DEFAULT_KEYBINDINGS);
+
+			expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("built-in shortcut for pasteImage"));
+			expect(shortcuts.has("ctrl+v")).toBe(true);
+
+			warnSpy.mockRestore();
+		});
+
+		it("blocks shortcuts for reserved actions even when rebound", async () => {
+			const extCode = `
+				export default function(pi) {
+					pi.registerShortcut("ctrl+x", {
+						description: "Conflicts with rebound reserved",
+						handler: async () => {},
+					});
+				}
+			`;
+			fs.writeFileSync(path.join(extensionsDir, "rebound-reserved.ts"), extCode);
+
+			const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
+
+			const result = await discoverAndLoadExtensions([], tempDir, tempDir);
+			const runner = new ExtensionRunner(result.extensions, result.runtime, tempDir, sessionManager, modelRegistry);
+			const keybindings = { ...DEFAULT_KEYBINDINGS, interrupt: "ctrl+x" as KeyId };
+			const shortcuts = runner.getShortcuts(keybindings);
+
+			expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("conflicts with built-in"));
+			expect(shortcuts.has("ctrl+x")).toBe(false);
+
+			warnSpy.mockRestore();
+		});
+
+		it("blocks shortcuts when reserved action has multiple keys", async () => {
+			const extCode = `
+				export default function(pi) {
+					pi.registerShortcut("ctrl+y", {
+						description: "Conflicts with multi-key reserved",
+						handler: async () => {},
+					});
+				}
+			`;
+			fs.writeFileSync(path.join(extensionsDir, "multi-reserved.ts"), extCode);
+
+			const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
+
+			const result = await discoverAndLoadExtensions([], tempDir, tempDir);
+			const runner = new ExtensionRunner(result.extensions, result.runtime, tempDir, sessionManager, modelRegistry);
+			const keybindings = { ...DEFAULT_KEYBINDINGS, clear: ["ctrl+x", "ctrl+y"] as KeyId[] };
+			const shortcuts = runner.getShortcuts(keybindings);
+
+			expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("conflicts with built-in"));
+			expect(shortcuts.has("ctrl+y")).toBe(false);
+
+			warnSpy.mockRestore();
+		});
+
+		it("warns but allows when non-reserved action has multiple keys", async () => {
+			const extCode = `
+				export default function(pi) {
+					pi.registerShortcut("ctrl+y", {
+						description: "Overrides multi-key non-reserved",
+						handler: async () => {},
+					});
+				}
+			`;
+			fs.writeFileSync(path.join(extensionsDir, "multi-non-reserved.ts"), extCode);
+
+			const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
+
+			const result = await discoverAndLoadExtensions([], tempDir, tempDir);
+			const runner = new ExtensionRunner(result.extensions, result.runtime, tempDir, sessionManager, modelRegistry);
+			const keybindings = { ...DEFAULT_KEYBINDINGS, pasteImage: ["ctrl+x", "ctrl+y"] as KeyId[] };
+			const shortcuts = runner.getShortcuts(keybindings);
+
+			expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("built-in shortcut for pasteImage"));
+			expect(shortcuts.has("ctrl+y")).toBe(true);
+
+			warnSpy.mockRestore();
+		});
+
 		it("warns when two extensions register same shortcut", async () => {
 			// Use a non-reserved shortcut
 			const extCode1 = `
@@ -80,7 +200,7 @@ describe("ExtensionRunner", () => {
 
 			const result = await discoverAndLoadExtensions([], tempDir, tempDir);
 			const runner = new ExtensionRunner(result.extensions, result.runtime, tempDir, sessionManager, modelRegistry);
-			const shortcuts = runner.getShortcuts();
+			const shortcuts = runner.getShortcuts(DEFAULT_KEYBINDINGS);
 
 			expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("shortcut conflict"));
 			// Last one wins