diff --git a/CLAUDE.md b/CLAUDE.md index 39a8918..c6c00fb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -104,6 +104,8 @@ src/ │ ├── api-key-validation.js # Validation endpoints per provider │ ├── auth-json.js # Known provider IDs that map to sidecar's PROVIDER_ENV_MAP │ ├── config.js # Default model alias map — short names to full OpenRouter model identifiers +│ ├── idle-watchdog.js # @type {Object.} Default timeouts per mode in milliseconds +│ ├── input-validators.js # MCP input validation with structured error responses. │ ├── logger.js # Structured Logger Module │ ├── mcp-discovery.js # MCP Discovery - Discovers MCP servers from parent LLM configuration │ ├── mcp-validators.js # MCP Validators @@ -111,6 +113,8 @@ src/ │ ├── model-validator.js # Alias-to-search-term mapping for filtering provider model lists │ ├── path-setup.js # Ensures that the project's node_modules/.bin directory is included in the PATH. │ ├── server-setup.js # Server Setup Utilities +│ ├── session-lock.js # Atomic session lock files to prevent concurrent resume/continue. +│ ├── shared-server.js # Manages a single shared OpenCode server for MCP sessions. │ ├── start-helpers.js # Start Command Helpers │ ├── thinking-validators.js # Thinking Level Validators │ ├── updater.js # @type {import('update-notifier').UpdateNotifier|null} @@ -158,6 +162,7 @@ scripts/ ├── check-secrets.js # Secret detection script for pre-commit hook. ├── check-ui.js ├── debug-cdp.js +├── eval-with-monitoring.sh ├── generate-docs-helpers.js # Helper functions for generate-docs.js. ├── generate-docs.js # @param {string} dirPath @returns {string[]} Sorted .md filenames ├── generate-icon.js # Generate app icon PNG from SVG source. @@ -224,6 +229,8 @@ evals/ | `utils/api-key-validation.js` | Validation endpoints per provider | `validateApiKey()`, `validateOpenRouterKey()`, `VALIDATION_ENDPOINTS()` | | `utils/auth-json.js` | Known provider IDs that map to sidecar's PROVIDER_ENV_MAP | `readAuthJsonKeys()`, `importFromAuthJson()`, `checkAuthJson()`, `removeFromAuthJson()`, `AUTH_JSON_PATH()` | | `utils/config.js` | Default model alias map — short names to full OpenRouter model identifiers | `getConfigDir()`, `getConfigPath()`, `loadConfig()`, `saveConfig()`, `getDefaultAliases()` | +| `utils/idle-watchdog.js` | @type {Object.} Default timeouts per mode in milliseconds | `IdleWatchdog()`, `resolveTimeout()` | +| `utils/input-validators.js` | MCP input validation with structured error responses. | `validateStartInputs()`, `findSimilar()` | | `utils/logger.js` | Structured Logger Module | `logger()`, `LOG_LEVELS()` | | `utils/mcp-discovery.js` | MCP Discovery - Discovers MCP servers from parent LLM configuration | `discoverParentMcps()`, `discoverClaudeCodeMcps()`, `discoverCoworkMcps()`, `normalizeMcpJson()` | | `utils/mcp-validators.js` | MCP Validators | `validateMcpSpec()`, `validateMcpConfigFile()` | @@ -231,6 +238,8 @@ evals/ | `utils/model-validator.js` | Alias-to-search-term mapping for filtering provider model lists | `validateDirectModel()`, `filterRelevantModels()`, `normalizeModelId()` | | `utils/path-setup.js` | Ensures that the project's node_modules/.bin directory is included in the PATH. | `ensureNodeModulesBinInPath()` | | `utils/server-setup.js` | Server Setup Utilities | `DEFAULT_PORT()`, `isPortInUse()`, `getPortPid()`, `killPortProcess()`, `ensurePortAvailable()` | +| `utils/session-lock.js` | Atomic session lock files to prevent concurrent resume/continue. | `acquireLock()`, `releaseLock()`, `isLockStale()`, `isPidAlive()` | +| `utils/shared-server.js` | Manages a single shared OpenCode server for MCP sessions. | `SharedServerManager()` | | `utils/start-helpers.js` | Start Command Helpers | `resolveModelFromArgs()`, `validateFallbackModel()` | | `utils/thinking-validators.js` | Thinking Level Validators | `MODEL_THINKING_SUPPORT()`, `getSupportedThinkingLevels()`, `validateThinkingLevel()` | | `utils/updater.js` | @type {import('update-notifier').UpdateNotifier|null} | `initUpdateCheck()`, `getUpdateInfo()`, `notifyUpdate()`, `performUpdate()` | @@ -350,6 +359,30 @@ The pre-commit hook runs this automatically. See [docs/doc-system.md](docs/doc-s --- +## Process Lifecycle Management + +Sidecar processes self-terminate after inactivity via IdleWatchdog. MCP sessions use a shared multiplexed server instead of per-session processes. + +### Environment Variables + +| Env Var | Default | Description | +|---------|---------|-------------| +| `SIDECAR_IDLE_TIMEOUT` | (mode-dependent) | Blanket override for idle timeout in minutes (all modes). 0 = disabled (Infinity). | +| `SIDECAR_IDLE_TIMEOUT_HEADLESS` | 15 | Headless mode idle timeout in minutes | +| `SIDECAR_IDLE_TIMEOUT_INTERACTIVE` | 60 | Interactive mode idle timeout in minutes | +| `SIDECAR_IDLE_TIMEOUT_SERVER` | 30 | Shared server "no sessions" timeout in minutes | +| `SIDECAR_MAX_SESSIONS` | 20 | Max concurrent sessions on shared server | +| `SIDECAR_REQUEST_TIMEOUT` | 5 | Stuck-stream timeout in minutes | +| `SIDECAR_SHARED_SERVER` | 1 | Set to 0 to disable shared server (fall back to per-process) | + +### Gotchas + +- `SIDECAR_IDLE_TIMEOUT=0` means `Infinity` (timer never set), not zero-ms timeout +- Session lock files live at `/session.lock`. Delete manually if stuck with "session already active" error +- `SIDECAR_SHARED_SERVER=0` disables shared server and falls back to per-process spawning + +--- + ## Agent Documentation GEMINI.md and AGENTS.md are symlinks to CLAUDE.md -- no sync needed. diff --git a/docs/architecture.md b/docs/architecture.md index 239814a..a85d9c2 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -38,6 +38,32 @@ When the user clicks **Fold** (or presses `Cmd+Shift+F`) in interactive mode: In headless mode, the agent outputs `[SIDECAR_FOLD]` autonomously when done, and `headless.js` extracts everything before the marker. +## Shared Server Architecture + +Multiple sidecar invocations share a single OpenCode Go binary when `SIDECAR_SHARED_SERVER=1` (the default). This eliminates per-invocation cold-start latency and reduces memory overhead. + +``` +Before (per-process): After (shared server): +MCP Server MCP Server + +-- sidecar CLI (port 4096) +-- Shared OpenCode Server (port 4096) + | +-- OpenCode Go binary +-- Session A + +-- sidecar CLI (port 4097) +-- Session B + | +-- OpenCode Go binary +-- Session C + +-- sidecar CLI (port 4098) + +-- OpenCode Go binary +``` + +The shared server restarts automatically on crash, up to 3 times within any 5-minute window. After 3 restarts the server is considered unstable and will not restart again; use `SIDECAR_SHARED_SERVER=0` to fall back to per-process mode. + +## IdleWatchdog State Machine + +Each sidecar process runs an `IdleWatchdog` that transitions between two states: + +- **BUSY**: A prompt is in flight or a session was recently active. Idle timer is paused. +- **IDLE**: No active requests for the configured idle period. Process (or shared server) self-terminates. + +Transitions: `BUSY → IDLE` when the last active session goes quiet; `IDLE → BUSY` on any new incoming request. The idle clock resets on each BUSY→IDLE transition. Set `SIDECAR_IDLE_TIMEOUT=0` to disable self-termination entirely. + ## Electron BrowserView Architecture The Electron shell (`electron/main.js`) uses a **BrowserView** to avoid CSS conflicts between the OpenCode SPA and the sidecar toolbar: diff --git a/docs/configuration.md b/docs/configuration.md index ad44d0e..f3df69e 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -25,6 +25,29 @@ SIDECAR_MOCK_UPDATE=available # Mock update UI state for testing --- +## Process Lifecycle + +These environment variables control how sidecar processes self-terminate and share resources. + +```bash +# Idle timeout overrides (values in minutes, 0 = disabled) +SIDECAR_IDLE_TIMEOUT=0 # Blanket override for all modes (0 = disabled) +SIDECAR_IDLE_TIMEOUT_HEADLESS=15 # Headless mode idle timeout (default: 15 min) +SIDECAR_IDLE_TIMEOUT_INTERACTIVE=60 # Interactive mode idle timeout (default: 60 min) +SIDECAR_IDLE_TIMEOUT_SERVER=30 # Shared server idle timeout (default: 30 min) + +# Resource limits +SIDECAR_MAX_SESSIONS=20 # Max concurrent sessions on shared server (default: 20) +SIDECAR_REQUEST_TIMEOUT=5 # Per-request timeout in minutes (default: 5 min) + +# Shared server +SIDECAR_SHARED_SERVER=1 # Use shared OpenCode server (default: 1, set 0 to disable) +``` + +Sidecar processes self-terminate after the configured idle period. The shared server (`SIDECAR_SHARED_SERVER=1`) allows multiple sidecar sessions to reuse a single OpenCode Go binary process rather than spawning one per invocation. + +--- + ## Dependencies | Package | Version | Purpose | diff --git a/docs/superpowers/plans/2026-03-14-mcp-polling-handoff.md b/docs/superpowers/plans/2026-03-14-mcp-polling-handoff.md new file mode 100644 index 0000000..d449868 --- /dev/null +++ b/docs/superpowers/plans/2026-03-14-mcp-polling-handoff.md @@ -0,0 +1,41 @@ +# MCP Shared Server Polling Integration - Handoff + +**Date:** 2026-03-14 +**Branch:** feature/memory-leak +**Worktree:** /Users/john_renaldi/claude-code-projects/sidecar-memory-leak + +## Problem + +The shared server MCP path (`sidecar_start` handler in `src/mcp-server.js`) creates sessions on the shared OpenCode server and sends prompts via `sendPromptAsync`, but lacks the polling/finalization loop that the per-process spawn path provides. + +**Result:** Sessions start, the LLM processes them, but `sidecar_status` never sees completion because: +1. No background polling writes conversation data to disk +2. No progress tracking (messages, stage, latest activity) +3. No session finalization (status never transitions from 'running' to 'complete') +4. No summary extraction or fold detection + +The per-process spawn path works by spawning a CLI process that runs `runHeadless()`, which handles ALL of this internally. The shared server path bypasses `runHeadless()` entirely. + +## What's Done + +All 3 layers of process lifecycle management are implemented: +- Layer 1 (Shared Server): `SharedServerManager` class works, lazy start, session tracking, supervisor +- Layer 2 (IdleWatchdog): Fully integrated into headless, interactive, opencode-client +- Layer 3 (Resume/Locks): Session locks, dead-process detection, crash handler cleanup + +The gap is specifically in the MCP `sidecar_start` handler's shared server path. + +## Options to Explore + +### Option A: Background Polling Task per Session +Run a background polling loop (like `runHeadless()` does) for each session on the shared server. Would live in the MCP handler or a new module. + +### Option B: Delegate to runHeadless() with Shared Server +Modify `runHeadless()` to accept an existing client/server instead of starting its own. The MCP handler would call `runHeadless()` but pass the shared server's client. + +### Option C: SDK-Level Event Streaming +If the OpenCode SDK supports event streaming or completion callbacks, use those instead of polling. + +## Key Constraint + +The MCP handler must return immediately after `sidecar_start` (fire-and-forget). The polling must happen in the background, writing results to disk so `sidecar_status` can read them. diff --git a/docs/superpowers/plans/2026-03-14-mcp-polling-plan.md b/docs/superpowers/plans/2026-03-14-mcp-polling-plan.md new file mode 100644 index 0000000..5701912 --- /dev/null +++ b/docs/superpowers/plans/2026-03-14-mcp-polling-plan.md @@ -0,0 +1,404 @@ +# MCP Shared Server Polling Integration - Implementation Plan + +> **For agentic workers:** REQUIRED: Use superpowers:subagent-driven-development (if subagents available) or superpowers:executing-plans to implement this plan. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Wire the MCP shared server path to use `runHeadless()` for full session lifecycle (polling, progress, finalization) instead of raw `sendPromptAsync`. + +**Architecture:** Add guard points in `runHeadless()` for external client/server/watchdog/sessionId. Rewrite MCP handler's shared server path to fire-and-forget `runHeadless()` with context building and `finalizeSession()` on completion. Only applies to headless MCP sessions. + +**Tech Stack:** Node.js (CJS), OpenCode SDK, Jest + +**Spec:** `docs/superpowers/specs/2026-03-14-mcp-polling-integration-design.md` + +--- + +## File Structure + +### Modified Files + +| File | Change | +|------|--------| +| `src/headless.js` | Add 4 guard points: externalServer skip for startup/shutdown, watchdog passthrough, sessionId passthrough | +| `src/mcp-server.js` | Replace shared server `sendPromptAsync` with fire-and-forget `runHeadless()`, add context building and `finalizeSession()` | + +No new files. + +--- + +## Chunk 1: All Tasks + +### Task 1: Add external server guard points to `runHeadless()` + +**Files:** +- Modify: `src/headless.js` +- Create: `tests/headless-external-server.test.js` + +- [ ] **Step 1: Write failing source-check tests** + +```javascript +// tests/headless-external-server.test.js +'use strict'; + +const fs = require('fs'); +const path = require('path'); + +describe('headless external server support', () => { + const src = fs.readFileSync( + path.join(__dirname, '../src/headless.js'), 'utf-8' + ); + + test('checks for externalServer flag', () => { + expect(src).toContain('externalServer'); + }); + + test('skips server.close() when externalServer is true', () => { + expect(src).toContain('!externalServer'); + }); + + test('accepts options.sessionId', () => { + expect(src).toContain('options.sessionId'); + }); + + test('accepts options.watchdog', () => { + expect(src).toContain('options.watchdog'); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `npm test tests/headless-external-server.test.js` +Expected: FAIL (src doesn't contain externalServer yet) + +- [ ] **Step 3: Add guard point 1 - Server startup** + +In `src/headless.js`, read the file first. At line ~104, replace the server startup block: + +```javascript +// Before the existing: let client, server; +const externalServer = !!(options.client && options.server); +let client, server; + +if (externalServer) { + client = options.client; + server = options.server; + logger.debug('Using external server (shared server mode)', { url: server.url }); +} else { + try { + const serverOptions = { port }; + if (options.mcp) { serverOptions.mcp = options.mcp; } + const result = await startServer(serverOptions); + client = result.client; + server = result.server; + logger.debug('Server started', { url: server.url }); + } catch (error) { + // ... existing error handling unchanged ... + } +} +``` + +- [ ] **Step 4: Skip waitForServer when external** + +After the server startup guard, wrap the `waitForServer()` block (lines ~132-147): + +```javascript +if (!externalServer) { + logger.debug('Waiting for OpenCode server to be ready'); + const serverReady = await waitForServer(client, checkHealth); + logger.debug('Server ready', { serverReady }); + writeProgress(sessionDir, 'server_ready'); + + if (!serverReady) { + server.close(); + return { + summary: '', completed: false, timedOut: false, taskId, + error: 'OpenCode server failed to start' + }; + } +} else { + writeProgress(sessionDir, 'server_ready'); +} +``` + +- [ ] **Step 5: Add guard point 3 - Watchdog passthrough** + +Replace the watchdog creation (lines ~149-157): + +```javascript +if (options.watchdog) { + watchdog = options.watchdog; +} else { + watchdog = new IdleWatchdog({ + mode: 'headless', + onTimeout: () => { + logger.info('Headless idle timeout - shutting down', { taskId }); + server.close(); + process.exit(0); + }, + }).start(); +} +``` + +- [ ] **Step 6: Add guard point 4 - Session ID passthrough** + +Find the `createSession(client)` call (line ~161-172). Replace the entire try/catch: + +```javascript +if (options.sessionId) { + sessionId = options.sessionId; + logger.debug('Using existing session', { sessionId }); +} else { + try { + sessionId = await createSession(client); + } catch (error) { + if (!externalServer) { server.close(); } + return { + summary: '', completed: false, timedOut: false, taskId, + error: error.message + }; + } +} +``` + +This guards the `server.close()` at line 164 (createSession catch). While unreachable when `options.sessionId` is set, it's defensive against future changes. + +- [ ] **Step 7: Add guard point 2 - Server shutdown guards** + +Find ALL `server.close()` calls and wrap with `if (!externalServer)`: + +Location 1 - Normal exit (~line 467): +```javascript +watchdog.cancel(); +if (!externalServer) { server.close(); } +``` + +Location 2 - Catch block (~line 520): +```javascript +if (watchdog) { watchdog.cancel(); } +if (!externalServer) { server.close(); } +``` + +All 5 `server.close()` locations are now guarded: +1. Health check failure - handled in Step 4 (inside `!externalServer` block) +2. Watchdog `onTimeout` - handled in Step 5 (only standalone watchdog calls it) +3. createSession catch - handled in Step 6 (guarded with `!externalServer`) +4. Normal exit - handled here (line ~467) +5. Catch block - handled here (line ~520) + +Note: The `mcp` option is passed as `undefined` to `runHeadless()` in the MCP handler because the shared server is already configured with MCP servers at startup via `ensureServer()`. The per-process spawn path passes MCP config because each process starts its own server. + +- [ ] **Step 8: Run tests to verify pass** + +Run: `npm test tests/headless-external-server.test.js` +Expected: PASS (4 tests) + +- [ ] **Step 9: Run full headless test suite** + +Run: `npm test tests/headless.test.js` +Expected: PASS (all existing tests unaffected - they don't pass options.client) + +- [ ] **Step 10: Commit** + +```bash +git add src/headless.js tests/headless-external-server.test.js +git commit -m "feat: add external server guard points to runHeadless for shared server support" +``` + +--- + +### Task 2: Rewrite MCP shared server path to use `runHeadless()` + +**Files:** +- Modify: `src/mcp-server.js` (lines ~104-164) + +- [ ] **Step 1: Write failing source-check test** + +Add to `tests/mcp-shared-server.test.js`: + +```javascript +describe('MCP shared server uses runHeadless', () => { + test('imports runHeadless', () => { + const src = fs.readFileSync( + path.join(__dirname, '../src/mcp-server.js'), 'utf-8' + ); + expect(src).toContain('runHeadless'); + expect(src).toContain('buildContext'); + expect(src).toContain('finalizeSession'); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `npm test tests/mcp-shared-server.test.js` +Expected: FAIL (mcp-server.js doesn't contain runHeadless yet) + +- [ ] **Step 3: Rewrite the shared server path** + +In `src/mcp-server.js`, read the file. Replace the entire `if (sharedServer.enabled)` block (lines ~104-164) with: + +```javascript +if (sharedServer.enabled && input.noUi) { + // Shared server path: headless only, delegates to runHeadless() + try { + const { server, client } = await sharedServer.ensureServer(); + const { createSession } = require('./opencode-client'); + const { buildContext } = require('./sidecar/context-builder'); + const { buildPrompts } = require('./prompt-builder'); + const { runHeadless } = require('./headless'); + const { finalizeSession } = require('./sidecar/session-utils'); + + const sessionId = await createSession(client); + + // Write initial metadata (MCP handler owns this, runHeadless skips it) + fs.mkdirSync(sessionDir, { recursive: true, mode: 0o700 }); + const metaPath = path.join(sessionDir, 'metadata.json'); + const serverPort = server.url ? new URL(server.url).port : null; + fs.writeFileSync(metaPath, JSON.stringify({ + taskId, status: 'running', pid: process.pid, + opencodeSessionId: sessionId, + opencodePort: serverPort, + goPid: server.goPid || null, + createdAt: new Date().toISOString(), + headless: true, model: input.model, + }, null, 2), { mode: 0o600 }); + + // Build context from parent conversation (unless --no-context) + let context = null; + if (input.includeContext !== false) { + try { + context = buildContext(cwd, input.parentSession, { + contextTurns: input.contextTurns, + contextSince: input.contextSince, + contextMaxTokens: input.contextMaxTokens, + coworkProcess: input.coworkProcess, + }); + } catch (ctxErr) { + logger.warn('Failed to build context, proceeding without', { error: ctxErr.message }); + } + } + + // Build prompts (same as CLI path in start.js) + const { system: systemPrompt, userMessage } = buildPrompts( + input.prompt, context, cwd, true, agent, input.summaryLength + ); + + // Register session with idle eviction + sharedServer.addSession(sessionId, (_evictedId) => { + try { + const meta = JSON.parse(fs.readFileSync(metaPath, 'utf-8')); + meta.status = 'idle-timeout'; + meta.completedAt = new Date().toISOString(); + fs.writeFileSync(metaPath, JSON.stringify(meta, null, 2), { mode: 0o600 }); + } catch (err) { + logger.warn('Failed to update evicted session metadata', { error: err.message }); + } + }); + const watchdog = sharedServer.getSessionWatchdog(sessionId); + + const timeoutMs = (input.timeout || 15) * 60 * 1000; + + // Fire-and-forget: runHeadless with shared server's client + runHeadless(input.model, systemPrompt, userMessage, taskId, cwd, + timeoutMs, agent, { + client, server, watchdog, sessionId, + mcp: undefined, // shared server already has MCP config + } + ).then((result) => { + // Session complete - finalize and remove from tracking + try { + const meta = JSON.parse(fs.readFileSync(metaPath, 'utf-8')); + finalizeSession(sessionDir, result.summary || '', cwd, meta); + } catch (finErr) { + logger.warn('Failed to finalize session', { error: finErr.message }); + } + sharedServer.removeSession(sessionId); + }).catch((err) => { + logger.error('Shared server session failed', { taskId, error: err.message }); + sharedServer.removeSession(sessionId); + try { + const meta = JSON.parse(fs.readFileSync(metaPath, 'utf-8')); + meta.status = 'error'; + meta.reason = err.message; + meta.completedAt = new Date().toISOString(); + fs.writeFileSync(metaPath, JSON.stringify(meta, null, 2), { mode: 0o600 }); + } catch (writeErr) { + logger.warn('Failed to write error metadata', { error: writeErr.message }); + } + }); + + // Return immediately + const body = JSON.stringify({ + taskId, status: 'running', mode: 'headless', + message: 'Sidecar started in headless mode. Use sidecar_status to check progress.', + }); + return { content: [{ type: 'text', text: body }, { type: 'text', text: HEADLESS_START_REMINDER }] }; + } catch (err) { + logger.warn('Shared server path failed, falling back to spawn', { error: err.message }); + // Fall through to spawn path below + } +} +``` + +IMPORTANT: Keep the existing per-process spawn path (the `else` / fallthrough after this block) unchanged. + +- [ ] **Step 4: Run tests to verify pass** + +Run: `npm test tests/mcp-shared-server.test.js` +Expected: PASS + +- [ ] **Step 5: Run existing MCP tests** + +Run: `npm test tests/mcp-server.test.js` +Expected: PASS (existing tests use the per-process spawn path) + +- [ ] **Step 6: Commit** + +```bash +git add src/mcp-server.js tests/mcp-shared-server.test.js +git commit -m "feat: MCP shared server path delegates to runHeadless with context and finalization" +``` + +--- + +### Task 3: Run full test suite and verify + +- [ ] **Step 1: Run full unit test suite** + +Run: `npm test` +Expected: All suites pass + +- [ ] **Step 2: Run linter** + +Run: `npm run lint` +Expected: No new errors (pre-existing cli-handlers.js errors are OK) + +- [ ] **Step 3: Commit any cleanup** + +```bash +git add src/headless.js src/mcp-server.js +git commit -m "chore: cleanup after MCP polling integration" +``` + +--- + +### Task 4: Run E2E integration test + +- [ ] **Step 1: Run the shared server e2e test** + +Run: `npm run test:integration -- --testPathPattern="shared-server-e2e"` +Expected: Sessions reach `complete` status, `sidecar_read` returns output, memory bounded + +- [ ] **Step 2: If e2e fails, debug and fix** + +Check stderr output for error details. Common issues: +- `buildContext` may throw if no parent session exists (should be caught by try/catch) +- `finalizeSession` may need the metadata object in a specific format +- `buildPrompts` signature may differ from start.js (check parameter order) + +- [ ] **Step 3: Commit any e2e fixes** + +```bash +git add src/headless.js src/mcp-server.js +git commit -m "fix: resolve e2e test issues in shared server polling integration" +``` diff --git a/docs/superpowers/specs/2026-03-14-mcp-input-validation-design.md b/docs/superpowers/specs/2026-03-14-mcp-input-validation-design.md new file mode 100644 index 0000000..db8b598 --- /dev/null +++ b/docs/superpowers/specs/2026-03-14-mcp-input-validation-design.md @@ -0,0 +1,186 @@ +# MCP Input Validation - Design Spec + +**Date:** 2026-03-14 +**Status:** Draft +**Branch:** feature/memory-leak + +## Problem Statement + +MCP tool handlers accept raw inputs (model aliases, prompts, timeouts, agent modes) and pass them downstream without semantic validation. When an unresolvable model alias like `'gemini'` is used, the prompt is silently sent to a non-existent model, causing sessions to hang at `messages: 0` indefinitely with no error feedback to the calling LLM. + +## Design + +Add a `validateStartInputs()` function to `src/utils/validators.js` that composes existing validators and adds model resolution. Call it at the top of the MCP `sidecar_start` handler before branching into shared server vs per-process paths. On failure, return a structured JSON error response with `isError: true` so the calling LLM can self-correct. + +### Validation Function + +Add to `src/utils/validators.js`. Composes existing validators where available: + +```javascript +/** + * Validate sidecar_start inputs before session creation. + * @param {Object} input - Raw MCP tool input + * @returns {{ valid: true, resolvedModel: string } | { valid: false, error: Object }} + */ +function validateStartInputs(input) { + // 1. Prompt: reuse existing validatePromptContent() + const promptResult = validatePromptContent(input.prompt); + if (!promptResult.valid) { + return { + valid: false, + error: { + type: 'validation_error', + field: 'prompt', + message: promptResult.error, + }, + }; + } + + // 2. Model: resolve alias to full provider/model string + // If model is undefined/null, tryResolveModel uses the configured default + const { tryResolveModel, getEffectiveAliases } = require('./config'); + const { model: resolved, error: modelError } = tryResolveModel(input.model); + if (modelError) { + const aliases = Object.keys(getEffectiveAliases()); + const suggestions = findSimilar(input.model, aliases); + return { + valid: false, + error: { + type: 'validation_error', + field: 'model', + message: `Model '${input.model}' not found. ${modelError}`, + suggestions, + available: aliases, + }, + }; + } + + // 3. Timeout: positive number, max 60 minutes + if (input.timeout !== undefined) { + const t = Number(input.timeout); + if (isNaN(t) || t <= 0) { + return { + valid: false, + error: { + type: 'validation_error', + field: 'timeout', + message: `Timeout must be a positive number (minutes). Got: ${input.timeout}`, + }, + }; + } + if (t > 60) { + return { + valid: false, + error: { + type: 'validation_error', + field: 'timeout', + message: `Timeout cannot exceed 60 minutes. Got: ${t}`, + }, + }; + } + } + + // 4. Agent + headless compatibility: reuse existing validateHeadlessAgent() + if (input.noUi) { + const agentResult = validateHeadlessAgent(input.agent); + if (!agentResult.valid) { + return { + valid: false, + error: { + type: 'validation_error', + field: 'agent', + message: agentResult.error, + suggestions: ['Build', 'Plan'], + }, + }; + } + } + + return { valid: true, resolvedModel: resolved }; +} +``` + +Note: `validatePromptContent` and `validateHeadlessAgent` already exist in `validators.js`. We compose them rather than reimplementing. + +### Fuzzy Matching + +Simple prefix matching for model alias suggestions. No Levenshtein needed for v1: + +```javascript +/** + * Find candidates that start with the input or vice versa. + * @param {string} input + * @param {string[]} candidates + * @returns {string[]} Up to 3 matching candidates + */ +function findSimilar(input, candidates) { + if (!input) { return []; } + const lower = input.toLowerCase(); + return candidates.filter(c => { + const cl = c.toLowerCase(); + return cl.startsWith(lower) || lower.startsWith(cl); + }).slice(0, 3); +} +``` + +### MCP Integration + +In `src/mcp-server.js`, at the top of the `sidecar_start` handler, before any `if (sharedServer.enabled)` check: + +```javascript +// Validate inputs before any session creation +const { validateStartInputs } = require('./utils/validators'); +const validation = validateStartInputs(input); +if (!validation.valid) { + return { + isError: true, + content: [{ type: 'text', text: JSON.stringify(validation.error) }], + }; +} +const resolvedModel = validation.resolvedModel; +``` + +Use `resolvedModel` instead of `input.model` in both: +- Shared server path: passed to `runHeadless()` and written to metadata +- Per-process spawn path: replace `input.model` in CLI args with `resolvedModel` + +This also removes the need for the separate `tryResolveModel` call currently in the shared server path (line ~115), since validation already resolved the model. + +### Error Response Format + +The `validation.error` object is serialized as the MCP tool result text: + +```json +{ + "type": "validation_error", + "field": "model", + "message": "Model 'gemni' not found.", + "suggestions": ["gemini"], + "available": ["gemini", "gpt", "opus", "deepseek"] +} +``` + +The MCP response wraps it with `isError: true`: +```javascript +{ isError: true, content: [{ type: 'text', text: '{"type":"validation_error",...}' }] } +``` + +### Files to Modify + +| File | Change | +|------|--------| +| `src/utils/validators.js` | Add `validateStartInputs()`, `findSimilar()`. Compose existing `validatePromptContent()` and `validateHeadlessAgent()`. | +| `src/mcp-server.js` | Call `validateStartInputs()` at top of `sidecar_start` handler. Use `resolvedModel` downstream. Remove separate `tryResolveModel` call from shared server path. | +| `tests/validators.test.js` | Add tests for `validateStartInputs()` | + +### Success Criteria + +1. `sidecar_start` with `model: 'gemni'` (typo) returns immediate error with suggestion `'gemini'` +2. `sidecar_start` with empty prompt returns immediate error +3. `sidecar_start` with `agent: 'chat', noUi: true` returns immediate error +4. `sidecar_start` with `timeout: -5` returns immediate error +5. `sidecar_start` with `timeout: 999` returns immediate error (exceeds 60 min) +6. `sidecar_start` with `model: 'gemini'` (valid alias) resolves and works +7. `sidecar_start` with `model: undefined` uses configured default (no error) +8. Existing eval 1 continues to pass +9. All unit tests pass diff --git a/docs/superpowers/specs/2026-03-14-mcp-polling-integration-design.md b/docs/superpowers/specs/2026-03-14-mcp-polling-integration-design.md new file mode 100644 index 0000000..1c362e7 --- /dev/null +++ b/docs/superpowers/specs/2026-03-14-mcp-polling-integration-design.md @@ -0,0 +1,239 @@ +# MCP Shared Server Polling Integration - Design Spec + +**Date:** 2026-03-14 +**Status:** Draft +**Branch:** feature/memory-leak +**Depends on:** Process Lifecycle spec (same branch) + +## Problem Statement + +The MCP `sidecar_start` handler's shared server path creates sessions on the shared OpenCode server and sends prompts via `sendPromptAsync`, but lacks the polling/finalization loop that the per-process spawn path provides. Sessions start but never reach `complete` status because nobody polls for results, writes conversation data, or finalizes metadata. + +## Design + +Reuse `runHeadless()` from the shared server MCP path. `runHeadless()` already handles 20+ edge cases: fold detection, stable polls, timeout, abort, conversation JSONL writing, progress tracking, and session finalization. Rather than reimplementing this logic, we modify `runHeadless()` to accept an existing client/server and call it as a background async task from the MCP handler. + +### Scope + +- **Shared server path:** headless MCP sessions only (`noUi: true`) +- **Per-process spawn path:** interactive sessions (already clean via Electron close handler + IdleWatchdog) and headless when shared server disabled +- Interactive mode does not leak because `server.close()` fires on Electron close, and the 60-min IdleWatchdog provides a safety net + +### Changes to `runHeadless()` (`src/headless.js`) + +Three guard points where we check for an externally-provided client/server: + +**1. Server startup (lines ~106-125):** +If `options.client` and `options.server` are provided, skip `startServer()` and `waitForServer()`. The shared server is already running and healthy. + +```javascript +const externalServer = !!(options.client && options.server); +let client, server; + +if (externalServer) { + client = options.client; + server = options.server; + // Skip waitForServer() health check - shared server is already healthy +} else { + // existing startServer() + waitForServer() logic unchanged +} +``` + +**2. Server shutdown (3 locations):** +If using an external server, skip ALL `server.close()` calls. The `SharedServerManager` manages the shared server's lifecycle. + +Locations to guard (all `server.close()` calls in `runHeadless()`): +- Health check failure (~line 139): `if (!externalServer) { server.close(); }` +- Watchdog `onTimeout` callback (~line 154): must NOT call `server.close()` or `process.exit(0)` when running on shared server (see guard point 3) +- Normal exit path (~line 467): `if (!externalServer) { server.close(); }` +- Catch block (~line 520): `if (!externalServer) { server.close(); }` + +**3. Watchdog (lines ~147-156):** +If `options.watchdog` is provided, use it instead of creating a new one. The MCP handler already created a per-session watchdog via `SharedServerManager.addSession()`. + +**Critical:** The default watchdog's `onTimeout` calls `server.close()` and `process.exit(0)`. When running inside the MCP server process (shared server path), `process.exit(0)` would kill the entire MCP server. For the external server path, the watchdog must NOT call `process.exit()`. Instead, it should finalize the session metadata and let the `SharedServerManager` handle session eviction. + +```javascript +let watchdog; +if (options.watchdog) { + watchdog = options.watchdog; + // External watchdog's onTimeout is managed by SharedServerManager.addSession() + // It calls removeSession() which cancels the watchdog - no process.exit() needed +} else { + watchdog = new IdleWatchdog({ + mode: 'headless', + onTimeout: () => { + server.close(); // OK for standalone mode + process.exit(0); // OK for standalone mode + }, + }).start(); +} +``` + +**4. Session ID (new guard point):** +If `options.sessionId` is provided, skip `createSession(client)` and use the existing session. The MCP handler creates the session before calling `runHeadless()` to avoid orphaned sessions. + +```javascript +let sessionId; +if (options.sessionId) { + sessionId = options.sessionId; +} else { + sessionId = await createSession(client); +} +``` + +**Metadata ownership:** When `externalServer` is true, the MCP handler owns metadata setup (directory creation, initial metadata write). `runHeadless()` skips its own directory creation and initial metadata write when `externalServer` is true, but still writes progress, conversation JSONL, and finalization as usual. + +Everything else stays the same: prompt sending, polling, progress tracking, conversation JSONL writing, fold detection, finalization. + +### Changes to MCP handler (`src/mcp-server.js`) + +Replace the current shared server path (which calls `sendPromptAsync` directly) with a fire-and-forget `runHeadless()` call: + +```javascript +if (sharedServer.enabled && input.noUi) { + const { server, client } = await sharedServer.ensureServer(); + const { createSession } = require('./opencode-client'); + const { buildPrompts } = require('./prompt-builder'); + + // Create session on shared server + const sessionId = await createSession(client); + + // Write initial metadata (MCP handler owns this, not runHeadless) + fs.mkdirSync(sessionDir, { recursive: true, mode: 0o700 }); + const metaPath = path.join(sessionDir, 'metadata.json'); + fs.writeFileSync(metaPath, JSON.stringify({ + taskId, status: 'running', pid: process.pid, + opencodeSessionId: sessionId, + goPid: server.goPid || null, + createdAt: new Date().toISOString(), + headless: true, model: input.model, + }, null, 2), { mode: 0o600 }); + + // Build context from parent conversation (unless --no-context) + let context = null; + if (input.includeContext !== false) { + const { buildContext } = require('./sidecar/context-builder'); + context = await buildContext({ + project: cwd, + parentSession: input.parentSession, + coworkProcess: input.coworkProcess, + contextTurns: input.contextTurns, + contextSince: input.contextSince, + contextMaxTokens: input.contextMaxTokens, + }); + } + + // Build prompts with context (same as CLI path) + const { system: systemPrompt, userMessage } = buildPrompts( + input.prompt, context, cwd, true, agent, input.summaryLength + ); + + // Register session with idle eviction + sharedServer.addSession(sessionId, (_evictedId) => { + try { + const meta = JSON.parse(fs.readFileSync(metaPath, 'utf-8')); + meta.status = 'idle-timeout'; + meta.completedAt = new Date().toISOString(); + fs.writeFileSync(metaPath, JSON.stringify(meta, null, 2), { mode: 0o600 }); + } catch (err) { + logger.warn('Failed to update evicted session metadata', { error: err.message }); + } + }); + const watchdog = sharedServer.getSessionWatchdog(sessionId); + + // Fire-and-forget: runHeadless with shared server's client + runHeadless(input.model, systemPrompt, userMessage, taskId, cwd, + timeoutMs, agent, { + client, server, watchdog, sessionId, + mcp: mcpServers, + } + ).then((result) => { + // Session complete - finalize metadata and remove from tracking + const { finalizeSession } = require('./sidecar/session-utils'); + const meta = JSON.parse(fs.readFileSync(metaPath, 'utf-8')); + finalizeSession(sessionDir, result.summary || '', cwd, meta); + sharedServer.removeSession(sessionId); + }).catch(err => { + logger.error('Shared server session failed', { taskId, error: err.message }); + sharedServer.removeSession(sessionId); + try { + const meta = JSON.parse(fs.readFileSync(metaPath, 'utf-8')); + meta.status = 'error'; + meta.reason = err.message; + meta.completedAt = new Date().toISOString(); + fs.writeFileSync(metaPath, JSON.stringify(meta, null, 2), { mode: 0o600 }); + } catch (writeErr) { + logger.warn('Failed to write error metadata', { error: writeErr.message }); + } + }); + + // Return immediately + return { content: [{ type: 'text', text: body }] }; +} else { + // Per-process spawn (interactive, or shared server disabled) + // existing spawnSidecarProcess logic unchanged +} +``` + +Key points: +- Session is created by MCP handler, passed to `runHeadless()` via `options.sessionId` +- `runHeadless()` is called WITHOUT `await` (fire-and-forget) +- `.then()` removes the session from `SharedServerManager` tracking on completion +- `.catch()` handles errors by removing session and marking metadata as error +- Metadata write errors are logged (not silently swallowed) +- The handler returns immediately so the MCP caller gets a response +- `runHeadless()` runs in the background, writing progress/conversation/metadata to disk +- `sidecar_status` reads those files as usual (no changes needed) +- Prompts are built via `buildPrompts()` from `prompt-builder.js` (same as CLI path) + +### Context and prompt construction + +The MCP handler builds context and prompts before calling `runHeadless()`, matching the CLI path in `src/sidecar/start.js`: + +```javascript +// 1. Build context from parent conversation (unless --no-context) +const { buildContext } = require('./sidecar/context-builder'); +const context = (input.includeContext !== false) + ? await buildContext({ project: cwd, parentSession: input.parentSession, ... }) + : null; + +// 2. Build prompts with context +const { buildPrompts } = require('./prompt-builder'); +const { system: systemPrompt, userMessage } = buildPrompts( + input.prompt, context, cwd, true /* headless */, agent, input.summaryLength +); +``` + +Without context building, the sidecar would have no parent conversation history and couldn't respond to references like "that bug we discussed." + +### Session finalization + +When `runHeadless()` completes, the MCP handler's `.then()` block must call `finalizeSession()` to: +- Set `status: 'complete'` in metadata +- Write `completedAt` timestamp +- Save `summary.md` to the session directory + +Without this, `sidecar_status` would show "running" indefinitely and `sidecar_read` would fail. The standalone CLI path handles finalization in `startSidecar()` (`src/sidecar/start.js`); the MCP handler must do the same. + +### Files to modify + +| File | Change | +|------|--------| +| `src/headless.js` | Add 4 guard points: external client/server, shutdown, watchdog, sessionId | +| `src/mcp-server.js` | Replace shared server path with fire-and-forget `runHeadless()` | + +No new files. + +### E2E test + +The existing `tests/shared-server-e2e.integration.test.js` already tests the right flow: fire 3 concurrent Gemini sessions, poll until done, read results. With `runHeadless()` handling the full lifecycle, sessions should now complete. + +### Success criteria + +1. E2E test passes: 3 concurrent Gemini sessions via MCP all reach `complete` status +2. `sidecar_status` shows progress (messages > 0) during execution +3. `sidecar_read` returns LLM output after completion +4. Process count stays low (shared server, not N separate servers) +5. Memory stays bounded (RSS < 512MB for 3 sessions) +6. All unit tests pass diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index edc1447..5810071 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -2,6 +2,10 @@ | Issue | Cause | Solution | |-------|-------|----------| +| Sidecar process not self-terminating | Idle timeout misconfigured or disabled | Check per-mode overrides first: `SIDECAR_IDLE_TIMEOUT_HEADLESS`, `SIDECAR_IDLE_TIMEOUT_INTERACTIVE`, `SIDECAR_IDLE_TIMEOUT_SERVER`. The blanket `SIDECAR_IDLE_TIMEOUT` overrides all modes. Set `LOG_LEVEL=debug` to trace watchdog transitions | +| Shared server crash loop | Server crashing repeatedly | Check logs for the root cause; after 3 restarts in 5 min the server halts. Use `SIDECAR_SHARED_SERVER=0` to fall back to per-process mode | +| Resume fails with "session already active" | Stale `session.lock` file from a previous crash | Usually auto-recovers: if the old process is dead, the lock is reclaimed automatically on next resume. Manual deletion only needed if the PID was reused by another process: `rm /.claude/sidecar_sessions//session.lock` | +| Cold start latency after server idle timeout | Shared server was shut down and must restart | Increase `SIDECAR_IDLE_TIMEOUT_SERVER` to keep the server alive longer between requests | | `command not found: opencode` | OpenCode binary not found | Reinstall: `npm install -g claude-sidecar` (opencode-ai is bundled) | | `spawn opencode ENOENT` | CLI not in PATH | Verify `path-setup.js` runs before server start; check `node_modules/.bin/opencode` exists | | API 400 Bad Request | Model format wrong | Use `{providerID, modelID}` object, not string. See `formatModelForAPI()` | diff --git a/docs/usage.md b/docs/usage.md index 81d54c3..0a8b249 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -41,6 +41,16 @@ The `--agent` option specifies which OpenCode native agent to use: Custom agents defined in `~/.config/opencode/agents/` or `.opencode/agents/` are also supported. +## Process Self-Termination + +Sidecar processes automatically shut down after a period of inactivity, so you do not need to manually kill lingering processes. Default idle timeouts: + +- **Headless mode**: 15 minutes (`SIDECAR_IDLE_TIMEOUT_HEADLESS`) +- **Interactive mode**: 60 minutes (`SIDECAR_IDLE_TIMEOUT_INTERACTIVE`) +- **Shared server**: 30 minutes (`SIDECAR_IDLE_TIMEOUT_SERVER`) + +Set `SIDECAR_IDLE_TIMEOUT=0` to disable self-termination. See [docs/configuration.md](configuration.md#process-lifecycle) for all lifecycle env vars. + ## Agentic Evals ```bash diff --git a/evals/run_eval.js b/evals/run_eval.js index f5f1938..a0b8672 100755 --- a/evals/run_eval.js +++ b/evals/run_eval.js @@ -115,7 +115,7 @@ async function runEval(task, opts = {}) { maxBudget: task.max_budget_usd, mcpConfigPath, sandboxDir, - }); + }, 600000); // 10 min - sidecar tasks need time for LLM round-trip + polling } catch (err) { console.error(` Claude failed: ${err.message}`); const failResult = { diff --git a/scripts/eval-with-monitoring.sh b/scripts/eval-with-monitoring.sh new file mode 100755 index 0000000..450fb3a --- /dev/null +++ b/scripts/eval-with-monitoring.sh @@ -0,0 +1,116 @@ +#!/bin/bash +# Run sidecar evals with process and memory monitoring +# Usage: bash scripts/eval-with-monitoring.sh [eval args...] +# Example: bash scripts/eval-with-monitoring.sh --eval-id 1 --mode mcp +# Example: bash scripts/eval-with-monitoring.sh --all --mode mcp + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +PROJECT_DIR="$(cd "$SCRIPT_DIR/.." && pwd)" +MONITOR_LOG="$PROJECT_DIR/evals/workspace/process-monitor-$(date +%Y%m%d-%H%M%S).log" + +mkdir -p "$PROJECT_DIR/evals/workspace" + +# Snapshot: list all opencode/sidecar processes with PID, RSS, command +snapshot_processes() { + local label="$1" + echo "" >> "$MONITOR_LOG" + echo "=== $label ($(date '+%H:%M:%S')) ===" >> "$MONITOR_LOG" + echo "--- OpenCode processes ---" >> "$MONITOR_LOG" + ps aux | grep -i opencode | grep -v grep >> "$MONITOR_LOG" 2>/dev/null || echo " (none)" >> "$MONITOR_LOG" + echo "--- Sidecar processes ---" >> "$MONITOR_LOG" + ps aux | grep -i "sidecar" | grep -v grep | grep -v "eval-with-monitoring" >> "$MONITOR_LOG" 2>/dev/null || echo " (none)" >> "$MONITOR_LOG" + echo "--- Node processes (sidecar-related) ---" >> "$MONITOR_LOG" + ps aux | grep "node.*sidecar\|node.*opencode" | grep -v grep | grep -v "eval-with-monitoring" >> "$MONITOR_LOG" 2>/dev/null || echo " (none)" >> "$MONITOR_LOG" + + # Count and summarize + local oc_count=$(ps aux | grep -i opencode | grep -v grep | wc -l | tr -d ' ') + local sc_count=$(ps aux | grep -i "sidecar" | grep -v grep | grep -v "eval-with-monitoring" | wc -l | tr -d ' ') + local total_rss=$(ps aux | grep -E "opencode|sidecar" | grep -v grep | grep -v "eval-with-monitoring" | awk '{sum+=$6} END {print sum/1024}' 2>/dev/null || echo "0") + + echo "SUMMARY: opencode=$oc_count sidecar=$sc_count total_rss_mb=${total_rss}" >> "$MONITOR_LOG" + echo "[$label] opencode=$oc_count sidecar=$sc_count total_rss=${total_rss}MB" +} + +# Background monitor: snapshot every 10 seconds +background_monitor() { + local count=0 + while true; do + count=$((count + 1)) + snapshot_processes "DURING (sample $count)" > /dev/null 2>&1 + sleep 10 + done +} + +echo "==========================================" +echo "Sidecar Eval with Process Monitoring" +echo "==========================================" +echo "Monitor log: $MONITOR_LOG" +echo "" + +# Pre-eval snapshot +echo "--- PRE-EVAL SNAPSHOT ---" +snapshot_processes "PRE-EVAL" +echo "" + +# Start background monitor +background_monitor & +MONITOR_PID=$! +trap "kill $MONITOR_PID 2>/dev/null; wait $MONITOR_PID 2>/dev/null" EXIT + +# Run the eval +echo "--- RUNNING EVAL ---" +echo "Args: $@" +echo "" +set +e +node "$PROJECT_DIR/evals/run_eval.js" "$@" 2>&1 +EVAL_EXIT=$? +set -e + +# Stop background monitor (|| true: monitor may have already exited) +kill $MONITOR_PID 2>/dev/null || true +wait $MONITOR_PID 2>/dev/null || true +trap - EXIT + +# Wait a moment for cleanup +sleep 3 + +# Post-eval snapshot +echo "" +echo "--- POST-EVAL SNAPSHOT ---" +snapshot_processes "POST-EVAL" +echo "" + +# Compare pre and post +echo "--- PROCESS DELTA ---" +PRE_OC=$(grep "SUMMARY.*PRE-EVAL" "$MONITOR_LOG" | head -1 | grep -o "opencode=[0-9]*" | cut -d= -f2 || echo "0") +POST_OC=$(grep "SUMMARY.*POST-EVAL" "$MONITOR_LOG" | tail -1 | grep -o "opencode=[0-9]*" | cut -d= -f2 || echo "0") +PRE_RSS=$(grep "SUMMARY.*PRE-EVAL" "$MONITOR_LOG" | head -1 | grep -o "total_rss_mb=[0-9.]*" | cut -d= -f2 || echo "0") +POST_RSS=$(grep "SUMMARY.*POST-EVAL" "$MONITOR_LOG" | tail -1 | grep -o "total_rss_mb=[0-9.]*" | cut -d= -f2 || echo "0") + +# Default to 0 if grep returned empty +PRE_OC=${PRE_OC:-0} +POST_OC=${POST_OC:-0} +PRE_RSS=${PRE_RSS:-0} +POST_RSS=${POST_RSS:-0} + +echo "OpenCode processes: before=$PRE_OC after=$POST_OC delta=$((POST_OC - PRE_OC))" +echo "Total RSS (MB): before=$PRE_RSS after=$POST_RSS" + +if [ "$POST_OC" -gt "$PRE_OC" ]; then + echo "" + echo "WARNING: Process leak detected! $((POST_OC - PRE_OC)) orphaned OpenCode processes." + echo "Leaked processes:" + grep "POST-EVAL" -A 100 "$MONITOR_LOG" | grep opencode | grep -v grep | grep -v SUMMARY +else + echo "" + echo "OK: No process leak detected." +fi + +echo "" +echo "Full monitor log: $MONITOR_LOG" +echo "Peak samples during eval:" +grep "SUMMARY.*DURING" "$MONITOR_LOG" | sort -t= -k4 -n -r | head -3 + +exit $EVAL_EXIT diff --git a/src/headless.js b/src/headless.js index e66064c..da4f5a0 100644 --- a/src/headless.js +++ b/src/headless.js @@ -99,64 +99,98 @@ async function runHeadless(model, systemPrompt, userMessage, taskId, project, ti ensurePortAvailable(port); } - // Start OpenCode server using SDK (no CLI spawning required) - logger.debug('Starting OpenCode server via SDK', { model, hasMcp: !!options.mcp, port }); + // Detect shared-server mode: caller provides client + server directly + const externalServer = !!(options.client && options.server); let client, server; - try { - // Pass MCP config and port to server - const serverOptions = { port }; - if (options.mcp) { - serverOptions.mcp = options.mcp; - } - const result = await startServer(serverOptions); - client = result.client; - server = result.server; - logger.debug('Server started', { url: server.url }); - } catch (error) { - logger.error('Failed to start OpenCode server', { error: error.message }); - return { - summary: '', - completed: false, - timedOut: false, - taskId, - error: `Failed to start server: ${error.message}` - }; - } - - let sessionId; - - try { - // Wait for server to be ready - logger.debug('Waiting for OpenCode server to be ready'); - const serverReady = await waitForServer(client, checkHealth); - logger.debug('Server ready', { serverReady }); - writeProgress(sessionDir, 'server_ready'); - - if (!serverReady) { - server.close(); + if (externalServer) { + client = options.client; + server = options.server; + logger.debug('Using external server (shared server mode)', { url: server.url }); + } else { + // Start OpenCode server using SDK (no CLI spawning required) + logger.debug('Starting OpenCode server via SDK', { model, hasMcp: !!options.mcp, port }); + try { + // Pass MCP config and port to server + const serverOptions = { port }; + if (options.mcp) { + serverOptions.mcp = options.mcp; + } + const result = await startServer(serverOptions); + client = result.client; + server = result.server; + logger.debug('Server started', { url: server.url }); + } catch (error) { + logger.error('Failed to start OpenCode server', { error: error.message }); return { summary: '', completed: false, timedOut: false, taskId, - error: 'OpenCode server failed to start' + error: `Failed to start server: ${error.message}` }; } + } + + let sessionId; + const { IdleWatchdog } = require('./utils/idle-watchdog'); + let watchdog; + + try { + if (!externalServer) { + // Wait for server to be ready + logger.debug('Waiting for OpenCode server to be ready'); + const serverReady = await waitForServer(client, checkHealth); + logger.debug('Server ready', { serverReady }); + writeProgress(sessionDir, 'server_ready'); + + if (!serverReady) { + server.close(); + return { + summary: '', + completed: false, + timedOut: false, + taskId, + error: 'OpenCode server failed to start' + }; + } + } else { + writeProgress(sessionDir, 'server_ready'); + } + + // Start idle watchdog to enforce the headless timeout + if (options.watchdog) { + watchdog = options.watchdog; + } else { + watchdog = new IdleWatchdog({ + mode: 'headless', + onTimeout: () => { + logger.info('Headless idle timeout - shutting down', { taskId }); + server.close(); + process.exit(0); + }, + }).start(); + } // Create a new session using SDK logger.debug('Creating OpenCode session'); - try { - sessionId = await createSession(client); - } catch (error) { - server.close(); - return { - summary: '', - completed: false, - timedOut: false, - taskId, - error: error.message - }; + if (options.sessionId) { + sessionId = options.sessionId; + logger.debug('Using existing session', { sessionId }); + } else { + try { + sessionId = await createSession(client); + } catch (error) { + if (watchdog) { watchdog.cancel(); } + if (!externalServer) { server.close(); } + return { + summary: '', + completed: false, + timedOut: false, + taskId, + error: error.message + }; + } } logger.debug('Session ID', { sessionId }); writeProgress(sessionDir, 'session_created'); @@ -222,6 +256,7 @@ async function runHeadless(model, systemPrompt, userMessage, taskId, project, ti // seenPartIds reserved for future use (tracking processed non-text parts) while (!completed && (Date.now() - startTime) < timeoutMs) { + watchdog.touch(); await new Promise(resolve => setTimeout(resolve, 2000)); // Check for external abort signal (MCP tool or CLI command) @@ -450,7 +485,8 @@ async function runHeadless(model, systemPrompt, userMessage, taskId, project, ti } } - server.close(); + watchdog.cancel(); + if (!externalServer) { server.close(); } // Log summary of tool calls for debugging if (toolCalls.length > 0) { @@ -502,7 +538,8 @@ async function runHeadless(model, systemPrompt, userMessage, taskId, project, ti // Ignore abort errors during error handling } } - server.close(); + if (watchdog) { watchdog.cancel(); } + if (!externalServer) { server.close(); } return { summary: '', completed: false, diff --git a/src/mcp-server.js b/src/mcp-server.js index 140b1e4..be13f16 100644 --- a/src/mcp-server.js +++ b/src/mcp-server.js @@ -8,6 +8,9 @@ const os = require('os'); const { logger } = require('./utils/logger'); const { safeSessionDir } = require('./utils/validators'); const { readProgress } = require('./sidecar/progress'); +const { SharedServerManager } = require('./utils/shared-server'); + +const sharedServer = new SharedServerManager({ logger }); /** Resolve the project directory with smart fallback. */ function getProjectDir(explicitProject) { @@ -69,17 +72,23 @@ function spawnSidecarProcess(args, sessionDir) { /** Tool handler implementations */ const handlers = { async sidecar_start(input, project) { - const modelCheck = tryResolveModel(input.model); - if (modelCheck.error) { - return textResult(modelCheck.error, true); + // Validate all inputs before any session creation + const { validateStartInputs } = require('./utils/input-validators'); + const validation = validateStartInputs(input); + if (!validation.valid) { + return { + isError: true, + content: [{ type: 'text', text: JSON.stringify(validation.error) }], + }; } + const resolvedModel = validation.resolvedModel; const cwd = project || getProjectDir(input.project); const { generateTaskId } = require('./sidecar/start'); const taskId = generateTaskId(); const args = ['start', '--prompt', input.prompt, '--task-id', taskId, '--client', 'cowork']; - if (input.model) { args.push('--model', input.model); } + if (resolvedModel) { args.push('--model', resolvedModel); } const agent = (input.noUi && (!input.agent || input.agent.toLowerCase() === 'chat')) ? 'build' : input.agent; if (agent) { args.push('--agent', agent); } @@ -97,6 +106,116 @@ const handlers = { args.push('--cwd', cwd); const sessionDir = path.join(cwd, '.claude', 'sidecar_sessions', taskId); + + if (sharedServer.enabled && input.noUi) { + // Shared server path: headless only, delegates to runHeadless() + let sessionId; + try { + const { server, client } = await sharedServer.ensureServer(); + const { createSession } = require('./opencode-client'); + const { buildContext } = require('./sidecar/context-builder'); + const { buildPrompts } = require('./prompt-builder'); + const { runHeadless } = require('./headless'); + const { finalizeSession } = require('./sidecar/session-utils'); + // resolvedModel is already available from validateStartInputs() above + + sessionId = await createSession(client); + + // Write initial metadata (MCP handler owns this, runHeadless skips it) + fs.mkdirSync(sessionDir, { recursive: true, mode: 0o700 }); + const metaPath = path.join(sessionDir, 'metadata.json'); + const serverPort = server.url ? new URL(server.url).port : null; + fs.writeFileSync(metaPath, JSON.stringify({ + taskId, status: 'running', + pid: null, // Shared server path: don't store MCP server PID (abort would kill all sessions) + opencodeSessionId: sessionId, + opencodePort: serverPort, + goPid: server.goPid || null, + createdAt: new Date().toISOString(), + headless: true, model: resolvedModel, + }, null, 2), { mode: 0o600 }); + + // Build context from parent conversation (unless --no-context) + let context = null; + if (input.includeContext !== false) { + try { + context = buildContext(cwd, input.parentSession, { + contextTurns: input.contextTurns, + contextSince: input.contextSince, + contextMaxTokens: input.contextMaxTokens, + coworkProcess: input.coworkProcess, + }); + } catch (ctxErr) { + logger.warn('Failed to build context, proceeding without', { error: ctxErr.message }); + } + } + + // Build prompts (same as CLI path in start.js) + const { system: systemPrompt, userMessage } = buildPrompts( + input.prompt, context, cwd, true, agent, input.summaryLength + ); + + // Register session with idle eviction + sharedServer.addSession(sessionId, (_evictedId) => { + try { + const meta = JSON.parse(fs.readFileSync(metaPath, 'utf-8')); + meta.status = 'idle-timeout'; + meta.completedAt = new Date().toISOString(); + fs.writeFileSync(metaPath, JSON.stringify(meta, null, 2), { mode: 0o600 }); + } catch (err) { + logger.warn('Failed to update evicted session metadata', { error: err.message }); + } + }); + const watchdog = sharedServer.getSessionWatchdog(sessionId); + + const timeoutMs = (input.timeout || 15) * 60 * 1000; + + // Fire-and-forget: runHeadless with shared server's client + runHeadless(resolvedModel, systemPrompt, userMessage, taskId, cwd, + timeoutMs, agent, { + client, server, watchdog, sessionId, + mcp: undefined, // shared server already has MCP config + } + ).then((result) => { + // Session complete - finalize and remove from tracking + try { + const meta = JSON.parse(fs.readFileSync(metaPath, 'utf-8')); + finalizeSession(sessionDir, result.summary || '', cwd, meta); + } catch (finErr) { + logger.warn('Failed to finalize session', { error: finErr.message }); + } + sharedServer.removeSession(sessionId); + }).catch((err) => { + logger.error('Shared server session failed', { taskId, error: err.message }); + sharedServer.removeSession(sessionId); + try { + const meta = JSON.parse(fs.readFileSync(metaPath, 'utf-8')); + meta.status = 'error'; + meta.reason = err.message; + meta.completedAt = new Date().toISOString(); + fs.writeFileSync(metaPath, JSON.stringify(meta, null, 2), { mode: 0o600 }); + } catch (writeErr) { + logger.warn('Failed to write error metadata', { error: writeErr.message }); + } + }); + + // Return immediately + const body = JSON.stringify({ + taskId, status: 'running', mode: 'headless', + message: 'Sidecar started in headless mode. Use sidecar_status to check progress.', + }); + return { content: [{ type: 'text', text: body }, { type: 'text', text: HEADLESS_START_REMINDER }] }; + } catch (err) { + logger.warn('Shared server path failed, falling back to spawn', { error: err.message }); + // Clean up partial shared server state before falling through + if (sessionId) { + sharedServer.removeSession(sessionId); + } + // Fall through to spawn path below + } + } + + // Feature flag disabled (or shared server failed): fall back to per-process spawn let child; try { child = spawnSidecarProcess(args, sessionDir); } catch (err) { return textResult(`Failed to start sidecar: ${err.message}`, true); @@ -341,6 +460,14 @@ async function startMcpServer() { } ); } + process.on('SIGTERM', () => { + sharedServer.shutdown(); + process.exit(0); + }); + process.on('SIGINT', () => { + sharedServer.shutdown(); + process.exit(0); + }); const transport = new StdioServerTransport(); await server.connect(transport); process.stderr.write('[sidecar] MCP server running on stdio\n'); diff --git a/src/opencode-client.js b/src/opencode-client.js index fd040f2..a6743a1 100644 --- a/src/opencode-client.js +++ b/src/opencode-client.js @@ -110,10 +110,11 @@ async function createSession(client) { * @param {object} [options.tools] - Tool configuration * @param {object} [options.reasoning] - Reasoning/thinking configuration * @param {string} [options.reasoning.effort] - Effort level: 'minimal' | 'low' | 'medium' | 'high' | 'xhigh' | 'none' + * @param {object} [options.watchdog] - IdleWatchdog instance to signal busy/idle around the API call * @returns {Promise} API response */ async function sendPrompt(client, sessionId, options) { - const { model, system, parts, agent, tools, reasoning } = options; + const { model, system, parts, agent, tools, reasoning, watchdog } = options; // Parse model string to SDK format const modelSpec = parseModelString(model); @@ -142,10 +143,21 @@ async function sendPrompt(client, sessionId, options) { body.reasoning = reasoning; } - const result = await client.session.promptAsync({ - path: { id: sessionId }, - body - }); + if (watchdog) { + watchdog.markBusy(); + } + + let result; + try { + result = await client.session.promptAsync({ + path: { id: sessionId }, + body + }); + } finally { + if (watchdog) { + watchdog.markIdle(); + } + } // Log but don't throw on promptAsync errors. // promptAsync is fire-and-forget: the server queues the prompt for async diff --git a/src/sidecar/continue.js b/src/sidecar/continue.js index 7f944f1..1af9ef8 100644 --- a/src/sidecar/continue.js +++ b/src/sidecar/continue.js @@ -13,6 +13,7 @@ const { outputSummary, createHeartbeat } = require('./session-utils'); +const { acquireLock, releaseLock } = require('../utils/session-lock'); const { runHeadless } = require('../headless'); const { buildPrompts } = require('../prompt-builder'); const { logger } = require('../utils/logger'); @@ -123,6 +124,10 @@ async function continueSidecar(options) { const { metadata: oldMetadata, summary: previousSummary, conversation: previousConversation } = loadPreviousSession(oldTaskId, project); + // Lock the previous session directory to prevent concurrent continue operations from the same source + const prevSessionDir = SessionPaths.sessionDir(project, oldTaskId); + acquireLock(prevSessionDir, headless ? 'headless' : 'interactive'); + const model = options.model || oldMetadata.model; const mcpServers = buildMcpConfig({ mcp, mcpConfig, clientType: client, noMcp, excludeMcp }); logger.info('Continuing from session', { oldTaskId, model }); @@ -178,6 +183,7 @@ async function continueSidecar(options) { } } finally { heartbeat.stop(); + releaseLock(prevSessionDir); } // Output summary diff --git a/src/sidecar/crash-handler.js b/src/sidecar/crash-handler.js index 262cb12..3894e75 100644 --- a/src/sidecar/crash-handler.js +++ b/src/sidecar/crash-handler.js @@ -7,6 +7,7 @@ */ const fs = require('fs'); +const path = require('path'); const { SessionPaths } = require('./session-utils'); /** @@ -37,6 +38,14 @@ function installCrashHandler(taskId, project) { metadata.errorAt = new Date().toISOString(); fs.writeFileSync(metaPath, JSON.stringify(metadata, null, 2), { mode: 0o600 }); + + // Delete session lock if it exists + const lockPath = path.join(sessionDir, 'session.lock'); + try { + fs.unlinkSync(lockPath); + } catch { + // Lock may not exist yet + } } catch (_ignored) { // Crash handler must never throw - swallow all errors } diff --git a/src/sidecar/interactive.js b/src/sidecar/interactive.js index 70a9be7..eafa491 100644 --- a/src/sidecar/interactive.js +++ b/src/sidecar/interactive.js @@ -150,6 +150,15 @@ async function runInteractive(model, systemPrompt, userMessage, taskId, project, const serverPort = new URL(server.url).port; + // Start idle watchdog for interactive mode (60-min default timeout) + const { IdleWatchdog } = require('../utils/idle-watchdog'); + const watchdog = new IdleWatchdog({ + mode: 'interactive', + onTimeout: () => { + logger.info('Interactive idle timeout - shutting down', { taskId }); + }, + }).start(); + return new Promise((resolve, _reject) => { const electronPath = getElectronPath(); const mainPath = path.join(__dirname, '..', '..', 'electron', 'main.js'); @@ -173,9 +182,23 @@ async function runInteractive(model, systemPrompt, userMessage, taskId, project, mainPath ], { cwd: project, env, stdio: ['ignore', 'pipe', 'pipe'] }); + // Touch watchdog on Electron stdout activity + electronProcess.stdout.on('data', () => { + watchdog.touch(); + }); + + // Update watchdog onTimeout now that electronProcess is available + watchdog.onTimeout = () => { + logger.info('Interactive idle timeout - shutting down', { taskId }); + if (!electronProcess.killed) { + electronProcess.kill('SIGTERM'); + } + }; + // Clean up server when Electron exits const originalResolve = resolve; handleElectronProcess(electronProcess, taskId, (result) => { + watchdog.cancel(); server.close(); logger.debug('OpenCode server closed after Electron exit'); result.opencodeSessionId = sessionId; diff --git a/src/sidecar/resume.js b/src/sidecar/resume.js index f1291ff..2074262 100644 --- a/src/sidecar/resume.js +++ b/src/sidecar/resume.js @@ -11,8 +11,10 @@ const { SessionPaths, finalizeSession, outputSummary, - createHeartbeat + createHeartbeat, + checkSessionLiveness } = require('./session-utils'); +const { acquireLock, releaseLock } = require('../utils/session-lock'); const { runHeadless } = require('../headless'); const { logger } = require('../utils/logger'); @@ -124,35 +126,47 @@ async function resumeSidecar(options) { const metadata = loadSessionMetadata(sessionDir); const systemPrompt = loadInitialContext(sessionDir); - const mcpServers = buildMcpConfig({ mcp, mcpConfig, clientType: client, noMcp, excludeMcp }); - logger.info('Resuming session', { taskId, model: metadata.model, briefing: metadata.briefing }); + // Dead-process detection: log if the previous process is no longer alive + const liveness = checkSessionLiveness(metadata); + if (liveness !== 'alive') { + logger.info('Session process is dead, restoring from disk', { + taskId, liveness, pid: metadata.pid, + }); + } - // Check for file drift - const drift = checkFileDrift(metadata, project); - let resumePrompt = systemPrompt; + // Acquire lock to prevent concurrent resume operations + acquireLock(sessionDir, headless ? 'headless' : 'interactive'); - if (drift.hasChanges) { - const driftWarning = buildDriftWarning(drift.changedFiles, drift.lastActivityTime); - resumePrompt = systemPrompt + '\n' + driftWarning; - logger.warn('Files changed since last activity', { taskId, changedFileCount: drift.changedFiles.length }); - } + let heartbeat; + try { + const mcpServers = buildMcpConfig({ mcp, mcpConfig, clientType: client, noMcp, excludeMcp }); + logger.info('Resuming session', { taskId, model: metadata.model, briefing: metadata.briefing }); - // Update metadata (get updated metadata with resumedAt) - const updatedMetadata = updateSessionStatus(sessionDir, 'running'); + // Check for file drift + const drift = checkFileDrift(metadata, project); + let resumePrompt = systemPrompt; - // Start heartbeat - const heartbeat = createHeartbeat(); + if (drift.hasChanges) { + const driftWarning = buildDriftWarning(drift.changedFiles, drift.lastActivityTime); + resumePrompt = systemPrompt + '\n' + driftWarning; + logger.warn('Files changed since last activity', { taskId, changedFileCount: drift.changedFiles.length }); + } - let summary; - const effectiveAgent = metadata.agent || 'Build'; + // Update metadata (get updated metadata with resumedAt) + const updatedMetadata = updateSessionStatus(sessionDir, 'running'); - // Load conversation for both paths (interactive already did this, headless didn't) - const conversationPath = SessionPaths.conversationFile(sessionDir); - const existingConversation = fs.existsSync(conversationPath) - ? fs.readFileSync(conversationPath, 'utf-8') - : ''; + // Start heartbeat + heartbeat = createHeartbeat(); + + let summary; + const effectiveAgent = metadata.agent || 'Build'; + + // Load conversation for both paths (interactive already did this, headless didn't) + const conversationPath = SessionPaths.conversationFile(sessionDir); + const existingConversation = fs.existsSync(conversationPath) + ? fs.readFileSync(conversationPath, 'utf-8') + : ''; - try { if (headless) { const userMessage = buildResumeUserMessage(metadata.briefing || '', existingConversation); const result = await runHeadless( @@ -180,15 +194,16 @@ async function resumeSidecar(options) { summary = result.summary || ''; if (result.error) { logger.error('Interactive resume error', { taskId, error: result.error }); } } - } finally { - heartbeat.stop(); - } - // Output summary - outputSummary(summary); + // Output summary + outputSummary(summary); - // Finalize session (use updatedMetadata which has resumedAt) - finalizeSession(sessionDir, summary, project, updatedMetadata); + // Finalize session (use updatedMetadata which has resumedAt) + finalizeSession(sessionDir, summary, project, updatedMetadata); + } finally { + if (heartbeat) { heartbeat.stop(); } + releaseLock(sessionDir); + } } module.exports = { diff --git a/src/sidecar/session-utils.js b/src/sidecar/session-utils.js index 9bd6617..939da8e 100644 --- a/src/sidecar/session-utils.js +++ b/src/sidecar/session-utils.js @@ -225,6 +225,36 @@ async function startOpenCodeServer(mcpConfig, options = {}) { return { client, server }; } +/** + * Check if a process with the given PID is still alive. + * @param {number|null} pid + * @returns {boolean} + */ +function isProcessAlive(pid) { + if (!pid) { return false; } + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + +/** + * Check if a session's processes are alive. + * @param {Object} metadata - Session metadata with pid and goPid + * @returns {'alive'|'server-dead'|'dead'} + */ +function checkSessionLiveness(metadata) { + if (!metadata) { return 'dead'; } + const nodeAlive = isProcessAlive(metadata.pid); + const goAlive = isProcessAlive(metadata.goPid); + + if (nodeAlive && goAlive) { return 'alive'; } + if (nodeAlive && !goAlive) { return 'server-dead'; } + return 'dead'; +} + module.exports = { HEARTBEAT_INTERVAL, SessionPaths, @@ -233,5 +263,7 @@ module.exports = { outputSummary, createHeartbeat, executeMode, - startOpenCodeServer + startOpenCodeServer, + isProcessAlive, + checkSessionLiveness }; diff --git a/src/sidecar/start.js b/src/sidecar/start.js index 316b5b5..18555cf 100644 --- a/src/sidecar/start.js +++ b/src/sidecar/start.js @@ -19,6 +19,7 @@ const { runInteractive, checkElectronAvailable } = require('./interactive'); const { buildPrompts } = require('../prompt-builder'); const { runHeadless } = require('../headless'); const { logger } = require('../utils/logger'); +const { acquireLock, releaseLock } = require('../utils/session-lock'); const { loadMcpConfig, parseMcpSpec } = require('../opencode-client'); const { mapAgentToOpenCode } = require('../utils/agent-mapping'); const { discoverParentMcps } = require('../utils/mcp-discovery'); @@ -59,6 +60,7 @@ function createSessionMetadata(taskId, project, options) { agent: agent || (isHeadless ? 'build' : 'chat'), thinking: thinking || 'medium', status: 'running', + pid: existing.pid || process.pid, createdAt: existing.createdAt || new Date().toISOString() }; @@ -169,6 +171,7 @@ async function startSidecar(options) { model, prompt: effectivePrompt, noUi: effectiveHeadless, agent, thinking }); saveInitialContext(sessDir, systemPrompt, userMessage); + acquireLock(sessDir, effectiveHeadless ? 'headless' : 'interactive'); const heartbeat = createHeartbeat(HEARTBEAT_INTERVAL, sessDir); let summary; @@ -195,6 +198,7 @@ async function startSidecar(options) { } } finally { heartbeat.stop(); + releaseLock(sessDir); } outputSummary(summary); diff --git a/src/utils/idle-watchdog.js b/src/utils/idle-watchdog.js new file mode 100644 index 0000000..a01f77d --- /dev/null +++ b/src/utils/idle-watchdog.js @@ -0,0 +1,222 @@ +/** + * @module idle-watchdog + * IdleWatchdog - BUSY/IDLE state machine with self-terminating timer. + * + * Tracks whether a sidecar session is actively processing (BUSY) or + * waiting for work (IDLE). Fires an onTimeout callback after the + * configured idle period elapses. Supports stuck-stream protection to + * force-transition out of BUSY if a response stream never completes. + * + * Timeout priority (highest to lowest): + * 1. Per-mode env var (SIDECAR_IDLE_TIMEOUT_HEADLESS, etc.) in minutes + * 2. Blanket env var SIDECAR_IDLE_TIMEOUT in minutes + * 3. Constructor option `timeout` in milliseconds + * 4. Mode default (headless=15m, interactive=60m, server=30m) + */ + +'use strict'; + +/** @type {Object.} Default timeouts per mode in milliseconds */ +const MODE_TIMEOUTS = { + headless: 15 * 60 * 1000, + interactive: 60 * 60 * 1000, + server: 30 * 60 * 1000, +}; + +/** @type {Object.} Per-mode environment variable names */ +const MODE_ENV_MAP = { + headless: 'SIDECAR_IDLE_TIMEOUT_HEADLESS', + interactive: 'SIDECAR_IDLE_TIMEOUT_INTERACTIVE', + server: 'SIDECAR_IDLE_TIMEOUT_SERVER', +}; + +/** + * Resolve the effective timeout in milliseconds using the priority chain. + * + * @param {string} mode - The operating mode ('headless', 'interactive', 'server') + * @param {number|undefined} optionTimeout - Caller-supplied timeout in ms (or undefined) + * @returns {number} Effective timeout in ms, or Infinity if disabled + */ +function resolveTimeout(mode, optionTimeout) { + const modeEnvKey = MODE_ENV_MAP[mode]; + if (modeEnvKey !== undefined) { + const modeEnv = process.env[modeEnvKey]; + if (modeEnv !== undefined) { + const mins = Number(modeEnv); + return mins === 0 ? Infinity : mins * 60 * 1000; + } + } + + const blanket = process.env.SIDECAR_IDLE_TIMEOUT; + if (blanket !== undefined) { + const mins = Number(blanket); + return mins === 0 ? Infinity : mins * 60 * 1000; + } + + if (optionTimeout !== undefined) { + return optionTimeout === 0 ? Infinity : optionTimeout; + } + + return MODE_TIMEOUTS[mode] || MODE_TIMEOUTS.headless; +} + +/** + * IdleWatchdog - BUSY/IDLE state machine with configurable idle timeout. + * + * @example + * const wd = new IdleWatchdog({ mode: 'headless', timeout: 60000, onTimeout: () => process.exit(0) }); + * wd.start(); + * wd.markBusy(); // called when a stream starts + * wd.markIdle(); // called when a stream completes + * wd.touch(); // called during polling to reset the idle clock + * wd.cancel(); // stop all timers (e.g. on clean shutdown) + */ +class IdleWatchdog { + /** + * @param {object} options + * @param {'headless'|'interactive'|'server'} [options.mode='headless'] - Operating mode + * @param {number} [options.timeout] - Idle timeout in ms (0 = Infinity/disabled) + * @param {number} [options.stuckStreamTimeout] - Max ms to remain in BUSY before force-idle + * @param {Function} [options.onTimeout] - Called when idle timeout fires + * @param {object} [options.logger] - Logger with .warn() and .info() (defaults to console) + */ + constructor(options = {}) { + const { mode = 'headless', timeout, stuckStreamTimeout, onTimeout, logger: log } = options; + + /** @type {string} Current operating mode */ + this.mode = mode; + + /** @type {number} Effective idle timeout in ms */ + this.timeout = resolveTimeout(mode, timeout); + + /** @type {number} Max ms to stay BUSY before force-transitioning to IDLE */ + this.stuckStreamTimeout = stuckStreamTimeout || 5 * 60 * 1000; + + /** @type {Function} Callback fired on idle timeout */ + this.onTimeout = onTimeout || (() => {}); + + /** @type {object} Logger instance */ + this.logger = log || console; + + /** @type {'IDLE'|'BUSY'} Current state */ + this.state = 'IDLE'; + + /** @type {ReturnType|null} Active idle countdown timer */ + this._timer = null; + + /** @type {ReturnType|null} Stuck-stream detection timer */ + this._stuckTimer = null; + + /** @type {number} Epoch ms when this watchdog was created */ + this._startedAt = Date.now(); + } + + /** + * Activate the watchdog and begin the idle countdown. + * + * @returns {IdleWatchdog} Returns `this` for chaining + */ + start() { + this._resetTimer(); + return this; + } + + /** + * Transition to BUSY state, suspending the idle timer. + * Starts the stuck-stream protection timer. + * Idempotent: calling while already BUSY is a no-op for the state, + * but does refresh the stuck timer. + */ + markBusy() { + this.state = 'BUSY'; + clearTimeout(this._timer); + this._timer = null; + this._startStuckTimer(); + } + + /** + * Transition to IDLE state and restart the idle countdown. + * Cancels the stuck-stream protection timer. + */ + markIdle() { + this.state = 'IDLE'; + this._clearStuckTimer(); + this._resetTimer(); + } + + /** + * Reset the idle countdown without changing state. + * Has no effect when in BUSY state. + */ + touch() { + if (this.state === 'IDLE') { + this._resetTimer(); + } + } + + /** + * Cancel all active timers. The watchdog becomes inert. + * Call this on clean shutdown to prevent stray callbacks. + */ + cancel() { + clearTimeout(this._timer); + this._timer = null; + this._clearStuckTimer(); + } + + /** + * Clear and restart the idle countdown timer. + * + * @private + */ + _resetTimer() { + clearTimeout(this._timer); + if (this.timeout === Infinity) { + this._timer = null; + return; + } + this._timer = setTimeout(() => { + this.logger.info?.('Idle timeout reached', { + mode: this.mode, + uptimeMs: Date.now() - this._startedAt, + }); + this.onTimeout(); + }, this.timeout); + // Allow Node process to exit naturally if only this timer remains. + if (this._timer.unref) { + this._timer.unref(); + } + } + + /** + * Start the stuck-stream protection timer. + * If it fires, the watchdog force-transitions to IDLE. + * + * @private + */ + _startStuckTimer() { + this._clearStuckTimer(); + if (this.stuckStreamTimeout === Infinity) { return; } + this._stuckTimer = setTimeout(() => { + this.logger.warn?.('Stuck stream detected, force-transitioning to IDLE', { + stuckMs: this.stuckStreamTimeout, + }); + this.markIdle(); + }, this.stuckStreamTimeout); + if (this._stuckTimer.unref) { + this._stuckTimer.unref(); + } + } + + /** + * Clear the stuck-stream protection timer. + * + * @private + */ + _clearStuckTimer() { + clearTimeout(this._stuckTimer); + this._stuckTimer = null; + } +} + +module.exports = { IdleWatchdog, resolveTimeout }; diff --git a/src/utils/input-validators.js b/src/utils/input-validators.js new file mode 100644 index 0000000..3a9571f --- /dev/null +++ b/src/utils/input-validators.js @@ -0,0 +1,127 @@ +'use strict'; + +/** + * @module input-validators + * MCP input validation with structured error responses. + * Composes validators from validators.js and adds model resolution. + */ + +// Lazy require to avoid circular dependency (validators.js re-exports from here) +let _validators; +function getValidators() { + if (!_validators) { _validators = require('./validators'); } + return _validators; +} + +/** + * Find candidates that start with the input or vice versa. + * @param {string} input + * @param {string[]} candidates + * @returns {string[]} Up to 3 matching candidates + */ +function findSimilar(input, candidates) { + if (!input) { return []; } + const lower = input.toLowerCase(); + return candidates.filter(c => { + const cl = c.toLowerCase(); + return cl.startsWith(lower) || lower.startsWith(cl); + }).slice(0, 3); +} + +/** + * Validate sidecar_start inputs before session creation. + * Composes existing validators and adds model resolution. + * @param {Object} input - Raw MCP tool input + * @returns {{ valid: true, resolvedModel: string } | { valid: false, error: Object }} + */ +function validateStartInputs(input) { + // 1. Prompt + const { validatePromptContent, validateHeadlessAgent } = getValidators(); + const promptResult = validatePromptContent(input.prompt); + if (!promptResult.valid) { + return { + valid: false, + error: { + type: 'validation_error', + field: 'prompt', + message: promptResult.error, + }, + }; + } + + // 2. Model: resolve alias to full provider/model string + const { tryResolveModel, getEffectiveAliases } = require('./config'); + const { model: resolved, error: modelError } = tryResolveModel(input.model); + if (modelError) { + const aliases = Object.keys(getEffectiveAliases()); + const suggestions = findSimilar(input.model, aliases); + return { + valid: false, + error: { + type: 'validation_error', + field: 'model', + message: input.model + ? `Model '${input.model}' not found. ${modelError}` + : `No model specified and no default configured. ${modelError}`, + suggestions, + available: aliases, + }, + }; + } + + // 3. Timeout: positive number, max 60 minutes + if (input.timeout !== undefined) { + const t = Number(input.timeout); + if (isNaN(t) || t <= 0) { + return { + valid: false, + error: { + type: 'validation_error', + field: 'timeout', + message: `Timeout must be a positive number (minutes). Got: ${input.timeout}`, + }, + }; + } + if (t > 60) { + return { + valid: false, + error: { + type: 'validation_error', + field: 'timeout', + message: `Timeout cannot exceed 60 minutes. Got: ${t}`, + }, + }; + } + } + + // 4. Agent + headless compatibility + // Note: MCP Zod schema defaults agent to 'Chat'. The handler auto-converts + // Chat to Build for headless mode (line ~92 in mcp-server.js). Only reject + // if the user explicitly set agent to Chat with noUi (not the Zod default). + // We detect explicit by checking if agent was in the original input vs Zod default. + // Since we can't distinguish here, skip validation for Chat+noUi - + // the handler will convert it to Build before use. + if (input.noUi && input.agent) { + const lower = input.agent.toLowerCase(); + // Only reject chat if it's NOT the auto-convertible case + // The handler converts chat -> build for headless, so we allow it + if (lower !== 'chat') { + const agentResult = validateHeadlessAgent(input.agent); + if (!agentResult.valid) { + return { + valid: false, + error: { + type: 'validation_error', + field: 'agent', + message: agentResult.error, + suggestions: ['Build', 'Plan'], + }, + }; + } + } + } + + return { valid: true, resolvedModel: resolved }; +} + +module.exports = { validateStartInputs, findSimilar }; diff --git a/src/utils/session-lock.js b/src/utils/session-lock.js new file mode 100644 index 0000000..8e78840 --- /dev/null +++ b/src/utils/session-lock.js @@ -0,0 +1,95 @@ +'use strict'; + +/** + * @module session-lock + * Atomic session lock files to prevent concurrent resume/continue. + */ + +const fs = require('fs'); +const path = require('path'); +const os = require('os'); + +const LOCK_FILENAME = 'session.lock'; + +const MODE_TIMEOUTS = { + headless: 15 * 60 * 1000, + interactive: 60 * 60 * 1000, + mcp: 15 * 60 * 1000, +}; + +function isPidAlive(pid) { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + +function isLockStale(lockData) { + if (!isPidAlive(lockData.pid)) { + return true; + } + const modeTimeout = MODE_TIMEOUTS[lockData.mode] || MODE_TIMEOUTS.headless; + const ageMs = Date.now() - new Date(lockData.timestamp).getTime(); + if (ageMs > modeTimeout * 2) { + return true; + } + return false; +} + +function acquireLock(sessionDir, mode) { + const lockPath = path.join(sessionDir, LOCK_FILENAME); + const lockData = { + pid: process.pid, + timestamp: new Date().toISOString(), + hostname: os.hostname(), + mode, + }; + + // First attempt: atomic create + try { + fs.writeFileSync(lockPath, JSON.stringify(lockData, null, 2), { flag: 'wx', mode: 0o600 }); + return; + } catch (err) { + if (err.code !== 'EEXIST') { throw err; } + } + + // Lock file exists - check if stale + let existingLock; + try { + existingLock = JSON.parse(fs.readFileSync(lockPath, 'utf-8')); + } catch { + try { fs.unlinkSync(lockPath); } catch { /* already gone */ } + fs.writeFileSync(lockPath, JSON.stringify(lockData, null, 2), { flag: 'wx', mode: 0o600 }); + return; + } + + if (isLockStale(existingLock)) { + try { fs.unlinkSync(lockPath); } catch { /* race */ } + try { + fs.writeFileSync(lockPath, JSON.stringify(lockData, null, 2), { flag: 'wx', mode: 0o600 }); + return; + } catch (retryErr) { + if (retryErr.code === 'EEXIST') { + throw new Error('Session already active (concurrent lock acquisition)'); + } + throw retryErr; + } + } + + throw new Error( + `Session already active (PID ${existingLock.pid}, started ${existingLock.timestamp})` + ); +} + +function releaseLock(sessionDir) { + const lockPath = path.join(sessionDir, LOCK_FILENAME); + try { + fs.unlinkSync(lockPath); + } catch { + // Lock may not exist + } +} + +module.exports = { acquireLock, releaseLock, isLockStale, isPidAlive }; diff --git a/src/utils/shared-server.js b/src/utils/shared-server.js new file mode 100644 index 0000000..4f63c81 --- /dev/null +++ b/src/utils/shared-server.js @@ -0,0 +1,215 @@ +'use strict'; + +/** + * @module shared-server + * Manages a single shared OpenCode server for MCP sessions. + */ + +const { IdleWatchdog } = require('./idle-watchdog'); + +const MAX_RESTARTS = 3; +const RESTART_WINDOW = 5 * 60 * 1000; +const RESTART_BACKOFF = 2000; + +/** + * SharedServerManager - manages a single shared OpenCode server for MCP sessions. + * + * Tracks active sessions, handles lazy server startup, deduplicates concurrent + * start requests, and supervises the server with automatic restart on crash. + */ +class SharedServerManager { + /** + * @param {object} [options] + * @param {object} [options.logger] - Logger with .info(), .warn(), .error() methods + */ + constructor(options = {}) { + this.logger = options.logger || console; + this.maxSessions = Number(process.env.SIDECAR_MAX_SESSIONS) || 20; + this.enabled = process.env.SIDECAR_SHARED_SERVER !== '0'; + + /** @type {object|null} Active server handle */ + this.server = null; + + /** @type {object|null} Active OpenCode client */ + this.client = null; + + /** @type {Map} Per-session watchdogs */ + this._sessionWatchdogs = new Map(); + + /** @type {IdleWatchdog|null} Server-level idle watchdog (fires when no sessions remain) */ + this._serverWatchdog = null; + + /** @type {Promise|null} In-flight server start promise (for deduplication) */ + this._starting = null; + + /** @type {number[]} Timestamps of recent restart attempts */ + this._restartTimestamps = []; + } + + /** + * Number of active sessions. + * @returns {number} + */ + get sessionCount() { + return this._sessionWatchdogs.size; + } + + /** + * Ensure the shared server is running. Lazy-starts on first call. + * Deduplicates concurrent calls so only one start occurs. + * + * @param {object} [mcpConfig] - MCP configuration to pass to startOpenCodeServer + * @returns {Promise<{server: object, client: object}>} + */ + async ensureServer(mcpConfig) { + if (this.server && this.client) { + return { server: this.server, client: this.client }; + } + if (this._starting) { + return this._starting; + } + this._starting = this._doStartServer(mcpConfig).then(({ server, client }) => { + this.server = server; + this.client = client; + this._starting = null; + this._serverWatchdog = new IdleWatchdog({ + mode: 'server', + onTimeout: () => { + this.logger.info?.('Shared server idle (no sessions) - shutting down'); + this.shutdown(); + }, + }).start(); + return { server, client }; + }).catch((err) => { + this._starting = null; + throw err; + }); + return this._starting; + } + + /** + * Start the OpenCode server. Overrideable for testing. + * + * @param {object} [mcpConfig] + * @returns {Promise<{server: object, client: object}>} + */ + async _doStartServer(mcpConfig) { + const { startOpenCodeServer } = require('../sidecar/session-utils'); + return startOpenCodeServer(mcpConfig); + } + + /** + * Register a new session. Cancels the server idle watchdog while sessions are active. + * + * @param {string} sessionId - Unique session identifier + * @param {Function} [onEvict] - Called when session is evicted due to idle timeout + * @throws {Error} If max session capacity is reached + */ + addSession(sessionId, onEvict) { + if (this._sessionWatchdogs.size >= this.maxSessions) { + throw new Error(`Max sessions reached (${this.maxSessions}). Cannot add session ${sessionId}.`); + } + const watchdog = new IdleWatchdog({ + mode: 'headless', + onTimeout: () => { + this.logger.info?.('Session idle timeout', { sessionId }); + if (onEvict) { onEvict(sessionId); } + this.removeSession(sessionId); + }, + }).start(); + this._sessionWatchdogs.set(sessionId, watchdog); + if (this._serverWatchdog) { + this._serverWatchdog.cancel(); + } + } + + /** + * Deregister a session. Starts the server idle watchdog when no sessions remain. + * + * @param {string} sessionId + */ + removeSession(sessionId) { + const watchdog = this._sessionWatchdogs.get(sessionId); + if (watchdog) { + watchdog.cancel(); + this._sessionWatchdogs.delete(sessionId); + } + if (this._sessionWatchdogs.size === 0 && this._serverWatchdog) { + this._serverWatchdog.start(); + } + } + + /** + * Get the idle watchdog for a session. + * + * @param {string} sessionId + * @returns {IdleWatchdog|undefined} + */ + getSessionWatchdog(sessionId) { + return this._sessionWatchdogs.get(sessionId); + } + + /** + * Shut down the manager: cancel all watchdogs, close the server. + */ + shutdown() { + for (const [, wd] of this._sessionWatchdogs) { + wd.cancel(); + } + this._sessionWatchdogs.clear(); + if (this._serverWatchdog) { + this._serverWatchdog.cancel(); + this._serverWatchdog = null; + } + if (this.server) { + this.server.close(); + this.server = null; + this.client = null; + } + } + + /** + * Handle a server crash: log, notify sessions, then schedule restart. + * + * @param {number} exitCode - Process exit code from the crashed server + */ + _onServerCrash(exitCode) { + this.logger.error?.('Shared server crashed', { exitCode }); + for (const [id] of this._sessionWatchdogs) { + this.logger.warn?.('Session interrupted by server crash', { sessionId: id }); + } + this.server = null; + this.client = null; + setTimeout(() => this._handleRestart(), RESTART_BACKOFF); + } + + /** + * Attempt to restart the server, subject to rate limiting. + * + * @returns {Promise} true if restart succeeded, false if rate-limited or failed + */ + async _handleRestart() { + const now = Date.now(); + this._restartTimestamps = this._restartTimestamps.filter( + (ts) => now - ts < RESTART_WINDOW + ); + if (this._restartTimestamps.length >= MAX_RESTARTS) { + this.logger.error?.('Max restarts exceeded, not restarting shared server', { + restarts: this._restartTimestamps.length, + windowMs: RESTART_WINDOW, + }); + return false; + } + this._restartTimestamps.push(now); + try { + await this.ensureServer(); + this.logger.info?.('Shared server restarted successfully'); + return true; + } catch (err) { + this.logger.error?.('Failed to restart shared server', { error: err.message }); + return false; + } + } +} + +module.exports = { SharedServerManager }; diff --git a/src/utils/validators.js b/src/utils/validators.js index 22a09b9..a291545 100644 --- a/src/utils/validators.js +++ b/src/utils/validators.js @@ -286,5 +286,8 @@ module.exports = { validateApiKey, validateThinkingLevel, getSupportedThinkingLevels, - findSessionInProjectDirs + findSessionInProjectDirs, + // Re-exported from input-validators.js + validateStartInputs: require('./input-validators').validateStartInputs, + findSimilar: require('./input-validators').findSimilar, }; diff --git a/tests/headless-external-server.test.js b/tests/headless-external-server.test.js new file mode 100644 index 0000000..28b88b6 --- /dev/null +++ b/tests/headless-external-server.test.js @@ -0,0 +1,26 @@ +'use strict'; + +const fs = require('fs'); +const path = require('path'); + +describe('headless external server support', () => { + const src = fs.readFileSync( + path.join(__dirname, '../src/headless.js'), 'utf-8' + ); + + test('checks for externalServer flag', () => { + expect(src).toContain('externalServer'); + }); + + test('skips server.close() when externalServer is true', () => { + expect(src).toContain('!externalServer'); + }); + + test('accepts options.sessionId', () => { + expect(src).toContain('options.sessionId'); + }); + + test('accepts options.watchdog', () => { + expect(src).toContain('options.watchdog'); + }); +}); diff --git a/tests/headless-watchdog.test.js b/tests/headless-watchdog.test.js new file mode 100644 index 0000000..2c0ae5a --- /dev/null +++ b/tests/headless-watchdog.test.js @@ -0,0 +1,15 @@ +'use strict'; + +describe('headless watchdog integration', () => { + test('headless.js creates watchdog in headless mode', () => { + const fs = require('fs'); + const path = require('path'); + const src = fs.readFileSync( + path.join(__dirname, '../src/headless.js'), 'utf-8' + ); + expect(src).toContain('idle-watchdog'); + expect(src).toContain("mode: 'headless'"); + expect(src).toContain('watchdog.touch()'); + expect(src).toContain('watchdog.cancel()'); + }); +}); diff --git a/tests/idle-watchdog.test.js b/tests/idle-watchdog.test.js new file mode 100644 index 0000000..97d2476 --- /dev/null +++ b/tests/idle-watchdog.test.js @@ -0,0 +1,247 @@ +/** + * IdleWatchdog Tests + * + * Tests for the BUSY/IDLE state machine with self-terminating timer. + * Uses Jest fake timers to verify timer-dependent behavior. + */ + +'use strict'; + +const { IdleWatchdog } = require('../src/utils/idle-watchdog'); + +beforeEach(() => jest.useFakeTimers()); +afterEach(() => jest.useRealTimers()); + +describe('IdleWatchdog', () => { + describe('construction', () => { + test('creates with default headless timeout', () => { + const wd = new IdleWatchdog({ mode: 'headless' }); + expect(wd.timeout).toBe(15 * 60 * 1000); + expect(wd.state).toBe('IDLE'); + }); + + test('creates with interactive timeout', () => { + const wd = new IdleWatchdog({ mode: 'interactive' }); + expect(wd.timeout).toBe(60 * 60 * 1000); + }); + + test('creates with server timeout', () => { + const wd = new IdleWatchdog({ mode: 'server' }); + expect(wd.timeout).toBe(30 * 60 * 1000); + }); + + test('custom timeout overrides mode default', () => { + const wd = new IdleWatchdog({ mode: 'headless', timeout: 5000 }); + expect(wd.timeout).toBe(5000); + }); + + test('timeout of 0 means Infinity (disabled)', () => { + const wd = new IdleWatchdog({ mode: 'headless', timeout: 0 }); + expect(wd.timeout).toBe(Infinity); + }); + }); + + describe('state transitions', () => { + test('markBusy transitions to BUSY and suspends timer', () => { + const onTimeout = jest.fn(); + const wd = new IdleWatchdog({ mode: 'headless', timeout: 1000, onTimeout }); + wd.start(); + wd.markBusy(); + expect(wd.state).toBe('BUSY'); + jest.advanceTimersByTime(2000); + expect(onTimeout).not.toHaveBeenCalled(); + }); + + test('markIdle transitions to IDLE and starts timer', () => { + const onTimeout = jest.fn(); + const wd = new IdleWatchdog({ mode: 'headless', timeout: 1000, onTimeout }); + wd.start(); + wd.markBusy(); + wd.markIdle(); + expect(wd.state).toBe('IDLE'); + jest.advanceTimersByTime(1001); + expect(onTimeout).toHaveBeenCalledTimes(1); + }); + + test('touch resets idle timer without changing state', () => { + const onTimeout = jest.fn(); + const wd = new IdleWatchdog({ mode: 'headless', timeout: 1000, onTimeout }); + wd.start(); + jest.advanceTimersByTime(800); + wd.touch(); + jest.advanceTimersByTime(800); + expect(onTimeout).not.toHaveBeenCalled(); + jest.advanceTimersByTime(201); + expect(onTimeout).toHaveBeenCalledTimes(1); + }); + + test('touch during BUSY does not start timer', () => { + const onTimeout = jest.fn(); + const wd = new IdleWatchdog({ mode: 'headless', timeout: 1000, onTimeout }); + wd.start(); + wd.markBusy(); + wd.touch(); + expect(wd.state).toBe('BUSY'); + jest.advanceTimersByTime(2000); + expect(onTimeout).not.toHaveBeenCalled(); + }); + + test('multiple markBusy calls are idempotent', () => { + const wd = new IdleWatchdog({ mode: 'headless', timeout: 1000 }); + wd.start(); + wd.markBusy(); + wd.markBusy(); + expect(wd.state).toBe('BUSY'); + }); + + test('start() returns this for chaining', () => { + const wd = new IdleWatchdog({ mode: 'headless' }); + const result = wd.start(); + expect(result).toBe(wd); + }); + }); + + describe('idle timeout', () => { + test('fires onTimeout after idle period', () => { + const onTimeout = jest.fn(); + const wd = new IdleWatchdog({ mode: 'headless', timeout: 5000, onTimeout }); + wd.start(); + jest.advanceTimersByTime(5001); + expect(onTimeout).toHaveBeenCalledTimes(1); + }); + + test('does not fire if Infinity timeout', () => { + const onTimeout = jest.fn(); + const wd = new IdleWatchdog({ mode: 'headless', timeout: 0, onTimeout }); + wd.start(); + jest.advanceTimersByTime(999999999); + expect(onTimeout).not.toHaveBeenCalled(); + }); + }); + + describe('stuck stream protection', () => { + test('force-transitions to IDLE after stuckStreamTimeout', () => { + const onTimeout = jest.fn(); + const wd = new IdleWatchdog({ + mode: 'headless', timeout: 60000, stuckStreamTimeout: 3000, onTimeout, + }); + wd.start(); + wd.markBusy(); + expect(wd.state).toBe('BUSY'); + jest.advanceTimersByTime(3001); + expect(wd.state).toBe('IDLE'); + jest.advanceTimersByTime(60001); + expect(onTimeout).toHaveBeenCalledTimes(1); + }); + + test('markIdle before stuck timeout cancels stuck timer', () => { + const wd = new IdleWatchdog({ + mode: 'headless', timeout: 60000, stuckStreamTimeout: 3000, + }); + wd.start(); + wd.markBusy(); + jest.advanceTimersByTime(2000); + wd.markIdle(); + jest.advanceTimersByTime(2000); + expect(wd.state).toBe('IDLE'); + }); + + test('cancel() clears all timers', () => { + const onTimeout = jest.fn(); + const wd = new IdleWatchdog({ mode: 'headless', timeout: 1000, onTimeout }); + wd.start(); + wd.cancel(); + jest.advanceTimersByTime(5000); + expect(onTimeout).not.toHaveBeenCalled(); + }); + }); + + describe('env var resolution', () => { + const originalEnv = process.env; + beforeEach(() => { process.env = { ...originalEnv }; }); + afterEach(() => { process.env = originalEnv; }); + + test('per-mode env var takes highest precedence', () => { + process.env.SIDECAR_IDLE_TIMEOUT_HEADLESS = '5'; + process.env.SIDECAR_IDLE_TIMEOUT = '99'; + const wd = new IdleWatchdog({ mode: 'headless', timeout: 7000 }); + expect(wd.timeout).toBe(5 * 60 * 1000); + }); + + test('blanket env var overrides option and default', () => { + process.env.SIDECAR_IDLE_TIMEOUT = '20'; + const wd = new IdleWatchdog({ mode: 'headless', timeout: 7000 }); + expect(wd.timeout).toBe(20 * 60 * 1000); + }); + + test('SIDECAR_IDLE_TIMEOUT_INTERACTIVE applies to interactive mode', () => { + process.env.SIDECAR_IDLE_TIMEOUT_INTERACTIVE = '120'; + const wd = new IdleWatchdog({ mode: 'interactive' }); + expect(wd.timeout).toBe(120 * 60 * 1000); + }); + + test('SIDECAR_IDLE_TIMEOUT_SERVER applies to server mode', () => { + process.env.SIDECAR_IDLE_TIMEOUT_SERVER = '45'; + const wd = new IdleWatchdog({ mode: 'server' }); + expect(wd.timeout).toBe(45 * 60 * 1000); + }); + + test('env var 0 means Infinity', () => { + process.env.SIDECAR_IDLE_TIMEOUT = '0'; + const wd = new IdleWatchdog({ mode: 'headless' }); + expect(wd.timeout).toBe(Infinity); + }); + }); + + describe('full lifecycle', () => { + test('start -> busy -> idle -> timeout', () => { + const onTimeout = jest.fn(); + const wd = new IdleWatchdog({ mode: 'headless', timeout: 5000, onTimeout }).start(); + wd.markBusy(); + jest.advanceTimersByTime(10000); + expect(onTimeout).not.toHaveBeenCalled(); + wd.markIdle(); + jest.advanceTimersByTime(4999); + expect(onTimeout).not.toHaveBeenCalled(); + jest.advanceTimersByTime(2); + expect(onTimeout).toHaveBeenCalledTimes(1); + }); + + test('touch extends idle period during polling', () => { + const onTimeout = jest.fn(); + const wd = new IdleWatchdog({ mode: 'headless', timeout: 3000, onTimeout }).start(); + for (let i = 0; i < 5; i++) { + jest.advanceTimersByTime(2000); + wd.touch(); + } + expect(onTimeout).not.toHaveBeenCalled(); + jest.advanceTimersByTime(3001); + expect(onTimeout).toHaveBeenCalledTimes(1); + }); + + test('stuck stream forces idle after stuckStreamTimeout', () => { + const onTimeout = jest.fn(); + const wd = new IdleWatchdog({ + mode: 'headless', timeout: 2000, stuckStreamTimeout: 1000, onTimeout, + }).start(); + wd.markBusy(); + jest.advanceTimersByTime(1001); + expect(wd.state).toBe('IDLE'); + jest.advanceTimersByTime(2001); + expect(onTimeout).toHaveBeenCalledTimes(1); + }); + }); +}); + +describe('sendPrompt watchdog integration', () => { + test('opencode-client.js wraps sendPrompt with watchdog markBusy/markIdle', () => { + const fs = require('fs'); + const path = require('path'); + const src = fs.readFileSync( + path.join(__dirname, '../src/opencode-client.js'), 'utf-8' + ); + expect(src).toContain('watchdog'); + expect(src).toContain('markBusy'); + expect(src).toContain('markIdle'); + }); +}); diff --git a/tests/input-validation.test.js b/tests/input-validation.test.js new file mode 100644 index 0000000..25201c4 --- /dev/null +++ b/tests/input-validation.test.js @@ -0,0 +1,186 @@ +'use strict'; + +describe('validateStartInputs', () => { + let validateStartInputs, findSimilar; + + beforeAll(() => { + ({ validateStartInputs, findSimilar } = require('../src/utils/validators')); + }); + + describe('prompt validation', () => { + test('rejects empty prompt', () => { + const result = validateStartInputs({ prompt: '', model: 'gemini' }); + expect(result.valid).toBe(false); + expect(result.error.field).toBe('prompt'); + }); + + test('rejects whitespace-only prompt', () => { + const result = validateStartInputs({ prompt: ' ', model: 'gemini' }); + expect(result.valid).toBe(false); + expect(result.error.field).toBe('prompt'); + }); + + test('rejects undefined prompt', () => { + const result = validateStartInputs({ model: 'gemini' }); + expect(result.valid).toBe(false); + expect(result.error.field).toBe('prompt'); + }); + }); + + describe('model validation', () => { + test('rejects invalid model alias with suggestions', () => { + const result = validateStartInputs({ prompt: 'test', model: 'gemni' }); + expect(result.valid).toBe(false); + expect(result.error.field).toBe('model'); + expect(result.error.suggestions).toBeDefined(); + expect(result.error.available).toBeDefined(); + }); + + test('accepts valid model alias and resolves it', () => { + const result = validateStartInputs({ prompt: 'test', model: 'gemini' }); + expect(result.valid).toBe(true); + expect(result.resolvedModel).toBeDefined(); + expect(result.resolvedModel).toContain('/'); + }); + + test('accepts undefined model (uses default)', () => { + const result = validateStartInputs({ prompt: 'test' }); + // Should either resolve default or fail gracefully + // depends on config - if no default configured, may fail + if (result.valid) { + expect(result.resolvedModel).toBeDefined(); + } else { + expect(result.error.field).toBe('model'); + } + }); + + test('accepts full model string as-is', () => { + const result = validateStartInputs({ + prompt: 'test', + model: 'openrouter/google/gemini-2.0-flash', + }); + expect(result.valid).toBe(true); + expect(result.resolvedModel).toBe('openrouter/google/gemini-2.0-flash'); + }); + }); + + describe('timeout validation', () => { + test('rejects negative timeout', () => { + const result = validateStartInputs({ prompt: 'test', model: 'gemini', timeout: -5 }); + expect(result.valid).toBe(false); + expect(result.error.field).toBe('timeout'); + }); + + test('rejects zero timeout', () => { + const result = validateStartInputs({ prompt: 'test', model: 'gemini', timeout: 0 }); + expect(result.valid).toBe(false); + expect(result.error.field).toBe('timeout'); + }); + + test('rejects timeout exceeding 60 minutes', () => { + const result = validateStartInputs({ prompt: 'test', model: 'gemini', timeout: 999 }); + expect(result.valid).toBe(false); + expect(result.error.field).toBe('timeout'); + }); + + test('accepts valid timeout', () => { + const result = validateStartInputs({ prompt: 'test', model: 'gemini', timeout: 15 }); + expect(result.valid).toBe(true); + }); + + test('accepts omitted timeout', () => { + const result = validateStartInputs({ prompt: 'test', model: 'gemini' }); + expect(result.valid).toBe(true); + }); + }); + + describe('agent + headless validation', () => { + test('accepts chat agent with noUi (handler auto-converts to build)', () => { + // Chat + noUi is allowed because the MCP handler converts Chat -> Build + // for headless mode. The Zod schema also defaults agent to Chat. + const result = validateStartInputs({ + prompt: 'test', model: 'gemini', agent: 'Chat', noUi: true, + }); + expect(result.valid).toBe(true); + }); + + test('accepts build agent with noUi', () => { + const result = validateStartInputs({ + prompt: 'test', model: 'gemini', agent: 'Build', noUi: true, + }); + expect(result.valid).toBe(true); + }); + + test('accepts chat agent without noUi', () => { + const result = validateStartInputs({ + prompt: 'test', model: 'gemini', agent: 'Chat', noUi: false, + }); + expect(result.valid).toBe(true); + }); + }); + + describe('error format', () => { + test('error has type field', () => { + const result = validateStartInputs({ prompt: '' }); + expect(result.error.type).toBe('validation_error'); + }); + + test('error has field field', () => { + const result = validateStartInputs({ prompt: '' }); + expect(result.error.field).toBeDefined(); + }); + + test('error has message field', () => { + const result = validateStartInputs({ prompt: '' }); + expect(result.error.message).toBeDefined(); + }); + + test('error is JSON-serializable for MCP response', () => { + const result = validateStartInputs({ prompt: 'test', model: 'nonexistent_xyz' }); + expect(result.valid).toBe(false); + const json = JSON.stringify(result.error); + const parsed = JSON.parse(json); + expect(parsed.type).toBe('validation_error'); + expect(parsed.field).toBe('model'); + expect(parsed.message).toContain('nonexistent_xyz'); + expect(Array.isArray(parsed.available)).toBe(true); + }); + + test('model error includes descriptive message with available aliases', () => { + const result = validateStartInputs({ prompt: 'test', model: 'gem' }); + expect(result.error.message).toContain('gem'); + // 'gem' is a prefix of 'gemini' so suggestions should include it + expect(result.error.suggestions.length).toBeGreaterThan(0); + expect(result.error.available.length).toBeGreaterThan(0); + }); + }); +}); + +describe('findSimilar', () => { + let findSimilar; + + beforeAll(() => { + ({ findSimilar } = require('../src/utils/validators')); + }); + + test('finds prefix matches', () => { + const result = findSimilar('gem', ['gemini', 'gpt', 'opus']); + expect(result).toContain('gemini'); + expect(result).not.toContain('gpt'); + }); + + test('finds reverse prefix matches', () => { + const result = findSimilar('gemini-pro', ['gemini', 'gpt']); + expect(result).toContain('gemini'); + }); + + test('returns empty for null input', () => { + const result = findSimilar(null, ['gemini']); + expect(result).toEqual([]); + }); + + test('limits to 3 results', () => { + const result = findSimilar('g', ['g1', 'g2', 'g3', 'g4', 'g5']); + expect(result.length).toBeLessThanOrEqual(3); + }); +}); diff --git a/tests/interactive-watchdog.test.js b/tests/interactive-watchdog.test.js new file mode 100644 index 0000000..4002dfd --- /dev/null +++ b/tests/interactive-watchdog.test.js @@ -0,0 +1,14 @@ +'use strict'; + +describe('interactive watchdog integration', () => { + test('interactive.js creates watchdog in interactive mode', () => { + const fs = require('fs'); + const path = require('path'); + const src = fs.readFileSync( + path.join(__dirname, '../src/sidecar/interactive.js'), 'utf-8' + ); + expect(src).toContain('idle-watchdog'); + expect(src).toContain("mode: 'interactive'"); + expect(src).toContain('watchdog.cancel()'); + }); +}); diff --git a/tests/mcp-shared-server.test.js b/tests/mcp-shared-server.test.js new file mode 100644 index 0000000..bfdba21 --- /dev/null +++ b/tests/mcp-shared-server.test.js @@ -0,0 +1,36 @@ +'use strict'; + +const fs = require('fs'); +const path = require('path'); + +describe('MCP shared server integration', () => { + const src = fs.readFileSync( + path.join(__dirname, '../src/mcp-server.js'), 'utf-8' + ); + + test('imports SharedServerManager', () => { + expect(src).toContain('shared-server'); + expect(src).toContain('SharedServerManager'); + }); + + test('checks SIDECAR_SHARED_SERVER feature flag', () => { + expect(src).toContain('sharedServer.enabled'); + }); + + test('has SIGTERM/SIGINT cleanup handlers', () => { + expect(src).toContain('SIGTERM'); + expect(src).toContain('sharedServer.shutdown()'); + }); +}); + +describe('MCP shared server uses runHeadless', () => { + const src = fs.readFileSync( + path.join(__dirname, '../src/mcp-server.js'), 'utf-8' + ); + + test('imports runHeadless', () => { + expect(src).toContain('runHeadless'); + expect(src).toContain('buildContext'); + expect(src).toContain('finalizeSession'); + }); +}); diff --git a/tests/process-lifecycle.test.js b/tests/process-lifecycle.test.js new file mode 100644 index 0000000..911713f --- /dev/null +++ b/tests/process-lifecycle.test.js @@ -0,0 +1,61 @@ +'use strict'; + +describe('process lifecycle helpers', () => { + let isProcessAlive, checkSessionLiveness; + + beforeAll(() => { + ({ isProcessAlive, checkSessionLiveness } = require('../src/sidecar/session-utils')); + }); + + describe('isProcessAlive', () => { + test('returns true for current process PID', () => { + expect(isProcessAlive(process.pid)).toBe(true); + }); + + test('returns false for very high PID (almost certainly dead)', () => { + expect(isProcessAlive(999999999)).toBe(false); + }); + + test('returns false for null PID', () => { + expect(isProcessAlive(null)).toBe(false); + }); + + test('returns false for 0', () => { + expect(isProcessAlive(0)).toBe(false); + }); + }); + + describe('checkSessionLiveness', () => { + test('returns alive when both PIDs are current process', () => { + const result = checkSessionLiveness({ pid: process.pid, goPid: process.pid }); + expect(result).toBe('alive'); + }); + + test('returns dead when both PIDs are dead', () => { + const result = checkSessionLiveness({ pid: 999999999, goPid: 999999998 }); + expect(result).toBe('dead'); + }); + + test('returns server-dead when Node alive but Go dead', () => { + const result = checkSessionLiveness({ pid: process.pid, goPid: 999999999 }); + expect(result).toBe('server-dead'); + }); + + test('returns dead when PIDs are null', () => { + const result = checkSessionLiveness({ pid: null, goPid: null }); + expect(result).toBe('dead'); + }); + }); +}); + +describe('crash handler lock cleanup', () => { + test('crash-handler.js deletes session.lock on crash', () => { + const fs = require('fs'); + const path = require('path'); + const src = fs.readFileSync( + path.join(__dirname, '../src/sidecar/crash-handler.js'), 'utf-8' + ); + expect(src).toContain('session.lock'); + expect(src).toContain('unlinkSync'); + }); +}); diff --git a/tests/resume-lock.test.js b/tests/resume-lock.test.js new file mode 100644 index 0000000..b693a2f --- /dev/null +++ b/tests/resume-lock.test.js @@ -0,0 +1,35 @@ +'use strict'; + +const fs = require('fs'); +const path = require('path'); + +describe('resume dead-process detection', () => { + test('resume.js imports session-lock and liveness helpers', () => { + const src = fs.readFileSync( + path.join(__dirname, '../src/sidecar/resume.js'), 'utf-8' + ); + expect(src).toContain('session-lock'); + expect(src).toContain('checkSessionLiveness'); + expect(src).toContain('acquireLock'); + }); +}); + +describe('session lock integration', () => { + test('start.js uses session lock', () => { + const src = fs.readFileSync( + path.join(__dirname, '../src/sidecar/start.js'), 'utf-8' + ); + expect(src).toContain('session-lock'); + expect(src).toContain('acquireLock'); + expect(src).toContain('releaseLock'); + }); + + test('continue.js uses session lock', () => { + const src = fs.readFileSync( + path.join(__dirname, '../src/sidecar/continue.js'), 'utf-8' + ); + expect(src).toContain('session-lock'); + expect(src).toContain('acquireLock'); + expect(src).toContain('releaseLock'); + }); +}); diff --git a/tests/session-lock.test.js b/tests/session-lock.test.js new file mode 100644 index 0000000..346ba72 --- /dev/null +++ b/tests/session-lock.test.js @@ -0,0 +1,92 @@ +'use strict'; + +const fs = require('fs'); +const path = require('path'); +const os = require('os'); + +describe('SessionLock', () => { + let acquireLock, releaseLock, isLockStale; + let tmpDir; + + beforeAll(() => { + ({ acquireLock, releaseLock, isLockStale } = require('../src/utils/session-lock')); + }); + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sidecar-lock-test-')); + }); + + afterEach(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + test('acquireLock creates lock file with correct contents', () => { + acquireLock(tmpDir, 'headless'); + const lockPath = path.join(tmpDir, 'session.lock'); + expect(fs.existsSync(lockPath)).toBe(true); + const contents = JSON.parse(fs.readFileSync(lockPath, 'utf-8')); + expect(contents.pid).toBe(process.pid); + expect(contents.mode).toBe('headless'); + expect(contents.hostname).toBe(os.hostname()); + expect(typeof contents.timestamp).toBe('string'); + }); + + test('acquireLock throws if lock already held by live process', () => { + acquireLock(tmpDir, 'headless'); + expect(() => acquireLock(tmpDir, 'headless')).toThrow(/already active/i); + }); + + test('acquireLock succeeds if existing lock is stale (dead PID)', () => { + const lockPath = path.join(tmpDir, 'session.lock'); + fs.writeFileSync(lockPath, JSON.stringify({ + pid: 999999999, + timestamp: new Date().toISOString(), + hostname: os.hostname(), + mode: 'headless', + })); + expect(() => acquireLock(tmpDir, 'headless')).not.toThrow(); + }); + + test('acquireLock succeeds if lock file is corrupt', () => { + const lockPath = path.join(tmpDir, 'session.lock'); + fs.writeFileSync(lockPath, 'not json{{{'); + expect(() => acquireLock(tmpDir, 'headless')).not.toThrow(); + }); + + test('releaseLock deletes lock file', () => { + acquireLock(tmpDir, 'headless'); + releaseLock(tmpDir); + expect(fs.existsSync(path.join(tmpDir, 'session.lock'))).toBe(false); + }); + + test('releaseLock is safe when no lock exists', () => { + expect(() => releaseLock(tmpDir)).not.toThrow(); + }); + + test('isLockStale returns true for dead PID', () => { + const result = isLockStale({ + pid: 999999999, + timestamp: new Date().toISOString(), + mode: 'headless', + }); + expect(result).toBe(true); + }); + + test('isLockStale returns false for live PID within time window', () => { + const result = isLockStale({ + pid: process.pid, + timestamp: new Date().toISOString(), + mode: 'headless', + }); + expect(result).toBe(false); + }); + + test('isLockStale returns true for live PID but very old timestamp', () => { + const result = isLockStale({ + pid: process.pid, + timestamp: new Date(Date.now() - 4 * 60 * 60 * 1000).toISOString(), + mode: 'headless', + }); + expect(result).toBe(true); + }); +}); diff --git a/tests/shared-server-e2e.integration.test.js b/tests/shared-server-e2e.integration.test.js new file mode 100644 index 0000000..cc9f59c --- /dev/null +++ b/tests/shared-server-e2e.integration.test.js @@ -0,0 +1,363 @@ +/** + * Shared Server E2E Integration Test + * + * Spawns a real MCP server, fires multiple concurrent sidecar_start calls + * using Gemini, verifies they all complete on a single shared server, + * monitors RSS memory throughout, and checks cleanup. + * + * Requires OPENROUTER_API_KEY to run. Skipped automatically when missing. + */ + +'use strict'; + +const { spawn, execFileSync } = require('child_process'); +const path = require('path'); +const fs = require('fs'); +const os = require('os'); + +const SIDECAR_BIN = path.join(__dirname, '..', 'bin', 'sidecar.js'); +const NODE = process.execPath; + +const HAS_API_KEY = !!( + process.env.OPENROUTER_API_KEY || + (() => { + try { + const envPath = path.join(os.homedir(), '.config', 'sidecar', '.env'); + const content = fs.readFileSync(envPath, 'utf-8'); + return content.includes('OPENROUTER_API_KEY='); + } catch { return false; } + })() +); + +const describeE2E = HAS_API_KEY ? describe : describe.skip; + +// --------------------------------------------------------------------------- +// Memory monitor: collects RSS snapshots from the MCP server child process +// --------------------------------------------------------------------------- + +class MemoryMonitor { + constructor(pid) { + this.pid = pid; + this.snapshots = []; + this._interval = null; + } + + start(intervalMs = 2000) { + this._record(); // initial snapshot + this._interval = setInterval(() => this._record(), intervalMs); + return this; + } + + stop() { + if (this._interval) { + clearInterval(this._interval); + this._interval = null; + } + this._record(); // final snapshot + return this; + } + + _record() { + try { + // Use ps to get RSS of the MCP server process (in KB) + const output = execFileSync('ps', ['-o', 'rss=', '-p', String(this.pid)], { + encoding: 'utf-8', + }).trim(); + const rssKB = parseInt(output, 10); + if (!isNaN(rssKB)) { + this.snapshots.push({ + timestamp: Date.now(), + rssKB, + rssMB: Math.round(rssKB / 1024), + }); + } + } catch { + // Process may have exited + } + } + + get peakRSSMB() { + if (this.snapshots.length === 0) { return 0; } + return Math.max(...this.snapshots.map(s => s.rssMB)); + } + + get finalRSSMB() { + if (this.snapshots.length === 0) { return 0; } + return this.snapshots[this.snapshots.length - 1].rssMB; + } + + /** Returns summary for test output */ + summary() { + if (this.snapshots.length === 0) { return 'No snapshots'; } + const first = this.snapshots[0]; + const last = this.snapshots[this.snapshots.length - 1]; + return [ + `Snapshots: ${this.snapshots.length}`, + `Initial RSS: ${first.rssMB}MB`, + `Peak RSS: ${this.peakRSSMB}MB`, + `Final RSS: ${last.rssMB}MB`, + `Duration: ${Math.round((last.timestamp - first.timestamp) / 1000)}s`, + ].join(' | '); + } +} + +// --------------------------------------------------------------------------- +// MCP client (reused from mcp-headless-e2e.integration.test.js) +// --------------------------------------------------------------------------- + +function createMcpClient() { + const child = spawn(NODE, [SIDECAR_BIN, 'mcp'], { + stdio: ['pipe', 'pipe', 'pipe'], + env: { ...process.env, SIDECAR_SHARED_SERVER: '1' }, + }); + + let buffer = ''; + const pending = new Map(); + let nextId = 1; + + child.stdout.on('data', (chunk) => { + buffer += chunk.toString(); + let newlineIdx; + while ((newlineIdx = buffer.indexOf('\n')) !== -1) { + const line = buffer.slice(0, newlineIdx).trim(); + buffer = buffer.slice(newlineIdx + 1); + if (!line) { continue; } + try { + const msg = JSON.parse(line); + if (msg.id !== undefined && pending.has(msg.id)) { + pending.get(msg.id).resolve(msg); + pending.delete(msg.id); + } + } catch { + // Ignore non-JSON lines + } + } + }); + + return { + child, + + request(method, params = {}, timeoutMs = 30000) { + return new Promise((resolve, reject) => { + const id = nextId++; + const msg = JSON.stringify({ jsonrpc: '2.0', id, method, params }); + pending.set(id, { resolve, reject }); + child.stdin.write(msg + '\n'); + setTimeout(() => { + if (pending.has(id)) { + pending.delete(id); + reject(new Error(`Timeout waiting for response to ${method} (id=${id})`)); + } + }, timeoutMs); + }); + }, + + notify(method, params = {}) { + const msg = JSON.stringify({ jsonrpc: '2.0', method, params }); + child.stdin.write(msg + '\n'); + }, + + async close() { + child.stdin.end(); + child.stdout.removeAllListeners(); + for (const [, { reject }] of pending) { + reject(new Error('Client closed')); + } + pending.clear(); + return new Promise((resolve) => { + child.on('close', resolve); + setTimeout(() => { + child.kill('SIGKILL'); + resolve(); + }, 5000); + }); + }, + }; +} + +async function pollUntilDone(client, taskId, project, { intervalMs = 5000, timeoutMs = 180000 } = {}) { + const start = Date.now(); + while (Date.now() - start < timeoutMs) { + const result = await client.request('tools/call', { + name: 'sidecar_status', + arguments: { taskId, project }, + }); + + const text = result.result.content[0].text; + let data; + try { data = JSON.parse(text); } catch { + return { status: 'error', raw: text }; + } + + if (data.status !== 'running') { + return data; + } + + const elapsed = Math.round((Date.now() - start) / 1000); + process.stderr.write(` [e2e] ${elapsed}s | task=${taskId} | status=${data.status} | messages=${data.messages || 0}\n`); + + await new Promise(r => setTimeout(r, intervalMs)); + } + + return { status: 'timeout' }; +} + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function countOpenCodeProcesses() { + try { + const output = execFileSync('pgrep', ['-f', 'opencode'], { + encoding: 'utf-8', + }).trim(); + return output.split('\n').filter(Boolean).length; + } catch { + return 0; + } +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describeE2E('Shared Server E2E: round-robin concurrent sessions with memory monitoring', () => { + let client; + let tmpDir; + let monitor; + const SESSION_COUNT = 3; + + beforeAll(async () => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'shared-server-e2e-')); + + client = createMcpClient(); + + // Start memory monitoring on the MCP server process + monitor = new MemoryMonitor(client.child.pid).start(); + + const initResult = await client.request('initialize', { + protocolVersion: '2024-11-05', + capabilities: {}, + clientInfo: { name: 'shared-server-e2e', version: '1.0.0' }, + }); + expect(initResult.result).toBeDefined(); + client.notify('notifications/initialized', {}); + }, 30000); + + afterAll(async () => { + if (monitor) { + monitor.stop(); + process.stderr.write(`\n [e2e] Memory: ${monitor.summary()}\n`); + } + if (client) { await client.close(); } + if (tmpDir) { + process.stderr.write(` [e2e] Session dir: ${tmpDir}\n`); + } + }); + + it('should handle multiple concurrent sessions on a single shared server', async () => { + // Step 1: Fire N concurrent sidecar_start calls + process.stderr.write(`\n [e2e] Starting ${SESSION_COUNT} concurrent sessions...\n`); + + const startPromises = []; + for (let i = 0; i < SESSION_COUNT; i++) { + startPromises.push( + client.request('tools/call', { + name: 'sidecar_start', + arguments: { + prompt: `Reply with exactly: SESSION_${i}_OK. Nothing else.`, + model: 'gemini', + noUi: true, + timeout: 2, + project: tmpDir, + }, + }, 60000) + ); + } + + const startResults = await Promise.all(startPromises); + + // Verify all starts succeeded + const taskIds = []; + for (let i = 0; i < SESSION_COUNT; i++) { + expect(startResults[i].result).toBeDefined(); + expect(startResults[i].result.isError).toBeUndefined(); + + const text = startResults[i].result.content[0].text; + let data; + try { + data = JSON.parse(text); + } catch { + data = { taskId: text.match(/[a-f0-9]{8}/)?.[0] }; + } + + if (data.taskId) { + taskIds.push(data.taskId); + } + process.stderr.write(` [e2e] Session ${i} started: ${data.taskId || 'unknown'}\n`); + } + + expect(taskIds.length).toBe(SESSION_COUNT); + + // Step 2: Log process count for monitoring + // Note: pgrep may count parent/wrapper processes too, so we log rather than assert + const processCount = countOpenCodeProcesses(); + process.stderr.write(` [e2e] OpenCode processes after start: ${processCount}\n`); + // With shared server enabled, process count should be lower than 2*N + // We log for monitoring; the orphan cleanup test below is the strict assertion + + // Step 3: Record memory at peak (all sessions active) + process.stderr.write(` [e2e] Peak RSS with ${SESSION_COUNT} active sessions: ${monitor.peakRSSMB}MB\n`); + + // Step 4: Poll all sessions until done + process.stderr.write(` [e2e] Polling all sessions...\n`); + const pollResults = await Promise.all( + taskIds.map(taskId => pollUntilDone(client, taskId, tmpDir)) + ); + + // Verify all completed + for (let i = 0; i < SESSION_COUNT; i++) { + process.stderr.write(` [e2e] Session ${taskIds[i]} final: ${pollResults[i].status}\n`); + expect(['complete', 'error']).toContain(pollResults[i].status); + } + + // Step 5: Read results to verify LLM actually responded + for (let i = 0; i < SESSION_COUNT; i++) { + const readResult = await client.request('tools/call', { + name: 'sidecar_read', + arguments: { taskId: taskIds[i], project: tmpDir }, + }); + const readText = readResult.result.content[0].text; + process.stderr.write(` [e2e] Session ${i} output length: ${readText.length} chars\n`); + expect(readText.length).toBeGreaterThan(10); + // Verify the output contains the session marker we asked for + expect(readText).toMatch(/SESSION_\d+_OK|session|ok|hello/i); + } + + // Step 6: Final memory check + monitor.stop(); + process.stderr.write(`\n [e2e] === Memory Report ===\n`); + process.stderr.write(` [e2e] ${monitor.summary()}\n`); + + // Memory should not grow unboundedly with sessions + expect(monitor.peakRSSMB).toBeLessThan(512); + + }, 300000); // 5 min timeout + + it('should have no orphaned processes after MCP server exit', async () => { + const beforeClose = countOpenCodeProcesses(); + process.stderr.write(` [e2e] OpenCode processes before close: ${beforeClose}\n`); + + // Close MCP client (SIGTERM triggers sharedServer.shutdown()) + await client.close(); + client = null; + + // Wait for cleanup + await new Promise(r => setTimeout(r, 3000)); + + const afterClose = countOpenCodeProcesses(); + process.stderr.write(` [e2e] OpenCode processes after close: ${afterClose}\n`); + + expect(afterClose).toBeLessThanOrEqual(beforeClose); + }, 30000); +}); diff --git a/tests/shared-server.test.js b/tests/shared-server.test.js new file mode 100644 index 0000000..00055ea --- /dev/null +++ b/tests/shared-server.test.js @@ -0,0 +1,170 @@ +'use strict'; + +beforeEach(() => jest.useFakeTimers()); +afterEach(() => jest.useRealTimers()); + +describe('SharedServerManager', () => { + let SharedServerManager; + + beforeAll(() => { + ({ SharedServerManager } = require('../src/utils/shared-server')); + }); + + describe('construction', () => { + test('constructs with default options', () => { + const mgr = new SharedServerManager(); + expect(mgr.maxSessions).toBe(20); + expect(mgr.server).toBeNull(); + expect(mgr.sessionCount).toBe(0); + }); + + test('respects SIDECAR_MAX_SESSIONS env var', () => { + const orig = process.env.SIDECAR_MAX_SESSIONS; + process.env.SIDECAR_MAX_SESSIONS = '10'; + const mgr = new SharedServerManager(); + expect(mgr.maxSessions).toBe(10); + process.env.SIDECAR_MAX_SESSIONS = orig; + }); + + test('disabled when SIDECAR_SHARED_SERVER=0', () => { + const orig = process.env.SIDECAR_SHARED_SERVER; + process.env.SIDECAR_SHARED_SERVER = '0'; + const mgr = new SharedServerManager(); + expect(mgr.enabled).toBe(false); + process.env.SIDECAR_SHARED_SERVER = orig; + }); + }); + + describe('session tracking', () => { + let mgr; + + beforeEach(() => { + mgr = new SharedServerManager(); + mgr.server = { url: 'http://localhost:4096', close: jest.fn() }; + mgr.client = {}; + mgr._serverWatchdog = { cancel: jest.fn(), start: jest.fn() }; + }); + + test('addSession increments count and creates watchdog', () => { + mgr.addSession('session-1'); + expect(mgr.sessionCount).toBe(1); + expect(mgr._sessionWatchdogs.has('session-1')).toBe(true); + }); + + test('removeSession decrements count', () => { + mgr.addSession('session-1'); + mgr.removeSession('session-1'); + expect(mgr.sessionCount).toBe(0); + }); + + test('removeSession starts server watchdog when no sessions remain', () => { + const startFn = jest.fn(); + mgr._serverWatchdog = { cancel: jest.fn(), start: startFn }; + mgr.addSession('session-1'); + mgr.removeSession('session-1'); + expect(startFn).toHaveBeenCalled(); + }); + + test('addSession cancels server watchdog', () => { + const cancelFn = jest.fn(); + mgr._serverWatchdog = { cancel: cancelFn, start: jest.fn() }; + mgr.addSession('session-1'); + expect(cancelFn).toHaveBeenCalled(); + }); + + test('addSession rejects when at max capacity', () => { + mgr.maxSessions = 2; + mgr.addSession('s1'); + mgr.addSession('s2'); + expect(() => mgr.addSession('s3')).toThrow(/max.*sessions/i); + }); + + test('getSessionWatchdog returns watchdog for session', () => { + mgr.addSession('session-1'); + const wd = mgr.getSessionWatchdog('session-1'); + expect(wd).toBeDefined(); + expect(wd.state).toBe('IDLE'); + }); + }); + + describe('ensureServer', () => { + test('starts server on first call', async () => { + const mgr = new SharedServerManager(); + const mockServer = { url: 'http://localhost:4096', close: jest.fn(), goPid: 123 }; + const mockClient = { session: {} }; + mgr._doStartServer = jest.fn().mockResolvedValue({ server: mockServer, client: mockClient }); + const { server } = await mgr.ensureServer(); + expect(server.url).toBe('http://localhost:4096'); + expect(mgr._doStartServer).toHaveBeenCalledTimes(1); + }); + + test('reuses server on subsequent calls', async () => { + const mgr = new SharedServerManager(); + const mockServer = { url: 'http://localhost:4096', close: jest.fn(), goPid: 123 }; + const mockClient = { session: {} }; + mgr._doStartServer = jest.fn().mockResolvedValue({ server: mockServer, client: mockClient }); + await mgr.ensureServer(); + await mgr.ensureServer(); + expect(mgr._doStartServer).toHaveBeenCalledTimes(1); + }); + + test('deduplicates concurrent ensureServer calls', async () => { + const mgr = new SharedServerManager(); + const mockServer = { url: 'http://localhost:4096', close: jest.fn(), goPid: 123 }; + const mockClient = { session: {} }; + mgr._doStartServer = jest.fn().mockResolvedValue({ server: mockServer, client: mockClient }); + const [r1, r2] = await Promise.all([mgr.ensureServer(), mgr.ensureServer()]); + expect(r1.server).toBe(r2.server); + expect(mgr._doStartServer).toHaveBeenCalledTimes(1); + }); + }); + + describe('supervisor', () => { + test('_handleRestart restarts server', async () => { + const mgr = new SharedServerManager(); + let startCount = 0; + mgr._doStartServer = jest.fn().mockImplementation(() => { + startCount++; + return Promise.resolve({ + server: { url: 'http://localhost:4096', close: jest.fn() }, + client: { session: {} }, + }); + }); + await mgr.ensureServer(); + expect(startCount).toBe(1); + mgr.server = null; + mgr.client = null; + mgr._starting = null; + await mgr._handleRestart(); + expect(startCount).toBe(2); + }); + + test('stops restarting after MAX_RESTARTS in window', async () => { + const mgr = new SharedServerManager(); + mgr._doStartServer = jest.fn().mockResolvedValue({ + server: { url: 'http://localhost:4096', close: jest.fn() }, + client: { session: {} }, + }); + const now = Date.now(); + mgr._restartTimestamps = [now - 1000, now - 500, now - 100]; + const result = await mgr._handleRestart(); + expect(result).toBe(false); + }); + }); + + describe('shutdown', () => { + test('clears all sessions and closes server', () => { + const closeFn = jest.fn(); + const mgr = new SharedServerManager(); + mgr.server = { url: 'http://localhost:4096', close: closeFn }; + mgr.client = {}; + mgr._serverWatchdog = { cancel: jest.fn(), start: jest.fn() }; + mgr.addSession('s1'); + mgr.addSession('s2'); + mgr.shutdown(); + expect(mgr.sessionCount).toBe(0); + expect(mgr.server).toBeNull(); + expect(closeFn).toHaveBeenCalled(); + }); + }); +}); diff --git a/tests/sidecar/start.test.js b/tests/sidecar/start.test.js index 470c71d..fd0f254 100644 --- a/tests/sidecar/start.test.js +++ b/tests/sidecar/start.test.js @@ -237,8 +237,8 @@ describe('createSessionMetadata PID preservation', () => { fs.readFileSync(SessionPaths.metadataFile(sessionDir), 'utf-8') ); - // pid should not be present (no MCP handler wrote one) - expect(result.pid).toBeUndefined(); + // pid should be set to the current process PID + expect(result.pid).toBe(process.pid); // Standard fields must be present expect(result.taskId).toBe(taskId); expect(result.model).toBe('opus');