feat: introduce UX Extension and Base Folder Strategy

packages/core/src/agents/local-executor.test.ts

@@ -13,43 +13,10 @@ import {
   afterEach,
   type Mock,
 } from 'vitest';
-
-const {
-  mockSendMessageStream,
-  mockScheduleAgentTools,
-  mockSetSystemInstruction,
-  mockCompress,
-  mockMaybeDiscoverMcpServer,
-  mockStopMcp,
-} = vi.hoisted(() => ({
-  mockSendMessageStream: vi.fn().mockResolvedValue({
-    async *[Symbol.asyncIterator]() {
-      yield {
-        type: 'chunk',
-        value: { candidates: [] },
-      };
-    },
-  }),
-  mockScheduleAgentTools: vi.fn(),
-  mockSetSystemInstruction: vi.fn(),
-  mockCompress: vi.fn(),
-  mockMaybeDiscoverMcpServer: vi.fn().mockResolvedValue(undefined),
-  mockStopMcp: vi.fn().mockResolvedValue(undefined),
-}));
-
-vi.mock('../tools/mcp-client-manager.js', () => ({
-  McpClientManager: class {
-    maybeDiscoverMcpServer = mockMaybeDiscoverMcpServer;
-    stop = mockStopMcp;
-  },
-}));
-
 import { debugLogger } from '../utils/debugLogger.js';
 import { LocalAgentExecutor, type ActivityCallback } from './local-executor.js';
 import { makeFakeConfig } from '../test-utils/config.js';
 import { ToolRegistry } from '../tools/tool-registry.js';
-import { PromptRegistry } from '../prompts/prompt-registry.js';
-import { ResourceRegistry } from '../resources/resource-registry.js';
 import { DiscoveredMCPTool } from '../tools/mcp-tool.js';
 import { LSTool } from '../tools/ls.js';
 import { LS_TOOL_NAME, READ_FILE_TOOL_NAME } from '../tools/tool-names.js';
@@ -91,18 +58,9 @@ import {
   type LocalAgentDefinition,
   type SubagentActivityEvent,
   type OutputConfig,
-  SubagentActivityErrorType,
 } from './types.js';
