#3588 · Destroyed-window broker cleanup

Bug · Priority: High · Effort: Low · desktop · 2026-09-12 · Issue

Base: 3b37d2790d084a47c96eb78267da5d159598f203

REPRODUCED at the broker boundary · Root-cause confidence: high

1. TL;DR

The broker throws during window cleanup when the native window can no longer expose its web contents. Its lookup rereads that native property even though the caller already supplies a saved numeric ID. Two fresh checkouts reproduced this with a destruction-aware window double, both without and with a control lease. This establishes the code defect, not a live Linux AppImage or Cinnamon dialog reproduction. No application instance or user store was accessed.

2. Claims vs findings

ClaimFindingEvidence
Closing a window can throw from broker cleanup.Verified at unit boundaryBoth new regression cases throw on unchanged main in two checkouts.
Linux release displays an error dialog.UnverifiedThis host is macOS arm64; no Linux AppImage was launched.
Installed plugin or stored data causes the error.Not required for this reproductionThe tests use neither plugins nor stored data. No data-integrity conclusion is inferred.

3. Environment

Darwin arm64; Node 22.22.3; repository-pinned pnpm 9.15.0 through Corepack. Two detached clean worktrees at the recorded base, named /tmp/slopcop-3588-primary and /tmp/slopcop-3588-verify. Frozen installs and full Turbo builds completed in both. No runtime ports, database, provider, or browser profile was used.

4. Minimal reproduction

  1. Check out the recorded main commit in a fresh worktree.
  2. Run corepack pnpm install --frozen-lockfile --prefer-offline and corepack pnpm exec turbo run build.
  3. Replace apps/desktop/test/desktop-browser-broker-snapshots.test.ts with the test file printed below.
  4. Run corepack pnpm exec turbo run test --filter=@bb/desktop -- desktop-browser-broker-snapshots.

The test registers a window, optionally acquires a lease, changes the window double to reject native access after destruction, and releases it by its saved ID. Expected: no exception, an empty registry, one notification, and idempotent second release. Actual:

AssertionError: expected [Function] to not throw an error but 'TypeError: Object has been destroyed' was thrown
Test Files  1 failed (1)
Tests  2 failed | 1 passed (3)
Complete reproduction test
import { describe, expect, it, vi } from "vitest";
import type { Session } from "electron";
import type { DesktopBrowserChanged } from "@bb/host-daemon-contract";

vi.mock("electron", () => ({
  BrowserWindow: class {},
  WebContentsView: class {},
  session: { fromPartition: () => ({}) },
  nativeImage: { createFromBuffer: () => ({}) },
}));

import { createDesktopBrowserBroker } from "../src/desktop-browser-broker.js";
import type {
  DesktopBrowserNativeTab,
  DesktopBrowserViewManager,
} from "../src/desktop-browser-view.js";

const THREAD_ID = "thr_23456789ab";
const PLUGIN_PANEL_ID = "plugin-panel:cloud-sandbox:cloud-machines:pane-1";

function nativeTab(tabId: string, threadId: string): DesktopBrowserNativeTab {
  return {
    tabId,
    threadId,
    url: "https://example.com",
    title: "Example",
    isLoading: false,
    canGoBack: false,
    canGoForward: false,
    errorText: null,
    generation: "tab-generation",
    profile: { kind: "personal" },
    presentation: "reveal",
  };
}

function createFakeWindow() {
  return {
    webContents: {
      id: 7,
      isDestroyed: () => false,
      send: vi.fn(),
    },
    isDestroyed: () => false,
    focus: () => undefined,
    show: () => undefined,
    restore: () => undefined,
    isMinimized: () => false,
    getContentBounds: () => ({ x: 0, y: 0, width: 800, height: 600 }),
    contentView: {
      addChildView: () => undefined,
      removeChildView: () => undefined,
    },
  };
}

describe("desktop browser broker snapshots", () => {
  it("publishes snapshots only for real threads and keeps plugin-panel tabs local", () => {
    let tabs = [
      nativeTab("thread-tab", THREAD_ID),
      nativeTab("panel-tab", PLUGIN_PANEL_ID),
    ];
    let notifyTabsChanged: () => void = () => undefined;
    const manager: Pick<
      DesktopBrowserViewManager,
      "listTabs" | "subscribeAutomationTabs" | "profileSession" | "destroyAll"
    > = {
      listTabs: ({ threadId }) =>
        tabs.filter((tab) => threadId === null || tab.threadId === threadId),
      subscribeAutomationTabs: (listener) => {
        notifyTabsChanged = listener;
        return () => undefined;
      },
      profileSession: () => ({}) as Session,
      destroyAll: () => undefined,
    };
    const broker = createDesktopBrowserBroker({
      manager: manager as DesktopBrowserViewManager,
      product: "Chrome/1",
    });
    const events: DesktopBrowserChanged[] = [];
    broker.subscribe((event) => events.push(event));
    const window = createFakeWindow();

    broker.registerWindow(window as never);
    broker.setHostId("host_local");
    tabs = [
      { ...tabs[0]!, title: "Navigated" },
      { ...tabs[1]!, title: "Navigated" },
    ];
    notifyTabsChanged();

    expect(events.length).toBeGreaterThan(0);
    expect(new Set(events.map((event) => event.threadId))).toEqual(
      new Set([THREAD_ID]),
    );
    expect(
      events.flatMap((event) => event.tabs.map((tab) => tab.tabId)),
    ).not.toContain("panel-tab");
  });
});


