diff --git a/README.md b/README.md index 683bf4e..52369d4 100644 --- a/README.md +++ b/README.md @@ -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 } diff --git a/src/agent/runner.ts b/src/agent/runner.ts index 35522bc..f1b6030 100644 --- a/src/agent/runner.ts +++ b/src/agent/runner.ts @@ -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 +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] } // ------------------------------------------------------------------------- diff --git a/tests/tool-filtering.test.ts b/tests/tool-filtering.test.ts index f268599..5cf09c0 100644 --- a/tests/tool-filtering.test.ts +++ b/tests/tool-filtering.test.ts @@ -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',