From 13a6f536c63dee7a7b6a79fdcb48d09641187db5 Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 14:04:00 +0300 Subject: [PATCH 01/16] Fix browser multi-tab controls and menu occlusion --- .../src/components/BrowserPanel.browser.tsx | 163 ++++++++++- .../BrowserPanel.overlay.browser.tsx | 75 +++++ .../src/components/BrowserPanel.overlay.ts | 197 +++++++++++++ apps/web/src/components/BrowserPanel.tsx | 260 +++++++----------- .../src/components/chat/RightDock.browser.tsx | 45 +++ apps/web/src/components/chat/RightDock.tsx | 7 +- apps/web/src/rightDockStore.logic.test.ts | 29 ++ apps/web/src/rightDockStore.logic.ts | 12 + 8 files changed, 614 insertions(+), 174 deletions(-) create mode 100644 apps/web/src/components/BrowserPanel.overlay.browser.tsx create mode 100644 apps/web/src/components/BrowserPanel.overlay.ts diff --git a/apps/web/src/components/BrowserPanel.browser.tsx b/apps/web/src/components/BrowserPanel.browser.tsx index 5f53a80dc..d235565b1 100644 --- a/apps/web/src/components/BrowserPanel.browser.tsx +++ b/apps/web/src/components/BrowserPanel.browser.tsx @@ -5,7 +5,8 @@ import "../index.css"; import type { NativeApi, ThreadBrowserState, ThreadId } from "@synara/contracts"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; -import { page } from "vitest/browser"; +import { useState } from "react"; +import { page, userEvent } from "vitest/browser"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { render } from "vitest-browser-react"; @@ -90,16 +91,65 @@ function renderLivePanel(onClosePanel: () => void) { const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); return render( +
+ +
+ , + ); +} + +function PreviewToLivePanel() { + const [runtimeMode, setRuntimeMode] = useState<"live" | "preview">("preview"); + return ( +
setRuntimeMode("live")} + onClosePanel={() => undefined} /> +
+ ); +} + +function renderPreviewToLivePanel() { + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + return render( + + , ); } +function liveBrowserApi(options?: { + openState?: ThreadBrowserState; + newTabState?: ThreadBrowserState; +}) { + const openState = options?.openState ?? browserState("tab-1"); + return { + browser: { + open: vi.fn(async () => openState), + hide: vi.fn(async () => undefined), + setPanelBounds: vi.fn(async () => undefined), + attachWebview: vi.fn(async () => openState), + detachWebview: vi.fn(async () => undefined), + newTab: vi.fn(async () => options?.newTabState ?? openState), + closeTab: vi.fn(async () => openState), + onState: vi.fn(() => () => undefined), + onCopyLink: vi.fn(() => () => undefined), + }, + projects: { + revokeHtmlArtifactPreview: vi.fn(async () => ({ revoked: false })), + }, + } as unknown as NativeApi; +} + describe("BrowserPanel interactions", () => { beforeEach(() => { useBrowserStateStore.getState().upsertThreadState(browserState("tab-1")); @@ -187,4 +237,111 @@ describe("BrowserPanel interactions", () => { expect(onClosePanel).toHaveBeenCalledOnce(); }); }); + + it("creates and activates a second tab from the visible tab-strip button", async () => { + const openState = browserState("tab-1"); + openState.version = 20; + openState.tabs = [openState.tabs[0]!]; + const secondTabState: ThreadBrowserState = { + ...openState, + version: openState.version + 1, + activeTabId: "tab-2", + tabs: [ + openState.tabs[0]!, + { + ...browserState("tab-2").tabs[1]!, + url: "about:blank", + title: "New tab", + lastCommittedUrl: "about:blank", + }, + ], + }; + const api = liveBrowserApi({ openState, newTabState: secondTabState }); + nativeApiTestState.api = api; + useBrowserStateStore.getState().upsertThreadState(openState); + + await renderLivePanel(vi.fn()); + const newTabButton = page.getByRole("button", { name: "New browser tab" }); + await expect.element(newTabButton).toBeVisible(); + const newTabElement = (await newTabButton.element()) as HTMLButtonElement; + newTabElement.click(); + newTabElement.click(); + + await vi.waitFor(() => { + expect(api.browser.newTab).toHaveBeenCalledWith({ + threadId: THREAD_ID, + activate: true, + }); + expect(api.browser.newTab).toHaveBeenCalledTimes(1); + expect(useBrowserStateStore.getState().threadStatesByThreadId[THREAD_ID]?.activeTabId).toBe( + "tab-2", + ); + }); + await expect.element(page.getByText("New tab", { exact: true })).toBeVisible(); + expect(page.getByRole("button", { name: "Close tab" }).elements()).toHaveLength(2); + }); + + it("preserves a new-tab click while a sleeping browser pane wakes", async () => { + const openState = browserState("tab-1"); + openState.version = 30; + openState.tabs = [openState.tabs[0]!]; + const secondTabState: ThreadBrowserState = { + ...openState, + version: openState.version + 1, + activeTabId: "tab-2", + tabs: [openState.tabs[0]!, browserState("tab-2").tabs[1]!], + }; + const api = liveBrowserApi({ openState, newTabState: secondTabState }); + nativeApiTestState.api = api; + useBrowserStateStore.getState().upsertThreadState(openState); + + await renderPreviewToLivePanel(); + const newTabButton = page.getByRole("button", { name: "New browser tab" }); + ((await newTabButton.element()) as HTMLButtonElement).click(); + + await vi.waitFor(() => { + expect(api.browser.open).toHaveBeenCalledOnce(); + expect(api.browser.newTab).toHaveBeenCalledOnce(); + expect(useBrowserStateStore.getState().threadStatesByThreadId[THREAD_ID]?.activeTabId).toBe( + "tab-2", + ); + }); + }); + + it("hides the native browser surface while an intersecting app menu is open", async () => { + const openState = browserState("tab-1"); + const api = liveBrowserApi({ openState }); + nativeApiTestState.api = api; + + await renderLivePanel(vi.fn()); + await vi.waitFor(() => expect(api.browser.open).toHaveBeenCalledOnce()); + await vi.waitFor(() => { + const webview = document.querySelector("webview"); + expect(webview).not.toBeNull(); + expect(webview?.style.visibility).not.toBe("hidden"); + }); + + ( + (await page.getByRole("button", { name: "Browser actions" }).element()) as HTMLButtonElement + ).click(); + await expect.element(page.getByRole("menuitem", { name: "New tab" })).toBeVisible(); + await vi.waitFor(() => { + const webview = document.querySelector("webview"); + expect(webview?.style.visibility).toBe("hidden"); + expect(webview?.style.pointerEvents).toBe("none"); + expect(api.browser.setPanelBounds).toHaveBeenCalledWith({ + threadId: THREAD_ID, + bounds: null, + surface: "renderer", + }); + }); + + await userEvent.keyboard("{Escape}"); + await vi.waitFor(() => { + expect(page.getByRole("menuitem", { name: "New tab" }).query()).toBeNull(); + const webview = document.querySelector("webview"); + expect(webview?.style.visibility).toBe("visible"); + expect(webview?.style.pointerEvents).toBe("auto"); + }); + }); }); diff --git a/apps/web/src/components/BrowserPanel.overlay.browser.tsx b/apps/web/src/components/BrowserPanel.overlay.browser.tsx new file mode 100644 index 000000000..8fd7862ba --- /dev/null +++ b/apps/web/src/components/BrowserPanel.overlay.browser.tsx @@ -0,0 +1,75 @@ +import { afterEach, describe, expect, it } from "vitest"; + +import { + hasNativeBrowserObscuringOverlay, + nativeBrowserOverlayMutationsRequireSync, +} from "./BrowserPanel.overlay"; + +function waitForMutations(action: () => void): Promise { + return new Promise((resolve) => { + const observer = new MutationObserver((records) => { + observer.disconnect(); + resolve(records); + }); + observer.observe(document.body, { attributes: true, childList: true, subtree: true }); + action(); + }); +} + +describe("native browser overlay coordination", () => { + afterEach(() => { + document.body.innerHTML = ""; + }); + + it("syncs for app overlay mounts but ignores unrelated document mutations", async () => { + const container = document.createElement("div"); + document.body.append(container); + + const unrelatedRecords = await waitForMutations(() => { + container.append(document.createElement("span")); + }); + expect(nativeBrowserOverlayMutationsRequireSync(unrelatedRecords)).toBe(false); + + const overlay = document.createElement("div"); + overlay.dataset.slot = "menu-popup"; + const overlayRecords = await waitForMutations(() => { + document.body.append(overlay); + }); + expect(nativeBrowserOverlayMutationsRequireSync(overlayRecords)).toBe(true); + + const removalRecords = await waitForMutations(() => { + overlay.remove(); + }); + expect(nativeBrowserOverlayMutationsRequireSync(removalRecords)).toBe(true); + }); + + it("only treats a visible, intersecting popup as an obstruction", () => { + const viewport = document.createElement("div"); + Object.assign(viewport.style, { + position: "fixed", + left: "100px", + top: "100px", + width: "240px", + height: "240px", + }); + const popup = document.createElement("div"); + popup.dataset.slot = "menu-popup"; + Object.assign(popup.style, { + position: "fixed", + left: "160px", + top: "160px", + width: "120px", + height: "120px", + }); + document.body.append(viewport, popup); + + expect(hasNativeBrowserObscuringOverlay(viewport)).toBe(true); + + popup.style.left = "500px"; + expect(hasNativeBrowserObscuringOverlay(viewport)).toBe(false); + + popup.style.left = "160px"; + popup.style.visibility = "hidden"; + expect(hasNativeBrowserObscuringOverlay(viewport)).toBe(false); + }); +}); diff --git a/apps/web/src/components/BrowserPanel.overlay.ts b/apps/web/src/components/BrowserPanel.overlay.ts new file mode 100644 index 000000000..96581d286 --- /dev/null +++ b/apps/web/src/components/BrowserPanel.overlay.ts @@ -0,0 +1,197 @@ +// FILE: BrowserPanel.overlay.ts +// Purpose: Keeps Electron's native browser surface behind app-owned menus and dialogs. +// Layer: Browser panel DOM/native-surface coordination + +// Electron guest surfaces are composited above ordinary renderer DOM. App-owned overlays +// therefore need the native surface hidden while they intersect the browser viewport. +const NATIVE_BROWSER_OBSCURING_OVERLAY_SELECTOR = [ + "[data-native-browser-overlay='true']", + "[data-slot='menu-positioner']", + "[data-slot='menu-popup']", + "[data-slot='menu-sub-content']", + "[data-slot='select-positioner']", + "[data-slot='select-popup']", + "[data-slot='combobox-positioner']", + "[data-slot='combobox-popup']", + "[data-slot='popover-positioner']", + "[data-slot='popover-popup']", + "[data-slot='dialog-backdrop']", + "[data-slot='dialog-popup']", + "[data-slot='dialog-viewport']", + "[data-slot='alert-dialog-backdrop']", + "[data-slot='alert-dialog-popup']", + "[data-slot='alert-dialog-viewport']", + "[data-slot='command-dialog-backdrop']", + "[data-slot='command-dialog-popup']", + "[data-slot='command-dialog-viewport']", + "[data-slot='toast-popup']", + "[role='dialog'][aria-modal='true']", +].join(", "); + +// The browser itself lives inside a sheet, and toast portals/positioners are just +// layout containers. Treating either as blockers hides the native surface unnecessarily. +const NATIVE_BROWSER_NON_OBSCURING_OVERLAY_SELECTOR = [ + "[data-panel-resize-overlay='true']", + "[data-slot='sheet-backdrop']", + "[data-slot='sheet-popup']", + "[data-slot='toast-portal']", + "[data-slot='toast-portal-anchored']", + "[data-slot='toast-viewport']", + "[data-slot='toast-viewport-anchored']", + "[data-slot='toast-positioner']", +].join(", "); + +const NATIVE_BROWSER_OVERLAY_SAMPLE_POINTS = [ + [0.5, 0.5], + [0.2, 0.2], + [0.8, 0.2], + [0.2, 0.8], + [0.8, 0.8], +] as const; + +export interface BrowserWebviewElement extends HTMLElement { + getWebContentsId?: () => number; +} + +export function setBrowserWebviewOverlayOcclusion( + webview: BrowserWebviewElement | null, + occluded: boolean, +): void { + if (!webview) { + return; + } + webview.style.visibility = occluded ? "hidden" : "visible"; + webview.style.pointerEvents = occluded ? "none" : "auto"; +} + +function isVisibleOverlayElement(element: HTMLElement): boolean { + const styles = window.getComputedStyle(element); + if (styles.display === "none" || styles.visibility === "hidden" || styles.opacity === "0") { + return false; + } + return element.getClientRects().length > 0; +} + +function isNativeBrowserNonObscuringOverlayElement(element: HTMLElement): boolean { + return ( + element.closest("[data-slot='toast-popup']") === null && + element.closest(NATIVE_BROWSER_NON_OBSCURING_OVERLAY_SELECTOR) !== null + ); +} + +function rectsIntersect(a: DOMRect, b: DOMRect): boolean { + return a.left < b.right && a.right > b.left && a.top < b.bottom && a.bottom > b.top; +} + +function candidateObscuresNativeBrowser(candidate: HTMLElement, element: HTMLElement): boolean { + if (candidate === element || candidate.contains(element) || element.contains(candidate)) { + return false; + } + if (!isVisibleOverlayElement(candidate)) { + return false; + } + + const elementRect = element.getBoundingClientRect(); + for (const candidateRect of candidate.getClientRects()) { + if (rectsIntersect(elementRect, candidateRect)) { + return true; + } + } + + return false; +} + +function hasTopLayerDomObstruction(element: HTMLElement): boolean { + const rect = element.getBoundingClientRect(); + if (rect.width <= 0 || rect.height <= 0) { + return false; + } + + for (const [xRatio, yRatio] of NATIVE_BROWSER_OVERLAY_SAMPLE_POINTS) { + const x = rect.left + rect.width * xRatio; + const y = rect.top + rect.height * yRatio; + if (x < 0 || y < 0 || x > window.innerWidth || y > window.innerHeight) { + continue; + } + + for (const hitElement of document.elementsFromPoint(x, y)) { + if (!(hitElement instanceof HTMLElement)) { + continue; + } + if (hitElement === element || element.contains(hitElement) || hitElement.contains(element)) { + continue; + } + if (isNativeBrowserNonObscuringOverlayElement(hitElement)) { + continue; + } + if (!isVisibleOverlayElement(hitElement)) { + continue; + } + return true; + } + } + + return false; +} + +export function hasNativeBrowserObscuringOverlay(element: HTMLElement): boolean { + const candidates = document.querySelectorAll( + NATIVE_BROWSER_OBSCURING_OVERLAY_SELECTOR, + ); + for (const candidate of candidates) { + if (candidateObscuresNativeBrowser(candidate, element)) { + return true; + } + } + + return hasTopLayerDomObstruction(element); +} + +function nodeContainsNativeBrowserOverlay(node: Node): boolean { + return ( + node instanceof Element && + (node.matches(NATIVE_BROWSER_OBSCURING_OVERLAY_SELECTOR) || + node.querySelector(NATIVE_BROWSER_OBSCURING_OVERLAY_SELECTOR) !== null) + ); +} + +// React portals do not resize the browser viewport, so their mount/unmount lifecycle must +// explicitly trigger a bounds sync. Filter aggressively to avoid syncing on streamed chat DOM. +export function nativeBrowserOverlayMutationsRequireSync( + mutations: readonly MutationRecord[], +): boolean { + return mutations.some((mutation) => { + if ( + mutation.target instanceof Element && + (mutation.target.matches(NATIVE_BROWSER_OBSCURING_OVERLAY_SELECTOR) || + mutation.target.closest(NATIVE_BROWSER_OBSCURING_OVERLAY_SELECTOR) !== null) + ) { + return true; + } + if (mutation.type !== "childList") { + return false; + } + return [...mutation.addedNodes, ...mutation.removedNodes].some( + nodeContainsNativeBrowserOverlay, + ); + }); +} + +export function isNativeBrowserTransitionSignalTarget( + target: EventTarget | null, + viewportElement: HTMLElement, +): boolean { + if (!(target instanceof HTMLElement)) { + return false; + } + + if (viewportElement.contains(target) || target.contains(viewportElement)) { + return true; + } + + return ( + target.closest(NATIVE_BROWSER_OBSCURING_OVERLAY_SELECTOR) !== null || + target.closest("[data-slot='sidebar-container']") !== null || + target.closest("[data-slot='sheet-popup']") !== null + ); +} diff --git a/apps/web/src/components/BrowserPanel.tsx b/apps/web/src/components/BrowserPanel.tsx index fa42853db..925e94c5d 100644 --- a/apps/web/src/components/BrowserPanel.tsx +++ b/apps/web/src/components/BrowserPanel.tsx @@ -68,6 +68,13 @@ import { type BrowserAddressSuggestion, type BrowserCopyFeedback, } from "./BrowserPanel.logic"; +import { + type BrowserWebviewElement, + hasNativeBrowserObscuringOverlay, + isNativeBrowserTransitionSignalTarget, + nativeBrowserOverlayMutationsRequireSync, + setBrowserWebviewOverlayOcclusion, +} from "./BrowserPanel.overlay"; import { DiffPanelLoadingState, DiffPanelShell, type DiffPanelMode } from "./DiffPanelShell"; import { LocalServerIdentity } from "./LocalServerIdentity"; import { Button } from "./ui/button"; @@ -99,20 +106,6 @@ const BROWSER_ACTION_MENU_ITEM_CLASS_NAME = "text-[var(--color-text-foreground)] data-highlighted:text-[var(--color-text-foreground)]"; const BROWSER_ACTION_MENU_ICON_CLASS_NAME = "inline-flex size-3.5 shrink-0 items-center justify-center text-[var(--color-text-foreground-secondary)] [&>svg]:size-3.5 [&>[data-slot=central-icon]]:size-3.5"; -const NATIVE_BROWSER_OBSCURING_OVERLAY_SELECTOR = [ - "[data-slot='dialog-backdrop']", - "[data-slot='dialog-popup']", - "[data-slot='dialog-viewport']", - "[data-slot='alert-dialog-backdrop']", - "[data-slot='alert-dialog-popup']", - "[data-slot='alert-dialog-viewport']", - "[data-slot='command-dialog-backdrop']", - "[data-slot='command-dialog-popup']", - "[data-slot='command-dialog-viewport']", - "[data-slot='toast-popup']", - "[role='dialog'][aria-modal='true']", -].join(", "); - function BrowserActionMenuIcon({ icon: Icon }: { icon: LucideIcon }) { return ( @@ -121,19 +114,6 @@ function BrowserActionMenuIcon({ icon: Icon }: { icon: LucideIcon }) { ); } -// The browser itself lives inside a sheet, and toast portals/positioners are just -// layout containers. Treating either as blockers hides the WebContentsView. -const NATIVE_BROWSER_NON_OBSCURING_OVERLAY_SELECTOR = [ - "[data-panel-resize-overlay='true']", - "[data-slot='sheet-backdrop']", - "[data-slot='sheet-popup']", - "[data-slot='toast-portal']", - "[data-slot='toast-portal-anchored']", - "[data-slot='toast-viewport']", - "[data-slot='toast-viewport-anchored']", - "[data-slot='toast-positioner']", -].join(", "); - interface BrowserViewportPerfCounters { syncAttempts: number; syncSkips: number; @@ -147,10 +127,6 @@ interface BrowserViewportPerfCounters { ignoredTransitionSignals: number; } -interface BrowserWebviewElement extends HTMLElement { - getWebContentsId?: () => number; -} - const VIEWPORT_TRANSITION_PROPERTIES = new Set([ "transform", "translate", @@ -201,129 +177,6 @@ function ignoreBrowserWebviewDetachError(): void { // Renderer webview detach is best-effort cleanup; a stale/destroyed guest is already gone. } -function setBrowserWebviewOverlayOcclusion( - webview: BrowserWebviewElement | null, - occluded: boolean, -): void { - if (!webview) { - return; - } - webview.style.visibility = occluded ? "hidden" : "visible"; - webview.style.pointerEvents = occluded ? "none" : "auto"; -} - -function isVisibleOverlayElement(element: HTMLElement): boolean { - const styles = window.getComputedStyle(element); - if (styles.display === "none" || styles.visibility === "hidden" || styles.opacity === "0") { - return false; - } - return element.getClientRects().length > 0; -} - -function isNativeBrowserNonObscuringOverlayElement(element: HTMLElement): boolean { - return ( - element.closest("[data-slot='toast-popup']") === null && - element.closest(NATIVE_BROWSER_NON_OBSCURING_OVERLAY_SELECTOR) !== null - ); -} - -const NATIVE_BROWSER_OVERLAY_SAMPLE_POINTS = [ - [0.5, 0.5], - [0.2, 0.2], - [0.8, 0.2], - [0.2, 0.8], - [0.8, 0.8], -] as const; - -function rectsIntersect(a: DOMRect, b: DOMRect): boolean { - return a.left < b.right && a.right > b.left && a.top < b.bottom && a.bottom > b.top; -} - -function candidateObscuresNativeBrowser(candidate: HTMLElement, element: HTMLElement): boolean { - if (candidate === element || candidate.contains(element) || element.contains(candidate)) { - return false; - } - if (!isVisibleOverlayElement(candidate)) { - return false; - } - - const elementRect = element.getBoundingClientRect(); - const candidateRects = candidate.getClientRects(); - for (const candidateRect of candidateRects) { - if (rectsIntersect(elementRect, candidateRect)) { - return true; - } - } - - return false; -} - -function hasTopLayerDomObstruction(element: HTMLElement): boolean { - const rect = element.getBoundingClientRect(); - if (rect.width <= 0 || rect.height <= 0) { - return false; - } - - for (const [xRatio, yRatio] of NATIVE_BROWSER_OVERLAY_SAMPLE_POINTS) { - const x = rect.left + rect.width * xRatio; - const y = rect.top + rect.height * yRatio; - if (x < 0 || y < 0 || x > window.innerWidth || y > window.innerHeight) { - continue; - } - - const hitElements = document.elementsFromPoint(x, y); - for (const hitElement of hitElements) { - if (!(hitElement instanceof HTMLElement)) { - continue; - } - if (hitElement === element || element.contains(hitElement) || hitElement.contains(element)) { - continue; - } - if (isNativeBrowserNonObscuringOverlayElement(hitElement)) { - continue; - } - if (!isVisibleOverlayElement(hitElement)) { - continue; - } - return true; - } - } - - return false; -} - -function hasNativeBrowserObscuringOverlay(element: HTMLElement): boolean { - const candidates = document.querySelectorAll( - NATIVE_BROWSER_OBSCURING_OVERLAY_SELECTOR, - ); - for (const candidate of candidates) { - if (candidateObscuresNativeBrowser(candidate, element)) { - return true; - } - } - - return hasTopLayerDomObstruction(element); -} - -function isNativeBrowserTransitionSignalTarget( - target: EventTarget | null, - viewportElement: HTMLElement, -): boolean { - if (!(target instanceof HTMLElement)) { - return false; - } - - if (viewportElement.contains(target) || target.contains(viewportElement)) { - return true; - } - - return ( - target.closest(NATIVE_BROWSER_OBSCURING_OVERLAY_SELECTOR) !== null || - target.closest("[data-slot='sidebar-container']") !== null || - target.closest("[data-slot='sheet-popup']") !== null - ); -} - function isBrowserPerfLoggingEnabled(): boolean { if (typeof window === "undefined") { return false; @@ -529,6 +382,8 @@ export function BrowserPanel({ const addressDraftsByTabIdRef = useRef(new Map()); const lastSyncedAddressByTabIdRef = useRef(new Map()); const previousActiveTabIdRef = useRef(null); + const pendingCreateTabRef = useRef(false); + const createTabInFlightRef = useRef(false); const artifactPreviewUrlsRef = useRef( new Set( threadBrowserState?.tabs.filter((tab) => tab.kind === "artifact").map((tab) => tab.url) ?? [], @@ -558,6 +413,7 @@ export function BrowserPanel({ const [isAddressFocused, setIsAddressFocused] = useState(false); const [workspaceReady, setWorkspaceReady] = useState(false); const [localError, setLocalError] = useState(null); + const [isCreatingTab, setIsCreatingTab] = useState(false); const runtimeReady = isLiveRuntime ? workspaceReady : true; const activeTab = threadBrowserState?.tabs.find((tab) => tab.id === threadBrowserState.activeTabId) ?? @@ -986,6 +842,24 @@ export function BrowserPanel({ scheduleSyncBounds(); }); observer.observe(element); + const overlayObserver = new MutationObserver((mutations) => { + if (nativeBrowserOverlayMutationsRequireSync(mutations)) { + scheduleSyncBounds(); + } + }); + overlayObserver.observe(document.body, { + attributes: true, + attributeFilter: [ + "data-open", + "data-closed", + "data-starting-style", + "data-ending-style", + "hidden", + "style", + ], + childList: true, + subtree: true, + }); window.addEventListener("resize", scheduleSyncBounds); window.addEventListener(PANEL_RESIZE_OVERLAY_SYNC_EVENT, scheduleSyncBounds); document.addEventListener("transitionrun", handleTransitionBounds, true); @@ -995,6 +869,7 @@ export function BrowserPanel({ return () => { setBrowserWebviewOverlayOcclusion(browserWebviewRef.current, false); observer.disconnect(); + overlayObserver.disconnect(); window.removeEventListener("resize", scheduleSyncBounds); window.removeEventListener(PANEL_RESIZE_OVERLAY_SYNC_EVENT, scheduleSyncBounds); document.removeEventListener("transitionrun", handleTransitionBounds, true); @@ -1123,23 +998,53 @@ export function BrowserPanel({ [api, ensureLiveRuntime, runBrowserAction, threadId, upsertThreadState], ); + const performCreateTab = useCallback(() => { + if (!api) { + return false; + } + if (createTabInFlightRef.current) { + return false; + } + createTabInFlightRef.current = true; + setIsCreatingTab(true); + void runBrowserAction(() => api.browser.newTab({ threadId, activate: true })) + .then((state) => { + if (state) { + upsertThreadState(state); + } + window.requestAnimationFrame(() => { + addressInputRef.current?.focus(); + addressInputRef.current?.select(); + }); + }) + .finally(() => { + createTabInFlightRef.current = false; + setIsCreatingTab(false); + }); + return true; + }, [api, runBrowserAction, threadId, upsertThreadState]); + const onCreateTab = useCallback(() => { - if (!ensureLiveRuntime()) { + if (!isLiveRuntime || !workspaceReady) { + if (!pendingCreateTabRef.current) { + pendingCreateTabRef.current = true; + if (!isLiveRuntime) { + requestLiveRuntime(); + } + } return; } - if (!api) { + performCreateTab(); + }, [isLiveRuntime, performCreateTab, requestLiveRuntime, workspaceReady]); + + useEffect(() => { + if (!isLiveRuntime || !workspaceReady || !pendingCreateTabRef.current) { return; } - void runBrowserAction(() => api.browser.newTab({ threadId, activate: true })).then((state) => { - if (state) { - upsertThreadState(state); - } - window.requestAnimationFrame(() => { - addressInputRef.current?.focus(); - addressInputRef.current?.select(); - }); - }); - }, [api, ensureLiveRuntime, runBrowserAction, threadId, upsertThreadState]); + if (performCreateTab()) { + pendingCreateTabRef.current = false; + } + }, [isLiveRuntime, performCreateTab, workspaceReady]); const onCaptureScreenshot = useCallback(() => { if (!ensureLiveRuntime()) { @@ -1484,7 +1389,10 @@ export function BrowserPanel({ /> {showBrowserAddressSuggestions ? ( -
+
{browserAddressSuggestions.map((suggestion) => (
+ {browserChromeStatus ? (
{ await screen.unmount(); } }); + + it("does not offer an already-open singleton panel as a new panel", async () => { + const onAddPane = vi.fn(); + const screen = await render( + true} + addMenuKinds={["browser", "sidechat"]} + onSelectPane={vi.fn()} + onClosePane={vi.fn()} + onCollapse={vi.fn()} + onOpenChange={vi.fn()} + onAddPane={onAddPane} + renderPane={() =>
Browser content
} + />, + ); + + try { + const addButton = page.getByRole("button", { name: "Add panel" }); + ((await addButton.element()) as HTMLButtonElement).click(); + await expect.element(page.getByRole("menuitem", { name: "Side" })).toBeVisible(); + expect(page.getByRole("menuitem", { name: "Browser" }).query()).toBeNull(); + } finally { + await screen.unmount(); + } + }); }); diff --git a/apps/web/src/components/chat/RightDock.tsx b/apps/web/src/components/chat/RightDock.tsx index e06149cb6..6182615b8 100644 --- a/apps/web/src/components/chat/RightDock.tsx +++ b/apps/web/src/components/chat/RightDock.tsx @@ -17,7 +17,7 @@ import type { RightDockPaneKind, RightDockThreadState, } from "~/rightDockStore.logic"; -import { resolveActivePane } from "~/rightDockStore.logic"; +import { filterAddableRightDockPaneKinds, resolveActivePane } from "~/rightDockStore.logic"; import { Button } from "../ui/button"; import { IconButton } from "../ui/icon-button"; import { Menu, MenuItem, MenuTrigger } from "../ui/menu"; @@ -120,6 +120,7 @@ function useKeepMountedPaneIds( export function RightDock(props: RightDockProps) { const activePane = resolveActivePane(props.state); + const addablePaneKinds = filterAddableRightDockPaneKinds(props.state, props.addMenuKinds); const onSelectPane = props.onSelectPane; const activePaneRuntimeMode = props.activePaneRuntimeMode ?? "live"; // The dock is the right-most surface when open, so its header sits under the @@ -230,7 +231,7 @@ export function RightDock(props: RightDockProps) { /> ))}
- {props.addMenuKinds.length > 0 ? ( + {addablePaneKinds.length > 0 ? ( - {props.addMenuKinds.map((kind) => { + {addablePaneKinds.map((kind) => { const { Icon, label } = getRightDockPaneMeta(kind); return ( props.onAddPane(kind)}> diff --git a/apps/web/src/rightDockStore.logic.test.ts b/apps/web/src/rightDockStore.logic.test.ts index 6946e7434..9f3c07376 100644 --- a/apps/web/src/rightDockStore.logic.test.ts +++ b/apps/web/src/rightDockStore.logic.test.ts @@ -5,6 +5,7 @@ import { SINGLETON_PANE_KINDS, closePaneInState, createDefaultRightDockState, + filterAddableRightDockPaneKinds, isRightDockPaneKind, openPaneInState, sanitizeRightDockStateByThreadId, @@ -97,6 +98,34 @@ describe("RIGHT_DOCK_PANE_KINDS (single source of truth)", () => { }); }); +describe("filterAddableRightDockPaneKinds", () => { + it("removes existing singleton panels from the Add panel menu", () => { + const state = openPaneInState( + openPaneInState(createDefaultRightDockState(), { + paneId: "browser-1", + kind: "browser", + }), + { paneId: "terminal-1", kind: "terminal" }, + ); + + expect( + filterAddableRightDockPaneKinds(state, ["browser", "diff", "terminal", "sidechat"]), + ).toEqual(["diff", "sidechat"]); + }); + + it("keeps multi-instance panel kinds available", () => { + const state = openPaneInState(createDefaultRightDockState(), { + paneId: "side-1", + kind: "sidechat", + }); + + expect(filterAddableRightDockPaneKinds(state, ["sidechat", "file"])).toEqual([ + "sidechat", + "file", + ]); + }); +}); + describe("isRightDockPaneKind", () => { it("accepts the known pane kinds", () => { for (const kind of [ diff --git a/apps/web/src/rightDockStore.logic.ts b/apps/web/src/rightDockStore.logic.ts index 2a82af1fa..b03f47ce4 100644 --- a/apps/web/src/rightDockStore.logic.ts +++ b/apps/web/src/rightDockStore.logic.ts @@ -61,6 +61,18 @@ export function isSingletonPaneKind(kind: RightDockPaneKind): boolean { return SINGLETON_PANE_KINDS.has(kind); } +// The header menu means "add", not "switch". Hide singleton kinds that already have a +// pane while keeping true multi-instance kinds available for another instance. +export function filterAddableRightDockPaneKinds( + state: RightDockThreadState, + kinds: readonly RightDockPaneKind[], +): RightDockPaneKind[] { + const openSingletonKinds = new Set( + state.panes.filter((pane) => isSingletonPaneKind(pane.kind)).map((pane) => pane.kind), + ); + return kinds.filter((kind) => !isSingletonPaneKind(kind) || !openSingletonKinds.has(kind)); +} + export function createDefaultRightDockState(): RightDockThreadState { return { open: false, From 4ad2910790bc09af86f39d7117435f8584b9b6a3 Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 16:48:30 +0300 Subject: [PATCH 02/16] Keep browser sessions alive under overlays --- .github/workflows/ci.yml | 7 + apps/desktop/package.json | 1 + .../browser-overlay-lifecycle.electron.ts | 161 ++++++++++++++++++ .../scripts/run-browser-overlay-lifecycle.mjs | 85 +++++++++ .../src/components/BrowserPanel.browser.tsx | 86 +++++++++- .../BrowserPanel.overlay.browser.tsx | 37 ++++ .../src/components/BrowserPanel.overlay.ts | 15 +- apps/web/src/components/BrowserPanel.tsx | 119 +++++++++---- package.json | 1 + 9 files changed, 471 insertions(+), 41 deletions(-) create mode 100644 apps/desktop/scripts/browser-overlay-lifecycle.electron.ts create mode 100644 apps/desktop/scripts/run-browser-overlay-lifecycle.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f97742eb0..bc3240616 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -87,6 +87,13 @@ jobs: cd apps/web ./node_modules/.bin/playwright install --with-deps chromium + - name: Electron browser overlay lifecycle + timeout-minutes: 2 + run: | + sudo chown root:root node_modules/electron/dist/chrome-sandbox + sudo chmod 4755 node_modules/electron/dist/chrome-sandbox + xvfb-run -a bun run test:desktop-browser-overlay-lifecycle + # Blocking product-behavior coverage. Linux-only pixel/font/layout # comparisons run separately below while their rendering is quarantined. - name: Browser test (stable) diff --git a/apps/desktop/package.json b/apps/desktop/package.json index 2a5d743b1..d3452ea8e 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -11,6 +11,7 @@ "start": "bun run scripts/start-electron.mjs", "typecheck": "tsc --noEmit", "test": "vitest run --passWithNoTests", + "test:browser-overlay-lifecycle": "bun scripts/run-browser-overlay-lifecycle.mjs", "smoke-test": "node scripts/smoke-test.mjs" }, "dependencies": { diff --git a/apps/desktop/scripts/browser-overlay-lifecycle.electron.ts b/apps/desktop/scripts/browser-overlay-lifecycle.electron.ts new file mode 100644 index 000000000..35c5aec91 --- /dev/null +++ b/apps/desktop/scripts/browser-overlay-lifecycle.electron.ts @@ -0,0 +1,161 @@ +// FILE: browser-overlay-lifecycle.electron.ts +// Purpose: Real-Electron regression for adopted renderer webview overlay lifetime. +// Layer: Desktop integration harness (bundled and launched by the sibling runner). + +import { app, BrowserWindow, type WebContents } from "electron"; +import type { ThreadId } from "@synara/contracts"; + +import { DesktopBrowserManager } from "../src/browserManager"; + +const THREAD_ID = "electron-overlay-lifecycle" as ThreadId; +const OVERLAY_HOLD_MS = 31_250; +const TEST_TIMEOUT_MS = 10_000; + +function invariant(condition: unknown, message: string): asserts condition { + if (!condition) { + throw new Error(message); + } +} + +function withTimeout(promise: Promise, label: string): Promise { + return Promise.race([ + promise, + new Promise((_resolve, reject) => { + setTimeout(() => reject(new Error(`Timed out waiting for ${label}.`)), TEST_TIMEOUT_MS); + }), + ]); +} + +async function main(): Promise { + await app.whenReady(); + + let resolveGuest: ((webContents: WebContents) => void) | null = null; + const guestPromise = new Promise((resolve) => { + resolveGuest = resolve; + }); + app.on("web-contents-created", (_event, webContents) => { + if (webContents.getType() === "webview") { + resolveGuest?.(webContents); + resolveGuest = null; + } + }); + + const hostWindow = new BrowserWindow({ + show: false, + width: 900, + height: 700, + webPreferences: { + contextIsolation: true, + nodeIntegration: false, + sandbox: true, + webviewTag: true, + }, + }); + const manager = new DesktopBrowserManager(); + manager.setWindow(hostWindow); + + try { + await hostWindow.loadURL( + `data:text/html;charset=utf-8,${encodeURIComponent(` + +
+ +
`)}`, + ); + const guest = await withTimeout(guestPromise, "the renderer webview"); + const guestId = guest.id; + + const opened = manager.open({ threadId: THREAD_ID }); + const tabId = opened.activeTabId; + invariant(tabId, "The browser session did not create an active tab."); + manager.setPanelBounds({ + threadId: THREAD_ID, + bounds: { x: 20, y: 30, width: 640, height: 480 }, + surface: "renderer", + }); + manager.attachWebview({ threadId: THREAD_ID, tabId, webContentsId: guestId }); + + const beforeOverlay = manager.getState({ threadId: THREAD_ID }); + invariant(beforeOverlay.activeTabId === tabId, "The adopted tab was not active."); + invariant(beforeOverlay.tabs[0]?.status === "live", "The adopted tab was not live."); + + await hostWindow.webContents.executeJavaScript(`(() => { + const webview = document.querySelector('#browser'); + if (!(webview instanceof HTMLElement)) throw new Error('Missing browser webview'); + webview.style.visibility = 'hidden'; + webview.style.pointerEvents = 'none'; + document.querySelector('#browser-host').style.width = '700px'; + })()`); + + await new Promise((resolve) => setTimeout(resolve, OVERLAY_HOLD_MS)); + + const whileOccluded = manager.getState({ threadId: THREAD_ID }); + invariant(!guest.isDestroyed(), "The adopted renderer webview was destroyed while occluded."); + invariant(whileOccluded.activeTabId === tabId, "The active tab changed while occluded."); + invariant( + whileOccluded.tabs.some((tab) => tab.id === tabId && tab.status === "live"), + "The adopted tab session was suspended while occluded.", + ); + + await hostWindow.webContents.executeJavaScript(`(() => { + const webview = document.querySelector('#browser'); + if (!(webview instanceof HTMLElement)) throw new Error('Missing browser webview'); + webview.style.visibility = 'visible'; + webview.style.pointerEvents = 'auto'; + })()`); + manager.setPanelBounds({ + threadId: THREAD_ID, + bounds: { x: 24, y: 34, width: 700, height: 480 }, + surface: "renderer", + }); + + const recoveredWidth = await hostWindow.webContents.executeJavaScript( + `document.querySelector('#browser').getBoundingClientRect().width`, + ); + const afterOverlay = manager.getState({ threadId: THREAD_ID }); + const internals = manager as unknown as { + runtimes: Map; + suspendTimers: Map>; + }; + const runtime = internals.runtimes.get(`${THREAD_ID}:${tabId}`); + + invariant(recoveredWidth === 700, `Renderer geometry recovered to ${recoveredWidth}, not 700.`); + invariant( + runtime?.webContents.id === guestId, + "A different runtime replaced the adopted webview.", + ); + invariant(runtime.ownsWebContents === false, "The adopted renderer runtime changed ownership."); + invariant(internals.suspendTimers.size === 0, "Overlay occlusion scheduled a thread suspend."); + invariant( + afterOverlay.activeTabId === tabId, + "The tab session did not recover after occlusion.", + ); + + console.log( + JSON.stringify({ + result: "passed", + heldOccludedMs: OVERLAY_HOLD_MS, + adoptedWebContentsId: guestId, + activeTabId: tabId, + recoveredWidth, + }), + ); + } finally { + manager.dispose(); + if (!hostWindow.isDestroyed()) { + hostWindow.destroy(); + } + } +} + +void main().then( + () => app.quit(), + (error) => { + console.error(error instanceof Error ? error.stack : error); + app.exit(1); + }, +); diff --git a/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs b/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs new file mode 100644 index 000000000..df330df80 --- /dev/null +++ b/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs @@ -0,0 +1,85 @@ +// Bundles the focused TypeScript harness into a temporary directory, then launches it +// in the repository's real Electron runtime. No generated test artifact enters the repo. +import { spawn, spawnSync } from "node:child_process"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; + +import { resolveElectronLaunchCommand } from "./electron-launcher.mjs"; + +const scriptDir = dirname(fileURLToPath(import.meta.url)); +const desktopDir = resolve(scriptDir, ".."); +const workspaceRoot = resolve(desktopDir, "../.."); +const tempDir = mkdtempSync(join(tmpdir(), "scient-browser-overlay-lifecycle-")); +const outputPath = join(tempDir, "browser-overlay-lifecycle.cjs"); +const entryPath = join(scriptDir, "browser-overlay-lifecycle.electron.ts"); + +const build = spawnSync( + process.execPath, + [ + "build", + entryPath, + "--target=node", + "--format=cjs", + "--external=electron", + `--outfile=${outputPath}`, + ], + { cwd: workspaceRoot, encoding: "utf8" }, +); +if (build.status !== 0) { + rmSync(tempDir, { recursive: true, force: true }); + process.stderr.write(build.stdout ?? ""); + process.stderr.write(build.stderr ?? ""); + process.exit(build.status ?? 1); +} + +const electronCommand = resolveElectronLaunchCommand(["--disable-gpu", outputPath], { + development: false, +}); +const child = spawn(electronCommand.electronPath, electronCommand.args, { + cwd: workspaceRoot, + detached: process.platform !== "win32", + env: { ...process.env, ELECTRON_ENABLE_LOGGING: "1" }, + stdio: ["ignore", "pipe", "pipe"], +}); + +let output = ""; +child.stdout.on("data", (chunk) => { + output += chunk.toString(); + process.stdout.write(chunk); +}); +child.stderr.on("data", (chunk) => { + output += chunk.toString(); + process.stderr.write(chunk); +}); + +function killChildTree() { + if (!child.pid) return; + if (process.platform === "win32") { + spawnSync("taskkill", ["/pid", String(child.pid), "/t", "/f"], { stdio: "ignore" }); + return; + } + try { + process.kill(-child.pid, "SIGKILL"); + } catch { + child.kill("SIGKILL"); + } +} + +const timeout = setTimeout(() => { + process.stderr.write("Electron browser overlay lifecycle test timed out.\n"); + killChildTree(); +}, 50_000); + +child.on("exit", (code, signal) => { + clearTimeout(timeout); + rmSync(tempDir, { recursive: true, force: true }); + if (code === 0 && output.includes('"result":"passed"')) { + process.exit(0); + } + process.stderr.write( + `Electron browser overlay lifecycle test failed (code=${String(code)}, signal=${String(signal)}).\n`, + ); + process.exit(code && code > 0 ? code : 1); +}); diff --git a/apps/web/src/components/BrowserPanel.browser.tsx b/apps/web/src/components/BrowserPanel.browser.tsx index d235565b1..90707cc58 100644 --- a/apps/web/src/components/BrowserPanel.browser.tsx +++ b/apps/web/src/components/BrowserPanel.browser.tsx @@ -140,6 +140,7 @@ function liveBrowserApi(options?: { attachWebview: vi.fn(async () => openState), detachWebview: vi.fn(async () => undefined), newTab: vi.fn(async () => options?.newTabState ?? openState), + selectTab: vi.fn(async ({ tabId }) => browserState(tabId)), closeTab: vi.fn(async () => openState), onState: vi.fn(() => () => undefined), onCopyLink: vi.fn(() => () => undefined), @@ -278,7 +279,7 @@ describe("BrowserPanel interactions", () => { ); }); await expect.element(page.getByText("New tab", { exact: true })).toBeVisible(); - expect(page.getByRole("button", { name: "Close tab" }).elements()).toHaveLength(2); + expect(page.getByRole("button", { name: /^Close tab:/ }).elements()).toHaveLength(2); }); it("preserves a new-tab click while a sleeping browser pane wakes", async () => { @@ -308,6 +309,47 @@ describe("BrowserPanel interactions", () => { }); }); + it("exposes roving tab semantics and activates adjacent tabs from the keyboard", async () => { + const openState = browserState("tab-1"); + const api = liveBrowserApi({ openState }); + nativeApiTestState.api = api; + useBrowserStateStore.getState().upsertThreadState(openState); + + await renderLivePanel(vi.fn()); + const tablist = page.getByRole("tablist", { name: "Browser tabs" }); + await expect.element(tablist).toBeVisible(); + const firstTab = page.getByRole("tab", { name: "ScientFactory" }); + const secondTab = page.getByRole("tab", { name: "Example" }); + await expect.element(firstTab).toHaveAttribute("aria-selected", "true"); + await expect.element(firstTab).toHaveAttribute("tabindex", "0"); + await expect.element(secondTab).toHaveAttribute("aria-selected", "false"); + await expect.element(secondTab).toHaveAttribute("tabindex", "-1"); + + const secondTabElement = (await secondTab.element()) as HTMLButtonElement; + ((await firstTab.element()) as HTMLButtonElement).focus(); + await userEvent.keyboard("{ArrowRight}"); + + await vi.waitFor(() => { + expect(api.browser.selectTab).toHaveBeenCalledWith({ + threadId: THREAD_ID, + tabId: "tab-2", + }); + expect(document.activeElement).toBe(secondTabElement); + }); + await expect.element(secondTab).toHaveAttribute("aria-selected", "true"); + await expect + .element(page.getByRole("tabpanel")) + .toHaveAttribute("aria-labelledby", secondTabElement.id); + + await userEvent.keyboard("{Delete}"); + await vi.waitFor(() => { + expect(api.browser.closeTab).toHaveBeenCalledWith({ + threadId: THREAD_ID, + tabId: "tab-2", + }); + }); + }); + it("hides the native browser surface while an intersecting app menu is open", async () => { const openState = browserState("tab-1"); const api = liveBrowserApi({ openState }); @@ -321,6 +363,7 @@ describe("BrowserPanel interactions", () => { expect(webview?.style.visibility).not.toBe("hidden"); }); + const initialBoundsCalls = vi.mocked(api.browser.setPanelBounds).mock.calls.length; ( (await page.getByRole("button", { name: "Browser actions" }).element()) as HTMLButtonElement ).click(); @@ -329,19 +372,50 @@ describe("BrowserPanel interactions", () => { const webview = document.querySelector("webview"); expect(webview?.style.visibility).toBe("hidden"); expect(webview?.style.pointerEvents).toBe("none"); - expect(api.browser.setPanelBounds).toHaveBeenCalledWith({ - threadId: THREAD_ID, - bounds: null, - surface: "renderer", - }); + expect(vi.mocked(api.browser.setPanelBounds).mock.calls.length).toBe(initialBoundsCalls); + expect(vi.mocked(api.browser.setPanelBounds).mock.calls).not.toContainEqual([ + { threadId: THREAD_ID, bounds: null, surface: "renderer" }, + ]); }); + const browserViewport = document.querySelector("webview")?.parentElement; + expect(browserViewport).not.toBeNull(); + Object.assign(browserViewport!.style, { width: "500px", right: "auto" }); + await userEvent.keyboard("{Escape}"); await vi.waitFor(() => { expect(page.getByRole("menuitem", { name: "New tab" }).query()).toBeNull(); const webview = document.querySelector("webview"); expect(webview?.style.visibility).toBe("visible"); expect(webview?.style.pointerEvents).toBe("auto"); + const latestBoundsCall = vi.mocked(api.browser.setPanelBounds).mock.calls.at(-1)?.[0]; + expect(latestBoundsCall?.bounds).toMatchObject({ width: 500 }); + }); + }); + + it("reserves null bounds for the local home that genuinely hides the page surface", async () => { + const openState = browserState("tab-1"); + openState.version = 50; + openState.tabs = [ + { + ...openState.tabs[0]!, + url: "about:blank", + lastCommittedUrl: "about:blank", + title: "New tab", + }, + ]; + const api = liveBrowserApi({ openState }); + nativeApiTestState.api = api; + useBrowserStateStore.getState().upsertThreadState(openState); + + await renderLivePanel(vi.fn()); + + await vi.waitFor(() => { + expect(api.browser.setPanelBounds).toHaveBeenCalledWith({ + threadId: THREAD_ID, + bounds: null, + surface: "renderer", + }); }); }); }); diff --git a/apps/web/src/components/BrowserPanel.overlay.browser.tsx b/apps/web/src/components/BrowserPanel.overlay.browser.tsx index 8fd7862ba..e70e680aa 100644 --- a/apps/web/src/components/BrowserPanel.overlay.browser.tsx +++ b/apps/web/src/components/BrowserPanel.overlay.browser.tsx @@ -72,4 +72,41 @@ describe("native browser overlay coordination", () => { popup.style.visibility = "hidden"; expect(hasNativeBrowserObscuringOverlay(viewport)).toBe(false); }); + + it.each(["autocomplete-popup", "preview-card-popup", "tooltip-popup", "sheet-popup"])( + "recognizes an intersecting %s surface", + (slot) => { + const viewport = document.createElement("div"); + Object.assign(viewport.style, { + position: "fixed", + left: "100px", + top: "100px", + width: "240px", + height: "240px", + }); + const overlay = document.createElement("div"); + overlay.dataset.slot = slot; + Object.assign(overlay.style, { + position: "fixed", + left: "160px", + top: "160px", + width: "120px", + height: "120px", + }); + document.body.append(viewport, overlay); + + expect(hasNativeBrowserObscuringOverlay(viewport)).toBe(true); + }, + ); + + it("does not treat the browser's own containing sheet as an obstruction", () => { + const sheet = document.createElement("div"); + sheet.dataset.slot = "sheet-popup"; + const viewport = document.createElement("div"); + Object.assign(viewport.style, { width: "240px", height: "240px" }); + sheet.append(viewport); + document.body.append(sheet); + + expect(hasNativeBrowserObscuringOverlay(viewport)).toBe(false); + }); }); diff --git a/apps/web/src/components/BrowserPanel.overlay.ts b/apps/web/src/components/BrowserPanel.overlay.ts index 96581d286..0b6bd33c2 100644 --- a/apps/web/src/components/BrowserPanel.overlay.ts +++ b/apps/web/src/components/BrowserPanel.overlay.ts @@ -9,12 +9,21 @@ const NATIVE_BROWSER_OBSCURING_OVERLAY_SELECTOR = [ "[data-slot='menu-positioner']", "[data-slot='menu-popup']", "[data-slot='menu-sub-content']", + "[data-slot='autocomplete-positioner']", + "[data-slot='autocomplete-popup']", "[data-slot='select-positioner']", "[data-slot='select-popup']", "[data-slot='combobox-positioner']", "[data-slot='combobox-popup']", "[data-slot='popover-positioner']", "[data-slot='popover-popup']", + "[data-slot='preview-card-positioner']", + "[data-slot='preview-card-popup']", + "[data-slot='tooltip-positioner']", + "[data-slot='tooltip-popup']", + "[data-slot='sheet-backdrop']", + "[data-slot='sheet-viewport']", + "[data-slot='sheet-popup']", "[data-slot='dialog-backdrop']", "[data-slot='dialog-popup']", "[data-slot='dialog-viewport']", @@ -28,12 +37,10 @@ const NATIVE_BROWSER_OBSCURING_OVERLAY_SELECTOR = [ "[role='dialog'][aria-modal='true']", ].join(", "); -// The browser itself lives inside a sheet, and toast portals/positioners are just -// layout containers. Treating either as blockers hides the native surface unnecessarily. +// Toast portals/positioners are layout containers. The actual toast popup remains a +// blocker, but treating its portal as one would hide the browser across the whole app. const NATIVE_BROWSER_NON_OBSCURING_OVERLAY_SELECTOR = [ "[data-panel-resize-overlay='true']", - "[data-slot='sheet-backdrop']", - "[data-slot='sheet-popup']", "[data-slot='toast-portal']", "[data-slot='toast-portal-anchored']", "[data-slot='toast-viewport']", diff --git a/apps/web/src/components/BrowserPanel.tsx b/apps/web/src/components/BrowserPanel.tsx index 925e94c5d..a78ab4a06 100644 --- a/apps/web/src/components/BrowserPanel.tsx +++ b/apps/web/src/components/BrowserPanel.tsx @@ -6,7 +6,7 @@ // Note: raw
@@ -1638,7 +1688,14 @@ export function BrowserPanel({
) : null} -
+
= 0 ? `${browserTabsId}-tab-${activeTabIndex}` : undefined + } + > {!isLiveRuntime ? ( Date: Fri, 24 Jul 2026 17:16:56 +0300 Subject: [PATCH 03/16] Harden browser overlay lifecycle coverage --- .../browser-overlay-lifecycle.electron.ts | 106 +++++++++----- .../browser-overlay-lifecycle.preload.ts | 11 ++ .../browser-overlay-lifecycle.renderer.ts | 63 ++++++++ .../scripts/run-browser-overlay-lifecycle.mjs | 138 ++++++++++++++---- .../src/components/BrowserPanel.browser.tsx | 55 ++++++- .../BrowserPanel.overlay.browser.tsx | 23 ++- .../src/components/BrowserPanel.overlay.ts | 26 ++++ apps/web/src/components/BrowserPanel.tsx | 38 +++-- .../web/src/components/RecentViewSwitcher.tsx | 5 +- apps/web/src/components/ui/sheet.tsx | 10 +- 10 files changed, 389 insertions(+), 86 deletions(-) create mode 100644 apps/desktop/scripts/browser-overlay-lifecycle.preload.ts create mode 100644 apps/desktop/scripts/browser-overlay-lifecycle.renderer.ts diff --git a/apps/desktop/scripts/browser-overlay-lifecycle.electron.ts b/apps/desktop/scripts/browser-overlay-lifecycle.electron.ts index 35c5aec91..fb792a009 100644 --- a/apps/desktop/scripts/browser-overlay-lifecycle.electron.ts +++ b/apps/desktop/scripts/browser-overlay-lifecycle.electron.ts @@ -2,14 +2,17 @@ // Purpose: Real-Electron regression for adopted renderer webview overlay lifetime. // Layer: Desktop integration harness (bundled and launched by the sibling runner). -import { app, BrowserWindow, type WebContents } from "electron"; +import { join } from "node:path"; + import type { ThreadId } from "@synara/contracts"; +import { app, BrowserWindow, ipcMain, type WebContents } from "electron"; import { DesktopBrowserManager } from "../src/browserManager"; const THREAD_ID = "electron-overlay-lifecycle" as ThreadId; const OVERLAY_HOLD_MS = 31_250; const TEST_TIMEOUT_MS = 10_000; +const BOUNDS_CHANNEL = "scient:test:browser-overlay:set-bounds"; function invariant(condition: unknown, message: string): asserts condition { if (!condition) { @@ -26,7 +29,32 @@ function withTimeout(promise: Promise, label: string): Promise { ]); } +async function waitForRendererHarness(hostWindow: BrowserWindow): Promise { + await withTimeout( + (async () => { + while (!hostWindow.isDestroyed()) { + const ready = await hostWindow.webContents.executeJavaScript( + `Boolean(window.scientBrowserOverlayLifecycle?.ready)`, + ); + if (ready) return; + await new Promise((resolve) => setTimeout(resolve, 25)); + } + throw new Error("The host window was destroyed before its renderer harness loaded."); + })(), + "the renderer coordination harness", + ); +} + async function main(): Promise { + const profileDir = process.env.SCIENT_BROWSER_OVERLAY_TEST_PROFILE; + const fixturePath = process.env.SCIENT_BROWSER_OVERLAY_TEST_FIXTURE; + const preloadPath = process.env.SCIENT_BROWSER_OVERLAY_TEST_PRELOAD; + invariant(profileDir, "Missing isolated Electron profile path."); + invariant(fixturePath, "Missing renderer fixture path."); + invariant(preloadPath, "Missing preload fixture path."); + app.setPath("userData", profileDir); + app.setPath("sessionData", join(profileDir, "session-data")); + await app.whenReady(); let resolveGuest: ((webContents: WebContents) => void) | null = null; @@ -47,49 +75,51 @@ async function main(): Promise { webPreferences: { contextIsolation: true, nodeIntegration: false, + preload: preloadPath, sandbox: true, webviewTag: true, }, }); const manager = new DesktopBrowserManager(); manager.setWindow(hostWindow); + const boundsEvents: Array<{ width: number; height: number } | null> = []; + ipcMain.handle(BOUNDS_CHANNEL, (_event, bounds: { width: number; height: number } | null) => { + boundsEvents.push(bounds); + manager.setPanelBounds({ + threadId: THREAD_ID, + bounds: bounds ? { x: 20, y: 30, width: bounds.width, height: bounds.height } : null, + surface: "renderer", + }); + }); try { - await hostWindow.loadURL( - `data:text/html;charset=utf-8,${encodeURIComponent(` - -
- -
`)}`, - ); + await hostWindow.loadFile(fixturePath); + await waitForRendererHarness(hostWindow); const guest = await withTimeout(guestPromise, "the renderer webview"); const guestId = guest.id; const opened = manager.open({ threadId: THREAD_ID }); const tabId = opened.activeTabId; invariant(tabId, "The browser session did not create an active tab."); - manager.setPanelBounds({ - threadId: THREAD_ID, - bounds: { x: 20, y: 30, width: 640, height: 480 }, - surface: "renderer", - }); manager.attachWebview({ threadId: THREAD_ID, tabId, webContentsId: guestId }); + const initialMode = await hostWindow.webContents.executeJavaScript( + `window.scientBrowserOverlayLifecycle.syncBounds()`, + ); + invariant(initialMode === "send", `Initial renderer bounds mode was ${String(initialMode)}.`); const beforeOverlay = manager.getState({ threadId: THREAD_ID }); invariant(beforeOverlay.activeTabId === tabId, "The adopted tab was not active."); invariant(beforeOverlay.tabs[0]?.status === "live", "The adopted tab was not live."); + const boundsEventsBeforeOverlay = boundsEvents.length; - await hostWindow.webContents.executeJavaScript(`(() => { - const webview = document.querySelector('#browser'); - if (!(webview instanceof HTMLElement)) throw new Error('Missing browser webview'); - webview.style.visibility = 'hidden'; - webview.style.pointerEvents = 'none'; - document.querySelector('#browser-host').style.width = '700px'; - })()`); + const openMode = await hostWindow.webContents.executeJavaScript( + `window.scientBrowserOverlayLifecycle.openOverlay()`, + ); + invariant(openMode === "suppress", `Overlay renderer bounds mode was ${String(openMode)}.`); + invariant( + boundsEvents.length === boundsEventsBeforeOverlay, + "Opening the overlay sent a lifecycle-changing bounds event.", + ); await new Promise((resolve) => setTimeout(resolve, OVERLAY_HOLD_MS)); @@ -100,21 +130,18 @@ async function main(): Promise { whileOccluded.tabs.some((tab) => tab.id === tabId && tab.status === "live"), "The adopted tab session was suspended while occluded.", ); + invariant( + !boundsEvents.slice(boundsEventsBeforeOverlay).includes(null), + "Overlay occlusion sent null bounds and started the hide lifecycle.", + ); - await hostWindow.webContents.executeJavaScript(`(() => { - const webview = document.querySelector('#browser'); - if (!(webview instanceof HTMLElement)) throw new Error('Missing browser webview'); - webview.style.visibility = 'visible'; - webview.style.pointerEvents = 'auto'; - })()`); - manager.setPanelBounds({ - threadId: THREAD_ID, - bounds: { x: 24, y: 34, width: 700, height: 480 }, - surface: "renderer", - }); + const closeMode = await hostWindow.webContents.executeJavaScript( + `window.scientBrowserOverlayLifecycle.closeOverlay()`, + ); + invariant(closeMode === "send", `Recovered renderer bounds mode was ${String(closeMode)}.`); const recoveredWidth = await hostWindow.webContents.executeJavaScript( - `document.querySelector('#browser').getBoundingClientRect().width`, + `document.querySelector('#browser-host').getBoundingClientRect().width`, ); const afterOverlay = manager.getState({ threadId: THREAD_ID }); const internals = manager as unknown as { @@ -124,6 +151,10 @@ async function main(): Promise { const runtime = internals.runtimes.get(`${THREAD_ID}:${tabId}`); invariant(recoveredWidth === 700, `Renderer geometry recovered to ${recoveredWidth}, not 700.`); + invariant( + boundsEvents.at(-1)?.width === 700, + "The renderer did not send recovered geometry after the overlay closed.", + ); invariant( runtime?.webContents.id === guestId, "A different runtime replaced the adopted webview.", @@ -141,10 +172,13 @@ async function main(): Promise { heldOccludedMs: OVERLAY_HOLD_MS, adoptedWebContentsId: guestId, activeTabId: tabId, + openMode, + closeMode, recoveredWidth, }), ); } finally { + ipcMain.removeHandler(BOUNDS_CHANNEL); manager.dispose(); if (!hostWindow.isDestroyed()) { hostWindow.destroy(); diff --git a/apps/desktop/scripts/browser-overlay-lifecycle.preload.ts b/apps/desktop/scripts/browser-overlay-lifecycle.preload.ts new file mode 100644 index 000000000..3be4fe3ea --- /dev/null +++ b/apps/desktop/scripts/browser-overlay-lifecycle.preload.ts @@ -0,0 +1,11 @@ +// FILE: browser-overlay-lifecycle.preload.ts +// Purpose: Minimal isolated bridge for the Electron overlay lifecycle regression. + +import { contextBridge, ipcRenderer } from "electron"; + +const BOUNDS_CHANNEL = "scient:test:browser-overlay:set-bounds"; + +contextBridge.exposeInMainWorld("scientBrowserOverlayTestApi", { + setPanelBounds: (bounds: { width: number; height: number } | null) => + ipcRenderer.invoke(BOUNDS_CHANNEL, bounds), +}); diff --git a/apps/desktop/scripts/browser-overlay-lifecycle.renderer.ts b/apps/desktop/scripts/browser-overlay-lifecycle.renderer.ts new file mode 100644 index 000000000..eb853a474 --- /dev/null +++ b/apps/desktop/scripts/browser-overlay-lifecycle.renderer.ts @@ -0,0 +1,63 @@ +// FILE: browser-overlay-lifecycle.renderer.ts +// Purpose: Exercise the production overlay classifier and bounds decision in a real renderer. + +import { + hasNativeBrowserObscuringOverlay, + resolveNativeBrowserBoundsSyncMode, + setBrowserWebviewOverlayOcclusion, + type BrowserWebviewElement, +} from "../../web/src/components/BrowserPanel.overlay"; + +declare global { + interface Window { + scientBrowserOverlayTestApi: { + setPanelBounds(bounds: { width: number; height: number } | null): Promise; + }; + scientBrowserOverlayLifecycle: { + ready: true; + syncBounds(): Promise; + openOverlay(): Promise; + closeOverlay(): Promise; + }; + } +} + +const host = document.querySelector("#browser-host"); +const webview = document.querySelector("#browser"); +if (!host || !webview) { + throw new Error("Missing browser overlay lifecycle fixture elements."); +} + +async function syncBounds(): Promise { + const obscuredByOverlay = hasNativeBrowserObscuringOverlay(host); + setBrowserWebviewOverlayOcclusion(webview, obscuredByOverlay); + const rect = host.getBoundingClientRect(); + const mode = resolveNativeBrowserBoundsSyncMode({ + obscuredByOverlay, + paneIsActuallyHidden: rect.width <= 0 || rect.height <= 0, + }); + if (mode === "suppress") { + return mode; + } + await window.scientBrowserOverlayTestApi.setPanelBounds( + mode === "hide" ? null : { width: rect.width, height: rect.height }, + ); + return mode; +} + +window.scientBrowserOverlayLifecycle = { + ready: true, + syncBounds, + async openOverlay() { + const overlay = document.createElement("div"); + overlay.id = "test-overlay"; + overlay.dataset.nativeBrowserOverlay = "true"; + document.body.append(overlay); + host.style.width = "700px"; + return syncBounds(); + }, + async closeOverlay() { + document.querySelector("#test-overlay")?.remove(); + return syncBounds(); + }, +}; diff --git a/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs b/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs index df330df80..1a96ab9f6 100644 --- a/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs +++ b/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs @@ -1,10 +1,10 @@ -// Bundles the focused TypeScript harness into a temporary directory, then launches it -// in the repository's real Electron runtime. No generated test artifact enters the repo. +// Bundles a hermetic Electron fixture, launches it with an isolated profile, and always +// tears down the detached process group and temporary state on completion or interruption. import { spawn, spawnSync } from "node:child_process"; -import { mkdtempSync, rmSync } from "node:fs"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { dirname, join, resolve } from "node:path"; -import { fileURLToPath } from "node:url"; +import { pathToFileURL, fileURLToPath } from "node:url"; import { resolveElectronLaunchCommand } from "./electron-launcher.mjs"; @@ -12,39 +12,79 @@ const scriptDir = dirname(fileURLToPath(import.meta.url)); const desktopDir = resolve(scriptDir, ".."); const workspaceRoot = resolve(desktopDir, "../.."); const tempDir = mkdtempSync(join(tmpdir(), "scient-browser-overlay-lifecycle-")); -const outputPath = join(tempDir, "browser-overlay-lifecycle.cjs"); -const entryPath = join(scriptDir, "browser-overlay-lifecycle.electron.ts"); +const profileDir = join(tempDir, "profile"); +const electronOutputPath = join(tempDir, "browser-overlay-lifecycle.cjs"); +const preloadOutputPath = join(tempDir, "browser-overlay-lifecycle.preload.cjs"); +const rendererOutputPath = join(tempDir, "browser-overlay-lifecycle.renderer.js"); +const fixturePath = join(tempDir, "browser-overlay-lifecycle.html"); -const build = spawnSync( - process.execPath, - [ - "build", - entryPath, - "--target=node", - "--format=cjs", - "--external=electron", - `--outfile=${outputPath}`, - ], - { cwd: workspaceRoot, encoding: "utf8" }, -); -if (build.status !== 0) { - rmSync(tempDir, { recursive: true, force: true }); - process.stderr.write(build.stdout ?? ""); - process.stderr.write(build.stderr ?? ""); - process.exit(build.status ?? 1); +const builds = [ + { + entry: join(scriptDir, "browser-overlay-lifecycle.electron.ts"), + output: electronOutputPath, + args: ["--target=node", "--format=cjs", "--external=electron"], + }, + { + entry: join(scriptDir, "browser-overlay-lifecycle.preload.ts"), + output: preloadOutputPath, + args: ["--target=node", "--format=cjs", "--external=electron"], + }, + { + entry: join(scriptDir, "browser-overlay-lifecycle.renderer.ts"), + output: rendererOutputPath, + args: ["--target=browser", "--format=iife"], + }, +]; + +for (const build of builds) { + const result = spawnSync( + process.execPath, + ["build", build.entry, ...build.args, `--outfile=${build.output}`], + { cwd: workspaceRoot, encoding: "utf8" }, + ); + if (result.status !== 0) { + rmSync(tempDir, { recursive: true, force: true }); + process.stderr.write(result.stdout ?? ""); + process.stderr.write(result.stderr ?? ""); + process.exit(result.status ?? 1); + } } -const electronCommand = resolveElectronLaunchCommand(["--disable-gpu", outputPath], { +writeFileSync( + fixturePath, + ` + +
+ +
+ `, + "utf8", +); + +const electronCommand = resolveElectronLaunchCommand(["--disable-gpu", electronOutputPath], { development: false, }); const child = spawn(electronCommand.electronPath, electronCommand.args, { cwd: workspaceRoot, detached: process.platform !== "win32", - env: { ...process.env, ELECTRON_ENABLE_LOGGING: "1" }, + env: { + ...process.env, + ELECTRON_ENABLE_LOGGING: "1", + SCIENT_BROWSER_OVERLAY_TEST_FIXTURE: fixturePath, + SCIENT_BROWSER_OVERLAY_TEST_PRELOAD: preloadOutputPath, + SCIENT_BROWSER_OVERLAY_TEST_PROFILE: profileDir, + }, stdio: ["ignore", "pipe", "pipe"], }); let output = ""; +let finished = false; +let requestedExitCode = null; child.stdout.on("data", (chunk) => { output += chunk.toString(); process.stdout.write(chunk); @@ -55,7 +95,7 @@ child.stderr.on("data", (chunk) => { }); function killChildTree() { - if (!child.pid) return; + if (!child.pid || child.exitCode !== null || child.signalCode !== null) return; if (process.platform === "win32") { spawnSync("taskkill", ["/pid", String(child.pid), "/t", "/f"], { stdio: "ignore" }); return; @@ -67,19 +107,57 @@ function killChildTree() { } } +function finish(code) { + if (finished) return; + finished = true; + clearTimeout(timeout); + rmSync(tempDir, { recursive: true, force: true }); + process.exit(code); +} + +function interrupt(signal, code) { + if (finished) return; + requestedExitCode = code; + process.stderr.write(`Electron browser overlay lifecycle test interrupted by ${signal}.\n`); + killChildTree(); + setTimeout(() => finish(code), 2_000).unref(); +} + +process.once("SIGINT", () => interrupt("SIGINT", 130)); +process.once("SIGTERM", () => interrupt("SIGTERM", 143)); +process.once("uncaughtException", (error) => { + process.stderr.write(`${error instanceof Error ? error.stack : String(error)}\n`); + interrupt("uncaughtException", 1); +}); +process.once("unhandledRejection", (error) => { + process.stderr.write(`${error instanceof Error ? error.stack : String(error)}\n`); + interrupt("unhandledRejection", 1); +}); + const timeout = setTimeout(() => { + requestedExitCode = 1; process.stderr.write("Electron browser overlay lifecycle test timed out.\n"); killChildTree(); }, 50_000); +child.on("error", (error) => { + process.stderr.write(`${error.stack ?? error.message}\n`); + requestedExitCode = 1; + killChildTree(); + finish(1); +}); + child.on("exit", (code, signal) => { - clearTimeout(timeout); - rmSync(tempDir, { recursive: true, force: true }); + if (requestedExitCode !== null) { + finish(requestedExitCode); + return; + } if (code === 0 && output.includes('"result":"passed"')) { - process.exit(0); + finish(0); + return; } process.stderr.write( `Electron browser overlay lifecycle test failed (code=${String(code)}, signal=${String(signal)}).\n`, ); - process.exit(code && code > 0 ? code : 1); + finish(code && code > 0 ? code : 1); }); diff --git a/apps/web/src/components/BrowserPanel.browser.tsx b/apps/web/src/components/BrowserPanel.browser.tsx index 90707cc58..dab8fd866 100644 --- a/apps/web/src/components/BrowserPanel.browser.tsx +++ b/apps/web/src/components/BrowserPanel.browser.tsx @@ -30,6 +30,7 @@ vi.mock("~/nativeApi", async (importOriginal) => ({ import { useBrowserStateStore } from "../browserStateStore"; import { BrowserPanel } from "./BrowserPanel"; +import { RecentViewSwitcher } from "./RecentViewSwitcher"; const THREAD_ID = "thread-browser-copy" as ThreadId; @@ -87,7 +88,7 @@ function renderPanel() { ); } -function renderLivePanel(onClosePanel: () => void) { +function renderLivePanel(onClosePanel: () => void, options?: { showRecentViews?: boolean }) { const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); return render( @@ -98,6 +99,25 @@ function renderLivePanel(onClosePanel: () => void) { runtimeMode="live" onClosePanel={onClosePanel} /> + {options?.showRecentViews ? ( + + ) : null}
, ); @@ -130,6 +150,7 @@ function renderPreviewToLivePanel() { function liveBrowserApi(options?: { openState?: ThreadBrowserState; newTabState?: ThreadBrowserState; + closeTabState?: ThreadBrowserState; }) { const openState = options?.openState ?? browserState("tab-1"); return { @@ -141,7 +162,7 @@ function liveBrowserApi(options?: { detachWebview: vi.fn(async () => undefined), newTab: vi.fn(async () => options?.newTabState ?? openState), selectTab: vi.fn(async ({ tabId }) => browserState(tabId)), - closeTab: vi.fn(async () => openState), + closeTab: vi.fn(async () => options?.closeTabState ?? openState), onState: vi.fn(() => () => undefined), onCopyLink: vi.fn(() => () => undefined), }, @@ -311,7 +332,13 @@ describe("BrowserPanel interactions", () => { it("exposes roving tab semantics and activates adjacent tabs from the keyboard", async () => { const openState = browserState("tab-1"); - const api = liveBrowserApi({ openState }); + const closeTabState: ThreadBrowserState = { + ...openState, + version: openState.version + 1, + activeTabId: "tab-1", + tabs: [openState.tabs[0]!], + }; + const api = liveBrowserApi({ openState, closeTabState }); nativeApiTestState.api = api; useBrowserStateStore.getState().upsertThreadState(openState); @@ -324,9 +351,12 @@ describe("BrowserPanel interactions", () => { await expect.element(firstTab).toHaveAttribute("tabindex", "0"); await expect.element(secondTab).toHaveAttribute("aria-selected", "false"); await expect.element(secondTab).toHaveAttribute("tabindex", "-1"); + const tablistElement = (await tablist.element()) as HTMLElement; + expect(tablistElement.querySelectorAll('[tabindex="0"]')).toHaveLength(1); + const firstTabElement = (await firstTab.element()) as HTMLButtonElement; const secondTabElement = (await secondTab.element()) as HTMLButtonElement; - ((await firstTab.element()) as HTMLButtonElement).focus(); + firstTabElement.focus(); await userEvent.keyboard("{ArrowRight}"); await vi.waitFor(() => { @@ -347,6 +377,23 @@ describe("BrowserPanel interactions", () => { threadId: THREAD_ID, tabId: "tab-2", }); + expect(document.activeElement).toBe(firstTabElement); + }); + }); + + it("occludes the native surface for the real recent-view switcher", async () => { + const openState = browserState("tab-1"); + const api = liveBrowserApi({ openState }); + nativeApiTestState.api = api; + + await renderLivePanel(vi.fn(), { showRecentViews: true }); + await expect.element(page.getByRole("listbox", { name: "Recent views" })).toBeVisible(); + await vi.waitFor(() => { + const webview = document.querySelector("webview"); + expect(webview?.style.visibility).toBe("hidden"); + expect(vi.mocked(api.browser.setPanelBounds).mock.calls).not.toContainEqual([ + { threadId: THREAD_ID, bounds: null, surface: "renderer" }, + ]); }); }); diff --git a/apps/web/src/components/BrowserPanel.overlay.browser.tsx b/apps/web/src/components/BrowserPanel.overlay.browser.tsx index e70e680aa..4c72f9494 100644 --- a/apps/web/src/components/BrowserPanel.overlay.browser.tsx +++ b/apps/web/src/components/BrowserPanel.overlay.browser.tsx @@ -100,13 +100,32 @@ describe("native browser overlay coordination", () => { ); it("does not treat the browser's own containing sheet as an obstruction", () => { + const backdrop = document.createElement("div"); + backdrop.dataset.slot = "sheet-backdrop"; + backdrop.dataset.nativeBrowserOverlayOwner = "owning-sheet"; + Object.assign(backdrop.style, { position: "fixed", inset: "0" }); + const sheetViewport = document.createElement("div"); + sheetViewport.dataset.slot = "sheet-viewport"; + sheetViewport.dataset.nativeBrowserOverlayOwner = "owning-sheet"; const sheet = document.createElement("div"); sheet.dataset.slot = "sheet-popup"; const viewport = document.createElement("div"); - Object.assign(viewport.style, { width: "240px", height: "240px" }); + Object.assign(viewport.style, { + position: "fixed", + left: "100px", + top: "100px", + width: "240px", + height: "240px", + }); sheet.append(viewport); - document.body.append(sheet); + sheetViewport.append(sheet); + document.body.append(backdrop, sheetViewport); expect(hasNativeBrowserObscuringOverlay(viewport)).toBe(false); + + const otherBackdrop = backdrop.cloneNode() as HTMLElement; + otherBackdrop.dataset.nativeBrowserOverlayOwner = "other-sheet"; + document.body.append(otherBackdrop); + expect(hasNativeBrowserObscuringOverlay(viewport)).toBe(true); }); }); diff --git a/apps/web/src/components/BrowserPanel.overlay.ts b/apps/web/src/components/BrowserPanel.overlay.ts index 0b6bd33c2..e74ef88a6 100644 --- a/apps/web/src/components/BrowserPanel.overlay.ts +++ b/apps/web/src/components/BrowserPanel.overlay.ts @@ -60,6 +60,18 @@ export interface BrowserWebviewElement extends HTMLElement { getWebContentsId?: () => number; } +export type NativeBrowserBoundsSyncMode = "send" | "hide" | "suppress"; + +export function resolveNativeBrowserBoundsSyncMode(options: { + obscuredByOverlay: boolean; + paneIsActuallyHidden: boolean; +}): NativeBrowserBoundsSyncMode { + if (options.paneIsActuallyHidden) { + return "hide"; + } + return options.obscuredByOverlay ? "suppress" : "send"; +} + export function setBrowserWebviewOverlayOcclusion( webview: BrowserWebviewElement | null, occluded: boolean, @@ -90,10 +102,21 @@ function rectsIntersect(a: DOMRect, b: DOMRect): boolean { return a.left < b.right && a.right > b.left && a.top < b.bottom && a.bottom > b.top; } +function sharesNativeBrowserOverlayOwner(candidate: HTMLElement, element: HTMLElement): boolean { + const candidateOwner = candidate.getAttribute("data-native-browser-overlay-owner"); + const elementOwner = element + .closest("[data-native-browser-overlay-owner]") + ?.getAttribute("data-native-browser-overlay-owner"); + return Boolean(candidateOwner && candidateOwner === elementOwner); +} + function candidateObscuresNativeBrowser(candidate: HTMLElement, element: HTMLElement): boolean { if (candidate === element || candidate.contains(element) || element.contains(candidate)) { return false; } + if (sharesNativeBrowserOverlayOwner(candidate, element)) { + return false; + } if (!isVisibleOverlayElement(candidate)) { return false; } @@ -128,6 +151,9 @@ function hasTopLayerDomObstruction(element: HTMLElement): boolean { if (hitElement === element || element.contains(hitElement) || hitElement.contains(element)) { continue; } + if (sharesNativeBrowserOverlayOwner(hitElement, element)) { + continue; + } if (isNativeBrowserNonObscuringOverlayElement(hitElement)) { continue; } diff --git a/apps/web/src/components/BrowserPanel.tsx b/apps/web/src/components/BrowserPanel.tsx index a78ab4a06..46900c833 100644 --- a/apps/web/src/components/BrowserPanel.tsx +++ b/apps/web/src/components/BrowserPanel.tsx @@ -73,6 +73,7 @@ import { hasNativeBrowserObscuringOverlay, isNativeBrowserTransitionSignalTarget, nativeBrowserOverlayMutationsRequireSync, + resolveNativeBrowserBoundsSyncMode, setBrowserWebviewOverlayOcclusion, } from "./BrowserPanel.overlay"; import { DiffPanelLoadingState, DiffPanelShell, type DiffPanelMode } from "./DiffPanelShell"; @@ -740,24 +741,29 @@ export function BrowserPanel({ setBrowserWebviewOverlayOcclusion(browserWebviewRef.current, webviewOccluded); const rect = element.getBoundingClientRect(); const paneIsActuallyHidden = showLocalServersHome || rect.width <= 0 || rect.height <= 0; + const boundsSyncMode = resolveNativeBrowserBoundsSyncMode({ + obscuredByOverlay, + paneIsActuallyHidden, + }); // App-owned menus, dialogs, and other transient overlays only occlude the renderer // surface. Sending null bounds here tells main that the pane itself is hidden, which // starts the 30-second runtime suspension timer and can destroy the adopted webview // while its DOM node remains mounted. Keep the last native geometry/lifecycle active; // a close mutation or resize will send the current real bounds when the overlay leaves. - if (obscuredByOverlay && !paneIsActuallyHidden) { + if (boundsSyncMode === "suppress") { lastMeasuredBoundsKeyRef.current = "renderer:overlay-occluded"; perfCountersRef.current.syncSkips += 1; return; } - const bounds = paneIsActuallyHidden - ? null - : { - x: rect.left, - y: rect.top, - width: rect.width, - height: rect.height, - }; + const bounds = + boundsSyncMode === "hide" + ? null + : { + x: rect.left, + y: rect.top, + width: rect.width, + height: rect.height, + }; const nextKey = bounds ? `renderer:${Math.round(bounds.x)}:${Math.round(bounds.y)}:${Math.round(bounds.width)}:${Math.round(bounds.height)}` : "renderer:hidden"; @@ -1256,7 +1262,7 @@ export function BrowserPanel({ }, [copyFeedback]); const onCloseTab = useCallback( - (tabId: string) => { + (tabId: string, options?: { restoreTabFocus?: boolean }) => { if (!ensureLiveRuntime()) { return; } @@ -1276,6 +1282,14 @@ export function BrowserPanel({ return; } upsertThreadState(state); + if (options?.restoreTabFocus && state.activeTabId) { + const nextActiveIndex = state.tabs.findIndex((tab) => tab.id === state.activeTabId); + if (nextActiveIndex >= 0) { + window.requestAnimationFrame(() => { + document.getElementById(`${browserTabsId}-tab-${nextActiveIndex}`)?.focus(); + }); + } + } if (shouldCloseBrowserPanelAfterTabClose(state)) { onClosePanel(); } @@ -1283,6 +1297,7 @@ export function BrowserPanel({ }, [ api, + browserTabsId, ensureLiveRuntime, onClosePanel, runBrowserAction, @@ -1616,7 +1631,7 @@ export function BrowserPanel({ onKeyDown={(event) => { if (event.key === "Delete") { event.preventDefault(); - onCloseTab(tab.id); + onCloseTab(tab.id, { restoreTabFocus: true }); return; } const tabs = threadBrowserState?.tabs ?? []; @@ -1646,6 +1661,7 @@ export function BrowserPanel({ variant="ghost" size="icon-sm" className={closeButtonClassName(isActive)} + tabIndex={-1} onClick={(event) => { event.stopPropagation(); onCloseTab(tab.id); diff --git a/apps/web/src/components/RecentViewSwitcher.tsx b/apps/web/src/components/RecentViewSwitcher.tsx index b2c744807..61e937bac 100644 --- a/apps/web/src/components/RecentViewSwitcher.tsx +++ b/apps/web/src/components/RecentViewSwitcher.tsx @@ -98,7 +98,10 @@ export function RecentViewSwitcher(props: { : 0; return ( -
+
- - + + Date: Fri, 24 Jul 2026 17:37:26 +0300 Subject: [PATCH 04/16] Complete browser tab and overlay semantics --- .../scripts/run-browser-overlay-lifecycle.mjs | 3 +- .../src/browserManager.reliability.test.ts | 30 +++++ apps/desktop/src/browserManager.ts | 3 +- .../src/components/BrowserPanel.browser.tsx | 119 +++++++++++++++++- apps/web/src/components/BrowserPanel.tsx | 9 +- apps/web/src/components/ui/undoSnackbar.tsx | 6 +- apps/web/src/routes/_chat.$threadId.tsx | 13 +- 7 files changed, 171 insertions(+), 12 deletions(-) diff --git a/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs b/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs index 1a96ab9f6..e2477d3d1 100644 --- a/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs +++ b/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs @@ -1,7 +1,7 @@ // Bundles a hermetic Electron fixture, launches it with an isolated profile, and always // tears down the detached process group and temporary state on completion or interruption. import { spawn, spawnSync } from "node:child_process"; -import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { dirname, join, resolve } from "node:path"; import { pathToFileURL, fileURLToPath } from "node:url"; @@ -13,6 +13,7 @@ const desktopDir = resolve(scriptDir, ".."); const workspaceRoot = resolve(desktopDir, "../.."); const tempDir = mkdtempSync(join(tmpdir(), "scient-browser-overlay-lifecycle-")); const profileDir = join(tempDir, "profile"); +mkdirSync(join(profileDir, "session-data"), { recursive: true }); const electronOutputPath = join(tempDir, "browser-overlay-lifecycle.cjs"); const preloadOutputPath = join(tempDir, "browser-overlay-lifecycle.preload.cjs"); const rendererOutputPath = join(tempDir, "browser-overlay-lifecycle.renderer.js"); diff --git a/apps/desktop/src/browserManager.reliability.test.ts b/apps/desktop/src/browserManager.reliability.test.ts index 698e4fa6e..b866b0a48 100644 --- a/apps/desktop/src/browserManager.reliability.test.ts +++ b/apps/desktop/src/browserManager.reliability.test.ts @@ -130,6 +130,36 @@ describe("DesktopBrowserManager reliability", () => { manager.dispose(); }); + it("selects the adjacent tab when the active tab closes", () => { + const manager = new DesktopBrowserManager(); + const opened = manager.open({ threadId: THREAD_ID }); + const firstTabId = opened.activeTabId; + const withSecondTab = manager.newTab({ + threadId: THREAD_ID, + url: "https://second.example/", + }); + const secondTabId = withSecondTab.activeTabId; + const withThirdTab = manager.newTab({ + threadId: THREAD_ID, + url: "https://third.example/", + }); + + manager.selectTab({ threadId: THREAD_ID, tabId: firstTabId ?? "" }); + const afterClosingFirst = manager.closeTab({ + threadId: THREAD_ID, + tabId: firstTabId ?? "", + }); + expect(afterClosingFirst.activeTabId).toBe(secondTabId); + + manager.selectTab({ threadId: THREAD_ID, tabId: secondTabId ?? "" }); + const afterClosingSecond = manager.closeTab({ + threadId: THREAD_ID, + tabId: secondTabId ?? "", + }); + expect(afterClosingSecond.activeTabId).toBe(withThirdTab.activeTabId); + manager.dispose(); + }); + it("replaces a destroyed tracked runtime before navigating", async () => { const manager = new DesktopBrowserManager(); const opened = manager.open({ threadId: THREAD_ID }); diff --git a/apps/desktop/src/browserManager.ts b/apps/desktop/src/browserManager.ts index 951db5c29..5ec845c15 100644 --- a/apps/desktop/src/browserManager.ts +++ b/apps/desktop/src/browserManager.ts @@ -1007,6 +1007,7 @@ export class DesktopBrowserManager { closeTab(input: BrowserTabInput): ThreadBrowserState { const state = this.ensureWorkspace(input.threadId); + const closedTabIndex = state.tabs.findIndex((tab) => tab.id === input.tabId); const closedTab = state.tabs.find((tab) => tab.id === input.tabId); const nextTabs = state.tabs.filter((tab) => tab.id !== input.tabId); if (nextTabs.length === state.tabs.length) { @@ -1028,7 +1029,7 @@ export class DesktopBrowserManager { } if (!state.activeTabId || state.activeTabId === input.tabId) { - state.activeTabId = nextTabs[Math.max(0, nextTabs.length - 1)]?.id ?? null; + state.activeTabId = nextTabs[Math.min(closedTabIndex, nextTabs.length - 1)]?.id ?? null; } const bounds = this.getVisibleBoundsForThread(input.threadId); diff --git a/apps/web/src/components/BrowserPanel.browser.tsx b/apps/web/src/components/BrowserPanel.browser.tsx index dab8fd866..3b49e9926 100644 --- a/apps/web/src/components/BrowserPanel.browser.tsx +++ b/apps/web/src/components/BrowserPanel.browser.tsx @@ -31,6 +31,7 @@ vi.mock("~/nativeApi", async (importOriginal) => ({ import { useBrowserStateStore } from "../browserStateStore"; import { BrowserPanel } from "./BrowserPanel"; import { RecentViewSwitcher } from "./RecentViewSwitcher"; +import { showUndoSnackbar, UndoSnackbarProvider } from "./ui/undoSnackbar"; const THREAD_ID = "thread-browser-copy" as ThreadId; @@ -88,11 +89,14 @@ function renderPanel() { ); } -function renderLivePanel(onClosePanel: () => void, options?: { showRecentViews?: boolean }) { +function renderLivePanel( + onClosePanel: (options?: { restoreFocus?: boolean }) => void, + options?: { showRecentViews?: boolean; withUndoSnackbar?: boolean }, +) { const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); - return render( + const content = ( -
+
void, options?: { showRecentViews?: /> ) : null}
- , + + ); + return render( + options?.withUndoSnackbar ? {content} : content, ); } @@ -181,7 +188,9 @@ describe("BrowserPanel interactions", () => { nativeApiTestState.api = undefined; useBrowserStateStore.getState().removeThreadState(THREAD_ID); vi.restoreAllMocks(); - document.body.innerHTML = ""; + document + .querySelectorAll("[data-browser-panel-test-fallback]") + .forEach((element) => element.remove()); }); it("surfaces clipboard rejection locally", async () => { @@ -397,6 +406,106 @@ describe("BrowserPanel interactions", () => { }); }); + it("occludes the native surface when the global Undo snackbar mounts", async () => { + const openState = browserState("tab-1"); + const api = liveBrowserApi({ openState }); + nativeApiTestState.api = api; + + await renderLivePanel(vi.fn(), { withUndoSnackbar: true }); + await vi.waitFor(() => { + expect(document.querySelector("webview")?.style.visibility).toBe("visible"); + }); + + showUndoSnackbar({ title: "Thread archived", onUndo: async () => true }); + await expect.element(page.getByRole("button", { name: "Undo" })).toBeVisible(); + await vi.waitFor(() => { + expect(document.querySelector("webview")?.style.visibility).toBe("hidden"); + expect(vi.mocked(api.browser.setPanelBounds).mock.calls).not.toContainEqual([ + { threadId: THREAD_ID, bounds: null, surface: "renderer" }, + ]); + }); + ((await page.getByRole("button", { name: "Dismiss" }).element()) as HTMLButtonElement).click(); + await vi.waitFor(() => { + expect(page.getByRole("button", { name: "Undo" }).query()).toBeNull(); + expect(document.querySelector("webview")?.style.visibility).toBe("visible"); + }); + }); + + it("moves focus to the adjacent tab when deleting the first of three tabs", async () => { + const openState = browserState("tab-1"); + openState.tabs = [ + ...openState.tabs, + { + ...openState.tabs[1]!, + id: "tab-3", + url: "https://third.example/", + title: "Third", + lastCommittedUrl: "https://third.example/", + }, + ]; + const closeTabState: ThreadBrowserState = { + ...openState, + version: openState.version + 1, + activeTabId: "tab-2", + tabs: openState.tabs.slice(1), + }; + const api = liveBrowserApi({ openState, closeTabState }); + nativeApiTestState.api = api; + useBrowserStateStore.getState().upsertThreadState(openState); + + await renderLivePanel(vi.fn()); + const firstTab = (await page + .getByRole("tab", { name: "ScientFactory" }) + .element()) as HTMLButtonElement; + const secondTab = (await page + .getByRole("tab", { name: "Example" }) + .element()) as HTMLButtonElement; + firstTab.focus(); + await userEvent.keyboard("{Delete}"); + + await vi.waitFor(() => { + expect(api.browser.closeTab).toHaveBeenCalledWith({ + threadId: THREAD_ID, + tabId: "tab-1", + }); + expect(document.activeElement).toBe(secondTab); + }); + }); + + it("requests a parent focus fallback when keyboard deletion closes the final tab", async () => { + const openState = browserState("tab-1"); + openState.tabs = [openState.tabs[0]!]; + const closedState: ThreadBrowserState = { + ...openState, + version: openState.version + 1, + open: false, + activeTabId: null, + tabs: [], + }; + const api = liveBrowserApi({ openState, closeTabState: closedState }); + nativeApiTestState.api = api; + useBrowserStateStore.getState().upsertThreadState(openState); + const fallback = document.createElement("button"); + fallback.textContent = "Open Browser"; + fallback.dataset.browserPanelTestFallback = "true"; + document.body.append(fallback); + const onClosePanel = vi.fn((options?: { restoreFocus?: boolean }) => { + if (options?.restoreFocus) fallback.focus(); + }); + + await renderLivePanel(onClosePanel); + const tab = (await page + .getByRole("tab", { name: "ScientFactory" }) + .element()) as HTMLButtonElement; + tab.focus(); + await userEvent.keyboard("{Delete}"); + + await vi.waitFor(() => { + expect(onClosePanel).toHaveBeenCalledWith({ restoreFocus: true }); + expect(document.activeElement).toBe(fallback); + }); + }); + it("hides the native browser surface while an intersecting app menu is open", async () => { const openState = browserState("tab-1"); const api = liveBrowserApi({ openState }); diff --git a/apps/web/src/components/BrowserPanel.tsx b/apps/web/src/components/BrowserPanel.tsx index 46900c833..d1807034d 100644 --- a/apps/web/src/components/BrowserPanel.tsx +++ b/apps/web/src/components/BrowserPanel.tsx @@ -87,7 +87,7 @@ import { Skeleton } from "./ui/skeleton"; interface BrowserPanelProps { mode: DiffPanelMode; threadId: ThreadId; - onClosePanel: () => void; + onClosePanel: (options?: { restoreFocus?: boolean }) => void; runtimeMode?: DockPaneRuntimeMode; onRequestLive?: () => void; } @@ -1291,7 +1291,7 @@ export function BrowserPanel({ } } if (shouldCloseBrowserPanelAfterTabClose(state)) { - onClosePanel(); + onClosePanel({ restoreFocus: options?.restoreTabFocus === true }); } }); }, @@ -1561,7 +1561,10 @@ export function BrowserPanel({ Open externally - + onClosePanel()} + > Close browser panel diff --git a/apps/web/src/components/ui/undoSnackbar.tsx b/apps/web/src/components/ui/undoSnackbar.tsx index 3e59ad8e3..e1b7b0109 100644 --- a/apps/web/src/components/ui/undoSnackbar.tsx +++ b/apps/web/src/components/ui/undoSnackbar.tsx @@ -58,7 +58,11 @@ function UndoSnackbarSurface({ toast }: { toast: ToastObject } }; return ( - + diff --git a/apps/web/src/routes/_chat.$threadId.tsx b/apps/web/src/routes/_chat.$threadId.tsx index 2fd56322c..d1263c2d9 100644 --- a/apps/web/src/routes/_chat.$threadId.tsx +++ b/apps/web/src/routes/_chat.$threadId.tsx @@ -2255,7 +2255,18 @@ function SingleChatSurface(props: { closePane(props.threadId, pane.id)} + onClosePanel={(options) => { + closePane(props.threadId, pane.id); + if (options?.restoreFocus) { + window.requestAnimationFrame(() => { + document + .querySelector( + '[data-right-dock-content] [data-right-dock-empty-state] button:not([aria-disabled="true"])', + ) + ?.focus(); + }); + } + }} runtimeMode={context.runtimeMode} onRequestLive={requestActiveDockPaneLive} /> From e7fa9f4f8a83ee950ec906c147cfe75129856a9f Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 17:52:16 +0300 Subject: [PATCH 05/16] Restore focus after final browser tab closes --- .../chat/browserPanelFocus.browser.tsx | 70 +++++++++++++++++++ .../src/components/chat/browserPanelFocus.ts | 31 ++++++++ apps/web/src/routes/_chat.$threadId.tsx | 30 +++++--- 3 files changed, 123 insertions(+), 8 deletions(-) create mode 100644 apps/web/src/components/chat/browserPanelFocus.browser.tsx create mode 100644 apps/web/src/components/chat/browserPanelFocus.ts diff --git a/apps/web/src/components/chat/browserPanelFocus.browser.tsx b/apps/web/src/components/chat/browserPanelFocus.browser.tsx new file mode 100644 index 000000000..b6f247197 --- /dev/null +++ b/apps/web/src/components/chat/browserPanelFocus.browser.tsx @@ -0,0 +1,70 @@ +// FILE: browserPanelFocus.browser.tsx +// Purpose: Browser-level focus recovery regressions for final Browser-tab deletion. + +import { afterEach, describe, expect, it } from "vitest"; + +import { + restoreRightDockFocusAfterBrowserClose, + restoreSplitChatFocusAfterBrowserClose, +} from "./browserPanelFocus"; + +afterEach(() => { + document.body.replaceChildren(); +}); + +describe("browser panel focus recovery", () => { + it("focuses the newly active right-dock pane before other dock controls", () => { + document.body.innerHTML = ` +
+ + + +
+ `; + + expect(restoreRightDockFocusAfterBrowserClose(document)).toBe(true); + expect(document.activeElement?.textContent).toBe("Diff"); + }); + + it("focuses the empty-state action after closing the dock's final pane", () => { + document.body.innerHTML = ` +
+
+ +
+ +
+ `; + + expect(restoreRightDockFocusAfterBrowserClose(document)).toBe(true); + expect(document.activeElement?.textContent).toBe("Browser"); + }); + + it("returns focus to an enabled split-pane composer", () => { + document.body.innerHTML = ` +
+
+
+
+
+ `; + const pane = document.querySelector("[data-split-chat-pane]")!; + + expect(restoreSplitChatFocusAfterBrowserClose(pane)).toBe(true); + expect(document.activeElement).toBe(document.querySelector('[data-testid="composer-editor"]')); + }); + + it("uses the split pane itself when its composer is unavailable", () => { + document.body.innerHTML = ` +
+
+
+
+
+ `; + const pane = document.querySelector("[data-split-chat-pane]")!; + + expect(restoreSplitChatFocusAfterBrowserClose(pane)).toBe(true); + expect(document.activeElement).toBe(pane); + }); +}); diff --git a/apps/web/src/components/chat/browserPanelFocus.ts b/apps/web/src/components/chat/browserPanelFocus.ts new file mode 100644 index 000000000..5bb67376c --- /dev/null +++ b/apps/web/src/components/chat/browserPanelFocus.ts @@ -0,0 +1,31 @@ +// FILE: browserPanelFocus.ts +// Purpose: Restore keyboard focus after the Browser panel removes its final tab. +// Layer: Chat browser/dock accessibility helpers + +const ENABLED_BUTTON_SELECTOR = 'button:not(:disabled):not([aria-disabled="true"])'; + +function focusTarget(target: HTMLElement | null): boolean { + if (!target) return false; + target.focus(); + return target.ownerDocument.activeElement === target; +} + +export function restoreRightDockFocusAfterBrowserClose(document: Document): boolean { + const dock = document.querySelector("[data-right-dock-content]"); + if (!dock) return false; + + return focusTarget( + dock.querySelector('button[aria-pressed="true"]') ?? + dock.querySelector(`[data-right-dock-empty-state] ${ENABLED_BUTTON_SELECTOR}`) ?? + dock.querySelector(`${ENABLED_BUTTON_SELECTOR}[aria-label="Add panel"]`) ?? + dock.querySelector(`${ENABLED_BUTTON_SELECTOR}[aria-label="Collapse panel"]`), + ); +} + +export function restoreSplitChatFocusAfterBrowserClose(pane: HTMLElement): boolean { + return focusTarget( + pane.querySelector( + '[data-chat-composer-form="true"] [data-testid="composer-editor"][contenteditable="true"]', + ) ?? pane, + ); +} diff --git a/apps/web/src/routes/_chat.$threadId.tsx b/apps/web/src/routes/_chat.$threadId.tsx index d1263c2d9..9a03004f6 100644 --- a/apps/web/src/routes/_chat.$threadId.tsx +++ b/apps/web/src/routes/_chat.$threadId.tsx @@ -89,6 +89,10 @@ import { } from "../rightDockStore.logic"; import { RightDock } from "../components/chat/RightDock"; import { RightDockEmptyState } from "../components/chat/RightDockEmptyState"; +import { + restoreRightDockFocusAfterBrowserClose, + restoreSplitChatFocusAfterBrowserClose, +} from "../components/chat/browserPanelFocus"; import { CHAT_SURFACE_HEADER_ROW_CLASS_NAME, DOCK_HEADER_ICON_BUTTON_CLASS, @@ -296,7 +300,7 @@ function SplitPaneEmbeddedPanel(props: { panelOpen: boolean; panel: ChatRightPanel | null | undefined; threadId: ThreadIdType | null; - onClosePanel: () => void; + onClosePanel: (options?: { restoreFocus?: boolean }) => void; panelState: Pick; isFocused: boolean; onUpdatePanelState: ( @@ -413,7 +417,7 @@ function SplitPaneEmbeddedPanel(props: { props.onClosePanel()} panelState={props.panelState} liveRefreshEnabled={props.isFocused} onUpdatePanelState={props.onUpdatePanelState} @@ -831,6 +835,7 @@ function SplitPaneSurface(props: { side: SplitDropSide; }) => void; }) { + const surfaceRef = useRef(null); const paneScopeId = splitViewPaneScopeId(props.splitView.id, props.paneId); const panelOpen = props.panelState.panel !== null; const shouldRenderPanelContent = panelOpen || props.panelState.hasOpenedPanel; @@ -849,6 +854,10 @@ function SplitPaneSurface(props: { return (
{ + props.onClosePanel(); + if (options?.restoreFocus) { + window.requestAnimationFrame(() => { + if (surfaceRef.current) { + restoreSplitChatFocusAfterBrowserClose(surfaceRef.current); + } + }); + } + }} panelState={props.panelState} isFocused={props.isFocused} onUpdatePanelState={props.onUpdatePanelState} @@ -2259,11 +2277,7 @@ function SingleChatSurface(props: { closePane(props.threadId, pane.id); if (options?.restoreFocus) { window.requestAnimationFrame(() => { - document - .querySelector( - '[data-right-dock-content] [data-right-dock-empty-state] button:not([aria-disabled="true"])', - ) - ?.focus(); + restoreRightDockFocusAfterBrowserClose(document); }); } }} From 21ca452c82a0eb590a78ce473a9b236f5b5000d1 Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 17:57:14 +0300 Subject: [PATCH 06/16] Synchronize split-pane browser focus --- .../components/chat/browserPanelFocus.browser.tsx | 15 +++++++++++---- apps/web/src/components/chat/browserPanelFocus.ts | 6 +++++- apps/web/src/routes/_chat.$threadId.tsx | 2 +- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/apps/web/src/components/chat/browserPanelFocus.browser.tsx b/apps/web/src/components/chat/browserPanelFocus.browser.tsx index b6f247197..b0346fa1f 100644 --- a/apps/web/src/components/chat/browserPanelFocus.browser.tsx +++ b/apps/web/src/components/chat/browserPanelFocus.browser.tsx @@ -42,15 +42,22 @@ describe("browser panel focus recovery", () => { it("returns focus to an enabled split-pane composer", () => { document.body.innerHTML = ` -
+
+
`; - const pane = document.querySelector("[data-split-chat-pane]")!; + const pane = document.querySelector('[data-split-chat-pane="pane-1"]')!; + let focusedPaneId = "pane-2"; - expect(restoreSplitChatFocusAfterBrowserClose(pane)).toBe(true); + expect( + restoreSplitChatFocusAfterBrowserClose(pane, () => { + focusedPaneId = "pane-1"; + }), + ).toBe(true); + expect(focusedPaneId).toBe("pane-1"); expect(document.activeElement).toBe(document.querySelector('[data-testid="composer-editor"]')); }); @@ -64,7 +71,7 @@ describe("browser panel focus recovery", () => { `; const pane = document.querySelector("[data-split-chat-pane]")!; - expect(restoreSplitChatFocusAfterBrowserClose(pane)).toBe(true); + expect(restoreSplitChatFocusAfterBrowserClose(pane, () => undefined)).toBe(true); expect(document.activeElement).toBe(pane); }); }); diff --git a/apps/web/src/components/chat/browserPanelFocus.ts b/apps/web/src/components/chat/browserPanelFocus.ts index 5bb67376c..34ad57832 100644 --- a/apps/web/src/components/chat/browserPanelFocus.ts +++ b/apps/web/src/components/chat/browserPanelFocus.ts @@ -22,7 +22,11 @@ export function restoreRightDockFocusAfterBrowserClose(document: Document): bool ); } -export function restoreSplitChatFocusAfterBrowserClose(pane: HTMLElement): boolean { +export function restoreSplitChatFocusAfterBrowserClose( + pane: HTMLElement, + activatePane: () => void, +): boolean { + activatePane(); return focusTarget( pane.querySelector( '[data-chat-composer-form="true"] [data-testid="composer-editor"][contenteditable="true"]', diff --git a/apps/web/src/routes/_chat.$threadId.tsx b/apps/web/src/routes/_chat.$threadId.tsx index 9a03004f6..be12aa0ec 100644 --- a/apps/web/src/routes/_chat.$threadId.tsx +++ b/apps/web/src/routes/_chat.$threadId.tsx @@ -919,7 +919,7 @@ function SplitPaneSurface(props: { if (options?.restoreFocus) { window.requestAnimationFrame(() => { if (surfaceRef.current) { - restoreSplitChatFocusAfterBrowserClose(surfaceRef.current); + restoreSplitChatFocusAfterBrowserClose(surfaceRef.current, props.onFocus); } }); } From ef25657190a0a712c3f481a6d193ddbe9fa116b7 Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 18:10:13 +0300 Subject: [PATCH 07/16] Harden Electron overlay lifecycle runner --- .../scripts/run-browser-overlay-lifecycle.mjs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs b/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs index e2477d3d1..d713ac31e 100644 --- a/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs +++ b/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs @@ -14,6 +14,9 @@ const workspaceRoot = resolve(desktopDir, "../.."); const tempDir = mkdtempSync(join(tmpdir(), "scient-browser-overlay-lifecycle-")); const profileDir = join(tempDir, "profile"); mkdirSync(join(profileDir, "session-data"), { recursive: true }); +// Allow both bounded 10s fixture startup waits, the required >30s overlay hold, +// Electron launch/load time, and cleanup without making a cold run race its watchdog. +const PROCESS_TIMEOUT_MS = 90_000; const electronOutputPath = join(tempDir, "browser-overlay-lifecycle.cjs"); const preloadOutputPath = join(tempDir, "browser-overlay-lifecycle.preload.cjs"); const rendererOutputPath = join(tempDir, "browser-overlay-lifecycle.renderer.js"); @@ -67,9 +70,14 @@ writeFileSync( "utf8", ); -const electronCommand = resolveElectronLaunchCommand(["--disable-gpu", electronOutputPath], { - development: false, -}); +const electronCommand = resolveElectronLaunchCommand( + [ + "--disable-gpu", + electronOutputPath, + ...(process.platform === "darwin" ? ["-ApplePersistenceIgnoreState", "YES"] : []), + ], + { development: false }, +); const child = spawn(electronCommand.electronPath, electronCommand.args, { cwd: workspaceRoot, detached: process.platform !== "win32", @@ -139,7 +147,7 @@ const timeout = setTimeout(() => { requestedExitCode = 1; process.stderr.write("Electron browser overlay lifecycle test timed out.\n"); killChildTree(); -}, 50_000); +}, PROCESS_TIMEOUT_MS); child.on("error", (error) => { process.stderr.write(`${error.stack ?? error.message}\n`); From 4e005b90404e1cfaabe8765beccfed085107a492 Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 18:19:06 +0300 Subject: [PATCH 08/16] Isolate macOS Electron test bundle state --- .../scripts/run-browser-overlay-lifecycle.mjs | 39 ++++++++++++++++++- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs b/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs index d713ac31e..bdfcc105e 100644 --- a/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs +++ b/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs @@ -1,7 +1,7 @@ // Bundles a hermetic Electron fixture, launches it with an isolated profile, and always // tears down the detached process group and temporary state on completion or interruption. import { spawn, spawnSync } from "node:child_process"; -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { dirname, join, resolve } from "node:path"; import { pathToFileURL, fileURLToPath } from "node:url"; @@ -21,6 +21,34 @@ const electronOutputPath = join(tempDir, "browser-overlay-lifecycle.cjs"); const preloadOutputPath = join(tempDir, "browser-overlay-lifecycle.preload.cjs"); const rendererOutputPath = join(tempDir, "browser-overlay-lifecycle.renderer.js"); const fixturePath = join(tempDir, "browser-overlay-lifecycle.html"); +const macTestBundleId = `com.scientfactory.scient.browser-overlay-test.${process.pid}`; +const macSavedStatePath = join(tmpdir(), `${macTestBundleId}.savedState`); + +function createMacTestElectronExecutable(originalExecutablePath) { + const originalAppPath = dirname(dirname(dirname(originalExecutablePath))); + const testAppPath = join(tempDir, "ScientBrowserOverlayTest.app"); + const testContentsPath = join(testAppPath, "Contents"); + const clone = spawnSync("cp", ["-cR", originalAppPath, testAppPath], { encoding: "utf8" }); + if (clone.status !== 0) { + rmSync(tempDir, { recursive: true, force: true }); + throw new Error( + `Could not clone the Electron test app: ${clone.stderr || clone.stdout || "unknown error"}`, + ); + } + + const originalInfoPath = join(originalAppPath, "Contents", "Info.plist"); + const originalInfo = readFileSync(originalInfoPath, "utf8"); + const info = originalInfo.replace( + /(CFBundleIdentifier<\/key>\s*)[^<]+(<\/string>)/, + `$1${macTestBundleId}$2`, + ); + if (info === originalInfo) { + rmSync(tempDir, { recursive: true, force: true }); + throw new Error("Could not isolate the Electron test app bundle identifier."); + } + writeFileSync(join(testContentsPath, "Info.plist"), info, "utf8"); + return join(testContentsPath, "MacOS", "Electron"); +} const builds = [ { @@ -78,7 +106,11 @@ const electronCommand = resolveElectronLaunchCommand( ], { development: false }, ); -const child = spawn(electronCommand.electronPath, electronCommand.args, { +const electronExecutablePath = + process.platform === "darwin" + ? createMacTestElectronExecutable(electronCommand.electronPath) + : electronCommand.electronPath; +const child = spawn(electronExecutablePath, electronCommand.args, { cwd: workspaceRoot, detached: process.platform !== "win32", env: { @@ -121,6 +153,9 @@ function finish(code) { finished = true; clearTimeout(timeout); rmSync(tempDir, { recursive: true, force: true }); + if (process.platform === "darwin") { + rmSync(macSavedStatePath, { recursive: true, force: true }); + } process.exit(code); } From 417edb0e3035ec3815477a4629291c535eba53e1 Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 18:24:20 +0300 Subject: [PATCH 09/16] Keep Electron lifecycle tests fully hermetic --- apps/desktop/scripts/electron-launcher.mjs | 8 +++-- .../scripts/run-browser-overlay-lifecycle.mjs | 34 +++++++++++-------- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/apps/desktop/scripts/electron-launcher.mjs b/apps/desktop/scripts/electron-launcher.mjs index 3c3075ba2..4b46ec4bd 100644 --- a/apps/desktop/scripts/electron-launcher.mjs +++ b/apps/desktop/scripts/electron-launcher.mjs @@ -137,9 +137,13 @@ function buildMacLauncher(electronBinaryPath) { return targetBinaryPath; } -export function resolveElectronPath() { +export function resolveElectronPackagePath() { const require = createRequire(import.meta.url); - const electronBinaryPath = require("electron"); + return require("electron"); +} + +export function resolveElectronPath() { + const electronBinaryPath = resolveElectronPackagePath(); if (process.platform !== "darwin") { return electronBinaryPath; diff --git a/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs b/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs index bdfcc105e..c22bab3ee 100644 --- a/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs +++ b/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs @@ -2,11 +2,11 @@ // tears down the detached process group and temporary state on completion or interruption. import { spawn, spawnSync } from "node:child_process"; import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; -import { tmpdir } from "node:os"; +import { homedir, tmpdir } from "node:os"; import { dirname, join, resolve } from "node:path"; import { pathToFileURL, fileURLToPath } from "node:url"; -import { resolveElectronLaunchCommand } from "./electron-launcher.mjs"; +import { resolveElectronPackagePath, resolveLinuxSandboxArgs } from "./electron-launcher.mjs"; const scriptDir = dirname(fileURLToPath(import.meta.url)); const desktopDir = resolve(scriptDir, ".."); @@ -22,7 +22,10 @@ const preloadOutputPath = join(tempDir, "browser-overlay-lifecycle.preload.cjs") const rendererOutputPath = join(tempDir, "browser-overlay-lifecycle.renderer.js"); const fixturePath = join(tempDir, "browser-overlay-lifecycle.html"); const macTestBundleId = `com.scientfactory.scient.browser-overlay-test.${process.pid}`; -const macSavedStatePath = join(tmpdir(), `${macTestBundleId}.savedState`); +const macSavedStatePaths = [ + join(tmpdir(), `${macTestBundleId}.savedState`), + join(homedir(), "Library", "Saved Application State", `${macTestBundleId}.savedState`), +]; function createMacTestElectronExecutable(originalExecutablePath) { const originalAppPath = dirname(dirname(dirname(originalExecutablePath))); @@ -98,19 +101,18 @@ writeFileSync( "utf8", ); -const electronCommand = resolveElectronLaunchCommand( - [ - "--disable-gpu", - electronOutputPath, - ...(process.platform === "darwin" ? ["-ApplePersistenceIgnoreState", "YES"] : []), - ], - { development: false }, -); +const electronPackagePath = resolveElectronPackagePath(); const electronExecutablePath = process.platform === "darwin" - ? createMacTestElectronExecutable(electronCommand.electronPath) - : electronCommand.electronPath; -const child = spawn(electronExecutablePath, electronCommand.args, { + ? createMacTestElectronExecutable(electronPackagePath) + : electronPackagePath; +const electronArgs = [ + ...resolveLinuxSandboxArgs(electronPackagePath, { development: false }), + "--disable-gpu", + electronOutputPath, + ...(process.platform === "darwin" ? ["-ApplePersistenceIgnoreState", "YES"] : []), +]; +const child = spawn(electronExecutablePath, electronArgs, { cwd: workspaceRoot, detached: process.platform !== "win32", env: { @@ -154,7 +156,9 @@ function finish(code) { clearTimeout(timeout); rmSync(tempDir, { recursive: true, force: true }); if (process.platform === "darwin") { - rmSync(macSavedStatePath, { recursive: true, force: true }); + for (const savedStatePath of macSavedStatePaths) { + rmSync(savedStatePath, { recursive: true, force: true }); + } } process.exit(code); } From f52361c6b340999271d0bc032bce7700da984806 Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 18:42:27 +0300 Subject: [PATCH 10/16] Resolve Electron sandbox from workspace package --- .github/workflows/ci.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bc3240616..bd34a1c2f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -90,8 +90,11 @@ jobs: - name: Electron browser overlay lifecycle timeout-minutes: 2 run: | - sudo chown root:root node_modules/electron/dist/chrome-sandbox - sudo chmod 4755 node_modules/electron/dist/chrome-sandbox + ELECTRON_BINARY="$(cd apps/desktop && bun -e 'console.log(require("electron"))')" + ELECTRON_SANDBOX="$(dirname "$ELECTRON_BINARY")/chrome-sandbox" + test -f "$ELECTRON_SANDBOX" + sudo chown root:root "$ELECTRON_SANDBOX" + sudo chmod 4755 "$ELECTRON_SANDBOX" xvfb-run -a bun run test:desktop-browser-overlay-lifecycle # Blocking product-behavior coverage. Linux-only pixel/font/layout From 85b119635bef7f1027922353d9d297cea41f6cf4 Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 18:50:14 +0300 Subject: [PATCH 11/16] Guarantee Electron test harness cleanup --- .../scripts/run-browser-overlay-lifecycle.mjs | 45 +++++++++++++------ 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs b/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs index c22bab3ee..9608bc4f0 100644 --- a/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs +++ b/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs @@ -27,13 +27,25 @@ const macSavedStatePaths = [ join(homedir(), "Library", "Saved Application State", `${macTestBundleId}.savedState`), ]; +function cleanupTemporaryState() { + rmSync(tempDir, { recursive: true, force: true }); + if (process.platform === "darwin") { + for (const savedStatePath of macSavedStatePaths) { + rmSync(savedStatePath, { recursive: true, force: true }); + } + } +} + +// Keep setup failures hermetic too: package resolution, sandbox validation, fixture +// generation, and process spawning can all fail before the child handlers are installed. +process.once("exit", cleanupTemporaryState); + function createMacTestElectronExecutable(originalExecutablePath) { const originalAppPath = dirname(dirname(dirname(originalExecutablePath))); const testAppPath = join(tempDir, "ScientBrowserOverlayTest.app"); const testContentsPath = join(testAppPath, "Contents"); const clone = spawnSync("cp", ["-cR", originalAppPath, testAppPath], { encoding: "utf8" }); if (clone.status !== 0) { - rmSync(tempDir, { recursive: true, force: true }); throw new Error( `Could not clone the Electron test app: ${clone.stderr || clone.stdout || "unknown error"}`, ); @@ -46,7 +58,6 @@ function createMacTestElectronExecutable(originalExecutablePath) { `$1${macTestBundleId}$2`, ); if (info === originalInfo) { - rmSync(tempDir, { recursive: true, force: true }); throw new Error("Could not isolate the Electron test app bundle identifier."); } writeFileSync(join(testContentsPath, "Info.plist"), info, "utf8"); @@ -78,7 +89,6 @@ for (const build of builds) { { cwd: workspaceRoot, encoding: "utf8" }, ); if (result.status !== 0) { - rmSync(tempDir, { recursive: true, force: true }); process.stderr.write(result.stdout ?? ""); process.stderr.write(result.stderr ?? ""); process.exit(result.status ?? 1); @@ -138,15 +148,28 @@ child.stderr.on("data", (chunk) => { }); function killChildTree() { - if (!child.pid || child.exitCode !== null || child.signalCode !== null) return; + if (!child.pid || child.exitCode !== null || child.signalCode !== null) return true; if (process.platform === "win32") { - spawnSync("taskkill", ["/pid", String(child.pid), "/t", "/f"], { stdio: "ignore" }); - return; + const result = spawnSync("taskkill", ["/pid", String(child.pid), "/t", "/f"], { + encoding: "utf8", + }); + if (result.status !== 0) { + process.stderr.write( + `Could not terminate Electron process tree: ${result.stderr || result.stdout || "taskkill failed"}\n`, + ); + return false; + } + return true; } try { process.kill(-child.pid, "SIGKILL"); + return true; } catch { - child.kill("SIGKILL"); + const killed = child.kill("SIGKILL"); + if (!killed) { + process.stderr.write("Could not terminate Electron process tree.\n"); + } + return killed; } } @@ -154,12 +177,7 @@ function finish(code) { if (finished) return; finished = true; clearTimeout(timeout); - rmSync(tempDir, { recursive: true, force: true }); - if (process.platform === "darwin") { - for (const savedStatePath of macSavedStatePaths) { - rmSync(savedStatePath, { recursive: true, force: true }); - } - } + cleanupTemporaryState(); process.exit(code); } @@ -186,6 +204,7 @@ const timeout = setTimeout(() => { requestedExitCode = 1; process.stderr.write("Electron browser overlay lifecycle test timed out.\n"); killChildTree(); + setTimeout(() => finish(1), 2_000).unref(); }, PROCESS_TIMEOUT_MS); child.on("error", (error) => { From fc3c253c1ed1d2a903cae06b556a3c9960079326 Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 18:55:59 +0300 Subject: [PATCH 12/16] Register Electron fixture cleanup immediately --- .../desktop/scripts/run-browser-overlay-lifecycle.mjs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs b/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs index 9608bc4f0..2c7a7b031 100644 --- a/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs +++ b/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs @@ -12,6 +12,14 @@ const scriptDir = dirname(fileURLToPath(import.meta.url)); const desktopDir = resolve(scriptDir, ".."); const workspaceRoot = resolve(desktopDir, "../.."); const tempDir = mkdtempSync(join(tmpdir(), "scient-browser-overlay-lifecycle-")); + +function cleanupTempDir() { + rmSync(tempDir, { recursive: true, force: true }); +} + +// Register cleanup before any other setup can fail after creating the directory. +process.once("exit", cleanupTempDir); + const profileDir = join(tempDir, "profile"); mkdirSync(join(profileDir, "session-data"), { recursive: true }); // Allow both bounded 10s fixture startup waits, the required >30s overlay hold, @@ -28,7 +36,7 @@ const macSavedStatePaths = [ ]; function cleanupTemporaryState() { - rmSync(tempDir, { recursive: true, force: true }); + cleanupTempDir(); if (process.platform === "darwin") { for (const savedStatePath of macSavedStatePaths) { rmSync(savedStatePath, { recursive: true, force: true }); @@ -38,6 +46,7 @@ function cleanupTemporaryState() { // Keep setup failures hermetic too: package resolution, sandbox validation, fixture // generation, and process spawning can all fail before the child handlers are installed. +process.removeListener("exit", cleanupTempDir); process.once("exit", cleanupTemporaryState); function createMacTestElectronExecutable(originalExecutablePath) { From be035c23a2567310a412270a991ebfec1e5ee1d3 Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 19:06:06 +0300 Subject: [PATCH 13/16] Restore focus after browser close controls --- .../src/components/BrowserPanel.browser.tsx | 41 ++++++++++++++++++- apps/web/src/components/BrowserPanel.tsx | 2 +- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/apps/web/src/components/BrowserPanel.browser.tsx b/apps/web/src/components/BrowserPanel.browser.tsx index 3b49e9926..a19090208 100644 --- a/apps/web/src/components/BrowserPanel.browser.tsx +++ b/apps/web/src/components/BrowserPanel.browser.tsx @@ -257,7 +257,13 @@ describe("BrowserPanel interactions", () => { }, } as unknown as NativeApi; useBrowserStateStore.getState().upsertThreadState(openState); - const onClosePanel = vi.fn(); + const fallback = document.createElement("button"); + fallback.textContent = "Open Browser"; + fallback.dataset.browserPanelTestFallback = "true"; + document.body.append(fallback); + const onClosePanel = vi.fn((options?: { restoreFocus?: boolean }) => { + if (options?.restoreFocus) fallback.focus(); + }); await renderLivePanel(onClosePanel); const closeButton = await page.getByRole("button", { name: "Close Browser" }).element(); @@ -265,7 +271,38 @@ describe("BrowserPanel interactions", () => { await vi.waitFor(() => { expect(closeTab).toHaveBeenCalledWith({ threadId: THREAD_ID, tabId: "tab-1" }); - expect(onClosePanel).toHaveBeenCalledOnce(); + expect(onClosePanel).toHaveBeenCalledWith({ restoreFocus: true }); + expect(document.activeElement).toBe(fallback); + }); + }); + + it("moves focus to the adjacent tab when its visible close button closes the active tab", async () => { + const openState = browserState("tab-1"); + const closeTabState: ThreadBrowserState = { + ...openState, + version: openState.version + 1, + activeTabId: "tab-2", + tabs: openState.tabs.slice(1), + }; + const api = liveBrowserApi({ openState, closeTabState }); + nativeApiTestState.api = api; + useBrowserStateStore.getState().upsertThreadState(openState); + + await renderLivePanel(vi.fn()); + const secondTab = (await page + .getByRole("tab", { name: "Example" }) + .element()) as HTMLButtonElement; + const closeButton = (await page + .getByRole("button", { name: "Close tab: ScientFactory" }) + .element()) as HTMLButtonElement; + closeButton.click(); + + await vi.waitFor(() => { + expect(api.browser.closeTab).toHaveBeenCalledWith({ + threadId: THREAD_ID, + tabId: "tab-1", + }); + expect(document.activeElement).toBe(secondTab); }); }); diff --git a/apps/web/src/components/BrowserPanel.tsx b/apps/web/src/components/BrowserPanel.tsx index d1807034d..26064e153 100644 --- a/apps/web/src/components/BrowserPanel.tsx +++ b/apps/web/src/components/BrowserPanel.tsx @@ -1667,7 +1667,7 @@ export function BrowserPanel({ tabIndex={-1} onClick={(event) => { event.stopPropagation(); - onCloseTab(tab.id); + onCloseTab(tab.id, { restoreTabFocus: true }); }} > From 528f204ed17f4ce4eacd0af75ef2223685e2404a Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 19:11:14 +0300 Subject: [PATCH 14/16] Complete browser close focus recovery --- .../src/components/BrowserPanel.browser.tsx | 73 ++++++++++++++++++- apps/web/src/components/BrowserPanel.tsx | 13 +++- 2 files changed, 82 insertions(+), 4 deletions(-) diff --git a/apps/web/src/components/BrowserPanel.browser.tsx b/apps/web/src/components/BrowserPanel.browser.tsx index a19090208..5bf34bbee 100644 --- a/apps/web/src/components/BrowserPanel.browser.tsx +++ b/apps/web/src/components/BrowserPanel.browser.tsx @@ -262,7 +262,9 @@ describe("BrowserPanel interactions", () => { fallback.dataset.browserPanelTestFallback = "true"; document.body.append(fallback); const onClosePanel = vi.fn((options?: { restoreFocus?: boolean }) => { - if (options?.restoreFocus) fallback.focus(); + if (options?.restoreFocus) { + window.requestAnimationFrame(() => fallback.focus()); + } }); await renderLivePanel(onClosePanel); @@ -306,6 +308,75 @@ describe("BrowserPanel interactions", () => { }); }); + it("does not reclaim focus when an asynchronous tab close finishes after focus moved away", async () => { + const openState = browserState("tab-1"); + const closeTabState: ThreadBrowserState = { + ...openState, + version: openState.version + 1, + activeTabId: "tab-2", + tabs: openState.tabs.slice(1), + }; + let resolveCloseTab: ((state: ThreadBrowserState) => void) | undefined; + const closeTabResult = new Promise((resolve) => { + resolveCloseTab = resolve; + }); + const api = liveBrowserApi({ openState, closeTabState }); + vi.mocked(api.browser.closeTab).mockImplementation(async () => closeTabResult); + nativeApiTestState.api = api; + useBrowserStateStore.getState().upsertThreadState(openState); + const outsideControl = document.createElement("button"); + outsideControl.textContent = "Composer"; + outsideControl.dataset.browserPanelTestFallback = "true"; + document.body.append(outsideControl); + + await renderLivePanel(vi.fn()); + const closeButton = (await page + .getByRole("button", { name: "Close tab: ScientFactory" }) + .element()) as HTMLButtonElement; + closeButton.focus(); + closeButton.click(); + await vi.waitFor(() => expect(api.browser.closeTab).toHaveBeenCalledOnce()); + outsideControl.focus(); + resolveCloseTab?.(closeTabState); + + await vi.waitFor(() => { + expect(useBrowserStateStore.getState().threadStatesByThreadId[THREAD_ID]?.activeTabId).toBe( + "tab-2", + ); + }); + expect(document.activeElement).toBe(outsideControl); + }); + + it("restores parent focus when the Browser actions menu closes the panel", async () => { + const openState = browserState("tab-1"); + const api = liveBrowserApi({ openState }); + nativeApiTestState.api = api; + const fallback = document.createElement("button"); + fallback.textContent = "Open Browser"; + fallback.dataset.browserPanelTestFallback = "true"; + document.body.append(fallback); + const onClosePanel = vi.fn((options?: { restoreFocus?: boolean }) => { + if (options?.restoreFocus) { + window.requestAnimationFrame(() => fallback.focus()); + } + }); + + await renderLivePanel(onClosePanel); + ( + (await page.getByRole("button", { name: "Browser actions" }).element()) as HTMLButtonElement + ).click(); + const closePanelItemLocator = page.getByRole("menuitem", { name: "Close browser panel" }); + await expect.element(closePanelItemLocator).toBeVisible(); + const closePanelItem = (await closePanelItemLocator.element()) as HTMLElement; + closePanelItem.focus(); + await userEvent.keyboard("{Enter}"); + + await vi.waitFor(() => { + expect(onClosePanel).toHaveBeenCalledWith({ restoreFocus: true }); + expect(document.activeElement).toBe(fallback); + }); + }); + it("creates and activates a second tab from the visible tab-strip button", async () => { const openState = browserState("tab-1"); openState.version = 20; diff --git a/apps/web/src/components/BrowserPanel.tsx b/apps/web/src/components/BrowserPanel.tsx index 26064e153..cc39e0723 100644 --- a/apps/web/src/components/BrowserPanel.tsx +++ b/apps/web/src/components/BrowserPanel.tsx @@ -1269,6 +1269,7 @@ export function BrowserPanel({ if (!api) { return; } + const focusOrigin = document.activeElement; const closingTab = threadBrowserState?.tabs.find((tab) => tab.id === tabId); void runBrowserAction(async () => { if (closingTab?.kind === "artifact") { @@ -1282,7 +1283,13 @@ export function BrowserPanel({ return; } upsertThreadState(state); - if (options?.restoreTabFocus && state.activeTabId) { + const activeElement = document.activeElement; + const shouldRestoreTabFocus = + options?.restoreTabFocus === true && + (activeElement === focusOrigin || + activeElement === document.body || + activeElement === null); + if (shouldRestoreTabFocus && state.activeTabId) { const nextActiveIndex = state.tabs.findIndex((tab) => tab.id === state.activeTabId); if (nextActiveIndex >= 0) { window.requestAnimationFrame(() => { @@ -1291,7 +1298,7 @@ export function BrowserPanel({ } } if (shouldCloseBrowserPanelAfterTabClose(state)) { - onClosePanel({ restoreFocus: options?.restoreTabFocus === true }); + onClosePanel({ restoreFocus: shouldRestoreTabFocus }); } }); }, @@ -1563,7 +1570,7 @@ export function BrowserPanel({ onClosePanel()} + onClick={() => onClosePanel({ restoreFocus: true })} > Close browser panel From 2568520e4d2c33858e2a1ba4426d5160caae4236 Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 19:35:06 +0300 Subject: [PATCH 15/16] Handle early Electron test interruptions --- .../scripts/run-browser-overlay-lifecycle.mjs | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs b/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs index 2c7a7b031..1a2d7f1f8 100644 --- a/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs +++ b/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs @@ -12,6 +12,7 @@ const scriptDir = dirname(fileURLToPath(import.meta.url)); const desktopDir = resolve(scriptDir, ".."); const workspaceRoot = resolve(desktopDir, "../.."); const tempDir = mkdtempSync(join(tmpdir(), "scient-browser-overlay-lifecycle-")); +let child = null; function cleanupTempDir() { rmSync(tempDir, { recursive: true, force: true }); @@ -49,6 +50,17 @@ function cleanupTemporaryState() { process.removeListener("exit", cleanupTempDir); process.once("exit", cleanupTemporaryState); +function interruptSetup(signal, code) { + process.stderr.write(`Electron browser overlay lifecycle test interrupted by ${signal}.\n`); + killChildTree(); + cleanupTemporaryState(); + process.exit(code); +} + +let handleInterrupt = interruptSetup; +process.once("SIGINT", () => handleInterrupt("SIGINT", 130)); +process.once("SIGTERM", () => handleInterrupt("SIGTERM", 143)); + function createMacTestElectronExecutable(originalExecutablePath) { const originalAppPath = dirname(dirname(dirname(originalExecutablePath))); const testAppPath = join(tempDir, "ScientBrowserOverlayTest.app"); @@ -131,7 +143,7 @@ const electronArgs = [ electronOutputPath, ...(process.platform === "darwin" ? ["-ApplePersistenceIgnoreState", "YES"] : []), ]; -const child = spawn(electronExecutablePath, electronArgs, { +child = spawn(electronExecutablePath, electronArgs, { cwd: workspaceRoot, detached: process.platform !== "win32", env: { @@ -157,7 +169,7 @@ child.stderr.on("data", (chunk) => { }); function killChildTree() { - if (!child.pid || child.exitCode !== null || child.signalCode !== null) return true; + if (!child?.pid || child.exitCode !== null || child.signalCode !== null) return true; if (process.platform === "win32") { const result = spawnSync("taskkill", ["/pid", String(child.pid), "/t", "/f"], { encoding: "utf8", @@ -195,11 +207,13 @@ function interrupt(signal, code) { requestedExitCode = code; process.stderr.write(`Electron browser overlay lifecycle test interrupted by ${signal}.\n`); killChildTree(); + // Do not defer filesystem cleanup until child exit: interruption can also + // occur before child listeners are installed, and the wrapper may be exiting. + cleanupTemporaryState(); setTimeout(() => finish(code), 2_000).unref(); } -process.once("SIGINT", () => interrupt("SIGINT", 130)); -process.once("SIGTERM", () => interrupt("SIGTERM", 143)); +handleInterrupt = interrupt; process.once("uncaughtException", (error) => { process.stderr.write(`${error instanceof Error ? error.stack : String(error)}\n`); interrupt("uncaughtException", 1); From fe51e9fef01be5f5364d21f414ecc244d53b87af Mon Sep 17 00:00:00 2001 From: Yaacov Date: Fri, 24 Jul 2026 19:47:31 +0300 Subject: [PATCH 16/16] Cover earliest Electron setup interruption --- .../scripts/run-browser-overlay-lifecycle.mjs | 27 +++++++-- .../src/browserOverlayLifecycleRunner.test.ts | 59 +++++++++++++++++++ 2 files changed, 81 insertions(+), 5 deletions(-) create mode 100644 apps/desktop/src/browserOverlayLifecycleRunner.test.ts diff --git a/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs b/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs index 1a2d7f1f8..43db0ed08 100644 --- a/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs +++ b/apps/desktop/scripts/run-browser-overlay-lifecycle.mjs @@ -11,16 +11,35 @@ import { resolveElectronPackagePath, resolveLinuxSandboxArgs } from "./electron- const scriptDir = dirname(fileURLToPath(import.meta.url)); const desktopDir = resolve(scriptDir, ".."); const workspaceRoot = resolve(desktopDir, "../.."); -const tempDir = mkdtempSync(join(tmpdir(), "scient-browser-overlay-lifecycle-")); +let tempDir = null; let child = null; function cleanupTempDir() { - rmSync(tempDir, { recursive: true, force: true }); + if (!tempDir) return; + const path = tempDir; + rmSync(path, { recursive: true, force: true }); + tempDir = null; } +let handleInterrupt = (signal, code) => { + process.stderr.write(`Electron browser overlay lifecycle test interrupted by ${signal}.\n`); + cleanupTempDir(); + process.exit(code); +}; +process.once("SIGINT", () => handleInterrupt("SIGINT", 130)); +process.once("SIGTERM", () => handleInterrupt("SIGTERM", 143)); + +tempDir = mkdtempSync(join(tmpdir(), "scient-browser-overlay-lifecycle-")); // Register cleanup before any other setup can fail after creating the directory. process.once("exit", cleanupTempDir); +const setupSignalProbePath = process.env.SCIENT_BROWSER_OVERLAY_TEST_SETUP_SIGNAL_PROBE?.trim(); +if (setupSignalProbePath) { + writeFileSync(setupSignalProbePath, tempDir, "utf8"); + await new Promise((resolveProbe) => setTimeout(resolveProbe, 30_000)); + throw new Error("Setup signal probe was not interrupted."); +} + const profileDir = join(tempDir, "profile"); mkdirSync(join(profileDir, "session-data"), { recursive: true }); // Allow both bounded 10s fixture startup waits, the required >30s overlay hold, @@ -57,9 +76,7 @@ function interruptSetup(signal, code) { process.exit(code); } -let handleInterrupt = interruptSetup; -process.once("SIGINT", () => handleInterrupt("SIGINT", 130)); -process.once("SIGTERM", () => handleInterrupt("SIGTERM", 143)); +handleInterrupt = interruptSetup; function createMacTestElectronExecutable(originalExecutablePath) { const originalAppPath = dirname(dirname(dirname(originalExecutablePath))); diff --git a/apps/desktop/src/browserOverlayLifecycleRunner.test.ts b/apps/desktop/src/browserOverlayLifecycleRunner.test.ts new file mode 100644 index 000000000..42a4b60cc --- /dev/null +++ b/apps/desktop/src/browserOverlayLifecycleRunner.test.ts @@ -0,0 +1,59 @@ +import { spawn } from "node:child_process"; +import { once } from "node:events"; +import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join, resolve } from "node:path"; +import { setTimeout as delay } from "node:timers/promises"; +import { fileURLToPath } from "node:url"; + +import { expect, it } from "vitest"; + +const runnerPath = resolve( + dirname(fileURLToPath(import.meta.url)), + "../scripts/run-browser-overlay-lifecycle.mjs", +); + +async function waitForFile(path: string, timeoutMs: number): Promise { + const deadline = Date.now() + timeoutMs; + while (!existsSync(path)) { + if (Date.now() >= deadline) throw new Error(`Timed out waiting for ${path}`); + await delay(10); + } +} + +it.skipIf(process.platform === "win32")( + "cleans temporary state when interrupted during earliest setup", + async () => { + const probeRoot = mkdtempSync(join(tmpdir(), "scient-browser-overlay-signal-test-")); + const markerPath = join(probeRoot, "ready"); + const child = spawn(process.execPath, [runnerPath], { + env: { + ...process.env, + SCIENT_BROWSER_OVERLAY_TEST_SETUP_SIGNAL_PROBE: markerPath, + }, + stdio: ["ignore", "ignore", "pipe"], + }); + let stderr = ""; + child.stderr.on("data", (chunk) => { + stderr += chunk.toString(); + }); + + try { + await waitForFile(markerPath, 5_000); + const harnessTempDir = readFileSync(markerPath, "utf8"); + const exit = once(child, "exit"); + expect(child.kill("SIGTERM")).toBe(true); + const [code, signal] = await exit; + + // Bun reports a handled SIGTERM as the terminating signal while Node + // reports the explicit 143 exit code used by the same handler. + expect(code === 143 || signal === "SIGTERM").toBe(true); + expect(stderr).toContain("interrupted by SIGTERM"); + expect(existsSync(harnessTempDir)).toBe(false); + } finally { + if (child.exitCode === null && child.signalCode === null) child.kill("SIGKILL"); + rmSync(probeRoot, { recursive: true, force: true }); + } + }, + 10_000, +);