Incorporate verified upstream fixes (#1504, #1596, #1562)

skills/brainstorming/scripts/server.cjs

@@ -81,6 +81,14 @@ 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;
 
+const DEFAULT_IDLE_TIMEOUT_MS = 2 * 60 * 60 * 1000; // 2 hours
+
+function getIdleTimeoutMs(value = process.env.BRAINSTORM_IDLE_TIMEOUT_MS) {
+  const parsed = Number(value);
+  if (!Number.isSafeInteger(parsed) || parsed <= 0) return DEFAULT_IDLE_TIMEOUT_MS;
+  return parsed;
+}
+
 const MIME_TYPES = {
   '.html': 'text/html', '.css': 'text/css', '.js': 'application/javascript',
   '.json': 'application/json', '.png': 'image/png', '.jpg': 'image/jpeg',
@@ -231,7 +239,9 @@ function handleMessage(text) {
   }
   touchActivity();
   console.log(JSON.stringify({ source: 'user-event', ...event }));
-  if (event.choice) {
+  // event may be null (JSON.parse('null')); property access on null throws
+  // and would escape the socket 'data' handler as an uncaughtException.
+  if (event && event.choice) {
     const eventsFile = path.join(STATE_DIR, 'events');
     fs.appendFileSync(eventsFile, JSON.stringify(event) + '\n');
   }
@@ -246,7 +256,7 @@ function broadcast(msg) {
 
 // ========== Activity Tracking ==========
 
-const IDLE_TIMEOUT_MS = 30 * 60 * 1000; // 30 minutes
+const IDLE_TIMEOUT_MS = getIdleTimeoutMs();
 let lastActivity = Date.now();
 
 function touchActivity() {
@@ -316,7 +326,7 @@ 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
+  // Check every 60s: exit if owner process died or the session is idle.
   const lifecycleCheck = setInterval(() => {
     if (!ownerAlive()) shutdown('owner process exited');
     else if (Date.now() - lastActivity > IDLE_TIMEOUT_MS) shutdown('idle timeout');
@@ -340,7 +350,8 @@ function startServer() {
     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
+      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');
@@ -351,4 +362,4 @@ if (require.main === module) {
   startServer();
 }
 
-module.exports = { computeAcceptKey, encodeFrame, decodeFrame, OPCODES };
+module.exports = { computeAcceptKey, encodeFrame, decodeFrame, getIdleTimeoutMs, OPCODES };

skills/brainstorming/scripts/start-server.sh

@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
 # Start the brainstorm server and output connection info
-# Usage: start-server.sh [--project-dir <path>] [--host <bind-host>] [--url-host <display-host>] [--foreground] [--background]
+# Usage: start-server.sh [--project-dir <path>] [--host <bind-host>] [--url-host <display-host>] [--idle-timeout-minutes <minutes>] [--foreground] [--background]
 #
 # Starts server on a random high port, outputs JSON with URL.
 # Each session gets its own directory to avoid conflicts.
@@ -11,6 +11,8 @@
 #   --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 <minutes>
+#                         Override inactivity timeout (default: 120).
 #   --foreground          Run server in the current terminal (no backgrounding).
 #   --background          Force background mode (overrides Codex auto-foreground).
 
@@ -22,6 +24,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 +39,10 @@ while [[ $# -gt 0 ]]; do
       URL_HOST="$2"
       shift 2
       ;;
+    --idle-timeout-minutes)
+      IDLE_TIMEOUT_MINUTES="$2"
+      shift 2
+      ;;
     --foreground|--no-daemon)
       FOREGROUND="true"
       shift
@@ -51,6 +58,15 @@ while [[ $# -gt 0 ]]; do
   esac
 done
 
+IDLE_TIMEOUT_MS=""
+if [[ -n "$IDLE_TIMEOUT_MINUTES" ]]; then
+  if ! [[ "$IDLE_TIMEOUT_MINUTES" =~ ^[0-9]+$ ]] || [[ "$IDLE_TIMEOUT_MINUTES" -eq 0 ]]; then
+    echo "{\"error\": \"--idle-timeout-minutes must be a positive integer\"}"
+    exit 1
+  fi
+  IDLE_TIMEOUT_MS=$((IDLE_TIMEOUT_MINUTES * 60 * 1000))
+fi
+
 if [[ -z "$URL_HOST" ]]; then
   if [[ "$BIND_HOST" == "127.0.0.1" || "$BIND_HOST" == "localhost" ]]; then
     URL_HOST="localhost"
@@ -107,16 +123,26 @@ if [[ -z "$OWNER_PID" || "$OWNER_PID" == "1" ]]; then
   OWNER_PID="$PPID"
 fi
 
+SERVER_ENV=(
+  BRAINSTORM_DIR="$SESSION_DIR"
+  BRAINSTORM_HOST="$BIND_HOST"
+  BRAINSTORM_URL_HOST="$URL_HOST"
+  BRAINSTORM_OWNER_PID="$OWNER_PID"
+)
+if [[ -n "$IDLE_TIMEOUT_MS" ]]; then
+  SERVER_ENV+=(BRAINSTORM_IDLE_TIMEOUT_MS="$IDLE_TIMEOUT_MS")
+fi
+
 # Foreground mode for environments that reap detached/background processes.
 if [[ "$FOREGROUND" == "true" ]]; then
   echo "$$" > "$PID_FILE"
-  env BRAINSTORM_DIR="$SESSION_DIR" BRAINSTORM_HOST="$BIND_HOST" BRAINSTORM_URL_HOST="$URL_HOST" BRAINSTORM_OWNER_PID="$OWNER_PID" node server.cjs
+  env "${SERVER_ENV[@]}" node server.cjs
   exit $?
 fi
 
 # Start server, capturing output to log file
 # Use nohup to survive shell exit; disown to remove from job table
-nohup env BRAINSTORM_DIR="$SESSION_DIR" BRAINSTORM_HOST="$BIND_HOST" BRAINSTORM_URL_HOST="$URL_HOST" BRAINSTORM_OWNER_PID="$OWNER_PID" node server.cjs > "$LOG_FILE" 2>&1 &
+nohup env "${SERVER_ENV[@]}" node server.cjs > "$LOG_FILE" 2>&1 &
 SERVER_PID=$!
 disown "$SERVER_PID" 2>/dev/null
 echo "$SERVER_PID" > "$PID_FILE"

skills/brainstorming/visual-companion.md

@@ -90,11 +90,12 @@ scripts/start-server.sh \
 ```
 
 Use `--url-host` to control what hostname is printed in the returned URL JSON.
+Use `--idle-timeout-minutes` when a session may sit idle for longer than the 2-hour default.
 
 ## 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.
+   - 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 2 hours of inactivity by default; restart with `--idle-timeout-minutes` if the session needs a longer window.
    - Use semantic filenames: `platform.html`, `visual-style.html`, `layout.html`
    - **Never reuse filenames** — each screen gets a fresh file
    - Use Write tool — **never use cat/heredoc** (dumps noise into terminal)

skills/using-git-worktrees/SKILL.md

@@ -116,15 +116,27 @@ cd "$path"
 Auto-detect and run appropriate setup:
 
 ```bash
-# Node.js
-if [ -f package.json ]; then npm install; fi
+# Node.js — detect package manager via lockfile.
+# Critical for pnpm/yarn workspaces: `npm install` ignores workspace
+# protocols, won't create `node_modules/<pkg>` symlinks for workspace
+# packages, and can leave the tree in a state where tools that resolve
+# imports through `node_modules` (LSP, type checkers) silently break.
+if   [ -f pnpm-lock.yaml ];     then pnpm install
+elif [ -f yarn.lock ];          then yarn install
+elif [ -f bun.lockb ];          then bun install
+elif [ -f package-lock.json ];  then npm install
+elif [ -f package.json ];       then npm install
+fi
 
 # Rust
 if [ -f Cargo.toml ]; then cargo build; fi
 
 # Python
-if [ -f requirements.txt ]; then pip install -r requirements.txt; fi
-if [ -f pyproject.toml ]; then poetry install; fi
+if   [ -f uv.lock ];          then uv sync
+elif [ -f poetry.lock ];      then poetry install
+elif [ -f pyproject.toml ];   then poetry install
+elif [ -f requirements.txt ]; then pip install -r requirements.txt
+fi
 
 # Go
 if [ -f go.mod ]; then go mod download; fi

tests/brainstorm-server/server.test.js

@@ -16,6 +16,7 @@ const path = require('path');
 const assert = require('assert');
 
 const SERVER_PATH = path.join(__dirname, '../../skills/brainstorming/scripts/server.cjs');
+const { getIdleTimeoutMs } = require(SERVER_PATH);
 const TEST_PORT = 3334;
 const TEST_DIR = '/tmp/brainstorm-test';
 const CONTENT_DIR = path.join(TEST_DIR, 'content');
@@ -112,6 +113,26 @@ async function runTests() {
       assert.strictEqual(info.port, TEST_PORT);
       assert.strictEqual(info.screen_dir, CONTENT_DIR, 'screen_dir should point to content/');
       assert.strictEqual(info.state_dir, STATE_DIR, 'state_dir should point to state/');
+      assert.strictEqual(info.idle_timeout_ms, 2 * 60 * 60 * 1000, 'should report default idle timeout');
+      return Promise.resolve();
+    });
+
+    await test('defaults idle timeout to 2 hours', () => {
+      assert.strictEqual(getIdleTimeoutMs(undefined), 2 * 60 * 60 * 1000);
+      assert.strictEqual(getIdleTimeoutMs(''), 2 * 60 * 60 * 1000);
+      return Promise.resolve();
+    });
+
+    await test('accepts positive BRAINSTORM_IDLE_TIMEOUT_MS values', () => {
+      assert.strictEqual(getIdleTimeoutMs('900000'), 900000);
+      assert.strictEqual(getIdleTimeoutMs('1'), 1);
+      return Promise.resolve();
+    });
+
+    await test('ignores invalid BRAINSTORM_IDLE_TIMEOUT_MS values', () => {
+      assert.strictEqual(getIdleTimeoutMs('0'), 2 * 60 * 60 * 1000);
+      assert.strictEqual(getIdleTimeoutMs('-1'), 2 * 60 * 60 * 1000);
+      assert.strictEqual(getIdleTimeoutMs('not-a-number'), 2 * 60 * 60 * 1000);
       return Promise.resolve();
     });
 
@@ -295,6 +316,53 @@ async function runTests() {
       ws.close();
     });
 
+    await test('handles JSON null from client without crashing', async () => {
+      // JSON.parse('null') yields null; reading .choice on null throws and,
+      // before the handleMessage guard, killed the server process.
+      const ws = new WebSocket(`ws://localhost:${TEST_PORT}`);
+      await new Promise(resolve => ws.on('open', resolve));
+
+      ws.send('null');
+      await sleep(300);
+
+      const res = await fetch(`http://localhost:${TEST_PORT}/`);
+      assert.strictEqual(res.status, 200, 'Server should still be running after JSON null');
+      ws.close();
+    });
+
+    await test('handles JSON primitive values from client without crashing', async () => {
+      // Numbers, strings, booleans are valid JSON top-level values. They
+      // should be ignored (no .choice, so no events file write) without
+      // throwing.
+      const ws = new WebSocket(`ws://localhost:${TEST_PORT}`);
+      await new Promise(resolve => ws.on('open', resolve));
+
+      ws.send('42');
+      ws.send('"hello"');
+      ws.send('true');
+      await sleep(300);
+
+      const res = await fetch(`http://localhost:${TEST_PORT}/`);
+      assert.strictEqual(res.status, 200, 'Server should still be running after primitive values');
+      ws.close();
+    });
+
+    await test('does NOT write events file for JSON null', async () => {
+      // Regression for the null guard: ensure null doesn't slip past the
+      // truthiness check and write a stray events file.
+      const eventsFile = path.join(STATE_DIR, 'events');
+      if (fs.existsSync(eventsFile)) fs.unlinkSync(eventsFile);
+
+      const ws = new WebSocket(`ws://localhost:${TEST_PORT}`);
+      await new Promise(resolve => ws.on('open', resolve));
+
+      ws.send('null');
+      await sleep(300);
+
+      assert(!fs.existsSync(eventsFile), 'state/events should not exist for null payload');
+      ws.close();
+    });
+
     // ========== File Watching ==========
     console.log('\n--- File Watching ---');
 
@@ -410,6 +478,19 @@ async function runTests() {
       return Promise.resolve();
     });
 
+    // ========== start-server.sh Configuration ==========
+    console.log('\n--- start-server.sh Configuration ---');
+
+    await test('start-server.sh exposes idle timeout configuration', () => {
+      const script = fs.readFileSync(
+        path.join(__dirname, '../../skills/brainstorming/scripts/start-server.sh'), 'utf-8'
+      );
+      assert(script.includes('--idle-timeout-minutes'), 'Should document idle timeout flag');
+      assert(script.includes('IDLE_TIMEOUT_MINUTES'), 'Should parse timeout minutes');
+      assert(script.includes('BRAINSTORM_IDLE_TIMEOUT_MS'), 'Should pass timeout to server process');
+      return Promise.resolve();
+    });
+
     // ========== Summary ==========
     console.log(`\n--- Results: ${passed} passed, ${failed} failed ---`);
     if (failed > 0) process.exit(1);