feat(core): Standardize Tool and Agent Invocation constructors (Phase 2) (#15775)

packages/core/src/agents/delegate-to-agent-tool.test.ts

@@ -100,6 +100,8 @@ describe('DelegateToAgentTool', () => {
       config,
       { arg1: 'valid' },
       messageBus,
+      mockAgentDef.name,
+      mockAgentDef.name,
     );
   });
 

packages/core/src/agents/delegate-to-agent-tool.ts

@@ -127,12 +127,17 @@ export class DelegateToAgentTool extends BaseDeclarativeTool<
 
   protected createInvocation(
     params: DelegateParams,
+    messageBus?: MessageBus,
+    _toolName?: string,
+    _toolDisplayName?: string,
   ): ToolInvocation<DelegateParams, ToolResult> {
     return new DelegateInvocation(
       params,
       this.registry,
       this.config,
-      this.messageBus,
+      messageBus ?? this.messageBus,
+      _toolName,
+      _toolDisplayName,
     );
   }
 }
@@ -146,8 +151,15 @@ class DelegateInvocation extends BaseToolInvocation<
     private readonly registry: AgentRegistry,
     private readonly config: Config,
     messageBus?: MessageBus,
+    _toolName?: string,
+    _toolDisplayName?: string,
   ) {
-    super(params, messageBus, DELEGATE_TO_AGENT_TOOL_NAME);
+    super(
+      params,
+      messageBus,
+      _toolName ?? DELEGATE_TO_AGENT_TOOL_NAME,
+      _toolDisplayName,
+    );
   }
 
   getDescription(): string {

packages/core/src/agents/local-invocation.ts

@@ -44,8 +44,15 @@ export class LocalSubagentInvocation extends BaseToolInvocation<
     private readonly config: Config,
     params: AgentInputs,
     messageBus?: MessageBus,
+    _toolName?: string,
+    _toolDisplayName?: string,
   ) {
-    super(params, messageBus, definition.name, definition.displayName);
+    super(
+      params,
+      messageBus,
+      _toolName ?? definition.name,
+      _toolDisplayName ?? definition.displayName,
+    );
   }
 
   /**

packages/core/src/agents/remote-invocation.ts

@@ -26,8 +26,15 @@ export class RemoteAgentInvocation extends BaseToolInvocation<
     private readonly definition: RemoteAgentDefinition,
     params: AgentInputs,
     messageBus?: MessageBus,
+    _toolName?: string,
+    _toolDisplayName?: string,
   ) {
-    super(params, messageBus, definition.name, definition.displayName);
+    super(
+      params,
+      messageBus,
+      _toolName ?? definition.name,
+      _toolDisplayName ?? definition.displayName,
+    );
   }
 
   getDescription(): string {

packages/core/src/agents/subagent-tool-wrapper.test.ts

@@ -120,6 +120,8 @@ describe('SubagentToolWrapper', () => {
         mockConfig,
         params,
         undefined,
+        mockDefinition.name,
+        mockDefinition.displayName,
       );
     });
 
@@ -139,6 +141,8 @@ describe('SubagentToolWrapper', () => {
         mockConfig,
         params,
         mockMessageBus,
+        mockDefinition.name,
+        mockDefinition.displayName,
       );
     });
 

packages/core/src/agents/subagent-tool-wrapper.ts

@@ -67,17 +67,30 @@ export class SubagentToolWrapper extends BaseDeclarativeTool<
    */
   protected createInvocation(
     params: AgentInputs,
+    messageBus?: MessageBus,
+    _toolName?: string,
+    _toolDisplayName?: string,
   ): ToolInvocation<AgentInputs, ToolResult> {
     const definition = this.definition;
+    const effectiveMessageBus = messageBus ?? this.messageBus;
+
     if (definition.kind === 'remote') {
-      return new RemoteAgentInvocation(definition, params, this.messageBus);
+      return new RemoteAgentInvocation(
+        definition,
+        params,
+        effectiveMessageBus,
+        _toolName,
+        _toolDisplayName,
+      );
     }
 
     return new LocalSubagentInvocation(
       definition,
       this.config,
       params,
-      this.messageBus,
+      effectiveMessageBus,
+      _toolName,
+      _toolDisplayName,
     );
   }
 }

