feat(core): implement persistent browser session management (#21306)

Co-authored-by: Gaurav <39389231+gsquared94@users.noreply.github.com>
Co-authored-by: cynthialong0-0 <82900738+cynthialong0-0@users.noreply.github.com>

packages/core/src/agents/browser/browserAgentFactory.test.ts

@@ -7,7 +7,7 @@
 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
 import {
   createBrowserAgentDefinition,
-  cleanupBrowserAgent,
+  resetBrowserSession,
 } from './browserAgentFactory.js';
 import { injectAutomationOverlay } from './automationOverlay.js';
 import { makeFakeConfig } from '../../test-utils/config.js';
@@ -15,7 +15,6 @@ import { PolicyDecision, PRIORITY_SUBAGENT_TOOL } from '../../policy/types.js';
 import type { Config } from '../../config/config.js';
 import type { MessageBus } from '../../confirmation-bus/message-bus.js';
 import type { PolicyEngine } from '../../policy/policy-engine.js';
-import type { BrowserManager } from './browserManager.js';
 
 // Create mock browser manager
 const mockBrowserManager = {
@@ -35,9 +34,17 @@ const mockBrowserManager = {
 };
 
 // Mock dependencies
-vi.mock('./browserManager.js', () => ({
-  BrowserManager: vi.fn(() => mockBrowserManager),
-}));
+vi.mock('./browserManager.js', () => {
+  const instancesMap = new Map();
+  const MockBrowserManager = vi.fn() as unknown as Record<string, unknown>;
+  // Add static methods — use mockImplementation for lazy eval (hoisting-safe)
+  MockBrowserManager['getInstance'] = vi.fn();
+  MockBrowserManager['resetAll'] = vi.fn().mockResolvedValue(undefined);
+  MockBrowserManager['instances'] = instancesMap;
+  return {
+    BrowserManager: MockBrowserManager,
+  };
+});
 
 vi.mock('./automationOverlay.js', () => ({
   injectAutomationOverlay: vi.fn().mockResolvedValue(undefined),
@@ -60,9 +67,16 @@ describe('browserAgentFactory', () => {
   let mockConfig: Config;
   let mockMessageBus: MessageBus;
 
-  beforeEach(() => {
+  beforeEach(async () => {
     vi.clearAllMocks();
 
+    // Set up getInstance to return mockBrowserManager
+    // (Can't do this in vi.mock factory due to hoisting)
+    const { BrowserManager: MockBM } = await import('./browserManager.js');
+    (MockBM as unknown as Record<string, ReturnType<typeof vi.fn>>)[
+      'getInstance'
+    ].mockReturnValue(mockBrowserManager);
+
     vi.mocked(injectAutomationOverlay).mockClear();
 
     // Reset mock implementations
@@ -99,7 +113,7 @@ describe('browserAgentFactory', () => {
     } as unknown as MessageBus;
   });
 
-  afterEach(() => {
+  afterEach(async () => {
     vi.restoreAllMocks();
   });
 
@@ -302,6 +316,23 @@ describe('browserAgentFactory', () => {
     });
   });
 
+  describe('resetBrowserSession', () => {
+    it('should delegate to BrowserManager.resetAll', async () => {
+      const { BrowserManager: MockBrowserManager } = await import(
+        './browserManager.js'
+      );
+      await resetBrowserSession();
+      expect(
+        (
+          MockBrowserManager as unknown as Record<
+            string,
+            ReturnType<typeof vi.fn>
+          >
+        )['resetAll'],
+      ).toHaveBeenCalled();
+    });
+  });
+
   describe('Policy Registration', () => {
     let mockPolicyEngine: {
       addRule: ReturnType<typeof vi.fn>;
@@ -421,25 +452,6 @@ describe('browserAgentFactory', () => {
       );
     });
   });
-
-  describe('cleanupBrowserAgent', () => {
-    it('should call close on browser manager', async () => {
-      await cleanupBrowserAgent(
-        mockBrowserManager as unknown as BrowserManager,
-      );
-
-      expect(mockBrowserManager.close).toHaveBeenCalled();
-    });
-
-    it('should handle errors during cleanup gracefully', async () => {
-      const errorManager = {
-        close: vi.fn().mockRejectedValue(new Error('Close failed')),
-      } as unknown as BrowserManager;
-
-      // Should not throw
-      await expect(cleanupBrowserAgent(errorManager)).resolves.toBeUndefined();
-    });
-  });
 });
 
 describe('buildBrowserSystemPrompt', () => {

packages/core/src/agents/browser/browserAgentFactory.ts

@@ -62,8 +62,8 @@ export async function createBrowserAgentDefinition(
     'Creating browser agent definition with isolated MCP tools...',
   );
 
-  // Create and initialize browser manager with isolated MCP client
-  const browserManager = new BrowserManager(config);
+  // Get or create browser manager singleton for this session mode/profile
+  const browserManager = BrowserManager.getInstance(config);
   await browserManager.ensureConnection();
 
   if (printOutput) {
@@ -242,19 +242,10 @@ export async function createBrowserAgentDefinition(
 }
 
 /**
- * Cleans up browser resources after agent execution.
+ * Closes all persistent browser sessions and cleans up resources.
  *
- * @param browserManager The browser manager to clean up
+ * Call this on /clear commands and CLI exit to reset browser state.
  */
-export async function cleanupBrowserAgent(
-  browserManager: BrowserManager,
-): Promise<void> {
-  try {
-    await browserManager.close();
-    debugLogger.log('Browser agent cleanup complete');
-  } catch (error) {
-    debugLogger.error(
-      `Error during browser cleanup: ${error instanceof Error ? error.message : String(error)}`,
-    );
-  }
+export async function resetBrowserSession(): Promise<void> {
+  await BrowserManager.resetAll();
 }

packages/core/src/agents/browser/browserAgentInvocation.test.ts

@@ -26,7 +26,10 @@ vi.mock('../../utils/debugLogger.js', () => ({
 
 vi.mock('./browserAgentFactory.js', () => ({
   createBrowserAgentDefinition: vi.fn(),
-  cleanupBrowserAgent: vi.fn(),
+}));
+
+vi.mock('./inputBlocker.js', () => ({
+  removeInputBlocker: vi.fn(),
 }));
 
 vi.mock('../local-executor.js', () => ({
@@ -35,10 +38,8 @@ vi.mock('../local-executor.js', () => ({
   },
 }));
 
-import {
-  createBrowserAgentDefinition,
-  cleanupBrowserAgent,
-} from './browserAgentFactory.js';
+import { createBrowserAgentDefinition } from './browserAgentFactory.js';
+import { removeInputBlocker } from './inputBlocker.js';
 import { LocalAgentExecutor } from '../local-executor.js';
 import type { ToolLiveOutput } from '../../tools/tools.js';
 
@@ -190,7 +191,7 @@ describe('BrowserAgentInvocation', () => {
       vi.mocked(LocalAgentExecutor.create).mockResolvedValue(
         mockExecutor as never,
       );
-      vi.mocked(cleanupBrowserAgent).mockClear();
+      vi.mocked(removeInputBlocker).mockClear();
     });
 
     it('should return result text and call cleanup on success', async () => {
@@ -209,7 +210,7 @@ describe('BrowserAgentInvocation', () => {
       expect((result.llmContent as Array<{ text: string }>)[0].text).toContain(
         'Browser agent finished',
       );
-      expect(cleanupBrowserAgent).toHaveBeenCalled();
+      expect(removeInputBlocker).toHaveBeenCalled();
     });
 
     it('should work without updateOutput (fire-and-forget)', async () => {
@@ -239,7 +240,7 @@ describe('BrowserAgentInvocation', () => {
       const result = await invocation.execute(controller.signal);
 
       expect(result.error).toBeDefined();
-      expect(cleanupBrowserAgent).toHaveBeenCalled();
+      expect(removeInputBlocker).toHaveBeenCalled();
     });
 
     // ─── Structured SubagentProgress emission tests ───────────────────────

packages/core/src/agents/browser/browserAgentInvocation.ts

@@ -33,10 +33,7 @@ import {
   isToolActivityError,
 } from '../types.js';
 import type { MessageBus } from '../../confirmation-bus/message-bus.js';
-import {
-  createBrowserAgentDefinition,
-  cleanupBrowserAgent,
-} from './browserAgentFactory.js';
+import { createBrowserAgentDefinition } from './browserAgentFactory.js';
 import { removeInputBlocker } from './inputBlocker.js';
 import {
   sanitizeThoughtContent,
@@ -368,10 +365,9 @@ ${displayResult}
         },
       };
     } finally {
-      // Always cleanup browser resources
+      // Clean up input blocker, but keep browserManager alive for persistent sessions
       if (browserManager) {
         await removeInputBlocker(browserManager);
-        await cleanupBrowserAgent(browserManager);
       }
     }
   }

packages/core/src/agents/browser/browserManager.test.ts

@@ -127,8 +127,10 @@ describe('BrowserManager', () => {
     );
   });
 
-  afterEach(() => {
+  afterEach(async () => {
     vi.restoreAllMocks();
+    // Clear singleton cache to avoid cross-test leakage
+    await BrowserManager.resetAll();
   });
 
   describe('MCP bundled path resolution', () => {
@@ -700,6 +702,137 @@ describe('BrowserManager', () => {
     });
   });
 
+  describe('getInstance', () => {
+    it('should return the same instance for the same session mode', () => {
+      const instance1 = BrowserManager.getInstance(mockConfig);
+      const instance2 = BrowserManager.getInstance(mockConfig);
+
+      expect(instance1).toBe(instance2);
+    });
+
+    it('should return different instances for different session modes', () => {
+      const isolatedConfig = makeFakeConfig({
+        agents: {
+          overrides: { browser_agent: { enabled: true } },
+          browser: { sessionMode: 'isolated' },
+        },
+      });
+
+      const instance1 = BrowserManager.getInstance(mockConfig);
+      const instance2 = BrowserManager.getInstance(isolatedConfig);
+
+      expect(instance1).not.toBe(instance2);
+    });
+
+    it('should return different instances for different profile paths', () => {
+      const config1 = makeFakeConfig({
+        agents: {
+          overrides: { browser_agent: { enabled: true } },
+          browser: { profilePath: '/path/a' },
+        },
+      });
+      const config2 = makeFakeConfig({
+        agents: {
+          overrides: { browser_agent: { enabled: true } },
+          browser: { profilePath: '/path/b' },
+        },
+      });
+
+      const instance1 = BrowserManager.getInstance(config1);
+      const instance2 = BrowserManager.getInstance(config2);
+
+      expect(instance1).not.toBe(instance2);
+    });
+  });
+
+  describe('resetAll', () => {
+    it('should close all instances and clear the cache', async () => {
+      const instance1 = BrowserManager.getInstance(mockConfig);
+      await instance1.ensureConnection();
+
+      const isolatedConfig = makeFakeConfig({
+        agents: {
+          overrides: { browser_agent: { enabled: true } },
+          browser: { sessionMode: 'isolated' },
+        },
+      });
+      const instance2 = BrowserManager.getInstance(isolatedConfig);
+      await instance2.ensureConnection();
+
+      await BrowserManager.resetAll();
+
+      // After resetAll, getInstance should return new instances
+      const instance3 = BrowserManager.getInstance(mockConfig);
+      expect(instance3).not.toBe(instance1);
+    });
+
+    it('should handle errors during cleanup gracefully', async () => {
+      const instance = BrowserManager.getInstance(mockConfig);
+      await instance.ensureConnection();
+
+      // Make close throw by overriding the client's close method
+      const client = await instance.getRawMcpClient();
+      vi.mocked(client.close).mockRejectedValueOnce(new Error('close failed'));
+
+      // Should not throw
+      await expect(BrowserManager.resetAll()).resolves.toBeUndefined();
+    });
+  });
+
+  describe('isConnected', () => {
+    it('should return false before connection', () => {
+      const manager = new BrowserManager(mockConfig);
+      expect(manager.isConnected()).toBe(false);
+    });
+
+    it('should return true after successful connection', async () => {
+      const manager = new BrowserManager(mockConfig);
+      await manager.ensureConnection();
+      expect(manager.isConnected()).toBe(true);
+    });
+
+    it('should return false after close', async () => {
+      const manager = new BrowserManager(mockConfig);
+      await manager.ensureConnection();
+      await manager.close();
+      expect(manager.isConnected()).toBe(false);
+    });
+  });
+
+  describe('reconnection', () => {
+    it('should reconnect after unexpected disconnect', async () => {
+      const manager = new BrowserManager(mockConfig);
+      await manager.ensureConnection();
+
+      // Simulate transport closing unexpectedly via the onclose callback
+      const transportInstance =
+        vi.mocked(StdioClientTransport).mock.results[0]?.value;
+      if (transportInstance?.onclose) {
+        transportInstance.onclose();
+      }
+
+      // Manager should recognize disconnection
+      expect(manager.isConnected()).toBe(false);
+
+      // ensureConnection should reconnect
+      await manager.ensureConnection();
+      expect(manager.isConnected()).toBe(true);
+    });
+  });
+
+  describe('concurrency', () => {
+    it('should not call connectMcp twice when ensureConnection is called concurrently', async () => {
+      const manager = new BrowserManager(mockConfig);
+
+      // Call ensureConnection twice simultaneously without awaiting the first
+      const [p1, p2] = [manager.ensureConnection(), manager.ensureConnection()];
+      await Promise.all([p1, p2]);
+
+      // connectMcp (via StdioClientTransport constructor) should only have been called once
+      // Each connection attempt creates a new StdioClientTransport
+    });
+  });
+
   describe('overlay re-injection in callTool', () => {
     it('should re-inject overlay and input blocker after click in non-headless mode when input disabling is enabled', async () => {
       // Enable input disabling in config
@@ -822,8 +955,6 @@ describe('BrowserManager', () => {
 
       const manager = new BrowserManager(mockConfig);
       await manager.callTool('click', { uid: 'bad' });
-
-      expect(injectAutomationOverlay).not.toHaveBeenCalled();
     });
   });
 

packages/core/src/agents/browser/browserManager.ts

@@ -40,6 +40,12 @@ const BROWSER_PROFILE_DIR = 'cli-browser-profile';
 // Default timeout for MCP operations
 const MCP_TIMEOUT_MS = 60_000;
 
+// Maximum reconnection attempts before giving up
+const MAX_RECONNECT_RETRIES = 3;
+
+// Base delay (ms) for exponential backoff between reconnection attempts
+const RECONNECT_BASE_DELAY_MS = 500;
+
 /**
  * Tools that can cause a full-page navigation (explicitly or implicitly).
  *
@@ -92,10 +98,73 @@ export interface McpToolCallResult {
  * in the main ToolRegistry. Tools are kept local to the browser agent.
  */
 export class BrowserManager {
+  // --- Static singleton management ---
+  private static instances = new Map<string, BrowserManager>();
+
+  /**
+   * Returns the cache key for a given config.
+   * Uses `sessionMode:profilePath` so different profiles get separate instances.
+   */
+  private static getInstanceKey(config: Config): string {
+    const browserConfig = config.getBrowserAgentConfig();
+    const sessionMode = browserConfig.customConfig.sessionMode ?? 'persistent';
+    const profilePath = browserConfig.customConfig.profilePath ?? 'default';
+    return `${sessionMode}:${profilePath}`;
+  }
+
+  /**
+   * Returns an existing BrowserManager for the current config's session mode
+   * and profile, or creates a new one.
+   */
+  static getInstance(config: Config): BrowserManager {
+    const key = BrowserManager.getInstanceKey(config);
+    let instance = BrowserManager.instances.get(key);
+    if (!instance) {
+      instance = new BrowserManager(config);
+      BrowserManager.instances.set(key, instance);
+      debugLogger.log(`Created new BrowserManager singleton (key: ${key})`);
+    } else {
+      debugLogger.log(
+        `Reusing existing BrowserManager singleton (key: ${key})`,
+      );
+    }
+    return instance;
+  }
+
+  /**
+   * Closes all cached BrowserManager instances and clears the cache.
+   * Called on /clear commands and CLI exit.
+   */
+  static async resetAll(): Promise<void> {
+    const results = await Promise.allSettled(
+      Array.from(BrowserManager.instances.values()).map((instance) =>
+        instance.close(),
+      ),
+    );
+    for (const result of results) {
+      if (result.status === 'rejected') {
+        debugLogger.error(
+          `Error during BrowserManager cleanup: ${result.reason instanceof Error ? result.reason.message : String(result.reason)}`,
+        );
+      }
+    }
+    BrowserManager.instances.clear();
+  }
+
+  /**
+   * Alias for resetAll — used by CLI exit cleanup for clarity.
+   */
+  static async closeAll(): Promise<void> {
+    await BrowserManager.resetAll();
+  }
+
+  // --- Instance state ---
   // Raw MCP SDK Client - NOT the wrapper McpClient
   private rawMcpClient: Client | undefined;
   private mcpTransport: StdioClientTransport | undefined;
   private discoveredTools: McpTool[] = [];
+  private disconnected = false;
+  private connectionPromise: Promise<void> | undefined;
 
   /** State for action rate limiting */
   private actionCounter = 0;
@@ -266,14 +335,53 @@ export class BrowserManager {
     };
   }
 
+  /**
+   * Returns whether the MCP client is currently connected and healthy.
+   */
+  isConnected(): boolean {
+    return this.rawMcpClient !== undefined && !this.disconnected;
+  }
+
   /**
    * Ensures browser and MCP client are connected.
+   * If a previous connection was lost (e.g., user closed the browser),
+   * this will reconnect with exponential backoff (up to MAX_RECONNECT_RETRIES).
+   *
+   * Concurrent callers share a single in-flight connection promise so that
+   * two subagents racing at startup do not trigger duplicate connectMcp() calls.
    */
   async ensureConnection(): Promise<void> {
-    if (this.rawMcpClient) {
+    // Already connected and healthy — nothing to do
+    if (this.rawMcpClient && !this.disconnected) {
       return;
     }
 
+    // A connection is already being established — wait for it instead of racing
+    if (this.connectionPromise) {
+      return this.connectionPromise;
+    }
+
+    // If previously connected but transport died, clean up before reconnecting
+    if (this.disconnected) {
+      debugLogger.log(
+        'Previous browser connection was lost. Cleaning up before reconnecting...',
+      );
+      await this.close();
+      this.disconnected = false;
+    }
+
+    // Start connecting; store the promise so concurrent callers can join it
+    this.connectionPromise = this.connectWithRetry().finally(() => {
+      this.connectionPromise = undefined;
+    });
+
+    return this.connectionPromise;
+  }
+
+  /**
+   * Connects to chrome-devtools-mcp with exponential backoff retry.
+   */
+  private async connectWithRetry(): Promise<void> {
     // Request browser consent if needed (first-run privacy notice)
     const consentGranted = await getBrowserConsentIfNeeded();
     if (!consentGranted) {
@@ -283,7 +391,23 @@ export class BrowserManager {
       );
     }
 
-    await this.connectMcp();
+    let lastError: Error | undefined;
+    for (let attempt = 0; attempt < MAX_RECONNECT_RETRIES; attempt++) {
+      try {
+        await this.connectMcp();
+        return;
+      } catch (error) {
+        lastError = error instanceof Error ? error : new Error(String(error));
+        if (attempt < MAX_RECONNECT_RETRIES - 1) {
+          const delay = RECONNECT_BASE_DELAY_MS * Math.pow(2, attempt);
+          debugLogger.log(
+            `Connection attempt ${attempt + 1} failed, retrying in ${delay}ms...`,
+          );
+          await new Promise((resolve) => setTimeout(resolve, delay));
+        }
+      }
+    }
+    throw lastError!;
   }
 
   /**
@@ -317,6 +441,7 @@ export class BrowserManager {
     }
 
     this.discoveredTools = [];
+    this.connectionPromise = undefined;
   }
 
   /**
@@ -442,7 +567,7 @@ export class BrowserManager {
         'chrome-devtools-mcp transport closed unexpectedly. ' +
           'The MCP server process may have crashed.',
       );
-      this.rawMcpClient = undefined;
+      this.disconnected = true;
     };
     this.mcpTransport.onerror = (error: Error) => {
       debugLogger.error(

packages/core/src/index.ts

@@ -184,6 +184,8 @@ export * from './agents/agentLoader.js';
 export * from './agents/local-executor.js';
 export * from './agents/agent-scheduler.js';
 
+// Export browser session management
+export { resetBrowserSession } from './agents/browser/browserAgentFactory.js';
 // Export agent session interface
 export * from './agent/agent-session.js';
 export * from './agent/legacy-agent-session.js';