diff --git a/packages/coding-agent/docs/keybindings.md b/packages/coding-agent/docs/keybindings.md index 1ed72083f..333b94e39 100644 --- a/packages/coding-agent/docs/keybindings.md +++ b/packages/coding-agent/docs/keybindings.md @@ -130,6 +130,26 @@ Modifier combinations: `ctrl+shift+x`, `alt+ctrl+x`, `ctrl+shift+alt+x`, `ctrl+1 | `app.tree.unfoldOrDown` | `ctrl+right`, `alt+right` | Unfold current branch segment, or jump to the next segment start or branch end | | `app.tree.editLabel` | `shift+l` | Edit the label on the selected tree node | | `app.tree.toggleLabelTimestamp` | `shift+t` | Toggle label timestamps in the tree | +| `app.tree.filter.default` | `ctrl+d` | Set tree filter to default view | +| `app.tree.filter.noTools` | `ctrl+t` | Toggle tree filter that hides tool results | +| `app.tree.filter.userOnly` | `ctrl+u` | Toggle tree filter that shows only user messages | +| `app.tree.filter.labeledOnly` | `ctrl+l` | Toggle tree filter that shows only labeled entries | +| `app.tree.filter.all` | `ctrl+a` | Toggle tree filter that shows all entries | +| `app.tree.filter.cycleForward` | `ctrl+o` | Cycle tree filter forward | +| `app.tree.filter.cycleBackward` | `shift+ctrl+o` | Cycle tree filter backward | + +### Scoped Models Selector + +Used inside the scoped models selector (opened via `/scoped-models`). + +| Keybinding id | Default | Description | +|--------|---------|-------------| +| `app.models.save` | `ctrl+s` | Save current model selection to settings | +| `app.models.enableAll` | `ctrl+a` | Enable all models (or all matching the current search) | +| `app.models.clearAll` | `ctrl+x` | Clear all models (or all matching the current search) | +| `app.models.toggleProvider` | `ctrl+p` | Toggle all models for the current provider | +| `app.models.reorderUp` | `alt+up` | Move the selected model up in the cycle order | +| `app.models.reorderDown` | `alt+down` | Move the selected model down in the cycle order | ## Custom Configuration diff --git a/packages/coding-agent/src/core/extensions/runner.ts b/packages/coding-agent/src/core/extensions/runner.ts index 7fd63c530..2360848d7 100644 --- a/packages/coding-agent/src/core/extensions/runner.ts +++ b/packages/coding-agent/src/core/extensions/runner.ts @@ -84,6 +84,10 @@ const buildBuiltinKeybindings = (resolvedKeybindings: KeybindingsConfig): BuiltI const restrictOverride = (RESERVED_KEYBINDINGS_FOR_EXTENSION_CONFLICTS as readonly string[]).includes(keybinding); for (const key of keyList) { const normalizedKey = key.toLowerCase() as KeyId; + // If multiple actions bind the same key, the reserved action wins so extensions + // remain blocked by reserved shortcuts regardless of iteration order. + const existing = builtinKeybindings[normalizedKey]; + if (existing?.restrictOverride && !restrictOverride) continue; builtinKeybindings[normalizedKey] = { keybinding, restrictOverride, diff --git a/packages/coding-agent/src/core/keybindings.ts b/packages/coding-agent/src/core/keybindings.ts index 4f5f10f64..925003ccd 100644 --- a/packages/coding-agent/src/core/keybindings.ts +++ b/packages/coding-agent/src/core/keybindings.ts @@ -39,6 +39,19 @@ export interface AppKeybindings { "app.session.rename": true; "app.session.delete": true; "app.session.deleteNoninvasive": true; + "app.models.save": true; + "app.models.enableAll": true; + "app.models.clearAll": true; + "app.models.toggleProvider": true; + "app.models.reorderUp": true; + "app.models.reorderDown": true; + "app.tree.filter.default": true; + "app.tree.filter.noTools": true; + "app.tree.filter.userOnly": true; + "app.tree.filter.labeledOnly": true; + "app.tree.filter.all": true; + "app.tree.filter.cycleForward": true; + "app.tree.filter.cycleBackward": true; } export type AppKeybinding = keyof AppKeybindings; @@ -134,6 +147,58 @@ export const KEYBINDINGS = { defaultKeys: "ctrl+backspace", description: "Delete session when query is empty", }, + "app.models.save": { + defaultKeys: "ctrl+s", + description: "Save model selection", + }, + "app.models.enableAll": { + defaultKeys: "ctrl+a", + description: "Enable all models", + }, + "app.models.clearAll": { + defaultKeys: "ctrl+x", + description: "Clear all models", + }, + "app.models.toggleProvider": { + defaultKeys: "ctrl+p", + description: "Toggle all models for provider", + }, + "app.models.reorderUp": { + defaultKeys: "alt+up", + description: "Move model up in order", + }, + "app.models.reorderDown": { + defaultKeys: "alt+down", + description: "Move model down in order", + }, + "app.tree.filter.default": { + defaultKeys: "ctrl+d", + description: "Tree filter: default view", + }, + "app.tree.filter.noTools": { + defaultKeys: "ctrl+t", + description: "Tree filter: hide tool results", + }, + "app.tree.filter.userOnly": { + defaultKeys: "ctrl+u", + description: "Tree filter: user messages only", + }, + "app.tree.filter.labeledOnly": { + defaultKeys: "ctrl+l", + description: "Tree filter: labeled entries only", + }, + "app.tree.filter.all": { + defaultKeys: "ctrl+a", + description: "Tree filter: show all entries", + }, + "app.tree.filter.cycleForward": { + defaultKeys: "ctrl+o", + description: "Tree filter: cycle forward", + }, + "app.tree.filter.cycleBackward": { + defaultKeys: "shift+ctrl+o", + description: "Tree filter: cycle backward", + }, } as const satisfies KeybindingDefinitions; const KEYBINDING_NAME_MIGRATIONS = { diff --git a/packages/coding-agent/src/modes/interactive/components/scoped-models-selector.ts b/packages/coding-agent/src/modes/interactive/components/scoped-models-selector.ts index ab7c741fb..b5fbff0ed 100644 --- a/packages/coding-agent/src/modes/interactive/components/scoped-models-selector.ts +++ b/packages/coding-agent/src/modes/interactive/components/scoped-models-selector.ts @@ -12,6 +12,7 @@ import { } from "@mariozechner/pi-tui"; import { theme } from "../theme/theme.js"; import { DynamicBorder } from "./dynamic-border.js"; +import { keyText } from "./keybinding-hints.js"; // EnabledIds: null = all enabled (no filter), string[] = explicit ordered list type EnabledIds = string[] | null; @@ -126,7 +127,9 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl this.addChild(new DynamicBorder()); this.addChild(new Spacer(1)); this.addChild(new Text(theme.fg("accent", theme.bold("Model Configuration")), 0, 0)); - this.addChild(new Text(theme.fg("muted", "Session-only. Ctrl+S to save to settings."), 0, 0)); + this.addChild( + new Text(theme.fg("muted", `Session-only. ${keyText("app.models.save")} to save to settings.`), 0, 0), + ); this.addChild(new Spacer(1)); // Search input @@ -162,7 +165,15 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl const enabledCount = this.enabledIds?.length ?? this.allIds.length; const allEnabled = this.enabledIds === null; const countText = allEnabled ? "all enabled" : `${enabledCount}/${this.allIds.length} enabled`; - const parts = ["Enter toggle", "^A all", "^X clear", "^P provider", "Alt+↑↓ reorder", "^S save", countText]; + const parts = [ + `${keyText("tui.select.confirm")} toggle`, + `${keyText("app.models.enableAll")} all`, + `${keyText("app.models.clearAll")} clear`, + `${keyText("app.models.toggleProvider")} provider`, + `${keyText("app.models.reorderUp")}/${keyText("app.models.reorderDown")} reorder`, + `${keyText("app.models.save")} save`, + countText, + ]; return this.isDirty ? theme.fg("dim", ` ${parts.join(" · ")} `) + theme.fg("warning", "(unsaved)") : theme.fg("dim", ` ${parts.join(" · ")}`); @@ -237,12 +248,14 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl return; } - // Alt+Up/Down - Reorder enabled models - if (matchesKey(data, Key.alt("up")) || matchesKey(data, Key.alt("down"))) { + // Reorder enabled models + const reorderUp = kb.matches(data, "app.models.reorderUp"); + const reorderDown = kb.matches(data, "app.models.reorderDown"); + if (reorderUp || reorderDown) { if (this.enabledIds === null) return; const item = this.filteredItems[this.selectedIndex]; if (item && isEnabled(this.enabledIds, item.fullId)) { - const delta = matchesKey(data, Key.alt("up")) ? -1 : 1; + const delta = reorderUp ? -1 : 1; const currentIndex = this.enabledIds.indexOf(item.fullId); const newIndex = currentIndex + delta; // Only move if within bounds @@ -258,7 +271,7 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl } // Toggle on Enter - if (matchesKey(data, Key.enter)) { + if (kb.matches(data, "tui.select.confirm")) { const item = this.filteredItems[this.selectedIndex]; if (item) { this.enabledIds = toggle(this.enabledIds, item.fullId); @@ -269,8 +282,8 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl return; } - // Ctrl+A - Enable all (filtered if search active, otherwise all) - if (matchesKey(data, Key.ctrl("a"))) { + // Enable all (filtered if search active, otherwise all) + if (kb.matches(data, "app.models.enableAll")) { const targetIds = this.searchInput.getValue() ? this.filteredItems.map((i) => i.fullId) : undefined; this.enabledIds = enableAll(this.enabledIds, this.allIds, targetIds); this.isDirty = true; @@ -279,8 +292,8 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl return; } - // Ctrl+X - Clear all (filtered if search active, otherwise all) - if (matchesKey(data, Key.ctrl("x"))) { + // Clear all (filtered if search active, otherwise all) + if (kb.matches(data, "app.models.clearAll")) { const targetIds = this.searchInput.getValue() ? this.filteredItems.map((i) => i.fullId) : undefined; this.enabledIds = clearAll(this.enabledIds, this.allIds, targetIds); this.isDirty = true; @@ -289,8 +302,8 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl return; } - // Ctrl+P - Toggle provider of current item - if (matchesKey(data, Key.ctrl("p"))) { + // Toggle provider of current item + if (kb.matches(data, "app.models.toggleProvider")) { const item = this.filteredItems[this.selectedIndex]; if (item) { const provider = item.model.provider; @@ -306,8 +319,8 @@ export class ScopedModelsSelectorComponent extends Container implements Focusabl return; } - // Ctrl+S - Save/persist to settings - if (matchesKey(data, Key.ctrl("s"))) { + // Save/persist to settings + if (kb.matches(data, "app.models.save")) { this.callbacks.onPersist(this.enabledIds === null ? null : [...this.enabledIds]); this.isDirty = false; this.footerText.setText(this.getFooterText()); diff --git a/packages/coding-agent/src/modes/interactive/components/tree-selector.ts b/packages/coding-agent/src/modes/interactive/components/tree-selector.ts index e4626a269..17762af72 100644 --- a/packages/coding-agent/src/modes/interactive/components/tree-selector.ts +++ b/packages/coding-agent/src/modes/interactive/components/tree-selector.ts @@ -4,7 +4,6 @@ import { type Focusable, getKeybindings, Input, - matchesKey, Spacer, Text, TruncatedText, @@ -938,39 +937,39 @@ class TreeList implements Component { } else { this.onCancel?.(); } - } else if (matchesKey(keyData, "ctrl+d")) { + } else if (kb.matches(keyData, "app.tree.filter.default")) { // Direct filter: default this.filterMode = "default"; this.foldedNodes.clear(); this.applyFilter(); - } else if (matchesKey(keyData, "ctrl+t")) { + } else if (kb.matches(keyData, "app.tree.filter.noTools")) { // Toggle filter: no-tools ↔ default this.filterMode = this.filterMode === "no-tools" ? "default" : "no-tools"; this.foldedNodes.clear(); this.applyFilter(); - } else if (matchesKey(keyData, "ctrl+u")) { + } else if (kb.matches(keyData, "app.tree.filter.userOnly")) { // Toggle filter: user-only ↔ default this.filterMode = this.filterMode === "user-only" ? "default" : "user-only"; this.foldedNodes.clear(); this.applyFilter(); - } else if (matchesKey(keyData, "ctrl+l")) { + } else if (kb.matches(keyData, "app.tree.filter.labeledOnly")) { // Toggle filter: labeled-only ↔ default this.filterMode = this.filterMode === "labeled-only" ? "default" : "labeled-only"; this.foldedNodes.clear(); this.applyFilter(); - } else if (matchesKey(keyData, "ctrl+a")) { + } else if (kb.matches(keyData, "app.tree.filter.all")) { // Toggle filter: all ↔ default this.filterMode = this.filterMode === "all" ? "default" : "all"; this.foldedNodes.clear(); this.applyFilter(); - } else if (matchesKey(keyData, "shift+ctrl+o")) { + } else if (kb.matches(keyData, "app.tree.filter.cycleBackward")) { // Cycle filter backwards const modes: FilterMode[] = ["default", "no-tools", "user-only", "labeled-only", "all"]; const currentIndex = modes.indexOf(this.filterMode); this.filterMode = modes[(currentIndex - 1 + modes.length) % modes.length]; this.foldedNodes.clear(); this.applyFilter(); - } else if (matchesKey(keyData, "ctrl+o")) { + } else if (kb.matches(keyData, "app.tree.filter.cycleForward")) { // Cycle filter forwards: default → no-tools → user-only → labeled-only → all → default const modes: FilterMode[] = ["default", "no-tools", "user-only", "labeled-only", "all"]; const currentIndex = modes.indexOf(this.filterMode); @@ -1178,11 +1177,19 @@ export class TreeSelectorComponent extends Container implements Focusable { this.addChild(new Spacer(1)); this.addChild(new DynamicBorder()); this.addChild(new Text(theme.bold(" Session Tree"), 1, 0)); + const filterKeys = [ + keyText("app.tree.filter.default"), + keyText("app.tree.filter.noTools"), + keyText("app.tree.filter.userOnly"), + keyText("app.tree.filter.labeledOnly"), + keyText("app.tree.filter.all"), + ].join("/"); + const cycleKeys = `${keyText("app.tree.filter.cycleForward")}/${keyText("app.tree.filter.cycleBackward")}`; this.addChild( new TruncatedText( theme.fg( "muted", - ` ↑/↓: move. ←/→: page. ^←/^→ or Alt+←/Alt+→: fold/branch. ${keyText("app.tree.editLabel")}: label. ^D/^T/^U/^L/^A: filters (^O/⇧^O cycle). ${keyText("app.tree.toggleLabelTimestamp")}: label time`, + ` ↑/↓: move. ←/→: page. ^←/^→ or Alt+←/Alt+→: fold/branch. ${keyText("app.tree.editLabel")}: label. ${filterKeys}: filters (${cycleKeys} cycle). ${keyText("app.tree.toggleLabelTimestamp")}: label time`, ), 0, 0, diff --git a/packages/coding-agent/test/extensions-runner.test.ts b/packages/coding-agent/test/extensions-runner.test.ts index 21d519afe..7719ed487 100644 --- a/packages/coding-agent/test/extensions-runner.test.ts +++ b/packages/coding-agent/test/extensions-runner.test.ts @@ -180,6 +180,29 @@ describe("ExtensionRunner", () => { warnSpy.mockRestore(); }); + it("blocks shortcuts when reserved key is also bound to non-reserved actions", async () => { + const extCode = ` + export default function(pi) { + pi.registerShortcut("ctrl+p", { + description: "Conflicts with shared reserved default", + handler: async () => {}, + }); + } + `; + fs.writeFileSync(path.join(extensionsDir, "shared-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(defaultKeybindings); + + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("conflicts with built-in")); + expect(shortcuts.has("ctrl+p")).toBe(false); + + warnSpy.mockRestore(); + }); + it("blocks shortcuts when reserved action has multiple keys", async () => { const extCode = ` export default function(pi) { diff --git a/packages/coding-agent/test/suite/regressions/3217-scoped-model-order.test.ts b/packages/coding-agent/test/suite/regressions/3217-scoped-model-order.test.ts index b1c159e84..da5411c8f 100644 --- a/packages/coding-agent/test/suite/regressions/3217-scoped-model-order.test.ts +++ b/packages/coding-agent/test/suite/regressions/3217-scoped-model-order.test.ts @@ -1,6 +1,7 @@ -import type { TUI } from "@mariozechner/pi-tui"; +import { setKeybindings, type TUI } from "@mariozechner/pi-tui"; import stripAnsi from "strip-ansi"; -import { afterEach, beforeAll, describe, expect, it } from "vitest"; +import { afterEach, beforeAll, beforeEach, describe, expect, it } from "vitest"; +import { KeybindingsManager } from "../../../src/core/keybindings.js"; import { ModelSelectorComponent } from "../../../src/modes/interactive/components/model-selector.js"; import { ScopedModelsSelectorComponent } from "../../../src/modes/interactive/components/scoped-models-selector.js"; import { initTheme } from "../../../src/modes/interactive/theme/theme.js"; @@ -23,6 +24,11 @@ describe("issue #3217 scoped model ordering", () => { initTheme("dark"); }); + beforeEach(() => { + // Ensure test isolation: keybindings are a global singleton + setKeybindings(new KeybindingsManager()); + }); + afterEach(() => { while (harnesses.length > 0) { harnesses.pop()?.cleanup();