From 0de67c537a1434c271d3fa767cf444497b3137ef Mon Sep 17 00:00:00 2001
From: Qiaochu Hu <110803307+hobostay@users.noreply.github.com>
Date: Wed, 22 Jul 2026 15:43:29 +0800
Subject: [PATCH] fix(extension): roll back CDP attach when a domain enable
 fails
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If `chrome.debugger.attach` succeeds but a subsequent CDP domain enable
rejects — most notably `Page.enable`, when the tab navigates to a
chrome:// URL or the Web Store in the gap between the two calls — the
driver left the tab in an unrecoverable state. `attachedTabs` never
recorded the tabId, so the next `ensureAttached` re-attached and failed
permanently with "Another debugger is already attached" until the
extension was reloaded.

Detach the debuggee directly in the failure path (the public `detach()`
is a no-op here because the id is not cached yet) so the next attempt
starts from a clean slate. Adds a regression test that fails on the
previous code, where the rollback never ran and `detach` was never
called.

Co-Authored-By: Claude <noreply@anthropic.com>
---
 .../__tests__/chromium-cdp.test.ts            | 24 +++++++++++++++++++
 .../src/browser-driver/chromium-cdp.ts        | 24 +++++++++++++++----
 2 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/apps/extension/src/browser-driver/__tests__/chromium-cdp.test.ts b/apps/extension/src/browser-driver/__tests__/chromium-cdp.test.ts
index f6d88d3..728b166 100644
--- a/apps/extension/src/browser-driver/__tests__/chromium-cdp.test.ts
+++ b/apps/extension/src/browser-driver/__tests__/chromium-cdp.test.ts
@@ -67,6 +67,30 @@ describe("ChromiumCdp", () => {
     expect(cdp.isAttached(5)).toBe(false);
   });
 
+  it("rolls back the raw attach when a domain enable fails so the tab is not left stuck", async () => {
+    const { api } = fakeApi();
+    (api.sendCommand as ReturnType<typeof vi.fn>).mockImplementation(
+      async (_target, method: string) => {
+        if (method === "Page.enable") throw new Error("Page.enable rejected");
+        return {};
+      },
+    );
+    const cdp = new ChromiumCdp(api);
+
+    // Raw attach succeeds, but Page.enable fails. Without the rollback
+    // the debugger would stay attached while `attachedTabs` omits the id,
+    // leaving the tab stuck on "Another debugger is already attached" for
+    // every later call until the extension is reloaded.
+    await expect(cdp.ensureAttached(42)).rejects.toThrow(/Page\.enable rejected/);
+    expect(cdp.isAttached(42)).toBe(false);
+    expect(api.detach).toHaveBeenCalledWith({ tabId: 42 });
+
+    // The rollback detached, so a fresh attach can succeed afterwards.
+    (api.sendCommand as ReturnType<typeof vi.fn>).mockResolvedValue({});
+    await expect(cdp.ensureAttached(42)).resolves.toBeUndefined();
+    expect(cdp.isAttached(42)).toBe(true);
+  });
+
   it("send() rejects on chrome.runtime.lastError-style failures", async () => {
     const { api } = fakeApi();
     (api.sendCommand as ReturnType<typeof vi.fn>).mockImplementation(
diff --git a/apps/extension/src/browser-driver/chromium-cdp.ts b/apps/extension/src/browser-driver/chromium-cdp.ts
index 3848e97..429529b 100644
--- a/apps/extension/src/browser-driver/chromium-cdp.ts
+++ b/apps/extension/src/browser-driver/chromium-cdp.ts
@@ -155,10 +155,26 @@ export class ChromiumCdp {
     }
     const attach = (async () => {
       await this.api.attach({ tabId }, CDP_PROTOCOL_VERSION);
-      await this.enablePageDomain(tabId);
-      await this.enableConsoleDomains(tabId);
-      await this.enableNetworkDomainBestEffort(tabId);
-      this.attachedTabs.add(tabId);
+      try {
+        await this.enablePageDomain(tabId);
+        await this.enableConsoleDomains(tabId);
+        await this.enableNetworkDomainBestEffort(tabId);
+        this.attachedTabs.add(tabId);
+      } catch (err) {
+        // A CDP domain enable failed after the raw attach succeeded
+        // (e.g. `Page.enable` rejects because the tab just navigated to
+        // a chrome:// URL or the Web Store). `attachedTabs` does not yet
+        // hold `tabId`, so without rolling back the debugger would stay
+        // attached and the next `ensureAttached` would re-attach and hit
+        // "Another debugger is already attached" forever — the tab is
+        // unusable until the extension is reloaded. Detach directly
+        // (`this.detach()` is a no-op here because `attachedTabs` lacks
+        // the id) so the next attempt starts from a clean slate.
+        await this.api.detach({ tabId }).catch((detachErr) => {
+          console.debug("[bsk cdp] rollback detach failed", { tabId, detachErr });
+        });
+        throw err;
+      }
     })()
       .catch((err) => {
         // Chrome surfaces "Another debugger is already attached" when