packages/core/src/core/coreToolScheduler.test.ts

@@ -20,6 +20,7 @@ import type {
   Config,
   ToolRegistry,
   AnyToolInvocation,
+  MessageBus,
 } from '../index.js';
 import {
   DEFAULT_TRUNCATE_TOOL_OUTPUT_LINES,
@@ -62,10 +63,17 @@ class TestApprovalTool extends BaseDeclarativeTool<{ id: string }, ToolResult> {
     );
   }
 
-  protected createInvocation(params: {
-    id: string;
-  }): ToolInvocation<{ id: string }, ToolResult> {
-    return new TestApprovalInvocation(this.config, params);
+  protected createInvocation(
+    params: { id: string },
+    messageBus?: MessageBus,
+    _toolName?: string,
+    _toolDisplayName?: string,
+  ): ToolInvocation<{ id: string }, ToolResult> {
+    return new TestApprovalInvocation(
+      this.config,
+      params,
+      messageBus ?? this.messageBus,
+    );
   }
 }
 
@@ -76,8 +84,9 @@ class TestApprovalInvocation extends BaseToolInvocation<
   constructor(
     private config: Config,
     params: { id: string },
+    messageBus?: MessageBus,
   ) {
-    super(params);
+    super(params, messageBus);
   }
 
   getDescription(): string {
@@ -124,8 +133,9 @@ class AbortDuringConfirmationInvocation extends BaseToolInvocation<
     private readonly abortController: AbortController,
     private readonly abortError: Error,
     params: Record<string, unknown>,
+    messageBus?: MessageBus,
   ) {
-    super(params);
+    super(params, messageBus);
   }
 
   override async shouldConfirmExecute(
@@ -166,11 +176,15 @@ class AbortDuringConfirmationTool extends BaseDeclarativeTool<
 
   protected createInvocation(
     params: Record<string, unknown>,
+    messageBus?: MessageBus,
+    _toolName?: string,
+    _toolDisplayName?: string,
   ): ToolInvocation<Record<string, unknown>, ToolResult> {
     return new AbortDuringConfirmationInvocation(
       this.abortController,
       this.abortError,
       params,
+      messageBus ?? this.messageBus,
     );
   }
 }
@@ -727,8 +741,8 @@ class MockEditToolInvocation extends BaseToolInvocation<
   Record<string, unknown>,
   ToolResult
 > {
-  constructor(params: Record<string, unknown>) {
-    super(params);
+  constructor(params: Record<string, unknown>, messageBus?: MessageBus) {
+    super(params, messageBus);
   }
 
   getDescription(): string {
@@ -769,8 +783,11 @@ class MockEditTool extends BaseDeclarativeTool<
 
   protected createInvocation(
     params: Record<string, unknown>,
+    messageBus?: MessageBus,
+    _toolName?: string,
+    _toolDisplayName?: string,
   ): ToolInvocation<Record<string, unknown>, ToolResult> {
-    return new MockEditToolInvocation(params);
+    return new MockEditToolInvocation(params, messageBus ?? this.messageBus);
   }
 }
 

packages/core/src/test-utils/mock-tool.ts

@@ -47,7 +47,7 @@ class MockToolInvocation extends BaseToolInvocation<
   constructor(
     private readonly tool: MockTool,
     params: { [key: string]: unknown },
-    messageBus: MessageBus,
+    messageBus?: MessageBus,
   ) {
     super(params, messageBus, tool.name, tool.displayName);
   }
@@ -122,9 +122,11 @@ export class MockTool extends BaseDeclarativeTool<
 
   protected createInvocation(
     params: { [key: string]: unknown },
-    messageBus: MessageBus,
+    messageBus?: MessageBus,
+    _toolName?: string,
+    _toolDisplayName?: string,
   ): ToolInvocation<{ [key: string]: unknown }, ToolResult> {
-    return new MockToolInvocation(this, params, messageBus);
+    return new MockToolInvocation(this, params, messageBus ?? this.messageBus);
   }
 }
 
@@ -144,7 +146,7 @@ export class MockModifiableToolInvocation extends BaseToolInvocation<
   constructor(
     private readonly tool: MockModifiableTool,
     params: Record<string, unknown>,
-    messageBus: MessageBus,
+    messageBus?: MessageBus,
   ) {
     super(params, messageBus, tool.name, tool.displayName);
   }
@@ -228,8 +230,14 @@ export class MockModifiableTool
 
   protected createInvocation(
     params: Record<string, unknown>,
-    messageBus: MessageBus,
+    messageBus?: MessageBus,
+    _toolName?: string,
+    _toolDisplayName?: string,
   ): ToolInvocation<Record<string, unknown>, ToolResult> {
-    return new MockModifiableToolInvocation(this, params, messageBus);
+    return new MockModifiableToolInvocation(
+      this,
+      params,
+      messageBus ?? this.messageBus,
+    );
   }
 }

