Merge origin/main into garrytan/tilde-fix-design

Resolves overlap between main #1025 (headed browser auto-shutdown fix) and
this wave's #994 (server persists across Bash calls) + our SIGTERM mode-gate
follow-up. Both touch the parent-PID watchdog and SIGTERM handlers.

Mechanical resolution only — test inversion follows in next commit.

CHANGELOG / VERSION:
- main shipped v0.18.1.0 with #1025 while this wave was open. Bumped this
  branch to v0.18.2.0 with its own entry on top, per CLAUDE.md branch-scoped
  CHANGELOG rules. Both entries preserved, contiguous version sequence.
- package.json synced to v0.18.2.0.

browse/src/server.ts (parent-PID watchdog):
- Combined main's outer env-var gate (BROWSE_HEADED=1 skips the watchdog
  entirely) with our inner mode check (when watchdog runs and parent dies,
  decision tree by mode: active picker > headed/tunnel > normal).
- Defense-in-depth: env var skips the setInterval cost, runtime mode check
  catches anything the env var missed.

browse/src/server.ts (SIGTERM/SIGINT handlers):
- Combined main's lambda wrap (avoids signal name being passed as exitCode
  → NaN → exit 1) with our mode-aware SIGTERM handler. Active picker check
  short-circuits regardless of mode.
- Wrapped SIGINT in a lambda too — main's #1025 fix applied to that handler,
  but our wave didn't have it. Now SIGINT exits with code 0 reliably.

Note: browse/test/watchdog.test.ts test 3 (added by main) currently FAILS
because main's "watchdog kills on parent death" expectation contradicts
post-#994 "server stays alive on parent death." Inverted in the next commit.

browse/src/browser-manager.ts

@@ -72,6 +72,12 @@ 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.
+  // Returns void or a Promise; rejections are caught and fall back to exit(2).
+  public onDisconnect: (() => void | Promise<void>) | null = null;
+
   getConnectionMode(): 'launched' | 'headed' { return this.connectionMode; }
 
   // ─── Watch Mode Methods ─────────────────────────────────
@@ -467,13 +473,32 @@ 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, or if the callback
+    // throws/rejects — never leave the process running with a dead browser.
     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) {
+          process.exit(2);
+          return;
+        }
+        try {
+          const result = this.onDisconnect();
+          if (result && typeof (result as Promise<void>).catch === 'function') {
+            (result as Promise<void>).catch((err) => {
+              console.error('[browse] onDisconnect rejected:', err);
+              process.exit(2);
+            });
+          }
+        } catch (err) {
+          console.error('[browse] onDisconnect threw:', err);
+          process.exit(2);
+        }
       });
     }
 

browse/src/cli.ts

@@ -210,12 +210,20 @@ async function startServer(extraEnv?: Record<string, string>): Promise<ServerSta
 
   let proc: any = null;
 
+  // Allow the caller to opt out of the parent-process watchdog by setting
+  // BROWSE_PARENT_PID=0 in the environment. Useful for CI, non-interactive
+  // shells, and short-lived Bash invocations that need the server to outlive
+  // the spawning CLI. Defaults to the current process PID (watchdog active).
+  // Parse as int so stray whitespace ("0\n") still opts out — matches the
+  // server's own parseInt at server.ts:760.
+  const parentPid = parseInt(process.env.BROWSE_PARENT_PID || '', 10) === 0 ? '0' : String(process.pid);
+
   if (IS_WINDOWS && NODE_SERVER_SCRIPT) {
     // Windows: Bun.spawn() + proc.unref() doesn't truly detach on Windows —
     // when the CLI exits, the server dies with it. Use Node's child_process.spawn
     // with { detached: true } instead, which is the gold standard for Windows
     // process independence. Credit: PR #191 by @fqueiro.
-    const extraEnvStr = JSON.stringify({ BROWSE_STATE_FILE: config.stateFile, BROWSE_PARENT_PID: String(process.pid), ...(extraEnv || {}) });
+    const extraEnvStr = JSON.stringify({ BROWSE_STATE_FILE: config.stateFile, BROWSE_PARENT_PID: parentPid, ...(extraEnv || {}) });
     const launcherCode =
       `const{spawn}=require('child_process');` +
       `spawn(process.execPath,[${JSON.stringify(NODE_SERVER_SCRIPT)}],` +
@@ -226,7 +234,7 @@ async function startServer(extraEnv?: Record<string, string>): Promise<ServerSta
     // macOS/Linux: Bun.spawn + unref works correctly
     proc = Bun.spawn(['bun', 'run', SERVER_SCRIPT], {
       stdio: ['ignore', 'pipe', 'pipe'],
-      env: { ...process.env, BROWSE_STATE_FILE: config.stateFile, BROWSE_PARENT_PID: String(process.pid), ...extraEnv },
+      env: { ...process.env, BROWSE_STATE_FILE: config.stateFile, BROWSE_PARENT_PID: parentPid, ...extraEnv },
     });
     proc.unref();
   }
