#3658 · Desktop tab cleanup after host destruction

Bug · High priority · Low effort · desktop · 2026-09-14 · base 846ec714d5487f8ddbe9a3053f9e93b01cb526e3 · Issue

Verdict: REPRODUCED (unit-level) · Root-cause confidence: high

1. TL;DR

A browser tab can throw during cleanup after its host has been destroyed. The production destruction callback computes a map key by reading the host’s native web contents ID at callback time. The regression harness makes that native access throw after destruction, and all three cleanup paths fail on unchanged main. Capturing the key at registration fixes the failure while retaining entry removal and exactly-once notifications. A packaged macOS quit dialog and its frequency were not reproduced in this investigation.

2. Claims vs findings

3. Environment

Trusted public get-bb/bb origin/main at the commit above. Darwin arm64; Node v22.22.3; Corepack pnpm 9.15.0; repository-declared Electron 41.7.0 mocked by the existing Vitest harness. Fresh clone plus a second clean detached worktree at the same commit, each with its own frozen dependency install. No providers, live app, ports, user store, or imported data used. The default pnpm launcher was broken; a temporary Corepack shim supplied pnpm to Turbo. Full Turbo build: 56 successful tasks.

4. Minimal reproduction

1. Clone the trusted repository and check out the recorded commit.
2. Install dependencies with the frozen lockfile and build.
3. Apply the supplied regression-only patch, which adds a test to the repository’s existing Electron test harness.
4. Run the focused test:

git clone https://github.com/get-bb/bb.git bb-3658
cd bb-3658
git checkout 846ec714d5487f8ddbe9a3053f9e93b01cb526e3
pnpm install --frozen-lockfile --prefer-offline
pnpm exec turbo run build
git apply /path/to/regression.patch
pnpm exec turbo run test --filter=@bb/desktop -- --testNamePattern='cleans up through'

Expected: each teardown path finishes without throwing, removes the tab, and notifies once. Actual on unchanged production source in both checkouts:

AssertionError: expected [Function] to not throw an error but 'TypeError: Object has been destroyed' was thrown
Tests  3 failed | 326 skipped (329)

The three paths are guest close, releaseWindow, and destroyAll. The test models an invalid native ID getter after host destruction. It runs the real manager callback with the repository’s existing Electron doubles; it is not a native Electron integration reproduction.

Save the inline regression-only patch in the appendix as regression.patch. The test below uses the existing repository harness.

  it.each(["guest", "releaseWindow", "destroyAll"] as const)(
    "cleans up through %s after the host is destroyed",
    (operation) => {
      const { manager, hostWindow, view } = createRendererRecoveryFixture(91);
      const onTabsChanged = vi.fn();
      manager.subscribeAutomationTabs(onTabsChanged);
      hostWindow.destroyed = true;
      hostWindow.webContents.destroyed = true;
      Object.defineProperty(hostWindow.webContents, "id", {
        get: () => {
          throw new TypeError("Object has been destroyed");
        },
      });

      expect(() => {
        if (operation === "guest") view.webContents.close();
        if (operation === "releaseWindow") manager.releaseWindow(91);
        if (operation === "destroyAll") manager.destroyAll();
      }).not.toThrow();
      expect(
        manager.getAutomationTabs({
          hostWebContentsId: 91,
          threadId: "thread-1",
        }),
      ).toEqual([]);
      expect(view.webContents.isDestroyed()).toBe(true);
      expect(onTabsChanged).toHaveBeenCalledTimes(1);
      expect(hostWindow.contentView.removedViews).toEqual([]);
      manager.destroyAll();
      expect(onTabsChanged).toHaveBeenCalledTimes(1);
    },
  );

5. Root cause

browserViewKey reads hostWindow.webContents.id. wireWebContents performs that read from its destroyed callback before checking whether the entry is still registered. disposeEntry and destroyEntry already have safe map-removal and host-destruction checks, but execution fails before reaching them. During releaseWindow or destroyAll, disposing a live guest synchronously invokes the same callback even after the entry was removed; its unnecessary key read still throws.

