From 3aab7654174cffd98f046276a964aabdad9344f0 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 14 Aug 2026 18:46:46 -0700 Subject: [PATCH] =?UTF-8?q?fix(browse):=20capture=20active-tab=20state=20b?= =?UTF-8?q?efore=20close()=20=E2=80=94=20last-tab=20auto-create=20raced=20?= =?UTF-8?q?the=20close=20event?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closeTab checked `tabId === this.activeTabId` AFTER awaiting page.close(), but the page 'close' event handler can fire during that await and reassign activeTabId — losing the race meant the last-tab auto-create never ran, leaving the manager with zero tabs. Capture wasActive before closing, and only reassign activeTabId when it no longer points at a live tab. Part of the test-integrity repairs unmasked by the suite-truncation fix. Contributed by @time-attack (PR #2230, browser-manager hunk). Co-Authored-By: Claude Fable 5 --- browse/src/browser-manager.ts | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/browse/src/browser-manager.ts b/browse/src/browser-manager.ts index 4b378cc4f..203fc88c4 100644 --- a/browse/src/browser-manager.ts +++ b/browse/src/browser-manager.ts @@ -798,19 +798,31 @@ export class BrowserManager { const page = this.pages.get(tabId); if (!page) throw new Error(`Tab ${tabId} not found`); + // Capture BEFORE close(): the page 'close' event handler wired in + // wirePageEvents() can fire while page.close() is awaited. It removes + // the tab from the maps and reassigns activeTabId (to 0 when no tabs + // remain), so a post-close `tabId === this.activeTabId` check is + // order-dependent — whether the event dispatches before or after + // close() resolves varies across Playwright/Chromium versions and + // machines, and losing the race means the last-tab auto-create below + // never runs, leaving the manager with zero tabs. + const wasActive = tabId === this.activeTabId; + await page.close(); this.pages.delete(tabId); this.tabSessions.delete(tabId); this.tabOwnership.delete(tabId); // Switch to another tab if we closed the active one - if (tabId === this.activeTabId) { + if (wasActive) { const remaining = [...this.pages.keys()]; - if (remaining.length > 0) { - this.activeTabId = remaining[remaining.length - 1]; - } else { + if (remaining.length === 0) { // No tabs left — create a new blank one await this.newTab(); + } else if (!this.pages.has(this.activeTabId)) { + // The 'close' handler may have already switched to a valid tab; + // only reassign when activeTabId no longer points at a live tab. + this.activeTabId = remaining[remaining.length - 1]; } } }