From f057b4a30b9feca4b866a55ba5b58ab6d2383c6e Mon Sep 17 00:00:00 2001 From: Jesse Vincent Date: Tue, 9 Jun 2026 15:08:09 -0700 Subject: [PATCH] feat(brainstorm-server): 4h configurable idle timeout; close WS on shutdown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- skills/brainstorming/scripts/server.cjs | 24 +++++- skills/brainstorming/scripts/start-server.sh | 14 +++ tests/brainstorm-server/lifecycle.test.js | 89 ++++++++++++++++++++ tests/brainstorm-server/package.json | 2 +- 4 files changed, 124 insertions(+), 5 deletions(-) create mode 100644 tests/brainstorm-server/lifecycle.test.js diff --git a/skills/brainstorming/scripts/server.cjs b/skills/brainstorming/scripts/server.cjs index 78bf7c37..d1ac4e58 100644 --- a/skills/brainstorming/scripts/server.cjs +++ b/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'); diff --git a/skills/brainstorming/scripts/start-server.sh b/skills/brainstorming/scripts/start-server.sh index 5ef391b1..358e1842 100755 --- a/skills/brainstorming/scripts/start-server.sh +++ b/skills/brainstorming/scripts/start-server.sh @@ -11,6 +11,7 @@ # --host Host/interface to bind (default: 127.0.0.1). # Use 0.0.0.0 in remote/containerized environments. # --url-host Hostname shown in returned URL JSON. +# --idle-timeout-minutes 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" diff --git a/tests/brainstorm-server/lifecycle.test.js b/tests/brainstorm-server/lifecycle.test.js new file mode 100644 index 00000000..aaeb346c --- /dev/null +++ b/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); }); diff --git a/tests/brainstorm-server/package.json b/tests/brainstorm-server/package.json index a089f03c..a4044701 100644 --- a/tests/brainstorm-server/package.json +++ b/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"