Fix browser pane close and runtime recovery

apps/desktop/src/browserManager.reliability.test.ts

@@ -0,0 +1,179 @@
+// FILE: browserManager.reliability.test.ts
+// Purpose: Verifies browser session closure and recovery from destroyed Electron runtimes.
+// Layer: Desktop unit test
+// Depends on: DesktopBrowserManager with a minimal Electron session mock
+
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+const electron = vi.hoisted(() => {
+  const createdWebContents: Array<{ loadURL: ReturnType<typeof vi.fn> }> = [];
+  let nextWebContentsId = 1;
+
+  function createWebContents() {
+    let currentUrl = "about:blank";
+    const webContents = {
+      id: nextWebContentsId++,
+      debugger: {
+        isAttached: () => false,
+        detach: vi.fn(),
+      },
+      navigationHistory: {
+        canGoBack: () => false,
+        canGoForward: () => false,
+      },
+      isDestroyed: () => false,
+      getURL: () => currentUrl,
+      getTitle: () => currentUrl,
+      isLoading: () => false,
+      getProcessId: () => 42,
+      loadURL: vi.fn(async (url: string) => {
+        currentUrl = url;
+      }),
+      setUserAgent: vi.fn(),
+      setWindowOpenHandler: vi.fn(),
+      on: vi.fn(),
+      removeListener: vi.fn(),
+      close: vi.fn(),
+      reload: vi.fn(),
+      goBack: vi.fn(),
+      goForward: vi.fn(),
+      openDevTools: vi.fn(),
+    };
+    createdWebContents.push(webContents);
+    return webContents;
+  }
+
+  return {
+    setUserAgent: vi.fn(),
+    onBeforeSendHeaders: vi.fn(),
+    createdWebContents,
+    createWebContents,
+  };
+});
+
+vi.mock("electron", () => ({
+  app: {
+    userAgentFallback: "Mozilla/5.0 Chrome/124.0.0.0 Electron/40.0.0 Scient/0.5.12 Safari/537.36",
+    getName: () => "Scient",
+    getPreferredSystemLanguages: () => ["en-US"],
+  },
+  BrowserWindow: class {
+    readonly mocked = true;
+  },
+  clipboard: {
+    writeImage: vi.fn(),
+    writeText: vi.fn(),
+  },
+  nativeImage: {
+    createFromBuffer: vi.fn(),
+  },
+  session: {
+    fromPartition: () => ({
+      setUserAgent: electron.setUserAgent,
+      webRequest: {
+        onBeforeSendHeaders: electron.onBeforeSendHeaders,
+      },
+    }),
+  },
+  shell: {
+    openExternal: vi.fn(),
+  },
+  webContents: {
+    fromId: vi.fn(),
+  },
+  WebContentsView: class {
+    readonly webContents = electron.createWebContents();
+    readonly setBounds = vi.fn();
+  },
+}));
+
+import type { ThreadId } from "@synara/contracts";
+
+import { DesktopBrowserManager } from "./browserManager";
+
+const THREAD_ID = "thread-close-tab" as ThreadId;
+
+describe("DesktopBrowserManager reliability", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    electron.createdWebContents.splice(0);
+  });
+
+  it("closes the browser session when its final tab closes", () => {
+    const manager = new DesktopBrowserManager();
+    const opened = manager.open({ threadId: THREAD_ID });
+    const tabId = opened.activeTabId;
+
+    expect(tabId).toBeTruthy();
+    const closed = manager.closeTab({ threadId: THREAD_ID, tabId: tabId ?? "" });
+
+    expect(closed.open).toBe(false);
+    expect(closed.tabs).toEqual([]);
+    expect(closed.activeTabId).toBeNull();
+    manager.dispose();
+  });
+
+  it("keeps the browser open while another tab remains", () => {
+    const manager = new DesktopBrowserManager();
+    const opened = manager.open({ threadId: THREAD_ID });
+    const firstTabId = opened.activeTabId;
+    const withSecondTab = manager.newTab({
+      threadId: THREAD_ID,
+      url: "https://example.com/",
+    });
+
+    const next = manager.closeTab({ threadId: THREAD_ID, tabId: firstTabId ?? "" });
+
+    expect(next.open).toBe(true);
+    expect(next.tabs).toHaveLength(1);
+    expect(next.activeTabId).toBe(withSecondTab.activeTabId);
+    manager.dispose();
+  });
+
+  it("replaces a destroyed tracked runtime before navigating", async () => {
+    const manager = new DesktopBrowserManager();
+    const opened = manager.open({ threadId: THREAD_ID });
+    const tabId = opened.activeTabId;
+    expect(tabId).toBeTruthy();
+
+    const runtimeKey = `${THREAD_ID}:${tabId}`;
+    const internals = manager as unknown as {
+      runtimes: Map<
+        string,
+        {
+          key: string;
+          threadId: ThreadId;
+          tabId: string;
+          webContents: { isDestroyed: () => boolean };
+          view: null;
+          ownsWebContents: boolean;
+          listenerDisposers: Array<() => void>;
+        }
+      >;
+    };
+    internals.runtimes.set(runtimeKey, {
+      key: runtimeKey,
+      threadId: THREAD_ID,
+      tabId: tabId ?? "",
+      webContents: { isDestroyed: () => true },
+      view: null,
+      ownsWebContents: true,
+      listenerDisposers: [],
+    });
+
+    expect(() =>
+      manager.navigate({
+        threadId: THREAD_ID,
+        tabId: tabId ?? "",
+        url: "https://example.com/",
+      }),
+    ).not.toThrow();
+
+    await vi.waitFor(() => {
+      expect(electron.createdWebContents).toHaveLength(1);
+      expect(electron.createdWebContents[0]?.loadURL).toHaveBeenCalledWith("https://example.com/");
+    });
+    expect(manager.getState({ threadId: THREAD_ID }).lastError).toBeNull();
+    manager.dispose();
+  });
+});