-import {
-  ToolConfirmationOutcome,
-  type AnyDeclarativeTool,
-  type AnyToolInvocation,
-} from '../tools/tools.js';
-import {
-  type ToolCallRequestInfo,
-  CoreToolCallStatus,
-} from '../scheduler/types.js';
-
+import type { AnyDeclarativeTool, AnyToolInvocation } from '../tools/tools.js';
+import type { ToolCallRequestInfo } from '../scheduler/types.js';
 import { CompressionStatus } from '../core/turn.js';
 import { ChatCompressionService } from '../services/chatCompressionService.js';
 import type {
@@ -112,6 +70,18 @@ import type {
 import { getModelConfigAlias, type AgentRegistry } from './registry.js';
 import type { ModelRouterService } from '../routing/modelRouterService.js';
 
+const {
+  mockSendMessageStream,
+  mockScheduleAgentTools,
+  mockSetSystemInstruction,
+  mockCompress,
+} = vi.hoisted(() => ({
+  mockSendMessageStream: vi.fn(),
+  mockScheduleAgentTools: vi.fn(),
+  mockSetSystemInstruction: vi.fn(),
+  mockCompress: vi.fn(),
+}));
+
 let mockChatHistory: Content[] = [];
 const mockSetHistory = vi.fn((newHistory: Content[]) => {
   mockChatHistory = newHistory;
@@ -374,76 +344,6 @@ describe('LocalAgentExecutor', () => {
   });
 
   describe('create (Initialization and Validation)', () => {
-    it('should explicitly map execution context properties to prevent unintended propagation', async () => {
-      const definition = createTestDefinition([LS_TOOL_NAME]);
-      const mockGeminiClient =
-        {} as unknown as import('../core/client.js').GeminiClient;
-      const mockSandboxManager =
-        {} as unknown as import('../services/sandboxManager.js').SandboxManager;
-      const extendedContext = {
-        config: mockConfig,
-        promptId: mockConfig.promptId,
-        toolRegistry: parentToolRegistry,
-        promptRegistry: mockConfig.promptRegistry,
-        resourceRegistry: mockConfig.resourceRegistry,
-        messageBus: mockConfig.messageBus,
-        geminiClient: mockGeminiClient,
-        sandboxManager: mockSandboxManager,
-        unintendedProperty: 'should not be here',
-      } as unknown as import('../config/agent-loop-context.js').AgentLoopContext;
-
-      const executor = await LocalAgentExecutor.create(
-        definition,
-        extendedContext,
-        onActivity,
-      );
-
-      mockModelResponse([
-        {
-          name: TASK_COMPLETE_TOOL_NAME,
-          args: { finalResult: 'done' },
-          id: 'call1',
-        },
-      ]);
-
-      await executor.run({ goal: 'test' }, signal);
-
-      const chatConstructorArgs = MockedGeminiChat.mock.calls[0];
-      const executionContext = chatConstructorArgs[0];
-
-      expect(executionContext).toBeDefined();
-      expect(executionContext.config).toBe(extendedContext.config);
-      expect(executionContext.promptId).toBe(extendedContext.promptId);
-      expect(executionContext.geminiClient).toBe(extendedContext.geminiClient);
-      expect(executionContext.sandboxManager).toBe(
-        extendedContext.sandboxManager,
-      );
-
-      const agentToolRegistry = executor['toolRegistry'];
-      const agentPromptRegistry = executor['promptRegistry'];
-      const agentResourceRegistry = executor['resourceRegistry'];
-
-      expect(executionContext.toolRegistry).toBe(agentToolRegistry);
-      expect(executionContext.promptRegistry).toBe(agentPromptRegistry);
-      expect(executionContext.resourceRegistry).toBe(agentResourceRegistry);
-
-      expect(executionContext.messageBus).toBe(
-        agentToolRegistry.getMessageBus(),
-      );
-
-      // Ensure the unintended property was not spread
-      expect(
-        (executionContext as unknown as { unintendedProperty?: string })
-          .unintendedProperty,
-      ).toBeUndefined();
-
-      // Ensure registries and message bus are not the parent's
-      expect(executionContext.toolRegistry).not.toBe(
-        extendedContext.toolRegistry,
-      );
-      expect(executionContext.messageBus).not.toBe(extendedContext.messageBus);
-    });
-
     it('should create successfully with allowed tools', async () => {
       const definition = createTestDefinition([LS_TOOL_NAME]);
       const executor = await LocalAgentExecutor.create(
@@ -1022,7 +922,6 @@ describe('LocalAgentExecutor', () => {
           data: expect.objectContaining({
             context: 'protocol_violation',
             error: expectedError,
-            errorType: SubagentActivityErrorType.GENERIC,
           }),
         }),
       );
@@ -1068,7 +967,6 @@ describe('LocalAgentExecutor', () => {
             context: 'tool_call',
             name: TASK_COMPLETE_TOOL_NAME,
             error: expectedError,
-            errorType: SubagentActivityErrorType.GENERIC,
           }),
         }),
       );
@@ -1172,7 +1070,7 @@ describe('LocalAgentExecutor', () => {
               if (callsStarted === 2) resolveCalls();
               await vi.advanceTimersByTimeAsync(100);
               return {
-                status: CoreToolCallStatus.Success,
+                status: 'success',
                 request: reqInfo,
                 tool: {} as AnyDeclarativeTool,
                 invocation: {} as AnyToolInvocation,
@@ -1190,7 +1088,7 @@ describe('LocalAgentExecutor', () => {
                   ],
                   error: undefined,
                   errorType: undefined,
-                  contentLength: 0,
+                  contentLength: undefined,
                 },
               };
             }),
@@ -1228,10 +1126,10 @@ describe('LocalAgentExecutor', () => {
       expect(parts).toEqual(
         expect.arrayContaining([
           expect.objectContaining({
-            functionResponse: expect.objectContaining({ name: LS_TOOL_NAME }),
+            functionResponse: expect.objectContaining({ id: 'c1' }),
           }),
           expect.objectContaining({
-            functionResponse: expect.objectContaining({ name: LS_TOOL_NAME }),
+            functionResponse: expect.objectContaining({ id: 'c2' }),
           }),
         ]),
       );
@@ -1302,7 +1200,6 @@ describe('LocalAgentExecutor', () => {
           data: expect.objectContaining({
             context: 'tool_call_unauthorized',
             name: READ_FILE_TOOL_NAME,
-            errorType: SubagentActivityErrorType.GENERIC,
           }),
         }),
       );
@@ -1356,7 +1253,6 @@ describe('LocalAgentExecutor', () => {
             context: 'tool_call',
             name: TASK_COMPLETE_TOOL_NAME,
             error: expect.stringContaining('Output validation failed'),
-            errorType: SubagentActivityErrorType.GENERIC,
           }),
         }),
       );
@@ -1403,7 +1299,6 @@ describe('LocalAgentExecutor', () => {
           type: 'ERROR',
           data: expect.objectContaining({
             error: `Error: Failed to create chat object: ${getErrorMessage(initError)}`,
-            errorType: SubagentActivityErrorType.GENERIC,
           }),
         }),
       );
@@ -1432,7 +1327,7 @@ describe('LocalAgentExecutor', () => {
       ]);
       mockScheduleAgentTools.mockResolvedValueOnce([
         {
-          status: CoreToolCallStatus.Error,
+          status: 'error',
           request: {
             callId: 'call1',
             name: LS_TOOL_NAME,
@@ -1483,7 +1378,6 @@ describe('LocalAgentExecutor', () => {
             context: 'tool_call',
             name: LS_TOOL_NAME,
             error: toolErrorMessage,
-            errorType: SubagentActivityErrorType.GENERIC,
           }),
         }),
       );
@@ -1506,157 +1400,6 @@ describe('LocalAgentExecutor', () => {
       expect(output.terminate_reason).toBe(AgentTerminateMode.GOAL);
       expect(output.result).toBe('Aborted due to tool failure.');
     });
-
-    it('should handle a soft tool rejection (outcome: Cancel) and provide direct instructions to the model', async () => {
-      const definition = createTestDefinition([LS_TOOL_NAME]);
-      const executor = await LocalAgentExecutor.create(
-        definition,
-        mockConfig,
-        onActivity,
-      );
-
-      // Turn 1: Model calls a tool that will be rejected
-      mockModelResponse([
-        { name: LS_TOOL_NAME, args: { path: '/secret' }, id: 'call1' },
-      ]);
-      mockScheduleAgentTools.mockResolvedValueOnce([
-        {
-          status: 'cancelled',
-          request: {
-            callId: 'call1',
-            name: LS_TOOL_NAME,
-            args: { path: '/secret' },
-            isClientInitiated: false,
-            prompt_id: 'test-prompt',
-          },
-          tool: {} as AnyDeclarativeTool,
-          invocation: {} as AnyToolInvocation,
-          outcome: ToolConfirmationOutcome.Cancel, // Soft rejection
-          response: {
-            callId: 'call1',
-            resultDisplay: '',
-            responseParts: [
-              {
-                functionResponse: {
-                  name: LS_TOOL_NAME,
-                  response: {
-                    error:
-                      '[Operation Cancelled] Reason: User denied execution.',
-                  },
-                  id: 'call1',
-                },
-              },
-            ],
-            error: undefined,
-            errorType: undefined,
-            contentLength: 0,
-          },
-        },
-      ]);
-
-      // Turn 2: Model sees the rejection + consolidated instructions and completes
-      mockModelResponse([
-        {
-          name: TASK_COMPLETE_TOOL_NAME,
-          args: { finalResult: 'User rejected access to /secret.' },
-          id: 'call2',
-        },
-      ]);
-
-      const output = await executor.run(
-        { goal: 'Soft rejection test' },
-        signal,
-      );
-
-      // Verify the activity stream reported the consolidated instruction
-      expect(activities).toContainEqual(
-        expect.objectContaining({
-          type: 'ERROR',
-          data: expect.objectContaining({
-            context: 'tool_call',
-            name: LS_TOOL_NAME,
-            error: expect.stringContaining('User rejected this operation'),
-            errorType: SubagentActivityErrorType.REJECTED,
-          }),
-        }),
-      );
-
-      // Verify the instruction was sent back to the model as the tool error
-      const turn2Params = getMockMessageParams(1);
-      const parts = turn2Params.message as Part[];
-      const errorMsg = parts[0].functionResponse?.response?.['error'];
-      expect(typeof errorMsg).toBe('string');
-      if (typeof errorMsg === 'string') {
-        expect(errorMsg).toContain('User rejected this operation');
-        expect(errorMsg).toContain('acknowledge this, rethink your strategy');
-      }
-
-      expect(output.terminate_reason).toBe(AgentTerminateMode.GOAL);
-      expect(output.result).toBe('User rejected access to /secret.');
-    });
-
-    it('should handle a hard tool abort (cancelled with no outcome) and terminate the agent', async () => {
-      const definition = createTestDefinition([LS_TOOL_NAME]);
-      const executor = await LocalAgentExecutor.create(
-        definition,
-        mockConfig,
-        onActivity,
-      );
-
-      // Turn 1: Model calls a tool that will be aborted (e.g. Ctrl+C)
-      mockModelResponse([
-        { name: LS_TOOL_NAME, args: { path: '/secret' }, id: 'call1' },
-      ]);
-      mockScheduleAgentTools.mockResolvedValueOnce([
-        {
-          status: 'cancelled',
-          request: {
-            callId: 'call1',
-            name: LS_TOOL_NAME,
-            args: { path: '/secret' },
-            isClientInitiated: false,
-            prompt_id: 'test-prompt',
-          },
-          tool: {} as AnyDeclarativeTool,
-          invocation: {} as AnyToolInvocation,
-          outcome: undefined, // Hard abort
-          response: {
-            callId: 'call1',
-            resultDisplay: '',
-            responseParts: [
-              {
-                functionResponse: {
-                  name: LS_TOOL_NAME,
-                  response: { error: 'Request cancelled.' },
-                  id: 'call1',
-                },
-              },
-            ],
-            error: undefined,
-            errorType: undefined,
-            contentLength: 0,
-          },
-        },
-      ]);
-
-      const output = await executor.run({ goal: 'Hard abort test' }, signal);
-
-      // Verify the activity stream reported the cancellation
-      expect(activities).toContainEqual(
-        expect.objectContaining({
-          type: 'ERROR',
-          data: expect.objectContaining({
-            context: 'tool_call',
-            name: LS_TOOL_NAME,
-            error: 'Request cancelled.',
-            errorType: SubagentActivityErrorType.CANCELLED,
-          }),
-        }),
-      );
-
-      // Agent should terminate with ABORTED status
-      expect(output.terminate_reason).toBe(AgentTerminateMode.ABORTED);
-    });
   });
 
   describe('Model Routing', () => {
@@ -1851,7 +1594,6 @@ describe('LocalAgentExecutor', () => {
           data: expect.objectContaining({
             context: 'timeout',
             error: 'Agent timed out after 0.5 minutes.',
-            errorType: SubagentActivityErrorType.GENERIC,
           }),
         }),
       );
@@ -2040,7 +1782,6 @@ describe('LocalAgentExecutor', () => {
           data: expect.objectContaining({
             context: 'recovery_turn',
             error: 'Graceful recovery attempt failed. Reason: stop',
-            errorType: SubagentActivityErrorType.GENERIC,
           }),
         }),
       );
@@ -2124,7 +1865,6 @@ describe('LocalAgentExecutor', () => {
           data: expect.objectContaining({
             context: 'recovery_turn',
             error: 'Graceful recovery attempt failed. Reason: stop',
-            errorType: SubagentActivityErrorType.GENERIC,
           }),
         }),
       );
