fix: update filtering logic to allow custom tools
This commit is contained in:
parent
a8e9bdcacd
commit
ff008aad31
|
|
@ -199,19 +199,19 @@ Predefined tool sets for common use cases:
|
|||
const readonlyAgent: AgentConfig = {
|
||||
name: 'reader',
|
||||
model: 'claude-sonnet-4-6',
|
||||
toolPreset: 'readonly', // file_read, grep
|
||||
toolPreset: 'readonly', // file_read, grep, glob
|
||||
}
|
||||
|
||||
const readwriteAgent: AgentConfig = {
|
||||
name: 'editor',
|
||||
model: 'claude-sonnet-4-6',
|
||||
toolPreset: 'readwrite', // file_read, file_write, file_edit, grep
|
||||
toolPreset: 'readwrite', // file_read, file_write, file_edit, grep, glob
|
||||
}
|
||||
|
||||
const fullAgent: AgentConfig = {
|
||||
name: 'executor',
|
||||
model: 'claude-sonnet-4-6',
|
||||
toolPreset: 'full', // all built-in tools including bash
|
||||
toolPreset: 'full', // file_read, file_write, file_edit, grep, glob, bash
|
||||
}
|
||||
```
|
||||
|
||||
|
|
@ -223,7 +223,7 @@ Combine presets with allowlists and denylists for precise control:
|
|||
const customAgent: AgentConfig = {
|
||||
name: 'custom',
|
||||
model: 'claude-sonnet-4-6',
|
||||
toolPreset: 'readwrite', // Start with: file_read, file_write, file_edit, grep
|
||||
toolPreset: 'readwrite', // Start with: file_read, file_write, file_edit, grep, glob
|
||||
tools: ['file_read', 'grep'], // Allowlist: intersect with preset = file_read, grep
|
||||
disallowedTools: ['grep'], // Denylist: subtract = file_read only
|
||||
}
|
||||
|
|
|
|||
|
|
@ -42,11 +42,13 @@ import type { ToolExecutor } from '../tool/executor.js'
|
|||
|
||||
/** Predefined tool sets for common agent use cases. */
|
||||
export const TOOL_PRESETS = {
|
||||
readonly: ['file_read', 'grep'],
|
||||
readwrite: ['file_read', 'file_write', 'file_edit', 'grep'],
|
||||
full: ['file_read', 'file_write', 'file_edit', 'grep', 'bash'],
|
||||
readonly: ['file_read', 'grep', 'glob'],
|
||||
readwrite: ['file_read', 'file_write', 'file_edit', 'grep', 'glob'],
|
||||
full: ['file_read', 'file_write', 'file_edit', 'grep', 'glob', 'bash'],
|
||||
} as const satisfies Record<string, readonly string[]>
|
||||
|
||||
const BUILT_IN_TOOL_NAMES = new Set(TOOL_PRESETS.full as readonly string[])
|
||||
|
||||
/** Framework-level disallowed tools for safety rails. */
|
||||
export const AGENT_FRAMEWORK_DISALLOWED: readonly string[] = [
|
||||
// Empty for now, infrastructure for future built-in tools
|
||||
|
|
@ -213,6 +215,13 @@ export class AgentRunner {
|
|||
*/
|
||||
private resolveTools(): LLMToolDef[] {
|
||||
// Validate configuration for contradictions
|
||||
if (this.options.toolPreset && this.options.allowedTools) {
|
||||
console.warn(
|
||||
'AgentRunner: both toolPreset and allowedTools are set. ' +
|
||||
'Final tool access will be the intersection of both.'
|
||||
)
|
||||
}
|
||||
|
||||
if (this.options.allowedTools && this.options.disallowedTools) {
|
||||
const overlap = this.options.allowedTools.filter(tool =>
|
||||
this.options.disallowedTools!.includes(tool)
|
||||
|
|
@ -225,30 +234,33 @@ export class AgentRunner {
|
|||
}
|
||||
}
|
||||
|
||||
let tools = this.toolRegistry.toToolDefs()
|
||||
const allTools = this.toolRegistry.toToolDefs()
|
||||
const customTools = allTools.filter(t => !BUILT_IN_TOOL_NAMES.has(t.name))
|
||||
let builtInTools = allTools.filter(t => BUILT_IN_TOOL_NAMES.has(t.name))
|
||||
|
||||
// 1. Apply preset filter if set
|
||||
if (this.options.toolPreset) {
|
||||
const presetTools = new Set(TOOL_PRESETS[this.options.toolPreset] as readonly string[])
|
||||
tools = tools.filter(t => presetTools.has(t.name))
|
||||
builtInTools = builtInTools.filter(t => presetTools.has(t.name))
|
||||
}
|
||||
|
||||
// 2. Apply allowlist filter if set
|
||||
if (this.options.allowedTools) {
|
||||
tools = tools.filter(t => this.options.allowedTools!.includes(t.name))
|
||||
builtInTools = builtInTools.filter(t => this.options.allowedTools!.includes(t.name))
|
||||
}
|
||||
|
||||
// 3. Apply denylist filter if set
|
||||
if (this.options.disallowedTools) {
|
||||
const denied = new Set(this.options.disallowedTools)
|
||||
tools = tools.filter(t => !denied.has(t.name))
|
||||
builtInTools = builtInTools.filter(t => !denied.has(t.name))
|
||||
}
|
||||
|
||||
// 4. Apply framework-level safety rails
|
||||
const frameworkDenied = new Set(AGENT_FRAMEWORK_DISALLOWED)
|
||||
tools = tools.filter(t => !frameworkDenied.has(t.name))
|
||||
builtInTools = builtInTools.filter(t => !frameworkDenied.has(t.name))
|
||||
|
||||
return tools
|
||||
// Custom tools stay available regardless of built-in filtering rules.
|
||||
return [...builtInTools, ...customTools]
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -89,15 +89,15 @@ describe('Tool filtering', () => {
|
|||
|
||||
describe('TOOL_PRESETS', () => {
|
||||
it('readonly preset has correct tools', () => {
|
||||
expect(TOOL_PRESETS.readonly).toEqual(['file_read', 'grep'])
|
||||
expect(TOOL_PRESETS.readonly).toEqual(['file_read', 'grep', 'glob'])
|
||||
})
|
||||
|
||||
it('readwrite preset has correct tools', () => {
|
||||
expect(TOOL_PRESETS.readwrite).toEqual(['file_read', 'file_write', 'file_edit', 'grep'])
|
||||
expect(TOOL_PRESETS.readwrite).toEqual(['file_read', 'file_write', 'file_edit', 'grep', 'glob'])
|
||||
})
|
||||
|
||||
it('full preset has correct tools', () => {
|
||||
expect(TOOL_PRESETS.full).toEqual(['file_read', 'file_write', 'file_edit', 'grep', 'bash'])
|
||||
expect(TOOL_PRESETS.full).toEqual(['file_read', 'file_write', 'file_edit', 'grep', 'glob', 'bash'])
|
||||
})
|
||||
})
|
||||
|
||||
|
|
@ -124,7 +124,7 @@ describe('Tool filtering', () => {
|
|||
const tools = (runner as any).resolveTools() as LLMToolDef[]
|
||||
const toolNames = tools.map((t: LLMToolDef) => t.name).sort()
|
||||
|
||||
expect(toolNames).toEqual(['file_read', 'grep'])
|
||||
expect(toolNames).toEqual(['custom_tool', 'file_read', 'grep'])
|
||||
})
|
||||
|
||||
it('readwrite preset filters correctly', () => {
|
||||
|
|
@ -136,7 +136,7 @@ describe('Tool filtering', () => {
|
|||
const tools = (runner as any).resolveTools() as LLMToolDef[]
|
||||
const toolNames = tools.map((t: LLMToolDef) => t.name).sort()
|
||||
|
||||
expect(toolNames).toEqual(['file_edit', 'file_read', 'file_write', 'grep'])
|
||||
expect(toolNames).toEqual(['custom_tool', 'file_edit', 'file_read', 'file_write', 'grep'])
|
||||
})
|
||||
|
||||
it('full preset filters correctly', () => {
|
||||
|
|
@ -148,7 +148,7 @@ describe('Tool filtering', () => {
|
|||
const tools = (runner as any).resolveTools() as LLMToolDef[]
|
||||
const toolNames = tools.map((t: LLMToolDef) => t.name).sort()
|
||||
|
||||
expect(toolNames).toEqual(['bash', 'file_edit', 'file_read', 'file_write', 'grep'])
|
||||
expect(toolNames).toEqual(['bash', 'custom_tool', 'file_edit', 'file_read', 'file_write', 'grep'])
|
||||
})
|
||||
})
|
||||
|
||||
|
|
@ -162,7 +162,7 @@ describe('Tool filtering', () => {
|
|||
const tools = (runner as any).resolveTools() as LLMToolDef[]
|
||||
const toolNames = tools.map((t: LLMToolDef) => t.name).sort()
|
||||
|
||||
expect(toolNames).toEqual(['bash', 'file_read'])
|
||||
expect(toolNames).toEqual(['bash', 'custom_tool', 'file_read'])
|
||||
})
|
||||
|
||||
it('empty allowlist returns no tools', () => {
|
||||
|
|
@ -172,7 +172,7 @@ describe('Tool filtering', () => {
|
|||
})
|
||||
|
||||
const tools = (runner as any).resolveTools()
|
||||
expect(tools).toHaveLength(0)
|
||||
expect((tools as LLMToolDef[]).map(t => t.name)).toEqual(['custom_tool'])
|
||||
})
|
||||
})
|
||||
|
||||
|
|
@ -186,7 +186,7 @@ describe('Tool filtering', () => {
|
|||
const tools = (runner as any).resolveTools() as LLMToolDef[]
|
||||
const toolNames = tools.map((t: LLMToolDef) => t.name).sort()
|
||||
|
||||
expect(toolNames).toEqual(['file_edit', 'file_read', 'file_write', 'grep'])
|
||||
expect(toolNames).toEqual(['custom_tool', 'file_edit', 'file_read', 'file_write', 'grep'])
|
||||
})
|
||||
|
||||
it('empty denylist returns all tools', () => {
|
||||
|
|
@ -215,7 +215,7 @@ describe('Tool filtering', () => {
|
|||
const tools = (runner as any).resolveTools() as LLMToolDef[]
|
||||
const toolNames = tools.map((t: LLMToolDef) => t.name).sort()
|
||||
|
||||
expect(toolNames).toEqual(['file_read', 'grep'])
|
||||
expect(toolNames).toEqual(['custom_tool', 'file_read', 'grep'])
|
||||
})
|
||||
|
||||
it('preset filters first, then allowlist intersects, then denylist subtracts', () => {
|
||||
|
|
@ -230,7 +230,23 @@ describe('Tool filtering', () => {
|
|||
})
|
||||
|
||||
const tools = (runner as any).resolveTools()
|
||||
expect(tools).toHaveLength(0)
|
||||
expect((tools as LLMToolDef[]).map(t => t.name)).toEqual(['custom_tool'])
|
||||
})
|
||||
})
|
||||
|
||||
describe('resolveTools - custom tool behavior', () => {
|
||||
it('always includes custom tools regardless of filtering', () => {
|
||||
const runner = new AgentRunner(mockAdapter, registry, executor, {
|
||||
model: 'test-model',
|
||||
toolPreset: 'readonly',
|
||||
allowedTools: ['file_read'],
|
||||
disallowedTools: ['file_read', 'bash', 'grep'],
|
||||
})
|
||||
|
||||
const tools = (runner as any).resolveTools() as LLMToolDef[]
|
||||
const toolNames = tools.map((t: LLMToolDef) => t.name).sort()
|
||||
|
||||
expect(toolNames).toEqual(['custom_tool'])
|
||||
})
|
||||
})
|
||||
|
||||
|
|
@ -259,6 +275,20 @@ describe('Tool filtering', () => {
|
|||
)
|
||||
})
|
||||
|
||||
it('warns when both toolPreset and allowedTools are set', () => {
|
||||
const runner = new AgentRunner(mockAdapter, registry, executor, {
|
||||
model: 'test-model',
|
||||
toolPreset: 'readonly',
|
||||
allowedTools: ['file_read', 'bash'],
|
||||
})
|
||||
|
||||
;(runner as any).resolveTools()
|
||||
|
||||
expect(consoleWarnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('both toolPreset and allowedTools are set')
|
||||
)
|
||||
})
|
||||
|
||||
it('does not warn when no overlap between allowedTools and disallowedTools', () => {
|
||||
const runner = new AgentRunner(mockAdapter, registry, executor, {
|
||||
model: 'test-model',
|
||||
|
|
|
|||
Loading…
Reference in New Issue