mirror of
https://github.com/obra/superpowers.git
synced 2026-06-11 05:09:05 +08:00
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
This commit is contained in:
@@ -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');
|
||||
|
||||
@@ -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
tests/brainstorm-server/lifecycle.test.js
Normal file
89
tests/brainstorm-server/lifecycle.test.js
Normal file
@@ -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); });
|
||||
@@ -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"
|
||||
|
||||
Reference in New Issue
Block a user