From 24e7efc32f27dc8363d556596e59cddb741ab122 Mon Sep 17 00:00:00 2001
From: zigggy-stardust <hi@spaceodditty.com>
Date: Thu, 13 Aug 2026 13:45:27 +0900
Subject: [PATCH] Fix brainstorm server crashing on shutdown when session dir
 is gone

shutdown() wrote state/server-stopped without checking that STATE_DIR
still exists. When the session directory is removed while the server
sits idle, that write throws ENOENT from inside the lifecycle interval,
so every cleanup step below it is skipped -- watcher.close(),
clearInterval(), socket teardown and server.close() never run -- and the
process dies on an uncaught exception with exit code 1 instead of
exiting 0.

State-dir bookkeeping is now best effort, matching the pattern already
used for PORT_FILE: skip it when STATE_DIR is gone, guard it with
try/catch, and always fall through to the real cleanup. watcher.close()
is guarded too, since the content dir disappears in the same scenario.
shutdown() is also made idempotent so a second lifecycle tick cannot
call server.close() twice.

The sentinel is deliberately not recreated when the directory is
missing: nothing is left to consume it, and re-creating a tree the user
just deleted would be surprising.

Adds a lifecycle regression test that removes the session dir while the
server is idle and asserts a clean exit 0 with no ENOENT.
---
 skills/brainstorming/scripts/server.cjs   | 25 ++++++++++++++++-------
 tests/brainstorm-server/lifecycle.test.js | 21 +++++++++++++++++++
 2 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/skills/brainstorming/scripts/server.cjs b/skills/brainstorming/scripts/server.cjs
index a828b35af64..c9559efc2ac 100644
--- a/skills/brainstorming/scripts/server.cjs
+++ b/skills/brainstorming/scripts/server.cjs
@@ -613,15 +613,26 @@ function startServer() {
   });
   watcher.on('error', (err) => console.error('fs.watch error:', err.message));
 
+  let shuttingDown = false;
   function shutdown(reason) {
+    if (shuttingDown) return;
+    shuttingDown = true;
     console.log(JSON.stringify({ type: 'server-stopped', reason }));
-    const infoFile = path.join(STATE_DIR, 'server-info');
-    if (fs.existsSync(infoFile)) fs.unlinkSync(infoFile);
-    fs.writeFileSync(
-      path.join(STATE_DIR, 'server-stopped'),
-      JSON.stringify({ reason, timestamp: Date.now() }) + '\n'
-    );
-    watcher.close();
+    // State-dir bookkeeping is best effort. The session directory can be removed
+    // while the server sits idle, and a missing or read-only state dir must never
+    // abort shutdown -- throwing here would skip every cleanup step below and kill
+    // the process with an uncaught exception instead of exiting 0.
+    try {
+      if (fs.existsSync(STATE_DIR)) {
+        const infoFile = path.join(STATE_DIR, 'server-info');
+        if (fs.existsSync(infoFile)) fs.unlinkSync(infoFile);
+        fs.writeFileSync(
+          path.join(STATE_DIR, 'server-stopped'),
+          JSON.stringify({ reason, timestamp: Date.now() }) + '\n'
+        );
+      }
+    } catch (e) { /* best effort */ }
+    try { watcher.close(); } catch (e) { /* content dir may be gone */ }
     clearInterval(lifecycleCheck);
     // Close any upgraded WebSocket sockets so server.close() can complete and
     // the process actually exits instead of lingering on an open connection.
diff --git a/tests/brainstorm-server/lifecycle.test.js b/tests/brainstorm-server/lifecycle.test.js
index c43d8c0de2c..865e4f0a723 100644
--- a/tests/brainstorm-server/lifecycle.test.js
+++ b/tests/brainstorm-server/lifecycle.test.js
@@ -508,6 +508,27 @@ async function runTests() {
     assert(exited, 'idle shutdown must still fire despite a flood of unauthenticated requests');
   });
 
+  await test('idle shutdown exits 0 when the session dir was removed', async () => {
+    const dir = fs.mkdtempSync('/tmp/bs-life-');
+    const srv = spawn('node', [SERVER], { env: { ...process.env, BRAINSTORM_PORT: 3420, BRAINSTORM_DIR: dir, BRAINSTORM_IDLE_TIMEOUT_MS: 400, BRAINSTORM_LIFECYCLE_CHECK_MS: 100 } });
+    let out = ''; srv.stdout.on('data', d => out += d.toString());
+    let err = ''; srv.stderr.on('data', d => err += d.toString());
+    let code = null; srv.on('exit', c => { code = c; });
+    for (let i = 0; i < 60 && !out.includes('server-started'); i++) await sleep(50);
+
+    // The session dir can be cleaned up while the server sits idle. Shutdown
+    // bookkeeping is best effort: if writing state/server-stopped throws, the
+    // cleanup below it never runs and the process dies on an uncaught exception.
+    fs.rmSync(dir, { recursive: true, force: true });
+
+    for (let i = 0; i < 40 && code === null; i++) await sleep(100);
+    if (code === null) await killAndWait(srv);
+    fs.rmSync(dir, { recursive: true, force: true });
+
+    assert.strictEqual(code, 0, `idle shutdown must exit 0 with the session dir gone, got ${code}: ${err}`);
+    assert(!/ENOENT/.test(err), `shutdown must not throw ENOENT, got: ${err}`);
+  });
+
   console.log(`\n--- Results: ${passed} passed, ${failed} failed ---`);
   if (failed > 0) process.exit(1);
 }