@@ -826,12 +834,12 @@ Refs:           After 'snapshot', use @e1, @e2... as selectors:
         BROWSE_HEADED: '1',
         BROWSE_PORT: '34567',
         BROWSE_SIDEBAR_CHAT: '1',
+        // Disable parent-process watchdog: the user controls the headed browser
+        // window lifecycle. The CLI exits immediately after connect, so watching
+        // it would kill the server ~15s later. Cleanup happens via browser
+        // disconnect event or $B disconnect.
+        BROWSE_PARENT_PID: '0',
       };
-      // If parent explicitly set BROWSE_PARENT_PID=0 (pair-agent disabling
-      // self-termination), pass it through so startServer doesn't override it.
-      if (process.env.BROWSE_PARENT_PID === '0') {
-        serverEnv.BROWSE_PARENT_PID = '0';
-      }
       const newState = await startServer(serverEnv);
 
       // Print connected status

browse/src/server.ts

@@ -757,8 +757,20 @@ const idleCheckInterval = setInterval(() => {
 // server can become an orphan — keeping chrome-headless-shell alive and
 // causing console-window flicker on Windows. Poll the parent PID every 15s
 // and self-terminate if it is gone.
+//
+// Headed mode (BROWSE_HEADED=1 or BROWSE_PARENT_PID=0): The user controls
+// the browser window lifecycle. The CLI exits immediately after connect,
+// so the watchdog would kill the server prematurely. Disabled in both cases
+// as defense-in-depth — the CLI sets PID=0 for headed mode, and the server
+// also checks BROWSE_HEADED in case a future launcher forgets.
+// Cleanup happens via browser disconnect event or $B disconnect.
 const BROWSE_PARENT_PID = parseInt(process.env.BROWSE_PARENT_PID || '0', 10);
-if (BROWSE_PARENT_PID > 0) {
+// Outer gate: if the spawner explicitly marks this as headed (env var set at
+// launch time), skip registering the watchdog entirely. Cheaper than entering
+// the closure every 15s. The CLI's connect path sets BROWSE_HEADED=1 + PID=0,
+// so this branch is the normal path for /open-gstack-browser.
+const IS_HEADED_WATCHDOG = process.env.BROWSE_HEADED === '1';
+if (BROWSE_PARENT_PID > 0 && !IS_HEADED_WATCHDOG) {
   let parentGone = false;
   setInterval(() => {
     try {
@@ -786,6 +798,10 @@ if (BROWSE_PARENT_PID > 0) {
       }
     }
   }, 15_000);
+} else if (IS_HEADED_WATCHDOG) {
+  console.log('[browse] Parent-process watchdog disabled (headed mode)');
+} else if (BROWSE_PARENT_PID === 0) {
+  console.log('[browse] Parent-process watchdog disabled (BROWSE_PARENT_PID=0)');
 }
 
 // ─── Command Sets (from commands.ts — single source of truth) ───
@@ -812,6 +828,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.
@@ -1199,7 +1219,7 @@ async function handleCommand(body: any, tokenInfo?: TokenInfo | null): Promise<R
   });
 }
 
-async function shutdown() {
+async function shutdown(exitCode: number = 0) {
   if (isShuttingDown) return;
   isShuttingDown = true;
 
@@ -1240,12 +1260,18 @@ async function shutdown() {
   // Clean up state file
   safeUnlinkQuiet(config.stateFile);
 
-  process.exit(0);
+  process.exit(exitCode);
 }
 
 // Handle signals
-// SIGINT (Ctrl+C): user intentionally stopping → shutdown
-process.on('SIGINT', shutdown);
+//
+// Node passes the signal name (e.g. 'SIGTERM') as the first arg to listeners.
+// Wrap calls to shutdown() so it receives no args — otherwise the string gets
+// passed as exitCode and process.exit() coerces it to NaN, exiting with code 1
+// instead of 0. (Caught in v0.18.1.0 #1025.)
+//
+// SIGINT (Ctrl+C): user intentionally stopping → shutdown.
+process.on('SIGINT', () => shutdown());
 // 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
@@ -1253,6 +1279,8 @@ process.on('SIGINT', shutdown);
 // - Headed / tunnel mode: idle timeout doesn't apply in these modes. Respect
 //   SIGTERM so external tooling (systemd, supervisord, CI) can shut cleanly
 //   without waiting forever. Ctrl+C and /stop still work either way.
+// - Active cookie picker: never tear down mid-import regardless of mode —
+//   would strand the picker UI with "Failed to fetch."
 process.on('SIGTERM', () => {
   if (hasActivePicker()) {
     console.log('[browse] Received SIGTERM but cookie picker is active, ignoring to avoid stranding the picker UI');