Skip to content

Make brainstorm companion idle timeout configurable#1689

Open
VeraPyuyi wants to merge 1 commit into
obra:devfrom
VeraPyuyi:codex/brainstorm-idle-timeout
Open

Make brainstorm companion idle timeout configurable#1689
VeraPyuyi wants to merge 1 commit into
obra:devfrom
VeraPyuyi:codex/brainstorm-idle-timeout

Conversation

@VeraPyuyi

Copy link
Copy Markdown

What problem are you trying to solve?

Fixes #1237.

The brainstorming visual companion can silently expire in the middle of a real brainstorm. The previous idle timeout was 30 minutes, which is short for interactive design/review sessions where the user may step away, discuss options, or work in parallel. When the server stops, the browser just becomes unreachable and the agent can keep referring to a stale URL.

While testing the idle path, I also found that shutdown needed to close upgraded WebSocket sockets explicitly. With a browser connection open, the server could write server-stopped and remove server-info but keep the Node process alive.

What does this PR change?

  • Raises the default visual companion idle timeout to 2 hours and exposes idle_timeout_ms in startup JSON / state/server-info.
  • Adds --idle-timeout-minutes <minutes> to skills/brainstorming/scripts/start-server.sh with explicit validation.
  • Closes active WebSocket clients during idle shutdown so the Node process actually exits.
  • Adds timer-bound validation to avoid Node timer overflow edge cases.
  • Adds regression coverage for idle shutdown with an open WebSocket, invalid timer env vars, and start-script argument parsing.

Is this change appropriate for the core library?

Yes. This changes the core brainstorming visual companion used by Superpowers itself, and addresses a general UX/reliability issue for all users of the visual companion. It does not add a new skill, harness, third-party service, or project-specific configuration.

What alternatives did you consider?

  • Only change the constant from 30 minutes to 2 hours. That would reduce the symptom but would not let users tune long workshops and would have missed the open-WebSocket shutdown bug.
  • Remove idle shutdown entirely. That would prevent expiry but risks leaving background servers running indefinitely.
  • Add only documentation telling agents to check server-info. That helps diagnosis but does not address the overly short default or the shutdown edge case.

Does this PR contain multiple unrelated changes?

No. The script, server, docs, and tests all support the same visual companion idle-timeout lifecycle fix.

Existing PRs

I did not find an open PR directly fixing the idle timeout default/configuration plus the WebSocket shutdown behavior. #1639 touches visual companion internals more broadly; #856 focuses on WebSocket reconnect/status; #1553 and #1504 are brainstorm-server robustness/security fixes in adjacent code paths.

Environment tested

Harness (e.g. Claude Code, Cursor) Harness version Model Model version/ID
Local deterministic tests on Windows + Git Bash Node v24.13.1 / Git Bash n/a n/a

New harness support (required if this PR adds a new harness)

Not applicable. This PR does not add a new harness.

Clean-session transcript for "Let's make a react todo list"
Not applicable; no new harness support.

Evaluation

  • Initial prompt: find a replacement contribution area after maintainers clarified that an internal sync tool PR was out of scope.
  • How many eval sessions did you run AFTER making the change? 0 LLM eval sessions; this is a deterministic Node/Bash server lifecycle bugfix, not behavior-shaping skill prose.
  • Deterministic validation:
    • C:\Program Files\Git\bin\bash.exe tests/brainstorm-server/run-tests.sh -> passed
      • npm test -> 27 passed, 0 failed
      • node ws-protocol.test.js -> 32 passed, 0 failed
      • bash start-server.test.sh -> 5 passed, 0 failed
    • git diff --check -> passed
    • bash -n skills/brainstorming/scripts/start-server.sh tests/brainstorm-server/start-server.test.sh tests/brainstorm-server/run-tests.sh -> passed
    • scripts/lint-shell.sh ... could not run locally because shellcheck is not installed on this machine.

Rigor

  • If this is a skills change: I used superpowers:writing-skills and completed adversarial pressure testing (paste results below)
  • This change was tested adversarially, not just on the happy path
  • I did not modify carefully-tuned content (Red Flags table, rationalizations, "human partner" language) without extensive evals showing the change is an improvement

Pre-submit review found and I fixed these issues before opening the PR:

  • Idle shutdown with an open WebSocket could leave the Node process alive.
  • The new start-script test needed a clear runnable entry point.
  • Timer values above Node's maximum delay needed explicit bounds.

Human review

  • A human has reviewed the COMPLETE proposed diff before submission

arittr pushed a commit that referenced this pull request Jun 11, 2026
…utdown

The companion shut down after only 30 minutes idle — too short for real
brainstorming, where a single question can sit far longer. And shutdown() never
closed upgraded WebSocket sockets, so an open browser connection could keep the
Node process alive after it was supposed to exit.

- Default idle timeout raised to 4 hours, configurable via BRAINSTORM_IDLE_TIMEOUT_MS
  and start-server.sh --idle-timeout-minutes (validated positive integer).
- Reported as idle_timeout_ms in the server-started JSON / server-info.
- shutdown() now destroys all client sockets so the process exits even with an
  open WebSocket.
- Watchdog check interval is configurable (BRAINSTORM_LIFECYCLE_CHECK_MS, default
  60s) so the lifecycle can be tested without minute-long waits.

Adds lifecycle.test.js (configured timeout reported; idle shutdown exits despite
an open WS — teeth-verified; the start-server flag). Wires ws-protocol,
lifecycle, and stop-server suites into npm test.

Closes #1237
Refs #1689
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant