diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index b19bd4917..151342073 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -5,6 +5,7 @@ - Added `gallery` CLI command to render built-in tool renderer output across streaming, in-progress, success, and failure states - Added `omp gallery` filtering and rendering options (`--tool`, `--state`, `--width`, `--expanded`, and `--plain`) for focused renderer previews and plain-text output +- Added `omp gallery` fidelity for tools whose renderers are attached on the tool instance (`lsp`, `task`): the gallery now drives them through the same custom-tool render branch production uses, so regressions in that path surface in the gallery rather than only in a live session. - Added `app.display.reset`, bound to `Ctrl+L` by default, to force an immediate terminal display reset/redraw without resizing the window. ### Changed @@ -17,6 +18,7 @@ - Fixed framed read results rendering with an extra blank row above and below the output block. - Fixed collapsed search result previews that could show only "… N more matches" when the first grouped section exceeded the preview budget. Collapsed search output now compacts to match rows, fills the budget with visible hits before the summary, and keeps truncation details out of the bottom user-visible notice. - Fixed boolean environment flag overrides that were ORed with settings, so `PI_INTENT_TRACING=0`, `PI_AUTO_QA=0`, and per-backend eval flags now take precedence when present while falling back to config when unset. +- Fixed custom-rendered tools that set `mergeCallAndResult` (e.g. `lsp`) rendering a redundant tool-name line above the framed result once a result arrived. `ToolExecutionComponent`'s custom-tool branch now emits the fallback label only when the tool has no `renderCall` and the call is not suppressed by an existing result, matching the built-in renderer branch. ## [15.9.67] - 2026-06-06 ### Added diff --git a/packages/coding-agent/src/cli/gallery-cli.ts b/packages/coding-agent/src/cli/gallery-cli.ts index 744691a15..921e571db 100644 --- a/packages/coding-agent/src/cli/gallery-cli.ts +++ b/packages/coding-agent/src/cli/gallery-cli.ts @@ -45,10 +45,26 @@ const GENERIC_ERROR: GalleryResult = { isError: true, }; -/** Build the fake `AgentTool` the component needs for its label and edit mode. */ +/** + * Build the fake `AgentTool` the component needs for its label, edit mode, and — + * for `customRendered` fixtures — the renderer functions that route it through + * the same custom-tool branch production uses (see {@link GalleryFixture}). + */ function fakeToolFor(name: string, fixture: GalleryFixture | undefined): AgentTool | undefined { - if (!fixture?.label && !fixture?.editMode) return undefined; - return { name, label: fixture.label ?? name, mode: fixture.editMode } as unknown as AgentTool; + if (!fixture?.label && !fixture?.editMode && !fixture?.customRendered) return undefined; + const tool: Record = { name, label: fixture.label ?? name, mode: fixture.editMode }; + if (fixture.customRendered) { + const renderer = toolRenderers[name] as + | { renderCall?: unknown; renderResult?: unknown; mergeCallAndResult?: unknown; inline?: unknown } + | undefined; + if (renderer) { + tool.renderCall = renderer.renderCall; + tool.renderResult = renderer.renderResult; + tool.mergeCallAndResult = renderer.mergeCallAndResult; + tool.inline = renderer.inline; + } + } + return tool as unknown as AgentTool; } /** The curated fixture for a tool, or a generic one for registry tools lacking sample data. */ diff --git a/packages/coding-agent/src/cli/gallery-fixtures/agentic.ts b/packages/coding-agent/src/cli/gallery-fixtures/agentic.ts index f2dd796ef..d1c262abe 100644 --- a/packages/coding-agent/src/cli/gallery-fixtures/agentic.ts +++ b/packages/coding-agent/src/cli/gallery-fixtures/agentic.ts @@ -4,6 +4,7 @@ import type { GalleryFixture } from "./types"; export const agenticFixtures: Record = { task: { label: "Task", + customRendered: true, // Streaming: agent chosen, first task fully arrived, second still landing. streamingArgs: { agent: "task", diff --git a/packages/coding-agent/src/cli/gallery-fixtures/codeintel.ts b/packages/coding-agent/src/cli/gallery-fixtures/codeintel.ts index 5f10a9e2d..0d9faca65 100644 --- a/packages/coding-agent/src/cli/gallery-fixtures/codeintel.ts +++ b/packages/coding-agent/src/cli/gallery-fixtures/codeintel.ts @@ -4,6 +4,7 @@ import type { GalleryFixture } from "./types"; export const codeintelFixtures: Record = { lsp: { label: "LSP", + customRendered: true, streamingArgs: { action: "references", file: "src/server/auth.ts", diff --git a/packages/coding-agent/src/cli/gallery-fixtures/types.ts b/packages/coding-agent/src/cli/gallery-fixtures/types.ts index 77c051ce6..97b7da510 100644 --- a/packages/coding-agent/src/cli/gallery-fixtures/types.ts +++ b/packages/coding-agent/src/cli/gallery-fixtures/types.ts @@ -16,6 +16,15 @@ export interface GalleryFixture { label?: string; /** Edit mode for edit-like tools so the streaming preview dispatches correctly. */ editMode?: EditMode; + /** + * Set for tools whose real `AgentTool` attaches `renderCall`/`renderResult` + * directly on the instance (e.g. `lsp`, `task`). The harness then attaches + * the registry renderer onto the fake tool so the component routes through + * the custom-tool branch — the same path production takes — instead of the + * built-in registry branch. The two branches can diverge, so exercising the + * real one keeps the gallery honest for these tools. + */ + customRendered?: boolean; /** * Arguments shown during the streaming state — a partial view of {@link args} * as if the tool-call JSON were still arriving. May include `__partialJson` diff --git a/packages/coding-agent/src/modes/components/tool-execution.ts b/packages/coding-agent/src/modes/components/tool-execution.ts index b232bc282..4fd08f692 100644 --- a/packages/coding-agent/src/modes/components/tool-execution.ts +++ b/packages/coding-agent/src/modes/components/tool-execution.ts @@ -171,6 +171,7 @@ let toolExecutionInstanceSeq = 0; export class ToolExecutionComponent extends Container { #contentBox: Box; // Used for custom tools and bash visual truncation #contentText: Text; // For built-in tools (with its own padding/bg) + #leadingSpacer: Spacer; // Blank line above the content; collapsed for self-delimiting framed boxes #multiFileBoxes: (Box | Spacer)[] = []; // Extra boxes for multi-file edit results #imageComponents: Image[] = []; #imageSpacers: Spacer[] = []; @@ -245,7 +246,8 @@ export class ToolExecutionComponent extends Container { this.#cwd = cwd; this.#args = args; - this.addChild(new Spacer(1)); + this.#leadingSpacer = new Spacer(1); + this.addChild(this.#leadingSpacer); // Always create both - contentBox for custom tools/bash/tools with renderers, contentText for other built-ins this.#contentBox = new Box(1, 1, (text: string) => theme.bg("toolPendingBg", text)); @@ -585,6 +587,8 @@ export class ToolExecutionComponent extends Container { this.#renderState.expanded = this.#expanded; this.#renderState.isPartial = this.#isPartial; this.#renderState.spinnerFrame = this.#spinnerFrame; + // Self-delimiting framed boxes don't need the leading blank line for separation. + let hasFramedBlock = false; // Check for custom tool rendering if (this.#tool && (this.#tool.renderCall || this.#tool.renderResult)) { @@ -601,22 +605,28 @@ export class ToolExecutionComponent extends Container { // call preview once result lines exist. this.#renderState.renderContext = this.#buildRenderContext(); - // Render call component + // Render call component. The fallback label only stands in for a + // missing `renderCall`; when the call is intentionally suppressed + // (mergeCallAndResult once a result exists) we render nothing here so + // the result component isn't preceded by a redundant tool-name line. const shouldRenderCall = !this.#result || !mergeCallAndResult; - if (shouldRenderCall && tool.renderCall) { - try { - const callComponent = tool.renderCall(this.#getCallArgsForRender(), this.#renderState, theme); - if (callComponent) { - contentBoxHasFramedBlock = addBoxChild(this.#contentBox, callComponent) || contentBoxHasFramedBlock; + if (shouldRenderCall) { + if (tool.renderCall) { + try { + const callComponent = tool.renderCall(this.#getCallArgsForRender(), this.#renderState, theme); + if (callComponent) { + contentBoxHasFramedBlock = + addBoxChild(this.#contentBox, callComponent) || contentBoxHasFramedBlock; + } + } catch (err) { + logger.warn("Tool renderer failed", { tool: this.#toolName, error: String(err) }); + // Fall back to default on error + addBoxChild(this.#contentBox, 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 + } else { + // No custom renderCall, show tool name addBoxChild(this.#contentBox, new Text(theme.fg("toolTitle", theme.bold(this.#toolLabel)), 0, 0)); } - } else { - // No custom renderCall, show tool name - addBoxChild(this.#contentBox, new Text(theme.fg("toolTitle", theme.bold(this.#toolLabel)), 0, 0)); } // Render result component if we have a result @@ -657,6 +667,7 @@ export class ToolExecutionComponent extends Container { } } setBoxPaddingForFramedBlock(this.#contentBox, contentBoxHasFramedBlock); + hasFramedBlock = contentBoxHasFramedBlock; } else if (this.#toolName in toolRenderers) { // Built-in tools with renderers const renderer = toolRenderers[this.#toolName]; @@ -700,6 +711,7 @@ export class ToolExecutionComponent extends Container { if (resultComponent) { const fileBoxHasFramedBlock = addBoxChild(fileBox, resultComponent); setBoxPaddingForFramedBlock(fileBox, fileBoxHasFramedBlock); + if (fileBoxHasFramedBlock) hasFramedBlock = true; } } catch (err) { logger.warn("Tool renderer failed", { tool: this.#toolName, error: String(err) }); @@ -783,6 +795,7 @@ export class ToolExecutionComponent extends Container { } } setBoxPaddingForFramedBlock(this.#contentBox, contentBoxHasFramedBlock); + hasFramedBlock = contentBoxHasFramedBlock; } } else { // Other built-in tools: use Text directly with caching @@ -790,6 +803,8 @@ export class ToolExecutionComponent extends Container { this.#contentText.setText(this.#formatToolExecution()); } + this.#leadingSpacer.setLines(hasFramedBlock ? 0 : 1); + // Handle images (same for both custom and built-in) for (const img of this.#imageComponents) { this.removeChild(img); diff --git a/packages/coding-agent/test/gallery-cli.test.ts b/packages/coding-agent/test/gallery-cli.test.ts new file mode 100644 index 000000000..d7087f0b4 --- /dev/null +++ b/packages/coding-agent/test/gallery-cli.test.ts @@ -0,0 +1,79 @@ +import { beforeAll, describe, expect, it } from "bun:test"; +import { GALLERY_STATES, renderGalleryState, resolveFixture } from "../src/cli/gallery-cli"; +import type { GalleryFixture } from "../src/cli/gallery-fixtures"; +import { resetSettingsForTest, Settings } from "../src/config/settings"; +import { initTheme } from "../src/modes/theme/theme"; +import { toolRenderers } from "../src/tools/renderers"; + +beforeAll(async () => { + resetSettingsForTest(); + await Settings.init({ inMemory: true }); + await initTheme(false, undefined, undefined, "dark", "light"); +}); + +describe("gallery harness", () => { + it("renders every registered tool in every lifecycle state without throwing", async () => { + for (const name in toolRenderers) { + const fixture = resolveFixture(name); + for (const state of GALLERY_STATES) { + const lines = await renderGalleryState(name, fixture, state, 100); + // A renderer that produces no lines for a state is a regression: the + // component should always emit at least the call header or result. + expect(lines.length, `${name}/${state} rendered nothing`).toBeGreaterThan(0); + } + } + }); + + it("routes each state to the matching args/result (streaming args vs result, success vs error)", async () => { + const fixture: GalleryFixture = { + label: "Bash", + streamingArgs: { command: "echo STREAM_MARK" }, + args: { command: "echo PROGRESS_MARK" }, + result: { content: [{ type: "text", text: "SUCCESS_OUT" }], details: { exitCode: 0 } }, + errorResult: { content: [{ type: "text", text: "ERROR_OUT" }], isError: true, details: { exitCode: 1 } }, + }; + const render = async (state: (typeof GALLERY_STATES)[number]) => + Bun.stripANSI((await renderGalleryState("bash", fixture, state, 100)).join("\n")); + + const streaming = await render("streaming"); + expect(streaming).toContain("STREAM_MARK"); + expect(streaming).not.toContain("PROGRESS_MARK"); + expect(streaming).not.toContain("SUCCESS_OUT"); + + const progress = await render("progress"); + expect(progress).toContain("PROGRESS_MARK"); + expect(progress).not.toContain("SUCCESS_OUT"); + + const success = await render("success"); + expect(success).toContain("SUCCESS_OUT"); + expect(success).not.toContain("ERROR_OUT"); + + const error = await render("error"); + expect(error).toContain("ERROR_OUT"); + expect(error).not.toContain("SUCCESS_OUT"); + }); + + it("routes customRendered tools (lsp, task) through the custom-tool branch", async () => { + // `lsp`/`task` attach their renderers on the real AgentTool, so the gallery + // must reproduce that path. With a result present and mergeCallAndResult, the + // custom branch must NOT emit a redundant tool-name line above the result box + // (regression guard for tool-execution's custom-branch fallback label). + const lsp = resolveFixture("lsp"); + expect(lsp.customRendered).toBe(true); + const lines = await renderGalleryState("lsp", lsp, "error", 100); + const stripped = lines.map(line => Bun.stripANSI(line).trim()); + // The framed result header is present... + expect(stripped.some(line => line.includes("LSP references"))).toBe(true); + // ...but no standalone "LSP" label line precedes it. + expect(stripped).not.toContain("LSP"); + }); + + it("falls back to a generic fixture for registry tools without curated sample data", () => { + // resolveFixture never returns undefined for a registry tool, even one + // missing from the curated fixtures, so the gallery cannot crash on a newly + // added renderer. + const fixture = resolveFixture("a-tool-that-has-no-fixture"); + expect(fixture.args).toBeDefined(); + expect(fixture.result.content.length).toBeGreaterThan(0); + }); +});