Merge PR #7774: fix(tui): gate built-in renderers by tool provenance (@roboomp)
This commit is contained in:
@@ -63,6 +63,9 @@
|
||||
### Fixed
|
||||
|
||||
- Fixed session-tree rows rendering as bare bullets: bookkeeping entries (title changes, credential pins, mode and service-tier changes, TTSR injections, reset boundaries, session init) had no display text at all, so they drew as empty rows. They are now hidden in the default view like other settings entries, and labelled with what they recorded in `all` mode.
|
||||
### Fixed
|
||||
|
||||
- Fixed extension and custom tools inheriting a same-named built-in TUI renderer, which could replace successful result content with incorrect built-in status text ([#7770](https://github.com/can1357/oh-my-pi/issues/7770)).
|
||||
|
||||
## [17.2.9] - 2026-08-05
|
||||
|
||||
|
||||
@@ -153,7 +153,7 @@ export async function renderGalleryState(
|
||||
const component = new ToolExecutionComponent(
|
||||
componentName,
|
||||
streamingArgs,
|
||||
{ showImages: false },
|
||||
{ showImages: false, useBuiltInRenderer: !fixture.customRendered },
|
||||
tool,
|
||||
ui,
|
||||
getProjectDir(),
|
||||
|
||||
@@ -123,6 +123,8 @@ export interface AgentHubDeps {
|
||||
ui?: TUI;
|
||||
/** Tool lookup for transcript renderers (labels, custom render functions). */
|
||||
getTool?: (name: string) => AgentTool | undefined;
|
||||
/** Whether the active registry entry came from a built-in factory. */
|
||||
isBuiltInTool?: (name: string) => boolean;
|
||||
/** Extension message renderers for custom messages in the transcript. */
|
||||
getMessageRenderer?: (customType: string) => MessageRenderer | undefined;
|
||||
/** Cwd used by tool renderers for path shortening; defaults to the project dir. */
|
||||
@@ -206,6 +208,7 @@ export class AgentHubOverlayComponent extends Container implements SelectListMou
|
||||
// Transcript-viewer launch deps (passed through to AgentTranscriptViewer).
|
||||
#ui: TUI;
|
||||
#getTool: ((name: string) => AgentTool | undefined) | undefined;
|
||||
#isBuiltInTool: ((name: string) => boolean) | undefined;
|
||||
#getMessageRenderer: ((customType: string) => MessageRenderer | undefined) | undefined;
|
||||
#cwd: string;
|
||||
#hideThinkingBlock: (() => boolean) | undefined;
|
||||
@@ -238,6 +241,7 @@ export class AgentHubOverlayComponent extends Container implements SelectListMou
|
||||
requestComponentRender: () => deps.requestRender(),
|
||||
} as unknown as TUI);
|
||||
this.#getTool = deps.getTool;
|
||||
this.#isBuiltInTool = deps.isBuiltInTool;
|
||||
this.#getMessageRenderer = deps.getMessageRenderer;
|
||||
this.#cwd = deps.cwd ?? getProjectDir();
|
||||
this.#hideThinkingBlock = deps.hideThinkingBlock;
|
||||
@@ -364,6 +368,7 @@ export class AgentHubOverlayComponent extends Container implements SelectListMou
|
||||
lifecycle: this.#remote ? undefined : this.#lifecycle,
|
||||
ui: this.#ui,
|
||||
getTool: this.#getTool,
|
||||
isBuiltInTool: this.#isBuiltInTool,
|
||||
getMessageRenderer: this.#getMessageRenderer,
|
||||
cwd: this.#cwd,
|
||||
hideThinkingBlock: this.#hideThinkingBlock,
|
||||
|
||||
@@ -43,6 +43,8 @@ export interface AgentTranscriptViewerDeps {
|
||||
lifecycle?: () => AgentLifecycleManager;
|
||||
ui: TUI;
|
||||
getTool?: (name: string) => AgentTool | undefined;
|
||||
/** Whether the active registry entry came from a built-in factory. */
|
||||
isBuiltInTool?: (name: string) => boolean;
|
||||
getMessageRenderer?: (customType: string) => MessageRenderer | undefined;
|
||||
cwd: string;
|
||||
hideThinkingBlock?: () => boolean;
|
||||
@@ -161,6 +163,7 @@ export class AgentTranscriptViewer implements Component {
|
||||
this.#builder = new ChatTranscriptBuilder({
|
||||
ui: deps.ui,
|
||||
getTool: deps.getTool,
|
||||
isBuiltInTool: deps.isBuiltInTool,
|
||||
getMessageRenderer: deps.getMessageRenderer,
|
||||
cwd: deps.cwd,
|
||||
hideThinkingBlock: deps.hideThinkingBlock,
|
||||
|
||||
@@ -61,6 +61,8 @@ import { CollapsedSyntheticMessageComponent, UserMessageComponent } from "./user
|
||||
export interface ChatTranscriptBuilderDeps {
|
||||
ui: TUI;
|
||||
getTool?: (name: string) => AgentTool | undefined;
|
||||
/** Whether the active registry entry came from a built-in factory. */
|
||||
isBuiltInTool?: (name: string) => boolean;
|
||||
getMessageRenderer?: (customType: string) => MessageRenderer | undefined;
|
||||
cwd: string;
|
||||
hideThinkingBlock?: () => boolean;
|
||||
@@ -392,6 +394,7 @@ export class ChatTranscriptBuilder {
|
||||
content.name,
|
||||
content.arguments,
|
||||
{
|
||||
useBuiltInRenderer: this.deps.isBuiltInTool?.(content.name) ?? true,
|
||||
// Stable ids and Kitty placeholder cells keep images anchored
|
||||
// while the transcript viewport scrolls and reflows.
|
||||
showImages: settings.get("terminal.showImages"),
|
||||
|
||||
@@ -23,7 +23,7 @@ import { formatDefaultToolExecution } from "../../tools/default-renderer";
|
||||
import { EVAL_DEFAULT_PREVIEW_LINES } from "../../tools/eval";
|
||||
import { isWaitingPollDetails } from "../../tools/hub";
|
||||
import { formatStatusIcon, replaceTabs, resolveImageOptions } from "../../tools/render-utils";
|
||||
import { type FirstResultViewportRepaint, toolRenderers } from "../../tools/renderers";
|
||||
import { type FirstResultViewportRepaint, type ToolRenderer, toolRenderers } from "../../tools/renderers";
|
||||
import { TODO_STRIKE_TOTAL_FRAMES, type TodoToolDetails } from "../../tools/todo";
|
||||
import type { XdevState } from "../../tools/xdev";
|
||||
import { isFramedBlockComponent, markFramedBlockComponent, renderStatusLine, WidthAwareText } from "../../tui";
|
||||
@@ -226,6 +226,8 @@ export interface ToolExecutionOptions {
|
||||
/** Session-persistent edit clipboard register, forked per preview frame. */
|
||||
clipboard?: Clipboard;
|
||||
showImages?: boolean; // default: true (only used if terminal supports images)
|
||||
/** Allow the name-keyed renderer registry only when the active tool is the built-in implementation. */
|
||||
useBuiltInRenderer?: boolean;
|
||||
editFuzzyThreshold?: number;
|
||||
editAllowFuzzy?: boolean;
|
||||
/** Live-region probe used to settle detached task progress once the block
|
||||
@@ -312,6 +314,7 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac
|
||||
// forcing the common image-free result to re-shape on every resize tick.
|
||||
#renderedImageCount = 0;
|
||||
#tool?: AgentTool;
|
||||
#renderer?: ToolRenderer;
|
||||
#ui: ToolExecutionUi;
|
||||
#cwd: string;
|
||||
#result?: {
|
||||
@@ -398,6 +401,7 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac
|
||||
super();
|
||||
this.#toolName = toolName;
|
||||
this.#toolLabel = tool?.label ?? toolName;
|
||||
this.#renderer = options.useBuiltInRenderer === false ? undefined : toolRenderers[toolName];
|
||||
this.#showImages = options.showImages ?? true;
|
||||
this.#editFuzzyThreshold = options.editFuzzyThreshold;
|
||||
this.#editAllowFuzzy = options.editAllowFuzzy;
|
||||
@@ -418,10 +422,9 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac
|
||||
this.#contentBox = new Box(0, 1);
|
||||
this.#contentText = new WidthAwareText(contentWidth => this.#renderDefaultCard(contentWidth), 1, 1);
|
||||
|
||||
// Use Box for custom tools or built-in tools that have renderers
|
||||
const hasRenderer = toolName in toolRenderers;
|
||||
// Use Box for custom tools or built-in tools with rich renderers.
|
||||
const hasCustomRenderer = !!(tool?.renderCall || tool?.renderResult);
|
||||
this.#usesContentBox = hasCustomRenderer || hasRenderer;
|
||||
this.#usesContentBox = hasCustomRenderer || this.#renderer !== undefined;
|
||||
if (this.#usesContentBox) {
|
||||
this.addChild(this.#contentBox);
|
||||
} else {
|
||||
@@ -668,12 +671,7 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac
|
||||
const isStreamingArgs = !this.#argsComplete && (isEditLikeToolName(this.#toolName) || this.#toolName === "write");
|
||||
const isBackgroundAsyncRunning =
|
||||
(this.#result?.details as { async?: { state?: string } } | undefined)?.async?.state === "running";
|
||||
const renderer = toolRenderers[this.#toolName] as
|
||||
| {
|
||||
animatedPendingPreview?: boolean | ((args: unknown) => boolean);
|
||||
animatedPartialResult?: boolean | ((args: unknown) => boolean);
|
||||
}
|
||||
| undefined;
|
||||
const renderer = this.#renderer;
|
||||
const pendingAnimation = renderer?.animatedPendingPreview;
|
||||
const partialAnimation = renderer?.animatedPartialResult;
|
||||
const pendingCallConsumesSpinner =
|
||||
@@ -933,7 +931,7 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac
|
||||
|
||||
#rendererFlag(name: "forceResultViewportRepaintOnSettle"): boolean {
|
||||
const toolValue = (this.#tool as Record<string, unknown> | undefined)?.[name];
|
||||
const rendererValue = toolRenderers[this.#toolName]?.[name];
|
||||
const rendererValue = this.#renderer?.[name];
|
||||
return toolValue === true || (toolValue === undefined && rendererValue === true);
|
||||
}
|
||||
|
||||
@@ -948,8 +946,7 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac
|
||||
if (this.#result !== undefined) return false;
|
||||
const toolValue = (this.#tool as { forceFirstResultViewportRepaint?: FirstResultViewportRepaint } | undefined)
|
||||
?.forceFirstResultViewportRepaint;
|
||||
const value =
|
||||
toolValue !== undefined ? toolValue : toolRenderers[this.#toolName]?.forceFirstResultViewportRepaint;
|
||||
const value = toolValue !== undefined ? toolValue : this.#renderer?.forceFirstResultViewportRepaint;
|
||||
if (typeof value === "function") return value(this.#args, this.#renderState);
|
||||
return value === true;
|
||||
}
|
||||
@@ -1101,9 +1098,9 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac
|
||||
const customFramed = this.#contentBox.children.some(isFramedBlockComponent);
|
||||
this.#contentBox.setPaddingX(customFramed ? 0 : 1);
|
||||
this.#contentBox.setBgFn(customFramed ? undefined : stateBgFn);
|
||||
} else if (this.#toolName in toolRenderers) {
|
||||
// Built-in tools with renderers
|
||||
const renderer = toolRenderers[this.#toolName];
|
||||
} else if (this.#renderer) {
|
||||
// The active registry entry is a built-in tool with a rich renderer.
|
||||
const renderer = this.#renderer;
|
||||
|
||||
// Clean up previous multi-file boxes
|
||||
for (const box of this.#multiFileBoxes) {
|
||||
|
||||
@@ -1073,6 +1073,7 @@ export class EventController {
|
||||
content.name,
|
||||
renderArgs,
|
||||
{
|
||||
useBuiltInRenderer: this.ctx.viewSession.hasBuiltInTool(content.name),
|
||||
snapshots: getFileSnapshotStore(this.ctx.viewSession),
|
||||
clipboard: getEditClipboard(this.ctx.viewSession),
|
||||
showImages: settings.get("terminal.showImages"),
|
||||
@@ -1322,6 +1323,7 @@ export class EventController {
|
||||
event.toolName,
|
||||
event.args,
|
||||
{
|
||||
useBuiltInRenderer: this.ctx.viewSession.hasBuiltInTool(event.toolName),
|
||||
snapshots: getFileSnapshotStore(this.ctx.viewSession),
|
||||
clipboard: getEditClipboard(this.ctx.viewSession),
|
||||
showImages: settings.get("terminal.showImages"),
|
||||
|
||||
@@ -2026,6 +2026,7 @@ export class SelectorController {
|
||||
remote: this.ctx.collabGuest?.hubRemote,
|
||||
ui: this.ctx.ui,
|
||||
getTool: name => this.ctx.session.getToolByName(name),
|
||||
isBuiltInTool: name => this.ctx.session.hasBuiltInTool(name),
|
||||
getMessageRenderer: type => this.ctx.session.extensionRunner?.getMessageRenderer(type),
|
||||
cwd: this.ctx.sessionManager.getCwd(),
|
||||
hideThinkingBlock: () => this.ctx.effectiveHideThinkingBlock,
|
||||
|
||||
@@ -500,6 +500,7 @@ export class UiHelpers {
|
||||
content.name,
|
||||
renderArgs,
|
||||
{
|
||||
useBuiltInRenderer: this.ctx.viewSession.hasBuiltInTool(content.name),
|
||||
snapshots: getFileSnapshotStore(this.ctx.viewSession),
|
||||
clipboard: getEditClipboard(this.ctx.viewSession),
|
||||
showImages: settings.get("terminal.showImages"),
|
||||
|
||||
@@ -104,6 +104,37 @@ describe("ToolExecutionComponent custom renderer failures", () => {
|
||||
}).not.toThrow();
|
||||
expect(text).toContain(rawResultText);
|
||||
});
|
||||
|
||||
it("renders a same-named extension tool result with the generic renderer", () => {
|
||||
const resultText = "recalled postgres memory";
|
||||
const tool: AgentTool = {
|
||||
name: "recall",
|
||||
label: "Extension Recall",
|
||||
description: "recalls external memory",
|
||||
parameters: { type: "object", additionalProperties: true },
|
||||
async execute() {
|
||||
return { content: [{ type: "text", text: resultText }] };
|
||||
},
|
||||
};
|
||||
const ui: ToolExecutionUi = {
|
||||
requestRender() {},
|
||||
requestComponentRender(_component: Component) {},
|
||||
resetDisplay() {},
|
||||
};
|
||||
const component = new ToolExecutionComponent(
|
||||
"recall",
|
||||
{ query: "project context" },
|
||||
{ showImages: false, useBuiltInRenderer: false },
|
||||
tool,
|
||||
ui,
|
||||
process.cwd(),
|
||||
);
|
||||
component.updateResult({ content: [{ type: "text", text: resultText }] }, false);
|
||||
|
||||
const rendered = visibleText(component.render(80));
|
||||
expect(rendered).toContain(resultText);
|
||||
expect(rendered).not.toContain("no matches");
|
||||
});
|
||||
});
|
||||
|
||||
describe("MCP result Markdown rendering", () => {
|
||||
|
||||
Reference in New Issue
Block a user