[Agent Builder] agent execution - cleanup (elastic#253052)

## Summary

Some cleanup and refactoring around the agent execution's code and APIs

- expose agent registry from server-side plugin start contract (as we
were doing for tools)
- remove `AgentServiceStart.execute` - was a passthrough to
`Runner.runAgent` just introducing another layer of dependencies.
- improve contract mocks
- use own tool prefix for attachment tools
- do not show link to tool definition in reasoning panel for "internal"
tools (filestore, attachments)
- expose execution service from server-side contract
- remove skill registration API from start contract (for now)

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

Author: pgayvallet

x-pack/platform/packages/shared/agent-builder/agent-builder-common/base/namespaces.ts

@@ -13,6 +13,7 @@ export const internalNamespaces = {
   platformCore: 'platform.core',
   platformDashboard: 'platform.dashboard',
   filestore: 'filestore',
+  attachments: 'attachments',
   observability: 'observability',
   security: 'security',
 } as const;
@@ -22,6 +23,7 @@ export const internalNamespaces = {
  */
 export const protectedNamespaces: string[] = [
   internalNamespaces.platformCore,
+  internalNamespaces.attachments,
   internalNamespaces.filestore,
   internalNamespaces.observability,
   internalNamespaces.platformDashboard, // Owned by dashboard_agent plugin

x-pack/platform/packages/shared/agent-builder/agent-builder-common/index.ts

@@ -16,6 +16,7 @@ export {
   type ToolDefinition,
   type ToolDefinitionWithSchema,
   platformCoreTools,
+  attachmentTools,
   filestoreTools,
   defaultAgentToolIds,
   editableToolTypes,

x-pack/platform/packages/shared/agent-builder/agent-builder-common/tools/constants.ts

@@ -30,21 +30,32 @@ export const platformCoreTools = {
   productDocumentation: platformCoreTool('product_documentation'),
   cases: platformCoreTool('cases'),
   integrationKnowledge: platformCoreTool('integration_knowledge'),
-  // Attachment tools
-  attachmentRead: platformCoreTool('attachment_read'),
-  attachmentUpdate: platformCoreTool('attachment_update'),
-  attachmentAdd: platformCoreTool('attachment_add'),
-  attachmentList: platformCoreTool('attachment_list'),
-  attachmentDiff: platformCoreTool('attachment_diff'),
 } as const;
 
+export const attachmentTools = {
+  read: `${internalNamespaces.attachments}.read`,
+  update: `${internalNamespaces.attachments}.update`,
+  add: `${internalNamespaces.attachments}.add`,
+  list: `${internalNamespaces.attachments}.list`,
+  diff: `${internalNamespaces.attachments}.diff`,
+};
+
 export const filestoreTools = {
   read: `${internalNamespaces.filestore}.read`,
   ls: `${internalNamespaces.filestore}.ls`,
   grep: `${internalNamespaces.filestore}.grep`,
   glob: `${internalNamespaces.filestore}.glob`,
 };
 
+export const isAttachmentTool = (toolName: string) =>
+  Object.values(attachmentTools).includes(toolName);
+
+export const isFilestoreTool = (toolName: string) =>
+  Object.values(filestoreTools).includes(toolName);
+
+export const isInternalTool = (toolName: string) =>
+  isAttachmentTool(toolName) || isFilestoreTool(toolName);
+
 /**
  * List of tool types which can be created / edited by a user.
  */

x-pack/platform/packages/shared/agent-builder/agent-builder-common/tools/index.ts

@@ -9,10 +9,14 @@ export { ToolType, type ToolDefinition, type ToolDefinitionWithSchema } from './
 export { isReservedToolId, validateToolId, toolIdRegexp, toolIdMaxLength } from './tool_ids';
 export {
   platformCoreTools,
+  attachmentTools,
   filestoreTools,
   activeToolsCountWarningThreshold,
   defaultAgentToolIds,
   editableToolTypes,
+  isInternalTool,
+  isAttachmentTool,
+  isFilestoreTool,
 } from './constants';
 export {
   type ByIdsToolSelection,

x-pack/platform/plugins/shared/agent_builder/public/application/components/conversations/conversation_rounds/round_thinking/steps/tool_call_display.tsx

@@ -6,9 +6,10 @@
  */
 
 import type { ToolCallStep } from '@kbn/agent-builder-common/chat/conversation';
+import { isInternalTool } from '@kbn/agent-builder-common/tools';
 import type { ReactNode } from 'react';
 import React from 'react';
-import { EuiLink, EuiText } from '@elastic/eui';
+import { EuiLink, EuiText, EuiCode } from '@elastic/eui';
 import { FormattedMessage } from '@kbn/i18n-react';
 import { i18n } from '@kbn/i18n';
 import { useNavigation } from '../../../../../hooks/use_navigation';
@@ -44,7 +45,9 @@ export const ToolCallDisplay: React.FC<ToolCallDisplayProps> = ({ step, icon, te
             id="xpack.agentBuilder.thinking.toolCallThinkingItem"
             defaultMessage="Calling tool {tool}"
             values={{
-              tool: (
+              tool: isInternalTool(toolId) ? (
+                <EuiCode>{toolId}</EuiCode>
+              ) : (
                 <code>
                   <EuiLink
                     href={toolHref}

x-pack/platform/plugins/shared/agent_builder/server/mocks.ts

@@ -7,12 +7,15 @@
 
 import type { AgentBuilderPluginSetup, AgentBuilderPluginStart } from './types';
 import { createMockedExecutableTool, createToolRegistryMock } from './test_utils/tools';
+import { createMockedAgentRegistry } from './test_utils/agents';
 import { createFormatContextMock } from './test_utils/attachments';
 import { createToolHandlerContextMock } from './test_utils/runner';
 
 export type { ToolHandlerContextMock } from './test_utils/runner';
 
-const createSetupContractMock = (): jest.Mocked<AgentBuilderPluginSetup> => {
+export type AgentBuilderPluginSetupMock = jest.MockedObjectDeep<AgentBuilderPluginSetup>;
+
+const createSetupContractMock = (): AgentBuilderPluginSetupMock => {
   return {
     agents: {
       register: jest.fn(),
@@ -32,18 +35,21 @@ const createSetupContractMock = (): jest.Mocked<AgentBuilderPluginSetup> => {
   };
 };
 
-const createStartContractMock = (): jest.Mocked<AgentBuilderPluginStart> => {
+export type AgentBuilderPluginStartMock = jest.MockedObjectDeep<AgentBuilderPluginStart>;
+
+const createStartContractMock = (): AgentBuilderPluginStartMock => {
   return {
     agents: {
       runAgent: jest.fn(),
+      getRegistry: jest.fn().mockImplementation(() => createMockedAgentRegistry()),
     },
     tools: {
       execute: jest.fn(),
       getRegistry: jest.fn().mockImplementation(() => createToolRegistryMock()),
     },
-    skills: {
-      register: jest.fn(),
-      unregister: jest.fn(),
+    execution: {
+      executeAgent: jest.fn(),
+      getExecution: jest.fn(),
     },
   };
 };

x-pack/platform/plugins/shared/agent_builder/server/plugin.ts

@@ -169,23 +169,25 @@ export class AgentBuilderPlugin
       analyticsService: this.analyticsService,
     });
 
-    const { tools, agents, skills, runnerFactory } = startServices;
+    const { tools, agents, runnerFactory, execution } = startServices;
     const runner = runnerFactory.getRunner();
 
     if (this.home) {
       registerSampleData(this.home, this.logger);
     }
+
     return {
       agents: {
-        runAgent: agents.execute.bind(agents),
+        getRegistry: ({ request }) => agents.getRegistry({ request }),
+        runAgent: runner.runAgent.bind(runner),
       },
       tools: {
         getRegistry: ({ request }) => tools.getRegistry({ request }),
         execute: runner.runTool.bind(runner),
       },
-      skills: {
-        register: skills.registerSkill.bind(skills),
-        unregister: skills.unregisterSkill.bind(skills),
+      execution: {
+        executeAgent: execution.executeAgent.bind(execution),
+        getExecution: execution.getExecution.bind(execution),
       },
     };
   }

x-pack/platform/plugins/shared/agent_builder/server/routes/attachments.test.ts

@@ -12,7 +12,7 @@ import type {
   ConversationRound,
   VersionedAttachment,
 } from '@kbn/agent-builder-common';
-import { ConversationRoundStatus } from '@kbn/agent-builder-common';
+import { ConversationRoundStatus, attachmentTools } from '@kbn/agent-builder-common';
 import { ConversationRoundStepType } from '@kbn/agent-builder-common';
 import { ToolResultType } from '@kbn/agent-builder-common/tools/tool_result';
 import { registerAttachmentRoutes } from './attachments';
@@ -571,7 +571,7 @@ describe('Attachment Routes', () => {
           {
             type: ConversationRoundStepType.toolCall,
             tool_call_id: 'tc-1',
-            tool_id: 'platform.core.attachment_read',
+            tool_id: attachmentTools.read,
             params: { attachment_id: 'att-1' },
             results: [
               {

x-pack/platform/plugins/shared/agent_builder/server/routes/attachments.ts

@@ -8,7 +8,7 @@
 import path from 'path';
 import { schema } from '@kbn/config-schema';
 import type { ConversationRound, ToolCallStep } from '@kbn/agent-builder-common';
-import { isToolCallStep } from '@kbn/agent-builder-common';
+import { isToolCallStep, attachmentTools } from '@kbn/agent-builder-common';
 import { createAttachmentStateManager } from '@kbn/agent-builder-server/attachments';
 import { ATTACHMENT_REF_ACTOR } from '@kbn/agent-builder-common/attachments';
 import type { RouteDependencies } from './types';
@@ -32,11 +32,7 @@ function isAttachmentReferencedInRounds(
   attachmentId: string,
   rounds: ConversationRound[]
 ): boolean {
-  const attachmentToolIds = [
-    'platform.core.attachment_read',
-    'platform.core.attachment_update',
-    'platform.core.attachment_diff',
-  ];
+  const attachmentToolIds = [attachmentTools.read, attachmentTools.update, attachmentTools.diff];
 
   for (const round of rounds) {
     for (const step of round.steps) {

x-pack/platform/plugins/shared/agent_builder/server/services/agents/agents_service.test.ts

@@ -33,7 +33,6 @@ const createClientMock = createClient as jest.MockedFunction<typeof createClient
 const runToolRefCleanupMock = runToolRefCleanup as jest.MockedFunction<typeof runToolRefCleanup>;
 
 const createStartDeps = (): AgentsServiceStartDeps => ({
-  getRunner: () => ({ runAgent: jest.fn() } as any),
   security: securityServiceMock.createStart(),
   elasticsearch: elasticsearchServiceMock.createStart(),
   uiSettings: uiSettingsServiceMock.createStartContract(),