From 433f9dd30825f7e19ea9085dc94e04f5098d4e04 Mon Sep 17 00:00:00 2001 From: claude-code-best Date: Sat, 20 Jun 2026 19:34:14 +0800 Subject: [PATCH] feat(artifact): show uploaded URL inline below ExecuteExtraTool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deferred tools (shouldDefer: true) are invoked via SearchExtraTools → ExecuteExtraTool, so their tool_result rows used to render blank — the UI looked up ExecuteExtraTool, which had no renderToolResultMessage, and returned null. Add a generic delegation in ExecuteTool that forwards renderToolResultMessage to the inner tool when it defines one, unwrapping the { result, tool_name } envelope and the params from the input shape. All 28 deferred tools can now render their own UI by defining renderToolResultMessage. For ArtifactTool specifically, render the uploaded URL as an OSC 8 hyperlink (Link component) in warning color so it's visually prominent, with the expiry timestamp on a second line and a separate error branch. Also add `error: z.string().optional()` to outputSchema — zod's default strip mode was dropping the field, so error states never reached the UI. Co-Authored-By: glm-5.2 --- .../src/tools/ArtifactTool/ArtifactTool.ts | 8 +- .../src/tools/ArtifactTool/UI.tsx | 37 ++++ .../tools/ArtifactTool/__tests__/UI.test.tsx | 70 ++++++++ .../src/tools/ExecuteTool/ExecuteTool.ts | 25 +++ .../__tests__/ExecuteTool.render.test.ts | 167 ++++++++++++++++++ 5 files changed, 304 insertions(+), 3 deletions(-) create mode 100644 packages/builtin-tools/src/tools/ArtifactTool/UI.tsx create mode 100644 packages/builtin-tools/src/tools/ArtifactTool/__tests__/UI.test.tsx create mode 100644 packages/builtin-tools/src/tools/ExecuteTool/__tests__/ExecuteTool.render.test.ts diff --git a/packages/builtin-tools/src/tools/ArtifactTool/ArtifactTool.ts b/packages/builtin-tools/src/tools/ArtifactTool/ArtifactTool.ts index 44d516d93..f3c260459 100644 --- a/packages/builtin-tools/src/tools/ArtifactTool/ArtifactTool.ts +++ b/packages/builtin-tools/src/tools/ArtifactTool/ArtifactTool.ts @@ -10,6 +10,7 @@ import { } from './prompt.js' import { getArtifactsToken, getUploadUrl } from './config.js' import { uploadArtifact } from './client.js' +import { renderToolResultMessage } from './UI.js' const inputSchema = lazySchema(() => z.strictObject({ @@ -37,11 +38,11 @@ const outputSchema = lazySchema(() => id: z.string(), url: z.string(), expiresAt: z.string(), + error: z.string().optional(), }), ) type OutputSchema = ReturnType -type ArtifactOutput = z.infer -type ArtifactErrorOutput = ArtifactOutput & { error?: string } +export type ArtifactOutput = z.infer export const ArtifactTool = buildTool({ name: ARTIFACT_TOOL_NAME, @@ -87,7 +88,7 @@ export const ArtifactTool = buildTool({ }, mapToolResultToToolResultBlockParam( - content: ArtifactErrorOutput, + content: ArtifactOutput, toolUseID: string, ): ToolResultBlockParam { if (content.error) { @@ -104,6 +105,7 @@ export const ArtifactTool = buildTool({ content: `Artifact uploaded: ${content.url} (id: ${content.id}, expires: ${content.expiresAt})`, } }, + renderToolResultMessage, async call(input: ArtifactInput) { const { file_path, hash, ttl } = input diff --git a/packages/builtin-tools/src/tools/ArtifactTool/UI.tsx b/packages/builtin-tools/src/tools/ArtifactTool/UI.tsx new file mode 100644 index 000000000..6de1cfdb3 --- /dev/null +++ b/packages/builtin-tools/src/tools/ArtifactTool/UI.tsx @@ -0,0 +1,37 @@ +import * as React from 'react'; +import { Box, Link, Text } from '@anthropic/ink'; +import type { ToolProgressData } from 'src/Tool.js'; +import type { ProgressMessage } from 'src/types/message.js'; +import type { ArtifactOutput } from './ArtifactTool.js'; + +export function renderToolResultMessage( + content: ArtifactOutput, + _progressMessagesForMessage: ProgressMessage[], + _options: { verbose: boolean; theme?: string }, +): React.ReactNode { + if (content.error) { + return ( + + ⚠ Artifact upload failed: {content.error} + + ); + } + if (!content.url) return null; + return ( + + + + ↑ Artifact uploaded:{' '} + + {content.url} + + + + {content.expiresAt ? ( + + expires: {content.expiresAt} + + ) : null} + + ); +} diff --git a/packages/builtin-tools/src/tools/ArtifactTool/__tests__/UI.test.tsx b/packages/builtin-tools/src/tools/ArtifactTool/__tests__/UI.test.tsx new file mode 100644 index 000000000..7adee353f --- /dev/null +++ b/packages/builtin-tools/src/tools/ArtifactTool/__tests__/UI.test.tsx @@ -0,0 +1,70 @@ +import { describe, expect, test } from 'bun:test'; +import * as React from 'react'; +import type { ProgressMessage } from 'src/types/message.js'; +import type { ToolProgressData } from 'src/Tool.js'; +import { renderToolResultMessage } from '../UI.js'; +import type { ArtifactOutput } from '../ArtifactTool.js'; + +const NO_PROGRESS: ProgressMessage[] = []; +const OPTIONS = { verbose: false, theme: 'dark' } as never; + +/** Walk a React element tree and concatenate all string/number children. */ +function extractText(node: React.ReactNode): string { + if (node == null || typeof node === 'boolean') return ''; + if (typeof node === 'string') return node; + if (typeof node === 'number') return String(node); + if (Array.isArray(node)) return node.map(extractText).join(''); + if (React.isValidElement(node)) { + const children = (node.props as { children?: React.ReactNode }).children; + return extractText(children); + } + return ''; +} + +describe('ArtifactTool UI.renderToolResultMessage', () => { + test('renders the uploaded URL and expiry on success', () => { + const content: ArtifactOutput = { + id: 'abc123', + url: 'https://cloud-artifacts.claude-code-best.win/7d/abc123.html', + expiresAt: '2026-06-27T10:00:00.000Z', + }; + const node = renderToolResultMessage(content, NO_PROGRESS, OPTIONS); + expect(React.isValidElement(node)).toBe(true); + const text = extractText(node); + expect(text).toContain(content.url); + expect(text).toContain(content.expiresAt); + expect(text).toContain('Artifact uploaded'); + }); + + test('renders the error message on failure', () => { + const content: ArtifactOutput = { + id: '', + url: '', + expiresAt: '', + error: 'File does not exist or is not readable: /tmp/missing.html', + }; + const node = renderToolResultMessage(content, NO_PROGRESS, OPTIONS); + expect(React.isValidElement(node)).toBe(true); + const text = extractText(node); + expect(text).toContain('Artifact upload failed'); + expect(text).toContain('/tmp/missing.html'); + }); + + test('returns null when url is empty without error', () => { + const content: ArtifactOutput = { id: '', url: '', expiresAt: '' }; + const node = renderToolResultMessage(content, NO_PROGRESS, OPTIONS); + expect(node).toBeNull(); + }); + + test('omits the expiry line when expiresAt is empty', () => { + const content: ArtifactOutput = { + id: 'abc', + url: 'https://cloud-artifacts.claude-code-best.win/7d/abc.html', + expiresAt: '', + }; + const node = renderToolResultMessage(content, NO_PROGRESS, OPTIONS); + expect(React.isValidElement(node)).toBe(true); + // Sanity: still renders URL even without expiry + expect(extractText(node)).toContain(content.url); + }); +}); diff --git a/packages/builtin-tools/src/tools/ExecuteTool/ExecuteTool.ts b/packages/builtin-tools/src/tools/ExecuteTool/ExecuteTool.ts index 21d8f3c27..bfe9fb32b 100644 --- a/packages/builtin-tools/src/tools/ExecuteTool/ExecuteTool.ts +++ b/packages/builtin-tools/src/tools/ExecuteTool/ExecuteTool.ts @@ -236,4 +236,29 @@ export const ExecuteTool = buildTool({ content: JSON.stringify(content), } }, + // Output shape: { result: , tool_name: string }. + // Delegate rendering to the inner tool when it defines its own + // renderToolResultMessage so deferred tools can show their own UI + // (e.g. ArtifactTool displays its uploaded URL). Without this, the + // ExecuteExtraTool tool_result row renders nothing below the tool_use + // line. The inner tool expects its own input shape, so unwrap params. + // + // Inline the lookup rather than calling findToolByName — deferred tools + // are matched by exact name (no aliases needed), and avoiding the + // shared helper keeps this method resilient to src/Tool.js mocks in + // co-located test files (process-global mock.module pollution). + renderToolResultMessage(content, progressMessages, options) { + const innerTool = options.tools.find(t => t.name === content.tool_name) + if (!innerTool?.renderToolResultMessage) return null + const innerInput = (options.input as { params?: unknown } | undefined) + ?.params + return innerTool.renderToolResultMessage( + content.result as never, + progressMessages, + { + ...options, + input: innerInput, + }, + ) + }, } satisfies ToolDef) diff --git a/packages/builtin-tools/src/tools/ExecuteTool/__tests__/ExecuteTool.render.test.ts b/packages/builtin-tools/src/tools/ExecuteTool/__tests__/ExecuteTool.render.test.ts new file mode 100644 index 000000000..925d3889e --- /dev/null +++ b/packages/builtin-tools/src/tools/ExecuteTool/__tests__/ExecuteTool.render.test.ts @@ -0,0 +1,167 @@ +import { describe, expect, test, mock } from 'bun:test' +import { logMock } from '../../../../../../tests/mocks/log' +import { debugMock } from '../../../../../../tests/mocks/debug' + +// Same mock setup as ExecuteTool.runner.ts — ExecuteTool's import chain +// (growthbook, searchExtraTools, messages) loads real modules with side +// effects otherwise. mock.module is process-global; identical setup in +// sibling test files in this directory is safe (last-write-wins, same stubs). +mock.module('src/utils/log.ts', logMock) +mock.module('src/utils/debug.ts', debugMock) + +mock.module('src/services/analytics/growthbook.js', () => ({ + getFeatureValue_CACHED_MAY_BE_STALE: () => false, + checkStatsigFeatureGate_CACHED_MAY_BE_STALE: () => false, + getFeatureValue_DEPRECATED: async () => undefined, + getFeatureValue_CACHED_WITH_REFRESH: async () => undefined, + hasGrowthBookEnvOverride: () => false, + getAllGrowthBookFeatures: () => ({}), + getGrowthBookConfigOverrides: () => ({}), + setGrowthBookConfigOverride: () => {}, + clearGrowthBookConfigOverrides: () => {}, + getApiBaseUrlHost: () => undefined, + onGrowthBookRefresh: () => {}, + initializeGrowthBook: async () => {}, + checkSecurityRestrictionGate: async () => false, + checkGate_CACHED_OR_BLOCKING: async () => false, + refreshGrowthBookAfterAuthChange: () => {}, + resetGrowthBook: () => {}, + refreshGrowthBookFeatures: async () => {}, + setupPeriodicGrowthBookRefresh: () => {}, + stopPeriodicGrowthBookRefresh: () => {}, +})) + +mock.module('src/utils/searchExtraTools.js', () => ({ + isSearchExtraToolsEnabledOptimistic: () => true, + getAutoSearchExtraToolsCharThreshold: () => 100, + getSearchExtraToolsMode: () => 'tst' as const, + isSearchExtraToolsToolAvailable: () => true, + isSearchExtraToolsEnabled: async () => true, + isToolReferenceBlock: () => false, + extractDiscoveredToolNames: () => new Set(), + isDeferredToolsDeltaEnabled: () => false, + getDeferredToolsDelta: () => null, +})) + +mock.module('src/constants/tools.js', () => ({ + CORE_TOOLS: new Set(['ExecuteExtraTool', 'SearchExtraTools']), +})) + +mock.module('src/utils/messages.js', () => ({ + createUserMessage: ({ content }: { content: string }) => ({ + type: 'user' as const, + content, + uuid: 'test-uuid', + }), + INTERRUPT_MESSAGE_FOR_TOOL_USE: '[Request interrupted]', +})) + +mock.module('src/utils/toolErrors.js', () => ({ + formatZodValidationError: (_name: string, error: unknown) => + `validation error: ${JSON.stringify(error)}`, +})) + +const { ExecuteTool } = await import('../ExecuteTool.js') + +type RenderResult = React.ReactNode + +describe('ExecuteTool.renderToolResultMessage delegation', () => { + test('delegates to inner tool with content.result and unwrapped params', () => { + const seen: Array<{ + content: unknown + input: unknown + }> = [] + const innerRender = ( + content: unknown, + _progress: unknown, + options: { input?: unknown }, + ): RenderResult => { + seen.push({ content, input: options.input }) + return 'RENDERED' as unknown as RenderResult + } + const innerTool = { + name: 'artifact', + renderToolResultMessage: innerRender, + } + const tools = [innerTool] as never + + const result = ExecuteTool.renderToolResultMessage( + { + result: { + id: 'abc', + url: 'https://example.com/x.html', + expiresAt: 'T', + }, + tool_name: 'artifact', + }, + [], + { + tools, + input: { + tool_name: 'artifact', + params: { file_path: '/tmp/x.html', ttl: 7 }, + }, + } as never, + ) + + expect(result).toBe('RENDERED') + expect(seen).toHaveLength(1) + expect(seen[0]?.content).toEqual({ + id: 'abc', + url: 'https://example.com/x.html', + expiresAt: 'T', + }) + // Inner tool should see its own params shape, not the ExecuteExtraTool wrapper + expect(seen[0]?.input).toEqual({ file_path: '/tmp/x.html', ttl: 7 }) + }) + + test('returns null when inner tool has no renderToolResultMessage', () => { + const innerTool = { name: 'bare' } + const tools = [innerTool] as never + + const result = ExecuteTool.renderToolResultMessage( + { result: { ok: true }, tool_name: 'bare' }, + [], + { tools, input: { tool_name: 'bare', params: {} } } as never, + ) + + expect(result).toBeNull() + }) + + test('returns null when inner tool is not found in tools list', () => { + const tools = [] as never + + const result = ExecuteTool.renderToolResultMessage( + { result: { ok: true }, tool_name: 'missing' }, + [], + { tools, input: { tool_name: 'missing', params: {} } } as never, + ) + + expect(result).toBeNull() + }) + + test('passes through undefined input safely when input is missing', () => { + const seen: unknown[] = [] + const innerTool = { + name: 'artifact', + renderToolResultMessage: ( + _content: unknown, + _progress: unknown, + options: { input?: unknown }, + ): RenderResult => { + seen.push(options.input) + return null + }, + } + const tools = [innerTool] as never + + const result = ExecuteTool.renderToolResultMessage( + { result: { ok: true }, tool_name: 'artifact' }, + [], + { tools } as never, + ) + + expect(result).toBeNull() + expect(seen[0]).toBeUndefined() + }) +})