Skip to content

Commit 433f9dd

Browse files
claude-code-bestglm-5.2
andcommitted
feat(artifact): show uploaded URL inline below ExecuteExtraTool
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 <zai-org@claude-code-best.win>
1 parent 14f43e9 commit 433f9dd

5 files changed

Lines changed: 304 additions & 3 deletions

File tree

packages/builtin-tools/src/tools/ArtifactTool/ArtifactTool.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
} from './prompt.js'
1111
import { getArtifactsToken, getUploadUrl } from './config.js'
1212
import { uploadArtifact } from './client.js'
13+
import { renderToolResultMessage } from './UI.js'
1314

1415
const inputSchema = lazySchema(() =>
1516
z.strictObject({
@@ -37,11 +38,11 @@ const outputSchema = lazySchema(() =>
3738
id: z.string(),
3839
url: z.string(),
3940
expiresAt: z.string(),
41+
error: z.string().optional(),
4042
}),
4143
)
4244
type OutputSchema = ReturnType<typeof outputSchema>
43-
type ArtifactOutput = z.infer<OutputSchema>
44-
type ArtifactErrorOutput = ArtifactOutput & { error?: string }
45+
export type ArtifactOutput = z.infer<OutputSchema>
4546

4647
export const ArtifactTool = buildTool({
4748
name: ARTIFACT_TOOL_NAME,
@@ -87,7 +88,7 @@ export const ArtifactTool = buildTool({
8788
},
8889

8990
mapToolResultToToolResultBlockParam(
90-
content: ArtifactErrorOutput,
91+
content: ArtifactOutput,
9192
toolUseID: string,
9293
): ToolResultBlockParam {
9394
if (content.error) {
@@ -104,6 +105,7 @@ export const ArtifactTool = buildTool({
104105
content: `Artifact uploaded: ${content.url} (id: ${content.id}, expires: ${content.expiresAt})`,
105106
}
106107
},
108+
renderToolResultMessage,
107109

108110
async call(input: ArtifactInput) {
109111
const { file_path, hash, ttl } = input
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import * as React from 'react';
2+
import { Box, Link, Text } from '@anthropic/ink';
3+
import type { ToolProgressData } from 'src/Tool.js';
4+
import type { ProgressMessage } from 'src/types/message.js';
5+
import type { ArtifactOutput } from './ArtifactTool.js';
6+
7+
export function renderToolResultMessage(
8+
content: ArtifactOutput,
9+
_progressMessagesForMessage: ProgressMessage<ToolProgressData>[],
10+
_options: { verbose: boolean; theme?: string },
11+
): React.ReactNode {
12+
if (content.error) {
13+
return (
14+
<Box>
15+
<Text color="error">⚠ Artifact upload failed: {content.error}</Text>
16+
</Box>
17+
);
18+
}
19+
if (!content.url) return null;
20+
return (
21+
<Box flexDirection="column">
22+
<Box>
23+
<Text>
24+
<Text color="success"></Text> Artifact uploaded:{' '}
25+
<Link url={content.url}>
26+
<Text color="warning">{content.url}</Text>
27+
</Link>
28+
</Text>
29+
</Box>
30+
{content.expiresAt ? (
31+
<Box>
32+
<Text dimColor>expires: {content.expiresAt}</Text>
33+
</Box>
34+
) : null}
35+
</Box>
36+
);
37+
}
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
import { describe, expect, test } from 'bun:test';
2+
import * as React from 'react';
3+
import type { ProgressMessage } from 'src/types/message.js';
4+
import type { ToolProgressData } from 'src/Tool.js';
5+
import { renderToolResultMessage } from '../UI.js';
6+
import type { ArtifactOutput } from '../ArtifactTool.js';
7+
8+
const NO_PROGRESS: ProgressMessage<ToolProgressData>[] = [];
9+
const OPTIONS = { verbose: false, theme: 'dark' } as never;
10+
11+
/** Walk a React element tree and concatenate all string/number children. */
12+
function extractText(node: React.ReactNode): string {
13+
if (node == null || typeof node === 'boolean') return '';
14+
if (typeof node === 'string') return node;
15+
if (typeof node === 'number') return String(node);
16+
if (Array.isArray(node)) return node.map(extractText).join('');
17+
if (React.isValidElement(node)) {
18+
const children = (node.props as { children?: React.ReactNode }).children;
19+
return extractText(children);
20+
}
21+
return '';
22+
}
23+
24+
describe('ArtifactTool UI.renderToolResultMessage', () => {
25+
test('renders the uploaded URL and expiry on success', () => {
26+
const content: ArtifactOutput = {
27+
id: 'abc123',
28+
url: 'https://cloud-artifacts.claude-code-best.win/7d/abc123.html',
29+
expiresAt: '2026-06-27T10:00:00.000Z',
30+
};
31+
const node = renderToolResultMessage(content, NO_PROGRESS, OPTIONS);
32+
expect(React.isValidElement(node)).toBe(true);
33+
const text = extractText(node);
34+
expect(text).toContain(content.url);
35+
expect(text).toContain(content.expiresAt);
36+
expect(text).toContain('Artifact uploaded');
37+
});
38+
39+
test('renders the error message on failure', () => {
40+
const content: ArtifactOutput = {
41+
id: '',
42+
url: '',
43+
expiresAt: '',
44+
error: 'File does not exist or is not readable: /tmp/missing.html',
45+
};
46+
const node = renderToolResultMessage(content, NO_PROGRESS, OPTIONS);
47+
expect(React.isValidElement(node)).toBe(true);
48+
const text = extractText(node);
49+
expect(text).toContain('Artifact upload failed');
50+
expect(text).toContain('/tmp/missing.html');
51+
});
52+
53+
test('returns null when url is empty without error', () => {
54+
const content: ArtifactOutput = { id: '', url: '', expiresAt: '' };
55+
const node = renderToolResultMessage(content, NO_PROGRESS, OPTIONS);
56+
expect(node).toBeNull();
57+
});
58+
59+
test('omits the expiry line when expiresAt is empty', () => {
60+
const content: ArtifactOutput = {
61+
id: 'abc',
62+
url: 'https://cloud-artifacts.claude-code-best.win/7d/abc.html',
63+
expiresAt: '',
64+
};
65+
const node = renderToolResultMessage(content, NO_PROGRESS, OPTIONS);
66+
expect(React.isValidElement(node)).toBe(true);
67+
// Sanity: still renders URL even without expiry
68+
expect(extractText(node)).toContain(content.url);
69+
});
70+
});

packages/builtin-tools/src/tools/ExecuteTool/ExecuteTool.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,4 +236,29 @@ export const ExecuteTool = buildTool({
236236
content: JSON.stringify(content),
237237
}
238238
},
239+
// Output shape: { result: <inner tool output>, tool_name: string }.
240+
// Delegate rendering to the inner tool when it defines its own
241+
// renderToolResultMessage so deferred tools can show their own UI
242+
// (e.g. ArtifactTool displays its uploaded URL). Without this, the
243+
// ExecuteExtraTool tool_result row renders nothing below the tool_use
244+
// line. The inner tool expects its own input shape, so unwrap params.
245+
//
246+
// Inline the lookup rather than calling findToolByName — deferred tools
247+
// are matched by exact name (no aliases needed), and avoiding the
248+
// shared helper keeps this method resilient to src/Tool.js mocks in
249+
// co-located test files (process-global mock.module pollution).
250+
renderToolResultMessage(content, progressMessages, options) {
251+
const innerTool = options.tools.find(t => t.name === content.tool_name)
252+
if (!innerTool?.renderToolResultMessage) return null
253+
const innerInput = (options.input as { params?: unknown } | undefined)
254+
?.params
255+
return innerTool.renderToolResultMessage(
256+
content.result as never,
257+
progressMessages,
258+
{
259+
...options,
260+
input: innerInput,
261+
},
262+
)
263+
},
239264
} satisfies ToolDef<InputSchema, Output>)
Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
import { describe, expect, test, mock } from 'bun:test'
2+
import { logMock } from '../../../../../../tests/mocks/log'
3+
import { debugMock } from '../../../../../../tests/mocks/debug'
4+
5+
// Same mock setup as ExecuteTool.runner.ts — ExecuteTool's import chain
6+
// (growthbook, searchExtraTools, messages) loads real modules with side
7+
// effects otherwise. mock.module is process-global; identical setup in
8+
// sibling test files in this directory is safe (last-write-wins, same stubs).
9+
mock.module('src/utils/log.ts', logMock)
10+
mock.module('src/utils/debug.ts', debugMock)
11+
12+
mock.module('src/services/analytics/growthbook.js', () => ({
13+
getFeatureValue_CACHED_MAY_BE_STALE: () => false,
14+
checkStatsigFeatureGate_CACHED_MAY_BE_STALE: () => false,
15+
getFeatureValue_DEPRECATED: async () => undefined,
16+
getFeatureValue_CACHED_WITH_REFRESH: async () => undefined,
17+
hasGrowthBookEnvOverride: () => false,
18+
getAllGrowthBookFeatures: () => ({}),
19+
getGrowthBookConfigOverrides: () => ({}),
20+
setGrowthBookConfigOverride: () => {},
21+
clearGrowthBookConfigOverrides: () => {},
22+
getApiBaseUrlHost: () => undefined,
23+
onGrowthBookRefresh: () => {},
24+
initializeGrowthBook: async () => {},
25+
checkSecurityRestrictionGate: async () => false,
26+
checkGate_CACHED_OR_BLOCKING: async () => false,
27+
refreshGrowthBookAfterAuthChange: () => {},
28+
resetGrowthBook: () => {},
29+
refreshGrowthBookFeatures: async () => {},
30+
setupPeriodicGrowthBookRefresh: () => {},
31+
stopPeriodicGrowthBookRefresh: () => {},
32+
}))
33+
34+
mock.module('src/utils/searchExtraTools.js', () => ({
35+
isSearchExtraToolsEnabledOptimistic: () => true,
36+
getAutoSearchExtraToolsCharThreshold: () => 100,
37+
getSearchExtraToolsMode: () => 'tst' as const,
38+
isSearchExtraToolsToolAvailable: () => true,
39+
isSearchExtraToolsEnabled: async () => true,
40+
isToolReferenceBlock: () => false,
41+
extractDiscoveredToolNames: () => new Set<string>(),
42+
isDeferredToolsDeltaEnabled: () => false,
43+
getDeferredToolsDelta: () => null,
44+
}))
45+
46+
mock.module('src/constants/tools.js', () => ({
47+
CORE_TOOLS: new Set(['ExecuteExtraTool', 'SearchExtraTools']),
48+
}))
49+
50+
mock.module('src/utils/messages.js', () => ({
51+
createUserMessage: ({ content }: { content: string }) => ({
52+
type: 'user' as const,
53+
content,
54+
uuid: 'test-uuid',
55+
}),
56+
INTERRUPT_MESSAGE_FOR_TOOL_USE: '[Request interrupted]',
57+
}))
58+
59+
mock.module('src/utils/toolErrors.js', () => ({
60+
formatZodValidationError: (_name: string, error: unknown) =>
61+
`validation error: ${JSON.stringify(error)}`,
62+
}))
63+
64+
const { ExecuteTool } = await import('../ExecuteTool.js')
65+
66+
type RenderResult = React.ReactNode
67+
68+
describe('ExecuteTool.renderToolResultMessage delegation', () => {
69+
test('delegates to inner tool with content.result and unwrapped params', () => {
70+
const seen: Array<{
71+
content: unknown
72+
input: unknown
73+
}> = []
74+
const innerRender = (
75+
content: unknown,
76+
_progress: unknown,
77+
options: { input?: unknown },
78+
): RenderResult => {
79+
seen.push({ content, input: options.input })
80+
return 'RENDERED' as unknown as RenderResult
81+
}
82+
const innerTool = {
83+
name: 'artifact',
84+
renderToolResultMessage: innerRender,
85+
}
86+
const tools = [innerTool] as never
87+
88+
const result = ExecuteTool.renderToolResultMessage(
89+
{
90+
result: {
91+
id: 'abc',
92+
url: 'https://example.com/x.html',
93+
expiresAt: 'T',
94+
},
95+
tool_name: 'artifact',
96+
},
97+
[],
98+
{
99+
tools,
100+
input: {
101+
tool_name: 'artifact',
102+
params: { file_path: '/tmp/x.html', ttl: 7 },
103+
},
104+
} as never,
105+
)
106+
107+
expect(result).toBe('RENDERED')
108+
expect(seen).toHaveLength(1)
109+
expect(seen[0]?.content).toEqual({
110+
id: 'abc',
111+
url: 'https://example.com/x.html',
112+
expiresAt: 'T',
113+
})
114+
// Inner tool should see its own params shape, not the ExecuteExtraTool wrapper
115+
expect(seen[0]?.input).toEqual({ file_path: '/tmp/x.html', ttl: 7 })
116+
})
117+
118+
test('returns null when inner tool has no renderToolResultMessage', () => {
119+
const innerTool = { name: 'bare' }
120+
const tools = [innerTool] as never
121+
122+
const result = ExecuteTool.renderToolResultMessage(
123+
{ result: { ok: true }, tool_name: 'bare' },
124+
[],
125+
{ tools, input: { tool_name: 'bare', params: {} } } as never,
126+
)
127+
128+
expect(result).toBeNull()
129+
})
130+
131+
test('returns null when inner tool is not found in tools list', () => {
132+
const tools = [] as never
133+
134+
const result = ExecuteTool.renderToolResultMessage(
135+
{ result: { ok: true }, tool_name: 'missing' },
136+
[],
137+
{ tools, input: { tool_name: 'missing', params: {} } } as never,
138+
)
139+
140+
expect(result).toBeNull()
141+
})
142+
143+
test('passes through undefined input safely when input is missing', () => {
144+
const seen: unknown[] = []
145+
const innerTool = {
146+
name: 'artifact',
147+
renderToolResultMessage: (
148+
_content: unknown,
149+
_progress: unknown,
150+
options: { input?: unknown },
151+
): RenderResult => {
152+
seen.push(options.input)
153+
return null
154+
},
155+
}
156+
const tools = [innerTool] as never
157+
158+
const result = ExecuteTool.renderToolResultMessage(
159+
{ result: { ok: true }, tool_name: 'artifact' },
160+
[],
161+
{ tools } as never,
162+
)
163+
164+
expect(result).toBeNull()
165+
expect(seen[0]).toBeUndefined()
166+
})
167+
})

0 commit comments

Comments
 (0)