feat(browse): clean-drain disconnect detection + shutdown postmortem

browse/src/browser-manager.ts

@@ -40,6 +40,40 @@ export function isCustomChromium(): boolean {
   return p.includes('GBrowser') || p.includes('gbrowser');
 }
 
+/**
+ * Classify a Chromium `disconnected` event as a clean drain (the user just
+ * closed the last tab, Chromium had nothing left to display) vs a real crash.
+ *
+ * Before this distinction existed, every disconnect — including the totally
+ * benign "user closed last tab" path — printed `FATAL: Chromium process
+ * crashed` and `process.exit(1)`, which killed the sidebar-agent and any
+ * in-flight PTY work. The smoking gun in the postmortem log was the pair:
+ *
+ *   [browse] Tab closed (id=N, remaining=0)
+ *   [browse] FATAL: Chromium process crashed or was killed.
+ *
+ * Same event, two stories. After: `pages.size === 0` means the disconnect
+ * is the natural consequence of the close, exit 0, no FATAL. Pages still
+ * live means real crash, exit 1 — the original behavior, preserved.
+ *
+ * Pure function on purpose: unit-testable without spinning up Chromium.
+ */
+export type DisconnectKind = 'clean-drain' | 'crash';
+export interface DisconnectClassification {
+  kind: DisconnectKind;
+  reason: string;
+  exitCode: 0 | 1;
+}
+export function classifyDisconnect(opts: {
+  pagesSize: number;
+  mode: 'launched' | 'rehead';
+}): DisconnectClassification {
+  if (opts.pagesSize === 0) {
+    return { kind: 'clean-drain', reason: `browser-empty-${opts.mode}`, exitCode: 0 };
+  }
+  return { kind: 'crash', reason: `chromium-crash-${opts.mode}`, exitCode: 1 };
+}
+
 export type { RefEntry };
 
 // Re-export TabSession for consumers
@@ -119,9 +153,17 @@ export class BrowserManager {
 
   // 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.
-  // Returns void or a Promise; rejections are caught and fall back to exit(2).
-  public onDisconnect: (() => void | Promise<void>) | null = null;
+  // (sidebar-agent, state file, profile locks) before exiting.
+  // Returns void or a Promise; rejections are caught and fall back to the
+  // passed exit code.
+  // Reason is a short tag describing why the disconnect fired; server.ts
+  // writes it into ~/.gstack/last-shutdown.json for post-mortem diagnostics.
+  // exitCode is the process exit code the caller should use after cleanup:
+  // 0 = clean drain (last tab closed, Chromium had nothing to display),
+  // 1 = real crash (Chromium died with pages still live),
+  // 2 = user-explicit close (existing convention from the page-close chain;
+  //     also the default when no exit code is specified).
+  public onDisconnect: ((reason?: string, exitCode?: 0 | 1 | 2) => void | Promise<void>) | null = null;
 
   getConnectionMode(): 'launched' | 'headed' { return this.connectionMode; }
 
@@ -246,11 +288,24 @@ export class BrowserManager {
       ...(this.proxyConfig ? { proxy: this.proxyConfig } : {}),
     });
 
