Merge external-oauth into main: memory-leak fixes + resource-stats logging

Dockerfile

@@ -52,6 +52,7 @@ RUN apt-get update \
   libssl3 \
   zlib1g \
   lsb-release \
+  procps \
   wget \
   xdg-utils \
   && rm -rf /var/lib/apt/lists/*

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@missionsquad/mcp-api",
-  "version": "1.11.11",
+  "version": "1.11.19",
   "description": "MCP Servers exposed via HTTP API",
   "main": "dist/index.js",
   "repository": "missionsquad/mcp-api",
@@ -30,7 +30,7 @@
   "author": "Jayson Jacobs",
   "license": "Apache-2.0",
   "dependencies": {
-    "@missionsquad/puppeteer-scraper": "^1.1.1",
+    "@missionsquad/puppeteer-scraper": "^1.1.2",
     "@modelcontextprotocol/sdk": "^1.13.0",
     "@types/bcrypt": "^5.0.2",
     "base64-js": "^1.5.1",

src/builtin-servers/servers/searxng.ts

@@ -12,6 +12,8 @@ export class BuiltInSearxngServer extends BaseBuiltInServer {
 
   private scraper: PuppeteerScraper | null = null
   private scraperReady = false
+  private stopped = false
+  private retryTimer: NodeJS.Timeout | null = null
   private readonly MAX_PUPPETEER_RETRIES = 5
   private readonly INITIAL_RETRY_DELAY_MS = 15000
 
@@ -97,14 +99,22 @@ export class BuiltInSearxngServer extends BaseBuiltInServer {
   }
 
   async stop(): Promise<void> {
+    // Mark stopped and cancel any pending init retry so a scheduled attempt
+    // cannot recreate the scraper/browser after shutdown.
+    this.stopped = true
+    if (this.retryTimer) {
+      clearTimeout(this.retryTimer)
+      this.retryTimer = null
+    }
     if (this.scraper) {
       try {
-        // Puppeteer cleanup would go here
-        // Note: PuppeteerScraper needs a cleanup method
+        await this.scraper.closeBrowser()
         log({ level: 'info', msg: 'Stopped Puppeteer scraper' })
       } catch (error) {
         log({ level: 'error', msg: 'Error stopping Puppeteer scraper', error })
       }
+      this.scraper = null
+      this.scraperReady = false
     }
   }
 
@@ -273,38 +283,72 @@ export class BuiltInSearxngServer extends BaseBuiltInServer {
   }
 
   private async initializePuppeteerWithRetries(retryCount = 0): Promise<void> {
+    // Bail out if the server was stopped before this (possibly delayed) attempt ran.
+    if (this.stopped) {
+      return
+    }
+    let scraper: PuppeteerScraper | undefined
     try {
-      log({ 
-        level: 'info', 
-        msg: `Starting Puppeteer initialization (Attempt ${retryCount + 1}/${this.MAX_PUPPETEER_RETRIES})...` 
+      log({
+        level: 'info',
+        msg: `Starting Puppeteer initialization (Attempt ${retryCount + 1}/${this.MAX_PUPPETEER_RETRIES})...`
       })
-      
-      this.scraper = new PuppeteerScraper({
+
+      scraper = new PuppeteerScraper({
         headless: true,
         ignoreHTTPSErrors: true,
         blockResources: false,
         cacheSize: 1000,
         enableGPU: false
       })
-
-      await this.scraper.init()
+
+      await scraper.init()
+
+      // stop() may have been called while init() was in flight; if so, close the
+      // freshly created browser instead of publishing it.
+      if (this.stopped) {
+        await scraper.closeBrowser()
+        return
+      }
+
+      this.scraper = scraper
       this.scraperReady = true
       log({ level: 'info', msg: 'Puppeteer initialized successfully.' })
     } catch (error) {
-      log({ 
-        level: 'error', 
-        msg: `Failed to initialize Puppeteer on attempt ${retryCount + 1}:`, 
-        error 
+      log({
+        level: 'error',
+        msg: `Failed to initialize Puppeteer on attempt ${retryCount + 1}:`,
+        error
       })
-
+
+      // Close any partially-created browser before retrying so repeated init
+      // failures cannot leak Chromium processes. closeBrowser() is a no-op if the
+      // scraper already cleaned up internally.
+      if (scraper) {
+        try {
+          await scraper.closeBrowser()
+        } catch (closeError) {
+          log({ level: 'error', msg: 'Error closing partially-initialized Puppeteer scraper', error: closeError })
+        }
+      }
+
+      if (this.stopped) {
+        return
+      }
+
       if (retryCount < this.MAX_PUPPETEER_RETRIES - 1) {
         const delay = this.INITIAL_RETRY_DELAY_MS * Math.pow(2, retryCount)
         log({ level: 'info', msg: `Retrying in ${delay / 1000} seconds...` })
-        setTimeout(() => this.initializePuppeteerWithRetries(retryCount + 1), delay)
+        this.retryTimer = setTimeout(() => {
+          this.retryTimer = null
+          this.initializePuppeteerWithRetries(retryCount + 1)
+        }, delay)
+        // Do not keep the process alive solely for a pending retry.
+        this.retryTimer.unref()
       } else {
-        log({ 
-          level: 'error', 
-          msg: 'Max retries reached. Puppeteer initialization failed permanently for this session.' 
+        log({
+          level: 'error',
+          msg: 'Max retries reached. Puppeteer initialization failed permanently for this session.'
         })
       }
     }

src/env.ts

@@ -2,6 +2,18 @@ import dotenv from 'dotenv'
 
 dotenv.config()
 
+/**
+ * Parses a millisecond-interval env var. Returns `fallback` when the var is
+ * unset, blank, or not a finite number (so a typo defaults to the intended
+ * interval rather than silently disabling the feature). An explicit finite
+ * value — including `0`, which callers treat as "disabled" — is returned as-is.
+ */
+function parseIntervalMs(raw: string | undefined, fallback: number): number {
+  if (raw === undefined || raw.trim() === '') return fallback
+  const parsed = Number(raw)
+  return Number.isFinite(parsed) ? parsed : fallback
+}
+
 export const env = {
   DEBUG: /true/i.test(process.env.DEBUG || 'false'),
   ENABLE_OAUTH_LOGGING: /true/i.test(process.env.ENABLE_OAUTH_LOGGING || 'false'),
@@ -19,6 +31,7 @@ export const env = {
     return { repo, name }
   }),
   SEARXNG_URL: process.env.SEARXNG_URL,
