馃毃 SLOP COP 馃毃 路 new-issue-autopilot

#4234 路 Retained browser owner cleanup

Bug 路 Priority Medium 路 Effort Low 路 desktop 路 2026-09-24
Trusted base: 9bfe862b3506f5b220f627af3513797152a2b39d 路 Issue

PARTIALLY REPRODUCED 路 root-cause confidence: high for the observer gap; medium for the reported visual path.

1. TL;DR

The lifecycle observer does not hide registered retained browser views when its owning thread changes or it unmounts. A newly authored regression fails on both transitions in two clean checkouts of trusted main. This establishes a missing owner-level cleanup, using a recording desktop API rather than Electron. The ordinary BrowserTabContent component does have layout cleanup that hides its view, so this test alone does not establish how sidebar navigation leaves a visible retained view outside that cleanup. The complete native More sequence and page restoration remain unverified; there is no native screenshot.

2. Claims vs findings

ClaimFindingEvidence
Departing observer does not hide retained viewsVerifiedTwo failing cases in each checkout
More navigation leaves a native page over another threadUnverified end to endNo native desktop UI run or screenshot; API recording only
Normal mounted content never hides on exitNot supportedBrowserTabContent has layout-effect hide cleanup
Returning restores the same pageUnverifiedNo Electron page exercised

3. Environment

Darwin arm64; Node 22.22.3; Corepack pnpm 9.15.0; Vitest 4.1.1, jsdom. Fresh detached worktrees /tmp/bb-repro-4234-a and /tmp/bb-repro-4234-b, both at the base above. No providers, running app, ports, browser profiles or application data directories were used. Production files were unchanged. Frozen installs succeeded using Corepack after the machine's default pnpm entrypoint failed to resolve its installed module.

4. Minimal reproduction

  1. Create a clean checkout at the base above.
  2. Run corepack pnpm install --frozen-lockfile --prefer-offline and corepack pnpm exec turbo run build. Ensure pnpm on PATH invokes Corepack when this machine's default pnpm launcher is broken.
  3. Copy the test into apps/app/src/components/secondary-panel/issue4234-lifecycle.test.tsx.
  4. Run pnpm exec turbo run test --filter=@bb/app -- issue4234-lifecycle.

The fixture registers two retained views for the owner and one unrelated view, models them as initially visible, then changes or unmounts the observer. It asserts that pages are not destroyed and only the unrelated view remains visible. Registering a retained view is an explicit precondition, not a reproduction of how the native UI reaches that state.

Expected: [ 'unrelated' ]
Actual:   [ 'first', 'second', 'unrelated' ]
Test Files  1 failed (1)
Tests       2 failed (2)
// @vitest-environment jsdom
import { cleanup, render } from "@testing-library/react";
import { afterEach, expect, it, vi } from "vitest";
import { createBbDesktopApi, createNoopDesktopBrowserApi } from "@/test/bb-desktop-test-utils";
import { BrowserTabLifecycleObserver } from "./BrowserTabDeck";
import { registerBrowserView, resetBrowserViewPersistence } from "./browserViewVisibilityCoordinator";

const originalDesktop = window.bbDesktop;
afterEach(() => {
  cleanup();
  window.bbDesktop = originalDesktop;
  resetBrowserViewPersistence();
});

it.each(["switch", "unmount"])("hides all retained owner views on %s without destroying pages", (action) => {
  const visible = new Set(["first", "second", "unrelated"]);
  const detach = vi.fn();
  const api = {
    ...createNoopDesktopBrowserApi(),
    detach,
    setVisible: vi.fn(({ tabId, visible: show }: { tabId: string; visible: boolean }) => {
      if (show) visible.add(tabId);
      else visible.delete(tabId);
    }),
  };
  window.bbDesktop = createBbDesktopApi({
    lastCheckedAt: null, latestVersion: null, pendingVersion: null,
    platform: "macos", updateAvailable: false, updateDownloaded: false, version: "test",
  }, api);
  for (const tabId of visible) {
    registerBrowserView({ environmentId: null, tabId, threadId: tabId === "unrelated" ? "other" : "owner" });
  }
  const view = render(<BrowserTabLifecycleObserver browserTabs={[]} threadId="owner" />);
  expect([...visible]).toEqual(["first", "second", "unrelated"]);
  if (action === "switch") view.rerender(<BrowserTabLifecycleObserver browserTabs={[]} threadId="destination" />);
  else view.unmount();
  expect(detach).not.toHaveBeenCalled();
  expect([...visible]).toEqual(["unrelated"]);
});

5. Root cause

BrowserTabLifecycleObserver only destroys removed tabs if the previous and current thread IDs match. It returns no departure cleanup and overwrites its prior snapshot after a thread change. The persistence registry retains thread ownership, but exposes destruction rather than hide-by-owner. No call reaches the recording API for either tested transition.

Qualification: BrowserTabContent's layout effect already hides a currently mounted view during cleanup. The focused reproduction deliberately exercises retained registry entries without mounted content. Therefore it proves the missing defensive ownership cleanup, not the complete native visual root cause. ThreadDetailView mounts the observer with the active thread ID and browser tabs.

6. Proposed fix

On observer departure, synchronously hide every registered view for that owner while preserving registrations and pages. Keep tab-close destruction separate and leave other threads' views untouched. Verify returning to the owner restores the same native page. Before treating the visual path as confirmed, run More navigation in an isolated native desktop and trace both component cleanup and native visibility calls.

7. PR review

Open PR #4011 was discovered through repository PR metadata and reviewed statically as untrusted data. Its observer layout cleanup and hide-by-thread helper address the verified gap. It also includes separate persistence and recovery changes, which this report does not validate. No PR code was checked out or executed. Verdict: relevant existing candidate, not runtime-verified here. No duplicate PR was created. GitHub currently reports no closing-issue relationship, but its implementation covers the same verified mechanism.

8. Related issues

Other reload/archive claims in the report were not used as proof and were not rerun. This investigation isolates owner transitions with no reload or archive.

9. Verification

The same agent repeated the test in the second clean checkout at exactly the recorded base. Both cases failed at the expected visibility assertion, not at setup or imports. Run A: 34 ms test execution; run B: 29 ms. Neither test run was replayed from Turbo cache. No correction to the focused result was needed; the verdict is deliberately partial because native visual behavior was not reproduced.

Both full Turbo builds passed (60 tasks each). The existing BrowserTabDeck ordering suite also passed all 9 tests. Control log 路 Build A 路 Build B.

10. Appendix

Run A log 路 Run B log. Repository visibility was public. Issue and PR text were treated only as untrusted claims; no supplied scripts, patches or external evidence URLs were executed or fetched.

> AGENT GENERATED