diff --git a/skills/brainstorming/scripts/server.cjs b/skills/brainstorming/scripts/server.cjs index 8728cc38..90eff801 100644 --- a/skills/brainstorming/scripts/server.cjs +++ b/skills/brainstorming/scripts/server.cjs @@ -192,6 +192,16 @@ function getNewestScreen() { return files.length > 0 ? files[0].path : null; } +function urlHostForHttp(host) { + const h = String(host); + if (h.startsWith('[') && h.endsWith(']')) return h; + return h.includes(':') ? '[' + h + ']' : h; +} + +function companionUrl() { + return 'http://' + urlHostForHttp(URL_HOST) + ':' + PORT + '/?key=' + TOKEN; +} + function isRegularFileInsideContentDir(filePath) { let stat, realContentDir, realFilePath; try { @@ -424,7 +434,7 @@ function maybeOpenBrowser() { if (!process.env.BRAINSTORM_OPEN) return; // opt-in: only after the user approves the companion if (HOST !== '127.0.0.1' && HOST !== 'localhost') return; if (clients.size > 0) return; // the user already opened it - const url = 'http://' + URL_HOST + ':' + PORT + '/?key=' + TOKEN; // must carry the key or the gate 403s it + const url = companionUrl(); // must carry the key or the gate 403s it const cp = require('child_process'); // Operator-provided launcher: run as given (this env var is trusted operator input). if (process.env.BRAINSTORM_OPEN_CMD) { @@ -572,7 +582,7 @@ function startServer() { } const info = JSON.stringify({ type: 'server-started', port: Number(PORT), host: HOST, - url_host: URL_HOST, url: 'http://' + URL_HOST + ':' + PORT + '/?key=' + TOKEN, + url_host: URL_HOST, url: companionUrl(), screen_dir: CONTENT_DIR, state_dir: STATE_DIR, idle_timeout_ms: IDLE_TIMEOUT_MS }); console.log(info); diff --git a/skills/brainstorming/scripts/start-server.sh b/skills/brainstorming/scripts/start-server.sh index e56b133b..b8f79a4b 100755 --- a/skills/brainstorming/scripts/start-server.sh +++ b/skills/brainstorming/scripts/start-server.sh @@ -79,6 +79,21 @@ if [[ -n "$IDLE_TIMEOUT_MINUTES" ]]; then export BRAINSTORM_IDLE_TIMEOUT_MS=$(( IDLE_TIMEOUT_MINUTES * 60 * 1000 )) fi +is_windows_like_shell() { + case "${OSTYPE:-}" in + msys*|cygwin*|mingw*) return 0 ;; + esac + if [[ -n "${MSYSTEM:-}" ]]; then + return 0 + fi + local uname_s + uname_s="$(uname -s 2>/dev/null || true)" + case "$uname_s" in + MSYS*|MINGW*|CYGWIN*) return 0 ;; + esac + return 1 +} + # Some environments reap detached/background processes. Auto-foreground when detected. if [[ -n "${CODEX_CI:-}" && "$FOREGROUND" != "true" && "$FORCE_BACKGROUND" != "true" ]]; then FOREGROUND="true" @@ -86,10 +101,7 @@ fi # Windows/Git Bash reaps nohup background processes. Auto-foreground when detected. if [[ "$FOREGROUND" != "true" && "$FORCE_BACKGROUND" != "true" ]]; then - case "${OSTYPE:-}" in - msys*|cygwin*|mingw*) FOREGROUND="true" ;; - esac - if [[ -n "${MSYSTEM:-}" ]]; then + if is_windows_like_shell; then FOREGROUND="true" fi fi @@ -139,10 +151,7 @@ fi # Passing a PID node cannot verify causes server to log owner-pid-invalid # and self-terminate at the 60-second lifecycle check. Clear it so the # watchdog is disabled and the idle timeout becomes the only shutdown trigger. -case "${OSTYPE:-}" in - msys*|cygwin*|mingw*) OWNER_PID="" ;; -esac -if [[ -n "${MSYSTEM:-}" ]]; then +if is_windows_like_shell; then OWNER_PID="" fi diff --git a/skills/brainstorming/scripts/stop-server.sh b/skills/brainstorming/scripts/stop-server.sh index 1bfcd082..923c4c1a 100755 --- a/skills/brainstorming/scripts/stop-server.sh +++ b/skills/brainstorming/scripts/stop-server.sh @@ -20,7 +20,12 @@ PID_FILE="${STATE_DIR}/server.pid" # not a reused/unrelated process whose PID was recycled into a stale pid file. is_brainstorm_server() { kill -0 "$1" 2>/dev/null || return 1 - case "$(ps -p "$1" -o command= 2>/dev/null)" in + local command_line + command_line="$(ps -p "$1" -o command= 2>/dev/null || true)" + if [[ -z "$command_line" ]]; then + command_line="$(ps -f -p "$1" 2>/dev/null | sed '1d' || true)" + fi + case "$command_line" in *node*server.cjs*) ;; *) return 1 ;; esac diff --git a/tests/brainstorm-server/lifecycle.test.js b/tests/brainstorm-server/lifecycle.test.js index 4c3a88a3..95f7bab4 100644 --- a/tests/brainstorm-server/lifecycle.test.js +++ b/tests/brainstorm-server/lifecycle.test.js @@ -21,6 +21,7 @@ const STOP = path.join(__dirname, '../../skills/brainstorming/scripts/stop-serve const sleep = ms => new Promise(r => setTimeout(r, ms)); function waitForExit(child, timeoutMs = 2000) { + if (child.exitCode !== null || child.signalCode !== null) return Promise.resolve(true); return new Promise(resolve => { let settled = false; const finish = (exited) => { @@ -33,10 +34,38 @@ function waitForExit(child, timeoutMs = 2000) { }); } +async function killAndWait(child, timeoutMs = 2000) { + if (!child || child.exitCode !== null || child.signalCode !== null) return true; + const exited = waitForExit(child, timeoutMs); + child.kill(); + if (await exited) return true; + + child.kill('SIGKILL'); + return waitForExit(child, 500); +} + +async function waitForFile(file, timeoutMs = 3000) { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (fs.existsSync(file)) return true; + await sleep(50); + } + return fs.existsSync(file); +} + function firstServerStarted(out) { return JSON.parse(out.trim().split('\n').find(l => l.includes('server-started'))); } +function openCaptureCommand(dir, marker) { + const scriptPath = path.resolve(dir, 'capture-open.cjs'); + const markerPath = path.resolve(marker); + fs.writeFileSync(scriptPath, + "const fs = require('fs');\n" + + "fs.appendFileSync(process.argv[2], process.argv[3] + '\\n');\n"); + return `node ${JSON.stringify(scriptPath)} ${JSON.stringify(markerPath)}`; +} + async function runTests() { let passed = 0, failed = 0; async function test(name, fn) { @@ -53,7 +82,8 @@ async function runTests() { 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 killAndWait(srv); + fs.rmSync(dir, { recursive: true, force: true }); } }); @@ -77,7 +107,7 @@ async function runTests() { assert(fs.existsSync(path.join(dir, 'state', 'server-stopped')), 'should write server-stopped'); } finally { try { ws.close(); } catch (e) {} - if (!exited) srv.kill(); + if (!exited) await killAndWait(srv); fs.rmSync(dir, { recursive: true, force: true }); } }); @@ -95,6 +125,30 @@ async function runTests() { } }); + await test('server-started URL brackets IPv6 URL hosts', async () => { + const dir = fs.mkdtempSync('/tmp/bs-ipv6-url-'); + const srv = spawn('node', [SERVER], { + env: { + ...process.env, + BRAINSTORM_PORT: 3421, + BRAINSTORM_HOST: '127.0.0.1', + BRAINSTORM_URL_HOST: '::1', + BRAINSTORM_TOKEN: 'ipv6token', + BRAINSTORM_DIR: dir, + BRAINSTORM_LIFECYCLE_CHECK_MS: 100000 + } + }); + let out = ''; srv.stdout.on('data', d => out += d.toString()); + try { + for (let i = 0; i < 60 && !out.includes('server-started'); i++) await sleep(50); + const info = firstServerStarted(out); + assert.strictEqual(info.url, 'http://[::1]:3421/?key=ipv6token'); + } finally { + await killAndWait(srv); + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + await test('persists the bound port AND key, and restores both on restart', async () => { const dir = fs.mkdtempSync('/tmp/bs-port-'); const portFile = path.join(dir, '.last-port'); @@ -116,7 +170,8 @@ async function runTests() { for (let i = 0; i < 60 && !outB.includes('server-started'); i++) await sleep(50); const infoB = firstServerStarted(outB); const keyB = new URL(infoB.url).searchParams.get('key'); - b.kill(); await sleep(100); fs.rmSync(dir, { recursive: true, force: true }); + await killAndWait(b); + fs.rmSync(dir, { recursive: true, force: true }); assert.strictEqual(infoB.port, infoA.port, 'restart should reuse the same port'); // Same key too — otherwise the open tab's cookie would 403 against the restart. @@ -159,9 +214,8 @@ async function runTests() { assert(opened, 'stored key should authenticate WS after restart'); } finally { try { if (ws) ws.close(); } catch (e) {} - try { if (a) a.kill(); } catch (e) {} - try { if (b) b.kill(); } catch (e) {} - await sleep(100); + await killAndWait(a); + await killAndWait(b); fs.rmSync(dir, { recursive: true, force: true }); } }); @@ -181,7 +235,9 @@ async function runTests() { const portB = firstServerStarted(outB).port; const persisted = fs.readFileSync(portFile, 'utf8').trim(); - a.kill(); b.kill(); await sleep(100); fs.rmSync(dir, { recursive: true, force: true }); + await killAndWait(a); + await killAndWait(b); + fs.rmSync(dir, { recursive: true, force: true }); assert.notStrictEqual(portB, 3415, 'must not bind the already-taken port'); assert(portB >= 49152, 'should fall back to a random high port'); @@ -193,14 +249,14 @@ async function runTests() { await test('auto-opens the browser once, on the first screen', async () => { const dir = fs.mkdtempSync('/tmp/bs-open-'); const marker = path.join(dir, 'opened.log'); - const openCmd = `sh -c 'echo "$0" >> ${marker}'`; // capture the launch instead of opening a browser + const openCmd = openCaptureCommand(dir, marker); // capture the launch instead of opening a browser const srv = spawn('node', [SERVER], { env: { ...process.env, BRAINSTORM_PORT: 3417, BRAINSTORM_DIR: dir, BRAINSTORM_OPEN: '1', BRAINSTORM_OPEN_CMD: openCmd, BRAINSTORM_LIFECYCLE_CHECK_MS: 100000 } }); let out = ''; srv.stdout.on('data', d => out += d.toString()); for (let i = 0; i < 60 && !out.includes('server-started'); i++) await sleep(50); // First screen, with no browser connected -> should auto-open. fs.writeFileSync(path.join(dir, 'content', 'first.html'), '

First

'); - await sleep(700); + await waitForFile(marker); // Second screen -> must NOT open again. fs.writeFileSync(path.join(dir, 'content', 'second.html'), '

Second

'); await sleep(700); @@ -211,7 +267,7 @@ async function runTests() { if (lines[0]) { status = await new Promise(r => require('http').get(lines[0], res => { res.resume(); r(res.statusCode); }).on('error', () => r(0))); } - srv.kill(); await sleep(100); + await killAndWait(srv); fs.rmSync(dir, { recursive: true, force: true }); assert.strictEqual(lines.length, 1, 'should open exactly once'); @@ -223,14 +279,14 @@ async function runTests() { await test('does NOT auto-open unless approved (BRAINSTORM_OPEN unset)', async () => { const dir = fs.mkdtempSync('/tmp/bs-open-'); const marker = path.join(dir, 'opened.log'); - const openCmd = `sh -c 'echo "$0" >> ${marker}'`; + const openCmd = openCaptureCommand(dir, marker); // BRAINSTORM_OPEN intentionally NOT set — auto-open must stay off. const srv = spawn('node', [SERVER], { env: { ...process.env, BRAINSTORM_PORT: 3418, BRAINSTORM_DIR: dir, BRAINSTORM_OPEN_CMD: openCmd, BRAINSTORM_LIFECYCLE_CHECK_MS: 100000 } }); let out = ''; srv.stdout.on('data', d => out += d.toString()); for (let i = 0; i < 60 && !out.includes('server-started'); i++) await sleep(50); fs.writeFileSync(path.join(dir, 'content', 'first.html'), '

First

'); await sleep(700); - srv.kill(); await sleep(100); + await killAndWait(srv); const opened = fs.existsSync(marker); fs.rmSync(dir, { recursive: true, force: true }); assert(!opened, 'must not open the browser without explicit approval'); @@ -248,7 +304,7 @@ async function runTests() { const hammer = setInterval(() => { require('http').get('http://localhost:3419/', r => r.resume()).on('error', () => {}); }, 60); for (let i = 0; i < 40 && !exited; i++) await sleep(100); clearInterval(hammer); - if (!exited) srv.kill(); + if (!exited) await killAndWait(srv); fs.rmSync(dir, { recursive: true, force: true }); assert(exited, 'idle shutdown must still fire despite a flood of unauthenticated requests'); diff --git a/tests/brainstorm-server/package.json b/tests/brainstorm-server/package.json index 70bf5254..dc0c5216 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 ws-protocol.test.js && node helper.test.js && node auth.test.js && node server.test.js && node lifecycle.test.js && bash stop-server.test.sh" + "test": "node ws-protocol.test.js && node helper.test.js && node auth.test.js && node server.test.js && node lifecycle.test.js && bash start-server.test.sh && bash stop-server.test.sh" }, "dependencies": { "ws": "^8.19.0" diff --git a/tests/brainstorm-server/start-server.test.sh b/tests/brainstorm-server/start-server.test.sh new file mode 100644 index 00000000..2667b888 --- /dev/null +++ b/tests/brainstorm-server/start-server.test.sh @@ -0,0 +1,95 @@ +#!/usr/bin/env bash +# Fast tests for start-server.sh shell-only platform decisions. +set -uo pipefail + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +REPO_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)" +START_SCRIPT="$REPO_ROOT/skills/brainstorming/scripts/start-server.sh" + +TEST_DIR="${TMPDIR:-/tmp}/brainstorm-start-test-$$" +passed=0 +failed=0 + +cleanup() { + rm -rf "$TEST_DIR" +} +trap cleanup EXIT + +pass() { + echo " PASS: $1" + passed=$((passed + 1)) +} + +fail() { + echo " FAIL: $1" + echo " $2" + failed=$((failed + 1)) +} + +make_fake_uname() { + local fake_bin="$1" + cat > "$fake_bin/uname" <<'EOF' +#!/usr/bin/env bash +if [[ "${1:-}" == "-s" ]]; then + echo "MINGW64_NT-10.0" +else + /usr/bin/uname "$@" +fi +EOF + chmod +x "$fake_bin/uname" +} + +echo "" +echo "--- start-server.sh platform detection ---" + +mkdir -p "$TEST_DIR/fake-bin" "$TEST_DIR/project" +make_fake_uname "$TEST_DIR/fake-bin" + +cat > "$TEST_DIR/fake-bin/node" <<'EOF' +#!/usr/bin/env bash +echo "CAPTURED_OWNER_PID=${BRAINSTORM_OWNER_PID:-__UNSET__}" +exit 0 +EOF +chmod +x "$TEST_DIR/fake-bin/node" + +captured=$( + PATH="$TEST_DIR/fake-bin:$PATH" \ + MSYSTEM="" \ + bash "$START_SCRIPT" --project-dir "$TEST_DIR/project" --foreground 2>/dev/null || true +) +owner_pid_value=$(echo "$captured" | grep "CAPTURED_OWNER_PID=" | head -1 | sed 's/CAPTURED_OWNER_PID=//') + +if [[ "$owner_pid_value" == "" || "$owner_pid_value" == "__UNSET__" ]]; then + pass "clears BRAINSTORM_OWNER_PID when uname reports a Windows-like shell" +else + fail "clears BRAINSTORM_OWNER_PID when uname reports a Windows-like shell" \ + "expected empty or unset, got '$owner_pid_value'" +fi + +rm -rf "$TEST_DIR/project"/* + +cat > "$TEST_DIR/fake-bin/node" <<'EOF' +#!/usr/bin/env bash +echo "FOREGROUND_MODE=true" +exit 0 +EOF +chmod +x "$TEST_DIR/fake-bin/node" + +captured=$( + PATH="$TEST_DIR/fake-bin:$PATH" \ + MSYSTEM="" \ + bash "$START_SCRIPT" --project-dir "$TEST_DIR/project" 2>/dev/null || true +) + +if echo "$captured" | grep -q "FOREGROUND_MODE=true"; then + pass "auto-foregrounds when uname reports a Windows-like shell" +else + fail "auto-foregrounds when uname reports a Windows-like shell" \ + "expected foreground node path, got: $captured" +fi + +echo "" +echo "--- Results: $passed passed, $failed failed ---" +if [[ $failed -gt 0 ]]; then + exit 1 +fi diff --git a/tests/brainstorm-server/stop-server.test.sh b/tests/brainstorm-server/stop-server.test.sh index 1c979767..37a0a576 100755 --- a/tests/brainstorm-server/stop-server.test.sh +++ b/tests/brainstorm-server/stop-server.test.sh @@ -13,11 +13,13 @@ SERVER="$SCRIPT_DIR/../../skills/brainstorming/scripts/server.cjs" PASS=0; FAIL=0 ok() { echo " PASS: $1"; PASS=$((PASS + 1)); } bad() { echo " FAIL: $1"; echo " $2"; FAIL=$((FAIL + 1)); } +reap_job() { wait "$1" 2>/dev/null || true; } # --- Test 1: an unrelated, reused PID must NOT be killed --- SESS="$(mktemp -d)"; mkdir -p "$SESS/state" sleep 600 & UNRELATED=$! +disown "$UNRELATED" 2>/dev/null || true echo "$UNRELATED" > "$SESS/state/server.pid" OUT="$("$STOP" "$SESS")" if kill -0 "$UNRELATED" 2>/dev/null; then @@ -29,12 +31,14 @@ else bad "unrelated reused PID was KILLED" "$OUT" fi kill -9 "$UNRELATED" 2>/dev/null +reap_job "$UNRELATED" rm -rf "$SESS" # --- Test 2: a real brainstorm server IS stopped --- SESS="$(mktemp -d)"; mkdir -p "$SESS/content" "$SESS/state" BRAINSTORM_DIR="$SESS" BRAINSTORM_PORT=3399 node "$SERVER" > /dev/null 2>&1 & SRV=$! +disown "$SRV" 2>/dev/null || true for _ in $(seq 1 40); do kill -0 "$SRV" 2>/dev/null && break; sleep 0.1; done sleep 0.4 echo "$SRV" > "$SESS/state/server.pid" @@ -43,7 +47,9 @@ sleep 0.3 if kill -0 "$SRV" 2>/dev/null; then bad "real brainstorm server still running after stop" "$OUT" kill -9 "$SRV" 2>/dev/null + reap_job "$SRV" else + reap_job "$SRV" case "$OUT" in *stopped*) ok "real brainstorm server is stopped" ;; *) bad "server stopped but status was not 'stopped'" "$OUT" ;; @@ -66,6 +72,7 @@ if command -v lsof > /dev/null 2>&1; then echo '{"type":"server-started","port":3499}' > "$SESS/state/server-info" # nothing listens on 3499 ( exec -a "node server.cjs" sleep 600 ) & IMPOSTOR=$! + disown "$IMPOSTOR" 2>/dev/null || true echo "$IMPOSTOR" > "$SESS/state/server.pid" OUT="$("$STOP" "$SESS")" if kill -0 "$IMPOSTOR" 2>/dev/null; then @@ -77,6 +84,7 @@ if command -v lsof > /dev/null 2>&1; then bad "killed a node server.cjs that was NOT on our recorded port" "$OUT" fi kill -9 "$IMPOSTOR" 2>/dev/null + reap_job "$IMPOSTOR" rm -rf "$SESS" else echo " SKIP: lsof unavailable — port cross-check test" diff --git a/tests/brainstorm-server/windows-lifecycle.test.sh b/tests/brainstorm-server/windows-lifecycle.test.sh index d86781f3..d178a755 100755 --- a/tests/brainstorm-server/windows-lifecycle.test.sh +++ b/tests/brainstorm-server/windows-lifecycle.test.sh @@ -80,14 +80,23 @@ get_port_from_info() { grep -o '"port":[0-9]*' "$1/state/server-info" | head -1 | sed 's/"port"://' } +get_key_from_info() { + grep -o '"url":"[^"]*key=[^"]*' "$1/state/server-info" | head -1 | sed 's/.*key=//' +} + http_check() { local port="$1" - node -e " + local key="${2:-}" + node - "$port" "$key" <<'NODE' const http = require('http'); - http.get('http://localhost:$port/', (res) => { + const port = Number(process.argv[2]); + const key = process.argv[3] || ''; + const path = key ? '/?key=' + encodeURIComponent(key) : '/'; + http.get({ hostname: '127.0.0.1', port, path }, (res) => { + res.resume(); process.exit(res.statusCode === 200 ? 0 : 1); }).on('error', () => process.exit(1)); - " 2>/dev/null +NODE } # ========== Platform Detection ========== @@ -227,6 +236,7 @@ else pass "Server starts successfully with empty OWNER_PID" SERVER_PORT=$(get_port_from_info "$TEST_DIR/survival") + SERVER_KEY=$(get_key_from_info "$TEST_DIR/survival") sleep 75 @@ -237,11 +247,11 @@ else "Server died. Log tail: $(tail -5 "$TEST_DIR/survival/.server.log" 2>/dev/null)" fi - if http_check "$SERVER_PORT"; then + if http_check "$SERVER_PORT" "$SERVER_KEY"; then pass "Server responds to HTTP after lifecycle check window" else fail "Server responds to HTTP after lifecycle check window" \ - "HTTP request to port $SERVER_PORT failed" + "Authenticated HTTP request to port $SERVER_PORT failed" fi if grep -q "owner process exited" "$TEST_DIR/survival/.server.log" 2>/dev/null; then