mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
fix(coding-agent): pass host keybinding hints to extension renderers (#11630)
Fixes #4748. ## Problem Extensions that import `keyText`/`keyHint` from `@earendil-works/pi-coding-agent` can resolve their own copy of the package (their own `node_modules`, or the bundled `cli.js` vs `dist/index.js`). That copy's keybindings are never set by the host, so it falls back to a manager with only `tui.*` bindings, and `keyHint("app.tools.expand", "to expand")` renders as ` to expand`. ## Change The host passes its own `keyText` and `keyHint` to extension renderers, the same way it passes `theme`: - `ToolRenderContext` (`renderCall`/`renderResult` context): `context.keyHint(...)` - `MessageRenderOptions` and `EntryRenderOptions`: `options.keyHint(...)` All three extend a new exported `KeybindingHints` interface. The docs and the `debug-provider` example now use the injected functions and warn against importing them in renderers. Not done: - Moving the `pi-tui` keybindings singleton to `globalThis`. Per the discussion in #7011, extension-facing state should be host-owned and passed through the extension API instead of shared through process-wide module state. - `ctx.ui.keyText`/`keyHint`. No reported case needs them outside renderers, RPC and print modes never install app keybindings, and adding them to `ExtensionUIContext` would break SDK hosts that implement it. ## Tests - `test/extension-keybinding-hints.test.ts`: tool, message, and entry renderers format a user-overridden `app.tools.expand` binding. - `npm run check` passes.
This commit is contained in:
parent
8bc94b19ff
commit
9376c8d17c
13 changed files with 130 additions and 18 deletions
|
|
@ -8,6 +8,7 @@
|
|||
- `pi.exec()` accepts a `maxBuffer` option that limits stdout and stderr combined, and its result has a `truncated` field that is true when output exceeded the limit ([#11360](https://github.com/earendil-works/pi/issues/11360))
|
||||
- Device-code login (for example GitHub Copilot) now opens the verification page and copies the code when you press Enter. Providers can turn either action off with `openBrowserOnConfirm: false` or `copyCodeOnConfirm: false` ([#9282](https://github.com/earendil-works/pi/issues/9282), [#9301](https://github.com/earendil-works/pi/pull/9301))
|
||||
- `pi update` can update Pi through a wrapper or another package manager. Set `PI_SELF_UPDATE_COMMAND` (for example `mise use -g npm:{packageName}@{version}`) or the global `selfUpdateCommand` setting; `pi update` runs it when pi.dev reports a newer release and fills `{packageName}`, `{version}`, and `{installSpec}` from that release. See [Custom update command](docs/cli.md#custom-update-command) ([#7756](https://github.com/earendil-works/pi/issues/7756), [#5607](https://github.com/earendil-works/pi/issues/5607))
|
||||
- Extensions can format the user's configured keybindings with `keyText()` and `keyHint()` from the tool render context and the message and entry renderer options. Importing these functions from `@earendil-works/pi-coding-agent` can resolve the extension's own copy of the package, which does not see pi's keybindings, so hints such as `keyHint("app.tools.expand", "to expand")` rendered without the key. The docs now recommend the injected functions ([#4748](https://github.com/earendil-works/pi/issues/4748))
|
||||
|
||||
### Changed
|
||||
|
||||
|
|
|
|||
|
|
@ -2520,6 +2520,7 @@ pi.registerTool({
|
|||
- `toolCallId`, `cwd`, `executionStarted`, `argsComplete`, `isPartial`, `expanded`, `showImages`, `isError`
|
||||
- `durationMs` - how long execution took, from the final result; `undefined` while running
|
||||
- `outputPad` - horizontal padding from the `outputPad` setting; renderers with `renderShell: "self"` apply it themselves
|
||||
- `keyText()`, `keyHint()` - format the user's configured keybindings, see [Keybinding Hints](#keybinding-hints)
|
||||
|
||||
Use `context.state` for cross-slot shared state. Keep slot-local caches on the returned component instance when you want to reuse and mutate the same component across renders.
|
||||
|
||||
|
|
@ -2570,24 +2571,24 @@ If a slot intentionally has no visible content, return an empty `Component` such
|
|||
|
||||
#### Keybinding Hints
|
||||
|
||||
Use `keyHint()` to display keybinding hints that respect the active keybinding configuration:
|
||||
Use `keyHint()` and `keyText()` from the render context to display keybinding hints that respect the user's keybinding configuration:
|
||||
|
||||
```typescript
|
||||
import { keyHint } from "@earendil-works/pi-coding-agent";
|
||||
|
||||
renderResult(result, { expanded }, theme, context) {
|
||||
let text = theme.fg("success", "✓ Done");
|
||||
if (!expanded) {
|
||||
text += ` (${keyHint("app.tools.expand", "to expand")})`;
|
||||
text += ` (${context.keyHint("app.tools.expand", "to expand")})`;
|
||||
}
|
||||
return new Text(text, 0, 0);
|
||||
}
|
||||
```
|
||||
|
||||
Available functions:
|
||||
- `keyHint(keybinding, description)` - Formats a configured keybinding id such as `"app.tools.expand"` or `"tui.select.confirm"`
|
||||
- `keyText(keybinding)` - Returns the raw configured key text for a keybinding id
|
||||
- `rawKeyHint(key, description)` - Format a raw key string
|
||||
The same functions are available as `options.keyHint()`/`options.keyText()` in message and entry renderers:
|
||||
|
||||
- `keyHint(keybinding, description)` - Formats a configured keybinding id such as `"app.tools.expand"` or `"tui.select.confirm"`, followed by the description
|
||||
- `keyText(keybinding)` - Returns the raw configured key text for a keybinding id, or `""` if the id has no keys
|
||||
|
||||
In renderers, use these instead of importing `keyHint()` or `keyText()` from `@earendil-works/pi-coding-agent`. An extension can resolve its own copy of the package, for example from its own `node_modules`, and that copy does not see pi's configured keybindings: `app.*` ids then format as `""`. `rawKeyHint(key, description)` formats a literal key string and is safe to import.
|
||||
|
||||
Use namespaced keybinding ids:
|
||||
- Coding-agent ids use the `app.*` namespace, for example `app.tools.expand`, `app.editor.external`, `app.session.rename`
|
||||
|
|
@ -3144,6 +3145,8 @@ pi.registerMessageRenderer("my-extension", (message, options, theme) => {
|
|||
|
||||
if (expanded && message.details) {
|
||||
text += "\n" + theme.fg("dim", JSON.stringify(message.details, null, 2));
|
||||
} else if (message.details) {
|
||||
text += ` (${options.keyHint("app.tools.expand", "to expand")})`;
|
||||
}
|
||||
|
||||
return new Text(text, outputPad, 0);
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@
|
|||
* as expandable custom entries.
|
||||
*/
|
||||
|
||||
import { type ExtensionAPI, keyHint } from "@earendil-works/pi-coding-agent";
|
||||
import type { ExtensionAPI } from "@earendil-works/pi-coding-agent";
|
||||
import { Box, Text } from "@earendil-works/pi-tui";
|
||||
|
||||
const ENTRY_TYPE = "debug-provider-events";
|
||||
|
|
@ -24,7 +24,7 @@ export default function (pi: ExtensionAPI) {
|
|||
let activeEvents: unknown[] | undefined;
|
||||
let completedEntry: ProviderDebugEntry | undefined;
|
||||
|
||||
pi.registerEntryRenderer<ProviderDebugEntry>(ENTRY_TYPE, (entry, { expanded }, theme) => {
|
||||
pi.registerEntryRenderer<ProviderDebugEntry>(ENTRY_TYPE, (entry, { expanded, keyHint }, theme) => {
|
||||
const data = entry.data;
|
||||
if (!data) return new Text(theme.fg("warning", "[provider debug] Missing event data"), 0, 0);
|
||||
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@
|
|||
|
||||
import type { ImageContent, TextContent } from "@earendil-works/pi-ai";
|
||||
import type { Component } from "@earendil-works/pi-tui";
|
||||
import { keyHint, keyText } from "../../modes/interactive/components/keybinding-hints.ts";
|
||||
import type { Theme } from "../../modes/interactive/theme/theme.ts";
|
||||
import type { ToolRenderContext, ToolRenderers } from "../extensions/types.ts";
|
||||
import { ansiLinesToHtml } from "./ansi-to-html.ts";
|
||||
|
|
@ -94,6 +95,8 @@ export function createToolHtmlRenderer(deps: ToolHtmlRendererDeps): ToolHtmlRend
|
|||
isError,
|
||||
durationMs: undefined,
|
||||
outputPad: 1,
|
||||
keyText,
|
||||
keyHint,
|
||||
};
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -109,6 +109,7 @@ export type {
|
|||
InputEvent,
|
||||
InputEventResult,
|
||||
InputSource,
|
||||
KeybindingHints,
|
||||
KeybindingsManager,
|
||||
LoadExtensionsResult,
|
||||
LsToolCallEvent,
|
||||
|
|
|
|||
|
|
@ -49,6 +49,7 @@ import type {
|
|||
Component,
|
||||
EditorComponent,
|
||||
EditorTheme,
|
||||
Keybinding,
|
||||
KeyId,
|
||||
OverlayHandle,
|
||||
OverlayOptions,
|
||||
|
|
@ -143,6 +144,19 @@ export interface WorkingIndicatorOptions {
|
|||
export type AutocompleteProviderFactory = (current: AutocompleteProvider) => AutocompleteProvider;
|
||||
export type EditorFactory = (tui: TUI, theme: EditorTheme, keybindings: KeybindingsManager) => EditorComponent;
|
||||
|
||||
/**
|
||||
* Formats the host's configured keybindings for display.
|
||||
* Extensions should use these instead of importing `keyText`/`keyHint`: an import may resolve to another copy of
|
||||
* pi-coding-agent, which does not see the host's keybindings.
|
||||
*/
|
||||
export interface KeybindingHints {
|
||||
/** Configured keys for a keybinding id, for example `"ctrl+o"`. Empty when the id has no keys. */
|
||||
keyText(keybinding: Keybinding): string;
|
||||
|
||||
/** Configured keys followed by a description, styled as a hint, for example `"ctrl+o to expand"`. */
|
||||
keyHint(keybinding: Keybinding, description: string): string;
|
||||
}
|
||||
|
||||
/**
|
||||
* UI context for extensions to request interactive UI.
|
||||
* Each mode (interactive, RPC, print) provides its own implementation.
|
||||
|
|
@ -470,7 +484,7 @@ export interface ToolRenderResultOptions {
|
|||
}
|
||||
|
||||
/** Context passed to tool renderers. */
|
||||
export interface ToolRenderContext<TState = any, TArgs = any> {
|
||||
export interface ToolRenderContext<TState = any, TArgs = any> extends KeybindingHints {
|
||||
/** Current tool call arguments. Shared across call/result renders for the same tool call. */
|
||||
args: TArgs;
|
||||
/** Unique id for this tool execution. Stable across call/result renders for the same tool call. */
|
||||
|
|
@ -1538,7 +1552,7 @@ export interface SessionBeforeTreeResult {
|
|||
// Message and Entry Rendering
|
||||
// ============================================================================
|
||||
|
||||
export interface MessageRenderOptions {
|
||||
export interface MessageRenderOptions extends KeybindingHints {
|
||||
expanded: boolean;
|
||||
/** Horizontal padding configured by the outputPad setting. */
|
||||
outputPad: number;
|
||||
|
|
@ -1556,7 +1570,7 @@ export interface MarkdownTransformContext {
|
|||
|
||||
export type MarkdownTransformer = (markdown: string, context: MarkdownTransformContext) => string;
|
||||
|
||||
export interface EntryRenderOptions {
|
||||
export interface EntryRenderOptions extends KeybindingHints {
|
||||
expanded: boolean;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -118,6 +118,7 @@ export type {
|
|||
InputEvent,
|
||||
InputEventResult,
|
||||
InputSource,
|
||||
KeybindingHints,
|
||||
KeybindingsManager,
|
||||
LoadExtensionsResult,
|
||||
LsToolCallEvent,
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ import { Box, Container, Spacer, Text } from "@earendil-works/pi-tui";
|
|||
import type { EntryRenderer } from "../../../core/extensions/types.ts";
|
||||
import type { CustomEntry } from "../../../core/session-manager.ts";
|
||||
import { theme } from "../theme/theme.ts";
|
||||
import { keyHint, keyText } from "./keybinding-hints.ts";
|
||||
|
||||
/**
|
||||
* Component that renders a custom session entry from extensions.
|
||||
|
|
@ -50,7 +51,7 @@ export class CustomEntryComponent extends Container {
|
|||
|
||||
let component: Component | undefined;
|
||||
try {
|
||||
component = this.renderer(this.entry, { expanded: this._expanded }, theme);
|
||||
component = this.renderer(this.entry, { expanded: this._expanded, keyText, keyHint }, theme);
|
||||
} catch (error) {
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
const box = new Box(this.outputPad, 1, (text) => theme.bg("customMessageBg", text));
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import { Box, Container, Markdown, type MarkdownTheme, Spacer, Text } from "@ear
|
|||
import type { MessageRenderer } from "../../../core/extensions/types.ts";
|
||||
import type { CustomMessage } from "../../../core/messages.ts";
|
||||
import { getMarkdownTheme, theme } from "../theme/theme.ts";
|
||||
import { keyHint, keyText } from "./keybinding-hints.ts";
|
||||
|
||||
/**
|
||||
* Component that renders a custom message entry from extensions.
|
||||
|
|
@ -70,7 +71,7 @@ export class CustomMessageComponent extends Container {
|
|||
try {
|
||||
const component = this.customRenderer(
|
||||
this.message,
|
||||
{ expanded: this._expanded, outputPad: this.outputPad },
|
||||
{ expanded: this._expanded, outputPad: this.outputPad, keyText, keyHint },
|
||||
theme,
|
||||
);
|
||||
if (component) {
|
||||
|
|
|
|||
|
|
@ -18,7 +18,7 @@ export type { ToolRenderers };
|
|||
import { formatToolCallWithArgs, getTextOutput as getRenderedTextOutput } from "../../../core/tools/render-utils.ts";
|
||||
import { ensurePngTranscoder } from "../../../utils/image-convert.ts";
|
||||
import { theme } from "../theme/theme.ts";
|
||||
import { keyHint } from "./keybinding-hints.ts";
|
||||
import { keyHint, keyText } from "./keybinding-hints.ts";
|
||||
|
||||
const FALLBACK_PREVIEW_LINES = 10;
|
||||
|
||||
|
|
@ -138,6 +138,8 @@ export class ToolExecutionComponent extends Container {
|
|||
isError: this.result?.isError ?? false,
|
||||
durationMs: this.isPartial ? undefined : this.result?.durationMs,
|
||||
outputPad: this.outputPad,
|
||||
keyText,
|
||||
keyHint,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import { afterEach, beforeAll, describe, expect, it, vi } from "vitest";
|
|||
import type { ToolRenderContext } from "../src/core/extensions/types.ts";
|
||||
import { codemodeRenderers } from "../src/extensions/codemode/renderer.ts";
|
||||
import type { CodemodeToolDetails } from "../src/extensions/codemode/tool.ts";
|
||||
import { keyHint, keyText } from "../src/modes/interactive/components/keybinding-hints.ts";
|
||||
import { ToolExecutionComponent } from "../src/modes/interactive/components/tool-execution.ts";
|
||||
import { initTheme, theme } from "../src/modes/interactive/theme/theme.ts";
|
||||
|
||||
|
|
@ -29,6 +30,8 @@ function render(
|
|||
isError,
|
||||
durationMs: undefined,
|
||||
outputPad: 1,
|
||||
keyText,
|
||||
keyHint,
|
||||
} satisfies ToolRenderContext;
|
||||
const component = codemodeRenderers.renderResult?.(
|
||||
result,
|
||||
|
|
|
|||
|
|
@ -23,7 +23,7 @@ describe("CustomMessageComponent", () => {
|
|||
};
|
||||
const component = new CustomMessageComponent(message, renderer, undefined, 1);
|
||||
|
||||
expect(optionsSeen).toEqual([{ expanded: false, outputPad: 1 }]);
|
||||
expect(optionsSeen).toMatchObject([{ expanded: false, outputPad: 1 }]);
|
||||
expect(
|
||||
component
|
||||
.render(40)
|
||||
|
|
@ -33,7 +33,7 @@ describe("CustomMessageComponent", () => {
|
|||
|
||||
component.setOutputPad(0);
|
||||
|
||||
expect(optionsSeen.at(-1)).toEqual({ expanded: false, outputPad: 0 });
|
||||
expect(optionsSeen.at(-1)).toMatchObject({ expanded: false, outputPad: 0 });
|
||||
expect(
|
||||
component
|
||||
.render(40)
|
||||
|
|
|
|||
|
|
@ -0,0 +1,82 @@
|
|||
// Issue #4748: extensions that import keyText/keyHint can resolve their own package copy, which does not see the
|
||||
// host's keybindings. The host passes its own keyText/keyHint to renderers instead.
|
||||
import { setKeybindings, Text, type TUI } from "@earendil-works/pi-tui";
|
||||
import { Type } from "typebox";
|
||||
import { beforeAll, describe, expect, test } from "vitest";
|
||||
import type { KeybindingHints, ToolDefinition } from "../src/core/extensions/types.ts";
|
||||
import { KeybindingsManager } from "../src/core/keybindings.ts";
|
||||
import type { CustomMessage } from "../src/core/messages.ts";
|
||||
import type { CustomEntry } from "../src/core/session-manager.ts";
|
||||
import { CustomEntryComponent } from "../src/modes/interactive/components/custom-entry.ts";
|
||||
import { CustomMessageComponent } from "../src/modes/interactive/components/custom-message.ts";
|
||||
import { ToolExecutionComponent } from "../src/modes/interactive/components/tool-execution.ts";
|
||||
import { initTheme } from "../src/modes/interactive/theme/theme.ts";
|
||||
import { stripAnsi } from "../src/utils/ansi.ts";
|
||||
|
||||
function hintText(hints: KeybindingHints): string {
|
||||
return `${hints.keyText("app.tools.expand")} | ${stripAnsi(hints.keyHint("app.tools.expand", "to expand"))}`;
|
||||
}
|
||||
|
||||
const EXPECTED = "ctrl+e | ctrl+e to expand";
|
||||
|
||||
describe("host keybinding hints for extensions", () => {
|
||||
beforeAll(() => {
|
||||
initTheme("dark");
|
||||
// A user override proves the hints read the host's configured keybindings, not the defaults.
|
||||
setKeybindings(new KeybindingsManager({ "app.tools.expand": "ctrl+e" }));
|
||||
});
|
||||
|
||||
test("tool renderers receive keyText and keyHint in the render context", () => {
|
||||
const seen: string[] = [];
|
||||
const toolDefinition: ToolDefinition = {
|
||||
name: "custom_tool",
|
||||
label: "custom_tool",
|
||||
description: "custom tool",
|
||||
parameters: Type.Any(),
|
||||
execute: async () => ({ content: [{ type: "text", text: "ok" }], details: {} }),
|
||||
renderCall: (_args, _theme, context) => {
|
||||
seen.push(hintText(context));
|
||||
return new Text("call", 0, 0);
|
||||
},
|
||||
};
|
||||
const tui = { requestRender: () => {} } as unknown as TUI;
|
||||
new ToolExecutionComponent("custom_tool", "tool-1", {}, {}, toolDefinition, tui, process.cwd()).render(80);
|
||||
|
||||
expect(seen.at(-1)).toBe(EXPECTED);
|
||||
});
|
||||
|
||||
test("message renderers receive keyText and keyHint in the render options", () => {
|
||||
const seen: string[] = [];
|
||||
const message: CustomMessage = {
|
||||
role: "custom",
|
||||
customType: "test",
|
||||
content: "custom",
|
||||
display: true,
|
||||
timestamp: Date.now(),
|
||||
};
|
||||
new CustomMessageComponent(message, (_message, options) => {
|
||||
seen.push(hintText(options));
|
||||
return new Text("custom", 0, 0);
|
||||
});
|
||||
|
||||
expect(seen).toEqual([EXPECTED]);
|
||||
});
|
||||
|
||||
test("entry renderers receive keyText and keyHint in the render options", () => {
|
||||
const seen: string[] = [];
|
||||
const entry: CustomEntry = {
|
||||
type: "custom",
|
||||
id: "entry-1",
|
||||
parentId: null,
|
||||
timestamp: new Date().toISOString(),
|
||||
customType: "test",
|
||||
data: undefined,
|
||||
};
|
||||
new CustomEntryComponent(entry, (_entry, options) => {
|
||||
seen.push(hintText(options));
|
||||
return new Text("custom", 0, 0);
|
||||
});
|
||||
|
||||
expect(seen).toEqual([EXPECTED]);
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue