From 9d15286f7d542d108f33aa166990aeabd5bd4c52 Mon Sep 17 00:00:00 2001 From: lucaronin Date: Wed, 29 Apr 2026 03:29:16 +0200 Subject: [PATCH] fix: stop orphaned mcp server processes --- docs/ABSTRACTIONS.md | 2 +- docs/ARCHITECTURE.md | 4 +- docs/GETTING-STARTED.md | 1 + mcp-server/index.js | 91 ++++++++++++++++++++++++++++++++++++++--- mcp-server/test.js | 56 +++++++++++++++++++++++++ src-tauri/src/lib.rs | 5 +-- 6 files changed, 146 insertions(+), 13 deletions(-) diff --git a/docs/ABSTRACTIONS.md b/docs/ABSTRACTIONS.md index 07f9337d..6997b7d8 100644 --- a/docs/ABSTRACTIONS.md +++ b/docs/ABSTRACTIONS.md @@ -352,7 +352,7 @@ Command-layer path access is fenced to the active vault before file operations r UI-only file actions operate on paths that are already selected or indexed in React state. Reveal-in-Finder and external-open calls route through the Tauri opener plugin, while copy-path uses the browser clipboard API; none of those actions mutate vault contents or bypass the backend write boundary. -The local MCP WebSocket bridge follows the same active-vault boundary. `useVaultSwitcher` calls `sync_mcp_bridge_vault` after the persisted selection loads and after each vault switch; the desktop command starts/restarts the bridge with that vault's canonical path, or stops it when there is no selected vault. MCP Node entrypoints require `VAULT_PATH` and fail clearly instead of falling back to `~/Laputa`. Manual MCP config export uses the same generated stdio entry as registration, so the copied snippet remains scoped to the active vault without writing third-party config files. +The local MCP WebSocket bridge follows the same active-vault boundary. `useVaultSwitcher` calls `sync_mcp_bridge_vault` after the persisted selection loads and after each vault switch; the desktop command starts/restarts the bridge with that vault's canonical path, or stops it when there is no selected vault. App exit uses the same child cleanup path and waits for the bridge process after killing it. MCP Node entrypoints require `VAULT_PATH` and fail clearly instead of falling back to `~/Laputa`. Manual MCP config export uses the same generated stdio entry as registration, so the copied snippet remains scoped to the active vault without writing third-party config files. External-client stdio MCP processes also exit when stdin closes; their UI-bridge reconnect timers and WebSocket are canceled during shutdown so disconnected clients do not leave extra Node processes behind. ### Vault Caching diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 5ad1b66e..3c0d2adc 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -330,7 +330,7 @@ Tolaria can register itself as an MCP server in: - `~/.cursor/mcp.json` (Cursor) - `~/.config/mcp/mcp.json` (generic MCP-compatible clients) -That setup is user-initiated through the status bar / command palette flow, not a startup side effect. Registration is non-destructive (additive, preserves other servers and Gemini settings), uses `upsert` semantics, and can be reversed by removing Tolaria's entry again. Tolaria verifies Node.js is available before writing config, writes an explicit `type: "stdio"` entry, pins `VAULT_PATH` to the active vault, and sets `WS_UI_PORT=9711` so UI actions route back to the desktop app. The same generated entry is exposed as a manual JSON snippet in the MCP setup dialog and through the AI panel copy action, giving users a transparent fallback for MCP-compatible tools Tolaria does not auto-configure. Packaged builds resolve `mcp-server/` from the installed resource directory next to the executable before falling back to macOS `Resources`, Linux bundle paths, and AppImage paths. The `useMcpStatus` hook tracks whether the active vault is explicitly connected (`checking | installed | not_installed`) and owns connect, disconnect, exact-snippet load, and copy-to-clipboard actions. Gemini CLI still owns its own install and sign-in; Tolaria only writes the MCP entry and optional vault guidance shim. The desktop WebSocket bridge is started only when a persisted active vault exists and is resynced from React state on vault changes; no selected vault stops the bridge instead of falling back to `~/Laputa`. +That setup is user-initiated through the status bar / command palette flow, not a startup side effect. Registration is non-destructive (additive, preserves other servers and Gemini settings), uses `upsert` semantics, and can be reversed by removing Tolaria's entry again. Tolaria verifies Node.js is available before writing config, writes an explicit `type: "stdio"` entry, pins `VAULT_PATH` to the active vault, and sets `WS_UI_PORT=9711` so UI actions route back to the desktop app. The same generated entry is exposed as a manual JSON snippet in the MCP setup dialog and through the AI panel copy action, giving users a transparent fallback for MCP-compatible tools Tolaria does not auto-configure. Packaged builds resolve `mcp-server/` from the installed resource directory next to the executable before falling back to macOS `Resources`, Linux bundle paths, and AppImage paths. The `useMcpStatus` hook tracks whether the active vault is explicitly connected (`checking | installed | not_installed`) and owns connect, disconnect, exact-snippet load, and copy-to-clipboard actions. Gemini CLI still owns its own install and sign-in; Tolaria only writes the MCP entry and optional vault guidance shim. The desktop WebSocket bridge is started only when a persisted active vault exists and is resynced from React state on vault changes; no selected vault stops the bridge instead of falling back to `~/Laputa`. Stdio MCP server processes are owned by the external client that launched them: when that client closes stdin, Tolaria cancels UI-bridge reconnect timers, closes any UI WebSocket, and exits the Node process instead of keeping it alive in the background. ### Architecture @@ -382,7 +382,7 @@ flowchart LR | `remove_mcp()` | Removes Tolaria's MCP entry from Claude Code, Gemini CLI, Cursor, and generic MCP configs | | `upsert_mcp_config(path, entry)` | Atomic config file update (create/merge, preserves others) | -The `WsBridgeChild` state wrapper in `lib.rs` ensures the bridge process is replaced on vault switches, stopped when no active vault is selected, and killed on app exit via the `RunEvent::Exit` handler. The same desktop layer now keeps the Tauri asset protocol scoped to the active vault instead of every filesystem path. +The `WsBridgeChild` state wrapper in `lib.rs` ensures the bridge process is replaced on vault switches, stopped when no active vault is selected, and killed plus waited on app exit via the `RunEvent::Exit` handler. The same desktop layer now keeps the Tauri asset protocol scoped to the active vault instead of every filesystem path. ## Search diff --git a/docs/GETTING-STARTED.md b/docs/GETTING-STARTED.md index d520aac4..34c5062f 100644 --- a/docs/GETTING-STARTED.md +++ b/docs/GETTING-STARTED.md @@ -432,3 +432,4 @@ BASE_URL="http://localhost:5173" npx playwright test tests/smoke/.spec.ts 2. **Setup dialog copy/actions**: Edit `src/components/McpSetupDialog.tsx` and `src/hooks/useMcpStatus.ts`; users should see the Node.js prerequisite, the exact generated manual config, and a copy action before Tolaria writes third-party config files 3. **Status hook/toasts**: Edit `src/hooks/useMcpStatus.ts` when setup, reconnect, disconnect, or failure messaging changes 4. **Gemini CLI compatibility**: Keep `~/.gemini/settings.json` in the registration path list and keep optional `GEMINI.md` generation behind `restore_vault_ai_guidance`; Gemini itself still needs its own install and sign-in outside Tolaria +5. **Process lifecycle**: Stdio MCP servers in `mcp-server/index.js` must exit when their external client closes stdin, and the desktop-owned `ws-bridge.js` child must be stopped on vault deselection, vault switch, and app exit diff --git a/mcp-server/index.js b/mcp-server/index.js index 6d887bd6..7e672bd6 100644 --- a/mcp-server/index.js +++ b/mcp-server/index.js @@ -30,27 +30,69 @@ const WS_UI_URL = `ws://localhost:${WS_UI_PORT}` // Connect as a WebSocket CLIENT to the UI bridge (run by ws-bridge.js). // The bridge relays messages to all other clients (the React frontend). let uiSocket = null +let reconnectTimer = null +let shutdownStarted = false const RECONNECT_INTERVAL_MS = 3000 function connectUiBridge() { + if (shutdownStarted) return + try { const ws = new WebSocket(WS_UI_URL) + uiSocket = ws ws.on('open', () => { - uiSocket = ws + if (shutdownStarted) { + closeUiSocket() + return + } console.error(`[mcp] Connected to UI bridge at ${WS_UI_URL}`) }) ws.on('close', () => { - uiSocket = null - setTimeout(connectUiBridge, RECONNECT_INTERVAL_MS) + if (uiSocket === ws) uiSocket = null + scheduleUiReconnect() }) ws.on('error', () => { // Silent — bridge may not be running yet, will retry }) } catch { - setTimeout(connectUiBridge, RECONNECT_INTERVAL_MS) + scheduleUiReconnect() } } -connectUiBridge() + +function scheduleUiReconnect() { + if (shutdownStarted) return + + clearUiReconnectTimer() + reconnectTimer = setTimeout(connectUiBridge, RECONNECT_INTERVAL_MS) + reconnectTimer.unref?.() +} + +function clearUiReconnectTimer() { + if (!reconnectTimer) return + + clearTimeout(reconnectTimer) + reconnectTimer = null +} + +function closeUiSocket() { + const socket = uiSocket + uiSocket = null + if (!socket) return + + socket.removeAllListeners() + socket.on('error', () => {}) + if (socket.readyState === WebSocket.CONNECTING) { + socket.terminate?.() + return + } + + try { + socket.close() + } catch { + // Ignore close races during process teardown. + } + socket.terminate?.() +} function broadcastUiAction(action, payload) { if (!uiSocket || uiSocket.readyState !== WebSocket.OPEN) return @@ -193,10 +235,47 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => { } }) +async function shutdown(exitCode = 0) { + if (shutdownStarted) return + + shutdownStarted = true + clearUiReconnectTimer() + closeUiSocket() + + try { + await server.close() + } catch (error) { + console.error(`[mcp] Error while closing server: ${error.message}`) + } + + process.exitCode = exitCode + setImmediate(() => process.exit(exitCode)) +} + async function main() { const transport = new StdioServerTransport() + server.onclose = () => { + void shutdown(0) + } + process.stdin.once('end', () => { + void shutdown(0) + }) + process.stdin.once('close', () => { + void shutdown(0) + }) + process.once('SIGINT', () => { + void shutdown(0) + }) + process.once('SIGTERM', () => { + void shutdown(0) + }) + + connectUiBridge() await server.connect(transport) console.error(`Tolaria MCP server running (vault: ${VAULT_PATH})`) } -main().catch(console.error) +main().catch((error) => { + console.error(error) + void shutdown(1) +}) diff --git a/mcp-server/test.js b/mcp-server/test.js index ed5a0cba..7c635b75 100644 --- a/mcp-server/test.js +++ b/mcp-server/test.js @@ -1,8 +1,10 @@ import { describe, it, before, after } from 'node:test' import assert from 'node:assert/strict' +import { spawn } from 'node:child_process' import fs from 'node:fs/promises' import path from 'node:path' import os from 'node:os' +import { fileURLToPath } from 'node:url' import { findMarkdownFiles, getNote, searchNotes, vaultContext, } from './vault.js' @@ -11,6 +13,7 @@ import { evaluateBridgeRequest } from './ws-bridge.js' let tmpDir const ACTIVE_VAULT_ERROR = 'Note path must stay inside the active vault' +const MCP_SERVER_DIR = path.dirname(fileURLToPath(import.meta.url)) before(async () => { tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'laputa-mcp-test-')) @@ -208,6 +211,34 @@ describe('requireVaultPath', () => { }) }) +describe('stdio process lifecycle', () => { + it('exits when the MCP client closes stdin', async () => { + const child = spawn(process.execPath, ['index.js'], { + cwd: MCP_SERVER_DIR, + env: { ...process.env, VAULT_PATH: tmpDir, WS_UI_PORT: '65534' }, + stdio: ['pipe', 'ignore', 'pipe'], + }) + let stderr = '' + child.stderr.setEncoding('utf8') + child.stderr.on('data', chunk => { + stderr += chunk + }) + + await sleep(200) + child.stdin.end() + + const exit = await waitForExit(child, 1_500) + if (!exit) { + child.kill() + await waitForExit(child, 1_000) + assert.fail(`MCP server stayed alive after stdin closed.\n${stderr}`) + } + + assert.equal(exit.signal, null) + assert.equal(exit.code, 0, stderr) + }) +}) + async function assertRejectsOutsideVault(prefix, resolveNotePath) { const outsideDir = await fs.mkdtemp(path.join(os.tmpdir(), prefix)) const outsideNote = path.join(outsideDir, 'outside.md') @@ -222,3 +253,28 @@ async function assertRejectsOutsideVault(prefix, resolveNotePath) { await fs.rm(outsideDir, { recursive: true, force: true }) } } + +function sleep(ms) { + return new Promise(resolve => setTimeout(resolve, ms)) +} + +function waitForExit(child, timeoutMs) { + return new Promise((resolve) => { + const timer = setTimeout(() => { + cleanup() + resolve(null) + }, timeoutMs) + + child.once('exit', onExit) + + function onExit(code, signal) { + cleanup() + resolve({ code, signal }) + } + + function cleanup() { + clearTimeout(timer) + child.off('exit', onExit) + } + }) +} diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index ebf7cce5..9d3167b5 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -461,10 +461,7 @@ fn handle_run_event(app_handle: &tauri::AppHandle, event: &tauri::RunEvent) { if let tauri::RunEvent::Exit = event { let state: tauri::State<'_, WsBridgeChild> = app_handle.state(); let mut guard = state.0.lock().unwrap(); - if let Some(ref mut child) = *guard { - let _ = child.kill(); - log::info!("ws-bridge child process killed on exit"); - } + stop_ws_bridge_child(&mut guard); } }