From dc3df796f19080f572845e1fb7442b41a6902ab0 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Thu, 16 Apr 2026 13:08:51 -0700 Subject: [PATCH] fix: disconnect handler runs full cleanup before exiting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the user closed the headed browser window, the disconnect handler in browser-manager.ts called process.exit(2) directly, bypassing the server's shutdown() function entirely. That meant: - sidebar-agent daemon kept polling a dead server - session state wasn't saved - Chromium profile locks (SingletonLock, SingletonSocket, SingletonCookie) weren't cleaned — causing "profile in use" errors on next $B connect - state file at .gstack/browse.json was left stale Now the disconnect handler calls onDisconnect(), which server.ts wires up to shutdown(2). Full cleanup runs first, then the process exits with code 2 — preserving the existing semantic that distinguishes user-close (exit 2) from crashes (exit 1). shutdown() now accepts an optional exitCode parameter (default 0) so the SIGTERM/SIGINT paths and the disconnect path can share cleanup code while preserving their distinct exit codes. Surfaced by Codex during /plan-eng-review of the watchdog fix. Co-Authored-By: Claude Opus 4.7 (1M context) --- browse/src/browser-manager.ts | 16 ++++++++++++++-- browse/src/server.ts | 8 ++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/browse/src/browser-manager.ts b/browse/src/browser-manager.ts index 63d78358..51c7348d 100644 --- a/browse/src/browser-manager.ts +++ b/browse/src/browser-manager.ts @@ -72,6 +72,11 @@ export class BrowserManager { private connectionMode: 'launched' | 'headed' = 'launched'; private intentionalDisconnect = false; + // Called when the headed browser disconnects without intentional teardown + // (user closed the window). Wired up by server.ts to run full cleanup + // (sidebar-agent, state file, profile locks) before exiting with code 2. + public onDisconnect: (() => void) | null = null; + getConnectionMode(): 'launched' | 'headed' { return this.connectionMode; } // ─── Watch Mode Methods ───────────────────────────────── @@ -467,13 +472,20 @@ export class BrowserManager { await this.newTab(); } - // Browser disconnect handler — exit code 2 distinguishes from crashes (1) + // Browser disconnect handler — exit code 2 distinguishes from crashes (1). + // Calls onDisconnect() to trigger full shutdown (kill sidebar-agent, save + // session, clean profile locks + state file) before exit. Falls back to + // direct process.exit(2) if no callback is wired up. if (this.browser) { this.browser.on('disconnected', () => { if (this.intentionalDisconnect) return; console.error('[browse] Real browser disconnected (user closed or crashed).'); console.error('[browse] Run `$B connect` to reconnect.'); - process.exit(2); + if (this.onDisconnect) { + this.onDisconnect(); + } else { + process.exit(2); + } }); } diff --git a/browse/src/server.ts b/browse/src/server.ts index 4982a62c..81548c1b 100644 --- a/browse/src/server.ts +++ b/browse/src/server.ts @@ -805,6 +805,10 @@ function emitInspectorEvent(event: any): void { // ─── Server ──────────────────────────────────────────────────── const browserManager = new BrowserManager(); +// When the user closes the headed browser window, run full cleanup +// (kill sidebar-agent, save session, remove profile locks, delete state file) +// before exiting with code 2. Exit code 2 distinguishes user-close from crashes (1). +browserManager.onDisconnect = () => shutdown(2); let isShuttingDown = false; // Test if a port is available by binding and immediately releasing. @@ -1192,7 +1196,7 @@ async function handleCommand(body: any, tokenInfo?: TokenInfo | null): Promise