浏览代码

feat(brainstorm-server): 4h configurable idle timeout; close WS on shutdown

The companion shut down after only 30 minutes idle — too short for real
brainstorming, where a single question can sit far longer. And shutdown() never
closed upgraded WebSocket sockets, so an open browser connection could keep the
Node process alive after it was supposed to exit.

- Default idle timeout raised to 4 hours, configurable via BRAINSTORM_IDLE_TIMEOUT_MS
  and start-server.sh --idle-timeout-minutes (validated positive integer).
- Reported as idle_timeout_ms in the server-started JSON / server-info.
- shutdown() now destroys all client sockets so the process exits even with an
  open WebSocket.
- Watchdog check interval is configurable (BRAINSTORM_LIFECYCLE_CHECK_MS, default
  60s) so the lifecycle can be tested without minute-long waits.

Adds lifecycle.test.js (configured timeout reported; idle shutdown exits despite
an open WS — teeth-verified; the start-server flag). Wires ws-protocol,
lifecycle, and stop-server suites into npm test.

Closes #1237
Refs #1689
Jesse Vincent 3 月之前
父节点
当前提交
56757f6877

+ 20 - 4
skills/brainstorming/scripts/server.cjs

@@ -255,7 +255,18 @@ function broadcast(msg) {
 
 // ========== Activity Tracking ==========
 
-const IDLE_TIMEOUT_MS = 30 * 60 * 1000; // 30 minutes
+// Idle timeout: shut down after this long with no activity. Default 4 hours;
+// override with BRAINSTORM_IDLE_TIMEOUT_MS (start-server.sh: --idle-timeout-minutes).
+const IDLE_TIMEOUT_MS = (() => {
+  const ms = Number(process.env.BRAINSTORM_IDLE_TIMEOUT_MS);
+  return Number.isFinite(ms) && ms > 0 ? ms : 4 * 60 * 60 * 1000;
+})();
+// How often the watchdog checks for owner-death / idleness. Configurable mainly
+// so tests can run fast; production default is 60s.
+const LIFECYCLE_CHECK_MS = (() => {
+  const ms = Number(process.env.BRAINSTORM_LIFECYCLE_CHECK_MS);
+  return Number.isFinite(ms) && ms > 0 ? ms : 60 * 1000;
+})();
 let lastActivity = Date.now();
 
 function touchActivity() {
@@ -317,6 +328,11 @@ function startServer() {
     );
     watcher.close();
     clearInterval(lifecycleCheck);
+    // Close any upgraded WebSocket sockets so server.close() can complete and
+    // the process actually exits instead of lingering on an open connection.
+    for (const socket of clients) {
+      try { socket.destroy(); } catch (e) { /* already gone */ }
+    }
     server.close(() => process.exit(0));
   }
 
@@ -325,11 +341,11 @@ function startServer() {
     try { process.kill(ownerPid, 0); return true; } catch (e) { return e.code === 'EPERM'; }
   }
 
-  // Check every 60s: exit if owner process died or idle for 30 minutes
+  // Periodically exit if the owner process died or we've been idle too long.
   const lifecycleCheck = setInterval(() => {
     if (!ownerAlive()) shutdown('owner process exited');
     else if (Date.now() - lastActivity > IDLE_TIMEOUT_MS) shutdown('idle timeout');
-  }, 60 * 1000);
+  }, LIFECYCLE_CHECK_MS);
   lifecycleCheck.unref();
 
   // Validate owner PID at startup. If it's already dead, the PID resolution
