fix: make visual companion idle timeout configurable#1596
Conversation
Reviewer noteThis PR is a narrow runtime fix for #1237. It addresses the reported 30-minute visual companion expiry by raising the default idle timeout to 2 hours and making it configurable at launch with Review surface: Verification: Known unrelated test drift: |
…#1596, obra#1562) Cherry-picks three verified fixes from obra/superpowers open PRs into the fork. - obra#1504: guard handleMessage against null/primitive WebSocket payloads. JSON.parse('null') returns null; reading .choice on it threw a TypeError that escaped the socket 'data' handler and killed the server process. - obra#1596: make the brainstorm server idle timeout configurable via --idle-timeout-minutes (BRAINSTORM_IDLE_TIMEOUT_MS) and raise the default from 30 minutes to 2 hours. Surface idle_timeout_ms in server-info. - obra#1562: detect the package manager by lockfile in using-git-worktrees Step 3 (pnpm/yarn/bun/npm for Node, uv/poetry/pip for Python) instead of running npm install / poetry install unconditionally. Verified: brainstorm-server suite 32/32 green (includes new null-crash and idle-timeout regression tests); --idle-timeout-minutes checked end-to-end (5 -> 300000ms, default -> 7200000ms, 0/abc -> error + exit 1).
|
Closing. The underlying bug (#1237) is real and the fix is directionally correct, but this PR is part of a 17-PR batch from the same author, which the contributor guidelines explicitly close regardless of merit:
The design also needs a brief discussion before any PR (we don't want to lock in — Claude Opus 4.7, Claude Code 2.1.150 |
What problem are you trying to solve?
Issue #1237 reports that the visual companion server silently exits after 30 minutes of inactivity during brainstorming. From the user's side the browser becomes unreachable, while the agent can keep referring to the stale URL as if the companion is still live. That breaks longer brainstorming sessions where a user may pause to think, talk with someone, or do parallel work.
Current
devhardcodes the timeout inskills/brainstorming/scripts/server.cjs:The start script has no launch-time override, and the visual companion guide documents the 30-minute timeout as fixed behavior.
What does this PR change?
This PR raises the default idle timeout to 2 hours and makes it configurable:
server.cjsnow reads a positiveBRAINSTORM_IDLE_TIMEOUT_MSoverride, falls back to a 2-hour default, and includesidle_timeout_msin the startup JSON /state/server-info.start-server.shaccepts--idle-timeout-minutes <minutes>, validates it as a positive integer, and passes the converted millisecond value to the server.visual-companion.mddocuments the 2-hour default and the new override flag.server.test.jscovers the new default, override parsing, invalid values, startup JSON, and start-script exposure.Is this change appropriate for the core library?
Yes. This is core visual companion runtime behavior, applies across projects and harnesses, and does not add dependencies or integrate any third-party service. It addresses a real observed Superpowers visual companion failure mode rather than project-specific configuration.
What alternatives did you consider?
server-infobefore every mention of the URL. The guide already tells agents to check server liveness before writing; strengthening behavior-shaping prose would require more adversarial eval. This PR instead fixes the runtime timeout that caused the reported 45-minute session to break.Does this PR contain multiple unrelated changes?
No. All changes support one behavior: visual companion sessions should not silently expire after only 30 minutes, and longer sessions should be configurable at launch.
Existing PRs
I searched all open and closed PRs for:
1237 visual companion timeout expires inactivity server stoppedvisual companion idle timeoutbrainstorm companion timeoutNo existing PR addressing this timeout/configurability issue was found.
Environment tested
tests/brainstorm-serverNew harness support (required if this PR adds a new harness)
Not applicable. This PR does not add or modify harness support.
Evaluation
Initial prompt/context: during issue triage, #1237 was selected as the next high-value issue because it has a concrete reported user failure and no duplicate PR. The goal was to keep the fix narrow: runtime timeout behavior, not a broad skill-flow rewrite.
Baseline before implementation:
After adding tests first,
npm testintests/brainstorm-serverfailed becausestate/server-infodid not includeidle_timeout_ms; expected7200000, actual wasundefined. This confirmed the test covered the missing runtime behavior.After implementation:
npm testResult:
29 passed, 0 failed.Additional verification:
Result:
31 passed, 0 failed.The returned startup JSON included
"idle_timeout_ms":300000; the test server was then stopped withstop-server.sh.Both returned
{"error": "--idle-timeout-minutes must be a positive integer"}and exited non-zero.Both passed.
I also ran
bash tests/brainstorm-server/windows-lifecycle.test.sh; it fails on currentdevbefore reaching this change's behavior because the test script points atskills/brainstorming/scripts/server.js, while the current server file isserver.cjs. That pre-existing test drift is handled separately in #1592, so I did not bundle it into this PR.Rigor
superpowers:writing-skillsand completed adversarial pressure testing (paste results below)This PR changes runtime code and a factual visual companion guide line. It does not alter the
brainstormingskill's hard gates, rationalization tables, or process rules. I used the RED/GREEN discipline fromsuperpowers:writing-skillsfor the skill-adjacent doc update: write failing runtime/script tests first, implement the minimum runtime behavior, then update the guide to match the new behavior. I did not run adversarial multi-session evals because this PR does not attempt to change agent decision policy.Human review
The complete diff was shown before submission. The human partner previously instructed me to treat shown diffs as reviewed for these PRs unless they say otherwise.