fix(browser): repair chrome mcp cleanup rebase

steipete · steipete · commit bc07969c9312 · 2026-05-24T06:13:24.000+01:00
diff --git a/extensions/browser/src/browser/chrome-mcp.test.ts b/extensions/browser/src/browser/chrome-mcp.test.ts
@@ -169,7 +169,6 @@ describe("chrome MCP page parsing", () => {
       "--no-usage-statistics",
       "--experimentalStructuredContent",
       "--experimental-page-id-routing",
-      "--no-usage-statistics",
       "--userDataDir",
       "/tmp/brave-profile",
     ]);
@@ -189,7 +188,6 @@ describe("chrome MCP page parsing", () => {
       "--no-usage-statistics",
       "--experimentalStructuredContent",
       "--experimental-page-id-routing",
-      "--no-usage-statistics",
     ]);
   });
 
@@ -206,7 +204,6 @@ describe("chrome MCP page parsing", () => {
       "--no-usage-statistics",
       "--experimentalStructuredContent",
       "--experimental-page-id-routing",
-      "--no-usage-statistics",
     ]);
   });
 
@@ -269,7 +266,6 @@ describe("chrome MCP page parsing", () => {
       "--no-usage-statistics",
       "--experimentalStructuredContent",
       "--experimental-page-id-routing",
-      "--no-usage-statistics",
     ]);
   });
 
@@ -307,7 +303,7 @@ describe("chrome MCP page parsing", () => {
     ]);
   });
 
-  it("runs Windows Chrome MCP tree cleanup before closing the stdio client", async () => {
+  it("uses Windows taskkill tree cleanup without waiting for SDK stdio close timeout", async () => {
     const session = createFakeSession();
     Object.assign(session, { ownsProcessTree: true });
     const closeOrder: string[] = [];
@@ -324,7 +320,23 @@ describe("chrome MCP page parsing", () => {
 
     await ensureChromeMcpAvailable("chrome-live", undefined, { ephemeral: true });
 
-    expect(closeOrder).toEqual(["taskkill:123", "client.close"]);
+    expect(closeOrder).toEqual(["taskkill:123"]);
+  });
+
+  it("falls back to SDK stdio close when Windows taskkill cleanup fails", async () => {
+    const session = createFakeSession();
+    Object.assign(session, { ownsProcessTree: true });
+    const closeMock = vi.fn().mockResolvedValue(undefined);
+    session.client.close = closeMock as typeof session.client.close;
+    setChromeMcpProcessCleanupDepsForTest({
+      platform: "win32",
+      taskkillProcessTree: vi.fn().mockRejectedValue(new Error("taskkill failed")),
+    });
+    setChromeMcpSessionFactoryForTest(async () => session);
+
+    await ensureChromeMcpAvailable("chrome-live", undefined, { ephemeral: true });
+
+    expect(closeMock).toHaveBeenCalledTimes(1);
   });
 
   it("redacts remote CDP URL secrets from attach failures", async () => {
diff --git a/extensions/browser/src/browser/chrome-mcp.ts b/extensions/browser/src/browser/chrome-mcp.ts
@@ -552,7 +552,7 @@ async function terminateChromeMcpProcessTree(
 
   const deps = chromeMcpProcessCleanupDepsForTest;
   if ((deps?.platform ?? process.platform) === "win32") {
-    await taskkillChromeMcpProcessTree(rootPid, deps).catch(() => {});
+    await taskkillChromeMcpProcessTree(rootPid, deps);
     return;
   }
 
@@ -599,16 +599,17 @@ async function closeChromeMcpClientAndProcess(params: {
     rootPid && (deps?.platform ?? process.platform) === "win32",
   );
   if (terminateBeforeClientClose) {
-    await terminateChromeMcpProcessTree(rootPid, descendantPids).catch((err) => {
+    try {
+      await terminateChromeMcpProcessTree(rootPid, descendantPids);
+    } catch (err) {
       log.trace(
         `Unable to pre-terminate Chrome MCP subprocess tree for pid ${rootPid}: ${err instanceof Error ? err.message : String(err)}`,
       );
-    });
-  }
-  await params.client.close().catch(() => {});
-  if (terminateBeforeClientClose) {
+      await params.client.close().catch(() => {});
+    }
     return;
   }
+  await params.client.close().catch(() => {});
   await terminateChromeMcpProcessTree(rootPid, descendantPids).catch((err) => {
     log.trace(
       `Unable to fully terminate Chrome MCP subprocess tree for pid ${rootPid}: ${err instanceof Error ? err.message : String(err)}`,
