smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

merged PR #5438: fix(tui): guard plugin tool renderer components against render crashes

Browse Source
This commit is contained in:
can1357
2026-07-16 03:32:01 +02:00
parent f9cc18c45a 2d519d8f89
commit 8e80eae501
3 changed files with 219 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/CHANGELOG.md
+1
View File
@@ -431,6 +431,7 @@
- Integrated testing guidance directly into the main system prompt for improved workflow cohesion
- Moved testing guidance into the main system prompt and removed the bundled Tester subagent.
- Fixed a crash when a plugin/custom tool renderer returns a component that throws during its later `render()` pass (e.g. `TypeError: th.bold is not a function` from a plugin that styles its header off an object without a `bold` method). `ToolExecutionComponent` now wraps every renderer-returned call/result component so a throwing `render()` degrades to the safe fallback (tool label or raw result text) instead of taking down the transcript ([#4978](https://github.com/can1357/oh-my-pi/issues/4978)).
## [16.3.14] - 2026-07-09
packages/coding-agent/src/modes/components/tool-execution.test.ts
+101
View File
@@ -0,0 +1,101 @@
import { beforeAll, describe, expect, it } from "bun:test";
import type { AgentTool } from "@oh-my-pi/pi-agent-core";
import { type Component, Text } from "@oh-my-pi/pi-tui";
import { Settings } from "../../config/settings";
import { getThemeByName, setThemeInstance, theme } from "../theme/theme";
import { ToolExecutionComponent, type ToolExecutionUi } from "./tool-execution";
class BoldTypeErrorComponent implements Component {
render(_width: number): readonly string[] {
throw new TypeError("th.bold is not a function");
}
}
function visibleText(lines: readonly string[]): string {
let text = lines.join("\n");
text = text.replace(/\x1b\]8;[^\x1b\x07]*(?:\x07|\x1b\\)/g, "");
text = text.replace(/\x1b\[[0-9;]*m/g, "");
return text;
}
describe("ToolExecutionComponent custom renderer failures", () => {
beforeAll(async () => {
await Settings.init({ inMemory: true });
const loaded = await getThemeByName("dark");
if (!loaded) throw new Error("theme unavailable");
setThemeInstance(loaded);
});
it("falls back to the custom tool label when a renderCall child component throws during render", () => {
const tool: AgentTool = {
name: "graphify_graph",
label: "Graphify Graph",
description: "renders a graph",
parameters: { type: "object", additionalProperties: true },
renderCall() {
return new BoldTypeErrorComponent();
},
async execute() {
return { content: [{ type: "text", text: "ok" }] };
},
};
const ui: ToolExecutionUi = {
requestRender() {},
requestComponentRender(_component: Component) {},
resetDisplay() {},
};
const component = new ToolExecutionComponent(
"graphify_graph",
{},
{ showImages: false },
tool,
ui,
process.cwd(),
);
let text = "";
expect(() => {
text = visibleText(component.render(80));
}).not.toThrow();
expect(text).toContain("Graphify Graph");
});
it("preserves raw result text when a renderResult child component throws during render", () => {
const rawResultText = "raw result survives child renderer failure";
const tool: AgentTool = {
name: "crashy_result_renderer",
label: "Crashy Result Renderer",
description: "renders result output",
parameters: { type: "object", additionalProperties: true },
renderCall() {
return new Text(theme.fg("toolTitle", theme.bold("Crashy Result Renderer")), 0, 0);
},
renderResult() {
return new BoldTypeErrorComponent();
},
async execute() {
return { content: [{ type: "text", text: rawResultText }] };
},
};
const ui: ToolExecutionUi = {
requestRender() {},
requestComponentRender(_component: Component) {},
resetDisplay() {},
};
const component = new ToolExecutionComponent(
"crashy_result_renderer",
{},
{ showImages: false },
tool,
ui,
process.cwd(),
);
component.updateResult({ content: [{ type: "text", text: rawResultText }] }, false);
let text = "";
expect(() => {
text = visibleText(component.render(80));
}).not.toThrow();
expect(text).toContain(rawResultText);
});
});
packages/coding-agent/src/modes/components/tool-execution.ts
+117 -9
View File
@@ -40,7 +40,7 @@ import {
} from "../../tools/render-utils";
import { type FirstResultViewportRepaint, toolRenderers } from "../../tools/renderers";
import { TODO_STRIKE_TOTAL_FRAMES, type TodoToolDetails } from "../../tools/todo";
import { isFramedBlockComponent, renderStatusLine, WidthAwareText } from "../../tui";
import { isFramedBlockComponent, markFramedBlockComponent, renderStatusLine, WidthAwareText } from "../../tui";
import { sanitizeWithOptionalSixelPassthrough } from "../../utils/sixel";
import { renderDiff } from "./diff";
@@ -148,6 +148,68 @@ function getArgsWithStreamedTextInput(args: unknown): unknown {
return input === undefined ? args : { ...record, input };
}
type ToolRendererStage = "call" | "result";
class SafeToolRendererComponent implements Component {
#toolName: string;
#stage: ToolRendererStage;
#component: Component;
#fallback: () => Component | undefined;
#warned = false;
readonly wantsKeyRelease: boolean | undefined;
constructor(
toolName: string,
stage: ToolRendererStage,
component: Component,
fallback: () => Component | undefined,
) {
this.#toolName = toolName;
this.#stage = stage;
this.#component = component;
this.#fallback = fallback;
this.wantsKeyRelease = component.wantsKeyRelease;
if (isFramedBlockComponent(component)) {
markFramedBlockComponent(this);
}
}
render(width: number): readonly string[] {
try {
return this.#component.render(width);
} catch (err) {
if (!this.#warned) {
this.#warned = true;
logger.warn("Tool renderer failed", { tool: this.#toolName, stage: this.#stage, error: String(err) });
}
return this.#fallback()?.render(width) ?? [];
}
}
handleInput(data: string): void {
const handleInput = this.#component.handleInput;
if (handleInput === undefined) return;
handleInput.call(this.#component, data);
}
invalidate(): void {
const invalidate = this.#component.invalidate;
if (invalidate === undefined) return;
invalidate.call(this.#component);
}
setIgnoreTight(ignore: boolean): void {
const setIgnoreTight = this.#component.setIgnoreTight;
if (setIgnoreTight === undefined) return;
setIgnoreTight.call(this.#component, ignore);
}
dispose(): void {
const dispose = this.#component.dispose;
if (dispose === undefined) return;
dispose.call(this.#component);
}
}
/**
* Transcript-side probe telling a block whether it is still inside the live
* (repaintable) region. Implemented by `TranscriptContainer`; injected rather
@@ -157,6 +219,14 @@ export interface TranscriptLiveRegionProbe {
isBlockInLiveRegion(component: Component): boolean;
}
/** Minimal TUI surface ToolExecutionComponent uses to schedule repaints and share image budget. */
export interface ToolExecutionUi {
requestRender(): void;
requestComponentRender(component: Component): void;
resetDisplay(): void;
imageBudget?: TUI["imageBudget"];
}
export interface ToolExecutionOptions {
snapshots?: SnapshotStore;
showImages?: boolean; // default: true (only used if terminal supports images)
@@ -240,7 +310,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;
#ui: TUI;
#ui: ToolExecutionUi;
#cwd: string;
#result?: {
content: Array<{ type: string; text?: string; data?: string; mimeType?: string }>;
@@ -308,7 +378,7 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac
args: any,
options: ToolExecutionOptions = {},
tool: AgentTool | undefined,
ui: TUI,
ui: ToolExecutionUi,
cwd: string = getProjectDir(),
_toolCallId?: string,
) {
@@ -902,8 +972,17 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac
if (tool.renderCall) {
try {
const callArgs = this.#getCallArgsForRender();
const callComponent = tool.renderCall(callArgs, this.#renderState, theme);
if (callComponent) this.#contentBox.addChild(callComponent as Component);
const callComponent = tool.renderCall(callArgs, this.#renderState, theme) as Component | undefined;
if (callComponent) {
this.#contentBox.addChild(
new SafeToolRendererComponent(
this.#toolName,
"call",
callComponent,
() => new Text(theme.fg("toolTitle", theme.bold(this.#toolLabel)), 0, 0),
),
);
}
} catch (err) {
logger.warn("Tool renderer failed", { tool: this.#toolName, error: String(err) });
// Fall back to default on error
@@ -934,7 +1013,15 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac
theme,
this.#args,
);
if (resultComponent) this.#contentBox.addChild(resultComponent);
if (resultComponent) {
this.#contentBox.addChild(
new SafeToolRendererComponent(this.#toolName, "result", resultComponent, () => {
const output = this.#getTextOutput();
if (!output) return undefined;
return new Text(theme.fg("toolOutput", replaceTabs(output)), 0, 0);
}),
);
}
} catch (err) {
logger.warn("Tool renderer failed", { tool: this.#toolName, error: String(err) });
// Fall back to showing raw output on error
@@ -991,7 +1078,11 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac
this.#renderState,
theme,
);
if (resultComponent) fileBox.addChild(resultComponent);
if (resultComponent) {
fileBox.addChild(
new SafeToolRendererComponent(this.#toolName, "result", resultComponent, () => undefined),
);
}
} catch (err) {
logger.warn("Tool renderer failed", { tool: this.#toolName, error: String(err) });
}
@@ -1038,7 +1129,16 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac
try {
const callArgs = this.#getCallArgsForRender();
const callComponent = renderer.renderCall(callArgs, this.#renderState, theme);
if (callComponent) this.#contentBox.addChild(callComponent);
if (callComponent) {
this.#contentBox.addChild(
new SafeToolRendererComponent(
this.#toolName,
"call",
callComponent,
() => new Text(theme.fg("toolTitle", theme.bold(this.#toolLabel)), 0, 0),
),
);
}
} catch (err) {
logger.warn("Tool renderer failed", { tool: this.#toolName, error: String(err) });
// Fall back to default on error
@@ -1059,7 +1159,15 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac
theme,
this.#getCallArgsForRender(),
);
if (resultComponent) this.#contentBox.addChild(resultComponent);
if (resultComponent) {
this.#contentBox.addChild(
new SafeToolRendererComponent(this.#toolName, "result", resultComponent, () => {
const output = this.#getTextOutput();
if (!output) return undefined;
return new Text(theme.fg("toolOutput", replaceTabs(output)), 0, 0);
}),
);
}
} catch (err) {
logger.warn("Tool renderer failed", { tool: this.#toolName, error: String(err) });
// Fall back to showing raw output on error