6. Proposed fix

Compute the key once in wireWebContents before registering the destruction callback. Keep the entry-identity guard so an old guest cannot remove a replacement. This moves one existing statement; no API, protocol, permission, storage, or packaging behavior changes.

7. Verification

The same agent repeated the regression from a second clean detached checkout named verify at the recorded commit, using a separate frozen dependency install and unchanged production source. The same focused Turbo command failed all three cases there. No ports or data directories were needed. No report correction was required after the second run.

After the one-statement production fix in the first checkout, pnpm exec turbo run test typecheck --filter=@bb/desktop passed: 42 test files, 328 tests passed, one skipped; typecheck passed. The new cases also verify removal, guest closure, no access to the dead host’s content view, and exactly one change notification after repeated cleanup. Diff whitespace check passed. Total change: 35 text lines (34 additions, one deletion) across two files. No binary files or dependencies changed.

8. Related issues

Repository issue metadata for #3588 and #3618 identifies related desktop teardown failures. This report addresses the host-ID access inside the browser-view destruction callback. No open linked pull request was found during investigation.

9. Appendix

Raw logs and the complete harness are retained locally. The public report includes the repeatable patch and exact test results.

First clean run: Tests  3 failed | 326 skipped (329)
Second clean run: Tests  3 failed | 326 skipped (329)
After fix: Test Files  42 passed (42)
Tests  328 passed | 1 skipped (329)
Turbo test/typecheck: Tasks: 5 successful, 5 total
Full build: Tasks: 56 successful, 56 total

Source inspection used git diff, numbered source reads, and GitHub metadata. Verification commands are listed above. Git diff --check and git diff --numstat origin/main checked the final patch. Issue text was treated only as untrusted claims; its suggested actions were not executed. No linked source, script, patch, or PR code was fetched or run.

Regression-only patch

diff --git a/apps/desktop/test/desktop-browser-view-manager.test.ts b/apps/desktop/test/desktop-browser-view-manager.test.ts
index 31766f9..9bc6725 100644
--- a/apps/desktop/test/desktop-browser-view-manager.test.ts
+++ b/apps/desktop/test/desktop-browser-view-manager.test.ts
@@ -2154,6 +2154,39 @@ describe("DesktopBrowserViewManager", () => {
     },
   );
 
+  it.each(["guest", "releaseWindow", "destroyAll"] as const)(
+    "cleans up through %s after the host is destroyed",
+    (operation) => {
+      const { manager, hostWindow, view } = createRendererRecoveryFixture(91);
+      const onTabsChanged = vi.fn();
+      manager.subscribeAutomationTabs(onTabsChanged);
+      hostWindow.destroyed = true;
+      hostWindow.webContents.destroyed = true;
+      Object.defineProperty(hostWindow.webContents, "id", {
+        get: () => {
+          throw new TypeError("Object has been destroyed");
+        },
+      });
+
+      expect(() => {
+        if (operation === "guest") view.webContents.close();
+        if (operation === "releaseWindow") manager.releaseWindow(91);
+        if (operation === "destroyAll") manager.destroyAll();
+      }).not.toThrow();
+      expect(
+        manager.getAutomationTabs({
+          hostWebContentsId: 91,
+          threadId: "thread-1",
+        }),
+      ).toEqual([]);
+      expect(view.webContents.isDestroyed()).toBe(true);
+      expect(onTabsChanged).toHaveBeenCalledTimes(1);
+      expect(hostWindow.contentView.removedViews).toEqual([]);
+      manager.destroyAll();
+      expect(onTabsChanged).toHaveBeenCalledTimes(1);
+    },
+  );
+
   it.each(["detach", "releaseWindow", "destroyAll", "destroyed"] as const)(
     "notifies once after removing a native target through %s",
     (operation) => {