Merge branch 'main' into akkr/subagents

packages/core/src/agents/a2aUtils.test.ts

@@ -10,6 +10,7 @@ import {
   extractIdsFromResponse,
   isTerminalState,
   A2AResultReassembler,
+  AUTH_REQUIRED_MSG,
 } from './a2aUtils.js';
 import type { SendMessageResult } from './a2a-client-manager.js';
 import type {
@@ -285,6 +286,66 @@ describe('a2aUtils', () => {
       );
     });
 
+    it('should handle auth-required state with a message', () => {
+      const reassembler = new A2AResultReassembler();
+
+      reassembler.update({
+        kind: 'status-update',
+        status: {
+          state: 'auth-required',
+          message: {
+            kind: 'message',
+            role: 'agent',
+            parts: [{ kind: 'text', text: 'I need your permission.' }],
+          } as Message,
+        },
+      } as unknown as SendMessageResult);
+
+      expect(reassembler.toString()).toContain('I need your permission.');
+      expect(reassembler.toString()).toContain(AUTH_REQUIRED_MSG);
+    });
+
+    it('should handle auth-required state without relying on metadata', () => {
+      const reassembler = new A2AResultReassembler();
+
+      reassembler.update({
+        kind: 'status-update',
+        status: {
+          state: 'auth-required',
+        },
+      } as unknown as SendMessageResult);
+
+      expect(reassembler.toString()).toContain(AUTH_REQUIRED_MSG);
+    });
+
+    it('should not duplicate the auth instruction OR agent message if multiple identical auth-required chunks arrive', () => {
+      const reassembler = new A2AResultReassembler();
+
+      const chunk = {
+        kind: 'status-update',
+        status: {
+          state: 'auth-required',
+          message: {
+            kind: 'message',
+            role: 'agent',
+            parts: [{ kind: 'text', text: 'You need to login here.' }],
+          } as Message,
+        },
+      } as unknown as SendMessageResult;
+
+      reassembler.update(chunk);
+      // Simulate multiple updates with the same overall state
+      reassembler.update(chunk);
+      reassembler.update(chunk);
+
+      const output = reassembler.toString();
+      // The substring should only appear exactly once
+      expect(output.split(AUTH_REQUIRED_MSG).length - 1).toBe(1);
+
+      // Crucially, the agent's actual custom message should ALSO only appear exactly once
+      expect(output.split('You need to login here.').length - 1).toBe(1);
+    });
+
     it('should fallback to history in a task chunk if no message or artifacts exist and task is terminal', () => {
       const reassembler = new A2AResultReassembler();
 

packages/core/src/agents/a2aUtils.ts

@@ -16,6 +16,8 @@ import type {
 } from '@a2a-js/sdk';
 import type { SendMessageResult } from './a2a-client-manager.js';
 
+export const AUTH_REQUIRED_MSG = `[Authorization Required] The agent has indicated it requires authorization to proceed. Please follow the agent's instructions.`;
+
 /**
  * Reassembles incremental A2A streaming updates into a coherent result.
  * Shows sequential status/messages followed by all reassembled artifacts.
@@ -33,6 +35,7 @@ export class A2AResultReassembler {
 
     switch (chunk.kind) {
       case 'status-update':
+        this.appendStateInstructions(chunk.status?.state);
         this.pushMessage(chunk.status?.message);
         break;
 
@@ -65,6 +68,7 @@ export class A2AResultReassembler {
         break;
 
       case 'task':
+        this.appendStateInstructions(chunk.status?.state);
         this.pushMessage(chunk.status?.message);
         if (chunk.artifacts) {
           for (const art of chunk.artifacts) {
@@ -106,6 +110,17 @@ export class A2AResultReassembler {
     }
   }
 
+  private appendStateInstructions(state: TaskState | undefined) {
+    if (state !== 'auth-required') {
+      return;
+    }
+
+    // Prevent duplicate instructions if multiple chunks report auth-required
+    if (!this.messageLog.includes(AUTH_REQUIRED_MSG)) {
+      this.messageLog.push(AUTH_REQUIRED_MSG);
+    }
+  }
+
   private pushMessage(message: Message | undefined) {
     if (!message) return;
     const text = extractPartsText(message.parts, '\n');

packages/core/src/code_assist/server.ts

@@ -508,6 +508,16 @@ export class CodeAssistServer implements ContentGenerator {
 }
 
 interface VpcScErrorResponse {
+  response?: {
+    data?: {
+      error?: {
+        details?: unknown[];
+      };
+    };
+  };
+}
+
+function isVpcScErrorResponse(error: unknown): error is VpcScErrorResponse & {
   response: {
     data: {
       error: {
@@ -515,9 +525,7 @@ interface VpcScErrorResponse {
       };
     };
   };
-}
-
-function isVpcScErrorResponse(error: unknown): error is VpcScErrorResponse {
+} {
   return (
     !!error &&
     typeof error === 'object' &&

packages/core/src/config/config.test.ts

@@ -2950,9 +2950,11 @@ describe('Plans Directory Initialization', () => {
 
   afterEach(() => {
     vi.mocked(fs.promises.mkdir).mockRestore();
+    vi.mocked(fs.promises.access).mockRestore?.();
   });
 
-  it('should create plans directory and add it to workspace context when plan is enabled', async () => {
+  it('should add plans directory to workspace context if it exists', async () => {
+    vi.spyOn(fs.promises, 'access').mockResolvedValue(undefined);
     const config = new Config({
       ...baseParams,
       plan: true,
@@ -2961,14 +2963,32 @@ describe('Plans Directory Initialization', () => {
     await config.initialize();
 
     const plansDir = config.storage.getPlansDir();
-    expect(fs.promises.mkdir).toHaveBeenCalledWith(plansDir, {
-      recursive: true,
-    });
+    // Should NOT create the directory eagerly
+    expect(fs.promises.mkdir).not.toHaveBeenCalled();
+    // Should check if it exists
+    expect(fs.promises.access).toHaveBeenCalledWith(plansDir);
 
     const context = config.getWorkspaceContext();
     expect(context.getDirectories()).toContain(plansDir);
   });
 
+  it('should NOT add plans directory to workspace context if it does not exist', async () => {
+    vi.spyOn(fs.promises, 'access').mockRejectedValue({ code: 'ENOENT' });
+    const config = new Config({
+      ...baseParams,
+      plan: true,
+    });
+
+    await config.initialize();
+
+    const plansDir = config.storage.getPlansDir();
+    expect(fs.promises.mkdir).not.toHaveBeenCalled();
+    expect(fs.promises.access).toHaveBeenCalledWith(plansDir);
+
+    const context = config.getWorkspaceContext();
+    expect(context.getDirectories()).not.toContain(plansDir);
+  });
+
   it('should NOT create plans directory or add it to workspace context when plan is disabled', async () => {
     const config = new Config({
       ...baseParams,

packages/core/src/config/config.ts

@@ -339,6 +339,15 @@ export interface GeminiCLIExtension {
    * Safety checkers contributed by this extension.
    */
   checkers?: SafetyCheckerRule[];
+  /**
+   * Planning features configuration contributed by this extension.
+   */
+  plan?: {
+    /**
+     * The directory where planning artifacts are stored.
+     */
+    directory?: string;
+  };
 }
 
 export interface ExtensionInstallMetadata {
@@ -1104,8 +1113,15 @@ export class Config implements McpContext {
     // Add plans directory to workspace context for plan file storage
     if (this.planEnabled) {
       const plansDir = this.storage.getPlansDir();
-      await fs.promises.mkdir(plansDir, { recursive: true });
-      this.workspaceContext.addDirectory(plansDir);
+      try {
+        await fs.promises.access(plansDir);
+        this.workspaceContext.addDirectory(plansDir);
+      } catch {
+        // Directory does not exist yet, so we don't add it to the workspace context.
+        // It will be created when the first plan is written. Since custom plan
+        // directories must be within the project root, they are automatically
+        // covered by the project-wide file discovery once created.
+      }
     }
 
     // Initialize centralized FileDiscoveryService

packages/core/src/policy/policies/plan.toml

@@ -72,7 +72,16 @@ priority = 70
 modes = ["plan"]
 
 [[rule]]
-toolName = ["glob", "grep_search", "list_directory", "read_file", "google_web_search", "activate_skill"]
+toolName = [
+  "glob",
+  "grep_search",
+  "list_directory",
+  "read_file",
+  "google_web_search",
+  "activate_skill",
+  "codebase_investigator",
+  "cli_help"
+]
 decision = "allow"
 priority = 70
 modes = ["plan"]

packages/core/src/policy/policy-engine.test.ts

@@ -1593,7 +1593,7 @@ describe('PolicyEngine', () => {
           modes: [ApprovalMode.PLAN],
         },
         {
-          toolName: 'codebase_investigator',
+          toolName: 'unknown_subagent',
           decision: PolicyDecision.ALLOW,
           priority: PRIORITY_SUBAGENT_TOOL,
         },
@@ -1605,7 +1605,7 @@ describe('PolicyEngine', () => {
       });
 
       const fixedResult = await fixedEngine.check(
-        { name: 'codebase_investigator' },
+        { name: 'unknown_subagent' },
         undefined,
       );
 

packages/core/src/policy/toml-loader.test.ts

@@ -909,7 +909,7 @@ priority = 100
       }
     });
 
-    it('should override default subagent rules when in Plan Mode', async () => {
+    it('should override default subagent rules when in Plan Mode for unknown subagents', async () => {
       const planTomlPath = path.resolve(__dirname, 'policies', 'plan.toml');
       const fileContent = await fs.readFile(planTomlPath, 'utf-8');
       const tempPolicyDir = await fs.mkdtemp(
@@ -931,9 +931,9 @@ priority = 100
           approvalMode: ApprovalMode.PLAN,
         });
 
-        // 3. Simulate a Subagent being registered (Dynamic Rule)
+        // 3. Simulate an unknown Subagent being registered (Dynamic Rule)
         engine.addRule({
-          toolName: 'codebase_investigator',
+          toolName: 'unknown_subagent',
           decision: PolicyDecision.ALLOW,
           priority: PRIORITY_SUBAGENT_TOOL,
           source: 'AgentRegistry (Dynamic)',
@@ -942,13 +942,13 @@ priority = 100
         // 4. Verify Behavior:
         // The Plan Mode "Catch-All Deny" (from plan.toml) should override the Subagent Allow
         const checkResult = await engine.check(
-          { name: 'codebase_investigator' },
+          { name: 'unknown_subagent' },
           undefined,
         );
 
         expect(
           checkResult.decision,
-          'Subagent should be DENIED in Plan Mode',
+          'Unknown subagent should be DENIED in Plan Mode',
         ).toBe(PolicyDecision.DENY);
 
         // 5. Verify Explicit Allows still work
@@ -958,6 +958,25 @@ priority = 100
           readResult.decision,
           'Explicitly allowed tools (read_file) should be ALLOWED in Plan Mode',
         ).toBe(PolicyDecision.ALLOW);
+
+        // 6. Verify Built-in Research Subagents are ALLOWED
+        const codebaseResult = await engine.check(
+          { name: 'codebase_investigator' },
+          undefined,
+        );
+        expect(
+          codebaseResult.decision,
+          'codebase_investigator should be ALLOWED in Plan Mode',
+        ).toBe(PolicyDecision.ALLOW);
+
+        const cliHelpResult = await engine.check(
+          { name: 'cli_help' },
+          undefined,
+        );
+        expect(
+          cliHelpResult.decision,
+          'cli_help should be ALLOWED in Plan Mode',
+        ).toBe(PolicyDecision.ALLOW);
       } finally {
         await fs.rm(tempPolicyDir, { recursive: true, force: true });
       }

packages/core/src/services/chatRecordingService.test.ts

@@ -309,23 +309,33 @@ describe('ChatRecordingService', () => {
   });
 
   describe('deleteSession', () => {
-    it('should delete the session file and tool outputs if they exist', () => {
+    it('should delete the session file, tool outputs, session directory, and logs if they exist', () => {
+      const sessionId = 'test-session-id';
       const chatsDir = path.join(testTempDir, 'chats');
+      const logsDir = path.join(testTempDir, 'logs');
+      const toolOutputsDir = path.join(testTempDir, 'tool-outputs');
+      const sessionDir = path.join(testTempDir, sessionId);
+
       fs.mkdirSync(chatsDir, { recursive: true });
-      const sessionFile = path.join(chatsDir, 'test-session-id.json');
+      fs.mkdirSync(logsDir, { recursive: true });
+      fs.mkdirSync(toolOutputsDir, { recursive: true });
+      fs.mkdirSync(sessionDir, { recursive: true });
+
+      const sessionFile = path.join(chatsDir, `${sessionId}.json`);
       fs.writeFileSync(sessionFile, '{}');
 
-      const toolOutputDir = path.join(
-        testTempDir,
-        'tool-outputs',
-        'session-test-session-id',
-      );
+      const logFile = path.join(logsDir, `session-${sessionId}.jsonl`);
+      fs.writeFileSync(logFile, '{}');
+
+      const toolOutputDir = path.join(toolOutputsDir, `session-${sessionId}`);
       fs.mkdirSync(toolOutputDir, { recursive: true });
 
-      chatRecordingService.deleteSession('test-session-id');
+      chatRecordingService.deleteSession(sessionId);
 
       expect(fs.existsSync(sessionFile)).toBe(false);
+      expect(fs.existsSync(logFile)).toBe(false);
       expect(fs.existsSync(toolOutputDir)).toBe(false);
+      expect(fs.existsSync(sessionDir)).toBe(false);
     });
 
     it('should not throw if session file does not exist', () => {

packages/core/src/services/chatRecordingService.ts

@@ -569,6 +569,13 @@ export class ChatRecordingService {
         fs.unlinkSync(sessionPath);
       }
 
+      // Cleanup Activity logs in the project logs directory
+      const logsDir = path.join(tempDir, 'logs');
+      const logPath = path.join(logsDir, `session-${sessionId}.jsonl`);
+      if (fs.existsSync(logPath)) {
+        fs.unlinkSync(logPath);
+      }
+
       // Cleanup tool outputs for this session
       const safeSessionId = sanitizeFilenamePart(sessionId);
       const toolOutputDir = path.join(
@@ -585,6 +592,13 @@ export class ChatRecordingService {
       ) {
         fs.rmSync(toolOutputDir, { recursive: true, force: true });
       }
+
+      // ALSO cleanup the session-specific directory (contains plans, tasks, etc.)
+      const sessionDir = path.join(tempDir, safeSessionId);
+      // Robustness: Ensure the path is strictly within the temp root
+      if (fs.existsSync(sessionDir) && sessionDir.startsWith(tempDir)) {
+        fs.rmSync(sessionDir, { recursive: true, force: true });
+      }
     } catch (error) {
       debugLogger.error('Error deleting session file.', error);
       throw error;