describe("desktop browser broker window cleanup", () => {
  it.each([false, true])("releases a destroyed window with active lease: %s", async (withLease) => {
    const tab = nativeTab("thread-tab", THREAD_ID);
    const manager: Pick<DesktopBrowserViewManager, "listTabs" | "subscribeAutomationTabs"> = {
      listTabs: () => [tab],
      subscribeAutomationTabs: () => () => undefined,
    };
    const broker = createDesktopBrowserBroker({
      manager: manager as DesktopBrowserViewManager,
      product: "Chrome/1",
    });
    const window = createFakeWindow();
    const webContents = window.webContents;
    let destroyed = false;
    window.isDestroyed = () => destroyed;
    Object.defineProperty(window, "webContents", {
      get() {
        if (destroyed) throw new TypeError("Object has been destroyed");
        return webContents;
      },
    });
    broker.registerWindow(window);
    broker.setHostId("host_local");
    const target = broker.getTarget(webContents.id)!;
    if (withLease) {
      await broker.execute({
        type: "desktop.browser.acquire_control",
        ...target,
        threadId: THREAD_ID,
        tabIds: [tab.tabId],
        leaseId: "cleanup-lease",
        controllerLabel: "Cleanup test",
        expiresAt: Date.now() + 60_000,
      });
    }
    const registryChanged = vi.fn();
    broker.subscribeInstances(registryChanged);
    destroyed = true;
    expect(() => broker.releaseWindow(webContents.id)).not.toThrow();
    expect(broker.listInstances()).toEqual([]);
    expect(registryChanged).toHaveBeenCalledTimes(1);
    expect(() => broker.releaseWindow(webContents.id)).not.toThrow();
    broker.dispose();
  });
});

5. Root cause

The closed handler calls releaseWindow with a previously saved ID. instanceForWindow instead evaluates entry.window.webContents.id. A destroyed native object cannot support this access. This is a deterministic lifecycle mismatch once native access is invalid; no timing race is needed in the test.

There is also a second cleanup path: releaseWindow revokes leases, which publishes through tabsFor before publish checks isDestroyed. Saving the ID only for lookup would therefore leave the active-lease case broken.

6. Proposed fix

Capture webContentsId in the private instance entry at registration. Use that ID for broker lookup and tab scopes, retaining native access only for operations on live windows. The local change preserves existing contracts and lease behavior. The desktop suite and typecheck pass: 42 test files, 322 tests passed, one skipped. The two regression cases failed before the change and pass afterward.

7. Verification

The same agent repeated the reproduction in /tmp/slopcop-3588-verify at the exact recorded base after a separate frozen install and full build, using the command in section 4. The two cleanup cases failed with the same TypeError, while the existing snapshot case passed. No production fix was applied in this second checkout. No report correction was needed. The attached file includes a subsequent type-annotation clarification that does not change runtime behavior.

Native window lifecycle and platform quit recipes remain not run: Linux/Cinnamon is unavailable. No screenshot is supplied because this report verifies a nonvisual broker exception, not the appearance of a dialog.

8. Related issues and pull requests

No linked open pull request was found in the issue timeline or open-PR search for 3588. Nearby desktop reports concern installation/runtime packaging and do not explain this broker lifecycle defect.

9. Appendix

First clean run: 2 failed, 1 passed.
Second clean run: 2 failed, 1 passed.
Fixed desktop suite: 42 files passed; 322 tests passed, 1 skipped.
Desktop typecheck: passed.
Full Turbo build: passed in both base checkouts.

Raw logs and test artifacts are retained locally; public reproduction is fully inline. The verification inventory check reports a pre-existing unmapped browser CLI family; no inventory baseline was changed.

The host's default pnpm launcher initially failed to locate its executable. A temporary pnpm shim invoking Corepack was placed first on PATH for Turbo subprocesses; no dependency or lockfile was changed. All issue content was treated as untrusted claims.