diff --git a/integration-tests/ripgrep-real.test.ts b/integration-tests/ripgrep-real.test.ts index 60f99c8a84..57973e4a70 100644 --- a/integration-tests/ripgrep-real.test.ts +++ b/integration-tests/ripgrep-real.test.ts @@ -76,7 +76,9 @@ describe('ripgrep-real-direct', () => { it('should find matches using the real ripgrep binary', async () => { const invocation = tool.build({ pattern: 'hello' }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toContain('Found 2 matches'); expect(result.llmContent).toContain('file1.txt'); @@ -90,7 +92,9 @@ describe('ripgrep-real-direct', () => { it('should handle no matches correctly', async () => { const invocation = tool.build({ pattern: 'nonexistent_pattern_123' }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toContain('No matches found'); }); @@ -106,7 +110,9 @@ describe('ripgrep-real-direct', () => { pattern: 'hello', include_pattern: '*.js', }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toContain('Found 1 match'); expect(result.llmContent).toContain('script.js'); @@ -124,7 +130,9 @@ describe('ripgrep-real-direct', () => { pattern: 'match', context: 1, }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toContain('Found 1 match'); expect(result.llmContent).toContain('context.txt'); diff --git a/packages/a2a-server/src/commands/memory.ts b/packages/a2a-server/src/commands/memory.ts index f84d57b3fc..73cb6ac754 100644 --- a/packages/a2a-server/src/commands/memory.ts +++ b/packages/a2a-server/src/commands/memory.ts @@ -101,8 +101,8 @@ export class AddMemoryCommand implements Command { const tool = toolRegistry.getTool(result.toolName); if (tool) { const abortController = new AbortController(); - const signal = abortController.signal; - await tool.buildAndExecute(result.toolArgs, signal, undefined, { + const abortSignal = abortController.signal; + await tool.buildAndExecute(result.toolArgs, abortSignal, undefined, { shellExecutionConfig: { sanitizationConfig: DEFAULT_SANITIZATION_CONFIG, sandboxManager: loopContext.sandboxManager, diff --git a/packages/cli/src/acp/acpClient.ts b/packages/cli/src/acp/acpClient.ts index b84c9d6b87..ed83417d56 100644 --- a/packages/cli/src/acp/acpClient.ts +++ b/packages/cli/src/acp/acpClient.ts @@ -1129,7 +1129,9 @@ export class Session { }); } - const toolResult: ToolResult = await invocation.execute(abortSignal); + const toolResult: ToolResult = await invocation.execute({ + abortSignal, + }); const content = toToolCallContent(toolResult); const updateContent: acp.ToolCallContent[] = content ? [content] : []; @@ -1671,7 +1673,7 @@ export class Session { kind: toAcpToolKind(readManyFilesTool.kind), }); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); const content = toToolCallContent(result) || { type: 'content', content: { diff --git a/packages/cli/src/ui/hooks/atCommandProcessor.ts b/packages/cli/src/ui/hooks/atCommandProcessor.ts index 477f9bb02a..512fe952ba 100644 --- a/packages/cli/src/ui/hooks/atCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/atCommandProcessor.ts @@ -533,7 +533,7 @@ async function readLocalFiles( let invocation: AnyToolInvocation | undefined = undefined; try { invocation = readManyFilesTool.build(toolArgs); - const result = await invocation.execute(signal); + const result = await invocation.execute({ abortSignal: signal }); const display: IndividualToolCallDisplay = { callId: `client-read-${userMessageTimestamp}`, name: readManyFilesTool.displayName, diff --git a/packages/core/src/agents/agent-tool.ts b/packages/core/src/agents/agent-tool.ts index fd57e9647e..d24636915c 100644 --- a/packages/core/src/agents/agent-tool.ts +++ b/packages/core/src/agents/agent-tool.ts @@ -11,7 +11,7 @@ import { type ToolResult, BaseToolInvocation, type ToolCallConfirmationDetails, - type ToolLiveOutput, + type ExecuteOptions, } from '../tools/tools.js'; import { type AgentLoopContext } from '../config/agent-loop-context.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; @@ -185,10 +185,8 @@ class DelegateInvocation extends BaseToolInvocation< return invocation.shouldConfirmExecute(abortSignal); } - async execute( - signal: AbortSignal, - updateOutput?: (output: ToolLiveOutput) => void, - ): Promise { + async execute(options: ExecuteOptions): Promise { + const { abortSignal: signal, updateOutput } = options; const hintedParams = this.withUserHints(this.mappedInputs); const invocation = this.buildChildInvocation(hintedParams); @@ -204,7 +202,10 @@ class DelegateInvocation extends BaseToolInvocation< }, async ({ metadata }) => { metadata.input = this.params; - const result = await invocation.execute(signal, updateOutput); + const result = await invocation.execute({ + abortSignal: signal, + updateOutput, + }); metadata.output = result; return result; }, diff --git a/packages/core/src/agents/browser/analyzeScreenshot.test.ts b/packages/core/src/agents/browser/analyzeScreenshot.test.ts index b37bd3666e..cfe1f42e08 100644 --- a/packages/core/src/agents/browser/analyzeScreenshot.test.ts +++ b/packages/core/src/agents/browser/analyzeScreenshot.test.ts @@ -99,7 +99,9 @@ describe('analyzeScreenshot', () => { const invocation = tool.build({ instruction: 'Find the blue submit button', }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); // Verify screenshot was captured expect(browserManager.callTool).toHaveBeenCalledWith( @@ -165,7 +167,7 @@ describe('analyzeScreenshot', () => { const invocation = tool.build({ instruction: 'Find the search bar', }); - await invocation.execute(new AbortController().signal); + await invocation.execute({ abortSignal: new AbortController().signal }); const contentGenerator = config.getContentGenerator(); expect(contentGenerator.generateContent).toHaveBeenCalledWith( @@ -194,7 +196,9 @@ describe('analyzeScreenshot', () => { const invocation = tool.build({ instruction: 'Find the button', }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.error).toBeDefined(); expect(result.llmContent).toContain('Failed to capture screenshot'); @@ -217,7 +221,9 @@ describe('analyzeScreenshot', () => { const invocation = tool.build({ instruction: 'Check the layout', }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.error).toBeDefined(); expect(result.llmContent).toContain('Visual model returned no analysis'); @@ -238,7 +244,9 @@ describe('analyzeScreenshot', () => { const invocation = tool.build({ instruction: 'Find the red error', }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.error).toBeDefined(); expect(result.llmContent).toContain( @@ -261,7 +269,9 @@ describe('analyzeScreenshot', () => { const invocation = tool.build({ instruction: 'Identify the element', }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.error).toBeDefined(); expect(result.llmContent).toContain( @@ -281,7 +291,9 @@ describe('analyzeScreenshot', () => { const invocation = tool.build({ instruction: 'Find something', }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.error).toBeDefined(); expect(result.llmContent).toContain('Visual analysis failed'); diff --git a/packages/core/src/agents/browser/analyzeScreenshot.ts b/packages/core/src/agents/browser/analyzeScreenshot.ts index 91fd5d66d6..7d702b0621 100644 --- a/packages/core/src/agents/browser/analyzeScreenshot.ts +++ b/packages/core/src/agents/browser/analyzeScreenshot.ts @@ -23,6 +23,7 @@ import { Kind, type ToolResult, type ToolInvocation, + type ExecuteOptions, } from '../../tools/tools.js'; import { Environment } from '@google/genai'; import type { MessageBus } from '../../confirmation-bus/message-bus.js'; @@ -80,7 +81,7 @@ class AnalyzeScreenshotInvocation extends BaseToolInvocation< return `Visual analysis: "${instruction}"`; } - async execute(signal: AbortSignal): Promise { + async execute({ abortSignal: signal }: ExecuteOptions): Promise { try { const instruction = String(this.params['instruction'] ?? ''); diff --git a/packages/core/src/agents/browser/browserAgentInvocation.test.ts b/packages/core/src/agents/browser/browserAgentInvocation.test.ts index ac90564f06..8b7b11144c 100644 --- a/packages/core/src/agents/browser/browserAgentInvocation.test.ts +++ b/packages/core/src/agents/browser/browserAgentInvocation.test.ts @@ -223,7 +223,10 @@ describe('BrowserAgentInvocation', () => { const controller = new AbortController(); const updateOutput: (output: ToolLiveOutput) => void = vi.fn(); - const result = await invocation.execute(controller.signal, updateOutput); + const result = await invocation.execute({ + abortSignal: controller.signal, + updateOutput, + }); expect(Array.isArray(result.llmContent)).toBe(true); expect((result.llmContent as Array<{ text: string }>)[0].text).toContain( @@ -242,7 +245,7 @@ describe('BrowserAgentInvocation', () => { const controller = new AbortController(); // Should not throw even with no updateOutput await expect( - invocation.execute(controller.signal), + invocation.execute({ abortSignal: controller.signal }), ).resolves.toBeDefined(); }); @@ -256,7 +259,9 @@ describe('BrowserAgentInvocation', () => { ); const controller = new AbortController(); - const result = await invocation.execute(controller.signal); + const result = await invocation.execute({ + abortSignal: controller.signal, + }); expect(result.error).toBeDefined(); expect(removeInputBlocker).toHaveBeenCalled(); @@ -298,7 +303,10 @@ describe('BrowserAgentInvocation', () => { mockMessageBus, ); - await invocation.execute(new AbortController().signal, updateOutput); + await invocation.execute({ + abortSignal: new AbortController().signal, + updateOutput, + }); const firstCall = updateOutput.mock.calls[0]?.[0] as SubagentProgress; expect(firstCall.isSubagentProgress).toBe(true); @@ -315,7 +323,10 @@ describe('BrowserAgentInvocation', () => { mockMessageBus, ); - await invocation.execute(new AbortController().signal, updateOutput); + await invocation.execute({ + abortSignal: new AbortController().signal, + updateOutput, + }); const lastCall = updateOutput.mock.calls[ updateOutput.mock.calls.length - 1 @@ -334,10 +345,10 @@ describe('BrowserAgentInvocation', () => { mockMessageBus, ); - const executePromise = invocation.execute( - new AbortController().signal, + const executePromise = invocation.execute({ + abortSignal: new AbortController().signal, updateOutput, - ); + }); // Allow createBrowserAgentDefinition to resolve and onActivity to be registered await Promise.resolve(); @@ -377,10 +388,10 @@ describe('BrowserAgentInvocation', () => { mockMessageBus, ); - const executePromise = invocation.execute( - new AbortController().signal, + const executePromise = invocation.execute({ + abortSignal: new AbortController().signal, updateOutput, - ); + }); // Allow createBrowserAgentDefinition to resolve and onActivity to be registered await Promise.resolve(); @@ -424,10 +435,10 @@ describe('BrowserAgentInvocation', () => { mockMessageBus, ); - const executePromise = invocation.execute( - new AbortController().signal, + const executePromise = invocation.execute({ + abortSignal: new AbortController().signal, updateOutput, - ); + }); await Promise.resolve(); await Promise.resolve(); @@ -475,10 +486,10 @@ describe('BrowserAgentInvocation', () => { mockMessageBus, ); - const executePromise = invocation.execute( - new AbortController().signal, + const executePromise = invocation.execute({ + abortSignal: new AbortController().signal, updateOutput, - ); + }); await Promise.resolve(); await Promise.resolve(); @@ -519,10 +530,10 @@ describe('BrowserAgentInvocation', () => { mockMessageBus, ); - const executePromise = invocation.execute( - new AbortController().signal, + const executePromise = invocation.execute({ + abortSignal: new AbortController().signal, updateOutput, - ); + }); await Promise.resolve(); await Promise.resolve(); @@ -564,10 +575,10 @@ describe('BrowserAgentInvocation', () => { mockMessageBus, ); - const executePromise = invocation.execute( - new AbortController().signal, + const executePromise = invocation.execute({ + abortSignal: new AbortController().signal, updateOutput, - ); + }); await Promise.resolve(); await Promise.resolve(); @@ -604,10 +615,10 @@ describe('BrowserAgentInvocation', () => { mockMessageBus, ); - const executePromise = invocation.execute( - new AbortController().signal, + const executePromise = invocation.execute({ + abortSignal: new AbortController().signal, updateOutput, - ); + }); await Promise.resolve(); await Promise.resolve(); @@ -647,10 +658,10 @@ describe('BrowserAgentInvocation', () => { mockMessageBus, ); - const executePromise = invocation.execute( - new AbortController().signal, + const executePromise = invocation.execute({ + abortSignal: new AbortController().signal, updateOutput, - ); + }); await Promise.resolve(); await Promise.resolve(); @@ -703,7 +714,10 @@ describe('BrowserAgentInvocation', () => { mockParams, mockMessageBus, ); - await invocation.execute(new AbortController().signal, vi.fn()); + await invocation.execute({ + abortSignal: new AbortController().signal, + updateOutput: vi.fn(), + }); expect(recordBrowserAgentTaskOutcome).toHaveBeenCalledWith( mockConfig, @@ -731,7 +745,10 @@ describe('BrowserAgentInvocation', () => { mockMessageBus, ); - await invocation.execute(new AbortController().signal, updateOutput); + await invocation.execute({ + abortSignal: new AbortController().signal, + updateOutput, + }); expect(recordBrowserAgentTaskOutcome).toHaveBeenCalledWith( mockConfig, @@ -751,7 +768,10 @@ describe('BrowserAgentInvocation', () => { mockParams, mockMessageBus, ); - await invocation.execute(new AbortController().signal, vi.fn()); + await invocation.execute({ + abortSignal: new AbortController().signal, + updateOutput: vi.fn(), + }); expect(cleanupBrowserAgent).not.toHaveBeenCalled(); }); @@ -807,7 +827,7 @@ describe('BrowserAgentInvocation', () => { mockMessageBus, ); - await invocation.execute(new AbortController().signal); + await invocation.execute({ abortSignal: new AbortController().signal }); // Verify list_pages was called expect(mockBrowserManager.callTool).toHaveBeenCalledWith( diff --git a/packages/core/src/agents/browser/browserAgentInvocation.ts b/packages/core/src/agents/browser/browserAgentInvocation.ts index e71d82cf55..a59ffc25b5 100644 --- a/packages/core/src/agents/browser/browserAgentInvocation.ts +++ b/packages/core/src/agents/browser/browserAgentInvocation.ts @@ -22,7 +22,7 @@ import { LocalAgentExecutor } from '../local-executor.js'; import { BaseToolInvocation, type ToolResult, - type ToolLiveOutput, + type ExecuteOptions, } from '../../tools/tools.js'; import { ToolErrorType } from '../../tools/tool-error.js'; import { @@ -107,10 +107,8 @@ export class BrowserAgentInvocation extends BaseToolInvocation< * 3. Runs the agent via LocalAgentExecutor * 4. Cleans up browser resources */ - async execute( - signal: AbortSignal, - updateOutput?: (output: ToolLiveOutput) => void, - ): Promise { + async execute(options: ExecuteOptions): Promise { + const { abortSignal: signal, updateOutput } = options; const invocationStartMs = Date.now(); let browserManager; let recentActivity: SubagentActivityItem[] = []; diff --git a/packages/core/src/agents/browser/mcpToolWrapper.test.ts b/packages/core/src/agents/browser/mcpToolWrapper.test.ts index 7a03a1daec..86d88fbd8a 100644 --- a/packages/core/src/agents/browser/mcpToolWrapper.test.ts +++ b/packages/core/src/agents/browser/mcpToolWrapper.test.ts @@ -139,7 +139,7 @@ describe('mcpToolWrapper', () => { ); const invocation = tools[1].build({ uid: 'elem-123' }); - await invocation.execute(new AbortController().signal); + await invocation.execute({ abortSignal: new AbortController().signal }); expect(mockBrowserManager.callTool).toHaveBeenCalledWith( 'click', @@ -158,7 +158,9 @@ describe('mcpToolWrapper', () => { ); const invocation = tools[0].build({ verbose: true }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toBe('Tool result'); expect(result.error).toBeUndefined(); @@ -177,7 +179,9 @@ describe('mcpToolWrapper', () => { ); const invocation = tools[1].build({ uid: 'invalid' }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.error).toBeDefined(); expect(result.error?.message).toBe('Element not found'); @@ -195,7 +199,9 @@ describe('mcpToolWrapper', () => { ); const invocation = tools[0].build({}); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.error).toBeDefined(); expect(result.error?.message).toBe('Connection lost'); @@ -212,7 +218,7 @@ describe('mcpToolWrapper', () => { const clickTool = tools.find((t) => t.name === 'click')!; const invocation = clickTool.build({ uid: 'elem-42' }); - await invocation.execute(new AbortController().signal); + await invocation.execute({ abortSignal: new AbortController().signal }); // callTool: suspend blocker + click + resume blocker expect(mockBrowserManager.callTool).toHaveBeenCalledTimes(3); @@ -257,7 +263,7 @@ describe('mcpToolWrapper', () => { const snapshotTool = tools.find((t) => t.name === 'take_snapshot')!; const invocation = snapshotTool.build({}); - await invocation.execute(new AbortController().signal); + await invocation.execute({ abortSignal: new AbortController().signal }); // callTool should only be called once for take_snapshot — no suspend/resume expect(mockBrowserManager.callTool).toHaveBeenCalledTimes(1); @@ -277,7 +283,7 @@ describe('mcpToolWrapper', () => { const clickTool = tools.find((t) => t.name === 'click')!; const invocation = clickTool.build({ uid: 'elem-42' }); - await invocation.execute(new AbortController().signal); + await invocation.execute({ abortSignal: new AbortController().signal }); // callTool should only be called once for click — no suspend/resume expect(mockBrowserManager.callTool).toHaveBeenCalledTimes(1); @@ -297,7 +303,9 @@ describe('mcpToolWrapper', () => { const clickTool = tools.find((t) => t.name === 'click')!; const invocation = clickTool.build({ uid: 'bad-elem' }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); // Should return error, not throw expect(result.error).toBeDefined(); @@ -328,7 +336,9 @@ describe('mcpToolWrapper', () => { const uploadTool = tools.find((t) => t.name === 'upload_file')!; const invocation = uploadTool.build({ path: 'test.txt' }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.error).toBeDefined(); expect(result.llmContent).toContain('File uploads are blocked'); @@ -345,7 +355,9 @@ describe('mcpToolWrapper', () => { const uploadTool = tools.find((t) => t.name === 'upload_file')!; const invocation = uploadTool.build({ path: 'test.txt' }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.error).toBeUndefined(); expect(result.llmContent).toBe('Tool result'); diff --git a/packages/core/src/agents/browser/mcpToolWrapper.ts b/packages/core/src/agents/browser/mcpToolWrapper.ts index a78e62d500..0a085b6c44 100644 --- a/packages/core/src/agents/browser/mcpToolWrapper.ts +++ b/packages/core/src/agents/browser/mcpToolWrapper.ts @@ -26,6 +26,7 @@ import { type ToolInvocation, type ToolCallConfirmationDetails, type PolicyUpdateOptions, + type ExecuteOptions, } from '../../tools/tools.js'; import type { MessageBus } from '../../confirmation-bus/message-bus.js'; import { @@ -117,7 +118,7 @@ class McpToolInvocation extends BaseToolInvocation< return this.shouldDisableInput && INTERACTIVE_TOOLS.has(this.toolName); } - async execute(signal: AbortSignal): Promise { + async execute({ abortSignal: signal }: ExecuteOptions): Promise { try { // Hard block for file uploads if configured if (this.blockFileUploads && this.toolName === 'upload_file') { diff --git a/packages/core/src/agents/local-invocation.test.ts b/packages/core/src/agents/local-invocation.test.ts index 854a32ec64..eaea2b9ffa 100644 --- a/packages/core/src/agents/local-invocation.test.ts +++ b/packages/core/src/agents/local-invocation.test.ts @@ -187,7 +187,10 @@ describe('LocalSubagentInvocation', () => { }; mockExecutorInstance.run.mockResolvedValue(mockOutput); - const result = await invocation.execute(signal, updateOutput); + const result = await invocation.execute({ + abortSignal: signal, + updateOutput, + }); expect(MockLocalAgentExecutor.create).toHaveBeenCalledWith( testDefinition, @@ -224,7 +227,10 @@ describe('LocalSubagentInvocation', () => { }; mockExecutorInstance.run.mockResolvedValue(mockOutput); - const result = await invocation.execute(signal, updateOutput); + const result = await invocation.execute({ + abortSignal: signal, + updateOutput, + }); const display = result.returnDisplay as SubagentProgress; expect(display.isSubagentProgress).toBe(true); @@ -254,7 +260,7 @@ describe('LocalSubagentInvocation', () => { return { result: 'Done', terminate_reason: AgentTerminateMode.GOAL }; }); - await invocation.execute(signal, updateOutput); + await invocation.execute({ abortSignal: signal, updateOutput }); expect(updateOutput).toHaveBeenCalledTimes(4); // Initial + 2 updates + Final completion const lastCall = updateOutput.mock.calls[3][0] as SubagentProgress; @@ -293,7 +299,7 @@ describe('LocalSubagentInvocation', () => { return { result: 'Done', terminate_reason: AgentTerminateMode.GOAL }; }); - await invocation.execute(signal, updateOutput); + await invocation.execute({ abortSignal: signal, updateOutput }); const calls = updateOutput.mock.calls; const lastCall = calls[calls.length - 1][0] as SubagentProgress; @@ -326,7 +332,7 @@ describe('LocalSubagentInvocation', () => { return { result: 'Done', terminate_reason: AgentTerminateMode.GOAL }; }); - await invocation.execute(signal, updateOutput); + await invocation.execute({ abortSignal: signal, updateOutput }); expect(updateOutput).toHaveBeenCalledTimes(4); // Initial + 2 updates + Final completion const lastCall = updateOutput.mock.calls[3][0] as SubagentProgress; @@ -360,7 +366,7 @@ describe('LocalSubagentInvocation', () => { return { result: 'Done', terminate_reason: AgentTerminateMode.GOAL }; }); - await invocation.execute(signal, updateOutput); + await invocation.execute({ abortSignal: signal, updateOutput }); expect(updateOutput).toHaveBeenCalled(); const lastCall = updateOutput.mock.calls[ @@ -404,7 +410,7 @@ describe('LocalSubagentInvocation', () => { }; }); - await invocation.execute(signal, updateOutput); + await invocation.execute({ abortSignal: signal, updateOutput }); expect(updateOutput).toHaveBeenCalledTimes(4); const lastCall = updateOutput.mock.calls[3][0] as SubagentProgress; @@ -433,7 +439,7 @@ describe('LocalSubagentInvocation', () => { }); // Execute without the optional callback - const result = await invocation.execute(signal); + const result = await invocation.execute({ abortSignal: signal }); expect(result.error).toBeUndefined(); const display = result.returnDisplay as SubagentProgress; expect(display.isSubagentProgress).toBe(true); @@ -445,7 +451,10 @@ describe('LocalSubagentInvocation', () => { const error = new Error('Model failed during execution.'); mockExecutorInstance.run.mockRejectedValue(error); - const result = await invocation.execute(signal, updateOutput); + const result = await invocation.execute({ + abortSignal: signal, + updateOutput, + }); expect(result.error).toBeUndefined(); expect(result.llmContent).toBe( @@ -466,7 +475,10 @@ describe('LocalSubagentInvocation', () => { const creationError = new Error('Failed to initialize tools.'); MockLocalAgentExecutor.create.mockRejectedValue(creationError); - const result = await invocation.execute(signal, updateOutput); + const result = await invocation.execute({ + abortSignal: signal, + updateOutput, + }); expect(mockExecutorInstance.run).not.toHaveBeenCalled(); expect(result.error).toBeUndefined(); @@ -487,10 +499,10 @@ describe('LocalSubagentInvocation', () => { mockExecutorInstance.run.mockRejectedValue(abortError); const controller = new AbortController(); - const executePromise = invocation.execute( - controller.signal, + const executePromise = invocation.execute({ + abortSignal: controller.signal, updateOutput, - ); + }); controller.abort(); await expect(executePromise).rejects.toThrow('Aborted'); @@ -507,9 +519,9 @@ describe('LocalSubagentInvocation', () => { }; mockExecutorInstance.run.mockResolvedValue(mockOutput); - await expect(invocation.execute(signal, updateOutput)).rejects.toThrow( - 'Operation cancelled by user', - ); + await expect( + invocation.execute({ abortSignal: signal, updateOutput }), + ).rejects.toThrow('Operation cancelled by user'); }); it('should publish SUBAGENT_ACTIVITY events to the MessageBus', async () => { @@ -529,7 +541,7 @@ describe('LocalSubagentInvocation', () => { return { result: 'Done', terminate_reason: AgentTerminateMode.GOAL }; }); - await invocation.execute(signal, updateOutput); + await invocation.execute({ abortSignal: signal, updateOutput }); expect(mockMessageBus.publish).toHaveBeenCalledWith( expect.objectContaining({ diff --git a/packages/core/src/agents/local-invocation.ts b/packages/core/src/agents/local-invocation.ts index 228d5010ec..186f015979 100644 --- a/packages/core/src/agents/local-invocation.ts +++ b/packages/core/src/agents/local-invocation.ts @@ -10,7 +10,7 @@ import { LocalAgentExecutor } from './local-executor.js'; import { BaseToolInvocation, type ToolResult, - type ToolLiveOutput, + type ExecuteOptions, } from '../tools/tools.js'; import { type LocalAgentDefinition, @@ -105,10 +105,8 @@ export class LocalSubagentInvocation extends BaseToolInvocation< * agent's thoughts, to the user interface. * @returns A `Promise` that resolves with the final `ToolResult`. */ - async execute( - signal: AbortSignal, - updateOutput?: (output: ToolLiveOutput) => void, - ): Promise { + async execute(options: ExecuteOptions): Promise { + const { abortSignal: signal, updateOutput } = options; const recentActivity: SubagentActivityItem[] = []; let executor: LocalAgentExecutor | undefined; diff --git a/packages/core/src/agents/remote-invocation.test.ts b/packages/core/src/agents/remote-invocation.test.ts index 3ff7ebe794..0ec7774192 100644 --- a/packages/core/src/agents/remote-invocation.test.ts +++ b/packages/core/src/agents/remote-invocation.test.ts @@ -142,7 +142,7 @@ describe('RemoteAgentInvocation', () => { {}, mockMessageBus, ); - await invocation.execute(new AbortController().signal); + await invocation.execute({ abortSignal: new AbortController().signal }); expect(mockClientManager.sendMessageStream).toHaveBeenCalledWith( 'test-agent', @@ -185,7 +185,7 @@ describe('RemoteAgentInvocation', () => { }, mockMessageBus, ); - await invocation.execute(new AbortController().signal); + await invocation.execute({ abortSignal: new AbortController().signal }); expect(mockClientManager.loadAgent).toHaveBeenCalledWith( 'test-agent', @@ -230,7 +230,7 @@ describe('RemoteAgentInvocation', () => { { query: 'hi' }, mockMessageBus, ); - await invocation.execute(new AbortController().signal); + await invocation.execute({ abortSignal: new AbortController().signal }); expect(A2AAuthProviderFactory.create).toHaveBeenCalledWith({ authConfig: mockAuth, @@ -264,7 +264,9 @@ describe('RemoteAgentInvocation', () => { { query: 'hi' }, mockMessageBus, ); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.returnDisplay).toMatchObject({ state: 'error' }); expect((result.returnDisplay as SubagentProgress).result).toContain( @@ -293,7 +295,7 @@ describe('RemoteAgentInvocation', () => { }, mockMessageBus, ); - await invocation.execute(new AbortController().signal); + await invocation.execute({ abortSignal: new AbortController().signal }); expect(mockClientManager.loadAgent).not.toHaveBeenCalled(); }); @@ -325,7 +327,9 @@ describe('RemoteAgentInvocation', () => { ); // Execute first time - const result1 = await invocation1.execute(new AbortController().signal); + const result1 = await invocation1.execute({ + abortSignal: new AbortController().signal, + }); expect(result1.returnDisplay).toMatchObject({ result: 'Response 1', }); @@ -357,7 +361,9 @@ describe('RemoteAgentInvocation', () => { }, mockMessageBus, ); - const result2 = await invocation2.execute(new AbortController().signal); + const result2 = await invocation2.execute({ + abortSignal: new AbortController().signal, + }); expect((result2.returnDisplay as SubagentProgress).result).toBe( 'Response 2', ); @@ -390,7 +396,7 @@ describe('RemoteAgentInvocation', () => { }, mockMessageBus, ); - await invocation3.execute(new AbortController().signal); + await invocation3.execute({ abortSignal: new AbortController().signal }); // Fourth call: Should start new task (taskId undefined) mockClientManager.sendMessageStream.mockImplementationOnce( @@ -412,7 +418,7 @@ describe('RemoteAgentInvocation', () => { }, mockMessageBus, ); - await invocation4.execute(new AbortController().signal); + await invocation4.execute({ abortSignal: new AbortController().signal }); expect(mockClientManager.sendMessageStream).toHaveBeenLastCalledWith( 'test-agent', @@ -447,7 +453,10 @@ describe('RemoteAgentInvocation', () => { { query: 'hi' }, mockMessageBus, ); - await invocation.execute(new AbortController().signal, updateOutput); + await invocation.execute({ + abortSignal: new AbortController().signal, + updateOutput, + }); expect(updateOutput).toHaveBeenCalledWith( expect.objectContaining({ @@ -495,7 +504,9 @@ describe('RemoteAgentInvocation', () => { { query: 'hi' }, mockMessageBus, ); - const result = await invocation.execute(controller.signal); + const result = await invocation.execute({ + abortSignal: controller.signal, + }); expect(result.returnDisplay).toMatchObject({ state: 'error' }); }); @@ -517,7 +528,9 @@ describe('RemoteAgentInvocation', () => { }, mockMessageBus, ); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.returnDisplay).toMatchObject({ state: 'error', @@ -550,7 +563,9 @@ describe('RemoteAgentInvocation', () => { }, mockMessageBus, ); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); // Just check that text is present, exact formatting depends on helper expect((result.returnDisplay as SubagentProgress).result).toContain( @@ -593,10 +608,10 @@ describe('RemoteAgentInvocation', () => { { query: 'hi' }, mockMessageBus, ); - const result = await invocation.execute( - new AbortController().signal, + const result = await invocation.execute({ + abortSignal: new AbortController().signal, updateOutput, - ); + }); expect(updateOutput).toHaveBeenCalledWith( expect.objectContaining({ @@ -670,7 +685,10 @@ describe('RemoteAgentInvocation', () => { { query: 'hi' }, mockMessageBus, ); - await invocation.execute(new AbortController().signal, updateOutput); + await invocation.execute({ + abortSignal: new AbortController().signal, + updateOutput, + }); expect(updateOutput).toHaveBeenCalledWith( expect.objectContaining({ @@ -738,7 +756,9 @@ describe('RemoteAgentInvocation', () => { { query: 'hi' }, mockMessageBus, ); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.returnDisplay).toMatchObject({ state: 'error' }); expect((result.returnDisplay as SubagentProgress).result).toContain( @@ -758,7 +778,9 @@ describe('RemoteAgentInvocation', () => { { query: 'hi' }, mockMessageBus, ); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.returnDisplay).toMatchObject({ state: 'error' }); expect((result.returnDisplay as SubagentProgress).result).toContain( @@ -787,7 +809,9 @@ describe('RemoteAgentInvocation', () => { { query: 'hi' }, mockMessageBus, ); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.returnDisplay).toMatchObject({ state: 'error' }); // Should contain both the partial output and the error message diff --git a/packages/core/src/agents/remote-invocation.ts b/packages/core/src/agents/remote-invocation.ts index 7dda4b0ee0..e0869603fe 100644 --- a/packages/core/src/agents/remote-invocation.ts +++ b/packages/core/src/agents/remote-invocation.ts @@ -9,6 +9,7 @@ import { type ToolConfirmationOutcome, type ToolResult, type ToolCallConfirmationDetails, + type ExecuteOptions, } from '../tools/tools.js'; import { DEFAULT_QUERY_STRING, @@ -28,7 +29,6 @@ import type { import { extractIdsFromResponse, A2AResultReassembler } from './a2aUtils.js'; import type { AuthenticationHandler } from '@a2a-js/sdk/client'; import { debugLogger } from '../utils/debugLogger.js'; -import type { AnsiOutput } from '../utils/terminalSerializer.js'; import { A2AAuthProviderFactory } from './auth-provider/factory.js'; import { A2AAgentError } from './a2a-errors.js'; @@ -126,10 +126,8 @@ export class RemoteAgentInvocation extends BaseToolInvocation< }; } - async execute( - _signal: AbortSignal, - updateOutput?: (output: string | AnsiOutput | SubagentProgress) => void, - ): Promise { + async execute(options: ExecuteOptions): Promise { + const { abortSignal: _signal, updateOutput } = options; // 1. Ensure the agent is loaded (cached by manager) // We assume the user has provided an access token via some mechanism (TODO), // or we rely on ADC. diff --git a/packages/core/src/core/coreToolHookTriggers.test.ts b/packages/core/src/core/coreToolHookTriggers.test.ts index 60c6836452..96b659812d 100644 --- a/packages/core/src/core/coreToolHookTriggers.test.ts +++ b/packages/core/src/core/coreToolHookTriggers.test.ts @@ -11,7 +11,7 @@ import { BaseToolInvocation, type ToolResult, type AnyDeclarativeTool, - type ToolLiveOutput, + type ExecuteOptions, } from '../tools/tools.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; import type { HookSystem } from '../hooks/hookSystem.js'; @@ -46,11 +46,7 @@ class MockBackgroundableInvocation extends BaseToolInvocation< getDescription() { return 'mock-pid'; } - async execute( - _signal: AbortSignal, - _updateOutput?: (output: ToolLiveOutput) => void, - options?: { setExecutionIdCallback?: (executionId: number) => void }, - ) { + async execute(options: ExecuteOptions) { options?.setExecutionIdCallback?.(4242); return { llmContent: 'pid', diff --git a/packages/core/src/core/coreToolHookTriggers.ts b/packages/core/src/core/coreToolHookTriggers.ts index c2748cbd0a..e7019fc86f 100644 --- a/packages/core/src/core/coreToolHookTriggers.ts +++ b/packages/core/src/core/coreToolHookTriggers.ts @@ -71,7 +71,7 @@ export async function executeToolWithHooks( signal: AbortSignal, tool: AnyDeclarativeTool, liveOutputCallback?: (outputChunk: ToolLiveOutput) => void, - options?: ExecuteOptions, + options?: Omit, config?: Config, originalRequestName?: string, skipBeforeHook?: boolean, @@ -154,11 +154,11 @@ export async function executeToolWithHooks( // Execute the actual tool. Tools that support backgrounding can optionally // surface an execution ID via the callback. - const toolResult: ToolResult = await invocation.execute( - signal, - liveOutputCallback, - options, - ); + const toolResult: ToolResult = await invocation.execute({ + ...options, + abortSignal: signal, + updateOutput: liveOutputCallback, + }); // Append notification if parameters were modified if (inputWasModified) { diff --git a/packages/core/src/test-utils/mock-tool.ts b/packages/core/src/test-utils/mock-tool.ts index f1c38dbeae..ea6097ac6e 100644 --- a/packages/core/src/test-utils/mock-tool.ts +++ b/packages/core/src/test-utils/mock-tool.ts @@ -14,7 +14,6 @@ import { Kind, type ToolCallConfirmationDetails, type ToolInvocation, - type ToolLiveOutput, type ToolResult, type ExecuteOptions, } from '../tools/tools.js'; @@ -54,11 +53,8 @@ class MockToolInvocation extends BaseToolInvocation< super(params, messageBus, tool.name, tool.displayName); } - execute( - signal: AbortSignal, - updateOutput?: (output: ToolLiveOutput) => void, - options?: ExecuteOptions, - ): Promise { + execute(options: ExecuteOptions): Promise { + const { abortSignal: signal, updateOutput } = options; return this.tool.execute( this.params, signal, @@ -158,11 +154,10 @@ export class MockModifiableToolInvocation extends BaseToolInvocation< super(params, messageBus, tool.name, tool.displayName); } - async execute( - _signal: AbortSignal, - _updateOutput?: (output: ToolLiveOutput) => void, - _options?: ExecuteOptions, - ): Promise { + async execute({ + abortSignal: _signal, + updateOutput: _updateOutput, + }: ExecuteOptions): Promise { const result = this.tool.executeFn(this.params); return ( result ?? { diff --git a/packages/core/src/tools/activate-skill.test.ts b/packages/core/src/tools/activate-skill.test.ts index 553a34dd43..b2a37479bf 100644 --- a/packages/core/src/tools/activate-skill.test.ts +++ b/packages/core/src/tools/activate-skill.test.ts @@ -107,7 +107,9 @@ describe('ActivateSkillTool', () => { it('should activate a valid skill and return its content in XML tags', async () => { const params = { name: 'test-skill' }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(mockConfig.getSkillManager().activateSkill).toHaveBeenCalledWith( 'test-skill', @@ -136,7 +138,9 @@ describe('ActivateSkillTool', () => { vi.mocked(mockConfig.getSkillManager().getSkill).mockReturnValue(null); const params = { name: 'test-skill' }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toContain('Error: Skill "test-skill" not found.'); expect(mockConfig.getSkillManager().activateSkill).not.toHaveBeenCalled(); diff --git a/packages/core/src/tools/activate-skill.ts b/packages/core/src/tools/activate-skill.ts index 21ee2e98c6..17e0b84f2f 100644 --- a/packages/core/src/tools/activate-skill.ts +++ b/packages/core/src/tools/activate-skill.ts @@ -15,6 +15,7 @@ import { type ToolCallConfirmationDetails, type ToolInvocation, type ToolConfirmationOutcome, + type ExecuteOptions, } from './tools.js'; import type { Config } from '../config/config.js'; import { ACTIVATE_SKILL_TOOL_NAME } from './tool-names.js'; @@ -107,7 +108,7 @@ ${folderStructure}`, return confirmationDetails; } - async execute(_signal: AbortSignal): Promise { + async execute({ abortSignal: _signal }: ExecuteOptions): Promise { const skillName = this.params.name; const skillManager = this.config.getSkillManager(); const skill = skillManager.getSkill(skillName); diff --git a/packages/core/src/tools/ask-user.test.ts b/packages/core/src/tools/ask-user.test.ts index 57a0556466..1b995e871c 100644 --- a/packages/core/src/tools/ask-user.test.ts +++ b/packages/core/src/tools/ask-user.test.ts @@ -410,7 +410,9 @@ describe('AskUserTool', () => { }); } - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.returnDisplay).toContain('User answered:'); expect(result.returnDisplay).toContain( ' Approach → Quick fix (Recommended)', @@ -453,7 +455,9 @@ describe('AskUserTool', () => { }); } - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.returnDisplay).toBe( 'User submitted without answering questions.', ); @@ -499,7 +503,9 @@ describe('AskUserTool', () => { await details.onConfirm(ToolConfirmationOutcome.Cancel); } - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.returnDisplay).toBe('User dismissed dialog'); expect(result.llmContent).toBe( 'User dismissed ask_user dialog without answering.', diff --git a/packages/core/src/tools/ask-user.ts b/packages/core/src/tools/ask-user.ts index 621d4c10d1..5574534a37 100644 --- a/packages/core/src/tools/ask-user.ts +++ b/packages/core/src/tools/ask-user.ts @@ -12,6 +12,7 @@ import { type ToolAskUserConfirmationDetails, type ToolConfirmationPayload, ToolConfirmationOutcome, + type ExecuteOptions, } from './tools.js'; import { ToolErrorType } from './tool-error.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; @@ -152,7 +153,7 @@ export class AskUserInvocation extends BaseToolInvocation< return `Asking user: ${this.params.questions.map((q) => q.question).join(', ')}`; } - async execute(_signal: AbortSignal): Promise { + async execute({ abortSignal: _signal }: ExecuteOptions): Promise { const questionTypes = this.params.questions.map((q) => q.type); if (this.confirmationOutcome === ToolConfirmationOutcome.Cancel) { diff --git a/packages/core/src/tools/complete-task.test.ts b/packages/core/src/tools/complete-task.test.ts index 6577c8786c..b10884ad73 100644 --- a/packages/core/src/tools/complete-task.test.ts +++ b/packages/core/src/tools/complete-task.test.ts @@ -63,7 +63,9 @@ describe('CompleteTaskTool', () => { it('should execute and return correct data', async () => { const invocation = tool.build({ result: 'Success message' }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.data).toEqual({ taskCompleted: true, @@ -133,7 +135,9 @@ describe('CompleteTaskTool', () => { it('should execute and return structured data', async () => { const outputValue = { report: 'Final findings', score: 42 }; const invocation = tool.build({ my_output: outputValue }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.data?.['taskCompleted']).toBe(true); expect(result.data?.['submittedOutput']).toBe( @@ -152,7 +156,9 @@ describe('CompleteTaskTool', () => { const outputValue = { report: 'Final findings', score: 42 }; const invocation = toolWithProcess.build({ my_output: outputValue }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.data?.['submittedOutput']).toBe('Score was 42'); }); diff --git a/packages/core/src/tools/complete-task.ts b/packages/core/src/tools/complete-task.ts index ec35b193ba..42798cd0e3 100644 --- a/packages/core/src/tools/complete-task.ts +++ b/packages/core/src/tools/complete-task.ts @@ -9,7 +9,9 @@ import { BaseToolInvocation, type ToolResult, Kind, + type ExecuteOptions, } from './tools.js'; + import { COMPLETE_TASK_TOOL_NAME, COMPLETE_TASK_DISPLAY_NAME, @@ -140,7 +142,7 @@ export class CompleteTaskInvocation< return 'Completing task and submitting results.'; } - async execute(_signal: AbortSignal): Promise { + async execute({ abortSignal: _signal }: ExecuteOptions): Promise { let submittedOutput: string | null = null; let outputValue: unknown; diff --git a/packages/core/src/tools/edit.test.ts b/packages/core/src/tools/edit.test.ts index 66111aed9d..075dca64b1 100644 --- a/packages/core/src/tools/edit.test.ts +++ b/packages/core/src/tools/edit.test.ts @@ -695,9 +695,9 @@ function doIt() { throw abortError; }); - await expect(invocation.execute(abortController.signal)).rejects.toBe( - abortError, - ); + await expect( + invocation.execute({ abortSignal: abortController.signal }), + ).rejects.toBe(abortError); calculateSpy.mockRestore(); }); @@ -714,7 +714,9 @@ function doIt() { }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toMatch(/Successfully modified file/); expect(fs.readFileSync(filePath, 'utf8')).toBe(newContent); @@ -737,7 +739,9 @@ function doIt() { new_string: 'replacement', }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toMatch(/0 occurrences found for old_string/); expect(result.returnDisplay).toMatch( /Failed to edit, could not find the string to replace./, @@ -768,7 +772,9 @@ function doIt() { }); const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.error).toBeUndefined(); expect(result.llmContent).toMatch(/Successfully modified file/); @@ -788,7 +794,7 @@ function doIt() { }; const invocation = tool.build(params); - await invocation.execute(new AbortController().signal); + await invocation.execute({ abortSignal: new AbortController().signal }); const finalContent = fs.readFileSync(filePath, 'utf8'); expect(finalContent).toBe(newContent); @@ -804,7 +810,7 @@ function doIt() { }; const invocation = tool.build(params); - await invocation.execute(new AbortController().signal); + await invocation.execute({ abortSignal: new AbortController().signal }); const finalContent = fs.readFileSync(filePath, 'utf8'); expect(finalContent).toBe(newContentWithCRLF); @@ -832,7 +838,9 @@ function doIt() { }); const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.error?.type).toBe( ToolErrorType.EDIT_NO_CHANGE_LLM_JUDGEMENT, @@ -876,7 +884,7 @@ function doIt() { .mockResolvedValueOnce(externallyModifiedContent); // Second call in `attemptSelfCorrection` const invocation = tool.build(params); - await invocation.execute(new AbortController().signal); + await invocation.execute({ abortSignal: new AbortController().signal }); // Assert that the file was read twice (initial read, then re-read for hash comparison). expect(readTextFileSpy).toHaveBeenCalledTimes(2); @@ -938,7 +946,9 @@ function doIt() { instruction: 'test', ...params, }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.error?.type).toBe(expectedError); }, ); @@ -1020,7 +1030,9 @@ function doIt() { ...(allow_multiple !== undefined && { allow_multiple }), }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); if (shouldSucceed) { expect(result.error).toBeUndefined(); @@ -1162,7 +1174,9 @@ function doIt() { ai_proposed_content: '', }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); if ( result.returnDisplay && @@ -1215,7 +1229,9 @@ function doIt() { }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.error?.type).toBe(ToolErrorType.EDIT_NO_OCCURRENCE_FOUND); expect(mockFixLLMEditWithInstruction).not.toHaveBeenCalled(); @@ -1236,7 +1252,7 @@ function doIt() { }; const invocation = tool.build(params); - await invocation.execute(new AbortController().signal); + await invocation.execute({ abortSignal: new AbortController().signal }); expect(mockFixLLMEditWithInstruction).toHaveBeenCalled(); }); @@ -1265,7 +1281,9 @@ function doIt() { }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(discoverJitContext).toHaveBeenCalled(); expect(result.llmContent).toContain('Newly Discovered Project Context'); @@ -1294,7 +1312,9 @@ function doIt() { }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).not.toContain( 'Newly Discovered Project Context', @@ -1329,7 +1349,9 @@ function doIt() { }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toMatch(/Successfully modified file/); diff --git a/packages/core/src/tools/edit.ts b/packages/core/src/tools/edit.ts index 55c7f2f9ab..f0b9b448a3 100644 --- a/packages/core/src/tools/edit.ts +++ b/packages/core/src/tools/edit.ts @@ -21,6 +21,7 @@ import { type ToolResult, type ToolResultDisplay, type PolicyUpdateOptions, + type ExecuteOptions, } from './tools.js'; import { buildFilePathArgsPattern } from '../policy/utils.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; @@ -832,7 +833,7 @@ class EditToolInvocation * @param params Parameters for the edit operation * @returns Result of the edit operation */ - async execute(signal: AbortSignal): Promise { + async execute({ abortSignal: signal }: ExecuteOptions): Promise { const validationError = this.config.validatePathAccess(this.resolvedPath); if (validationError) { return { diff --git a/packages/core/src/tools/enter-plan-mode.test.ts b/packages/core/src/tools/enter-plan-mode.test.ts index ed88a4b49b..0b477d7be6 100644 --- a/packages/core/src/tools/enter-plan-mode.test.ts +++ b/packages/core/src/tools/enter-plan-mode.test.ts @@ -121,7 +121,9 @@ describe('EnterPlanModeTool', () => { const invocation = tool.build({}); vi.mocked(fs.existsSync).mockReturnValue(true); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(mockConfig.setApprovalMode).toHaveBeenCalledWith( ApprovalMode.PLAN, @@ -134,7 +136,7 @@ describe('EnterPlanModeTool', () => { const invocation = tool.build({}); vi.mocked(fs.existsSync).mockReturnValue(false); - await invocation.execute(new AbortController().signal); + await invocation.execute({ abortSignal: new AbortController().signal }); expect(fs.mkdirSync).toHaveBeenCalledWith('/mock/plans/dir', { recursive: true, @@ -146,7 +148,9 @@ describe('EnterPlanModeTool', () => { const invocation = tool.build({ reason }); vi.mocked(fs.existsSync).mockReturnValue(true); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(mockConfig.setApprovalMode).toHaveBeenCalledWith( ApprovalMode.PLAN, @@ -177,7 +181,9 @@ describe('EnterPlanModeTool', () => { await details.onConfirm(ToolConfirmationOutcome.Cancel); } - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(mockConfig.setApprovalMode).not.toHaveBeenCalled(); expect(result.returnDisplay).toBe('Cancelled'); diff --git a/packages/core/src/tools/enter-plan-mode.ts b/packages/core/src/tools/enter-plan-mode.ts index 7e4ce658ba..81c3f095ce 100644 --- a/packages/core/src/tools/enter-plan-mode.ts +++ b/packages/core/src/tools/enter-plan-mode.ts @@ -12,6 +12,7 @@ import { Kind, type ToolInfoConfirmationDetails, ToolConfirmationOutcome, + type ExecuteOptions, } from './tools.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; import type { Config } from '../config/config.js'; @@ -114,7 +115,7 @@ export class EnterPlanModeInvocation extends BaseToolInvocation< }; } - async execute(_signal: AbortSignal): Promise { + async execute({ abortSignal: _signal }: ExecuteOptions): Promise { if (this.confirmationOutcome === ToolConfirmationOutcome.Cancel) { return { llmContent: 'User cancelled entering Plan Mode.', diff --git a/packages/core/src/tools/exit-plan-mode.test.ts b/packages/core/src/tools/exit-plan-mode.test.ts index ad643c6cb2..a8bd479052 100644 --- a/packages/core/src/tools/exit-plan-mode.test.ts +++ b/packages/core/src/tools/exit-plan-mode.test.ts @@ -135,9 +135,9 @@ describe('ExitPlanModeTool', () => { expect(result).toBe(false); // Verify it auto-approved internally - const executeResult = await invocation.execute( - new AbortController().signal, - ); + const executeResult = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(executeResult.llmContent).toContain('Plan approved'); }); @@ -164,7 +164,9 @@ describe('ExitPlanModeTool', () => { const invocation = tool.build({ plan_filename: planRelativePath }); await invocation.shouldConfirmExecute(new AbortController().signal); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toContain('Plan file is empty'); expect(result.llmContent).toContain('write content to the plan'); @@ -175,7 +177,9 @@ describe('ExitPlanModeTool', () => { const invocation = tool.build({ plan_filename: planRelativePath }); await invocation.shouldConfirmExecute(new AbortController().signal); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toContain('Plan file does not exist'); }); @@ -197,7 +201,9 @@ describe('ExitPlanModeTool', () => { approvalMode: ApprovalMode.DEFAULT, }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); const expectedPath = path.join(mockPlansDir, 'test.md'); expect(result).toEqual({ @@ -225,7 +231,9 @@ Read and follow the plan strictly during implementation.`, approvalMode: ApprovalMode.AUTO_EDIT, }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); const expectedPath = path.join(mockPlansDir, 'test.md'); expect(result).toEqual({ @@ -256,7 +264,9 @@ Read and follow the plan strictly during implementation.`, feedback: 'Please add more details.', }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); const expectedPath = path.join(mockPlansDir, 'test.md'); expect(result).toEqual({ @@ -282,7 +292,9 @@ Revise the plan based on the feedback.`, approved: false, }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); const expectedPath = path.join(mockPlansDir, 'test.md'); expect(result).toEqual({ @@ -308,7 +320,7 @@ Ask the user for specific feedback on how to improve the plan.`, approvalMode: ApprovalMode.AUTO_EDIT, }); - await invocation.execute(new AbortController().signal); + await invocation.execute({ abortSignal: new AbortController().signal }); expect(loggers.logPlanExecution).toHaveBeenCalledWith( mockConfig, @@ -330,7 +342,9 @@ Ask the user for specific feedback on how to improve the plan.`, await confirmDetails.onConfirm(ToolConfirmationOutcome.Cancel); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result).toEqual({ llmContent: @@ -348,7 +362,9 @@ Ask the user for specific feedback on how to improve the plan.`, // Simulate the scheduler's policy ALLOW path: execute() is called // directly without ever calling shouldConfirmExecute(), leaving // approvalPayload null. - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); const expectedPath = path.join(mockPlansDir, 'test.md'); expect(result.llmContent).toContain('Plan approved'); @@ -367,7 +383,9 @@ Ask the user for specific feedback on how to improve the plan.`, const invocation = tool.build({ plan_filename: planRelativePath }); // Directly call execute to trigger the internal getAllowApprovalMode - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toContain('YOLO mode'); expect(mockConfig.setApprovalMode).toHaveBeenCalledWith( @@ -381,7 +399,9 @@ Ask the user for specific feedback on how to improve the plan.`, const invocation = tool.build({ plan_filename: planRelativePath }); // Directly call execute to trigger the internal getAllowApprovalMode - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toContain('Default mode'); expect(mockConfig.setApprovalMode).toHaveBeenCalledWith( @@ -406,7 +426,9 @@ Ask the user for specific feedback on how to improve the plan.`, approvalMode: mode, }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toContain(expected); }; @@ -440,7 +462,7 @@ Ask the user for specific feedback on how to improve the plan.`, }); await expect( - invocation.execute(new AbortController().signal), + invocation.execute({ abortSignal: new AbortController().signal }), ).rejects.toThrow(/Unexpected approval mode/); }; diff --git a/packages/core/src/tools/exit-plan-mode.ts b/packages/core/src/tools/exit-plan-mode.ts index 483b1e5f3d..476aa88b7d 100644 --- a/packages/core/src/tools/exit-plan-mode.ts +++ b/packages/core/src/tools/exit-plan-mode.ts @@ -13,6 +13,7 @@ import { type ToolExitPlanModeConfirmationDetails, type ToolExitPlanModeConfirmationPayload, type ToolResult, + type ExecuteOptions, } from './tools.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; import path from 'node:path'; @@ -182,7 +183,7 @@ export class ExitPlanModeInvocation extends BaseToolInvocation< return path.join(this.config.storage.getPlansDir(), safeFilename); } - async execute(_signal: AbortSignal): Promise { + async execute({ abortSignal: _signal }: ExecuteOptions): Promise { const resolvedPlanPath = this.getResolvedPlanPath(); if (this.planValidationError) { diff --git a/packages/core/src/tools/get-internal-docs.test.ts b/packages/core/src/tools/get-internal-docs.test.ts index bee9265e70..190801110c 100644 --- a/packages/core/src/tools/get-internal-docs.test.ts +++ b/packages/core/src/tools/get-internal-docs.test.ts @@ -21,7 +21,7 @@ describe('GetInternalDocsTool (Integration)', () => { it('should find the documentation root and list files', async () => { const invocation = tool.build({}); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.error).toBeUndefined(); // Verify we found some files @@ -45,7 +45,7 @@ describe('GetInternalDocsTool (Integration)', () => { const expectedContent = await fs.readFile(expectedDocsPath, 'utf8'); const invocation = tool.build({ path: 'index.md' }); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.error).toBeUndefined(); expect(result.llmContent).toBe(expectedContent); @@ -55,7 +55,7 @@ describe('GetInternalDocsTool (Integration)', () => { it('should prevent access to files outside the docs directory (Path Traversal)', async () => { // Attempt to read package.json from the root const invocation = tool.build({ path: '../package.json' }); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.error).toBeDefined(); expect(result.error?.type).toBe(ToolErrorType.EXECUTION_FAILED); @@ -64,7 +64,7 @@ describe('GetInternalDocsTool (Integration)', () => { it('should handle non-existent files', async () => { const invocation = tool.build({ path: 'this-file-does-not-exist.md' }); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.error).toBeDefined(); expect(result.error?.type).toBe(ToolErrorType.EXECUTION_FAILED); diff --git a/packages/core/src/tools/get-internal-docs.ts b/packages/core/src/tools/get-internal-docs.ts index 23bda8f4dd..5d2f8821ae 100644 --- a/packages/core/src/tools/get-internal-docs.ts +++ b/packages/core/src/tools/get-internal-docs.ts @@ -11,6 +11,7 @@ import { type ToolInvocation, type ToolResult, type ToolCallConfirmationDetails, + type ExecuteOptions, } from './tools.js'; import { GET_INTERNAL_DOCS_TOOL_NAME } from './tool-names.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; @@ -96,7 +97,7 @@ class GetInternalDocsInvocation extends BaseToolInvocation< return 'Listing all available internal documentation.'; } - async execute(_signal: AbortSignal): Promise { + async execute({ abortSignal: _signal }: ExecuteOptions): Promise { try { const docsRoot = await getDocsRoot(); diff --git a/packages/core/src/tools/glob.test.ts b/packages/core/src/tools/glob.test.ts index f3390f5d3c..22b6c21e48 100644 --- a/packages/core/src/tools/glob.test.ts +++ b/packages/core/src/tools/glob.test.ts @@ -111,7 +111,7 @@ describe('GlobTool', () => { it('should find files matching a simple pattern in the root', async () => { const params: GlobToolParams = { pattern: '*.txt' }; const invocation = globTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Found 2 file(s)'); expect(result.llmContent).toContain(path.join(tempRootDir, 'fileA.txt')); expect(result.llmContent).toContain(path.join(tempRootDir, 'FileB.TXT')); @@ -121,7 +121,7 @@ describe('GlobTool', () => { it('should find files case-sensitively when case_sensitive is true', async () => { const params: GlobToolParams = { pattern: '*.txt', case_sensitive: true }; const invocation = globTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Found 1 file(s)'); expect(result.llmContent).toContain(path.join(tempRootDir, 'fileA.txt')); expect(result.llmContent).not.toContain( @@ -133,7 +133,7 @@ describe('GlobTool', () => { const params: GlobToolParams = { pattern: '*.TXT' }; const invocation = globTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('fileA.txt'); expect(result.llmContent).toContain('FileB.TXT'); @@ -145,7 +145,7 @@ describe('GlobTool', () => { case_sensitive: false, }; const invocation = globTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Found 2 file(s)'); expect(result.llmContent).toContain(path.join(tempRootDir, 'fileA.txt')); expect(result.llmContent).toContain(path.join(tempRootDir, 'FileB.TXT')); @@ -154,7 +154,7 @@ describe('GlobTool', () => { it('should find files using a pattern that includes a subdirectory', async () => { const params: GlobToolParams = { pattern: 'sub/*.md' }; const invocation = globTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Found 2 file(s)'); expect(result.llmContent).toContain( path.join(tempRootDir, 'sub', 'fileC.md'), @@ -167,7 +167,7 @@ describe('GlobTool', () => { it('should find files in a specified relative path (relative to rootDir)', async () => { const params: GlobToolParams = { pattern: '*.md', dir_path: 'sub' }; const invocation = globTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Found 2 file(s)'); expect(result.llmContent).toContain( path.join(tempRootDir, 'sub', 'fileC.md'), @@ -180,7 +180,7 @@ describe('GlobTool', () => { it('should find files using a deep globstar pattern (e.g., **/*.log)', async () => { const params: GlobToolParams = { pattern: '**/*.log' }; const invocation = globTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Found 1 file(s)'); expect(result.llmContent).toContain( path.join(tempRootDir, 'sub', 'deep', 'fileE.log'), @@ -190,7 +190,7 @@ describe('GlobTool', () => { it('should return "No files found" message when pattern matches nothing', async () => { const params: GlobToolParams = { pattern: '*.nonexistent' }; const invocation = globTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain( 'No files found matching pattern "*.nonexistent"', ); @@ -201,7 +201,7 @@ describe('GlobTool', () => { await fs.writeFile(path.join(tempRootDir, 'file[1].txt'), 'content'); const params: GlobToolParams = { pattern: 'file[1].txt' }; const invocation = globTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Found 1 file(s)'); expect(result.llmContent).toContain( path.join(tempRootDir, 'file[1].txt'), @@ -220,7 +220,7 @@ describe('GlobTool', () => { pattern: 'src/app/[test]/(dashboard)/testing/components/code.tsx', }; const invocation = globTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Found 1 file(s)'); expect(result.llmContent).toContain(filePath); }, 30000); @@ -228,7 +228,7 @@ describe('GlobTool', () => { it('should correctly sort files by modification time (newest first)', async () => { const params: GlobToolParams = { pattern: '*.sortme' }; const invocation = globTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); const llmContent = partListUnionToString(result.llmContent); const newerIndex = llmContent.indexOf('newer.sortme'); const olderIndex = llmContent.indexOf('older.sortme'); @@ -244,7 +244,7 @@ describe('GlobTool', () => { vi.mocked(glob.glob).mockRejectedValue(new Error('Glob failed')); const params: GlobToolParams = { pattern: '*' }; const invocation = globTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.error?.type).toBe(ToolErrorType.GLOB_EXECUTION_ERROR); }, 30000); }); @@ -383,7 +383,7 @@ describe('GlobTool', () => { const params: GlobToolParams = { pattern: '*_test.txt' }; const invocation = globTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Found 1 file(s)'); expect(result.llmContent).toContain('visible_test.txt'); @@ -403,7 +403,7 @@ describe('GlobTool', () => { const params: GlobToolParams = { pattern: 'visible_test.txt' }; const invocation = globTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Found 1 file(s)'); expect(result.llmContent).toContain('visible_test.txt'); @@ -422,7 +422,7 @@ describe('GlobTool', () => { respect_git_ignore: false, }; const invocation = globTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Found 1 file(s)'); expect(result.llmContent).toContain('ignored_test.txt'); @@ -443,7 +443,7 @@ describe('GlobTool', () => { respect_gemini_ignore: false, }; const invocation = globTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Found 1 file(s)'); expect(result.llmContent).toContain('gemini-ignored_test.txt'); diff --git a/packages/core/src/tools/glob.ts b/packages/core/src/tools/glob.ts index 9cef63759d..601f0cf7b8 100644 --- a/packages/core/src/tools/glob.ts +++ b/packages/core/src/tools/glob.ts @@ -16,6 +16,7 @@ import { type ToolResult, type PolicyUpdateOptions, type ToolConfirmationOutcome, + type ExecuteOptions, } from './tools.js'; import { shortenPath, makeRelative } from '../utils/paths.js'; import { type Config } from '../config/config.js'; @@ -129,7 +130,7 @@ class GlobToolInvocation extends BaseToolInvocation< }; } - async execute(signal: AbortSignal): Promise { + async execute({ abortSignal: signal }: ExecuteOptions): Promise { try { const workspaceContext = this.config.getWorkspaceContext(); const workspaceDirectories = workspaceContext.getDirectories(); diff --git a/packages/core/src/tools/grep.test.ts b/packages/core/src/tools/grep.test.ts index 8d12d3b89b..4af684b1cd 100644 --- a/packages/core/src/tools/grep.test.ts +++ b/packages/core/src/tools/grep.test.ts @@ -6,7 +6,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { GrepTool, type GrepToolParams } from './grep.js'; -import type { ToolResult, GrepResult } from './tools.js'; +import type { ToolResult, GrepResult, ExecuteOptions } from './tools.js'; import path from 'node:path'; import { isSubpath } from '../utils/paths.js'; import fs from 'node:fs/promises'; @@ -176,7 +176,7 @@ describe('GrepTool', () => { it('should find matches for a simple pattern in all files', async () => { const params: GrepToolParams = { pattern: 'world' }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain( 'Found 3 matches for pattern "world" in the workspace directory', ); @@ -196,7 +196,7 @@ describe('GrepTool', () => { await fs.writeFile(path.join(tempRootDir, '..env'), 'world in ..env'); const params: GrepToolParams = { pattern: 'world' }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('File: ..env'); expect(result.llmContent).toContain('L1: world in ..env'); }); @@ -209,13 +209,13 @@ describe('GrepTool', () => { const params: GrepToolParams = { pattern: 'hello' }; const invocation = grepTool.build(params) as unknown as { isCommandAvailable: (command: string) => Promise; - execute: (signal: AbortSignal) => Promise; + execute: (options: ExecuteOptions) => Promise; }; invocation.isCommandAvailable = vi.fn( async (command: string) => command === 'grep', ); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('File: ..env'); expect(result.llmContent).toContain('L1: hello'); expect(result.llmContent).not.toContain('secret.txt'); @@ -224,7 +224,7 @@ describe('GrepTool', () => { it('should find matches in a specific path', async () => { const params: GrepToolParams = { pattern: 'world', dir_path: 'sub' }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain( 'Found 1 match for pattern "world" in path "sub"', ); @@ -241,7 +241,7 @@ describe('GrepTool', () => { include_pattern: '*.js', }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain( 'Found 1 match for pattern "hello" in the workspace directory (filter: "*.js"):', ); @@ -265,7 +265,7 @@ describe('GrepTool', () => { include_pattern: '*.js', }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain( 'Found 1 match for pattern "hello" in path "sub" (filter: "*.js")', ); @@ -279,7 +279,7 @@ describe('GrepTool', () => { it('should return "No matches found" when pattern does not exist', async () => { const params: GrepToolParams = { pattern: 'nonexistentpattern' }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain( 'No matches found for pattern "nonexistentpattern" in the workspace directory.', ); @@ -291,7 +291,7 @@ describe('GrepTool', () => { it('should handle regex special characters correctly', async () => { const params: GrepToolParams = { pattern: 'foo.*bar' }; // Matches 'const foo = "bar";' const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain( 'Found 1 match for pattern "foo.*bar" in the workspace directory:', ); @@ -302,7 +302,7 @@ describe('GrepTool', () => { it('should be case-insensitive by default (JS fallback)', async () => { const params: GrepToolParams = { pattern: 'HELLO' }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain( 'Found 2 matches for pattern "HELLO" in the workspace directory:', ); @@ -325,7 +325,7 @@ describe('GrepTool', () => { vi.mocked(glob.globStream).mockRejectedValue(new Error('Glob failed')); const params: GrepToolParams = { pattern: 'hello' }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.error?.type).toBe(ToolErrorType.GREP_EXECUTION_ERROR); vi.mocked(glob.globStream).mockReset(); }, 30000); @@ -390,7 +390,7 @@ describe('GrepTool', () => { ); const params: GrepToolParams = { pattern: 'world' }; const invocation = multiDirGrepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); // Should find matches in both directories expect(result.llmContent).toContain( @@ -476,7 +476,7 @@ describe('GrepTool', () => { // Search only in the 'sub' directory of the first workspace const params: GrepToolParams = { pattern: 'world', dir_path: 'sub' }; const invocation = multiDirGrepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); // Should only find matches in the specified sub directory expect(result.llmContent).toContain( @@ -499,7 +499,7 @@ describe('GrepTool', () => { total_max_matches: 2, }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Found 2 matches'); expect(result.llmContent).toContain( @@ -522,7 +522,7 @@ describe('GrepTool', () => { max_matches_per_file: 1, }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); // fileA.txt has 2 worlds, but should only return 1. // sub/fileC.txt has 1 world, so total matches = 2. @@ -544,7 +544,7 @@ describe('GrepTool', () => { names_only: true, }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Found 2 files with matches'); expect(result.llmContent).toContain('fileA.txt'); @@ -565,7 +565,7 @@ describe('GrepTool', () => { dir_path: '.', }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Found 1 match'); expect(result.llmContent).toContain('copyright.txt'); @@ -585,7 +585,7 @@ describe('GrepTool', () => { const params: GrepToolParams = { pattern: 'Target match' }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain( 'Found 1 match for pattern "Target match"', @@ -607,7 +607,7 @@ describe('GrepTool', () => { const params: GrepToolParams = { pattern: 'Target match' }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); // MAX_LINE_LENGTH_TEXT_FILE is 2000. It should be truncated. expect(result.llmContent).toContain('... [truncated]'); diff --git a/packages/core/src/tools/grep.ts b/packages/core/src/tools/grep.ts index 3f6fd08ff3..34be588573 100644 --- a/packages/core/src/tools/grep.ts +++ b/packages/core/src/tools/grep.ts @@ -23,6 +23,7 @@ import { type ToolResult, type PolicyUpdateOptions, type ToolConfirmationOutcome, + type ExecuteOptions, } from './tools.js'; import { makeRelative, shortenPath } from '../utils/paths.js'; import { getErrorMessage, isNodeError } from '../utils/errors.js'; @@ -138,7 +139,7 @@ class GrepToolInvocation extends BaseToolInvocation< return null; } - async execute(signal: AbortSignal): Promise { + async execute({ abortSignal: signal }: ExecuteOptions): Promise { try { const workspaceContext = this.config.getWorkspaceContext(); const pathParam = this.params.dir_path; diff --git a/packages/core/src/tools/line-endings.test.ts b/packages/core/src/tools/line-endings.test.ts index 45c60e3b37..d4ba4ebd3f 100644 --- a/packages/core/src/tools/line-endings.test.ts +++ b/packages/core/src/tools/line-endings.test.ts @@ -192,7 +192,7 @@ describe('Line Ending Preservation', () => { await confirmDetails.onConfirm(ToolConfirmationOutcome.ProceedOnce); } - await invocation.execute(abortSignal); + await invocation.execute({ abortSignal }); const writtenContent = fs.readFileSync(filePath, 'utf8'); // Expect all newlines to be CRLF @@ -217,7 +217,7 @@ describe('Line Ending Preservation', () => { await confirmDetails.onConfirm(ToolConfirmationOutcome.ProceedOnce); } - await invocation.execute(abortSignal); + await invocation.execute({ abortSignal }); const writtenContent = fs.readFileSync(filePath, 'utf8'); @@ -265,7 +265,7 @@ describe('Line Ending Preservation', () => { await confirmDetails.onConfirm(ToolConfirmationOutcome.ProceedOnce); } - await invocation.execute(abortSignal); + await invocation.execute({ abortSignal }); const writtenContent = fs.readFileSync(filePath, 'utf8'); diff --git a/packages/core/src/tools/ls.test.ts b/packages/core/src/tools/ls.test.ts index e9a684719e..bc9a548bc2 100644 --- a/packages/core/src/tools/ls.test.ts +++ b/packages/core/src/tools/ls.test.ts @@ -127,7 +127,7 @@ describe('LSTool', () => { ); const invocation = lsTool.build({ dir_path: tempRootDir }); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('[DIR] subdir'); expect(result.llmContent).toContain('file1.txt'); @@ -146,7 +146,7 @@ describe('LSTool', () => { ); const invocation = lsTool.build({ dir_path: tempSecondaryDir }); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('secondary-file.txt'); expect(result.returnDisplay).toEqual({ @@ -159,7 +159,7 @@ describe('LSTool', () => { const emptyDir = path.join(tempRootDir, 'empty'); await fs.mkdir(emptyDir); const invocation = lsTool.build({ dir_path: emptyDir }); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toBe(`Directory ${emptyDir} is empty.`); expect(result.returnDisplay).toBe('Directory is empty.'); @@ -173,7 +173,7 @@ describe('LSTool', () => { dir_path: tempRootDir, ignore: ['*.log'], }); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('file1.txt'); expect(result.llmContent).not.toContain('file2.log'); @@ -189,7 +189,7 @@ describe('LSTool', () => { await fs.writeFile(path.join(tempRootDir, '.git'), ''); await fs.writeFile(path.join(tempRootDir, '.gitignore'), '*.log'); const invocation = lsTool.build({ dir_path: tempRootDir }); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('file1.txt'); expect(result.llmContent).not.toContain('file2.log'); @@ -207,7 +207,7 @@ describe('LSTool', () => { '*.log', ); const invocation = lsTool.build({ dir_path: tempRootDir }); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('file1.txt'); expect(result.llmContent).not.toContain('file2.log'); @@ -221,7 +221,7 @@ describe('LSTool', () => { await fs.writeFile(testPath, 'content1'); const invocation = lsTool.build({ dir_path: testPath }); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Path is not a directory'); expect(result.returnDisplay).toBe('Error: Path is not a directory.'); @@ -231,7 +231,7 @@ describe('LSTool', () => { it('should handle non-existent paths', async () => { const testPath = path.join(tempRootDir, 'does-not-exist'); const invocation = lsTool.build({ dir_path: testPath }); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Error listing directory'); expect(result.returnDisplay).toBe('Error: Failed to list directory.'); @@ -245,7 +245,7 @@ describe('LSTool', () => { await fs.mkdir(path.join(tempRootDir, 'y-dir')); const invocation = lsTool.build({ dir_path: tempRootDir }); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); const lines = ( typeof result.llmContent === 'string' ? result.llmContent : '' @@ -270,7 +270,7 @@ describe('LSTool', () => { vi.spyOn(fs, 'readdir').mockRejectedValueOnce(error); const invocation = lsTool.build({ dir_path: restrictedDir }); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Error listing directory'); expect(result.llmContent).toContain('permission denied'); @@ -295,7 +295,7 @@ describe('LSTool', () => { }); const invocation = lsTool.build({ dir_path: tempRootDir }); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); // Should still list the other files expect(result.llmContent).toContain('file1.txt'); @@ -360,7 +360,7 @@ describe('LSTool', () => { ); const invocation = lsTool.build({ dir_path: tempSecondaryDir }); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('secondary-file.txt'); expect(result.returnDisplay).toEqual({ @@ -378,7 +378,7 @@ describe('LSTool', () => { await fs.writeFile(path.join(tempRootDir, 'jit-file.txt'), 'content'); const invocation = lsTool.build({ dir_path: tempRootDir }); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(discoverJitContext).toHaveBeenCalled(); expect(result.llmContent).toContain('Newly Discovered Project Context'); @@ -395,7 +395,7 @@ describe('LSTool', () => { ); const invocation = lsTool.build({ dir_path: tempRootDir }); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).not.toContain( 'Newly Discovered Project Context', diff --git a/packages/core/src/tools/ls.ts b/packages/core/src/tools/ls.ts index 249a28372b..ea66028071 100644 --- a/packages/core/src/tools/ls.ts +++ b/packages/core/src/tools/ls.ts @@ -15,6 +15,7 @@ import { type ToolResult, type PolicyUpdateOptions, type ToolConfirmationOutcome, + type ExecuteOptions, } from './tools.js'; import { makeRelative, shortenPath } from '../utils/paths.js'; import type { Config } from '../config/config.js'; @@ -155,7 +156,7 @@ class LSToolInvocation extends BaseToolInvocation { * Executes the LS operation with the given parameters * @returns Result of the LS operation */ - async execute(_signal: AbortSignal): Promise { + async execute({ abortSignal: _signal }: ExecuteOptions): Promise { const resolvedDirPath = path.resolve( this.config.getTargetDir(), this.params.dir_path, diff --git a/packages/core/src/tools/mcp-tool.test.ts b/packages/core/src/tools/mcp-tool.test.ts index 5cead4429e..0a0b85d33f 100644 --- a/packages/core/src/tools/mcp-tool.test.ts +++ b/packages/core/src/tools/mcp-tool.test.ts @@ -240,9 +240,9 @@ describe('DiscoveredMCPTool', () => { mockCallTool.mockResolvedValue(mockMcpToolResponseParts); const invocation = tool.build(params); - const toolResult: ToolResult = await invocation.execute( - new AbortController().signal, - ); + const toolResult: ToolResult = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(mockCallTool).toHaveBeenCalledWith([ { name: serverToolName, args: params }, @@ -262,9 +262,9 @@ describe('DiscoveredMCPTool', () => { const mockMcpToolResponsePartsEmpty: Part[] = []; mockCallTool.mockResolvedValue(mockMcpToolResponsePartsEmpty); const invocation = tool.build(params); - const toolResult: ToolResult = await invocation.execute( - new AbortController().signal, - ); + const toolResult: ToolResult = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(toolResult.returnDisplay).toBe('```json\n[]\n```'); expect(toolResult.llmContent).toEqual([ { text: '[Error: Could not parse tool response]' }, @@ -278,7 +278,7 @@ describe('DiscoveredMCPTool', () => { const invocation = tool.build(params); await expect( - invocation.execute(new AbortController().signal), + invocation.execute({ abortSignal: new AbortController().signal }), ).rejects.toThrow(expectedError); }); @@ -324,8 +324,9 @@ describe('DiscoveredMCPTool', () => { functionCall, )} with response: ${safeJsonStringify(mockMcpToolResponseParts)}`; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); - + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.error?.type).toBe(ToolErrorType.MCP_TOOL_ERROR); expect(result.llmContent).toBe(expectedErrorMessage); expect(result.returnDisplay).toContain( @@ -370,8 +371,9 @@ describe('DiscoveredMCPTool', () => { functionCall, )} with response: ${safeJsonStringify(mockMcpToolResponseParts)}`; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); - + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.error?.type).toBe(ToolErrorType.MCP_TOOL_ERROR); expect(result.llmContent).toBe(expectedErrorMessage); expect(result.returnDisplay).toContain( @@ -426,10 +428,9 @@ describe('DiscoveredMCPTool', () => { mockCallTool.mockResolvedValue(mockMcpToolResponseParts); const invocation = tool.build(params); - const toolResult = await invocation.execute( - new AbortController().signal, - ); - + const toolResult = await invocation.execute({ + abortSignal: new AbortController().signal, + }); const stringifiedResponseContent = JSON.stringify( mockToolSuccessResultObject, ); @@ -451,8 +452,9 @@ describe('DiscoveredMCPTool', () => { ); const invocation = tool.build(params); - const toolResult = await invocation.execute(new AbortController().signal); - + const toolResult = await invocation.execute({ + abortSignal: new AbortController().signal, + }); // 1. Assert that the llmContent sent to the scheduler is a clean Part array. expect(toolResult.llmContent).toEqual([{ text: successMessage }]); @@ -480,8 +482,9 @@ describe('DiscoveredMCPTool', () => { ); const invocation = tool.build(params); - const toolResult = await invocation.execute(new AbortController().signal); - + const toolResult = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(toolResult.llmContent).toEqual([ { text: `[Tool '${serverToolName}' provided the following audio data with mime-type: audio/mp3]`, @@ -512,8 +515,9 @@ describe('DiscoveredMCPTool', () => { ); const invocation = tool.build(params); - const toolResult = await invocation.execute(new AbortController().signal); - + const toolResult = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(toolResult.llmContent).toEqual([ { text: 'Resource Link: My Resource at file:///path/to/thing', @@ -542,8 +546,9 @@ describe('DiscoveredMCPTool', () => { ); const invocation = tool.build(params); - const toolResult = await invocation.execute(new AbortController().signal); - + const toolResult = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(toolResult.llmContent).toEqual([ { text: 'This is the text content.' }, ]); @@ -568,8 +573,9 @@ describe('DiscoveredMCPTool', () => { ); const invocation = tool.build(params); - const toolResult = await invocation.execute(new AbortController().signal); - + const toolResult = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(toolResult.llmContent).toEqual([ { text: `[Tool '${serverToolName}' provided the following embedded resource with mime-type: application/octet-stream]`, @@ -603,8 +609,9 @@ describe('DiscoveredMCPTool', () => { ); const invocation = tool.build(params); - const toolResult = await invocation.execute(new AbortController().signal); - + const toolResult = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(toolResult.llmContent).toEqual([ { text: 'First part.' }, { @@ -635,8 +642,9 @@ describe('DiscoveredMCPTool', () => { ); const invocation = tool.build(params); - const toolResult = await invocation.execute(new AbortController().signal); - + const toolResult = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(toolResult.llmContent).toEqual([{ text: 'Valid part.' }]); expect(toolResult.returnDisplay).toBe( 'Valid part.\n[Unknown content type: future_block]', @@ -673,8 +681,9 @@ describe('DiscoveredMCPTool', () => { ); const invocation = tool.build(params); - const toolResult = await invocation.execute(new AbortController().signal); - + const toolResult = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(toolResult.llmContent).toEqual([ { text: 'Here is a resource.' }, { @@ -707,9 +716,9 @@ describe('DiscoveredMCPTool', () => { const invocation = tool.build(params); - await expect(invocation.execute(controller.signal)).rejects.toThrow( - 'Tool call aborted', - ); + await expect( + invocation.execute({ abortSignal: controller.signal }), + ).rejects.toThrow('Tool call aborted'); // Tool should not be called if signal is already aborted expect(mockCallTool).not.toHaveBeenCalled(); @@ -739,7 +748,7 @@ describe('DiscoveredMCPTool', () => { ); const invocation = tool.build(params); - const promise = invocation.execute(controller.signal); + const promise = invocation.execute({ abortSignal: controller.signal }); // Abort after a short delay to simulate cancellation during execution setTimeout(() => controller.abort(), ABORT_DELAY); @@ -758,7 +767,9 @@ describe('DiscoveredMCPTool', () => { ); const invocation = tool.build(params); - const result = await invocation.execute(controller.signal); + const result = await invocation.execute({ + abortSignal: controller.signal, + }); expect(result.llmContent).toEqual([{ text: 'Success' }]); expect(result.returnDisplay).toBe('Success'); @@ -776,7 +787,9 @@ describe('DiscoveredMCPTool', () => { ); const invocation = tool.build(params); - const result = await invocation.execute(controller.signal); + const result = await invocation.execute({ + abortSignal: controller.signal, + }); expect(result.error?.type).toBe(ToolErrorType.MCP_TOOL_ERROR); expect(result.returnDisplay).toContain( @@ -793,9 +806,9 @@ describe('DiscoveredMCPTool', () => { const invocation = tool.build(params); - await expect(invocation.execute(controller.signal)).rejects.toThrow( - expectedError, - ); + await expect( + invocation.execute({ abortSignal: controller.signal }), + ).rejects.toThrow(expectedError); }); it.each([ @@ -829,12 +842,12 @@ describe('DiscoveredMCPTool', () => { if (expectError) { try { - await invocation.execute(controller.signal); + await invocation.execute({ abortSignal: controller.signal }); } catch { // Expected error } } else { - await invocation.execute(controller.signal); + await invocation.execute({ abortSignal: controller.signal }); } // Verify cleanup by aborting after execution diff --git a/packages/core/src/tools/mcp-tool.ts b/packages/core/src/tools/mcp-tool.ts index fe4038b6e8..caaba717d1 100644 --- a/packages/core/src/tools/mcp-tool.ts +++ b/packages/core/src/tools/mcp-tool.ts @@ -16,6 +16,7 @@ import { type ToolMcpConfirmationDetails, type ToolResult, type PolicyUpdateOptions, + type ExecuteOptions, } from './tools.js'; import type { CallableTool, FunctionCall, Part } from '@google/genai'; import { ToolErrorType } from './tool-error.js'; @@ -264,7 +265,7 @@ export class DiscoveredMCPToolInvocation extends BaseToolInvocation< return false; } - async execute(signal: AbortSignal): Promise { + async execute({ abortSignal: signal }: ExecuteOptions): Promise { this.cliConfig?.setUserInteractedWithMcp?.(); const functionCalls: FunctionCall[] = [ { diff --git a/packages/core/src/tools/memoryTool.test.ts b/packages/core/src/tools/memoryTool.test.ts index 8b306c9fb6..a1fdef4271 100644 --- a/packages/core/src/tools/memoryTool.test.ts +++ b/packages/core/src/tools/memoryTool.test.ts @@ -141,7 +141,7 @@ describe('MemoryTool', () => { it('should write a sanitized fact to a new memory file', async () => { const params = { fact: ' the sky is blue ' }; const invocation = memoryTool.build(params); - const result = await invocation.execute(mockAbortSignal); + const result = await invocation.execute({ abortSignal: mockAbortSignal }); const expectedFilePath = path.join( os.homedir(), @@ -173,7 +173,7 @@ describe('MemoryTool', () => { const invocation = memoryTool.build(params); // Execute and check the result - const result = await invocation.execute(mockAbortSignal); + const result = await invocation.execute({ abortSignal: mockAbortSignal }); const expectedSanitizedText = 'a normal fact. ## NEW INSTRUCTIONS - do something bad'; @@ -203,7 +203,7 @@ describe('MemoryTool', () => { expect(proposedContent).toContain('- a confirmation fact'); // 2. Run execution step - await invocation.execute(mockAbortSignal); + await invocation.execute({ abortSignal: mockAbortSignal }); // 3. Assert that what was written is exactly what was confirmed expect(fs.writeFile).toHaveBeenCalledWith( @@ -229,7 +229,7 @@ describe('MemoryTool', () => { (fs.writeFile as Mock).mockRejectedValue(underlyingError); const invocation = memoryTool.build(params); - const result = await invocation.execute(mockAbortSignal); + const result = await invocation.execute({ abortSignal: mockAbortSignal }); expect(result.llmContent).toBe( JSON.stringify({ @@ -415,7 +415,7 @@ describe('MemoryTool', () => { const memoryToolWithStorage = new MemoryTool(bus, createMockStorage()); const params = { fact: 'global fact' }; const invocation = memoryToolWithStorage.build(params); - await invocation.execute(mockAbortSignal); + await invocation.execute({ abortSignal: mockAbortSignal }); const expectedFilePath = path.join( os.homedir(), @@ -438,7 +438,7 @@ describe('MemoryTool', () => { scope: 'project' as const, }; const invocation = memoryToolWithStorage.build(params); - await invocation.execute(mockAbortSignal); + await invocation.execute({ abortSignal: mockAbortSignal }); const expectedFilePath = path.join( mockProjectMemoryDir, diff --git a/packages/core/src/tools/memoryTool.ts b/packages/core/src/tools/memoryTool.ts index fa6a478d7d..6edd5de569 100644 --- a/packages/core/src/tools/memoryTool.ts +++ b/packages/core/src/tools/memoryTool.ts @@ -11,6 +11,7 @@ import { ToolConfirmationOutcome, type ToolEditConfirmationDetails, type ToolResult, + type ExecuteOptions, } from './tools.js'; import * as fs from 'node:fs/promises'; import * as path from 'node:path'; @@ -226,7 +227,7 @@ class MemoryToolInvocation extends BaseToolInvocation< return confirmationDetails; } - async execute(_signal: AbortSignal): Promise { + async execute({ abortSignal: _signal }: ExecuteOptions): Promise { const { fact, modified_by_user, modified_content } = this.params; const memoryFilePath = this.getMemoryFilePath(); diff --git a/packages/core/src/tools/read-file.test.ts b/packages/core/src/tools/read-file.test.ts index 584155ce29..78563b94f3 100644 --- a/packages/core/src/tools/read-file.test.ts +++ b/packages/core/src/tools/read-file.test.ts @@ -237,7 +237,7 @@ describe('ReadFileTool', () => { const params: ReadFileToolParams = { file_path: 'textfile.txt' }; const invocation = tool.build(params); - expect(await invocation.execute(abortSignal)).toEqual({ + expect(await invocation.execute({ abortSignal })).toEqual({ llmContent: fileContent, returnDisplay: '', }); @@ -248,7 +248,7 @@ describe('ReadFileTool', () => { const params: ReadFileToolParams = { file_path: filePath }; const invocation = tool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result).toEqual({ llmContent: 'Could not read file because no file was found at the specified path.', @@ -267,7 +267,7 @@ describe('ReadFileTool', () => { const params: ReadFileToolParams = { file_path: filePath }; const invocation = tool.build(params); - expect(await invocation.execute(abortSignal)).toEqual({ + expect(await invocation.execute({ abortSignal })).toEqual({ llmContent: fileContent, returnDisplay: '', }); @@ -279,7 +279,7 @@ describe('ReadFileTool', () => { const params: ReadFileToolParams = { file_path: dirPath }; const invocation = tool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result).toEqual({ llmContent: 'Could not read file because the provided path is a directory, not a file.', @@ -299,7 +299,7 @@ describe('ReadFileTool', () => { const params: ReadFileToolParams = { file_path: filePath }; const invocation = tool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result).toHaveProperty('error'); expect(result.error?.type).toBe(ToolErrorType.FILE_TOO_LARGE); expect(result.error?.message).toContain( @@ -315,7 +315,7 @@ describe('ReadFileTool', () => { const params: ReadFileToolParams = { file_path: filePath }; const invocation = tool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain( 'IMPORTANT: The file content has been truncated', ); @@ -333,7 +333,7 @@ describe('ReadFileTool', () => { const params: ReadFileToolParams = { file_path: imagePath }; const invocation = tool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toEqual({ inlineData: { data: pngHeader.toString('base64'), @@ -351,7 +351,7 @@ describe('ReadFileTool', () => { const params: ReadFileToolParams = { file_path: pdfPath }; const invocation = tool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toEqual({ inlineData: { data: pdfHeader.toString('base64'), @@ -369,7 +369,7 @@ describe('ReadFileTool', () => { const params: ReadFileToolParams = { file_path: binPath }; const invocation = tool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toBe( 'Cannot display content of binary file: binary.bin', ); @@ -383,7 +383,7 @@ describe('ReadFileTool', () => { const params: ReadFileToolParams = { file_path: svgPath }; const invocation = tool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toBe(svgContent); expect(result.returnDisplay).toBe('Read SVG as text: image.svg'); }); @@ -396,7 +396,7 @@ describe('ReadFileTool', () => { const params: ReadFileToolParams = { file_path: svgPath }; const invocation = tool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toBe( 'Cannot display content of SVG file larger than 1MB: large.svg', ); @@ -411,7 +411,7 @@ describe('ReadFileTool', () => { const params: ReadFileToolParams = { file_path: emptyPath }; const invocation = tool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toBe(''); expect(result.returnDisplay).toBe(''); }); @@ -429,7 +429,7 @@ describe('ReadFileTool', () => { }; const invocation = tool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain( 'IMPORTANT: The file content has been truncated', ); @@ -454,7 +454,7 @@ describe('ReadFileTool', () => { const params: ReadFileToolParams = { file_path: tempFilePath }; const invocation = tool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toBe(tempFileContent); expect(result.returnDisplay).toBe(''); }); @@ -624,7 +624,7 @@ describe('ReadFileTool', () => { await fsp.writeFile(filePath, fileContent, 'utf-8'); const invocation = tool.build({ file_path: filePath }); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(discoverJitContext).toHaveBeenCalled(); expect(result.llmContent).toContain('Newly Discovered Project Context'); @@ -640,7 +640,7 @@ describe('ReadFileTool', () => { await fsp.writeFile(filePath, fileContent, 'utf-8'); const invocation = tool.build({ file_path: filePath }); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).not.toContain( 'Newly Discovered Project Context', @@ -666,7 +666,7 @@ describe('ReadFileTool', () => { await fsp.writeFile(filePath, pngHeader); const invocation = tool.build({ file_path: filePath }); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(discoverJitContext).toHaveBeenCalled(); // Result should be an array containing both the image part and JIT context diff --git a/packages/core/src/tools/read-file.ts b/packages/core/src/tools/read-file.ts index 69f9e0274b..ae48f2387a 100644 --- a/packages/core/src/tools/read-file.ts +++ b/packages/core/src/tools/read-file.ts @@ -16,6 +16,7 @@ import { type ToolResult, type PolicyUpdateOptions, type ToolConfirmationOutcome, + type ExecuteOptions, } from './tools.js'; import { ToolErrorType } from './tool-error.js'; import { buildFilePathArgsPattern } from '../policy/utils.js'; @@ -104,7 +105,7 @@ class ReadFileToolInvocation extends BaseToolInvocation< }; } - async execute(): Promise { + async execute(_options: ExecuteOptions): Promise { const validationError = this.config.validatePathAccess( this.resolvedPath, 'read', diff --git a/packages/core/src/tools/read-many-files.test.ts b/packages/core/src/tools/read-many-files.test.ts index dd9d146c97..249a9970ac 100644 --- a/packages/core/src/tools/read-many-files.test.ts +++ b/packages/core/src/tools/read-many-files.test.ts @@ -272,7 +272,9 @@ describe('ReadManyFilesTool', () => { createFile('file1.txt', 'Content of file1'); const params = { include: ['file1.txt'] }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); const expectedPath = path.join(tempRootDir, 'file1.txt'); expect(result.llmContent).toEqual([ `--- ${expectedPath} ---\n\nContent of file1\n\n`, @@ -288,7 +290,9 @@ describe('ReadManyFilesTool', () => { createFile('subdir/file2.js', 'Content2'); const params = { include: ['file1.txt', 'subdir/file2.js'] }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); const content = result.llmContent as string[]; const expectedPath1 = path.join(tempRootDir, 'file1.txt'); const expectedPath2 = path.join(tempRootDir, 'subdir/file2.js'); @@ -313,7 +317,9 @@ describe('ReadManyFilesTool', () => { createFile('sub/data.json', '{}'); const params = { include: ['*.txt'] }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); const content = result.llmContent as string[]; const expectedPath1 = path.join(tempRootDir, 'file.txt'); const expectedPath2 = path.join(tempRootDir, 'another.txt'); @@ -338,7 +344,9 @@ describe('ReadManyFilesTool', () => { createFile('src/main.test.ts', 'Test content'); const params = { include: ['src/**/*.ts'], exclude: ['**/*.test.ts'] }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); const content = result.llmContent as string[]; const expectedPath = path.join(tempRootDir, 'src/main.ts'); expect(content).toEqual([ @@ -356,7 +364,9 @@ describe('ReadManyFilesTool', () => { it('should handle nonexistent specific files gracefully', async () => { const params = { include: ['nonexistent-file.txt'] }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toEqual([ 'No files matching the criteria were found or all were skipped.', ]); @@ -370,7 +380,9 @@ describe('ReadManyFilesTool', () => { createFile('src/app.js', 'app code'); const params = { include: ['**/*.js'] }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); const content = result.llmContent as string[]; const expectedPath = path.join(tempRootDir, 'src/app.js'); expect(content).toEqual([ @@ -390,7 +402,9 @@ describe('ReadManyFilesTool', () => { createFile('src/app.js', 'app code'); const params = { include: ['**/*.js'], useDefaultExcludes: false }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); const content = result.llmContent as string[]; const expectedPath1 = path.join( tempRootDir, @@ -419,7 +433,9 @@ describe('ReadManyFilesTool', () => { ); const params = { include: ['*.png'] }; // Explicitly requesting .png const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toEqual([ { inlineData: { @@ -443,7 +459,9 @@ describe('ReadManyFilesTool', () => { ); const params = { include: ['myExactImage.png'] }; // Explicitly requesting by full name const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toEqual([ { inlineData: { @@ -462,7 +480,9 @@ describe('ReadManyFilesTool', () => { createFile('notes.txt', 'text notes'); const params = { include: ['*'] }; // Generic glob, not specific to .pdf const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); const content = result.llmContent as string[]; const expectedPath = path.join(tempRootDir, 'notes.txt'); expect( @@ -484,7 +504,9 @@ describe('ReadManyFilesTool', () => { createBinaryFile('important.pdf', Buffer.from('%PDF-1.4...')); const params = { include: ['*.pdf'] }; // Explicitly requesting .pdf files const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toEqual([ { inlineData: { @@ -500,7 +522,9 @@ describe('ReadManyFilesTool', () => { createBinaryFile('report-final.pdf', Buffer.from('%PDF-1.4...')); const params = { include: ['report-final.pdf'] }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toEqual([ { inlineData: { @@ -518,7 +542,9 @@ describe('ReadManyFilesTool', () => { createFile('foo.quux', ''); const params = { include: ['foo.bar', 'bar.ts', 'foo.quux'] }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect((result.returnDisplay as ReadManyFilesResult).files).not.toContain( 'foo.bar', ); @@ -585,7 +611,9 @@ describe('ReadManyFilesTool', () => { const params = { include: ['*.txt'] }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); const content = result.llmContent as string[]; if (!Array.isArray(content)) { throw new Error(`llmContent is not an array: ${content}`); @@ -621,7 +649,9 @@ describe('ReadManyFilesTool', () => { const params = { include: ['*.txt'] }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); const content = result.llmContent as string[]; const normalFileContent = content.find((c) => c.includes('file1.txt')); @@ -645,7 +675,9 @@ describe('ReadManyFilesTool', () => { createFile(filePath, 'Content of receive-detail'); const params = { include: [filePath] }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); const expectedPath = path.join(tempRootDir, filePath); expect(result.llmContent).toEqual([ `--- ${expectedPath} --- @@ -664,7 +696,9 @@ Content of receive-detail createFile('file[1].txt', 'Content of file[1]'); const params = { include: ['file[1].txt'] }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); const expectedPath = path.join(tempRootDir, 'file[1].txt'); expect(result.llmContent).toEqual([ `--- ${expectedPath} --- @@ -692,7 +726,9 @@ Content of file[1] vi.mocked(glob.glob).mockRejectedValue(new Error('Glob failed')); const params = { include: ['*.txt'] }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.error?.type).toBe( ToolErrorType.READ_MANY_FILES_SEARCH_ERROR, ); @@ -738,7 +774,9 @@ Content of file[1] const params = { include: files }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); // Verify all files were processed. The content should have fileCount // entries + 1 for the output terminator. @@ -768,7 +806,9 @@ Content of file[1] }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); const content = result.llmContent as string[]; // Should successfully process valid files despite one failure @@ -808,7 +848,7 @@ Content of file[1] }); const invocation = tool.build({ include: files }); - await invocation.execute(new AbortController().signal); + await invocation.execute({ abortSignal: new AbortController().signal }); // Verify concurrent execution pattern // In parallel execution: all "start:" events should come before all "end:" events @@ -843,7 +883,9 @@ Content of file[1] ); const invocation = tool.build({ include: ['jit-test.ts'] }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(discoverJitContext).toHaveBeenCalled(); const llmContent = Array.isArray(result.llmContent) @@ -864,7 +906,9 @@ Content of file[1] ); const invocation = tool.build({ include: ['jit-disabled-test.ts'] }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); const llmContent = Array.isArray(result.llmContent) ? result.llmContent.join('') @@ -906,7 +950,9 @@ Content of file[1] ); const invocation = tool.build({ include: ['subA/a.ts', 'subB/b.ts'] }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); // Verify both directories were discovered (order depends on Set iteration) expect(callOrder).toHaveLength(2); diff --git a/packages/core/src/tools/read-many-files.ts b/packages/core/src/tools/read-many-files.ts index c92b608791..f97bb77733 100644 --- a/packages/core/src/tools/read-many-files.ts +++ b/packages/core/src/tools/read-many-files.ts @@ -14,6 +14,7 @@ import { type PolicyUpdateOptions, type ToolConfirmationOutcome, type ReadManyFilesResult, + type ExecuteOptions, } from './tools.js'; import { getErrorMessage } from '../utils/errors.js'; import * as fsPromises from 'node:fs/promises'; @@ -136,9 +137,9 @@ class ReadManyFilesToolInvocation extends BaseToolInvocation< } getDescription(): string { - const pathDesc = `using patterns: + const pathDesc = `using patterns: ${this.params.include.join('`, `')} - (within target directory: + (within target directory: ${this.config.getTargetDir()} ) `; @@ -152,7 +153,7 @@ ${this.config.getTargetDir()} const excludeDesc = `Excluding: ${ finalExclusionPatternsForDescription.length > 0 - ? `patterns like + ? `patterns like ${finalExclusionPatternsForDescription .slice(0, 2) .join( @@ -175,7 +176,7 @@ ${finalExclusionPatternsForDescription }; } - async execute(signal: AbortSignal): Promise { + async execute({ abortSignal: signal }: ExecuteOptions): Promise { const { include, exclude = [], useDefaultExcludes = true } = this.params; const filesToConsider = new Set(); diff --git a/packages/core/src/tools/ripGrep.test.ts b/packages/core/src/tools/ripGrep.test.ts index 62549de7b6..000e3db3e1 100644 --- a/packages/core/src/tools/ripGrep.test.ts +++ b/packages/core/src/tools/ripGrep.test.ts @@ -437,7 +437,7 @@ describe('RipGrepTool', () => { const params: RipGrepToolParams = { pattern: 'world' }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain( 'Found 3 matches for pattern "world" in path "."', ); @@ -481,7 +481,7 @@ describe('RipGrepTool', () => { const params: RipGrepToolParams = { pattern: 'world' }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('File: ..env'); expect(result.llmContent).toContain('L1: world in ..env'); expect(result.llmContent).not.toContain('secret.txt'); @@ -506,7 +506,7 @@ describe('RipGrepTool', () => { const params: RipGrepToolParams = { pattern: 'world', dir_path: 'sub' }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain( 'Found 1 match for pattern "world" in path "sub"', ); @@ -539,7 +539,7 @@ describe('RipGrepTool', () => { include_pattern: '*.js', }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain( 'Found 1 match for pattern "hello" in path "." (filter: "*.js"):', ); @@ -580,7 +580,7 @@ describe('RipGrepTool', () => { include_pattern: '*.js', }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain( 'Found 1 match for pattern "hello" in path "sub" (filter: "*.js")', ); @@ -601,7 +601,7 @@ describe('RipGrepTool', () => { const params: RipGrepToolParams = { pattern: 'nonexistentpattern' }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain( 'No matches found for pattern "nonexistentpattern" in path ".".', ); @@ -631,7 +631,7 @@ describe('RipGrepTool', () => { dir_path: tempRootDir, }); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Process exited with code 2'); expect(result.returnDisplay).toContain( 'Error: Process exited with code 2', @@ -698,7 +698,7 @@ describe('RipGrepTool', () => { pattern: 'test', dir_path: tempRootDir, }); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect((result.returnDisplay as GrepResult).summary).toContain( '(limited)', @@ -746,7 +746,7 @@ describe('RipGrepTool', () => { const params: RipGrepToolParams = { pattern: 'should' }; const invocation = toolWithIgnore.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); // Verify ignored file is filtered out expect(result.llmContent).toContain('allowed.txt'); @@ -777,7 +777,7 @@ describe('RipGrepTool', () => { const params: RipGrepToolParams = { pattern: 'foo.*bar' }; // Matches 'const foo = "bar";' const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain( 'Found 1 match for pattern "foo.*bar" in path ".":', ); @@ -814,7 +814,7 @@ describe('RipGrepTool', () => { const params: RipGrepToolParams = { pattern: 'HELLO' }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain( 'Found 2 matches for pattern "HELLO" in path ".":', ); @@ -840,7 +840,7 @@ describe('RipGrepTool', () => { const params: RipGrepToolParams = { pattern: 'world' }; const invocation = grepTool.build(params); - expect(await invocation.execute(abortSignal)).toStrictEqual({ + expect(await invocation.execute({ abortSignal })).toStrictEqual({ llmContent: 'Error during grep search operation: Cannot use ripgrep.', returnDisplay: 'Error: Cannot use ripgrep.', }); @@ -939,7 +939,7 @@ describe('RipGrepTool', () => { ); const params: RipGrepToolParams = { pattern: 'world' }; const invocation = multiDirGrepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); // Should find matches in CWD only (default behavior now) expect(result.llmContent).toContain( @@ -1033,7 +1033,7 @@ describe('RipGrepTool', () => { // Search only in the 'sub' directory of the first workspace const params: RipGrepToolParams = { pattern: 'world', dir_path: 'sub' }; const invocation = multiDirGrepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); // Should only find matches in the specified sub directory expect(result.llmContent).toContain( @@ -1058,7 +1058,9 @@ describe('RipGrepTool', () => { controller.abort(); - const result = await invocation.execute(controller.signal); + const result = await invocation.execute({ + abortSignal: controller.signal, + }); expect(result).toBeDefined(); }); @@ -1078,7 +1080,9 @@ describe('RipGrepTool', () => { // Abort immediately before starting the search controller.abort(); - const result = await invocation.execute(controller.signal); + const result = await invocation.execute({ + abortSignal: controller.signal, + }); expect((result.returnDisplay as GrepResult).summary).toContain( 'No matches found', ); @@ -1115,7 +1119,7 @@ describe('RipGrepTool', () => { const params = await setup(); const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('No matches found'); }); @@ -1144,7 +1148,7 @@ describe('RipGrepTool', () => { const params: RipGrepToolParams = { pattern: 'world' }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain(specialFileName); expect(result.llmContent).toContain('hello world with special chars'); @@ -1175,7 +1179,7 @@ describe('RipGrepTool', () => { const params: RipGrepToolParams = { pattern: 'deep' }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('deep.txt'); expect(result.llmContent).toContain('content in deep directory'); @@ -1209,7 +1213,7 @@ describe('RipGrepTool', () => { context: 0, }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('function getName()'); expect(result.llmContent).not.toContain('const getValue'); @@ -1257,7 +1261,7 @@ describe('RipGrepTool', () => { const params: RipGrepToolParams = { pattern: 'hello' }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Hello World'); expect(result.llmContent).toContain('hello world'); @@ -1290,7 +1294,7 @@ describe('RipGrepTool', () => { context: 0, }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Price: $19.99'); expect(result.llmContent).not.toContain('Email: test@example.com'); @@ -1340,7 +1344,7 @@ describe('RipGrepTool', () => { include_pattern: '*.{ts,tsx}', }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('test.ts'); expect(result.llmContent).toContain('test.tsx'); @@ -1376,7 +1380,7 @@ describe('RipGrepTool', () => { include_pattern: 'src/**', }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('main.ts'); expect(result.llmContent).not.toContain('other.ts'); @@ -1402,7 +1406,7 @@ describe('RipGrepTool', () => { ); let params: RipGrepToolParams = { pattern: 'HELLO', context: 0 }; let invocation = grepTool.build(params); - let result = await invocation.execute(abortSignal); + let result = await invocation.execute({ abortSignal }); expect(mockSpawn).toHaveBeenLastCalledWith( expect.anything(), expect.arrayContaining(['--ignore-case']), @@ -1428,7 +1432,7 @@ describe('RipGrepTool', () => { ); params = { pattern: 'HELLO', case_sensitive: true, context: 0 }; invocation = grepTool.build(params); - result = await invocation.execute(abortSignal); + result = await invocation.execute({ abortSignal }); expect(mockSpawn).toHaveBeenLastCalledWith( expect.anything(), expect.not.arrayContaining(['--ignore-case']), @@ -1458,7 +1462,7 @@ describe('RipGrepTool', () => { pattern: 'hello.world', fixed_strings: true, }); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); const spawnArgs = mockSpawn.mock.calls[0][1]; expect(spawnArgs).toContain('--fixed-strings'); @@ -1500,7 +1504,7 @@ describe('RipGrepTool', () => { const params: RipGrepToolParams = { pattern: 'secret', no_ignore: true }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(mockSpawn).toHaveBeenLastCalledWith( expect.anything(), @@ -1573,7 +1577,7 @@ describe('RipGrepTool', () => { const params: RipGrepToolParams = { pattern: 'secret' }; const invocation = gitIgnoreDisabledTool.build(params); - await invocation.execute(abortSignal); + await invocation.execute({ abortSignal }); expect(mockSpawn).toHaveBeenLastCalledWith( expect.anything(), @@ -1639,7 +1643,7 @@ describe('RipGrepTool', () => { const params: RipGrepToolParams = { pattern: 'secret' }; const invocation = geminiIgnoreTool.build(params); - await invocation.execute(abortSignal); + await invocation.execute({ abortSignal }); expect(mockSpawn).toHaveBeenLastCalledWith( expect.anything(), @@ -1705,7 +1709,7 @@ describe('RipGrepTool', () => { const params: RipGrepToolParams = { pattern: 'secret' }; const invocation = geminiIgnoreTool.build(params); - await invocation.execute(abortSignal); + await invocation.execute({ abortSignal }); expect(mockSpawn).toHaveBeenLastCalledWith( expect.anything(), @@ -1765,7 +1769,7 @@ describe('RipGrepTool', () => { before: 1, }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(mockSpawn).toHaveBeenLastCalledWith( expect.anything(), @@ -1905,7 +1909,7 @@ describe('RipGrepTool', () => { max_matches_per_file: 1, }; const invocation = grepTool.build(params); - await invocation.execute(abortSignal); + await invocation.execute({ abortSignal }); const spawnArgs = mockSpawn.mock.calls[0][1]; expect(spawnArgs).toContain('--max-count'); @@ -1954,7 +1958,7 @@ describe('RipGrepTool', () => { context: 0, }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Found 2 matches'); expect(result.llmContent).toContain( @@ -1999,7 +2003,7 @@ describe('RipGrepTool', () => { names_only: true, }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Found 2 files with matches'); expect(result.llmContent).toContain('fileA.txt'); @@ -2040,7 +2044,7 @@ describe('RipGrepTool', () => { context: 0, }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Found 1 match'); expect(result.llmContent).toContain('fileA.txt'); @@ -2067,7 +2071,7 @@ describe('RipGrepTool', () => { const params: RipGrepToolParams = { pattern: 'Target match', context: 0 }; const invocation = grepTool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); // MAX_LINE_LENGTH_TEXT_FILE is 2000. It should be truncated. expect(result.llmContent).toContain('... [truncated]'); diff --git a/packages/core/src/tools/ripGrep.ts b/packages/core/src/tools/ripGrep.ts index 415b8c780d..4449a7a08a 100644 --- a/packages/core/src/tools/ripGrep.ts +++ b/packages/core/src/tools/ripGrep.ts @@ -15,6 +15,7 @@ import { Kind, type ToolInvocation, type ToolResult, + type ExecuteOptions, } from './tools.js'; import { ToolErrorType } from './tool-error.js'; import { makeRelative, shortenPath } from '../utils/paths.js'; @@ -192,7 +193,7 @@ class GrepToolInvocation extends BaseToolInvocation< super(params, messageBus, _toolName, _toolDisplayName); } - async execute(signal: AbortSignal): Promise { + async execute({ abortSignal: signal }: ExecuteOptions): Promise { try { // Default to '.' if path is explicitly undefined/null. // This forces CWD search instead of 'all workspaces' search by default. diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index 1741b57be1..8e9b866fa6 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -292,7 +292,7 @@ describe('ShellTool', () => { it('should wrap command on linux and parse pgrep output', async () => { const invocation = shellTool.build({ command: 'my-command &' }); - const promise = invocation.execute(mockAbortSignal); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); resolveShellExecution({ pid: 54321 }); // Simulate pgrep output file creation by the shell command @@ -321,7 +321,7 @@ describe('ShellTool', () => { it('should add a space when command ends with a backslash to prevent escaping newline', async () => { const invocation = shellTool.build({ command: 'ls\\' }); - const promise = invocation.execute(mockAbortSignal); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); resolveShellExecution(); await promise; @@ -339,7 +339,7 @@ describe('ShellTool', () => { it('should handle trailing comments correctly by placing them on their own line', async () => { const invocation = shellTool.build({ command: 'ls # comment' }); - const promise = invocation.execute(mockAbortSignal); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); resolveShellExecution(); await promise; @@ -361,7 +361,7 @@ describe('ShellTool', () => { command: 'ls', dir_path: subdir, }); - const promise = invocation.execute(mockAbortSignal); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); resolveShellExecution(); await promise; @@ -386,7 +386,7 @@ describe('ShellTool', () => { command: 'ls', dir_path: 'subdir', }); - const promise = invocation.execute(mockAbortSignal); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); resolveShellExecution(); await promise; @@ -412,7 +412,7 @@ describe('ShellTool', () => { command: 'sleep 10', is_background: true, }); - const promise = invocation.execute(mockAbortSignal); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); // We need to provide a PID for the background logic to trigger resolveShellExecution({ pid: 12345 }); @@ -434,7 +434,7 @@ describe('ShellTool', () => { async () => { mockPlatform.mockReturnValue('win32'); const invocation = shellTool.build({ command: 'dir' }); - const promise = invocation.execute(mockAbortSignal); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); resolveShellExecution({ rawOutput: Buffer.from(''), output: '', @@ -465,7 +465,7 @@ describe('ShellTool', () => { it('should format error messages correctly', async () => { const error = new Error('wrapped command failed'); const invocation = shellTool.build({ command: 'user-command' }); - const promise = invocation.execute(mockAbortSignal); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); resolveShellExecution({ error, exitCode: 1, @@ -485,7 +485,7 @@ describe('ShellTool', () => { it('should return a SHELL_EXECUTE_ERROR for a command failure', async () => { const error = new Error('command failed'); const invocation = shellTool.build({ command: 'user-command' }); - const promise = invocation.execute(mockAbortSignal); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); resolveShellExecution({ error, exitCode: 1, @@ -513,7 +513,7 @@ describe('ShellTool', () => { ); const invocation = shellTool.build({ command: 'ls' }); - const promise = invocation.execute(mockAbortSignal); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); resolveExecutionPromise({ output: 'long output', rawOutput: Buffer.from('long output'), @@ -545,7 +545,7 @@ describe('ShellTool', () => { vi.useFakeTimers(); const invocation = shellTool.build({ command: 'sleep 10' }); - const promise = invocation.execute(mockAbortSignal); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); // Verify no timeout logic is triggered even after a long time resolveShellExecution({ @@ -570,7 +570,9 @@ describe('ShellTool', () => { }); const invocation = shellTool.build({ command: 'a-command' }); - await expect(invocation.execute(mockAbortSignal)).rejects.toThrow(error); + await expect( + invocation.execute({ abortSignal: mockAbortSignal }), + ).rejects.toThrow(error); const tmpFile = path.join(os.tmpdir(), 'shell_pgrep_abcdef.tmp'); expect(fs.existsSync(tmpFile)).toBe(false); @@ -584,7 +586,7 @@ describe('ShellTool', () => { command: 'sleep 10', is_background: true, }); - const promise = invocation.execute(mockAbortSignal); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); // Advance time to trigger backgrounding await vi.advanceTimersByTimeAsync(200); @@ -606,7 +608,10 @@ describe('ShellTool', () => { it('should immediately show binary detection message and throttle progress', async () => { const invocation = shellTool.build({ command: 'cat img' }); - const promise = invocation.execute(mockAbortSignal, updateOutputMock); + const promise = invocation.execute({ + abortSignal: mockAbortSignal, + updateOutput: updateOutputMock, + }); mockShellOutputCallback({ type: 'binary_detected' }); expect(updateOutputMock).toHaveBeenCalledOnce(); @@ -653,7 +658,10 @@ describe('ShellTool', () => { command: 'sleep 10', is_background: true, }); - const promise = invocation.execute(mockAbortSignal, updateOutputMock); + const promise = invocation.execute({ + abortSignal: mockAbortSignal, + updateOutput: updateOutputMock, + }); mockShellOutputCallback({ type: 'data', chunk: 'some output' }); expect(updateOutputMock).not.toHaveBeenCalled(); @@ -865,7 +873,7 @@ describe('ShellTool', () => { it('should not include Command in output', async () => { const invocation = shellTool.build({ command: 'echo hello' }); - const promise = invocation.execute(mockAbortSignal); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); resolveShellExecution({ output: 'hello', exitCode: 0 }); const result = await promise; @@ -874,7 +882,7 @@ describe('ShellTool', () => { it('should not include Directory in output', async () => { const invocation = shellTool.build({ command: 'ls', dir_path: 'subdir' }); - const promise = invocation.execute(mockAbortSignal); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); resolveShellExecution({ output: 'file.txt', exitCode: 0 }); const result = await promise; @@ -883,7 +891,7 @@ describe('ShellTool', () => { it('should not include Exit Code when command succeeds (exit code 0)', async () => { const invocation = shellTool.build({ command: 'echo hello' }); - const promise = invocation.execute(mockAbortSignal); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); resolveShellExecution({ output: 'hello', exitCode: 0 }); const result = await promise; @@ -892,7 +900,7 @@ describe('ShellTool', () => { it('should include Exit Code when command fails (non-zero exit code)', async () => { const invocation = shellTool.build({ command: 'false' }); - const promise = invocation.execute(mockAbortSignal); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); resolveShellExecution({ output: '', exitCode: 1 }); const result = await promise; @@ -901,7 +909,7 @@ describe('ShellTool', () => { it('should not include Error when there is no process error', async () => { const invocation = shellTool.build({ command: 'echo hello' }); - const promise = invocation.execute(mockAbortSignal); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); resolveShellExecution({ output: 'hello', exitCode: 0, error: null }); const result = await promise; @@ -910,7 +918,7 @@ describe('ShellTool', () => { it('should include Error when there is a process error', async () => { const invocation = shellTool.build({ command: 'bad-command' }); - const promise = invocation.execute(mockAbortSignal); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); resolveShellExecution({ output: '', exitCode: 1, @@ -923,7 +931,7 @@ describe('ShellTool', () => { it('should not include Signal when there is no signal', async () => { const invocation = shellTool.build({ command: 'echo hello' }); - const promise = invocation.execute(mockAbortSignal); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); resolveShellExecution({ output: 'hello', exitCode: 0, signal: null }); const result = await promise; @@ -932,7 +940,7 @@ describe('ShellTool', () => { it('should include Signal when process was killed by signal', async () => { const invocation = shellTool.build({ command: 'sleep 100' }); - const promise = invocation.execute(mockAbortSignal); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); resolveShellExecution({ output: '', exitCode: null, @@ -945,7 +953,7 @@ describe('ShellTool', () => { it('should not include Background PIDs when there are none', async () => { const invocation = shellTool.build({ command: 'echo hello' }); - const promise = invocation.execute(mockAbortSignal); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); resolveShellExecution({ output: 'hello', exitCode: 0 }); const result = await promise; @@ -954,7 +962,7 @@ describe('ShellTool', () => { it('should not include Process Group PGID when pid is not set', async () => { const invocation = shellTool.build({ command: 'echo hello' }); - const promise = invocation.execute(mockAbortSignal); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); resolveShellExecution({ output: 'hello', exitCode: 0, pid: undefined }); const result = await promise; @@ -963,7 +971,7 @@ describe('ShellTool', () => { it('should have minimal output for successful command', async () => { const invocation = shellTool.build({ command: 'echo hello' }); - const promise = invocation.execute(mockAbortSignal); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); resolveShellExecution({ output: 'hello', exitCode: 0, pid: undefined }); const result = await promise; @@ -1051,7 +1059,7 @@ describe('ShellTool', () => { mockSandboxManager = sandboxManager; const invocation = shellTool.build({ command: 'npm install' }); - const promise = invocation.execute(mockAbortSignal); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); resolveExecutionPromise({ exitCode: 1, @@ -1114,7 +1122,7 @@ describe('ShellTool', () => { mockSandboxManager = sandboxManager; const invocation = shellTool.build({ command: `ls ${homeDir}` }); - const promise = invocation.execute(mockAbortSignal); + const promise = invocation.execute({ abortSignal: mockAbortSignal }); resolveExecutionPromise({ exitCode: 1, diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index acbd5e72ff..e299d88e4c 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -26,7 +26,6 @@ import { type ToolCallConfirmationDetails, type ToolExecuteConfirmationDetails, type PolicyUpdateOptions, - type ToolLiveOutput, type ExecuteOptions, type ForcedToolDecision, } from './tools.js'; @@ -434,12 +433,13 @@ export class ShellToolInvocation extends BaseToolInvocation< return confirmationDetails; } - async execute( - signal: AbortSignal, - updateOutput?: (output: ToolLiveOutput) => void, - options?: ExecuteOptions, - ): Promise { - const { shellExecutionConfig, setExecutionIdCallback } = options ?? {}; + async execute(options: ExecuteOptions): Promise { + const { + abortSignal: signal, + updateOutput, + shellExecutionConfig, + setExecutionIdCallback, + } = options; const strippedCommand = stripShellWrapper(this.params.command); if (signal.aborted) { diff --git a/packages/core/src/tools/shellBackgroundTools.integration.test.ts b/packages/core/src/tools/shellBackgroundTools.integration.test.ts index 7cf41d1a01..ab96df7383 100644 --- a/packages/core/src/tools/shellBackgroundTools.integration.test.ts +++ b/packages/core/src/tools/shellBackgroundTools.integration.test.ts @@ -92,9 +92,9 @@ describe('Background Tools Integration', () => { (listInvocation as any).context = { config: { getSessionId: () => 'default' }, }; - const listResult = await listInvocation.execute( - new AbortController().signal, - ); + const listResult = await listInvocation.execute({ + abortSignal: new AbortController().signal, + }); expect(listResult.llmContent).toContain( `[PID ${pid}] RUNNING: \`node continuous_log\``, @@ -109,9 +109,9 @@ describe('Background Tools Integration', () => { (readInvocation as any).context = { config: { getSessionId: () => 'default' }, }; - const readResult = await readInvocation.execute( - new AbortController().signal, - ); + const readResult = await readInvocation.execute({ + abortSignal: new AbortController().signal, + }); expect(readResult.llmContent).toContain('Showing last'); expect(readResult.llmContent).toContain('Log line'); diff --git a/packages/core/src/tools/shellBackgroundTools.test.ts b/packages/core/src/tools/shellBackgroundTools.test.ts index 25af240ede..363b5600dd 100644 --- a/packages/core/src/tools/shellBackgroundTools.test.ts +++ b/packages/core/src/tools/shellBackgroundTools.test.ts @@ -36,7 +36,9 @@ describe('Background Tools', () => { const invocation = listTool.build({}); // eslint-disable-next-line @typescript-eslint/no-explicit-any (invocation as any).context = { config: { getSessionId: () => 'default' } }; - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toBe('No background processes found.'); }); @@ -64,7 +66,9 @@ describe('Background Tools', () => { const invocation = listTool.build({}); // eslint-disable-next-line @typescript-eslint/no-explicit-any (invocation as any).context = { config: { getSessionId: () => 'default' } }; - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toContain( `[PID ${pid}] RUNNING: \`unknown command\``, @@ -89,7 +93,9 @@ describe('Background Tools', () => { const invocation = listTool.build({}); // eslint-disable-next-line @typescript-eslint/no-explicit-any (invocation as any).context = { config: { getSessionId: () => 'default' } }; - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toContain( `- [PID ${pid}] EXITED: \`exited command\` (Exit Code: 1)`, @@ -113,7 +119,9 @@ describe('Background Tools', () => { const invocation = readTool.build({ pid }); // eslint-disable-next-line @typescript-eslint/no-explicit-any (invocation as any).context = { config: { getSessionId: () => 'default' } }; - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.error).toBeDefined(); expect(result.llmContent).toContain('No output log found'); }); @@ -146,7 +154,9 @@ describe('Background Tools', () => { const invocation = readTool.build({ pid, lines: 2 }); // eslint-disable-next-line @typescript-eslint/no-explicit-any (invocation as any).context = { config: { getSessionId: () => 'default' } }; - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toContain('Showing last 2 of 3 lines'); expect(result.llmContent).toContain('line 2\nline 3'); @@ -172,7 +182,9 @@ describe('Background Tools', () => { const invocation = readTool.build({ pid }); // eslint-disable-next-line @typescript-eslint/no-explicit-any (invocation as any).context = { config: { getSessionId: () => 'default' } }; // Asking for PID from another session - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.error).toBeDefined(); expect(result.llmContent).toContain('Access denied'); @@ -201,7 +213,9 @@ describe('Background Tools', () => { const invocation = readTool.build({ pid }); // eslint-disable-next-line @typescript-eslint/no-explicit-any (invocation as any).context = { config: { getSessionId: () => 'default' } }; - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toContain('Log is empty'); @@ -236,7 +250,9 @@ describe('Background Tools', () => { const invocation = readTool.build({ pid }); // eslint-disable-next-line @typescript-eslint/no-explicit-any (invocation as any).context = { config: { getSessionId: () => 'default' } }; - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.error).toBeDefined(); expect(result.llmContent).toContain('Error reading background log'); @@ -272,7 +288,9 @@ describe('Background Tools', () => { const invocation = readTool.build({ pid }); // eslint-disable-next-line @typescript-eslint/no-explicit-any (invocation as any).context = { config: { getSessionId: () => 'default' } }; - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toContain('Access is denied'); expect(result.error?.message).toContain('Symbolic link detected'); @@ -304,7 +322,9 @@ describe('Background Tools', () => { const invocation = readTool.build({ pid, lines: 2 }); // eslint-disable-next-line @typescript-eslint/no-explicit-any (invocation as any).context = { config: { getSessionId: () => 'default' } }; - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toContain('line4\nline5'); expect(result.llmContent).not.toContain('line1'); diff --git a/packages/core/src/tools/shellBackgroundTools.ts b/packages/core/src/tools/shellBackgroundTools.ts index 49cc0a9161..00220b24fc 100644 --- a/packages/core/src/tools/shellBackgroundTools.ts +++ b/packages/core/src/tools/shellBackgroundTools.ts @@ -11,7 +11,9 @@ import { BaseToolInvocation, Kind, type ToolResult, + type ExecuteOptions, } from './tools.js'; + import { ToolErrorType } from './tool-error.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; import type { AgentLoopContext } from '../config/agent-loop-context.js'; @@ -40,7 +42,7 @@ class ListBackgroundProcessesInvocation extends BaseToolInvocation< return 'Lists all active and recently completed background processes for the current session.'; } - async execute(_signal: AbortSignal): Promise { + async execute({ abortSignal: _signal }: ExecuteOptions): Promise { const processes = ShellExecutionService.listBackgroundProcesses( this.context.config.getSessionId(), ); @@ -128,7 +130,7 @@ class ReadBackgroundOutputInvocation extends BaseToolInvocation< return `Reading output for background process ${this.params.pid}`; } - async execute(_signal: AbortSignal): Promise { + async execute({ abortSignal: _signal }: ExecuteOptions): Promise { const pid = this.params.pid; if (this.params.delay_ms && this.params.delay_ms > 0) { diff --git a/packages/core/src/tools/tool-registry.test.ts b/packages/core/src/tools/tool-registry.test.ts index 006bfcd894..0f1e79ca25 100644 --- a/packages/core/src/tools/tool-registry.test.ts +++ b/packages/core/src/tools/tool-registry.test.ts @@ -605,7 +605,9 @@ describe('ToolRegistry', () => { ); const invocation = (discoveredTool as DiscoveredTool).build({}); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.error?.type).toBe( ToolErrorType.DISCOVERED_TOOL_EXECUTION_ERROR, diff --git a/packages/core/src/tools/tool-registry.ts b/packages/core/src/tools/tool-registry.ts index 5b174a97d7..c4c194620f 100644 --- a/packages/core/src/tools/tool-registry.ts +++ b/packages/core/src/tools/tool-registry.ts @@ -12,6 +12,7 @@ import { type AnyDeclarativeTool, type ToolResult, type ToolInvocation, + type ExecuteOptions, } from './tools.js'; import type { Config } from '../config/config.js'; import { ApprovalMode } from '../policy/types.js'; @@ -55,10 +56,10 @@ class DiscoveredToolInvocation extends BaseToolInvocation< return safeJsonStringify(this.params); } - async execute( - _signal: AbortSignal, - _updateOutput?: (output: string) => void, - ): Promise { + async execute({ + abortSignal: _signal, + updateOutput: _updateOutput, + }: ExecuteOptions): Promise { const callCommand = this.config.getToolCallCommand()!; const args = [this.originalToolName]; diff --git a/packages/core/src/tools/tools.ts b/packages/core/src/tools/tools.ts index 165104df30..cd6209079c 100644 --- a/packages/core/src/tools/tools.ts +++ b/packages/core/src/tools/tools.ts @@ -34,6 +34,8 @@ export type ForcedToolDecision = 'allow' | 'deny' | 'ask_user'; * only relevant to specific tool types. */ export interface ExecuteOptions { + abortSignal: AbortSignal; + updateOutput?: (output: ToolLiveOutput) => void; shellExecutionConfig?: ShellExecutionConfig; setExecutionIdCallback?: (executionId: number) => void; } @@ -90,16 +92,10 @@ export interface ToolInvocation< /** * Executes the tool with the validated parameters. - * @param signal AbortSignal for tool cancellation. - * @param updateOutput Optional callback to stream output. - * @param setExecutionIdCallback Optional callback for tools that expose a background execution handle. + * @param options Options for tool execution including signal and output updates. * @returns Result of the tool execution. */ - execute( - signal: AbortSignal, - updateOutput?: (output: ToolLiveOutput) => void, - options?: ExecuteOptions, - ): Promise; + execute(options: ExecuteOptions): Promise; /** * Returns tool-specific options for policy updates. @@ -374,11 +370,7 @@ export abstract class BaseToolInvocation< }); } - abstract execute( - signal: AbortSignal, - updateOutput?: (output: ToolLiveOutput) => void, - options?: ExecuteOptions, - ): Promise; + abstract execute(options: ExecuteOptions): Promise; toJSON() { return { @@ -609,10 +601,14 @@ export abstract class DeclarativeTool< params: TParams, signal: AbortSignal, updateOutput?: (output: ToolLiveOutput) => void, - options?: ExecuteOptions, + options?: Omit, ): Promise { const invocation = this.build(params); - return invocation.execute(signal, updateOutput, options); + return invocation.execute({ + ...options, + abortSignal: signal, + updateOutput, + }); } /** @@ -658,7 +654,7 @@ export abstract class DeclarativeTool< } try { - return await invocationOrError.execute(abortSignal); + return await invocationOrError.execute({ abortSignal }); } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error); diff --git a/packages/core/src/tools/topicTool.test.ts b/packages/core/src/tools/topicTool.test.ts index 25d2730e8c..f8e14e5022 100644 --- a/packages/core/src/tools/topicTool.test.ts +++ b/packages/core/src/tools/topicTool.test.ts @@ -82,7 +82,9 @@ describe('UpdateTopicTool', () => { [TOPIC_PARAM_SUMMARY]: 'The goal is to implement X. Previously we did Y.', [TOPIC_PARAM_STRATEGIC_INTENT]: 'Initial Move', }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toContain('Current topic: "New Chapter"'); expect(result.llmContent).toContain( @@ -105,7 +107,9 @@ describe('UpdateTopicTool', () => { [TOPIC_PARAM_TITLE]: 'New Chapter', [TOPIC_PARAM_STRATEGIC_INTENT]: 'Subsequent Move', }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.returnDisplay).not.toContain('## 📂 Topic:'); expect(result.returnDisplay).toBe( diff --git a/packages/core/src/tools/topicTool.ts b/packages/core/src/tools/topicTool.ts index 91d1b5abc5..2b298159d1 100644 --- a/packages/core/src/tools/topicTool.ts +++ b/packages/core/src/tools/topicTool.ts @@ -16,6 +16,7 @@ import { BaseToolInvocation, Kind, type ToolResult, + type ExecuteOptions, } from './tools.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; import { debugLogger } from '../utils/debugLogger.js'; @@ -50,7 +51,7 @@ class UpdateTopicInvocation extends BaseToolInvocation< return `Update tactical intent: "${intent || '...'}"`; } - async execute(): Promise { + async execute(_options: ExecuteOptions): Promise { const title = this.params[TOPIC_PARAM_TITLE]; const summary = this.params[TOPIC_PARAM_SUMMARY]; const strategicIntent = this.params[TOPIC_PARAM_STRATEGIC_INTENT]; diff --git a/packages/core/src/tools/trackerTools.ts b/packages/core/src/tools/trackerTools.ts index 1594cceca8..1abe9c6881 100644 --- a/packages/core/src/tools/trackerTools.ts +++ b/packages/core/src/tools/trackerTools.ts @@ -23,7 +23,12 @@ import { TRACKER_UPDATE_TASK_TOOL_NAME, TRACKER_VISUALIZE_TOOL_NAME, } from './tool-names.js'; -import type { ToolResult, TodoList, TodoStatus } from './tools.js'; +import type { + ToolResult, + TodoList, + TodoStatus, + ExecuteOptions, +} from './tools.js'; import { BaseDeclarativeTool, BaseToolInvocation, Kind } from './tools.js'; import { ToolErrorType } from './tool-error.js'; import type { TrackerTask, TaskType } from '../services/trackerTypes.js'; @@ -135,7 +140,9 @@ class TrackerCreateTaskInvocation extends BaseToolInvocation< return `Creating task: ${this.params.title}`; } - override async execute(_signal: AbortSignal): Promise { + override async execute({ + abortSignal: _signal, + }: ExecuteOptions): Promise { try { const task = await this.service.createTask({ title: this.params.title, @@ -225,7 +232,9 @@ class TrackerUpdateTaskInvocation extends BaseToolInvocation< return `Updating task ${this.params.id}`; } - override async execute(_signal: AbortSignal): Promise { + override async execute({ + abortSignal: _signal, + }: ExecuteOptions): Promise { const { id, ...updates } = this.params; try { const task = await this.service.updateTask(id, updates); @@ -305,7 +314,9 @@ class TrackerGetTaskInvocation extends BaseToolInvocation< return `Retrieving task ${this.params.id}`; } - override async execute(_signal: AbortSignal): Promise { + override async execute({ + abortSignal: _signal, + }: ExecuteOptions): Promise { const task = await this.service.getTask(this.params.id); if (!task) { return { @@ -379,7 +390,9 @@ class TrackerListTasksInvocation extends BaseToolInvocation< return 'Listing tasks.'; } - override async execute(_signal: AbortSignal): Promise { + override async execute({ + abortSignal: _signal, + }: ExecuteOptions): Promise { let tasks = await this.service.listTasks(); if (this.params.status) { tasks = tasks.filter((t) => t.status === this.params.status); @@ -466,7 +479,9 @@ class TrackerAddDependencyInvocation extends BaseToolInvocation< return `Adding dependency: ${this.params.taskId} depends on ${this.params.dependencyId}`; } - override async execute(_signal: AbortSignal): Promise { + override async execute({ + abortSignal: _signal, + }: ExecuteOptions): Promise { if (this.params.taskId === this.params.dependencyId) { return { llmContent: `Error: Task ${this.params.taskId} cannot depend on itself.`, @@ -576,7 +591,9 @@ class TrackerVisualizeInvocation extends BaseToolInvocation< return 'Visualizing the task graph.'; } - override async execute(_signal: AbortSignal): Promise { + override async execute({ + abortSignal: _signal, + }: ExecuteOptions): Promise { const tasks = await this.service.listTasks(); if (tasks.length === 0) { return { diff --git a/packages/core/src/tools/web-fetch.test.ts b/packages/core/src/tools/web-fetch.test.ts index 457a9e81dc..6d7a05e0a1 100644 --- a/packages/core/src/tools/web-fetch.test.ts +++ b/packages/core/src/tools/web-fetch.test.ts @@ -386,11 +386,13 @@ describe('WebFetchTool', () => { // Execute 10 times to hit the limit for (let i = 0; i < 10; i++) { - await invocation.execute(new AbortController().signal); + await invocation.execute({ abortSignal: new AbortController().signal }); } // The 11th time should fail due to rate limit - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.error?.type).toBe(ToolErrorType.WEB_FETCH_PROCESSING_ERROR); expect(result.error?.message).toContain( 'All requested URLs were skipped', @@ -413,18 +415,20 @@ describe('WebFetchTool', () => { }); await tool .build({ prompt: 'fetch https://ratelimit-multi.com' }) - .execute(new AbortController().signal); + .execute({ abortSignal: new AbortController().signal }); } // 11th call - should be rate limited and not use a mock await tool .build({ prompt: 'fetch https://ratelimit-multi.com' }) - .execute(new AbortController().signal); + .execute({ abortSignal: new AbortController().signal }); mockGenerateContent.mockResolvedValueOnce({ candidates: [{ content: { parts: [{ text: 'healthy response' }] } }], }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toContain('healthy response'); expect(result.llmContent).toContain( '[Warning] The following URLs were skipped:', @@ -450,7 +454,9 @@ describe('WebFetchTool', () => { candidates: [{ content: { parts: [{ text: 'healthy response' }] } }], }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(logWebFetchFallbackAttempt).toHaveBeenCalledTimes(2); expect(logWebFetchFallbackAttempt).toHaveBeenCalledWith( @@ -494,7 +500,9 @@ describe('WebFetchTool', () => { prompt: 'fetch https://url1.com and https://url2.com/', }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toBe('fallback processed response'); expect(result.returnDisplay).toContain( @@ -525,7 +533,9 @@ describe('WebFetchTool', () => { prompt: 'fetch https://public.com/ and https://private.com', }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toBe('fallback response'); // Verify private URL was NOT fetched (mockFetch would throw if it was called for private.com) @@ -538,7 +548,9 @@ describe('WebFetchTool', () => { const tool = new WebFetchTool(mockConfig, bus); const params = { prompt: 'fetch https://public.ip' }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.error?.type).toBe(ToolErrorType.WEB_FETCH_FALLBACK_FAILED); }); @@ -560,7 +572,7 @@ describe('WebFetchTool', () => { const tool = new WebFetchTool(mockConfig, bus); const params = { prompt: 'fetch https://public.ip' }; const invocation = tool.build(params); - await invocation.execute(new AbortController().signal); + await invocation.execute({ abortSignal: new AbortController().signal }); expect(logWebFetchFallbackAttempt).toHaveBeenCalledWith( mockConfig, @@ -628,7 +640,9 @@ describe('WebFetchTool', () => { const tool = new WebFetchTool(mockConfig, bus); const params = { prompt: 'fetch https://example.com' }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); const sanitizeXml = (text: string) => text @@ -934,7 +948,9 @@ describe('WebFetchTool', () => { await confirmationPromise; - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.error).toBeUndefined(); expect(result.llmContent).toContain('Fetched content'); }); @@ -957,7 +973,9 @@ describe('WebFetchTool', () => { const tool = new WebFetchTool(mockConfig, bus); const params = { url: 'https://example.com' }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toBe(content); expect(result.returnDisplay).toContain('Fetched text/plain content'); @@ -984,7 +1002,7 @@ describe('WebFetchTool', () => { const tool = new WebFetchTool(mockConfig, bus); const params = { url: 'https://example.com' }; const invocation = tool.build(params); - await invocation.execute(new AbortController().signal); + await invocation.execute({ abortSignal: new AbortController().signal }); expect(convert).toHaveBeenCalledWith( content, @@ -1016,7 +1034,9 @@ describe('WebFetchTool', () => { const tool = new WebFetchTool(mockConfig, bus); const params = { url: 'https://example.com/image.png' }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toEqual({ inlineData: { @@ -1037,7 +1057,9 @@ describe('WebFetchTool', () => { const tool = new WebFetchTool(mockConfig, bus); const params = { url: 'https://example.com/404' }; const invocation = tool.build(params); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toContain('Request failed with status 404'); expect(result.llmContent).toContain('val'); @@ -1054,7 +1076,9 @@ describe('WebFetchTool', () => { const tool = new WebFetchTool(mockConfig, bus); const invocation = tool.build({ url: 'https://example.com/large' }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toContain('Error'); expect(result.llmContent).toContain('exceeds size limit'); @@ -1079,7 +1103,9 @@ describe('WebFetchTool', () => { const invocation = tool.build({ url: 'https://example.com/large-stream', }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toContain('Error'); expect(result.llmContent).toContain('exceeds size limit'); @@ -1089,7 +1115,9 @@ describe('WebFetchTool', () => { const tool = new WebFetchTool(mockConfig, bus); // Manually bypass build() validation to test executeExperimental safety check const invocation = tool['createInvocation']({}, bus); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toContain('Error: No URL provided.'); expect(result.error?.type).toBe(ToolErrorType.INVALID_TOOL_PARAMS); @@ -1099,7 +1127,9 @@ describe('WebFetchTool', () => { const tool = new WebFetchTool(mockConfig, bus); // Manually bypass build() validation to test executeExperimental safety check const invocation = tool['createInvocation']({ url: 'not-a-url' }, bus); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toContain('Error: Invalid URL "not-a-url"'); expect(result.error?.type).toBe(ToolErrorType.INVALID_TOOL_PARAMS); @@ -1112,7 +1142,9 @@ describe('WebFetchTool', () => { { url: 'http://localhost' }, bus, ); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toContain( 'Error: Access to blocked or private host http://localhost/ is not allowed.', @@ -1131,7 +1163,9 @@ describe('WebFetchTool', () => { const tool = new WebFetchTool(mockConfig, bus); const invocation = tool.build({ url: 'https://example.com/large-text' }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect((result.llmContent as string).length).toBe(300000); // No truncation }); @@ -1147,7 +1181,9 @@ describe('WebFetchTool', () => { const tool = new WebFetchTool(mockConfig, bus); const invocation = tool.build({ url: 'https://example.com/large-text2' }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect((result.llmContent as string).length).toBeLessThan(300000); expect(result.llmContent).toContain( diff --git a/packages/core/src/tools/web-fetch.ts b/packages/core/src/tools/web-fetch.ts index 6c9068fddf..bc801c8c5d 100644 --- a/packages/core/src/tools/web-fetch.ts +++ b/packages/core/src/tools/web-fetch.ts @@ -13,6 +13,7 @@ import { type ToolInvocation, type ToolResult, type PolicyUpdateOptions, + type ExecuteOptions, } from './tools.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; import { ToolErrorType } from './tool-error.js'; @@ -761,7 +762,7 @@ Response: ${rawResponseText}`; } } - async execute(signal: AbortSignal): Promise { + async execute({ abortSignal: signal }: ExecuteOptions): Promise { if (this.context.config.getDirectWebFetch()) { return this.executeExperimental(signal); } diff --git a/packages/core/src/tools/web-search.test.ts b/packages/core/src/tools/web-search.test.ts index a2cdb08594..0fb9401687 100644 --- a/packages/core/src/tools/web-search.test.ts +++ b/packages/core/src/tools/web-search.test.ts @@ -104,7 +104,7 @@ describe('WebSearchTool', () => { }); const invocation = tool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toBe( 'Web search results for "successful query":\n\nHere are your results.', @@ -129,7 +129,7 @@ describe('WebSearchTool', () => { }); const invocation = tool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toBe( 'No search results or information found for query: "no results query"', @@ -143,7 +143,7 @@ describe('WebSearchTool', () => { (mockGeminiClient.generateContent as Mock).mockRejectedValue(testError); const invocation = tool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.error?.type).toBe(ToolErrorType.WEB_SEARCH_FAILED); expect(result.llmContent).toContain('Error:'); @@ -181,7 +181,7 @@ describe('WebSearchTool', () => { }); const invocation = tool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); const expectedLlmContent = `Web search results for "grounding query": @@ -252,7 +252,7 @@ Sources: }); const invocation = tool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); const expectedLlmContent = `Web search results for "multibyte query": diff --git a/packages/core/src/tools/web-search.ts b/packages/core/src/tools/web-search.ts index 2a29291437..58e4e8e559 100644 --- a/packages/core/src/tools/web-search.ts +++ b/packages/core/src/tools/web-search.ts @@ -13,6 +13,7 @@ import { Kind, type ToolInvocation, type ToolResult, + type ExecuteOptions, } from './tools.js'; import { ToolErrorType } from './tool-error.js'; @@ -84,7 +85,9 @@ class WebSearchToolInvocation extends BaseToolInvocation< return `Searching the web for: "${this.params.query}"`; } - async execute(signal: AbortSignal): Promise { + async execute({ + abortSignal: signal, + }: ExecuteOptions): Promise { const geminiClient = this.context.geminiClient; try { diff --git a/packages/core/src/tools/write-file.test.ts b/packages/core/src/tools/write-file.test.ts index aa8ff623ea..0227b18663 100644 --- a/packages/core/src/tools/write-file.test.ts +++ b/packages/core/src/tools/write-file.test.ts @@ -672,7 +672,7 @@ describe('WriteFileTool', () => { const params = { file_path: relativePath, content }; const invocation = tool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toMatch( /Successfully created and wrote to new file/, @@ -693,7 +693,7 @@ describe('WriteFileTool', () => { }); const invocation = tool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Error checking existing file'); expect(result.returnDisplay).toMatch( /Error checking existing file: Simulated read error for execute/, @@ -718,7 +718,7 @@ describe('WriteFileTool', () => { await confirmExecution(invocation); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(mockEnsureCorrectFileContent).toHaveBeenCalledWith( proposedContent, @@ -763,7 +763,7 @@ describe('WriteFileTool', () => { await confirmExecution(invocation); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(mockEnsureCorrectFileContent).toHaveBeenCalledWith( proposedContent, @@ -796,7 +796,7 @@ describe('WriteFileTool', () => { await confirmExecution(invocation); - await invocation.execute(abortSignal); + await invocation.execute({ abortSignal }); expect(fs.existsSync(dirPath)).toBe(true); expect(fs.statSync(dirPath).isDirectory()).toBe(true); @@ -833,7 +833,7 @@ describe('WriteFileTool', () => { ...(modified_by_user !== undefined && { modified_by_user }), }; const invocation = tool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); if (shouldIncludeMessage) { expect(result.llmContent).toMatch(/User modified the `content`/); @@ -851,7 +851,7 @@ describe('WriteFileTool', () => { const params = { file_path: filePath, content }; const invocation = tool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Here is the updated code:'); expect(result.llmContent).toContain(content); @@ -878,7 +878,7 @@ describe('WriteFileTool', () => { await confirmDetails.onConfirm(ToolConfirmationOutcome.ProceedOnce); } - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).toContain('Here is the updated code:'); // Should contain the modified line @@ -999,7 +999,7 @@ describe('WriteFileTool', () => { const params = { file_path: filePath, content }; const invocation = tool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.error?.type).toBe(errorType); const errorSuffix = errorCode ? ` (${errorCode})` : ''; @@ -1089,7 +1089,7 @@ describe('WriteFileTool', () => { const params = { file_path: filePath, content }; const invocation = tool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(discoverJitContext).toHaveBeenCalled(); expect(result.llmContent).toContain('Newly Discovered Project Context'); @@ -1106,7 +1106,7 @@ describe('WriteFileTool', () => { const params = { file_path: filePath, content }; const invocation = tool.build(params); - const result = await invocation.execute(abortSignal); + const result = await invocation.execute({ abortSignal }); expect(result.llmContent).not.toContain( 'Newly Discovered Project Context', diff --git a/packages/core/src/tools/write-file.ts b/packages/core/src/tools/write-file.ts index 1d36909dd4..5766789f0c 100644 --- a/packages/core/src/tools/write-file.ts +++ b/packages/core/src/tools/write-file.ts @@ -24,6 +24,7 @@ import { type ToolResult, type ToolConfirmationOutcome, type PolicyUpdateOptions, + type ExecuteOptions, } from './tools.js'; import { buildFilePathArgsPattern } from '../policy/utils.js'; import { ToolErrorType } from './tool-error.js'; @@ -261,7 +262,9 @@ class WriteFileToolInvocation extends BaseToolInvocation< return confirmationDetails; } - async execute(abortSignal: AbortSignal): Promise { + async execute({ + abortSignal: abortSignal, + }: ExecuteOptions): Promise { const validationError = this.config.validatePathAccess(this.resolvedPath); if (validationError) { return { diff --git a/packages/core/src/tools/write-todos.ts b/packages/core/src/tools/write-todos.ts index 746219ecd7..68cfd52d32 100644 --- a/packages/core/src/tools/write-todos.ts +++ b/packages/core/src/tools/write-todos.ts @@ -11,6 +11,7 @@ import { type ToolInvocation, type Todo, type ToolResult, + type ExecuteOptions, } from './tools.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; import { WRITE_TODOS_TOOL_NAME } from './tool-names.js'; @@ -53,10 +54,7 @@ class WriteTodosToolInvocation extends BaseToolInvocation< return `Set ${count} todo(s)`; } - async execute( - _signal: AbortSignal, - _updateOutput?: (output: string) => void, - ): Promise { + async execute({ abortSignal: _signal }: ExecuteOptions): Promise { const todos = this.params.todos ?? []; const todoListString = todos .map( diff --git a/packages/sdk/src/tool.test.ts b/packages/sdk/src/tool.test.ts index 819177c3b9..d26a4835df 100644 --- a/packages/sdk/src/tool.test.ts +++ b/packages/sdk/src/tool.test.ts @@ -60,7 +60,9 @@ describe('SdkTool Execution', () => { mockMessageBus, undefined, ); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.llmContent).toBe('Success: test'); expect(result.error).toBeUndefined(); @@ -86,7 +88,7 @@ describe('SdkTool Execution', () => { ); await expect( - invocation.execute(new AbortController().signal), + invocation.execute({ abortSignal: new AbortController().signal }), ).rejects.toThrow('Standard error'); }); @@ -108,7 +110,9 @@ describe('SdkTool Execution', () => { mockMessageBus, undefined, ); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.error).toBeDefined(); expect(result.error?.message).toBe('Visible error'); @@ -134,7 +138,9 @@ describe('SdkTool Execution', () => { mockMessageBus, undefined, ); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.error).toBeDefined(); expect(result.error?.message).toBe('Standard error'); diff --git a/packages/sdk/src/tool.ts b/packages/sdk/src/tool.ts index ce6bbfc05b..33bd602795 100644 --- a/packages/sdk/src/tool.ts +++ b/packages/sdk/src/tool.ts @@ -11,6 +11,7 @@ import { BaseToolInvocation, type ToolResult, type ToolInvocation, + type ExecuteOptions, Kind, type MessageBus, } from '@google/gemini-cli-core'; @@ -58,10 +59,10 @@ class SdkToolInvocation extends BaseToolInvocation< return `Executing ${this._toolName}...`; } - async execute( - _signal: AbortSignal, - _updateOutput?: (output: string) => void, - ): Promise { + async execute({ + abortSignal: _abortSignal, + updateOutput: _updateOutput, + }: ExecuteOptions): Promise { try { const result = await this.action(this.params, this.context); const output =