From a5b6522afe845f4f2762ad554e65df73d5b395a6 Mon Sep 17 00:00:00 2001
From: Garry Tan <garrytan@gmail.com>
Date: Sun, 16 Aug 2026 13:47:31 -0700
Subject: [PATCH] fix(browse): stop --force-restart kills the live daemon
 directly instead of booting a fresh one
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`browse stop --force-restart` on a live-but-busy daemon fell through the
stop short-circuit into ensureServer(), whose force-restart path kills the
daemon and then STARTS A FRESH ONE (daemon + Chromium, multi-second churn)
just so sendCommand('stop') can shut it down again — the #2254 churn in
force clothing. gstack-upgrade's Step 4.8 sends users down exactly this
path when a stale daemon is busy after an upgrade.

The stop short-circuit now handles it: live pid + --force-restart → kill
the daemon (tree-kill on Windows, TERM→KILL on POSIX), reap the orphaned
Chromium + clear profile locks, remove the state file, exit 0 — no server
is ever started. Pinned in stop-dead-daemon.test.ts: a wedged live "daemon"
is killed, the state file stays gone (a booted daemon would have rewritten
it), and no Starting/Restarting output appears.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 browse/src/cli.ts                    | 21 ++++++++++++-
 browse/test/stop-dead-daemon.test.ts | 45 ++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/browse/src/cli.ts b/browse/src/cli.ts
index c29833f52..08f7f24c9 100644
--- a/browse/src/cli.ts
+++ b/browse/src/cli.ts
@@ -1602,7 +1602,26 @@ Refs:           After 'snapshot', use @e1, @e2... as selectors:
       console.log('No daemon running (cleaned stale state) — nothing to stop.');
       process.exit(0);
     }
-    // Live daemon → fall through to the normal sendCommand('stop') path.
+    // stop --force-restart on a LIVE daemon (healthy or busy): kill it and
+    // clean up right here. Falling through would hand ensureServer() the
+    // force-restart flag, which kills the daemon and then BOOTS A FRESH ONE
+    // (daemon + Chromium, multi-second churn) just so sendCommand('stop')
+    // can shut it down again — the #2254 churn in force clothing, and
+    // gstack-upgrade's Step 4.8 sends users down exactly this path when a
+    // stale daemon is busy. The desired end state is "no daemon"; get there
+    // directly.
+    if (isProcessAlive(stopState.pid) && globalFlags.forceRestart) {
+      await killServer(stopState.pid);
+      // Reap the orphaned Chromium child + clear its profile locks so the
+      // NEXT launch is clean (same cleanup as the disconnect force path).
+      await killOrphanChromium();
+      cleanChromiumProfileLocks();
+      safeUnlinkQuiet(config.stateFile);
+      console.log('Daemon stopped (forced — tabs/cookies/logins discarded).');
+      process.exit(0);
+    }
+    // Live daemon without --force-restart → fall through to the normal
+    // sendCommand('stop') path (graceful shutdown; busy semantics apply).
   }
 
   // Special case: chain reads from stdin
diff --git a/browse/test/stop-dead-daemon.test.ts b/browse/test/stop-dead-daemon.test.ts
index 7ec8eb8f4..b92a669d3 100644
--- a/browse/test/stop-dead-daemon.test.ts
+++ b/browse/test/stop-dead-daemon.test.ts
@@ -15,6 +15,7 @@ import * as fs from 'fs';
 import * as net from 'net';
 import * as os from 'os';
 import * as path from 'path';
+import { isProcessAlive } from '../src/error-handling';
 
 function runCli(args: string[], env: Record<string, string>, timeoutMs = 30_000):
   Promise<{ code: number; stdout: string; stderr: string }> {
@@ -97,3 +98,47 @@ describe('#2254 stop on a dead daemon', () => {
     }
   }, 30_000);
 });
+
+describe('stop --force-restart on a LIVE daemon', () => {
+  test('kills it directly — never boots a fresh daemon just to stop it', async () => {
+    // A live-but-BUSY daemon: the pid is alive but nothing answers /health.
+    // Pre-fix, stop --force-restart fell through to ensureServer(), whose
+    // force-restart path killed the daemon and then STARTED a fresh one
+    // (daemon + Chromium) so sendCommand('stop') could stop it again —
+    // exactly what gstack-upgrade Step 4.8 triggers on a stale-busy daemon.
+    const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'browse-stop-force-'));
+    const stateFile = path.join(tmpDir, 'browse.json');
+    // Portable long-lived child standing in for the wedged daemon process.
+    const wedged = spawn('bun', ['-e', 'await Bun.sleep(300000)'], { stdio: 'ignore' });
+    try {
+      const port = await closedPort();
+      fs.writeFileSync(stateFile, JSON.stringify({
+        pid: wedged.pid,
+        port,
+        token: 'busy-token',
+        startedAt: new Date().toISOString(),
+        serverPath: '',
+        mode: 'launched' as const,
+      }, null, 2));
+
+      const result = await runCli(['stop', '--force-restart'], baseEnv(stateFile));
+
+      expect(result.code).toBe(0);
+      expect(result.stdout).toContain('Daemon stopped (forced');
+      // The load-bearing half: NO fresh daemon was booted to serve the stop.
+      // A spawned daemon would have re-written the state file.
+      expect(fs.existsSync(stateFile)).toBe(false);
+      expect(result.stderr).not.toContain('Starting server');
+      expect(result.stdout + result.stderr).not.toContain('Restarting');
+      // And the live pid is actually gone.
+      const deadline = Date.now() + 3000;
+      while (Date.now() < deadline && isProcessAlive(wedged.pid!)) {
+        await new Promise((r) => setTimeout(r, 100));
+      }
+      expect(isProcessAlive(wedged.pid!)).toBe(false);
+    } finally {
+      try { wedged.kill('SIGKILL'); } catch { /* already gone */ }
+      fs.rmSync(tmpDir, { recursive: true, force: true });
+    }
+  }, 30_000);
+});