Removed Git worktree directory
Public
Removed Git worktree directory
Agent
Pi
Model
GPT-5.2 Codex
Started
Jan 18, 6:19 PM
Messages179
Downloads0
Identified the target worktree directory.
UnderstandProvided git worktree removal commands.
EditConfirmed worktree removal.
Verify
You are given one or more GitHub PR URLs: https://github.com/badlogic/pi-mono/pull/826
For each PR URL, do the following in order:
packages/*/CHANGELOG.md files. Report whether an entry exists. If missing, state that a changelog entry is required before merge and that you will add it if the user decides to merge. Follow the changelog format rules in AGENTS.md. Verify:
### Breaking Changes, ### Added, ### Fixed, etc.)Fixed foo ([#123](https://github.com/badlogic/pi-mono/pull/123) by [@user](https://github.com/user))### Breaking Changes, not just ### FixedOutput format per PR: PR: Changelog:
If no issues are found, say so under Bad and Ugly.
Planning PR data retrieval
I'm setting up to fetch comprehensive PR details—title, body, comments, commits, files, author, labels—using gh pr view and gh pr diff, plus scanning for linked issues to retrieve via gh issue view. I'll also read the main branch files fully without using sed or cat.
>gh pr view 826 --json title,body,comments,commits,files,author,number,url{"author":{"id":"MDQ6VXNlcjY2NDA4NzQ=","is_bot":false,"login":"richardgill","name":"Richard Gill"},"body":"Hey 👋, this can wait until tomorrow :).\r\nAlso: Sorry - I should have raised an issue first for this one - very happy to throw away this code 👍\r\n\r\nMy usecase: \r\n\r\nI wanted to map the preset extension to `ctrl+p` which is the built-in shortcut to cycle through models. But even if I remap `ctrl+p` in `keybindings.json` I still cannot use the keybinding in an extension.\r\n\r\nThis PR changes three things:\r\n\r\n- Extension key binding logic checks against the user's actual keybindings, not the defaults.\r\n- Adds an extra warning for 'built-in' keybindings that you **can** override in an extension.\r\n- The checks are done on 'action' (e.g. `cycleModelForward`) not 'keybinding' (e.g. `ctrl+p`)\r\n\r\n\r\n\r\n\r\n","comments":[{"id":"IC_kwDOPbFNk87gcA6G","author":{"login":"badlogic"},"authorAssociation":"OWNER","body":"Will have a look. Note that folks that have previously gotten a PR merged to not need to raise an issue per se. It's just a gate for people throwing agent slop at the repo in form of terrible PRs. I want to force them to engage properly first.","createdAt":"2026-01-18T16:02:19Z","includesCreatedEdit":false,"isMinimized":false,"minimizedReason":"","reactionGroups":[],"url":"https://github.com/badlogic/pi-mono/pull/826#issuecomment-3765440134","viewerDidAuthor":true}],"commits":[{"authoredDate":"2026-01-18T13:01:12Z","authors":[{"email":"[REDACTED]","id":"MDQ6VXNlcjY2NDA4NzQ=","login":"richardgill","name":"Richard Gill"}],"committedDate":"2026-01-18T13:01:12Z","messageBody":"","messageHeadline":"Respect reserved keybindings when registering extensions","oid":"d0a6cf206ebdcc29fa56f40b1061e083d93c5d60"},{"authoredDate":"2026-01-18T13:01:54Z","authors":[{"email":"[REDACTED]","id":"MDQ6VXNlcjY2NDA4NzQ=","login":"richardgill","name":"Richard Gill"}],"committedDate":"2026-01-18T13:01:54Z","messageBody":"","messageHeadline":"Clarify comment about reserved extension keybindings","oid":"75bf1f77c81af92134e417c95c5dafde992cbc9b"},{"authoredDate":"2026-01-18T13:05:02Z","authors":[{"email":"[REDACTED]","id":"MDQ6VXNlcjY2NDA4NzQ=","login":"richardgill","name":"Richard Gill"}],"committedDate":"2026-01-18T13:05:02Z","messageBody":"","messageHeadline":"Normalize builtin keybinding keys to lowercase","oid":"a66cfa963459c830039cc88dec097ab22d09cd6a"},{"authoredDate":"2026-01-18T13:20:56Z","authors":[{"email":"[REDACTED]","id":"MDQ6VXNlcjY2NDA4NzQ=","login":"richardgill","name":"Richard Gill"}],"committedDate":"2026-01-18T13:20:56Z","messageBody":"","messageHeadline":"Reserve copy action during extension conflicts","oid":"4cd0d3e6266736767f692e38f97bed29ac2a574e"}],"files":[{"path":"packages/coding-agent/src/core/extensions/runner.ts","additions":57,"deletions":25},{"path":"packages/coding-agent/src/modes/interactive/interactive-mode.ts","additions":2,"deletions":2},{"path":"packages/coding-agent/test/extensions-runner.test.ts","additions":122,"deletions":2}],"number":826,"title":"Allow extensions to override default built-in keybindings which have been remapped","url":"https://github.com/badlogic/pi-mono/pull/826"}Fetching PR diff
>gh pr diff 826diff --git a/packages/coding-agent/src/core/extensions/runner.ts b/packages/coding-agent/src/core/extensions/runner.ts
index 4ce99b14d..637e902e5 100644
--- a/packages/coding-agent/src/core/extensions/runner.ts
+++ b/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,46 @@ 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",
+];
+
+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) {
+ const normalizedKey = key.toLowerCase() as KeyId;
+ builtinKeybindings[normalizedKey] = {
+ action: keyAction,
+ restrictOverride: restrictOverride,
+ };
+ }
+ }
+ return builtinKeybindings;
+};
+
/** Combined result from all before_agent_start handlers */
interface BeforeAgentStartCombinedResult {
messages?: NonNullable<BeforeAgentStartEventResult["message"]>[];
@@ -224,46 +265,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}' is built-in shortcut for ${builtInKeybinding.action} and ${shortcut.extensionPath}. Using ${shortcut.extensionPath}.`,
+ );
+ }
+
+ const existingExtensionShortcut = extensionShortcuts.get(normalizedKey);
+ if (existingExtensionShortcut) {
console.warn(
- `Extension shortcut conflict: '${key}' registered by both ${existing.extensionPath} and ${shortcut.extensionPath}. Using ${shortcut.extensionPath}.`,
+ `Extension shortcut conflict: '${key}' registered by both ${existingExtensionShortcut.extensionPath} and ${shortcut.extensionPath}. Using ${shortcut.extensionPath}.`,
);
}
- allShortcuts.set(normalizedKey, shortcut);
+ extensionShortcuts.set(normalizedKey, shortcut);
}
}
- return allShortcuts;
+ return extensionShortcuts;
}
onError(listener: ExtensionErrorListener): () => void {
diff --git a/packages/coding-agent/src/modes/interactive/interactive-mode.ts b/packages/coding-agent/src/modes/interactive/interactive-mode.ts
index 1a267f4aa..bc2836860 100644
--- a/packages/coding-agent/src/modes/interactive/interactive-mode.ts
+++ b/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**
diff --git a/packages/coding-agent/test/extensions-runner.test.ts b/packages/coding-agent/test/extensions-runner.test.ts
index daa6cc985..6e139ff18 100644
--- a/packages/coding-agent/test/extensions-runner.test.ts
+++ b/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 winsPlanning full file reads for relevant code
I identified that no issues are referenced in commits or comments, so I'll focus on reading full relevant files from the main branch without truncation. I'll prioritize runner.ts, interactive-mode.ts, extensions-runner.test.ts, and keybindings.ts to understand the behavior and verify against diffs, also checking related files like extension runner usage and extension registration as needed.
Planning thorough PR and changelog reads
I'll read the entire changelog file without truncation to check the Unreleased section and other entries. For the PR, I'll retrieve full details including description, comments, commits, review comments, and timeline events using a comprehensive PR view command to ensure I capture all relevant context.
>gh pr view 826 --json comments,reviewComments,reviewsUnknown JSON field: "reviewComments"
Available fields:
additions
assignees
author
autoMergeRequest
baseRefName
baseRefOid
body
changedFiles
closed
closedAt
closingIssuesReferences
comments
commits
createdAt
deletions
files
fullDatabaseId
headRefName
headRefOid
headRepository
headRepositoryOwner
id
isCrossRepository
isDraft
labels
latestReviews
maintainerCanModify
mergeCommit
mergeStateStatus
mergeable
mergedAt
mergedBy
milestone
number
potentialMergeCommit
projectCards
projectItems
reactionGroups
reviewDecision
reviewRequests
reviews
state
statusCheckRollup
title
updatedAt
url
Command exited with code 1Clarifying review data usage
Noting that review comments aren't separate; reviews field holds the needed data, so I'll fetch reviews accordingly.
>gh pr view 826 --json reviews{"reviews":[{"id":"PRR_kwDOPbFNk87bE5Tm","author":{"login":"richardgill"},"authorAssociation":"CONTRIBUTOR","body":"","submittedAt":"2026-01-18T13:59:22Z","includesCreatedEdit":false,"reactionGroups":[],"state":"COMMENTED","commit":{"oid":"4cd0d3e6266736767f692e38f97bed29ac2a574e"}}]}Confirming issue closing references
Noted to use the closingIssuesReferences field for linking issue closures in commits.
>gh pr view 826 --json closingIssuesReferences{"closingIssuesReferences":[]}Preparing to read updated main branch files
Preparing to import keybindings
I'm reading the entire keybindings.ts file to properly import keybindings in the diff.
Preparing for full interactive mode
Reconciling file read limits
The read tool truncates files at 50KB or 2000 lines despite instructions to read files fully without offset; to get the complete file, multiple reads with offsets will be necessary even though this slightly conflicts with the ideal of reading "in full" at once.
Confirming multipart reads with offset
The instructions conflict but the tool guidance to use offset/limit for large files takes priority here; I'll read large files in multiple segments using offset to fully retrieve content despite initial "no offset" advice for full reads.
Preparing to read full test file
Planning code and test comparison
Noting the current file is outdated compared to the diff with test updates; I’ll examine interactive-mode fully and related code paths like keybindings manager and extension runner to validate behavior, plus review the changelog for packages/coding-agent.
Planning doc search for keybindings
I'm preparing to search docs (extensions.md and README) for mentions of keybindings or shortcuts to check if any updates are needed related to reserved keybindings in the extension.
>rg -n "shortcut|keybinding" packages/coding-agent/README.md packages/coding-agent/docs packages/coding-agent/examplespackages/coding-agent/README.md:27: - [Keyboard Shortcuts](#keyboard-shortcuts)
packages/coding-agent/README.md:28: - [Custom Keybindings](#custom-keybindings)
packages/coding-agent/README.md:152:**VS Code (Integrated Terminal):** Add to `keybindings.json` to enable `Shift+Enter` for multi-line input:
packages/coding-agent/README.md:308:| `/hotkeys` | Show all keyboard shortcuts |
packages/coding-agent/README.md:379:All keyboard shortcuts can be customized via `~/.pi/agent/keybindings.json`. Each action can be bound to one or more keys.
packages/coding-agent/README.md:1118:Register custom keyboard shortcuts (shown in `/hotkeys`):
packages/coding-agent/examples/README.md:14:- Commands and keyboard shortcuts
packages/coding-agent/examples/extensions/overlay-test.ts:19: (_tui, theme, _keybindings, done) => new OverlayTestComponent(theme, done),
packages/coding-agent/examples/extensions/doom-overlay/index.ts:54: (tui, _theme, _keybindings, done) => {
packages/coding-agent/examples/extensions/overlay-qa-tests.ts:840: // (In real usage, a global keybinding would control visibility)
packages/coding-agent/examples/extensions/overlay-qa-tests.ts:871: th.fg("dim", " In real usage, a global keybinding"),
packages/coding-agent/docs/extensions.md:190: // Register tools, commands, shortcuts, flags
packages/coding-agent/docs/extensions.md:670:Emits `session_shutdown` event to all extensions before exiting. Available in all contexts (event handlers, tools, commands, shortcuts).
packages/coding-agent/docs/extensions.md:963:### pi.registerShortcut(shortcut, options)
packages/coding-agent/docs/extensions.md:965:Register a keyboard shortcut.
packages/coding-agent/docs/extensions.md:1325:Use `keyHint()` to display keybinding hints that respect user's keybinding configuration:
packages/coding-agent/docs/extensions.md:1341:- `appKeyHint(keybindings, action, description)` - App actions (requires `KeybindingsManager`)
packages/coding-agent/docs/extensions.md:1479:ctx.ui.setEditorComponent((tui, theme, keybindings) => new VimEditor(tui, theme, keybindings));
packages/coding-agent/docs/extensions.md:1508:const result = await ctx.ui.custom<boolean>((tui, theme, keybindings, done) => {
packages/coding-agent/docs/extensions.md:1528:- `keybindings` - App keybinding manager (for checking shortcuts)
packages/coding-agent/docs/extensions.md:1539: (tui, theme, keybindings, done) => new MyOverlayComponent({ onClose: done }),
packages/coding-agent/docs/extensions.md:1548: (tui, theme, keybindings, done) => new MyOverlayComponent({ onClose: done }),
packages/coding-agent/docs/extensions.md:1581: super.handleInput(data); // App keybindings + text editing
packages/coding-agent/docs/extensions.md:1587: ctx.ui.setEditorComponent((_tui, theme, keybindings) =>
packages/coding-agent/docs/extensions.md:1588: new VimEditor(theme, keybindings)
packages/coding-agent/docs/extensions.md:1595:- Extend `CustomEditor` (not base `Editor`) to get app keybindings (escape to abort, ctrl+d, model switching)
packages/coding-agent/docs/extensions.md:1597:- Factory receives `theme` and `keybindings` from the app
packages/coding-agent/examples/extensions/custom-header.ts:5: * (logo + keybinding hints) with a custom component showing the pi mascot.
packages/coding-agent/examples/extensions/custom-header.ts:67: description: "Restore built-in header with keybinding hints",
packages/coding-agent/docs/tui.md:113: (tui, theme, keybindings, done) => new MyDialog({ onClose: done }),
packages/coding-agent/docs/tui.md:122: (tui, theme, keybindings, done) => new SidePanel({ onClose: done }),
packages/coding-agent/docs/tui.md:758:Replace the main input editor with a custom implementation. Useful for modal editing (vim), different keybindings (emacs), or specialized input handling.
packages/coding-agent/docs/tui.md:815: // Factory receives theme and keybindings from the app
packages/coding-agent/docs/tui.md:816: ctx.ui.setEditorComponent((tui, theme, keybindings) =>
packages/coding-agent/docs/tui.md:817: new VimEditor(theme, keybindings)
packages/coding-agent/docs/tui.md:825:- **Extend `CustomEditor`** (not base `Editor`) to get app keybindings (escape to abort, ctrl+d to exit, model switching, etc.)
packages/coding-agent/docs/tui.md:827:- **Factory pattern**: `setEditorComponent` receives a factory function that gets `tui`, `theme`, and `keybindings`
packages/coding-agent/docs/tui.md:834:1. **Always use theme from callback** - Don't import theme directly. Use `theme` from the `ctx.ui.custom((tui, theme, keybindings, done) => ...)` callback.
packages/coding-agent/examples/extensions/plan-mode/README.md:18:- `Ctrl+Alt+P` - Toggle plan mode (shortcut)Planning targeted docs and code reads
I'm identifying key docs sections on registerShortcut and reserved keybindings for focused reading, while planning to read relevant code files from main branch in full as required. I'll also review interactive-mode's getShortcuts and related test diffs to track signature changes and keybinding hints.
Identifying changelog gap and doc inspection plan
I found no changelog entry for this coding-agent behavioral change, so I'll report it as missing and propose a "Fixed" entry crediting the external author. I'll also check README sections around registerShortcut and reserved shortcuts for necessary doc updates by searching and reading relevant snippets.
>rg -n "registerShortcut" packages/coding-agent/README.md1122: pi.registerShortcut("ctrl+shift+d", {Planning chunked file reading
I'm preparing to read around line 1110 by using an offset of 1090 and a limit of 80 lines to capture the needed code context.
Checking documentation for reserved shortcuts
I'm searching the docs, especially around the shortcut registration calls, to see if reserved keybindings are already documented and whether the README needs updating.
Assessing keybinding changes and docs impact
Noticing reserved keybindings are now clearly separated from overridable ones, with a warning added on conflicts; this likely warrants at least a mention in docs, especially for registerShortcut. I'll verify all getShortcuts usages reflect signature changes and confirm no regressions by searching across files.
>rg -n "getShortcuts\(" packages/coding-agent/src packages/coding-agent/testpackages/coding-agent/test/extensions-runner.test.ts:50: const shortcuts = runner.getShortcuts();
packages/coding-agent/test/extensions-runner.test.ts:83: const shortcuts = runner.getShortcuts();
packages/coding-agent/src/core/extensions/runner.ts:244: getShortcuts(): Map<KeyId, ExtensionShortcut> {
packages/coding-agent/src/modes/interactive/interactive-mode.ts:817: const shortcuts = extensionRunner.getShortcuts();
packages/coding-agent/src/modes/interactive/interactive-mode.ts:3508: const shortcuts = extensionRunner.getShortcuts();Clarifying reserved actions versus editor actions
I'm verifying that the new reserved action list mixes editor and app actions like "copy" and "submit," so I'll check the pi-tui package for the exact EditorAction and AppAction definitions to ensure these are correctly classified and handled.
Locating editor action definitions
I'm searching in the packages/tui directory to find the file defining EditorAction and editor keybindings to understand their structure and usage.
>rg -n "type EditorAction" packages/tui/srcpackages/tui/src/index.ts:30: type EditorAction,
packages/tui/src/keybindings.ts:6:export type EditorAction =Analyzing reserved actions update
The reserved actions list now explicitly blocks overrides for common editor commands like "copy" and "deleteToLineEnd," likely to prevent extensions from interfering with core behavior. This affects how built-in keybindings map keys to actions with restrictOverride flags, potentially causing conflicts (e.g., "ctrl+c" mapped to both "clear" and "copy") that need careful handling.
Examining keybinding override order and conflicts
The final built-in keybinding for a key is determined by iteration order over editor then app actions, causing later app actions to overwrite earlier editor ones even if reserved. This can lead to reserved actions being potentially bypassed if a non-reserved action later overwrites the same key, possibly allowing extensions to override keys still used by reserved actions—highlighting a subtle risk in conflict detection logic. Next, I’ll check how extension shortcuts integrate and whether built-in handlers block extension shortcuts at runtime.
>rg -n "onExtensionShortcut" packages/coding-agent packages/tuipackages/coding-agent/src/modes/interactive/interactive-mode.ts:851: this.defaultEditor.onExtensionShortcut = (data: string) => {
packages/coding-agent/src/modes/interactive/interactive-mode.ts:1246: customEditor.onExtensionShortcut = this.defaultEditor.onExtensionShortcut;
packages/coding-agent/src/modes/interactive/components/custom-editor.ts:16: public onExtensionShortcut?: (data: string) => boolean;
packages/coding-agent/src/modes/interactive/components/custom-editor.ts:32: if (this.onExtensionShortcut?.(data)) {Planning full read of custom-editor.ts
Clarifying extension shortcut precedence and conflict handling
Extension shortcuts are checked first and override built-in actions, making conflict detection crucial. The code restricts extensions from using reserved action keys based on current user keybindings, but extensions can override non-reserved built-in actions with a warning since onExtensionShortcut runs before built-in handlers.
Analyzing potential conflicts in effective keybinding mapping
The current buildBuiltinKeybindings mapping only records the last action for each key, risking that non-reserved actions later in the order overwrite reserved ones and allow extensions to override critical shortcuts inadvertently. Also, unbinding reserved actions by empty keys could let extensions reuse those keys, which might be intentional but deserves caution. The key normalization approach looks consistent but warrants verification against key matching utilities to ensure case-insensitivity.
Confirming keyId case normalization
I verified that keyIds are consistently lowercased in parsing and matching, so keys like "pageUp" become "pageup" for comparison, aligning with how built-in keybindings are handled. The reserved actions list now includes "copy," ensuring conflicts like ctrl+c are properly reserved.
Analyzing reserved actions and extension shortcut conflicts
The reserved actions list now includes more editor and app actions like "copy" and "deleteToLineEnd," ensuring keys like ctrl+c remain reserved even if remapped, preventing extension overrides. However, a subtle bug exists where user keybindings with non-reserved actions can override reserved actions by order, allowing extensions to override keys intended for reserved actions, which aligns with honoring user remaps but may cause conflicts. Tests cover these cases, confirming warnings for non-reserved conflicts and blocks for reserved ones; overall the system respects user intent while guarding core reserved shortcuts.
Checking public API impact of method signature change
I'm verifying if the changed method in the exported ExtensionRunner class is part of the public API, which could make the signature change breaking; initial exploration suggests it may be internal but potentially accessible externally.
Assessing potential API-breaking change
The method signature change on an exported class method could break typed external usage despite lacking documentation; this suggests a breaking change that should be noted in the changelog.
Identifying multiple review points on keybinding changes
The signature change on the exported method risks breaking external typed usage and needs a changelog entry; docs may require updates about extension shortcut conflicts reflecting user keybindings. Also, the reserved actions logic tied to current keybindings may imperfectly handle overlapping keys, suggesting a possible edge case to highlight in review. Tests cover new behavior, but running them isn’t confirmed.
Summarizing detailed PR review points
The review highlights a breaking signature change on the exported method requiring changelog attention, notes missing documentation for new action-based keybinding conflicts and reserved actions, and confirms added tests covering new behavior though not run. It also raises questions about whether to document the API break explicitly and update extension shortcut docs.
PR: https://github.com/badlogic/pi-mono/pull/826 Changelog:
### Fixed: Fixed extension shortcut conflict checks to respect user keybindings ([#826](https://github.com/badlogic/pi-mono/pull/826) by [@richardgill](https://github.com/richardgill))Good:
Bad:
ExtensionRunner.getShortcuts now requires an argument. This is a public SDK class, so the signature change is breaking for external callers. Consider making the parameter optional with a default or documenting it as a breaking change.Ugly:
Questions or Assumptions:
getShortcuts keep backward compatibility or be called out as a breaking change?Change summary:
Tests:
open the fiel and line in vscode, see vscode skill
Preparing to read skill file
Requesting file and line details
I'm asking the user to specify which file and line they want to open in VSCode before proceeding.
Which file path and line number should I open in VS Code?
getShortcuts
Planning to locate function and open in VSCode
I'm preparing to find the line number of getShortcuts by running ripgrep with line numbers, then open that file at the correct line in VSCode using code -g path:line.
>rg -n "getShortcuts" /Users/badlogic/workspaces/pi-mono/packages/coding-agent/src/core/extensions/runner.ts244: getShortcuts(): Map<KeyId, ExtensionShortcut> {Preparing to open file at line
I'm planning to open a specific file in VS Code at a particular line using the code -g path:line command in bash.
>code -g /Users/badlogic/workspaces/pi-mono/packages/coding-agent/src/core/extensions/runner.ts:244(no output)Opened /Users/badlogic/workspaces/pi-mono/packages/coding-agent/src/core/extensions/runner.ts:244 in VS Code.
explain why this would need an argument. it just returns the shortcuts extensions registered no?
Explaining effective keybindings usage
The new code uses effective keybindings to filter reserved shortcuts and warn about built-in conflicts by checking against the user's current keybindings. The getShortcuts function now requires these to build an accurate map of built-in keys, because without this input it cannot detect which keys the user has overridden. To provide this information, you can pass the effective configuration from the KeybindingsManager when running interactively or allow a default parameter in the runner. This approach ensures proper conflict detection and warning behavior.
It now needs the effective keybindings to decide which extension shortcuts are blocked or warned. The logic no longer checks a static list of reserved key strings. It builds a map of built in actions to the user configured keys, then blocks only those actions marked as reserved. Without the effective keybindings, it cannot know whether a default shortcut like Ctrl+P has been remapped, so it would either block too much or allow conflicts that should be blocked.
this sounds like a good change. can the implemention of the contributor be simplified while still being correct? are there any design decisions other than this additional argument to getShortcuts i need to look at? will this break existing extensions?
Analyzing extension runner changes
The diff introduces reserved actions now centralized in the keybindings module, overlapping with existing keybindings duplicates, suggesting possible simplification by unifying these lists and relying more on KeybindingsManager. Key normalization to lowercase is an explicit design choice, improving consistency. The update adds warnings for extensions overriding non-reserved built-in actions. This risks breaking code using ExtensionRunner’s public getShortcuts method but should not affect typical extensions. Reserved actions include common editor commands like "copy" and "selectConfirm," reflecting deliberate integration with keybindings concepts.
Summarizing keybindings and extension runner changes
The updated getShortcuts requires effective keybindings to identify current built-in bindings and detect freed keys, making the argument necessary. Simplification opportunities include computing reserved key sets once, passing a KeybindingsManager or precomputed map to ExtensionRunner at initialization, or providing an optional parameter with a default to maintain compatibility. Other design choices are normalizing keys to lowercase, maintaining a reserved action list that now covers app and editor actions, and warning extensions when overriding non-reserved built-ins, while allowing the extension binding to take precedence. This introduces stricter blocking of keys bound to reserved actions, reflecting user overrides in effective config. Typical extensions remain unaffected, but SDK consumers using getShortcuts directly face a breaking API change due to the new required argument.
It needs the argument to know the current effective keybindings, not just the defaults. The new behavior allows extension shortcuts to reuse keys that users have remapped away from built in actions, and it must still block keys that are currently bound to reserved actions. Without the effective config, it cannot distinguish those cases.
Simplification options while keeping behavior correct:
KeybindingsManager instead of the full config and call getEffectiveConfig() inside. That hides internal details from callers.DEFAULT_KEYBINDINGS for backward compatibility, then use effective config when available.Other design decisions in the PR to review:
Breaking impact:
getShortcuts.ExtensionRunner directly and call getShortcuts() will break at compile time because the method signature changed. If you want to avoid that, make the argument optional or accept KeybindingsManager and keep a default.i'm not at all concerend about SDK users, as they won't call getShortcuts ever.
would you say this PR is good to merge?
Recommending merge with minor notes
This is good to merge with a minor suggestion: no documentation changes are needed. The optional argument can be removed since it's not required, but the user isn't concerned about the SDK. The only requirement is adding a changelog entry. It may be worth adjusting tests accordingly. Do NOT start implementing changes unless asked.
Yes, good to merge once the changelog entry is added. I do not see blockers beyond that.
mege via gh cli, pull origin, fix up changelog
Confirming full changelog read needed
The changelog file exceeds 50KB and previous reads were truncated, so I must reread it fully without limits before editing to comply with the rule.
Planning segmented changelog reads
The changelog exceeds the tool's 50KB limit, so I'll read it in multiple sequential segments using offsets to cover the entire file before editing, satisfying the full-read requirement despite the partial-read rule. I'll then update the Unreleased Fixed section per the PR details.
Continuing code reading
Confirming full file coverage
I verified the file was read earlier in full and now with offsets, so the full content is covered. Next, I'll update the changelog's Unreleased Fixed section, ensuring I re-read the top part for exact context before editing.
Reading top portion for context
I'll read the file's top lines with offset and limit to precisely locate the Unreleased section, since this partial read aligns with the rules after having read the full file earlier.
Inserting new fixed bullet under Unreleased
I'm preparing to add a new bullet under the Fixed section for Unreleased, accurately formatting the entry to note a fix for extension shortcut conflict detection with proper PR and author links.
Adding focus propagation fix bullet
I'm adding a bullet noting that components with search inputs now correctly propagate focus state for cursor positioning, linked to issue #827.
Checking current git branch before PR workflow
>git status -sb## main...origin/main
M packages/coding-agent/CHANGELOG.mdCreating feature branch with local changes
Created a new feature branch from main preserving local changes to avoid stash. Next, I'll fetch the PR branch and merge or cherry-pick its changes into this branch to integrate both sets of commits safely.
Planning clean integration of PR commits with local changes
The safest approach is to create a feature branch from main including the changelog commit, fetch and rebase the PR branch onto the latest main, then rebase the feature branch onto the rebased PR branch to sequence commits properly without merge conflicts or merge commits.
>git checkout -b pr-826Switched to a new branch 'pr-826'Committing recent changes
>git add /Users/badlogic/workspaces/pi-mono/packages/coding-agent/CHANGELOG.md(no output)Committing changes
>git commit -m "chore(coding-agent): add changelog entry for shortcuts"Running formatting, linting, and type checking...
> pi-monorepo@0.0.3 check
> biome check --write . && tsgo --noEmit && npm run check -w @mariozechner/pi-web-ui
Checked 407 files in 242ms. No fixes applied.
> @mariozechner/pi-web-ui@0.49.0 check
> biome check --write . && tsc --noEmit && cd example && biome check --write . && tsc --noEmit
Checked 73 files in 37ms. No fixes applied.
Checked 3 files in 8ms. No fixes applied.
✅ All pre-commit checks passed!
[pr-826 04664b51] chore(coding-agent): add changelog entry for shortcuts
1 file changed, 1 insertion(+)Confirming automatic check run on commit
The commit triggered the automatic run of npm check as expected by hooks, aligning with the instructions to run it after code changes; now proceeding to fetch origin and the PR branch.
>git fetch origin(no output)Fetching PR branch
>git fetch origin pull/826/head:pr-826-upstreamFrom https://github.com/badlogic/pi-mono
* [new ref] refs/pull/826/head -> pr-826-upstreamChecking out and rebasing branch
>git checkout pr-826-upstreamSwitched to branch 'pr-826-upstream'Rebasing branch
>git rebase origin/mainRebasing (1/4)
Rebasing (2/4)
Rebasing (3/4)
Rebasing (4/4)
Successfully rebased and updated refs/heads/pr-826-upstream.Rebasing branch to include PR commits
>git checkout pr-826Switched to branch 'pr-826'Rebasing branch onto upstream PR
>git rebase pr-826-upstreamRebasing (1/1)
Successfully rebased and updated refs/heads/pr-826.Confirming merge readiness
The branch is correctly rebased with our commit on top of PR commits; next I’ll ensure the working tree is clean before merging into main.
>git status -sb## pr-826Updating main branch before merge
>git checkout mainSwitched to branch 'main'
Your branch is up to date with 'origin/main'.Updating local branch with rebase
>git pull --rebaseAlready up to date.Merging feature branch
Planning to fast-forward merge the feature branch into main since main is behind and the feature branch has new commits.
>git merge pr-826Updating c63f33d8..a67f6f99
Fast-forward
packages/coding-agent/CHANGELOG.md | 1 +
.../coding-agent/src/core/extensions/runner.ts | 82 +++++++++-----
.../src/modes/interactive/interactive-mode.ts | 4 +-
.../coding-agent/test/extensions-runner.test.ts | 124 ++++++++++++++++++++-
4 files changed, 182 insertions(+), 29 deletions(-)Verifying git status before push
>git status -sb## main...origin/main [ahead 5]Pushing code changes
>git pushTo https://github.com/badlogic/pi-mono.git
c63f33d8..a67f6f99 main -> mainPlanning PR state verification
I'll verify that the PR closes automatically by checking its state using the GitHub CLI.
>gh pr view 826 --json state,merged,mergedAt,urlUnknown JSON field: "merged"
Available fields:
additions
assignees
author
autoMergeRequest
baseRefName
baseRefOid
body
changedFiles
closed
closedAt
closingIssuesReferences
comments
commits
createdAt
deletions
files
fullDatabaseId
headRefName
headRefOid
headRepository
headRepositoryOwner
id
isCrossRepository
isDraft
labels
latestReviews
maintainerCanModify
mergeCommit
mergeStateStatus
mergeable
mergedAt
mergedBy
milestone
number
potentialMergeCommit
projectCards
projectItems
reactionGroups
reviewDecision
reviewRequests
reviews
state
statusCheckRollup
title
updatedAt
url
Command exited with code 1Confirming pull request field availability
I verified that the PR field "mergedAt" exists but there's no "merged" field, so I'll rely on "state" and "mergedAt" to determine PR status.
>gh pr view 826 --json state,mergedAt,closedAt,url{"closedAt":null,"mergedAt":null,"state":"OPEN","url":"https://github.com/badlogic/pi-mono/pull/826"}Clarifying PR auto-close behavior and comment style
The PR remains open because merging outside GitHub doesn't auto-close it, so we'll manually close it with gh pr close and add a direct, technical comment in the user's tone, such as "Merged into main with changelog entry added. Thanks."
Deciding to close PR with manual merge comment
Since the PR was merged manually outside GitHub, I'll close it using gh pr close with a comment referencing the manual merge by commit hash to clarify the status without triggering automatic merge conflicts.
>git log --oneline -1a67f6f99 chore(coding-agent): add changelog entry for shortcutsPreparing concise PR close comment
>gh pr close 826 --comment "Merged in main via a67f6f99. Added changelog entry. Thanks @richardgill."✓ Closed pull request badlogic/pi-mono#826 (Allow extensions to override default built-in keybindings which have been remapped)Checking pull request state
>gh pr view 826 --json state,closedAt,url{"closedAt":"2026-01-18T18:38:46Z","state":"CLOSED","url":"https://github.com/badlogic/pi-mono/pull/826"}Summarizing final actions and tests
I’m preparing a concise final summary noting the changelog update in packages/coding-agent/CHANGELOG.md, that PR 826 was fetched, rebased, merged, pushed, and closed with a comment, and that tests via npm run check ran automatically through git hooks.
Summary:
## [Unreleased] in packages/coding-agent/CHANGELOG.md for PR 826.main, pushed, and closed the PR with a comment.Tests:
npm run check (ran automatically during commit)how can we remove this work tree? /Users/badlogic/workspaces/pi-mono-pr810
Providing git worktree removal steps
I will explain how to remove a git worktree by using the git worktree remove command followed by manually deleting the associated directory if needed, avoiding any execution of commands.
Use git worktree remove from the main repo, not inside the worktree:
cd /Users/badlogic/workspaces/pi-monogit worktree remove /Users/badlogic/workspaces/pi-mono-pr810If it says the worktree is in use, make sure no process is using it, then retry. If it still fails and you are sure it is safe, use:
git worktree remove --force /Users/badlogic/workspaces/pi-mono-pr810As a last resort, remove the directory after the worktree is removed from git:
rm -rf /Users/badlogic/workspaces/pi-mono-pr810do it
Preparing to remove git worktree using bash
>cd /Users/badlogic/workspaces/pi-mono && git worktree remove /Users/badlogic/workspaces/pi-mono-pr810(no output)Removed worktree /Users/badlogic/workspaces/pi-mono-pr810.
cd /Users/badlogic/workspaces/pi-monogit worktree remove /Users/badlogic/workspaces/pi-mono-pr810git worktree remove --force /Users/badlogic/workspaces/pi-mono-pr810rm -rf /Users/badlogic/workspaces/pi-mono-pr810