diff --git a/skills/brainstorming/scripts/frame-template.html b/skills/brainstorming/scripts/frame-template.html index 51575a1e..1952fef2 100644 --- a/skills/brainstorming/scripts/frame-template.html +++ b/skills/brainstorming/scripts/frame-template.html @@ -197,7 +197,7 @@

Superpowers Brainstorming

-
Connected
+
Connecting…
diff --git a/skills/brainstorming/scripts/helper.js b/skills/brainstorming/scripts/helper.js index 456e9150..41a3aa61 100644 --- a/skills/brainstorming/scripts/helper.js +++ b/skills/brainstorming/scripts/helper.js @@ -28,6 +28,7 @@ const el = document.querySelector('.status'); if (!el) return; const map = { + connecting: ['Connecting…', 'var(--text-tertiary)'], connected: ['Connected', 'var(--success)'], reconnecting: ['Reconnecting…', 'var(--warning)'], disconnected: ['Disconnected', 'var(--error)'] @@ -55,7 +56,7 @@ function connect() { if (reconnectTimer) { clearTimeout(reconnectTimer); reconnectTimer = null; } - setStatus(everConnected ? 'reconnecting' : 'connected'); + setStatus(everConnected ? 'reconnecting' : 'connecting'); ws = new WebSocket(WS_URL); ws.onopen = () => { diff --git a/skills/brainstorming/scripts/server.cjs b/skills/brainstorming/scripts/server.cjs index 7e4e1b20..fc9f359b 100644 --- a/skills/brainstorming/scripts/server.cjs +++ b/skills/brainstorming/scripts/server.cjs @@ -278,14 +278,21 @@ function maybeOpenBrowser() { 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; - let cmd = process.env.BRAINSTORM_OPEN_CMD; - if (!cmd) { - if (process.platform === 'darwin') cmd = 'open'; - else if (/microsoft/i.test(require('os').release())) cmd = 'cmd.exe /c start ""'; // WSL → Windows browser - else if (process.env.DISPLAY || process.env.WAYLAND_DISPLAY) cmd = 'xdg-open'; - else return; // headless: nothing to open + const cp = require('child_process'); + // Operator-provided launcher: run as given (this env var is trusted operator input). + if (process.env.BRAINSTORM_OPEN_CMD) { + try { cp.exec(process.env.BRAINSTORM_OPEN_CMD + ' ' + JSON.stringify(url), () => {}); } catch (e) { /* best effort */ } + return; } - try { require('child_process').exec(cmd + ' ' + JSON.stringify(url), () => {}); } catch (e) { /* best effort */ } + // Platform launchers: pass the URL as an argv element via execFile (no shell), + // so a url-host containing shell metacharacters can't inject a command. + const isWSL = process.platform === 'linux' && /microsoft/i.test(require('os').release()); + let bin, args; + if (process.platform === 'darwin') { bin = 'open'; args = [url]; } + else if (process.platform === 'win32' || isWSL) { bin = 'cmd.exe'; args = ['/c', 'start', '', url]; } + else if (process.env.DISPLAY || process.env.WAYLAND_DISPLAY) { bin = 'xdg-open'; args = [url]; } + else return; // headless: nothing to open + try { cp.execFile(bin, args, () => {}); } catch (e) { /* best effort */ } } // ========== Activity Tracking ========== @@ -397,9 +404,16 @@ function startServer() { } } + // If the preferred port is already taken (e.g. a previous server is still + // alive), fall back to a random port once instead of failing. + let triedFallback = false; + function onListen() { - // Record the bound port so the next restart of this session can reuse it. - if (PORT_FILE) { + // Record the bound port so the next restart of this session reuses it — but + // ONLY when we got our preferred port. On a fallback we bound a *different* + // port because someone else holds the preferred one; persisting it would + // overwrite the shared .last-port and strand that other session's open tab. + if (PORT_FILE && !triedFallback) { try { fs.writeFileSync(PORT_FILE, String(PORT)); } catch (e) { /* best effort */ } } const info = JSON.stringify({ @@ -411,9 +425,6 @@ function startServer() { fs.writeFileSync(path.join(STATE_DIR, 'server-info'), info + '\n'); } - // If the preferred port is already taken (e.g. a previous server is still - // alive), fall back to a random port once instead of failing. - let triedFallback = false; server.on('error', (err) => { if (err.code === 'EADDRINUSE' && !triedFallback) { triedFallback = true; diff --git a/tests/brainstorm-server/helper.test.js b/tests/brainstorm-server/helper.test.js index e897afcd..a4d871ea 100644 --- a/tests/brainstorm-server/helper.test.js +++ b/tests/brainstorm-server/helper.test.js @@ -80,5 +80,84 @@ test('reloads on recovery and on reload messages', () => { assert(/location\.reload\(\)/.test(src), 'reloads to pick up restarted/updated content'); }); +console.log('\n--- Reconnect state machine (mocked browser) ---'); + +// Drive helper.js's browser code against mocked DOM/WebSocket/timers/clock so we +// can exercise the actual reconnect/status/tombstone behaviour, not just grep it. +function makeEnv() { + const state = { now: 1000, timers: [], reloads: 0, appended: [] }; + const sockets = []; + const statusEl = { textContent: '', style: { setProperty() {} } }; + class FakeWS { + constructor(url) { this.url = url; this.readyState = 0; this.onopen = this.onclose = this.onmessage = this.onerror = null; sockets.push(this); } + send() {} + close() { this.readyState = 3; if (this.onclose) this.onclose(); } + open() { this.readyState = 1; if (this.onopen) this.onopen(); } + } + FakeWS.OPEN = 1; + const env = { + module: { exports: {} }, + window: { location: { host: 'localhost:7777', reload() { state.reloads++; } } }, + document: { + querySelector: (s) => s === '.status' ? statusEl : null, + getElementById: () => null, + createElement: () => ({ style: {}, id: '' }), + addEventListener() {}, + body: { appendChild: (el) => state.appended.push(el) } + }, + WebSocket: FakeWS, + setTimeout: (fn, ms) => { state.timers.push({ fn, ms, fired: false, cleared: false }); return state.timers.length; }, + clearTimeout: (id) => { if (state.timers[id - 1]) state.timers[id - 1].cleared = true; }, + Date: { now: () => state.now }, + console + }; + return { + state, statusEl, sockets, + boot() { new Function(...Object.keys(env), src)(...Object.values(env)); }, + advance(ms) { state.now += ms; }, + last() { return sockets[sockets.length - 1]; }, + fireReconnect() { + const t = [...state.timers].reverse().find(x => !x.fired && !x.cleared); + if (!t) throw new Error('no reconnect scheduled'); + t.fired = true; t.fn(); + } + }; +} + +test('on disconnect shows Reconnecting and schedules a 500ms reconnect', () => { + const e = makeEnv(); e.boot(); + e.last().open(); + assert.strictEqual(e.statusEl.textContent, 'Connected'); + e.last().close(); + assert.strictEqual(e.statusEl.textContent, 'Reconnecting…'); + assert.strictEqual(e.state.timers[e.state.timers.length - 1].ms, 500); +}); + +test('reconnect delay backs off 500 -> 1000 -> 2000', () => { + const e = makeEnv(); e.boot(); + e.last().open(); e.last().close(); + e.fireReconnect(); e.last().close(); + e.fireReconnect(); e.last().close(); + assert.deepStrictEqual(e.state.timers.map(t => t.ms).slice(0, 3), [500, 1000, 2000]); +}); + +test('shows the tombstone and Disconnected after the grace period', () => { + const e = makeEnv(); e.boot(); + e.last().open(); e.last().close(); + e.advance(20000); // past TOMBSTONE_AFTER_MS while still down + e.fireReconnect(); e.last().close(); + assert.strictEqual(e.statusEl.textContent, 'Disconnected'); + assert.strictEqual(e.state.appended.length, 1, 'tombstone appended exactly once'); +}); + +test('reloads to recover when a tombstoned connection comes back', () => { + const e = makeEnv(); e.boot(); + e.last().open(); e.last().close(); + e.advance(20000); e.fireReconnect(); e.last().close(); // tombstone now shown + assert.strictEqual(e.state.reloads, 0); + e.fireReconnect(); e.last().open(); // server back (e.g. same-port restart) + assert.strictEqual(e.state.reloads, 1, 'reloads once on recovery'); +}); + console.log(`\n--- Results: ${passed} passed, ${failed} failed ---`); if (failed > 0) process.exit(1); diff --git a/tests/brainstorm-server/lifecycle.test.js b/tests/brainstorm-server/lifecycle.test.js index fbcb87c0..972d2e5f 100644 --- a/tests/brainstorm-server/lifecycle.test.js +++ b/tests/brainstorm-server/lifecycle.test.js @@ -117,11 +117,15 @@ async function runTests() { let outB = ''; b.stdout.on('data', d => outB += d.toString()); for (let i = 0; i < 60 && !outB.includes('server-started'); i++) await sleep(50); 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 }); assert.notStrictEqual(portB, 3415, 'must not bind the already-taken port'); assert(portB >= 49152, 'should fall back to a random high port'); + // The fallback must NOT clobber the shared port file — A still owns 3415 and + // its open tab must keep reconnecting there. + assert.strictEqual(persisted, '3415', 'fallback must not overwrite .last-port'); }); await test('auto-opens the browser once, on the first screen', async () => {