packages/core/src/tools/get-internal-docs.ts

@@ -80,8 +80,13 @@ class GetInternalDocsInvocation extends BaseToolInvocation<
   GetInternalDocsParams,
   ToolResult
 > {
-  constructor(params: GetInternalDocsParams, messageBus?: MessageBus) {
-    super(params, messageBus, GET_INTERNAL_DOCS_TOOL_NAME);
+  constructor(
+    params: GetInternalDocsParams,
+    messageBus?: MessageBus,
+    _toolName?: string,
+    _toolDisplayName?: string,
+  ) {
+    super(params, messageBus, _toolName, _toolDisplayName);
   }
 
   override async shouldConfirmExecute(
@@ -181,7 +186,14 @@ export class GetInternalDocsTool extends BaseDeclarativeTool<
   protected createInvocation(
     params: GetInternalDocsParams,
     messageBus?: MessageBus,
+    _toolName?: string,
+    _toolDisplayName?: string,
   ): ToolInvocation<GetInternalDocsParams, ToolResult> {
-    return new GetInternalDocsInvocation(params, messageBus);
+    return new GetInternalDocsInvocation(
+      params,
+      messageBus ?? this.messageBus,
+      _toolName ?? GetInternalDocsTool.Name,
+      _toolDisplayName,
+    );
   }
 }

packages/core/src/tools/glob.ts

@@ -355,7 +355,7 @@ export class GlobTool extends BaseDeclarativeTool<GlobToolParams, ToolResult> {
     return new GlobToolInvocation(
       this.config,
       params,
-      messageBus,
+      messageBus ?? this.messageBus,
       _toolName,
       _toolDisplayName,
     );

packages/core/src/tools/grep.ts

@@ -681,7 +681,7 @@ export class GrepTool extends BaseDeclarativeTool<GrepToolParams, ToolResult> {
     return new GrepToolInvocation(
       this.config,
       params,
-      messageBus,
+      messageBus ?? this.messageBus,
       _toolName,
       _toolDisplayName,
     );

packages/core/src/tools/ls.ts

@@ -337,7 +337,7 @@ export class LSTool extends BaseDeclarativeTool<LSToolParams, ToolResult> {
     return new LSToolInvocation(
       this.config,
       params,
-      messageBus,
+      messageBus ?? this.messageBus,
       _toolName,
       _toolDisplayName,
     );

packages/core/src/tools/mcp-tool.ts

@@ -290,11 +290,11 @@ export class DiscoveredMCPTool extends BaseDeclarativeTool<
       this.mcpTool,
       this.serverName,
       this.serverToolName,
-      this.displayName,
+      _displayName ?? this.displayName,
       this.trust,
       params,
       this.cliConfig,
-      _messageBus,
+      _messageBus ?? this.messageBus,
     );
   }
 }

packages/core/src/tools/message-bus-integration.test.ts

@@ -87,8 +87,18 @@ class TestTool extends BaseDeclarativeTool<TestParams, TestResult> {
     );
   }
 
-  protected createInvocation(params: TestParams, messageBus: MessageBus) {
-    return new TestToolInvocation(params, messageBus);
+  protected createInvocation(
+    params: TestParams,
+    messageBus?: MessageBus,
+    _toolName?: string,
+    _toolDisplayName?: string,
+  ) {
+    return new TestToolInvocation(
+      params,
+      messageBus ?? this.messageBus,
+      _toolName,
+      _toolDisplayName,
+    );
   }
 }
 
@@ -128,7 +138,7 @@ describe('Message Bus Integration', () => {
       expect(publishSpy).toHaveBeenCalledWith({
         type: MessageBusType.TOOL_CONFIRMATION_REQUEST,
         toolCall: {
-          name: 'TestToolInvocation',
+          name: 'test-tool',
           args: { testParam: 'test-value' },
         },
         correlationId: 'test-correlation-id',

packages/core/src/tools/read-file.ts

@@ -232,7 +232,7 @@ export class ReadFileTool extends BaseDeclarativeTool<
     return new ReadFileToolInvocation(
       this.config,
       params,
-      messageBus,
+      messageBus ?? this.messageBus,
       _toolName,
       _toolDisplayName,
     );

packages/core/src/tools/read-many-files.ts

@@ -535,7 +535,7 @@ Use this tool when the user's query implies needing the content of several files
     return new ReadManyFilesToolInvocation(
       this.config,
       params,
-      messageBus,
+      messageBus ?? this.messageBus,
       _toolName,
       _toolDisplayName,
     );

packages/core/src/tools/ripGrep.ts

@@ -594,7 +594,7 @@ export class RipGrepTool extends BaseDeclarativeTool<
       this.config,
       this.geminiIgnoreParser,
       params,
-      messageBus,
+      messageBus ?? this.messageBus,
       _toolName,
       _toolDisplayName,
     );

packages/core/src/tools/shell.ts

@@ -485,7 +485,7 @@ export class ShellTool extends BaseDeclarativeTool<
     return new ShellToolInvocation(
       this.config,
       params,
-      messageBus,
+      messageBus ?? this.messageBus,
       _toolName,
       _toolDisplayName,
     );

packages/core/src/tools/smart-edit.ts

@@ -955,11 +955,12 @@ A good instruction should concisely answer:
 
   protected createInvocation(
     params: EditToolParams,
+    messageBus?: MessageBus,
   ): ToolInvocation<EditToolParams, ToolResult> {
     return new EditToolInvocation(
       this.config,
       params,
-      this.messageBus,
+      messageBus ?? this.messageBus,
       this.name,
       this.displayName,
     );

packages/core/src/tools/tool-registry.ts

@@ -179,9 +179,9 @@ Signal: Signal number or \`(none)\` if no signal was received.
     return new DiscoveredToolInvocation(
       this.config,
       this.originalName,
-      this.name,
+      _toolName ?? this.name,
       params,
-      messageBus,
+      messageBus ?? this.messageBus,
     );
   }
 }

packages/core/src/tools/web-fetch.ts

@@ -459,7 +459,7 @@ export class WebFetchTool extends BaseDeclarativeTool<
     return new WebFetchToolInvocation(
       this.config,
       params,
-      messageBus,
+      messageBus ?? this.messageBus,
       _toolName,
       _toolDisplayName,
     );

packages/core/src/tools/web-search.ts

@@ -238,7 +238,7 @@ export class WebSearchTool extends BaseDeclarativeTool<
     return new WebSearchToolInvocation(
       this.config,
       params,
-      messageBus,
+      messageBus ?? this.messageBus,
       _toolName,
       _toolDisplayName,
     );

packages/core/src/tools/write-todos.ts

@@ -145,7 +145,7 @@ export class WriteTodosTool extends BaseDeclarativeTool<
 > {
   static readonly Name = WRITE_TODOS_TOOL_NAME;
 
-  constructor() {
+  constructor(messageBus?: MessageBus) {
     super(
       WriteTodosTool.Name,
       'WriteTodos',
@@ -180,6 +180,9 @@ export class WriteTodosTool extends BaseDeclarativeTool<
         required: ['todos'],
         additionalProperties: false,
       },
+      true, // isOutputMarkdown
+      false, // canUpdateOutput
+      messageBus,
     );
   }
 
@@ -248,13 +251,13 @@ export class WriteTodosTool extends BaseDeclarativeTool<
 
   protected createInvocation(
     params: WriteTodosToolParams,
-    _messageBus?: MessageBus,
+    messageBus?: MessageBus,
     _toolName?: string,
     _displayName?: string,
   ): ToolInvocation<WriteTodosToolParams, ToolResult> {
     return new WriteTodosToolInvocation(
       params,
-      _messageBus,
+      messageBus ?? this.messageBus,
       _toolName,
       _displayName,
     );