-
Notifications
You must be signed in to change notification settings - Fork 15.7k
feat: 支持 SearchExtraTools 能力以替代 Tool Search #442
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7be08f5
4fc95bd
8c157f0
c14b7ea
3ac866b
af0d7dc
bd22538
2cf18c4
547ce9e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,147 @@ | ||
| import { z } from 'zod/v4' | ||
| import { | ||
| buildTool, | ||
| findToolByName, | ||
| type Tool, | ||
| type ToolDef, | ||
| type ToolUseContext, | ||
| type ToolResult, | ||
| type Tools, | ||
| } from 'src/Tool.js' | ||
| import { lazySchema } from 'src/utils/lazySchema.js' | ||
| import { createUserMessage } from 'src/utils/messages.js' | ||
| import { DESCRIPTION, getPrompt } from './prompt.js' | ||
| import { EXECUTE_TOOL_NAME } from './constants.js' | ||
|
|
||
| export const inputSchema = lazySchema(() => | ||
| z.object({ | ||
| tool_name: z | ||
| .string() | ||
| .describe( | ||
| 'The exact name of the target tool to execute (e.g., "CronCreate", "mcp__server__action")', | ||
| ), | ||
| params: z | ||
| .record(z.string(), z.unknown()) | ||
| .describe('The parameters to pass to the target tool'), | ||
| }), | ||
| ) | ||
| type InputSchema = ReturnType<typeof inputSchema> | ||
|
|
||
| export const outputSchema = lazySchema(() => | ||
| z.object({ | ||
| result: z.unknown(), | ||
| tool_name: z.string(), | ||
| }), | ||
| ) | ||
| type OutputSchema = ReturnType<typeof outputSchema> | ||
|
|
||
| export type Output = z.infer<OutputSchema> | ||
|
|
||
| export const ExecuteTool = buildTool({ | ||
| name: EXECUTE_TOOL_NAME, | ||
| searchHint: 'execute run invoke call a deferred tool by name with parameters', | ||
| maxResultSizeChars: 100_000, | ||
| isConcurrencySafe() { | ||
| return false | ||
| }, | ||
| get inputSchema(): InputSchema { | ||
| return inputSchema() | ||
| }, | ||
| get outputSchema(): OutputSchema { | ||
| return outputSchema() | ||
| }, | ||
| async description() { | ||
| return DESCRIPTION | ||
| }, | ||
| async prompt() { | ||
| return getPrompt() | ||
| }, | ||
| async call(input, context, canUseTool, parentMessage, onProgress) { | ||
| const tools: Tools = context.options.tools ?? [] | ||
|
|
||
| const targetTool = findToolByName(tools, input.tool_name) | ||
| if (!targetTool) { | ||
| return { | ||
| data: { | ||
| result: null, | ||
| tool_name: input.tool_name, | ||
| }, | ||
| newMessages: [ | ||
| createUserMessage({ | ||
| content: `Tool "${input.tool_name}" not found. Use SearchExtraTools to discover available tools.`, | ||
| }), | ||
| ], | ||
| } | ||
| } | ||
|
|
||
| // Check if the target tool is currently enabled | ||
| if (!targetTool.isEnabled()) { | ||
| return { | ||
| data: { | ||
| result: null, | ||
| tool_name: input.tool_name, | ||
| }, | ||
| newMessages: [ | ||
| createUserMessage({ | ||
| content: `工具 "${input.tool_name}" 当前不可用:Remote Control 未连接。`, | ||
| }), | ||
| ], | ||
| } | ||
| } | ||
|
|
||
| // Check permissions on the target tool | ||
| const permResult = await targetTool.checkPermissions?.( | ||
| input.params as Record<string, unknown>, | ||
| context, | ||
| ) | ||
| if (permResult && permResult.behavior === 'deny') { | ||
| return { | ||
| data: { | ||
| result: null, | ||
| tool_name: input.tool_name, | ||
| }, | ||
| newMessages: [ | ||
| createUserMessage({ | ||
| content: `Permission denied for tool "${input.tool_name}": ${permResult.message ?? 'Permission denied'}`, | ||
| }), | ||
| ], | ||
| } | ||
| } | ||
|
Comment on lines
+93
to
+109
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Permission gating is incomplete for delegated execution. This path blocks only explicit Suggested guard pattern- if (permResult && permResult.behavior === 'deny') {
+ if (
+ permResult &&
+ permResult.behavior !== 'allow' &&
+ permResult.behavior !== 'passthrough'
+ ) {
return {
data: {
result: null,
tool_name: input.tool_name,
},
newMessages: [
createUserMessage({
- content: `Permission denied for tool "${input.tool_name}": ${permResult.message ?? 'Permission denied'}`,
+ content: `Tool "${input.tool_name}" requires permission before execution: ${permResult.message ?? 'Permission required'}`,
}),
],
}
}🤖 Prompt for AI Agents |
||
|
|
||
| // Delegate execution to the target tool | ||
| const targetResult: ToolResult<unknown> = await targetTool.call( | ||
| input.params as Record<string, unknown>, | ||
| context, | ||
| canUseTool, | ||
| parentMessage, | ||
| onProgress, | ||
| ) | ||
|
|
||
| return { | ||
| ...targetResult, | ||
| data: { | ||
| result: targetResult.data, | ||
| tool_name: input.tool_name, | ||
| }, | ||
| } | ||
| }, | ||
| async checkPermissions() { | ||
| return { | ||
| behavior: 'passthrough', | ||
| message: 'ExecuteExtraTool delegates permission to the target tool.', | ||
| } | ||
| }, | ||
| renderToolUseMessage(input) { | ||
| return `Executing ${input.tool_name}...` | ||
| }, | ||
| userFacingName() { | ||
| return 'ExecuteExtraTool' | ||
| }, | ||
| mapToolResultToToolResultBlockParam(content, toolUseID) { | ||
| return { | ||
| tool_use_id: toolUseID, | ||
| type: 'tool_result', | ||
| content: JSON.stringify(content), | ||
| } | ||
|
Comment on lines
+140
to
+145
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Inspect ToolDef signature and tool_result content expectations:"
fd -i "Tool.ts" -x sh -c 'echo "---- $1 ----"; sed -n "1,260p" "$1"' sh {}
echo
echo "Compare mapToolResultToToolResultBlockParam implementations across tools:"
rg -n "mapToolResultToToolResultBlockParam\\(" --type=ts --type=tsx -C3
echo
echo "Inspect tool_result payload construction callsites:"
rg -n "tool_use_id|type:\\s*'tool_result'|content:" --type=ts --type=tsx -C2Repository: claude-code-best/claude-code Length of output: 50431 🏁 Script executed: # Find ToolResultBlockParam type definition
rg -n "type ToolResultBlockParam|interface ToolResultBlockParam" --type=ts --type=tsx -A 10
# Check ExecuteTool implementation context
rg -n "class ExecuteTool|export.*ExecuteTool" --type=ts --type=tsx -B 2 -A 15
# Find where ExecuteTool.mapToolResultToToolResultBlockParam is called
rg -n "mapToolResultToToolResultBlockParam" packages/builtin-tools/src/tools/ExecuteTool/ExecuteTool.ts -B 10 -A 10Repository: claude-code-best/claude-code Length of output: 697 🏁 Script executed: # Find ToolResultBlockParam type definition in src/Tool
fd -name "Tool.ts" -o -name "Tool.js" | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && rg "ToolResultBlockParam" {} -B 2 -A 8'
# Also search for content property type in tool_result contexts
rg "type.*ToolResultBlockParam|interface.*ToolResultBlockParam" -A 10Repository: claude-code-best/claude-code Length of output: 44189 🏁 Script executed: # Find ToolResultBlockParam type definition from SDK
rg "ToolResultBlockParam" src/Tool.ts -A 5
# Check the ExecuteTool file to understand the full context
fd -name "ExecuteTool.ts" | head -1 | xargs cat | head -200
# Search for examples of structured content blocks
rg "content.*\[" --type=ts --type=tsx | grep -i "tool_result\|type.*text" | head -20Repository: claude-code-best/claude-code Length of output: 1595 🏁 Script executed: # Read ExecuteTool.ts file
fd -name "ExecuteTool.ts" -path "*/ExecuteTool/*" | xargs cat | head -250
# Also check what ToolResultBlockParam content type looks like from SDK
# by searching for examples in the codebase
rg "content:.*\[.*type.*text" --type=ts --type=tsx -A 2 | head -30Repository: claude-code-best/claude-code Length of output: 331 🏁 Script executed: # Find the ExecuteTool file using git
git ls-files | grep -i executetool
# Read the file
cat packages/builtin-tools/src/tools/ExecuteTool/ExecuteTool.ts | head -250
# Search for examples where content is structured (array with type/text)
rg 'content:.*\[' packages/builtin-tools/src/tools -A 3 | head -40Repository: claude-code-best/claude-code Length of output: 7617 🏁 Script executed: # Check what ToolResultBlockParam type actually allows for content
python3 << 'EOF'
import json
# Based on the code patterns, let me search for how content is actually used
# in the codebase to understand what type it should be
import subprocess
result = subprocess.run(
['rg', 'type.*content.*string|.*content.*array|ContentBlock', '--type=ts', '--type=tsx', '-A', '2'],
capture_output=True,
text=True
)
# Look for actual usage patterns
lines = result.stdout.split('\n')
for i, line in enumerate(lines[:100]):
if 'content' in line.lower():
print(line)
EOF
# Check SDK types by looking at how content is used in actual implementations
rg 'content:.*string|content:.*\[' packages/builtin-tools/src/tools -B 3 -A 1 | head -80Repository: claude-code-best/claude-code Length of output: 6276 🏁 Script executed: # Look at TeamDeleteTool and TeamCreateTool to see the proper pattern
rg "mapToolResultToToolResultBlockParam" packages/builtin-tools/src/tools/TeamDeleteTool packages/builtin-tools/src/tools/TeamCreateTool -A 10 | head -50
# Check how delegated tool outputs flow through the system
rg "targetResult.*data|tool_result.*content" packages/builtin-tools/src/tools/ExecuteTool -B 3 -A 3
# Verify if ExecuteTool's output type matches what it passes to mapToolResultToToolResultBlockParam
rg "Output.*result|type.*Output" packages/builtin-tools/src/tools/ExecuteTool/ExecuteTool.ts -B 2 -A 2Repository: claude-code-best/claude-code Length of output: 2745 Preserve structured ExecuteTool wraps delegated tool outputs, but 🤖 Prompt for AI Agents |
||
| }, | ||
| } satisfies ToolDef<InputSchema, Output>) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,165 @@ | ||
| import { describe, test, expect } from 'bun:test' | ||
| import { mock } from 'bun:test' | ||
| import { logMock } from '../../../../../../tests/mocks/log' | ||
| import { debugMock } from '../../../../../../tests/mocks/debug' | ||
|
|
||
| mock.module('src/utils/log.ts', logMock) | ||
| mock.module('src/utils/debug.ts', debugMock) | ||
|
|
||
| // Mock all heavy dependencies before importing ExecuteTool | ||
| 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: async () => 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 messages module | ||
| mock.module('src/utils/messages.js', () => ({ | ||
| createUserMessage: ({ content }: { content: string }) => ({ | ||
| type: 'user' as const, | ||
| content, | ||
| uuid: 'test-uuid', | ||
| }), | ||
| })) | ||
|
|
||
| const { ExecuteTool } = await import('../ExecuteTool.js') | ||
| const { EXECUTE_TOOL_NAME } = await import('../constants.js') | ||
|
|
||
| function makeContext(tools: unknown[] = []) { | ||
| return { | ||
| options: { | ||
| tools, | ||
| }, | ||
| cwd: '/tmp', | ||
| sessionId: 'test', | ||
| } as never | ||
| } | ||
|
|
||
| function makeMockTool(name: string, callResult: unknown = 'ok') { | ||
| return { | ||
| name, | ||
| call: async () => ({ data: callResult }), | ||
| checkPermissions: async () => ({ behavior: 'allow' as const }), | ||
| prompt: async () => `Description for ${name}`, | ||
| description: async () => `Description for ${name}`, | ||
| inputSchema: {}, | ||
| isEnabled: () => true, | ||
| isConcurrencySafe: () => true, | ||
| isReadOnly: () => false, | ||
| isMcp: false, | ||
| alwaysLoad: undefined, | ||
| shouldDefer: undefined, | ||
| searchHint: '', | ||
| userFacingName: () => name, | ||
| renderToolUseMessage: () => `Running ${name}`, | ||
| mapToolResultToToolResultBlockParam: (content: unknown, id: string) => ({ | ||
| tool_use_id: id, | ||
| type: 'tool_result', | ||
| content, | ||
| }), | ||
| } | ||
| } | ||
|
|
||
| describe('ExecuteTool', () => { | ||
| test('executes a target tool by name', async () => { | ||
| const mockTarget = makeMockTool('TestTool', { result: 'success' }) | ||
| const ctx = makeContext([mockTarget]) | ||
|
|
||
| const result = await ExecuteTool.call( | ||
| { tool_name: 'TestTool', params: {} }, | ||
| ctx, | ||
| async () => ({ behavior: 'allow' }), | ||
| { type: 'assistant', content: [], uuid: 'msg1' } as never, | ||
| undefined, | ||
| ) | ||
|
|
||
| expect(result.data).toEqual({ | ||
| result: { result: 'success' }, | ||
| tool_name: 'TestTool', | ||
| }) | ||
| }) | ||
|
|
||
| test('returns error when tool not found', async () => { | ||
| const ctx = makeContext([]) | ||
|
|
||
| const result = await ExecuteTool.call( | ||
| { tool_name: 'NonexistentTool', params: {} }, | ||
| ctx, | ||
| async () => ({ behavior: 'allow' }), | ||
| { type: 'assistant', content: [], uuid: 'msg1' } as never, | ||
| undefined, | ||
| ) | ||
|
|
||
| expect(result.data).toEqual({ | ||
| result: null, | ||
| tool_name: 'NonexistentTool', | ||
| }) | ||
| expect(result.newMessages).toBeDefined() | ||
| expect(result.newMessages!.length).toBeGreaterThan(0) | ||
| }) | ||
|
|
||
| test('returns permission denied when target denies', async () => { | ||
| const mockTarget = makeMockTool('SecretTool', 'secret') | ||
| mockTarget.checkPermissions = async () => | ||
| ({ | ||
| behavior: 'deny' as const, | ||
| message: 'Access denied', | ||
| }) as never | ||
| const ctx = makeContext([mockTarget]) | ||
|
|
||
| const result = await ExecuteTool.call( | ||
| { tool_name: 'SecretTool', params: {} }, | ||
| ctx, | ||
| async () => ({ behavior: 'allow' }), | ||
| { type: 'assistant', content: [], uuid: 'msg1' } as never, | ||
| undefined, | ||
| ) | ||
|
|
||
| expect(result.data).toEqual({ | ||
| result: null, | ||
| tool_name: 'SecretTool', | ||
| }) | ||
| expect(result.newMessages).toBeDefined() | ||
| }) | ||
|
|
||
| test('has correct name', () => { | ||
| expect(ExecuteTool.name).toBe(EXECUTE_TOOL_NAME) | ||
| }) | ||
|
|
||
| test('searchHint contains keywords', () => { | ||
| expect(ExecuteTool.searchHint).toContain('execute') | ||
| expect(ExecuteTool.searchHint).toContain('tool') | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Brief enablement logic bypasses opt-in and entitlement checks.
Switching from
isBriefEnabled()toisBridgeEnabled()removes all user controls that gate Brief activation:Opt-in requirement removed: Users who haven't explicitly opted in via
--brief,defaultView: 'chat',/brief, orCLAUDE_CODE_BRIEFenv var will now see the tool whenever the bridge is enabled.GrowthBook gate bypassed: The
tengu_kairos_brieffeature flag (refreshed every 5 minutes per line 98) is no longer checked, potentially exposing the tool outside the intended experiment cohort.Documentation mismatch: Lines 103-126 describe
isBriefEnabled()as the "unified activation gate" that "governs model-facing behavior as a unit: tool availability, system prompt section, tool-deferral bypass, and todo-nag suppression." That function is still exported but no longer used by the tool.If the intent is to gate on bridge availability in addition to the existing checks, the fix should be:
isEnabled() { - return isBridgeEnabled() + return isBriefEnabled() && isBridgeEnabled() },If the intent is to replace all Brief-specific gates with a simple bridge check, then
isBriefEnabled()should be removed and its call sites updated to prevent confusion.🤖 Prompt for AI Agents