From 931f61867c41ee5f5566a7fc992f4f9a984c2d65 Mon Sep 17 00:00:00 2001 From: roboomp Date: Thu, 6 Aug 2026 01:36:25 +0000 Subject: [PATCH] fix(tui): gated built-in renderers by tool provenance - Routed session tool provenance through live and rebuilt transcript render paths. - Kept same-named extension tools on the generic renderer while preserving native tool rendering. - Added regression coverage for an external recall result collision. Fixes #7770 --- packages/coding-agent/CHANGELOG.md | 4 +++ packages/coding-agent/src/cli/gallery-cli.ts | 2 +- .../src/modes/components/agent-hub.ts | 5 +++ .../components/agent-transcript-viewer.ts | 3 ++ .../components/chat-transcript-builder.ts | 3 ++ .../src/modes/components/tool-execution.ts | 29 ++++++++--------- .../src/modes/controllers/event-controller.ts | 2 ++ .../modes/controllers/selector-controller.ts | 1 + .../src/modes/utils/ui-helpers.ts | 1 + .../modes/components/tool-execution.test.ts | 31 +++++++++++++++++++ 10 files changed, 64 insertions(+), 17 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 456e88609..d9e4b36d3 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### 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 ### Breaking Changes diff --git a/packages/coding-agent/src/cli/gallery-cli.ts b/packages/coding-agent/src/cli/gallery-cli.ts index b27b1e0dd..385c401e1 100644 --- a/packages/coding-agent/src/cli/gallery-cli.ts +++ b/packages/coding-agent/src/cli/gallery-cli.ts @@ -153,7 +153,7 @@ export async function renderGalleryState( const component = new ToolExecutionComponent( componentName, streamingArgs, - { showImages: false }, + { showImages: false, useBuiltInRenderer: !fixture.customRendered }, tool, ui, getProjectDir(), diff --git a/packages/coding-agent/src/modes/components/agent-hub.ts b/packages/coding-agent/src/modes/components/agent-hub.ts index e0ff47538..f63acd014 100644 --- a/packages/coding-agent/src/modes/components/agent-hub.ts +++ b/packages/coding-agent/src/modes/components/agent-hub.ts @@ -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, diff --git a/packages/coding-agent/src/modes/components/agent-transcript-viewer.ts b/packages/coding-agent/src/modes/components/agent-transcript-viewer.ts index 0b8b8c428..138a58f88 100644 --- a/packages/coding-agent/src/modes/components/agent-transcript-viewer.ts +++ b/packages/coding-agent/src/modes/components/agent-transcript-viewer.ts @@ -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, diff --git a/packages/coding-agent/src/modes/components/chat-transcript-builder.ts b/packages/coding-agent/src/modes/components/chat-transcript-builder.ts index c02eb74fa..625b71de3 100644 --- a/packages/coding-agent/src/modes/components/chat-transcript-builder.ts +++ b/packages/coding-agent/src/modes/components/chat-transcript-builder.ts @@ -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"), diff --git a/packages/coding-agent/src/modes/components/tool-execution.ts b/packages/coding-agent/src/modes/components/tool-execution.ts index 016ac6884..0fb863e69 100644 --- a/packages/coding-agent/src/modes/components/tool-execution.ts +++ b/packages/coding-agent/src/modes/components/tool-execution.ts @@ -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 | 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) { diff --git a/packages/coding-agent/src/modes/controllers/event-controller.ts b/packages/coding-agent/src/modes/controllers/event-controller.ts index 0be56fdaf..265ac2a30 100644 --- a/packages/coding-agent/src/modes/controllers/event-controller.ts +++ b/packages/coding-agent/src/modes/controllers/event-controller.ts @@ -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"), diff --git a/packages/coding-agent/src/modes/controllers/selector-controller.ts b/packages/coding-agent/src/modes/controllers/selector-controller.ts index b3fd2384b..ff68a4979 100644 --- a/packages/coding-agent/src/modes/controllers/selector-controller.ts +++ b/packages/coding-agent/src/modes/controllers/selector-controller.ts @@ -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, diff --git a/packages/coding-agent/src/modes/utils/ui-helpers.ts b/packages/coding-agent/src/modes/utils/ui-helpers.ts index 4d0238b22..346b52d16 100644 --- a/packages/coding-agent/src/modes/utils/ui-helpers.ts +++ b/packages/coding-agent/src/modes/utils/ui-helpers.ts @@ -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"), diff --git a/packages/coding-agent/test/modes/components/tool-execution.test.ts b/packages/coding-agent/test/modes/components/tool-execution.test.ts index c95fe2360..1c641e973 100644 --- a/packages/coding-agent/test/modes/components/tool-execution.test.ts +++ b/packages/coding-agent/test/modes/components/tool-execution.test.ts @@ -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", () => {