From 7c4b497a847a4265458df2ebbb119f7b527712bb Mon Sep 17 00:00:00 2001
From: cynthialong0-0 <82900738+cynthialong0-0@users.noreply.github.com>
Date: Tue, 31 Mar 2026 10:43:37 -0700
Subject: [PATCH] fix(core): fix race condition between browser agent and main
 closing process (#24340)

---
 .../src/agents/browser/browserManager.test.ts | 58 ++++++++++++++++++-
 .../core/src/agents/browser/browserManager.ts | 10 +++-
 2 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/packages/core/src/agents/browser/browserManager.test.ts b/packages/core/src/agents/browser/browserManager.test.ts
index 6ccdb2390a..0be3d6da82 100644
--- a/packages/core/src/agents/browser/browserManager.test.ts
+++ b/packages/core/src/agents/browser/browserManager.test.ts
@@ -79,6 +79,7 @@ import * as fs from 'node:fs';
 import { Client } from '@modelcontextprotocol/sdk/client/index.js';
 import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js';
 import { getBrowserConsentIfNeeded } from '../../utils/browserConsent.js';
+import { debugLogger } from '../../utils/debugLogger.js';
 
 describe('BrowserManager', () => {
   let mockConfig: Config;
@@ -125,6 +126,16 @@ describe('BrowserManager', () => {
           }),
         }) as unknown as InstanceType<typeof Client>,
     );
+
+    vi.mocked(StdioClientTransport).mockImplementation(
+      () =>
+        ({
+          close: vi.fn().mockResolvedValue(undefined),
+          stderr: {
+            on: vi.fn(),
+          },
+        }) as unknown as InstanceType<typeof StdioClientTransport>,
+    );
   });
 
   afterEach(async () => {
@@ -682,11 +693,29 @@ describe('BrowserManager', () => {
   describe('close', () => {
     it('should close MCP connections', async () => {
       const manager = new BrowserManager(mockConfig);
-      const client = await manager.getRawMcpClient();
+      await manager.getRawMcpClient();
+
+      await manager.close();
+      expect(manager.isConnected()).toBe(false);
+    });
+
+    it('should NOT log error when transport closes during intentional close()', async () => {
+      const manager = new BrowserManager(mockConfig);
+      await manager.ensureConnection();
+
+      const transportInstance =
+        vi.mocked(StdioClientTransport).mock.results[0]?.value;
+
+      // Trigger onclose during close()
+      vi.spyOn(transportInstance, 'close').mockImplementation(async () => {
+        transportInstance.onclose?.();
+      });
 
       await manager.close();
 
-      expect(client.close).toHaveBeenCalled();
+      expect(debugLogger.error).not.toHaveBeenCalledWith(
+        expect.stringContaining('transport closed unexpectedly'),
+      );
     });
   });
 
@@ -765,6 +794,25 @@ describe('BrowserManager', () => {
       // Should not throw
       await expect(BrowserManager.resetAll()).resolves.toBeUndefined();
     });
+
+    it('should NOT log error when transport closes during resetAll()', async () => {
+      const instance = BrowserManager.getInstance(mockConfig);
+      await instance.ensureConnection();
+
+      const transportInstance =
+        vi.mocked(StdioClientTransport).mock.results[0]?.value;
+
+      // Trigger onclose during close() which is called by resetAll()
+      vi.spyOn(transportInstance, 'close').mockImplementation(async () => {
+        transportInstance.onclose?.();
+      });
+
+      await BrowserManager.resetAll();
+
+      expect(debugLogger.error).not.toHaveBeenCalledWith(
+        expect.stringContaining('transport closed unexpectedly'),
+      );
+    });
   });
 
   describe('isConnected', () => {
@@ -788,7 +836,7 @@ describe('BrowserManager', () => {
   });
 
   describe('reconnection', () => {
-    it('should reconnect after unexpected disconnect', async () => {
+    it('should reconnect after unexpected disconnect and log error', async () => {
       const manager = new BrowserManager(mockConfig);
       await manager.ensureConnection();
 
@@ -799,6 +847,10 @@ describe('BrowserManager', () => {
         transportInstance.onclose();
       }
 
+      expect(debugLogger.error).toHaveBeenCalledWith(
+        expect.stringContaining('transport closed unexpectedly'),
+      );
+
       // Manager should recognize disconnection
       expect(manager.isConnected()).toBe(false);
 
diff --git a/packages/core/src/agents/browser/browserManager.ts b/packages/core/src/agents/browser/browserManager.ts
index d3ff150f20..d95327a40d 100644
--- a/packages/core/src/agents/browser/browserManager.ts
+++ b/packages/core/src/agents/browser/browserManager.ts
@@ -176,6 +176,7 @@ export class BrowserManager {
   private mcpTransport: StdioClientTransport | undefined;
   private discoveredTools: McpTool[] = [];
   private disconnected = false;
+  private isClosing = false;
   private connectionPromise: Promise<void> | undefined;
 
   /** State for action rate limiting */
@@ -360,7 +361,7 @@ export class BrowserManager {
    */
   async ensureConnection(): Promise<void> {
     // Already connected and healthy — nothing to do
-    if (this.rawMcpClient && !this.disconnected) {
+    if (this.isConnected()) {
       return;
     }
 
@@ -424,6 +425,7 @@ export class BrowserManager {
    * the transport will terminate the browser.
    */
   async close(): Promise<void> {
+    this.isClosing = true;
     // Close MCP client first
     if (this.rawMcpClient) {
       try {
@@ -463,6 +465,7 @@ export class BrowserManager {
    * BrowserManager instance.
    */
   private async connectMcp(): Promise<void> {
+    this.isClosing = false;
     debugLogger.log('Connecting isolated MCP client to chrome-devtools-mcp...');
 
     // Create raw MCP SDK Client (not the wrapper McpClient)
@@ -571,11 +574,14 @@ export class BrowserManager {
     }
 
     this.mcpTransport.onclose = () => {
+      this.disconnected = true;
+      if (this.isClosing) {
+        return;
+      }
       debugLogger.error(
         'chrome-devtools-mcp transport closed unexpectedly. ' +
           'The MCP server process may have crashed.',
       );
-      this.disconnected = true;
     };
     this.mcpTransport.onerror = (error: Error) => {
       debugLogger.error(