apps/desktop/src/browserManager.ts

@@ -20,6 +20,7 @@ import {
   artifactPreviewNavigationAllowed,
   artifactPreviewRequestAllowed,
 } from "./artifactPreviewPolicy";
+import { loadBrowserRuntimeUrl } from "./browserRuntimeLoad";
 import type {
   BrowserAttachWebviewInput,
   BrowserCaptureScreenshotResult,
@@ -222,13 +223,6 @@ function normalizeBounds(bounds: BrowserPanelBounds | null): BrowserPanelBounds
   };
 }
 
-function isAbortedNavigationError(error: unknown): boolean {
-  if (!(error instanceof Error)) {
-    return false;
-  }
-  return /ERR_ABORTED|\(-3\)/i.test(error.message);
-}
-
 function mapBrowserLoadError(errorCode: number): string {
   switch (errorCode) {
     case -102:
@@ -932,23 +926,21 @@ export class DesktopBrowserManager {
     syncThreadLastError(state);
     this.markThreadStateChanged(input.threadId);
 
-    const runtime = this.runtimes.get(buildRuntimeKey(input.threadId, tab.id));
-    if (runtime) {
+    const runtimeKey = buildRuntimeKey(input.threadId, tab.id);
+    const shouldLoadRuntime =
+      this.runtimes.has(runtimeKey) || this.activeThreadId === input.threadId;
+    if (shouldLoadRuntime) {
+      if (this.activeThreadId === input.threadId) {
+        this.clearSuspendTimer(input.threadId);
+      }
+      // Re-resolve through ensureLiveRuntime so a destroyed-but-still-tracked WebContents is
+      // replaced before navigation instead of being handed to the async loader.
+      const runtime = this.ensureLiveRuntime(input.threadId, tab.id);
       const bounds = this.getVisibleBoundsForThread(input.threadId);
       if (state.activeTabId === tab.id && bounds) {
         this.attachRuntime(runtime, bounds);
       }
       void this.loadTab(input.threadId, tab.id, { force: true, runtime });
-    } else if (this.activeThreadId === input.threadId) {
-      // Load the target tab directly so we don't clobber its pending URL with a
-      // thread-wide runtime sync from the old live page state.
-      const nextRuntime = this.ensureLiveRuntime(input.threadId, tab.id);
-      this.clearSuspendTimer(input.threadId);
-      const bounds = this.getVisibleBoundsForThread(input.threadId);
-      if (state.activeTabId === tab.id && bounds) {
-        this.attachRuntime(nextRuntime, bounds);
-      }
-      void this.loadTab(input.threadId, tab.id, { force: true, runtime: nextRuntime });
     }
 
     this.emitState(input.threadId);
@@ -1021,27 +1013,20 @@ export class DesktopBrowserManager {
       return this.snapshotThreadState(input.threadId, state);
     }
 
+    if (nextTabs.length === 0) {
+      // The tab close button is also the natural close affordance for a one-tab browser.
+      // Tear down the browser session so the renderer can close this dock pane and reveal
+      // the surface chooser instead of immediately manufacturing another blank tab.
+      return this.close({ threadId: input.threadId });
+    }
+
     this.closePopupWindowsForTab(input.threadId, input.tabId);
     this.destroyRuntime(input.threadId, input.tabId);
     state.tabs = nextTabs;
     if (closedTab) {
       this.clearArtifactSession(input.threadId, closedTab);
     }
 
-    if (nextTabs.length === 0) {
-      // Closing the last tab keeps the browser open on a fresh blank tab (the same state
-      // as a brand-new browser session) so the user can type a new URL in the search box,
-      // instead of tearing the whole panel down.
-      const replacementTab = createBrowserTab();
-      state.tabs = [replacementTab];
-      state.activeTabId = replacementTab.id;
-      state.lastError = null;
-
-      this.markThreadStateChanged(input.threadId);
-      this.emitState(input.threadId);
-      return this.snapshotThreadState(input.threadId, state);
-    }
-
     if (!state.activeTabId || state.activeTabId === input.tabId) {
       state.activeTabId = nextTabs[Math.max(0, nextTabs.length - 1)]?.id ?? null;
     }
@@ -1869,41 +1854,64 @@ export class DesktopBrowserManager {
       return;
     }
 
-    const runtime = options.runtime ?? this.ensureLiveRuntime(threadId, tabId);
-    const webContents = runtime.webContents;
     const nextUrl = normalizeUrlInput(
       options.force === true ? tab.url : (tab.lastCommittedUrl ?? tab.url),
     );
-    const currentUrl = webContents.getURL();
-    const shouldLoad = options.force === true || currentUrl !== nextUrl || currentUrl.length === 0;
-
-    if (!shouldLoad) {
-      this.queueRuntimeStateSync(threadId, tabId);
-      return;
-    }
-
-    tab.url = nextUrl;
-    tab.status = "live";
-    tab.isLoading = true;
-    tab.lastError = null;
-    syncThreadLastError(state);
-    this.markThreadStateChanged(threadId);
-    this.emitState(threadId);
 
     try {
-      await webContents.loadURL(nextUrl);
-      this.queueRuntimeStateSync(threadId, tabId);
-    } catch (error) {
-      if (isAbortedNavigationError(error)) {
+      const runtime = options.runtime ?? this.ensureLiveRuntime(threadId, tabId);
+      const outcome = await loadBrowserRuntimeUrl({
+        webContents: runtime.webContents,
+        nextUrl,
+        force: options.force === true,
+        isCurrent: () => this.runtimes.get(runtime.key) === runtime,
+        onLoadStart: () => {
+          tab.url = nextUrl;
+          tab.status = "live";
+          tab.isLoading = true;
+          tab.lastError = null;
+          syncThreadLastError(state);
+          this.markThreadStateChanged(threadId);
+          this.emitState(threadId);
+        },
+      });
+
+      if (outcome === "loaded" || outcome === "unchanged" || outcome === "aborted") {
         this.queueRuntimeStateSync(threadId, tabId);
         return;
       }
 
+      if (outcome === "stale") {
+        // If this exact runtime died without its normal render-process-gone cleanup firing,
+        // remove it so the next visible activation creates a healthy replacement.
+        if (this.runtimes.get(runtime.key) === runtime && runtime.webContents.isDestroyed()) {
+          this.destroyRuntime(threadId, tabId);
+          const didChange = suspendTabState(tab) || syncThreadLastError(state);
+          if (didChange) {
+            this.markThreadStateChanged(threadId);
+            this.emitState(threadId);
+          }
+        }
+        return;
+      }
+
       tab.isLoading = false;
       tab.lastError = "Couldn't open this page.";
       syncThreadLastError(state);
       this.markThreadStateChanged(threadId);
       this.emitState(threadId);
+    } catch {
+      // Runtime construction and bookkeeping errors must be surfaced as browser state, never as
+      // unhandled promises from the fire-and-forget navigation paths above.
+      const currentTab = this.getTab(state, tabId);
+      if (!currentTab) {
+        return;
+      }
+      currentTab.isLoading = false;
+      currentTab.lastError = "Couldn't open this page.";
+      syncThreadLastError(state);
+      this.markThreadStateChanged(threadId);
+      this.emitState(threadId);
     }
   }