+  RESOURCE_STATS_INTERVAL_MS: parseIntervalMs(process.env.RESOURCE_STATS_INTERVAL_MS, 60000),
   PYTHON_BIN: process.env.PYTHON_BIN,
   PYTHON_VENV_DIR: process.env.PYTHON_VENV_DIR || 'packages/python',
   PIP_INDEX_URL: process.env.PIP_INDEX_URL,

src/index.ts

@@ -13,6 +13,7 @@ import { McpOAuthTokens } from './services/oauthTokens'
 import { McpUserSessions } from './services/userSessions'
 import { McpUserServerInstalls } from './services/userServerInstalls'
 import { McpDcrClients } from './services/dcrClients'
+import { ResourceStatsService } from './services/resourceStats'
 
 export type Resource = {
   init: () => Promise<void>
@@ -93,6 +94,17 @@ export class API {
     // Set up circular dependency between MCPService and PackageService
     mcpController.getMcpService().setPackageService(packagesController.getPackageService())
 
+    // Periodic resource-stats logging for mcp-api and its spawned MCP servers.
+    // Registered at the FRONT of resources so shutDown() stops it first: sampling
+    // must cease before the MCP/package services it samples begin tearing down,
+    // otherwise a tick mid-shutdown would spawn `ps` and log stale stats.
+    const resourceStatsService = new ResourceStatsService({
+      intervalMs: env.RESOURCE_STATS_INTERVAL_MS,
+      getTrackedProcesses: () => mcpController.getMcpService().getStdioServerPids()
+    })
+    await resourceStatsService.init()
+    this.resources.unshift(resourceStatsService)
+
     app.get('/healthz', (req, res) => {
       console.log('Health checked')
       res.status(200).send('OK')

src/services/mcp.ts

@@ -43,6 +43,7 @@ import {
   McpServerAlreadyExistsError
 } from './mcpErrors'
 import { validateExternalMcpUrl } from '../utils/ssrf'
+import { getStdioTransportPid } from '../utils/resourceStats'
 
 export interface MCPConnection {
   client: Client
@@ -918,6 +919,29 @@ const normalizeExternalAuthError = (
   return error
 }
 
+/**
+ * Maximum number of in-memory log lines retained for each MCP server and each
+ * per-user connection. Older lines are displaced once this count is exceeded.
+ */
+export const MAX_SERVER_LOG_LINES = 500
+
+/**
+ * Append a log line to an in-memory logs array, capping the array at
+ * {@link MAX_SERVER_LOG_LINES} entries by COUNT ONLY (no time-based eviction).
+ * When the cap is exceeded, the oldest lines are dropped so the array retains
+ * the most recent {@link MAX_SERVER_LOG_LINES} lines in insertion order.
+ *
+ * @param logs - The logs array to append to. A no-op when `undefined`.
+ * @param message - The log line to append.
+ */
+export function appendServerLog(logs: string[] | undefined, message: string): void {
+  if (!logs) return
+  logs.push(message)
+  if (logs.length > MAX_SERVER_LOG_LINES) {
+    logs.splice(0, logs.length - MAX_SERVER_LOG_LINES)
+  }
+}
+
 export class MCPService implements Resource {
   public servers: Record<string, MCPServer> = {}
   public userConnections: Record<UserServerKey, UserConnection> = {}
@@ -2254,7 +2278,7 @@ export class MCPService implements Resource {
     const transportErrorHandler = async (error: Error) => {
       log({ level: 'error', msg: `${serverKey} transport error: ${error.message}`, error })
       if (this.servers[serverKey]) {
-        this.servers[serverKey].logs?.push(error.message)
+        appendServerLog(this.servers[serverKey].logs, error.message)
       }
     }
 
@@ -2326,7 +2350,7 @@ export class MCPService implements Resource {
             const logMsg = data.toString().trim()
             log({ level: 'error', msg: `[${server.name}] stderr: ${logMsg}` })
             if (this.servers[serverKey]) {
-              this.servers[serverKey].logs?.push(logMsg)
+              appendServerLog(this.servers[serverKey].logs, logMsg)
             }
           }
 
@@ -2452,7 +2476,7 @@ export class MCPService implements Resource {
       log({ level: 'error', msg: `[${username}:${server.name}] transport error: ${error.message}`, error })
       const conn = this.userConnections[userKey]
       if (conn) {
-        conn.logs?.push(error.message)
+        appendServerLog(conn.logs, error.message)
       }
     }
 
@@ -3822,6 +3846,30 @@ export class MCPService implements Resource {
     return updatedServer
   }
 
+  /**
+   * Lists the OS process ids of currently running stdio MCP servers,
+   * labeled by server name, for resource monitoring.
+   *
+   * Only servers with status 'connected' are included: `connection` is not
+   * cleared on transport close, so a disconnected/error server could otherwise
+   * contribute a stale pid and mislabel an unrelated process if the OS reused it.
+   */
+  public getStdioServerPids(): { name: string; pid: number }[] {
+    const tracked: { name: string; pid: number }[] = []
+    for (const serverKey of Object.keys(this.servers)) {
+      const server = this.servers[serverKey]
+      if (server.transportType !== 'stdio') continue
+      if (server.status !== 'connected') continue
+      const transport = server.connection?.transport
+      if (!transport) continue
+      const pid = getStdioTransportPid(transport)
+      if (pid !== undefined) {
+        tracked.push({ name: server.name, pid })
+      }
+    }
+    return tracked
+  }
+
   public async stop() {
     log({ level: 'info', msg: 'Stopping MCP servers' })