@@ -349,7 +365,7 @@ function startServer() {
     const info = JSON.stringify({
       type: 'server-started', port: Number(PORT), host: HOST,
       url_host: URL_HOST, url: 'http://' + URL_HOST + ':' + PORT,
-      screen_dir: CONTENT_DIR, state_dir: STATE_DIR
+      screen_dir: CONTENT_DIR, state_dir: STATE_DIR, idle_timeout_ms: IDLE_TIMEOUT_MS
     });
     console.log(info);
     fs.writeFileSync(path.join(STATE_DIR, 'server-info'), info + '\n');

+ 14 - 0
skills/brainstorming/scripts/start-server.sh

@@ -11,6 +11,7 @@
 #   --host <bind-host>    Host/interface to bind (default: 127.0.0.1).
 #                         Use 0.0.0.0 in remote/containerized environments.
 #   --url-host <host>     Hostname shown in returned URL JSON.
+#   --idle-timeout-minutes <n>  Shut down after n minutes idle (default 240 = 4h).
 #   --foreground          Run server in the current terminal (no backgrounding).
 #   --background          Force background mode (overrides Codex auto-foreground).
 
@@ -22,6 +23,7 @@ FOREGROUND="false"
 FORCE_BACKGROUND="false"
 BIND_HOST="127.0.0.1"
 URL_HOST=""
+IDLE_TIMEOUT_MINUTES=""
 while [[ $# -gt 0 ]]; do
   case "$1" in
     --project-dir)
@@ -36,6 +38,10 @@ while [[ $# -gt 0 ]]; do
       URL_HOST="$2"
       shift 2
       ;;
+    --idle-timeout-minutes)
+      IDLE_TIMEOUT_MINUTES="$2"
+      shift 2
+      ;;
     --foreground|--no-daemon)
       FOREGROUND="true"
       shift
@@ -59,6 +65,14 @@ if [[ -z "$URL_HOST" ]]; then
   fi
 fi
 
+if [[ -n "$IDLE_TIMEOUT_MINUTES" ]]; then
+  if ! [[ "$IDLE_TIMEOUT_MINUTES" =~ ^[0-9]+$ ]] || [[ "$IDLE_TIMEOUT_MINUTES" -lt 1 ]]; then
+    echo "{\"error\": \"--idle-timeout-minutes must be a positive integer\"}"
+    exit 1
+  fi
+  export BRAINSTORM_IDLE_TIMEOUT_MS=$(( IDLE_TIMEOUT_MINUTES * 60 * 1000 ))
+fi
+
 # Some environments reap detached/background processes. Auto-foreground when detected.
 if [[ -n "${CODEX_CI:-}" && "$FOREGROUND" != "true" && "$FORCE_BACKGROUND" != "true" ]]; then
   FOREGROUND="true"

+ 89 - 0
tests/brainstorm-server/lifecycle.test.js

@@ -0,0 +1,89 @@
+/**
+ * Tests for the brainstorm server's lifecycle (idle timeout + shutdown).
+ *
+ * - The idle timeout is configurable (default 4h) and reported in server-info.
+ * - Idle shutdown must close any open WebSocket so the process actually exits,
+ *   not hang on a lingering connection.
+ * - start-server.sh exposes the timeout via --idle-timeout-minutes.
+ *
+ * Uses the `ws` npm package as a test client (test-only dependency).
+ */
+
+const { spawn, execFileSync } = require('child_process');
+const WebSocket = require('ws');
+const fs = require('fs');
+const path = require('path');
+const assert = require('assert');
+
+const SERVER = path.join(__dirname, '../../skills/brainstorming/scripts/server.cjs');
+const START = path.join(__dirname, '../../skills/brainstorming/scripts/start-server.sh');
+const STOP = path.join(__dirname, '../../skills/brainstorming/scripts/stop-server.sh');
+const sleep = ms => new Promise(r => setTimeout(r, ms));
+
+function firstServerStarted(out) {
+  return JSON.parse(out.trim().split('\n').find(l => l.includes('server-started')));
+}
+
+async function runTests() {
+  let passed = 0, failed = 0;
+  async function test(name, fn) {
+    try { await fn(); console.log(`  PASS: ${name}`); passed++; }
+    catch (e) { console.log(`  FAIL: ${name}`); console.log(`    ${e.message}`); failed++; }
+  }
+
+  await test('server-info reports the configured idle_timeout_ms', async () => {
+    const dir = fs.mkdtempSync('/tmp/bs-life-');
+    const srv = spawn('node', [SERVER], { env: { ...process.env, BRAINSTORM_PORT: 3401, BRAINSTORM_DIR: dir, BRAINSTORM_IDLE_TIMEOUT_MS: 1234567 } });
+    let out = ''; srv.stdout.on('data', d => out += d.toString());
+    for (let i = 0; i < 60 && !out.includes('server-started'); i++) await sleep(50);
+    try {
+      const info = firstServerStarted(out);
+      assert.strictEqual(info.idle_timeout_ms, 1234567, 'idle_timeout_ms should reflect the env override');
+    } finally {
+      srv.kill(); await sleep(100); fs.rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
+  await test('idle shutdown closes an open WebSocket and the process exits', async () => {
+    const dir = fs.mkdtempSync('/tmp/bs-life-');
+    const srv = spawn('node', [SERVER], { env: { ...process.env, BRAINSTORM_PORT: 3402, BRAINSTORM_DIR: dir, BRAINSTORM_IDLE_TIMEOUT_MS: 200, BRAINSTORM_LIFECYCLE_CHECK_MS: 100 } });
+    let out = ''; srv.stdout.on('data', d => out += d.toString());
+    let exited = false, code = null; srv.on('exit', c => { exited = true; code = c; });
+    for (let i = 0; i < 60 && !out.includes('server-started'); i++) await sleep(50);
+
+    const ws = new WebSocket('ws://localhost:3402');
+    await new Promise((res, rej) => { ws.on('open', res); ws.on('error', rej); });
+
+    // 200ms idle, checked every 100ms — should shut down and exit well within 4s,
+    // *despite* the open WS, only if shutdown() closes client sockets.
+    for (let i = 0; i < 40 && !exited; i++) await sleep(100);
+
+    try {
+      assert(exited, 'process must exit after idle shutdown even with an open WebSocket');
+      assert.strictEqual(code, 0, 'should exit cleanly (0)');
+      assert(fs.existsSync(path.join(dir, 'state', 'server-stopped')), 'should write server-stopped');
+    } finally {
+      try { ws.close(); } catch (e) {}
+      if (!exited) srv.kill();
+      fs.rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
+  await test('start-server.sh --idle-timeout-minutes sets the timeout', async () => {
+    const dir = fs.mkdtempSync('/tmp/bs-life-');
+    let info;
+    const out = execFileSync('bash', [START, '--project-dir', dir, '--idle-timeout-minutes', '5'], { encoding: 'utf8' });
+    info = firstServerStarted(out);
+    try {
+      assert.strictEqual(info.idle_timeout_ms, 5 * 60 * 1000, '5 minutes -> 300000 ms');
+    } finally {
+      execFileSync('bash', [STOP, path.dirname(info.state_dir)], { stdio: 'ignore' });
+      fs.rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
+  console.log(`\n--- Results: ${passed} passed, ${failed} failed ---`);
+  if (failed > 0) process.exit(1);
+}
+
+runTests().catch(err => { console.error('Test failed:', err); process.exit(1); });

+ 1 - 1
tests/brainstorm-server/package.json

@@ -2,7 +2,7 @@
   "name": "brainstorm-server-tests",
   "version": "1.0.0",
   "scripts": {
-    "test": "node server.test.js"
+    "test": "node ws-protocol.test.js && node server.test.js && node lifecycle.test.js && bash stop-server.test.sh"
   },
   "dependencies": {
     "ws": "^8.19.0"