-    // Chromium crash → exit with clear message
+    // Chromium disconnect → classifyDisconnect (see top of file) splits the
+    // event into clean-drain (last tab closed, exit 0) vs real crash (exit 1).
     this.browser.on('disconnected', () => {
-      console.error('[browse] FATAL: Chromium process crashed or was killed. Server exiting.');
-      console.error('[browse] Console/network logs flushed to .gstack/browse-*.log');
-      process.exit(1);
+      const c = classifyDisconnect({ pagesSize: this.pages.size, mode: 'launched' });
+      if (c.kind === 'clean-drain') {
+        console.log('[browse] Chromium quit after last tab closed. Shutting down cleanly.');
+      } else {
+        console.error('[browse] FATAL: Chromium process crashed or was killed. Server exiting.');
+        console.error('[browse] Console/network logs flushed to .gstack/browse-*.log');
+      }
+      try {
+        const result = this.onDisconnect?.(c.reason, c.exitCode);
+        if (result && typeof (result as Promise<void>).catch === 'function') {
+          (result as Promise<void>).catch(() => process.exit(c.exitCode));
+          return;
+        }
+      } catch {}
+      process.exit(c.exitCode);
     });
 
     const contextOptions: BrowserContextOptions = {
@@ -557,7 +612,7 @@ export class BrowserManager {
           return;
         }
         try {
-          const result = this.onDisconnect();
+          const result = this.onDisconnect('browser-disconnect');
           if (result && typeof (result as Promise<void>).catch === 'function') {
             (result as Promise<void>).catch((err) => {
               console.error('[browse] onDisconnect rejected:', err);
@@ -1336,8 +1391,20 @@ export class BrowserManager {
       if (this.browser) {
         this.browser.on('disconnected', () => {
           if (this.intentionalDisconnect) return;
-          console.error('[browse] FATAL: Chromium process crashed or was killed. Server exiting.');
-          process.exit(1);
+          const c = classifyDisconnect({ pagesSize: this.pages.size, mode: 'rehead' });
+          if (c.kind === 'clean-drain') {
+            console.log('[browse] Chromium quit after last tab closed. Shutting down cleanly.');
+          } else {
+            console.error('[browse] FATAL: Chromium process crashed or was killed. Server exiting.');
+          }
+          try {
+            const result = this.onDisconnect?.(c.reason, c.exitCode);
+            if (result && typeof (result as Promise<void>).catch === 'function') {
+              (result as Promise<void>).catch(() => process.exit(c.exitCode));
+              return;
+            }
+          } catch {}
+          process.exit(c.exitCode);
         });
       }
 

browse/src/cli.ts

@@ -241,10 +241,32 @@ async function startServer(extraEnv?: Record<string, string>): Promise<ServerSta
     Bun.spawnSync(['node', '-e', launcherCode], { stdio: ['ignore', 'ignore', 'ignore'] });
   } else {
     // macOS/Linux: Bun.spawn + unref works correctly
+    // Pipe server stdout/stderr to an append log so output survives after
+    // this CLI exits. Without this, proc.unref() leaves the server writing
+    // to FDs that nothing reads — every console.log/error from the
+    // detached server is lost, and post-mortem debugging is impossible.
+    const stateDir = path.dirname(config.stateFile);
+    const logPath = path.join(stateDir, 'browse-server.log');
+    let logFd: number = -1;
+    try {
+      fs.mkdirSync(stateDir, { recursive: true });
+      logFd = fs.openSync(logPath, 'a');
+      fs.writeSync(logFd, `\n=== ${new Date().toISOString()} CLI pid=${process.pid} parentPid=${parentPid} ===\n`);
+    } catch (err: any) {
+      console.warn(`[browse] Could not open server log ${logPath}: ${err?.message}`);
+    }
+    // Tee strategy: when we have a logFd, point the child's stdio at the
+    // file so output survives after this CLI exits. Otherwise fall back
+    // to 'pipe' so the startup-failure path below can still read stderr
+    // via proc.stderr.getReader().
     proc = Bun.spawn(['bun', 'run', SERVER_SCRIPT], {
-      stdio: ['ignore', 'pipe', 'pipe'],
+      stdio: ['ignore', logFd >= 0 ? logFd : 'pipe', logFd >= 0 ? logFd : 'pipe'],
       env: { ...process.env, BROWSE_STATE_FILE: config.stateFile, BROWSE_PARENT_PID: parentPid, ...extraEnv },
     });
+    // Stash the log path for the startup-failure path (proc.stderr is not a
+    // pipe when stdio was redirected to a file fd, so getReader() would
+    // throw — read the tail of the log file instead).
+    if (logFd >= 0) (proc as any).__logPath = logPath;
     proc.unref();
   }
 
@@ -261,8 +283,31 @@ async function startServer(extraEnv?: Record<string, string>): Promise<ServerSta
   }
 
   // Server didn't start in time — try to get error details
-  if (proc?.stderr) {
-    // macOS/Linux: read stderr from the spawned process
+  const teeLogPath = (proc as any)?.__logPath as string | undefined;
+  if (teeLogPath) {
+    // stderr was redirected to a file fd (tee mode) — read the LAST 64KB
+    // of the log file for diagnostics. Avoid readFileSync on a file that
+    // can grow across many failed-startup runs (the file is opened in
+    // append mode and never rotated by this code path). proc.stderr.getReader()
+    // would also throw here because there's no pipe attached.
+    let fd: number | undefined;
+    try {
+      const stat = fs.statSync(teeLogPath);
+      const TAIL_BYTES = 64 * 1024;
+      const start = Math.max(0, stat.size - TAIL_BYTES);
+      const length = stat.size - start;
+      const buf = Buffer.alloc(length);
+      fd = fs.openSync(teeLogPath, 'r');
+      fs.readSync(fd, buf, 0, length, start);
+      const tail = buf.toString('utf-8').split('\n').slice(-30).join('\n').trim();
+      if (tail) throw new Error(`Server failed to start (last 30 lines of ${teeLogPath}):\n${tail}`);
+    } catch (e: any) {
+      if (e.code !== 'ENOENT') throw e;
+    } finally {
+      if (fd !== undefined) try { fs.closeSync(fd); } catch {}
+    }
+  } else if (proc?.stderr && typeof proc.stderr.getReader === 'function') {
+    // macOS/Linux without tee: read stderr from the spawned process
     const reader = proc.stderr.getReader();
     const { value } = await reader.read();
     if (value) {

browse/src/server.ts

@@ -583,6 +583,39 @@ async function flushBuffers() {
 // Flush every 1 second
 const flushInterval = setInterval(flushBuffers, 1000);
 
+// ─── Last-Shutdown Recording ───────────────────────────────────
+// Write a postmortem JSON before exit so the user can tell why the server
+// died. Overwritten each shutdown — most recent reason wins. Specific
+// callers (idle-timeout, parent-exit, signal handlers, browser-disconnect)
+// record their reason BEFORE calling activeShutdown; the shutdown()
+// function itself records 'shutdown-called' as a fallback so every exit
+// path leaves a trail.
+let shutdownReasonRecorded = false;
+function recordShutdownReason(reason: string, detail: Record<string, any> = {}) {
+  try {
+    const stateDir = path.dirname(config.stateFile);
+    const outPath = path.join(stateDir, 'last-shutdown.json');
+    const payload = {
+      ts: new Date().toISOString(),
+      pid: process.pid,
+      reason,
+      mode: browserManager.getConnectionMode(),
+      uptimeSeconds: Math.round(process.uptime()),
+      detail,
+    };
+    // Atomic write: two browse instances writing to the same state dir
+    // can otherwise interleave JSON bytes and leave a corrupt file.
+    // tmp-then-rename is the standard POSIX pattern.
+    const tmpPath = `${outPath}.${process.pid}.tmp`;
+    fs.writeFileSync(tmpPath, JSON.stringify(payload, null, 2));
+    fs.renameSync(tmpPath, outPath);
+    shutdownReasonRecorded = true;
+    console.log(`[browse] Shutdown reason recorded: ${reason}`);
+  } catch (err: any) {
+    console.warn('[browse] Failed to record shutdown reason:', err?.message);
+  }
+}
+
 // ─── Idle Timer ────────────────────────────────────────────────
 let lastActivity = Date.now();
 
@@ -598,6 +631,7 @@ const idleCheckInterval = setInterval(() => {
   if (tunnelActive) return;
   if (Date.now() - lastActivity > IDLE_TIMEOUT_MS) {
     console.log(`[browse] Idle for ${IDLE_TIMEOUT_MS / 1000}s, shutting down`);
+    recordShutdownReason('idle-timeout', { idleSeconds: Math.round((Date.now() - lastActivity) / 1000) });
     activeShutdown?.();
   }
 }, 60_000);
@@ -641,6 +675,7 @@ if (BROWSE_PARENT_PID > 0 && !IS_HEADED_WATCHDOG) {
       const headed = browserManager.getConnectionMode() === 'headed';
       if (headed || tunnelActive) {
         console.log(`[browse] Parent process ${BROWSE_PARENT_PID} exited in ${headed ? 'headed' : 'tunnel'} mode, shutting down`);
+        recordShutdownReason('parent-exit', { parentPid: BROWSE_PARENT_PID, mode: headed ? 'headed' : 'tunnel' });
         activeShutdown?.();
       } else if (!parentGone) {
         parentGone = true;
@@ -678,10 +713,21 @@ 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 = () => activeShutdown?.(2);
+// Exit-code semantics (passed by browser-manager.ts via classifyDisconnect
+// for the Chromium 'disconnected' paths, or unset for the headed-mode
+// page-close chain that uses the existing user-close convention):
+//   0 — clean drain (Chromium quit after last tab closed)
+//   1 — real crash (Chromium died with pages still live, preserves the
+//                   legacy FATAL exit convention)
+//   2 — user-explicit close from the page-close chain (default when no
+//       exit code is passed)
+// activeShutdown runs the same full cleanup (kill sidebar-agent, save
+// session, remove profile locks, delete state file) regardless.
+browserManager.onDisconnect = (reason?: string, exitCode?: 0 | 1 | 2) => {
+  const code = exitCode ?? 2;
+  recordShutdownReason(reason || 'browser-disconnect', { exitCode: code });
+  return activeShutdown?.(code);
+};
 let isShuttingDown = false;
 
 // Test if a port is available by binding and immediately releasing.
@@ -1164,7 +1210,10 @@ async function handleCommand(body: any, tokenInfo?: TokenInfo | null): Promise<R
 // fighting with gstack's. CLI path is unchanged.
 if (import.meta.main) {
   // SIGINT (Ctrl+C): user intentionally stopping → shutdown.
-  process.on('SIGINT', () => activeShutdown?.());
+  process.on('SIGINT', () => {
+    recordShutdownReason('signal', { signal: 'SIGINT' });
+    activeShutdown?.();
+  });
   // SIGTERM behavior depends on mode:
   // - Normal (headless) mode: Claude Code's Bash sandbox fires SIGTERM when the
   //   parent shell exits between tool invocations. Ignoring it keeps the server
@@ -1182,6 +1231,7 @@ if (import.meta.main) {
     const headed = browserManager.getConnectionMode() === 'headed';
     if (headed || tunnelActive) {
       console.log(`[browse] Received SIGTERM in ${headed ? 'headed' : 'tunnel'} mode, shutting down`);
+      recordShutdownReason('signal', { signal: 'SIGTERM', mode: headed ? 'headed' : 'tunnel' });
       activeShutdown?.();
     } else {
       console.log('[browse] Received SIGTERM (ignoring — use /stop or Ctrl+C for intentional shutdown)');
@@ -1313,6 +1363,16 @@ export function buildFetchHandler(cfg: ServerConfig): ServerHandle {
     if (isShuttingDown) return;
     isShuttingDown = true;
 
+    // Fallback diagnostics: if no caller recorded a reason yet (e.g. /stop,
+    // explicit $B disconnect, or a future code path we forgot to wire),
+    // log something rather than exiting silently. Specific callers
+    // (idle-timeout, parent-exit, signal handlers, browser-disconnect)
+    // record their own more-specific reason BEFORE calling activeShutdown,
+    // and recordShutdownReason overwrites by design — most recent wins.
+    if (!shutdownReasonRecorded) {
+      recordShutdownReason('shutdown-called', { exitCode });
+    }
+
     console.log('[browse] Shutting down...');
     if (ownsTerminalAgent) {
       try {

browse/test/browser-manager-classify-disconnect.test.ts

@@ -0,0 +1,57 @@
+import { describe, test, expect } from 'bun:test';
+import { classifyDisconnect } from '../src/browser-manager';
+
+// Pins the FATAL-on-clean-tab-close regression: pages.size === 0 at
+// disconnect time means the user closed the last tab, not that Chromium
+// crashed. Before this split, both paths exited 1 and the daemon log
+// printed `FATAL: Chromium process crashed`, killing the sidebar-agent
+// every time a user closed their last tab in headed mode.
+
+describe('classifyDisconnect — clean-drain vs crash', () => {
+  test('launched mode, zero pages → clean-drain, exit 0, browser-empty-launched', () => {
+    expect(classifyDisconnect({ pagesSize: 0, mode: 'launched' })).toEqual({
+      kind: 'clean-drain',
+      reason: 'browser-empty-launched',
+      exitCode: 0,
+    });
+  });
+
+  test('launched mode, pages still live → crash, exit 1, chromium-crash-launched', () => {
+    expect(classifyDisconnect({ pagesSize: 1, mode: 'launched' })).toEqual({
+      kind: 'crash',
+      reason: 'chromium-crash-launched',
+      exitCode: 1,
+    });
+    expect(classifyDisconnect({ pagesSize: 5, mode: 'launched' })).toEqual({
+      kind: 'crash',
+      reason: 'chromium-crash-launched',
+      exitCode: 1,
+    });
+  });
+
+  test('rehead mode, zero pages → clean-drain, exit 0, browser-empty-rehead', () => {
+    expect(classifyDisconnect({ pagesSize: 0, mode: 'rehead' })).toEqual({
+      kind: 'clean-drain',
+      reason: 'browser-empty-rehead',
+      exitCode: 0,
+    });
+  });
+
+  test('rehead mode, pages still live → crash, exit 1, chromium-crash-rehead', () => {
+    expect(classifyDisconnect({ pagesSize: 3, mode: 'rehead' })).toEqual({
+      kind: 'crash',
+      reason: 'chromium-crash-rehead',
+      exitCode: 1,
+    });
+  });
+
+  test('reason tag always carries the mode for postmortem disambiguation', () => {
+    // ~/.gstack/last-shutdown.json reads this string. Tag must include
+    // the mode so an embedder log can tell launched-disconnect from
+    // rehead-disconnect at a glance.
+    const launched = classifyDisconnect({ pagesSize: 0, mode: 'launched' });
+    const rehead = classifyDisconnect({ pagesSize: 0, mode: 'rehead' });
+    expect(launched.reason.endsWith('-launched')).toBe(true);
+    expect(rehead.reason.endsWith('-rehead')).toBe(true);
+  });
+});