chore(evals): bump submodule to companion just-in-time scenario

Bump the evals submodule (ff3ee83 -> f1ac859) to include the brainstorming
visual-companion just-in-time eval scenario that validates the SKILL.md
consent-move in this PR (GREEN pass / RED fail on Quorum and drill).

Scope: dev's recorded pointer predates the drill->Quorum migration, so this
bump also carries that migration.

docs/superpowers/plans/2026-06-09-visual-companion-issues.md

@@ -0,0 +1,350 @@
+# Visual Brainstorming Companion — Issue & Change Catalog
+
+**Date:** 2026-06-09
+**Status:** Analysis / triage. We are implementing these ourselves; the referenced
+community PRs are evidence and reference material, **not** code we intend to merge.
+
+## Purpose
+
+A single place that captures every open issue and PR touching the visual
+brainstorming companion (the local server in `skills/brainstorming/scripts/`),
+distilled to the underlying problem and the change we'd make. Each item is
+grounded against the current code, not the PR author's description.
+
+## Scope decisions (Jesse, 2026-06-09)
+
+- **Not vendoring Alpine.js.** PR #1639 (interactive mockups via a vendored
+  Alpine build) is **dropped**. See E3.
+- **E1 (terminal-vs-HTML hard gate) is a workshop item.** We'll design it
+  together; it is not specced here.
+- **E2 (storage location, #975/#977) is deferred** for now.
+- **Remote serving is a first-class scenario.** Superpowers is general-purpose;
+  users connect from remote (SSH tunnel, Tailscale, `--host 0.0.0.0`). The
+  security fix MUST protect those users, not just loopback. **Decision: a
+  per-session secret key**, not a Host/Origin allowlist. The allowlist only
+  defends the loopback browser-confused-deputy; a direct remote client just
+  sends the expected `Host`, so the allowlist is theater for remote exposure. A
+  secret key is the only thing that authenticates a client uniformly across
+  loopback, tunnel, and direct-remote, and it also defeats DNS rebinding. See A1.
+
+## Component map
+
+| File | Role |
+|------|------|
+| `skills/brainstorming/scripts/server.cjs` | Zero-dep HTTP + WebSocket server (RFC 6455 hand-rolled). Serves the newest screen, watches `content/`, records events to `state/events`. |
+| `skills/brainstorming/scripts/helper.js` | Injected into every page. WebSocket client, click capture, `window.brainstorm` API. |
+| `skills/brainstorming/scripts/frame-template.html` | Frame (header, theme CSS, status dot, indicator bar) wrapped around content fragments. |
+| `skills/brainstorming/scripts/start-server.sh` | Launch wrapper. Session dir, host/url-host, owner-PID resolution, platform backgrounding. |
+| `skills/brainstorming/scripts/stop-server.sh` | Kills the server by PID file, cleans `/tmp` sessions. |
+| `skills/brainstorming/visual-companion.md` | Operator guide the agent reads when it accepts the companion. |
+| `skills/brainstorming/SKILL.md` | Where the companion is offered and the per-question decision lives. |
+
+## Disposition summary
+
+| ID | Item | Source | Disposition |
+|----|------|--------|-------------|
+| A1 | Per-session secret key on `/`, `/files/*`, and WS (supersedes Host/Origin) | issues #1014, PRs #1110/#1553 | **Do** — chosen approach |
+| A2 | ~~Host/Origin allowlist~~ | PRs #1110/#1553 | Dropped — subsumed by A1 |
+| A3 | Crash on `null` / non-object WS payload | PR #1504 | Do |
+| A4 | Frame-length bound in `decodeFrame` | issue #1446 | Already fixed — verify/close |
+| B1 | Dotfile screens served as content (`._*.html`) | PR #950 | Do |
+| B2 | `stop-server.sh` kills reused/stale PID | PR #1703 | Do |
+| B3 | WS client reconnect backoff + status indicator | PR #856 | Do |
+| C1 | Idle timeout too short / not configurable; WS not closed on shutdown | issue #1237 (PR #1689) | Do |
+| C2 | Server death is invisible to user/agent | issue #1237 (residual) | Do |
+| D1 | Permanent opt-out of the companion | issue #892 | Do — design choice open |
+| D2 | Free-text feedback from the browser | issue #957 | Do — needs server change too |
+| D3 | Auto-open the companion URL | PR #759 (#755) | Do |
+| D4 | Light/dark contrast helpers in the frame | PR #1683 | Do |
+| E1 | Hard-gate terminal-vs-HTML per question | PR #1037 | **Workshop** |
+| E2 | Move session state out of the working tree | issue #975 (PR #977) | **Deferred** |
+| E3 | Vendor Alpine.js for interactive mockups | PR #1639 | **Dropped** |
+| E4 | Shell-lint warnings in start/stop scripts | PR #1677 | Opportunistic only |
+
+---
+
+## A. Server security hardening (`server.cjs`)
+
+### A1 — Per-session secret key (chosen approach)
+
+**Threat model.** Two assets: confidentiality of the served screen (`/`) and
+files (`/files/*`), and integrity of `state/events` — a WebSocket client with a
+truthy `choice` writes there (`server.cjs:243-246`), and the agent reads it next
+turn as the user's selection, i.e. **prompt injection into a live session with
+full tool access**. Reachers: with the default `127.0.0.1` bind, a malicious
+page in the user's browser (a confused deputy — runs attacker JS *and* can reach
+loopback); with a remote bind (`--host 0.0.0.0`, tailnet/LAN), any host that can
+route to the port, directly, with no same-origin policy in the way. Today
+`handleUpgrade` (`server.cjs:176`) checks only `Sec-WebSocket-Key`, and
+`handleRequest` (`server.cjs:138`) checks nothing — both are wide open.
+
+**Why a key, not an allowlist.** A Host/Origin allowlist only defends the
+loopback browser-deputy. A direct remote client just sends the expected `Host`
+and forges/omits `Origin`, so the allowlist is theater for exactly the remote
+case we must protect. A per-session secret authenticates the client uniformly
+across loopback, SSH tunnel, and direct-remote, and it also kills DNS rebinding
+(the rebound page neither knows the key nor receives the host-scoped cookie).
+So the key **supersedes** A1/A2's allowlist entirely — no `BRAINSTORM_ALLOWED_HOSTS`.
+
+**Design.** Random token (`crypto.randomBytes(32)` hex), generated in
+`server.cjs` at startup (overridable via `BRAINSTORM_TOKEN` for deterministic
+tests):
+
+1. **URL carries it** as `?key=<token>`. The server already builds `url` in its
+   `server-started` JSON (`server.cjs:351`) and writes it to `state/server-info`
+   — appending `?key=` there means `start-server.sh` (greps and prints that
+   JSON) and the skill (hands the user that URL) need **no change**.
+2. **Cookie bootstrap.** A valid `?key` on `/` sets
+   `brainstorm-key-<port>=<token>; HttpOnly; SameSite=Strict; Path=/`. The
+   browser then auto-attaches it to same-origin subresources (`/files/*`) and
+   the WebSocket handshake, so the agent can write any URL style and it works,
+   and `helper.js` needs no change. Cookie name is **per-port** to avoid the
+   Jupyter multi-server collision (cookies aren't port-scoped).
+   `SameSite=Strict` is safe for CDN/Unsplash content — that cookie is host-
+   scoped, so outbound CDN requests never carry it; SameSite only governs
+   requests back to our origin, which are all same-site.
+3. **Auth gate** = valid `?key` **OR** valid cookie (compared with
+   `crypto.timingSafeEqual`) on `/`, `/files/*`, and the WS upgrade. Missing/bad
+   key → friendly **403 HTML page** ("this page needs the full URL your coding
+   agent gave you, including `?key=…`" — generic "coding agent", not "Claude",
+   since this ships on Codex/Gemini/Copilot too). WS upgrade → destroy socket.
+
+The query token is the source of truth; the cookie is a convenience that never
+bears initial-auth load.
+
+**Blast radius.** `server.cjs` (all logic). `helper.js` optional one-liner
+(append `?key=` from `location.search` to the WS URL as a cookie-blocked
+fallback). `start-server.sh` none. `visual-companion.md` doc note (URL now has
+`?key=`; don't strip it). Tests updated to pass the token.
+
+### A2 — Host/Origin allowlist — DROPPED
+
+Subsumed by A1. The secret key closes the WS-injection vector (#1014), the
+HTTP/WS DNS-rebinding read vector (PR #1553), and the cross-origin WS vector
+(PR #1110) in one mechanism, and unlike an allowlist it actually protects the
+remote-bind case. No `Host`/`Origin` validation, no `BRAINSTORM_ALLOWED_HOSTS`.
+
+### A3 — Server crashes on `null` / primitive WS payload
+
+**Problem.** `handleMessage` (`server.cjs:233`) does `JSON.parse(text)` then
+`if (event.choice)` at `server.cjs:243`. A client that sends the 4-byte text
+frame `null` yields `event === null`, and `null.choice` throws. The throw is
+**not** caught — `handleMessage` is called from the `socket.on('data')` handler
+(`server.cjs:207`) outside the `try/catch`, which only wraps `decodeFrame`. The
+result is an uncaught exception and process exit. Any local client can kill the
+server.
+
+**Change.** Guard the access: `if (event && event.choice)`. Minimal and exact —
+`JSON.parse` can't produce `undefined`, and primitives return `undefined` for
+`.choice` without throwing, so only `null` is the live hazard. (Avoid the
+broader fixes — a top-level `try/catch` or `process.on('uncaughtException')`
+would mask other bugs.)
+
+### A4 — Frame-length bound in `decodeFrame` (adjacent)
+
+Referenced by PR #1504 as #1446. The current code **already** bounds extended
+frame lengths: `MAX_FRAME_PAYLOAD_BYTES = 10MB` (`server.cjs:10`) is enforced at
+`server.cjs:58-67` before any `Buffer.alloc`. Action: verify #1446 against
+current `dev` and close if already resolved, rather than re-implementing.
+
+---
+
+## B. Server robustness / correctness
+
+### B1 — macOS resource-fork dotfiles served as screen content
+
+**Problem.** The newest-screen selector filters on `f.endsWith('.html')` only
+(`server.cjs:127-128`). On macOS/ExFAT, `._screen.html` resource-fork files pass
+that filter and, being written alongside the real file, can sort newest — so the
+browser gets binary metadata instead of the mockup. Four read sites share the
+weak filter: `getNewestScreen` (`server.cjs:127`), `knownFiles` init
+(`server.cjs:279`), the `fs.watch` handler (`server.cjs:286`), and the `/files/`
+endpoint (`server.cjs:154-156`).
+
+**Change.** Reject dotfiles (`!f.startsWith('.')`) at all four sites. Covers
+`._*`, `.DS_Store`, etc.
+
+### B2 — `stop-server.sh` can kill a reused PID
+
+**Problem.** `stop-server.sh` reads the PID from `state/server.pid`
+(`stop-server.sh:20`) and `kill`s it (`:23`, escalating to `-9` at `:35`)
+without confirming the PID still belongs to our server. After a reboot or PID
+wraparound the file can point at an unrelated process, which we'd then SIGKILL.
+
+**Change.** Before signalling, verify ownership — the PID's command is `node`
+running our `server.cjs`, ideally matching this session. If ownership can't be
+proven, fail closed (report `stale_pid`, don't kill). Keep the existing
+`stopped` / `not_running` outputs for the real cases.
+
+### B3 — WebSocket client: silent reconnect, stale "Connected"
+
+**Problem.** `helper.js` reconnects on a fixed 1s timer (`helper.js:21-23`),
+has no `onerror` handler, never nulls `ws` on close, and never clears a pending
+reconnect timer. The frame's status element is hardcoded to "Connected" with the
+dot pinned to `var(--success)` (`frame-template.html:77,200`). When the laptop
+sleeps or the server restarts, the page shows "Connected" over a dead socket and
+queues events with no feedback.
+
+**Change.**
+- `helper.js`: exponential backoff (500ms → ×2 → cap 30s, reset on open);
+  `onerror` delegating to `onclose`; `ws = null` on close; `clearTimeout` before
+  reconnecting.
+- `frame-template.html`: drive the status dot from a `--status-color` custom
+  property so JS can switch Connected (green) / Reconnecting (yellow) /
+  Disconnected (red).
+
+---
+
+## C. Lifecycle / timeout (issue #1237)
+
+### C1 — Idle timeout too short, not configurable, WS keeps process alive
+
+**Problem.** `IDLE_TIMEOUT_MS` is hardcoded to 30 minutes (`server.cjs:258`),
+enforced by the 60s lifecycle check (`server.cjs:329-332`). A single brainstorm
+question can sit longer than 30 min while the user thinks or steps away, so the
+server dies mid-session. Separately, `shutdown()` (`server.cjs:310-321`) calls
+`server.close()` but never closes the upgraded sockets in `clients`
+(`server.cjs:174`), so an open browser connection can keep the Node process
+alive past shutdown.
+
+**Change.**
+- Raise the default (≈2h) and make it configurable:
+  `--idle-timeout-minutes` in `start-server.sh` → an env var → `IDLE_TIMEOUT_MS`,
+  with validation against Node timer overflow.
+- Expose the effective timeout in the startup JSON / `state/server-info`.
+- In `shutdown()`, close every socket in `clients` so the process actually
+  exits.
+
+### C2 — Server death is invisible
+
+**Problem.** When the server exits it writes `state/server-stopped` and removes
+`state/server-info` (`server.cjs:312-317`), and the skill is *told* to check
+those files (`visual-companion.md:98`) — but it's soft guidance the model skips,
+and the browser just shows a generic "can't be reached." The user diagnoses it
+manually; the agent keeps referring to a dead URL.
+
+**Change (two parts, independent of C1):**
+- **Browser-facing tombstone.** Leave something at the last-served URL that says
+  "this companion expired — ask Claude to restart it" instead of a connection
+  error. Options to weigh: `helper.js` rendering a banner when the socket stays
+  down past backoff (works only while the page is loaded), vs. a more involved
+  approach that keeps a minimal responder alive to serve a tombstone page.
+- **Harder skill check.** Tighten `visual-companion.md` / `SKILL.md` so
+  "check `server-info`/`server-stopped` before referring to the URL or pushing a
+  screen" is a required step, not a note. Keep it lightweight — possibly a
+  one-line helper the agent always runs.
+
+---
+
+## D. Features
+
+### D1 — Permanent opt-out of the visual companion (issue #892)
+
+**Problem.** The companion is offered as its own message every session
+(`SKILL.md:25,151-152`). A user who never wants it pays that round-trip — and
+HTML generation — every time. There's no way to say "never offer this."
+
+**Change.** Before the offer step, the skill checks a user-level setting and
+skips the offer entirely when opt-out is set.
+
+**Design choice open.** Mechanism isn't settled:
+- Env var (e.g. `SUPERPOWERS_VISUAL_COMPANION=off`) the skill is told to read —
+  simplest, matches what the issue asks for, lives in `.zshrc`.
+- A plugin-settings file (`.claude/superpowers.local.md` frontmatter) — more
+  structured, per-project capable, but heavier and project-scoped.
+- Reliability caveat from the issue: a separate "no-companion" skill competes on
+  trigger words and isn't reliable — rejected.
+
+Pick the mechanism, then it's a small `SKILL.md` change plus a documented knob.
+
+### D2 — Free-text feedback from the browser (issue #957)
+
+**Problem.** The client only captures clicks on `[data-choice]`
+(`helper.js:36-62`). A user who wants to annotate a mockup ("wrong shade of
+blue") has to switch to the terminal, breaking the visual flow.
+
+**Change.** Add a feedback `<textarea>` whose submit emits
+`{"type":"feedback","text":...,"timestamp":...}` via the existing
+`window.brainstorm.send` path (`helper.js:82-85`).
+
+**Cross-cutting — server change required.** `handleMessage` only persists events
+when `event.choice` is truthy (`server.cjs:243`). A `feedback` event has no
+`choice`, so today it would be logged but **never written to `state/events`**,
+and the agent wouldn't see it. The persistence condition must also accept
+`feedback` events. Document the new event shape in `visual-companion.md`
+(Browser Events Format, `:247-259`). Decide the submit trigger (button vs blur
+vs both) and where the textarea renders (frame-level vs opt-in per screen).
+
+### D3 — Auto-open the companion URL (PR #759, issue #755)
+
+**Problem.** `start-server.sh` only prints the URL; the user opens it manually.
+In WSL2 especially, people expect the browser to open.
+
+**Change.** Best-effort opener after the `server-started` JSON is parsed:
+WSL → `cmd.exe /c start`, macOS → `open`, Linux → `xdg-open` only when
+`DISPLAY`/`WAYLAND_DISPLAY` is set. Swallow failures, never block startup, keep
+echoing the URL. Document in `visual-companion.md`. (Consider an opt-out for
+headless/remote runs where popping a browser is wrong — ties into D1's config
+mechanism.)
+
+### D4 — Light/dark contrast helpers (PR #1683)
+
+**Problem.** Content fragments are wrapped in the OS-aware frame
+(`frame-template.html`). In dark mode, quick mockups often use white inline
+backgrounds while inheriting low-contrast frame text, making cards/panels hard
+to read.
+
+**Change.** Add `.light-surface` / `.dark-surface` helper classes plus a
+conservative fallback for common inline light backgrounds, and document them in
+`visual-companion.md`'s CSS reference. Pure CSS in `frame-template.html`.
+
+---
+
+## E. Workshop / deferred / dropped
+
+### E1 — Hard-gate terminal-vs-HTML per question (PR #1037) — WORKSHOP
+
+The soft guidance already exists: "decide per-question," with browser-vs-terminal
+tests in `SKILL.md:156-161` and `visual-companion.md:5-25`. The complaint is that
+the model renders HTML for purely textual content (A/B lists, clarifying
+questions), wasting tokens and a turn. PR #1037 wraps the decision in a
+`<HARD-GATE>`. **Per Jesse, we'll workshop the wording/mechanism together** —
+this is behavior-shaping skill content and not specced here.
+
+### E2 — Move session state out of the working tree (issue #975 / PR #977) — DEFERRED
+
+Today `--project-dir` writes session state to `<project>/.superpowers/brainstorm/`
+(`start-server.sh:80-84`) and the skill tells the user to gitignore it
+(`visual-companion.md:48`). The ask is a `--state-dir` / `SUPERPOWERS_STATE_DIR`
+default outside the repo (XDG), keeping `--project-dir` as an alias.
+**Deferred by Jesse for now.** Captured so it isn't lost.
+
+### E3 — Vendor Alpine.js for interactive mockups (PR #1639) — DROPPED
+
+Adds a vendored Alpine build so mockups can be interactive (tabs, accordions,
+forms) without hand-rolled JS. **Dropped per Jesse** — we are not taking on a
+vendored third-party dependency in the companion runtime. The underlying need
+(interactive mockups) is not being pursued via this route.
+
+### E4 — Shell-lint warnings (PR #1677) — OPPORTUNISTIC
+
+SC2034 (and friends) in `start-server.sh` / `stop-server.sh`. Trivial; fold into
+B2/C1/D3 when we're already editing those scripts rather than as its own change.
+
+---
+
+## Suggested grouping for implementation
+
+These cluster into a few coherent passes (each independently testable against
+`tests/brainstorm-server/`):
+
+1. **Security pass** (IN PROGRESS, branch `brainstorm-companion-session-key`) —
+   A1 per-session key (supersedes A2) + A3 null-crash guard. Verify/close A4.
+   *Highest priority.*
+2. **Lifecycle pass** — C1 + C2 together (both touch `shutdown()` and the
+   server-death story).
+3. **Robustness pass** — B1, B2, B3 (independent, small).
+4. **Feature pass** — D1, D2, D3, D4 (D2 needs the server persistence tweak;
+   D1's config mechanism is shared with D3's opt-out).
+
+E1 is a separate workshop session. E2/E3 are out of scope for this round.

skills/brainstorming/SKILL.md

@@ -22,11 +22,11 @@ Every project goes through this process. A todo list, a single-function utility,
 You MUST create a task for each of these items and complete them in order:
 
 1. **Explore project context** — check files, docs, recent commits
-2. **Offer visual companion** (if topic will involve visual questions) — this is its own message, not combined with a clarifying question. See the Visual Companion section below.
+2. **Offer the visual companion just-in-time** — NOT upfront. The first time a question would genuinely be clearer shown than described, offer it then (its own message); on approval its browser tab opens for you. If no visual question ever arises, never offer it. See the Visual Companion section below.
 3. **Ask clarifying questions** — one at a time, understand purpose/constraints/success criteria
 4. **Propose 2-3 approaches** — with trade-offs and your recommendation
 5. **Present design** — in sections scaled to their complexity, get user approval after each section
-6. **Write design doc** — save to `docs/superpowers/specs/YYYY-MM-DD-<topic>-design.md` and commit (exactly this path — not `docs/specs/`)
+6. **Write design doc** — save to `docs/superpowers/specs/YYYY-MM-DD-<topic>-design.md` and commit
 7. **Spec self-review** — quick inline check for placeholders, contradictions, ambiguity, scope (see below)
 8. **User reviews written spec** — ask user to review the spec file before proceeding
 9. **Transition to implementation** — invoke writing-plans skill to create implementation plan
@@ -36,8 +36,6 @@ You MUST create a task for each of these items and complete them in order:
 ```dot
 digraph brainstorming {
     "Explore project context" [shape=box];
-    "Visual questions ahead?" [shape=diamond];
-    "Offer Visual Companion\n(own message, no other content)" [shape=box];
     "Ask clarifying questions" [shape=box];
     "Propose 2-3 approaches" [shape=box];
     "Present design sections" [shape=box];
@@ -47,10 +45,7 @@ digraph brainstorming {
     "User reviews spec?" [shape=diamond];
     "Invoke writing-plans skill" [shape=doublecircle];
 
-    "Explore project context" -> "Visual questions ahead?";
-    "Visual questions ahead?" -> "Offer Visual Companion\n(own message, no other content)" [label="yes"];
-    "Visual questions ahead?" -> "Ask clarifying questions" [label="no"];
-    "Offer Visual Companion\n(own message, no other content)" -> "Ask clarifying questions";
+    "Explore project context" -> "Ask clarifying questions";
     "Ask clarifying questions" -> "Propose 2-3 approaches";
     "Propose 2-3 approaches" -> "Present design sections";
     "Present design sections" -> "User approves design?";
@@ -109,7 +104,6 @@ digraph brainstorming {
 **Documentation:**
 
 - Write the validated design (spec) to `docs/superpowers/specs/YYYY-MM-DD-<topic>-design.md`
-  - The `docs/superpowers/` prefix is the convention; do not shorten it to `docs/specs/`
   - (User preferences for spec location override this default)
 - Use elements-of-style:writing-clearly-and-concisely skill if available
 - Commit the design document to git
@@ -149,10 +143,10 @@ Wait for the user's response. If they request changes, make them and re-run the
 
 A browser-based companion for showing mockups, diagrams, and visual options during brainstorming. Available as a tool — not a mode. Accepting the companion means it's available for questions that benefit from visual treatment; it does NOT mean every question goes through the browser.
 
-**Offering the companion:** When you anticipate that upcoming questions will involve visual content (mockups, layouts, diagrams), offer it once for consent:
-> "Some of what we're working on might be easier to explain if I can show it to you in a web browser. I can put together mockups, diagrams, comparisons, and other visuals as we go. This feature is still new and can be token-intensive. Want to try it? (Requires opening a local URL)"
+**Offering the companion (just-in-time):** Do NOT offer it upfront. Wait until a question would genuinely be clearer shown than told — a real mockup / layout / diagram question, not merely a UI *topic*. The first time that happens, offer it then, as its own message:
+> "This next part might be easier if I show you — I can put together mockups, diagrams, and comparisons in a browser tab as we go. It's still new and can be token-intensive. Want me to? I'll open it for you."
 
-**This offer MUST be its own message.** Do not combine it with clarifying questions, context summaries, or any other content. The message should contain ONLY the offer above and nothing else. Wait for the user's response before continuing. If they decline, proceed with text-only brainstorming.
+**This offer MUST be its own message.** Only the offer — no clarifying question, summary, or other content. Wait for the user's response. If they accept, start the server with `--open` so their browser opens to the first screen automatically. If they decline, continue text-only and don't offer again unless they raise it.
 
 **Per-question decision:** Even after the user accepts, decide FOR EACH QUESTION whether to use the browser or the terminal. The test: **would the user understand this better by seeing it than reading it?**
 

skills/brainstorming/scripts/frame-template.html

@@ -73,8 +73,8 @@
       flex-shrink: 0;
     }
     .header h1 { font-size: 0.85rem; font-weight: 500; color: var(--text-secondary); }
-    .header .status { font-size: 0.7rem; color: var(--success); display: flex; align-items: center; gap: 0.4rem; }
-    .header .status::before { content: ''; width: 6px; height: 6px; background: var(--success); border-radius: 50%; }
+    .header .status { font-size: 0.7rem; color: var(--status-color, var(--success)); display: flex; align-items: center; gap: 0.4rem; }
+    .header .status::before { content: ''; width: 6px; height: 6px; background: var(--status-color, var(--success)); border-radius: 50%; }
 
     .main { flex: 1; overflow-y: auto; }
     #frame-content { padding: 2rem; min-height: 100%; }
@@ -197,7 +197,7 @@
 <body>
   <div class="header">
     <h1><a href="https://github.com/obra/superpowers" style="color: inherit; text-decoration: none;">Superpowers Brainstorming</a></h1>
-    <div class="status">Connected</div>
+    <div class="status">Connecting…</div>
   </div>
 
   <div class="main">

skills/brainstorming/scripts/helper.js

@@ -1,26 +1,99 @@
 (function() {
+  const MIN_RECONNECT_MS = 500;
+  const MAX_RECONNECT_MS = 30000;
+  const TOMBSTONE_AFTER_MS = 15000; // show the "paused" overlay after this long disconnected
+
+  // Pure: next backoff delay (doubles, capped). Exported for unit tests.
+  function nextReconnectDelay(current, max) {
+    return Math.min(current * 2, max);
+  }
+  if (typeof module !== 'undefined' && module.exports) {
+    module.exports = { nextReconnectDelay, MIN_RECONNECT_MS, MAX_RECONNECT_MS, TOMBSTONE_AFTER_MS };
+  }
+
+  // Everything below is browser-only; bail out when loaded in Node (tests).
+  if (typeof window === 'undefined') return;
+
   const WS_URL = 'ws://' + window.location.host;
   let ws = null;
   let eventQueue = [];
+  let reconnectDelay = MIN_RECONNECT_MS;
+  let reconnectTimer = null;
+  let disconnectedSince = null;
+  let everConnected = false;
+  let tombstoneShown = false;
+
+  // Reflect connection state in the frame's status pill (absent on full-doc screens).
+  function setStatus(state) {
+    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)']
+    };
+    const [text, color] = map[state] || map.disconnected;
+    el.textContent = text;
+    el.style.setProperty('--status-color', color);
+  }
+
+  // Self-styled so it works on framed and full-document screens alike.
+  function showTombstone() {
+    if (tombstoneShown) return;
+    tombstoneShown = true;
+    const el = document.createElement('div');
+    el.id = 'bs-tombstone';
+    el.style.cssText = 'position:fixed;inset:0;z-index:99999;display:flex;' +
+      'align-items:center;justify-content:center;padding:2rem;text-align:center;' +
+      'background:rgba(20,20,22,0.92);color:#f5f5f7;font-family:system-ui,sans-serif';
+    el.innerHTML = '<div style="max-width:480px">' +
+      '<h2 style="margin:0 0 .5rem;font-weight:600">Companion paused</h2>' +
+      '<p style="margin:0;opacity:.85">This brainstorm companion has stopped. ' +
+      'Ask your coding agent to bring it back — this page reconnects automatically.</p></div>';
+    if (document.body) document.body.appendChild(el);
+  }
 
   function connect() {
+    if (reconnectTimer) { clearTimeout(reconnectTimer); reconnectTimer = null; }
+    setStatus(everConnected ? 'reconnecting' : 'connecting');
     ws = new WebSocket(WS_URL);
 
     ws.onopen = () => {
+      const recovered = tombstoneShown;
+      everConnected = true;
+      disconnectedSince = null;
+      reconnectDelay = MIN_RECONNECT_MS;
+      tombstoneShown = false;
+      setStatus('connected');
       eventQueue.forEach(e => ws.send(JSON.stringify(e)));
       eventQueue = [];
+      // Recovered from a tombstoned outage (e.g. the server restarted on the same
+      // port) — reload to pick up the restarted server's current screen.
+      if (recovered) window.location.reload();
     };
 
     ws.onmessage = (msg) => {
-      const data = JSON.parse(msg.data);
-      if (data.type === 'reload') {
-        window.location.reload();
-      }
+      let data;
+      try { data = JSON.parse(msg.data); } catch (e) { return; }
+      if (data.type === 'reload') window.location.reload();
     };
 
     ws.onclose = () => {
-      setTimeout(connect, 1000);
+      ws = null;
+      if (disconnectedSince === null) disconnectedSince = Date.now();
+      if (Date.now() - disconnectedSince >= TOMBSTONE_AFTER_MS) {
+        setStatus('disconnected');
+        showTombstone();
+      } else {
+        setStatus('reconnecting');
+      }
+      reconnectTimer = setTimeout(connect, reconnectDelay);
+      reconnectDelay = nextReconnectDelay(reconnectDelay, MAX_RECONNECT_MS);
     };
+
+    // Let onclose own reconnection so we don't schedule it twice.
+    ws.onerror = () => { try { ws.close(); } catch (e) {} };
   }
 
   function sendEvent(event) {

skills/brainstorming/scripts/server.cjs

@@ -82,7 +82,21 @@ function decodeFrame(buffer) {
 
 // ========== Configuration ==========
 
-const PORT = process.env.BRAINSTORM_PORT || (49152 + Math.floor(Math.random() * 16383));
+const PORT_FILE = process.env.BRAINSTORM_PORT_FILE || null;
+const randomPort = () => 49152 + Math.floor(Math.random() * 16383);
+// Prefer an explicit port, else the port this session last bound (so a restart
+// reuses it and an already-open browser tab reconnects), else a random high port.
+function preferredPort() {
+  if (process.env.BRAINSTORM_PORT) return Number(process.env.BRAINSTORM_PORT);
+  if (PORT_FILE) {
+    try {
+      const p = Number(fs.readFileSync(PORT_FILE, 'utf-8').trim());
+      if (Number.isInteger(p) && p > 1023 && p < 65536) return p;
+    } catch (e) { /* no prior port recorded */ }
+  }
+  return randomPort();
+}
+let PORT = preferredPort();
 const HOST = process.env.BRAINSTORM_HOST || '127.0.0.1';
 const URL_HOST = process.env.BRAINSTORM_URL_HOST || (HOST === '127.0.0.1' ? 'localhost' : HOST);
 const SESSION_DIR = process.env.BRAINSTORM_DIR || '/tmp/brainstorm';
@@ -90,6 +104,27 @@ const CONTENT_DIR = path.join(SESSION_DIR, 'content');
 const STATE_DIR = path.join(SESSION_DIR, 'state');
 let ownerPid = process.env.BRAINSTORM_OWNER_PID ? Number(process.env.BRAINSTORM_OWNER_PID) : null;
 
+// Per-session secret key. The companion is reachable by any local browser tab
+// and, when bound to a non-loopback host, by any host that can route to it.
+// The key authenticates the real client uniformly across loopback, tunnel, and
+// remote binds — and defeats DNS rebinding — where a Host/Origin allowlist
+// cannot. It rides the served URL as ?key= and is mirrored into a cookie on
+// first load so same-origin subresources and the WebSocket carry it for free.
+// Persisted alongside the port (BRAINSTORM_TOKEN_FILE) so a restart keeps the
+// same key and an already-open tab's cookie still validates.
+const TOKEN_FILE = process.env.BRAINSTORM_TOKEN_FILE || null;
+const TOKEN = (() => {
+  if (process.env.BRAINSTORM_TOKEN) return process.env.BRAINSTORM_TOKEN;
+  if (TOKEN_FILE) {
+    try {
+      const t = fs.readFileSync(TOKEN_FILE, 'utf-8').trim();
+      if (/^[0-9a-f]{32,}$/i.test(t)) return t;
+    } catch (e) { /* no prior token recorded */ }
+  }
+  return crypto.randomBytes(32).toString('hex');
+})();
+let COOKIE_NAME = 'brainstorm-key-' + PORT; // refined to the actual bound port in onListen
+
 const MIME_TYPES = {
   '.html': 'text/html', '.css': 'text/css', '.js': 'application/javascript',
   '.json': 'application/json', '.png': 'image/png', '.jpg': 'image/jpeg',
@@ -107,6 +142,16 @@ h1 { color: #333; } p { color: #666; }</style>
 <body><h1>Brainstorm Companion</h1>
 <p>Waiting for the agent to push a screen...</p></body></html>`;
 
+const FORBIDDEN_PAGE = `<!DOCTYPE html>
+<html>
+<head><meta charset="utf-8"><title>Session key required</title>
+<style>body { font-family: system-ui, sans-serif; padding: 2rem; max-width: 800px; margin: 0 auto; }
+h1 { color: #333; } p { color: #666; } code { background: #f0f0f0; padding: 0.1em 0.3em; border-radius: 4px; }</style>
+</head>
+<body><h1>Session key required</h1>
+<p>This page needs the full URL your coding agent gave you, including the
+<code>?key=&hellip;</code> part. Copy the complete URL and open it again.</p></body></html>`;
+
 const frameTemplate = fs.readFileSync(path.join(__dirname, 'frame-template.html'), 'utf-8');
 const helperScript = fs.readFileSync(path.join(__dirname, 'helper.js'), 'utf-8');
 const helperInjection = '<script>\n' + helperScript + '\n</script>';
@@ -124,7 +169,7 @@ function wrapInFrame(content) {
 
 function getNewestScreen() {
   const files = fs.readdirSync(CONTENT_DIR)
-    .filter(f => f.endsWith('.html'))
+    .filter(f => !f.startsWith('.') && f.endsWith('.html'))
     .map(f => {
       const fp = path.join(CONTENT_DIR, f);
       return { path: fp, mtime: fs.statSync(fp).mtime.getTime() };
@@ -133,11 +178,63 @@ function getNewestScreen() {
   return files.length > 0 ? files[0].path : null;
 }
 
+// ========== Authentication ==========
+
+function timingSafeEqualStr(a, b) {
+  const ab = Buffer.from(String(a));
+  const bb = Buffer.from(String(b));
+  if (ab.length !== bb.length) return false;
+  return crypto.timingSafeEqual(ab, bb);
+}
+
+function parseCookies(header) {
+  const out = {};
+  if (!header) return out;
+  for (const part of header.split(';')) {
+    const eq = part.indexOf('=');
+    if (eq < 0) continue;
+    out[part.slice(0, eq).trim()] = part.slice(eq + 1).trim();
+  }
+  return out;
+}
+
+// A request is authorized if it carries the session key as ?key= or as the
+// session cookie. Both are compared in constant time.
+function isAuthorized(req) {
+  const q = req.url.indexOf('?');
+  if (q >= 0) {
+    const key = new URLSearchParams(req.url.slice(q + 1)).get('key');
+    if (key && timingSafeEqualStr(key, TOKEN)) return true;
+  }
+  const cookie = parseCookies(req.headers['cookie'])[COOKIE_NAME];
+  if (cookie && timingSafeEqualStr(cookie, TOKEN)) return true;
+  return false;
+}
+
+function pathnameOf(url) {
+  const q = url.indexOf('?');
+  return q >= 0 ? url.slice(0, q) : url;
+}
+
 // ========== HTTP Request Handler ==========
 
 function handleRequest(req, res) {
-  touchActivity();
-  if (req.method === 'GET' && req.url === '/') {
+  if (!isAuthorized(req)) {
+    res.writeHead(403, { 'Content-Type': 'text/html; charset=utf-8' });
+    res.end(FORBIDDEN_PAGE);
+    return;
+  }
+  touchActivity(); // only authorized requests count as activity
+
+  // Mirror the key into a cookie so same-origin subresources (/files/*) and the
+  // WebSocket handshake carry it automatically, whatever URL style the agent
+  // writes. SameSite=Strict: a cross-site page can neither read the key nor ride
+  // the cookie; HttpOnly: page scripts can't exfiltrate it.
+  res.setHeader('Set-Cookie',
+    COOKIE_NAME + '=' + TOKEN + '; HttpOnly; SameSite=Strict; Path=/');
+
+  const pathname = pathnameOf(req.url);
+  if (req.method === 'GET' && pathname === '/') {
     const screenFile = getNewestScreen();
     let html = screenFile
       ? (raw => isFullDocument(raw) ? raw : wrapInFrame(raw))(fs.readFileSync(screenFile, 'utf-8'))
@@ -151,10 +248,12 @@ function handleRequest(req, res) {
 
     res.writeHead(200, { 'Content-Type': 'text/html; charset=utf-8' });
     res.end(html);
-  } else if (req.method === 'GET' && req.url.startsWith('/files/')) {
-    const fileName = req.url.slice(7);
-    const filePath = path.join(CONTENT_DIR, path.basename(fileName));
-    if (!fs.existsSync(filePath)) {
+  } else if (req.method === 'GET' && pathname.startsWith('/files/')) {
+    const fileName = path.basename(pathname.slice(7));
+    const filePath = path.join(CONTENT_DIR, fileName);
+    // Reject empty/dotfile names and anything that isn't a regular file —
+    // `/files/` would otherwise resolve to CONTENT_DIR and crash readFileSync (EISDIR).
+    if (!fileName || fileName.startsWith('.') || !fs.existsSync(filePath) || !fs.statSync(filePath).isFile()) {
       res.writeHead(404);
       res.end('Not found');
       return;
@@ -174,6 +273,8 @@ function handleRequest(req, res) {
 const clients = new Set();
 
 function handleUpgrade(req, socket) {
+  if (!isAuthorized(req)) { socket.destroy(); return; }
+
   const key = req.headers['sec-websocket-key'];
   if (!key) { socket.destroy(); return; }
 
@@ -240,7 +341,7 @@ function handleMessage(text) {
   }
   touchActivity();
   console.log(JSON.stringify({ source: 'user-event', ...event }));
-  if (event.choice) {
+  if (event && event.choice) {
     const eventsFile = path.join(STATE_DIR, 'events');
     fs.appendFileSync(eventsFile, JSON.stringify(event) + '\n');
   }
@@ -253,9 +354,48 @@ function broadcast(msg) {
   }
 }
 
+// Best-effort: open the user's browser the first time a screen is actually ready
+// to show. Skips when disabled, on a non-loopback (remote) bind, or when a
+// browser is already connected. Override the launcher with BRAINSTORM_OPEN_CMD.
+let browserOpened = false;
+function maybeOpenBrowser() {
+  if (browserOpened) return;
+  browserOpened = true;
+  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 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;
+  }
+  // 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 ==========
 
-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() {
@@ -276,14 +416,14 @@ function startServer() {
   // macOS fs.watch reports 'rename' for both new files and overwrites,
   // so we can't rely on eventType alone.
   const knownFiles = new Set(
-    fs.readdirSync(CONTENT_DIR).filter(f => f.endsWith('.html'))
+    fs.readdirSync(CONTENT_DIR).filter(f => !f.startsWith('.') && f.endsWith('.html'))
   );
 
   const server = http.createServer(handleRequest);
   server.on('upgrade', handleUpgrade);
 
   const watcher = fs.watch(CONTENT_DIR, (eventType, filename) => {
-    if (!filename || !filename.endsWith('.html')) return;
+    if (!filename || filename.startsWith('.') || !filename.endsWith('.html')) return;
 
     if (debounceTimers.has(filename)) clearTimeout(debounceTimers.get(filename));
     debounceTimers.set(filename, setTimeout(() => {
@@ -298,6 +438,7 @@ function startServer() {
         const eventsFile = path.join(STATE_DIR, 'events');
         if (fs.existsSync(eventsFile)) fs.unlinkSync(eventsFile);
         console.log(JSON.stringify({ type: 'screen-added', file: filePath }));
+        maybeOpenBrowser();
       } else {
         console.log(JSON.stringify({ type: 'screen-updated', file: filePath }));
       }
@@ -317,6 +458,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 +471,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
@@ -345,15 +491,46 @@ function startServer() {
     }
   }
 
-  server.listen(PORT, HOST, () => {
+  // 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() {
+    // Cookie name keys on the ACTUAL bound port (may differ from the preferred
+    // one after an EADDRINUSE fallback) so it can't collide with another server's
+    // cookie in the shared localhost jar.
+    COOKIE_NAME = 'brainstorm-key-' + PORT;
+    // Record the bound port AND token so the next restart of this session reuses
+    // them — but ONLY when we got our preferred port. On a fallback we bound a
+    // *different* port because someone else holds the preferred one; persisting
+    // would overwrite the shared files and strand that other session's open tab.
+    if (PORT_FILE && !triedFallback) {
+      try { fs.writeFileSync(PORT_FILE, String(PORT)); } catch (e) { /* best effort */ }
+      if (TOKEN_FILE) {
+        try { fs.writeFileSync(TOKEN_FILE, TOKEN, { mode: 0o600 }); } catch (e) { /* best effort */ }
+      }
+    }
     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
+      url_host: URL_HOST, url: 'http://' + URL_HOST + ':' + PORT + '/?key=' + TOKEN,
+      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');
+    // server-info embeds the key — keep it owner-only.
+    fs.writeFileSync(path.join(STATE_DIR, 'server-info'), info + '\n', { mode: 0o600 });
+  }
+
+  server.on('error', (err) => {
+    if (err.code === 'EADDRINUSE' && !triedFallback) {
+      triedFallback = true;
+      PORT = randomPort();
+      server.listen(PORT, HOST, onListen);
+    } else {
+      console.error('Server failed to bind:', err.message);
+      process.exit(1);
+    }
   });
+  server.listen(PORT, HOST, onListen);
 }
 
 if (require.main === module) {

skills/brainstorming/scripts/start-server.sh

@@ -11,6 +11,9 @@
 #   --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).
+#   --open                Auto-open the browser on the first screen (use only
+#                         after the user approves the visual companion).
 #   --foreground          Run server in the current terminal (no backgrounding).
 #   --background          Force background mode (overrides Codex auto-foreground).
 
@@ -22,6 +25,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 +40,14 @@ while [[ $# -gt 0 ]]; do
       URL_HOST="$2"
       shift 2
       ;;
+    --idle-timeout-minutes)
+      IDLE_TIMEOUT_MINUTES="$2"
+      shift 2
+      ;;
+    --open)
+      export BRAINSTORM_OPEN=1
+      shift
+      ;;
     --foreground|--no-daemon)
       FOREGROUND="true"
       shift
@@ -59,6 +71,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"
@@ -74,11 +94,19 @@ if [[ "$FOREGROUND" != "true" && "$FORCE_BACKGROUND" != "true" ]]; then
   fi
 fi
 
+# Session files (server.log, server-info, .last-token) embed the session key —
+# keep everything this script and the server create owner-only.
+umask 077
+
 # Generate unique session directory
 SESSION_ID="$$-$(date +%s)"
 
 if [[ -n "$PROJECT_DIR" ]]; then
   SESSION_DIR="${PROJECT_DIR}/.superpowers/brainstorm/${SESSION_ID}"
+  # Persist the bound port and key per project so a restart reuses them and an
+  # already-open browser tab reconnects to the same URL with a valid cookie.
+  export BRAINSTORM_PORT_FILE="${PROJECT_DIR}/.superpowers/brainstorm/.last-port"
+  export BRAINSTORM_TOKEN_FILE="${PROJECT_DIR}/.superpowers/brainstorm/.last-token"
 else
   SESSION_DIR="/tmp/brainstorm-${SESSION_ID}"
 fi

skills/brainstorming/scripts/stop-server.sh

@@ -16,9 +16,40 @@ fi
 STATE_DIR="${SESSION_DIR}/state"
 PID_FILE="${STATE_DIR}/server.pid"
 
+# Confirm a PID is actually our brainstorm server (node running server.cjs),
+# 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
+    *node*server.cjs*) ;;
+    *) return 1 ;;
+  esac
+  # Stronger check: if we recorded the bound port and lsof is available, require
+  # the PID to be the process actually LISTENING on this session's port. This
+  # rules out an unrelated `node ... server.cjs` (another project, an editor task
+  # runner, a different session) that happened to recycle the stale PID.
+  local info="${STATE_DIR}/server-info"
+  if [[ -f "$info" ]] && command -v lsof >/dev/null 2>&1; then
+    local port
+    port=$(sed -n 's/.*"port":\([0-9][0-9]*\).*/\1/p' "$info" | head -1)
+    if [[ -n "$port" ]]; then
+      [[ "$(lsof -nP -iTCP:"$port" -sTCP:LISTEN -t 2>/dev/null | head -1)" == "$1" ]] || return 1
+    fi
+  fi
+  return 0
+}
+
 if [[ -f "$PID_FILE" ]]; then
   pid=$(cat "$PID_FILE")
 
+  # Refuse to signal a PID we can't prove is our server. A stale pid file may
+  # point at an unrelated process after a reboot/PID wraparound.
+  if ! is_brainstorm_server "$pid"; then
+    rm -f "$PID_FILE"
+    echo '{"status": "stale_pid"}'
+    exit 0
+  fi
+
   # Try to stop gracefully, fallback to force if still alive
   kill "$pid" 2>/dev/null || true
 

skills/brainstorming/visual-companion.md

@@ -33,15 +33,25 @@ The server watches a directory for HTML files and serves the newest one to the b
 ## Starting a Session
 
 ```bash
-# Start server with persistence (mockups saved to project)
-scripts/start-server.sh --project-dir /path/to/project
+# Start AFTER the user approves the companion. --open auto-opens their browser on
+# the first screen; --project-dir persists mockups and enables same-port restart.
+scripts/start-server.sh --project-dir /path/to/project --open
 
-# Returns: {"type":"server-started","port":52341,"url":"http://localhost:52341",
+# Returns: {"type":"server-started","port":52341,
+#           "url":"http://localhost:52341/?key=ab12…",
 #           "screen_dir":"/path/to/project/.superpowers/brainstorm/12345-1706000000/content",
 #           "state_dir":"/path/to/project/.superpowers/brainstorm/12345-1706000000/state"}
 ```
 
-Save `screen_dir` and `state_dir` from the response. Tell user to open the URL.
+Save `screen_dir` and `state_dir` from the response. With `--open`, the browser opens itself when you push the first screen — you don't need to ask the user to open it, but still share the URL as a fallback (headless/remote setups won't auto-open).
+
+**The URL contains a session key (`?key=…`).** The server rejects any request
+without it, so always give the user the **complete** URL from the `url` field —
+never strip the query string, and never hand out a bare `http://host:port`. The
+key gates HTTP and WebSocket access so a stray browser tab or another machine on
+the network can't read the screens or inject events. After the first load the
+browser remembers the key via a cookie, so reloads and `/files/*` assets work
+without repeating it.
 
 **Finding connection info:** The server writes its startup JSON to `$STATE_DIR/server-info`. If you launched the server in the background and didn't capture stdout, read that file to get the URL and port. When using `--project-dir`, check `<project>/.superpowers/brainstorm/` for the session directory.
 
@@ -95,7 +105,7 @@ Use `--url-host` to control what hostname is printed in the returned URL JSON.
 ## The Loop
 
 1. **Check server is alive**, then **write HTML** to a new file in `screen_dir`:
-   - Before each write, check that `$STATE_DIR/server-info` exists. If it doesn't (or `$STATE_DIR/server-stopped` exists), the server has shut down — restart it with `start-server.sh` before continuing. The server auto-exits after 30 minutes of inactivity.
+   - **Required: confirm the server is alive before referring to the URL or pushing a screen.** Check that `$STATE_DIR/server-info` exists and `$STATE_DIR/server-stopped` does not. If it has shut down, restart it with `start-server.sh` using the **same `--project-dir`** — it reuses the same port, so the user's open tab reconnects on its own (it shows a "paused" overlay while the server is down) and you don't need to send a new URL. The server auto-exits after 4 hours idle (configurable with `--idle-timeout-minutes`).
    - Use semantic filenames: `platform.html`, `visual-style.html`, `layout.html`
    - **Never reuse filenames** — each screen gets a fresh file
    - Use your file-creation tool — **never use cat/heredoc** (dumps noise into terminal)

skills/subagent-driven-development/SKILL.md

@@ -11,8 +11,6 @@ Execute plan by dispatching fresh subagent per task, with two-stage review after
 
 **Core principle:** Fresh subagent per task + two-stage review (spec then quality) = high quality, fast iteration
 
-**Proportionality:** Review fanout scales with the change. When the entire plan is one trivial, fully-specified mechanical change — a one-line edit, a log statement, a constant bump — implement it directly (or with a single implementer subagent), verify it, and commit. Skip the review subagents and the final reviewer: a diff with no room for interpretation has nothing for a spec or quality review to catch, and three dispatches for one line cost more than the change itself. When in doubt whether a change is trivial, it is not — run the full pipeline. Within a multi-task plan, run the full pipeline for every task regardless of size; this exception applies only when the whole plan is one trivial change.
-
 **Continuous execution:** Do not pause to check in with your human partner between tasks. Execute all tasks from the plan without stopping. The only reasons to stop are: BLOCKED status you cannot resolve, ambiguity that genuinely prevents progress, or all tasks complete. "Should I continue?" prompts and progress summaries waste their time — they asked you to execute the plan, so execute it.
 
 ## When to Use
@@ -63,16 +61,11 @@ digraph process {
     }
 
     "Read plan, extract all tasks with full text, note context, create todos" [shape=box];
-    "Entire plan = one trivial mechanical change?" [shape=diamond];
-    "Implement directly, verify, commit (no review fanout)" [shape=box];
     "More tasks remain?" [shape=diamond];
     "Dispatch final code reviewer subagent for entire implementation" [shape=box];
     "Use superpowers:finishing-a-development-branch" [shape=box style=filled fillcolor=lightgreen];
 
-    "Read plan, extract all tasks with full text, note context, create todos" -> "Entire plan = one trivial mechanical change?";
-    "Entire plan = one trivial mechanical change?" -> "Implement directly, verify, commit (no review fanout)" [label="yes — see Proportionality"];
-    "Implement directly, verify, commit (no review fanout)" -> "Use superpowers:finishing-a-development-branch";
-    "Entire plan = one trivial mechanical change?" -> "Dispatch implementer subagent (./implementer-prompt.md)" [label="no"];
+    "Read plan, extract all tasks with full text, note context, create todos" -> "Dispatch implementer subagent (./implementer-prompt.md)";
     "Dispatch implementer subagent (./implementer-prompt.md)" -> "Implementer subagent asks questions?";
     "Implementer subagent asks questions?" -> "Answer questions, provide context" [label="yes"];
     "Answer questions, provide context" -> "Dispatch implementer subagent (./implementer-prompt.md)";
@@ -244,7 +237,7 @@ Done!
 
 **Never:**
 - Start implementation on main/master branch without explicit user consent
-- Skip reviews (spec compliance OR code quality) on a non-trivial task — the Proportionality exception covers only a plan that is one trivial mechanical change
+- Skip reviews (spec compliance OR code quality)
 - Proceed with unfixed issues
 - Dispatch multiple implementation subagents in parallel (conflicts)
 - Make subagent read plan file (provide full text instead)

skills/writing-plans/SKILL.md

@@ -7,12 +7,10 @@ description: Use when you have a spec or requirements for a multi-step task, bef
 
 ## Overview
 
-Write comprehensive implementation plans assuming the engineer has zero context for our codebase and questionable taste. Document everything they need to execute: which files to touch for each task, code, testing, docs they might need to check, how to test it. Give them the whole plan as bite-sized tasks. DRY. YAGNI. TDD. Frequent commits.
+Write comprehensive implementation plans assuming the engineer has zero context for our codebase and questionable taste. Document everything they need to know: which files to touch for each task, code, testing, docs they might need to check, how to test it. Give them the whole plan as bite-sized tasks. DRY. YAGNI. TDD. Frequent commits.
 
 Assume they are a skilled developer, but know almost nothing about our toolset or problem domain. Assume they don't know good test design very well.
 
-**Plans reference the spec; they never restate it.** The spec owns the WHAT and WHY — requirements, acceptance criteria, design decisions. The plan owns the HOW — tasks, files, code, commands. Cite the spec by path in the header and by section where a task needs context. Re-deriving spec content inline doubles the documents and lets them drift apart. "Zero context" means the engineer can execute each step mechanically; it does not mean the plan repeats what the spec already says — they can read the spec at the cited path.
-
 **Announce at start:** "I'm using the writing-plans skill to create the implementation plan."
 
 **Context:** If working in an isolated worktree, it should have been created via the `superpowers:using-git-worktrees` skill at execution time.
@@ -55,8 +53,6 @@ This structure informs the task decomposition. Each task should produce self-con
 
 **Goal:** [One sentence describing what this builds]
 
-**Spec:** [Path to the spec doc, e.g. `docs/superpowers/specs/YYYY-MM-DD-<topic>-design.md` — requirements and design decisions live there; do not restate them here]
-
 **Architecture:** [2-3 sentences about approach]
 
 **Tech Stack:** [Key technologies/libraries]

tests/brainstorm-server/auth.test.js

@@ -0,0 +1,201 @@
+/**
+ * Security tests for the brainstorm server's per-session key.
+ *
+ * The companion server is reachable by any local browser tab (default loopback
+ * bind) and by any host that can route to it (remote `--host 0.0.0.0` bind).
+ * A per-session secret key gates every endpoint so that neither a browser
+ * confused-deputy nor a direct remote client can read screens/files or inject
+ * events into state/events (prompt injection into a live agent session).
+ *
+ * Auth = a valid `?key=<token>` query param OR a valid session cookie.
+ *
+ * Uses the `ws` npm package as a test client (test-only dependency).
+ */
+
+const { spawn } = require('child_process');
+const http = require('http');
+const WebSocket = require('ws');
+const fs = require('fs');
+const path = require('path');
+const assert = require('assert');
+
+const SERVER_PATH = path.join(__dirname, '../../skills/brainstorming/scripts/server.cjs');
+const TEST_PORT = 3335;
+const TEST_DIR = '/tmp/brainstorm-auth-test';
+const CONTENT_DIR = path.join(TEST_DIR, 'content');
+const TOKEN = 'testtoken-0123456789abcdef0123456789abcdef';
+const COOKIE_NAME = `brainstorm-key-${TEST_PORT}`;
+
+function cleanup() {
+  if (fs.existsSync(TEST_DIR)) fs.rmSync(TEST_DIR, { recursive: true });
+}
+
+async function sleep(ms) {
+  return new Promise(resolve => setTimeout(resolve, ms));
+}
+
+// Raw HTTP GET with optional key query and Cookie header.
+function get(pathname, { key, cookie } = {}) {
+  const url = `http://localhost:${TEST_PORT}${pathname}` + (key !== undefined ? `?key=${key}` : '');
+  const headers = {};
+  if (cookie) headers['Cookie'] = cookie;
+  return new Promise((resolve, reject) => {
+    http.get(url, { headers }, (res) => {
+      let data = '';
+      res.on('data', chunk => data += chunk);
+      res.on('end', () => resolve({ status: res.statusCode, headers: res.headers, body: data }));
+    }).on('error', reject);
+  });
+}
+
+// Try to open a WebSocket; resolve 'opened' or 'rejected'.
+function wsConnect({ key, cookie } = {}) {
+  const url = `ws://localhost:${TEST_PORT}/` + (key !== undefined ? `?key=${key}` : '');
+  const opts = cookie ? { headers: { Cookie: cookie } } : {};
+  const ws = new WebSocket(url, opts);
+  return new Promise((resolve) => {
+    let settled = false;
+    const done = (outcome) => { if (!settled) { settled = true; resolve({ outcome, ws }); } };
+    ws.on('open', () => done('opened'));
+    ws.on('error', () => done('rejected'));
+    ws.on('close', () => done('rejected'));
+    setTimeout(() => done('rejected'), 1500);
+  });
+}
+
+function startServer() {
+  return spawn('node', [SERVER_PATH], {
+    env: { ...process.env, BRAINSTORM_PORT: TEST_PORT, BRAINSTORM_DIR: TEST_DIR, BRAINSTORM_TOKEN: TOKEN }
+  });
+}
+
+async function waitForServer(server) {
+  let stdout = '', stderr = '';
+  return new Promise((resolve, reject) => {
+    server.stdout.on('data', (d) => {
+      stdout += d.toString();
+      if (stdout.includes('server-started')) resolve({ stdout });
+    });
+    server.stderr.on('data', (d) => { stderr += d.toString(); });
+    server.on('error', reject);
+    setTimeout(() => reject(new Error(`Server didn't start. stderr: ${stderr}`)), 5000);
+  });
+}
+
+async function runTests() {
+  cleanup();
+  fs.mkdirSync(CONTENT_DIR, { recursive: true });
+  fs.writeFileSync(path.join(CONTENT_DIR, 'screen.html'), '<h2>Secret screen</h2>');
+  fs.writeFileSync(path.join(CONTENT_DIR, 'asset.txt'), 'secret asset');
+
+  const server = startServer();
+  let stdoutAccum = '';
+  server.stdout.on('data', (d) => { stdoutAccum += d.toString(); });
+  const { stdout: initialStdout } = await waitForServer(server);
+
+  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++; }
+  }
+
+  try {
+    console.log('\n--- Startup URL ---');
+
+    await test('server-started url includes the session key', () => {
+      const msg = JSON.parse(initialStdout.trim());
+      assert(msg.url.includes(`key=${TOKEN}`), `url should carry the key, got: ${msg.url}`);
+    });
+
+    console.log('\n--- HTTP / gate ---');
+
+    await test('GET / without key is rejected with 403', async () => {
+      const res = await get('/');
+      assert.strictEqual(res.status, 403, 'no-key request must be 403');
+    });
+
+    await test('403 page names "coding agent" and the key', async () => {
+      const res = await get('/');
+      assert(/coding agent/i.test(res.body), '403 body should reference the coding agent');
+      assert(/key/i.test(res.body), '403 body should mention the key');
+    });
+
+    await test('GET / with wrong key is rejected with 403', async () => {
+      const res = await get('/', { key: 'wrong-token' });
+      assert.strictEqual(res.status, 403);
+    });
+
+    await test('GET / with valid key serves the screen', async () => {
+      const res = await get('/', { key: TOKEN });
+      assert.strictEqual(res.status, 200);
+      assert(res.body.includes('Secret screen'), 'should serve the screen content');
+    });
+
+    await test('valid key load sets an HttpOnly SameSite=Strict cookie', async () => {
+      const res = await get('/', { key: TOKEN });
+      const setCookie = (res.headers['set-cookie'] || []).join('; ');
+      assert(setCookie.includes(`${COOKIE_NAME}=${TOKEN}`), `should set ${COOKIE_NAME}`);
+      assert(/HttpOnly/i.test(setCookie), 'cookie should be HttpOnly');
+      assert(/SameSite=Strict/i.test(setCookie), 'cookie should be SameSite=Strict');
+    });
+
+    await test('GET / with valid cookie (no query key) serves the screen', async () => {
+      const res = await get('/', { cookie: `${COOKIE_NAME}=${TOKEN}` });
+      assert.strictEqual(res.status, 200);
+      assert(res.body.includes('Secret screen'));
+    });
+
+    console.log('\n--- HTTP /files gate ---');
+
+    await test('GET /files without key is rejected with 403', async () => {
+      const res = await get('/files/asset.txt');
+      assert.strictEqual(res.status, 403);
+    });
+
+    await test('GET /files with valid key serves the file', async () => {
+      const res = await get('/files/asset.txt', { key: TOKEN });
+      assert.strictEqual(res.status, 200);
+      assert(res.body.includes('secret asset'));
+    });
+
+    console.log('\n--- WebSocket gate ---');
+
+    await test('WS upgrade without key is rejected', async () => {
+      const { outcome, ws } = await wsConnect();
+      ws.close();
+      assert.strictEqual(outcome, 'rejected', 'unauthenticated WS must not open');
+    });
+
+    await test('WS upgrade with valid key opens', async () => {
+      const { outcome, ws } = await wsConnect({ key: TOKEN });
+      ws.close();
+      assert.strictEqual(outcome, 'opened');
+    });
+
+    await test('WS upgrade with valid cookie opens', async () => {
+      const { outcome, ws } = await wsConnect({ cookie: `${COOKIE_NAME}=${TOKEN}` });
+      ws.close();
+      assert.strictEqual(outcome, 'opened');
+    });
+
+    console.log('\n--- Robustness (A3) ---');
+
+    await test('null payload over an authed WS does not crash the server', async () => {
+      const { ws } = await wsConnect({ key: TOKEN });
+      ws.send('null');
+      await sleep(300);
+      const res = await get('/', { key: TOKEN });
+      assert.strictEqual(res.status, 200, 'server must still respond after null payload');
+      ws.close();
+    });
+
+    console.log(`\n--- Results: ${passed} passed, ${failed} failed ---`);
+    if (failed > 0) process.exit(1);
+  } finally {
+    server.kill();
+    await sleep(100);
+    cleanup();
+  }
+}
+
+runTests().catch(err => { console.error('Test failed:', err); process.exit(1); });

tests/brainstorm-server/helper.test.js

@@ -0,0 +1,163 @@
+/**
+ * Tests for the injected browser client (helper.js).
+ *
+ * helper.js runs in the browser, so its DOM behaviour is exercised live; here we
+ * unit-test the pure reconnect-backoff function it exports and assert that the
+ * reconnect / status / tombstone wiring is present.
+ */
+
+const assert = require('assert');
+const fs = require('fs');
+const path = require('path');
+
+const HELPER = path.join(__dirname, '../../skills/brainstorming/scripts/helper.js');
+
+const src = fs.readFileSync(HELPER, 'utf-8');
+
+// helper.js is browser code, and the repo is an ES module package, so a plain
+// require() won't surface its exports. Evaluate the source in a CommonJS sandbox
+// with no `window`, so only the exported pure helpers run (not the browser code).
+const moduleShim = { exports: {} };
+new Function('module', src)(moduleShim);
+const { nextReconnectDelay, MIN_RECONNECT_MS, MAX_RECONNECT_MS, TOMBSTONE_AFTER_MS } = moduleShim.exports;
+
+let passed = 0, failed = 0;
+function test(name, fn) {
+  try { fn(); console.log(`  PASS: ${name}`); passed++; }
+  catch (e) { console.log(`  FAIL: ${name}`); console.log(`    ${e.message}`); failed++; }
+}
+
+console.log('\n--- Backoff (pure) ---');
+
+test('doubles the delay each call', () => {
+  assert.strictEqual(nextReconnectDelay(500, 30000), 1000);
+  assert.strictEqual(nextReconnectDelay(1000, 30000), 2000);
+  assert.strictEqual(nextReconnectDelay(2000, 30000), 4000);
+});
+
+test('caps at the maximum', () => {
+  assert.strictEqual(nextReconnectDelay(20000, 30000), 30000);
+  assert.strictEqual(nextReconnectDelay(30000, 30000), 30000);
+});
+
+test('full progression from MIN caps at MAX and never exceeds it', () => {
+  const seq = [MIN_RECONNECT_MS];
+  let d = MIN_RECONNECT_MS;
+  for (let i = 0; i < 10; i++) { d = nextReconnectDelay(d, MAX_RECONNECT_MS); seq.push(d); }
+  assert.strictEqual(seq[0], 500);
+  assert.deepStrictEqual(seq.slice(0, 7), [500, 1000, 2000, 4000, 8000, 16000, 30000]);
+  assert(seq.every(v => v <= MAX_RECONNECT_MS), 'never exceeds max');
+  assert.strictEqual(seq[seq.length - 1], 30000, 'settles at the cap');
+});
+
+test('exposes sane constants', () => {
+  assert.strictEqual(MIN_RECONNECT_MS, 500);
+  assert.strictEqual(MAX_RECONNECT_MS, 30000);
+  assert(TOMBSTONE_AFTER_MS >= 5000, 'tombstone grace is at least a few seconds');
+});
+
+console.log('\n--- Wiring (source) ---');
+
+test('reflects all three connection states', () => {
+  assert(/Connected/.test(src) && /Reconnecting/.test(src) && /Disconnected/.test(src),
+    'should set Connected / Reconnecting / Disconnected status');
+  assert(src.includes("setProperty('--status-color'"), 'drives the status dot via --status-color');
+});
+
+test('renders a tombstone overlay when paused', () => {
+  assert(src.includes('bs-tombstone'), 'creates the tombstone element');
+  assert(/Companion paused/.test(src), 'tombstone explains the companion paused');
+});
+
+test('hardens reconnection (onerror, null socket, clears pending timer)', () => {
+  assert(src.includes('onerror'), 'handles onerror');
+  assert(/ws = null/.test(src), 'nulls the socket on close so sendEvent queues');
+  assert(src.includes('clearTimeout'), 'clears a pending reconnect before scheduling another');
+  assert(src.includes('nextReconnectDelay'), 'uses exponential backoff for reconnects');
+});
+
+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);

tests/brainstorm-server/lifecycle.test.js

@@ -0,0 +1,203 @@
+/**
+ * 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_TOKEN: 'lifetoken', 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/?key=lifetoken');
+    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 });
+    }
+  });
+
+  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');
+    const tokenFile = path.join(dir, '.last-token');
+    const env = { ...process.env, BRAINSTORM_PORT_FILE: portFile, BRAINSTORM_TOKEN_FILE: tokenFile, BRAINSTORM_LIFECYCLE_CHECK_MS: 100000 };
+
+    const a = spawn('node', [SERVER], { env: { ...env, BRAINSTORM_DIR: path.join(dir, 's1') } });
+    let outA = ''; a.stdout.on('data', d => outA += d.toString());
+    for (let i = 0; i < 60 && !outA.includes('server-started'); i++) await sleep(50);
+    const infoA = firstServerStarted(outA);
+    const keyA = new URL(infoA.url).searchParams.get('key');
+    assert(fs.existsSync(portFile) && fs.existsSync(tokenFile), 'should write the port and token files');
+    a.kill(); await sleep(400); // free the port
+
+    const b = spawn('node', [SERVER], { env: { ...env, BRAINSTORM_DIR: path.join(dir, 's2') } });
+    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 infoB = firstServerStarted(outB);
+    const keyB = new URL(infoB.url).searchParams.get('key');
+    b.kill(); await sleep(100); 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.
+    assert.strictEqual(keyB, keyA, 'restart should reuse the same session key');
+  });
+
+  await test('falls back to a random port when the preferred port is taken', async () => {
+    const dir = fs.mkdtempSync('/tmp/bs-port-');
+    const portFile = path.join(dir, '.last-port');
+
+    const a = spawn('node', [SERVER], { env: { ...process.env, BRAINSTORM_DIR: path.join(dir, 'a'), BRAINSTORM_PORT: 3415, BRAINSTORM_LIFECYCLE_CHECK_MS: 100000 } });
+    let outA = ''; a.stdout.on('data', d => outA += d.toString());
+    for (let i = 0; i < 60 && !outA.includes('server-started'); i++) await sleep(50);
+
+    fs.writeFileSync(portFile, '3415'); // preferred port, but it's taken by A
+    const b = spawn('node', [SERVER], { env: { ...process.env, BRAINSTORM_DIR: path.join(dir, 'b'), BRAINSTORM_PORT_FILE: portFile, BRAINSTORM_LIFECYCLE_CHECK_MS: 100000 } });
+    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 () => {
+    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 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'), '<h2>First</h2>');
+    await sleep(700);
+    // Second screen -> must NOT open again.
+    fs.writeFileSync(path.join(dir, 'content', 'second.html'), '<h2>Second</h2>');
+    await sleep(700);
+
+    const lines = fs.existsSync(marker) ? fs.readFileSync(marker, 'utf8').trim().split('\n').filter(Boolean) : [];
+    // The opened URL must carry the key AND be reachable — a keyless URL hits 403.
+    let status = 0;
+    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);
+    fs.rmSync(dir, { recursive: true, force: true });
+
+    assert.strictEqual(lines.length, 1, 'should open exactly once');
+    assert(lines[0].includes('3417'), `should open the server URL, got: ${lines[0]}`);
+    assert(/[?&]key=/.test(lines[0]), `opened URL must carry the session key, got: ${lines[0]}`);
+    assert.strictEqual(status, 200, 'the opened URL must be reachable (valid key), not the 403 page');
+  });
+
+  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}'`;
+    // 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'), '<h2>First</h2>');
+    await sleep(700);
+    srv.kill(); await sleep(100);
+    const opened = fs.existsSync(marker);
+    fs.rmSync(dir, { recursive: true, force: true });
+    assert(!opened, 'must not open the browser without explicit approval');
+  });
+
+  await test('unauthenticated requests do not defeat the idle timeout', async () => {
+    const dir = fs.mkdtempSync('/tmp/bs-life-');
+    const srv = spawn('node', [SERVER], { env: { ...process.env, BRAINSTORM_PORT: 3419, BRAINSTORM_DIR: dir, BRAINSTORM_TOKEN: 'authtok', BRAINSTORM_IDLE_TIMEOUT_MS: 400, BRAINSTORM_LIFECYCLE_CHECK_MS: 100 } });
+    let out = ''; srv.stdout.on('data', d => out += d.toString());
+    let exited = false; srv.on('exit', () => { exited = true; });
+    for (let i = 0; i < 60 && !out.includes('server-started'); i++) await sleep(50);
+
+    // Flood with UNAUTHENTICATED (keyless → 403) requests. These must NOT count
+    // as activity, so the idle timeout still fires and the process exits.
+    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();
+    fs.rmSync(dir, { recursive: true, force: true });
+
+    assert(exited, 'idle shutdown must still fire despite a flood of unauthenticated requests');
+  });
+
+  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); });

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 helper.test.js && node auth.test.js && node server.test.js && node lifecycle.test.js && bash stop-server.test.sh"
   },
   "dependencies": {
     "ws": "^8.19.0"

tests/brainstorm-server/server.test.js

@@ -20,6 +20,9 @@ const TEST_PORT = 3334;
 const TEST_DIR = '/tmp/brainstorm-test';
 const CONTENT_DIR = path.join(TEST_DIR, 'content');
 const STATE_DIR = path.join(TEST_DIR, 'state');
+// Fixed session key so the test client can authenticate (see auth.test.js for
+// the security behavior itself; here we just need authorized requests).
+const TOKEN = 'testtoken-server-0123456789abcdef';
 
 function cleanup() {
   if (fs.existsSync(TEST_DIR)) {
@@ -32,8 +35,9 @@ async function sleep(ms) {
 }
 
 async function fetch(url) {
+  const authed = url + (url.includes('?') ? '&' : '?') + 'key=' + TOKEN;
   return new Promise((resolve, reject) => {
-    http.get(url, (res) => {
+    http.get(authed, (res) => {
       let data = '';
       res.on('data', chunk => data += chunk);
       res.on('end', () => resolve({
@@ -47,7 +51,7 @@ async function fetch(url) {
 
 function startServer() {
   return spawn('node', [SERVER_PATH], {
-    env: { ...process.env, BRAINSTORM_PORT: TEST_PORT, BRAINSTORM_DIR: TEST_DIR }
+    env: { ...process.env, BRAINSTORM_PORT: TEST_PORT, BRAINSTORM_DIR: TEST_DIR, BRAINSTORM_TOKEN: TOKEN }
   });
 }
 
@@ -179,6 +183,33 @@ async function runTests() {
       assert(!res.body.includes('"not"'), 'Should not serve JSON');
     });
 
+    await test('ignores macOS resource-fork dotfiles (._*.html) when serving', async () => {
+      // On macOS/ExFAT/SMB, the OS writes ._name.html sidecar files holding
+      // binary metadata. They end with .html but must never be served as a screen.
+      fs.writeFileSync(path.join(CONTENT_DIR, 'real-screen.html'), '<h2>Real Screen Content</h2>');
+      await sleep(100);
+      fs.writeFileSync(path.join(CONTENT_DIR, '._real-screen.html'), 'Mac OS X resource fork garbage');
+      await sleep(300);
+
+      const res = await fetch(`http://localhost:${TEST_PORT}/`);
+      assert(res.body.includes('Real Screen Content'), 'should serve the real screen, not the newer ._ sidecar');
+      assert(!res.body.includes('resource fork garbage'), 'must not serve ._*.html dotfile content');
+    });
+
+    await test('does not serve dotfiles via /files/', async () => {
+      fs.writeFileSync(path.join(CONTENT_DIR, '._secret.html'), 'dotfile body should not be served');
+      const res = await fetch(`http://localhost:${TEST_PORT}/files/._secret.html`);
+      assert.strictEqual(res.status, 404, '/files/ must 404 on dotfiles');
+    });
+
+    await test('GET /files/ (empty name) returns 404 and does not crash the server', async () => {
+      const res = await fetch(`http://localhost:${TEST_PORT}/files/`);
+      assert.strictEqual(res.status, 404, '/files/ (the content dir) must 404, not EISDIR-crash');
+      // The server must still be alive afterward.
+      const alive = await fetch(`http://localhost:${TEST_PORT}/`);
+      assert.strictEqual(alive.status, 200, 'server must survive a /files/ request');
+    });
+
     await test('returns 404 for non-root paths', async () => {
       const res = await fetch(`http://localhost:${TEST_PORT}/other`);
       assert.strictEqual(res.status, 404);
@@ -188,7 +219,7 @@ async function runTests() {
     console.log('\n--- WebSocket Communication ---');
 
     await test('accepts WebSocket upgrade on /', async () => {
-      const ws = new WebSocket(`ws://localhost:${TEST_PORT}`);
+      const ws = new WebSocket(`ws://localhost:${TEST_PORT}/?key=${TOKEN}`);
       await new Promise((resolve, reject) => {
         ws.on('open', resolve);
         ws.on('error', reject);
@@ -198,7 +229,7 @@ async function runTests() {
 
     await test('relays user events to stdout with source field', async () => {
       stdoutAccum = '';
-      const ws = new WebSocket(`ws://localhost:${TEST_PORT}`);
+      const ws = new WebSocket(`ws://localhost:${TEST_PORT}/?key=${TOKEN}`);
       await new Promise(resolve => ws.on('open', resolve));
 
       ws.send(JSON.stringify({ type: 'click', text: 'Test Button' }));
@@ -214,7 +245,7 @@ async function runTests() {
       const eventsFile = path.join(STATE_DIR, 'events');
       if (fs.existsSync(eventsFile)) fs.unlinkSync(eventsFile);
 
-      const ws = new WebSocket(`ws://localhost:${TEST_PORT}`);
+      const ws = new WebSocket(`ws://localhost:${TEST_PORT}/?key=${TOKEN}`);
       await new Promise(resolve => ws.on('open', resolve));
 
       ws.send(JSON.stringify({ type: 'click', choice: 'b', text: 'Option B' }));
@@ -232,7 +263,7 @@ async function runTests() {
       const eventsFile = path.join(STATE_DIR, 'events');
       if (fs.existsSync(eventsFile)) fs.unlinkSync(eventsFile);
 
-      const ws = new WebSocket(`ws://localhost:${TEST_PORT}`);
+      const ws = new WebSocket(`ws://localhost:${TEST_PORT}/?key=${TOKEN}`);
       await new Promise(resolve => ws.on('open', resolve));
 
       ws.send(JSON.stringify({ type: 'hover', text: 'Something' }));
@@ -244,8 +275,8 @@ async function runTests() {
     });
 
     await test('handles multiple concurrent WebSocket clients', async () => {
-      const ws1 = new WebSocket(`ws://localhost:${TEST_PORT}`);
-      const ws2 = new WebSocket(`ws://localhost:${TEST_PORT}`);
+      const ws1 = new WebSocket(`ws://localhost:${TEST_PORT}/?key=${TOKEN}`);
+      const ws2 = new WebSocket(`ws://localhost:${TEST_PORT}/?key=${TOKEN}`);
       await Promise.all([
         new Promise(resolve => ws1.on('open', resolve)),
         new Promise(resolve => ws2.on('open', resolve))
@@ -270,7 +301,7 @@ async function runTests() {
     });
 
     await test('cleans up closed clients from broadcast list', async () => {
-      const ws1 = new WebSocket(`ws://localhost:${TEST_PORT}`);
+      const ws1 = new WebSocket(`ws://localhost:${TEST_PORT}/?key=${TOKEN}`);
       await new Promise(resolve => ws1.on('open', resolve));
       ws1.close();
       await sleep(100);
@@ -282,7 +313,7 @@ async function runTests() {
     });
 
     await test('handles malformed JSON from client gracefully', async () => {
-      const ws = new WebSocket(`ws://localhost:${TEST_PORT}`);
+      const ws = new WebSocket(`ws://localhost:${TEST_PORT}/?key=${TOKEN}`);
       await new Promise(resolve => ws.on('open', resolve));
 
       // Send invalid JSON — server should not crash
@@ -299,7 +330,7 @@ async function runTests() {
     console.log('\n--- File Watching ---');
 
     await test('sends reload on new .html file', async () => {
-      const ws = new WebSocket(`ws://localhost:${TEST_PORT}`);
+      const ws = new WebSocket(`ws://localhost:${TEST_PORT}/?key=${TOKEN}`);
       await new Promise(resolve => ws.on('open', resolve));
 
       let gotReload = false;
@@ -319,7 +350,7 @@ async function runTests() {
       fs.writeFileSync(filePath, '<h2>Original</h2>');
       await sleep(500);
 
-      const ws = new WebSocket(`ws://localhost:${TEST_PORT}`);
+      const ws = new WebSocket(`ws://localhost:${TEST_PORT}/?key=${TOKEN}`);
       await new Promise(resolve => ws.on('open', resolve));
 
       let gotReload = false;
@@ -335,7 +366,7 @@ async function runTests() {
     });
 
     await test('does NOT send reload for non-.html files', async () => {
-      const ws = new WebSocket(`ws://localhost:${TEST_PORT}`);
+      const ws = new WebSocket(`ws://localhost:${TEST_PORT}/?key=${TOKEN}`);
       await new Promise(resolve => ws.on('open', resolve));
 
       let gotReload = false;
@@ -350,6 +381,22 @@ async function runTests() {
       ws.close();
     });
 
+    await test('does NOT send reload for ._*.html resource-fork dotfiles', async () => {
+      const ws = new WebSocket(`ws://localhost:${TEST_PORT}/?key=${TOKEN}`);
+      await new Promise(resolve => ws.on('open', resolve));
+
+      let gotReload = false;
+      ws.on('message', (data) => {
+        if (JSON.parse(data.toString()).type === 'reload') gotReload = true;
+      });
+
+      fs.writeFileSync(path.join(CONTENT_DIR, '._sidecar.html'), 'resource fork');
+      await sleep(500);
+
+      assert(!gotReload, 'a ._ dotfile appearing must not trigger a reload');
+      ws.close();
+    });
+
     await test('clears state/events on new screen', async () => {
       // Create an events file
       const eventsFile = path.join(STATE_DIR, 'events');

tests/brainstorm-server/stop-server.test.sh

@@ -0,0 +1,86 @@
+#!/usr/bin/env bash
+# Tests for stop-server.sh PID-ownership safety.
+#
+# A stale server.pid (e.g. after a reboot, when the kernel has recycled the PID)
+# can point at an unrelated, live process. stop-server.sh must verify the PID is
+# actually our brainstorm server before signalling it.
+
+set -u
+SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
+STOP="$SCRIPT_DIR/../../skills/brainstorming/scripts/stop-server.sh"
+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)); }
+
+# --- Test 1: an unrelated, reused PID must NOT be killed ---
+SESS="$(mktemp -d)"; mkdir -p "$SESS/state"
+sleep 600 &
+UNRELATED=$!
+echo "$UNRELATED" > "$SESS/state/server.pid"
+OUT="$("$STOP" "$SESS")"
+if kill -0 "$UNRELATED" 2>/dev/null; then
+  case "$OUT" in
+    *stale_pid*) ok "unrelated reused PID is left alone (stale_pid)" ;;
+    *) bad "unrelated PID survived but status was not stale_pid" "$OUT" ;;
+  esac
+else
+  bad "unrelated reused PID was KILLED" "$OUT"
+fi
+kill -9 "$UNRELATED" 2>/dev/null
+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=$!
+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"
+OUT="$("$STOP" "$SESS")"
+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
+else
+  case "$OUT" in
+    *stopped*) ok "real brainstorm server is stopped" ;;
+    *) bad "server stopped but status was not 'stopped'" "$OUT" ;;
+  esac
+fi
+rm -rf "$SESS"
+
+# --- Test 3: no pid file ---
+SESS="$(mktemp -d)"; mkdir -p "$SESS/state"
+OUT="$("$STOP" "$SESS")"
+case "$OUT" in
+  *not_running*) ok "missing pid file reports not_running" ;;
+  *) bad "missing pid file: unexpected status" "$OUT" ;;
+esac
+rm -rf "$SESS"
+
+# --- Test 4: a `node server.cjs` impostor NOT listening on our port is spared ---
+if command -v lsof > /dev/null 2>&1; then
+  SESS="$(mktemp -d)"; mkdir -p "$SESS/state"
+  echo '{"type":"server-started","port":3499}' > "$SESS/state/server-info" # nothing listens on 3499
+  ( exec -a "node server.cjs" sleep 600 ) &
+  IMPOSTOR=$!
+  echo "$IMPOSTOR" > "$SESS/state/server.pid"
+  OUT="$("$STOP" "$SESS")"
+  if kill -0 "$IMPOSTOR" 2>/dev/null; then
+    case "$OUT" in
+      *stale_pid*) ok "a node server.cjs not listening on our port is left alone" ;;
+      *) bad "impostor survived but status was not stale_pid" "$OUT" ;;
+    esac
+  else
+    bad "killed a node server.cjs that was NOT on our recorded port" "$OUT"
+  fi
+  kill -9 "$IMPOSTOR" 2>/dev/null
+  rm -rf "$SESS"
+else
+  echo "  SKIP: lsof unavailable — port cross-check test"
+fi
+
+echo "--- Results: $PASS passed, $FAIL failed ---"
+[ "$FAIL" -eq 0 ] || exit 1