@@ -2246,7 +1986,6 @@ describe('LocalAgentExecutor', () => {
           data: expect.objectContaining({
             context: 'recovery_turn',
             error: 'Graceful recovery attempt failed. Reason: stop',
-            errorType: SubagentActivityErrorType.GENERIC,
           }),
         }),
       );
@@ -2983,67 +2722,6 @@ describe('LocalAgentExecutor', () => {
     });
   });
 
-  describe('MCP Isolation', () => {
-    it('should initialize McpClientManager when mcpServers are defined', async () => {
-      const { MCPServerConfig } = await import('../config/config.js');
-      const mcpServers = {
-        'test-server': new MCPServerConfig('node', ['server.js']),
-      };
-
-      const definition = {
-        ...createTestDefinition(),
-        mcpServers,
-      };
-
-      vi.spyOn(mockConfig, 'getMcpClientManager').mockReturnValue({
-        maybeDiscoverMcpServer: mockMaybeDiscoverMcpServer,
-      } as unknown as ReturnType<typeof mockConfig.getMcpClientManager>);
-
-      await LocalAgentExecutor.create(definition, mockConfig);
-
-      const mcpManager = mockConfig.getMcpClientManager();
-      expect(mcpManager?.maybeDiscoverMcpServer).toHaveBeenCalledWith(
-        'test-server',
-        mcpServers['test-server'],
-        expect.objectContaining({
-          toolRegistry: expect.any(ToolRegistry),
-          promptRegistry: expect.any(PromptRegistry),
-          resourceRegistry: expect.any(ResourceRegistry),
-        }),
-      );
-    });
-
-    it('should inherit main registry tools', async () => {
-      const parentMcpTool = new DiscoveredMCPTool(
-        {} as unknown as CallableTool,
-        'main-server',
-        'tool1',
-        'desc1',
-        {},
-        mockConfig.getMessageBus(),
-      );
-
-      parentToolRegistry.registerTool(parentMcpTool);
-
-      const definition = createTestDefinition();
-      definition.toolConfig = undefined; // trigger inheritance
-
-      vi.spyOn(mockConfig, 'getMcpClientManager').mockReturnValue({
-        maybeDiscoverMcpServer: vi.fn(),
-      } as unknown as ReturnType<typeof mockConfig.getMcpClientManager>);
-      const executor = await LocalAgentExecutor.create(
-        definition,
-        mockConfig,
-        onActivity,
-      );
-      const agentTools = (
-        executor as unknown as { toolRegistry: ToolRegistry }
-      ).toolRegistry.getAllToolNames();
-
-      expect(agentTools).toContain(parentMcpTool.name);
-    });
-  });
-
   describe('DeclarativeTool instance tools (browser agent pattern)', () => {
     /**
      * The browser agent passes DeclarativeTool instances (not string names) in
@@ -3149,11 +2827,13 @@ describe('LocalAgentExecutor', () => {
       const navTool = new MockTool({ name: 'navigate_page' });
 
       const definition = createInstanceToolDefinition([clickTool, navTool]);
+
       const executor = await LocalAgentExecutor.create(
         definition,
         mockConfig,
         onActivity,
       );
+
       const registry = executor['toolRegistry'];
       expect(registry.getTool('click')).toBeDefined();
       expect(registry.getTool('navigate_page')).toBeDefined();
@@ -3373,104 +3053,5 @@ describe('LocalAgentExecutor', () => {
       const uniqueNames = new Set(names);
       expect(uniqueNames.size).toBe(names.length);
     });
-
-    describe('Memory Injection', () => {
-      it('should inject system instruction memory into system prompt', async () => {
-        const definition = createTestDefinition();
-        const executor = await LocalAgentExecutor.create(
-          definition,
-          mockConfig,
-          onActivity,
-        );
-
-        const mockMemory = 'Global memory constraint';
-        vi.spyOn(mockConfig, 'getSystemInstructionMemory').mockReturnValue(
-          mockMemory,
-        );
-
-        mockModelResponse([
-          {
-            name: TASK_COMPLETE_TOOL_NAME,
-            args: { finalResult: 'done' },
-            id: 'call1',
-          },
-        ]);
-
-        await executor.run({ goal: 'test' }, signal);
-
-        const chatConstructorArgs = MockedGeminiChat.mock.calls[0];
-        const systemInstruction = chatConstructorArgs[1] as string;
-
-        expect(systemInstruction).toContain(mockMemory);
-        expect(systemInstruction).toContain('<loaded_context>');
-      });
-
-      it('should inject environment memory into the first message when JIT is disabled', async () => {
-        const definition = createTestDefinition();
-        const executor = await LocalAgentExecutor.create(
-          definition,
-          mockConfig,
-          onActivity,
-        );
-
-        const mockMemory = 'Project memory rule';
-        vi.spyOn(mockConfig, 'getEnvironmentMemory').mockReturnValue(
-          mockMemory,
-        );
-        vi.spyOn(mockConfig, 'isJitContextEnabled').mockReturnValue(false);
-
-        mockModelResponse([
-          {
-            name: TASK_COMPLETE_TOOL_NAME,
-            args: { finalResult: 'done' },
-            id: 'call1',
-          },
-        ]);
-
-        await executor.run({ goal: 'test' }, signal);
-
-        const { message } = getMockMessageParams(0);
-        const parts = message as Part[];
-
-        expect(parts).toBeDefined();
-        const memoryPart = parts.find((p) => p.text?.includes(mockMemory));
-        expect(memoryPart).toBeDefined();
-        expect(memoryPart?.text).toBe(mockMemory);
-      });
-
-      it('should inject session memory into the first message when JIT is enabled', async () => {
-        const definition = createTestDefinition();
-        const executor = await LocalAgentExecutor.create(
-          definition,
-          mockConfig,
-          onActivity,
-        );
-
-        const mockMemory =
-          '<loaded_context>\nExtension memory rule\n</loaded_context>';
-        vi.spyOn(mockConfig, 'getSessionMemory').mockReturnValue(mockMemory);
-        vi.spyOn(mockConfig, 'isJitContextEnabled').mockReturnValue(true);
-
-        mockModelResponse([
-          {
-            name: TASK_COMPLETE_TOOL_NAME,
-            args: { finalResult: 'done' },
-            id: 'call1',
-          },
-        ]);
-
-        await executor.run({ goal: 'test' }, signal);
-
-        const { message } = getMockMessageParams(0);
-        const parts = message as Part[];
-
-        expect(parts).toBeDefined();
-        const memoryPart = parts.find((p) =>
-          p.text?.includes('Extension memory rule'),
-        );
-        expect(memoryPart).toBeDefined();
-        expect(memoryPart?.text).toContain(mockMemory);
-      });
-    });
   });
 });