feat(satellite): add cgroup v2 auto-detection and fix stderr log delivery

Lasim · Lasim · commit 14d7efc735cc · 2026-02-22T09:38:51.000+01:00
Cgroup v2 auto-detection:
- Add cgroup-detector.ts that reads /proc/self/cgroup at startup, verifies
  write access to the delegated subtree, moves the satellite process into a
  child cgroup to satisfy the "no internal process" constraint, and enables
  memory + pids controllers via cgroup.subtree_control
- ProcessSpawner detects cgroup availability once at startup and conditionally
  adds --use_cgroupv2, --cgroupv2_mount, --cgroup_mem_max, --cgroup_pids_max
  to nsjail for both MCP server and build command spawns
- --disable_clone_newcgroup is now omitted when cgroup is available (nsjail
  needs the cgroup namespace for per-process isolation)
- NSJAIL_MEMORY_LIMIT_MB default changed from 2048 to "inf" — Node.js v24
  WASM (undici HTTP parser) reserves ~10GB virtual address space; cgroup
  enforces the 512MB physical RAM cap when Delegate=yes is configured

Fix nsjail INFO lines consuming stderr rate limit:
- Move nsjail INFO pre-filter before rateLimiter.shouldAcceptLog() so the
  ~15 startup INFO mount lines do not burn rate limit slots before the actual
  MCP server output arrives

Fix missing event type in isValidEventType():
- Add mcp.server.log_rate_limit_exceeded to the validTypes array so EventBus
  does not reject it with event_emit_invalid_type

Fix crash exit logged to user-facing log buffer:
- Add exit handler that writes an error log entry on non-zero exit and
  immediately flushes the buffer so crash messages reach the backend without
  waiting for the 3-second batch interval

Expand stderr capture on build failures:
- Increase substring limit from 200 to 1000 characters in all npm install,
  npm run build, uv, and pip error messages to expose full failure context

Fix Python deployment: venv shebang absolute paths inside nsjail:
- resolvePythonEntryPoint now captures the module entry point string from
  [project.scripts] (e.g. "plane_mcp.__main__:main") and returns it as
  moduleEntryPoint
- github-deployment.ts uses python3 -m &lt;module&gt; when moduleEntryPoint is
  available, avoiding the venv shebang script that contains an absolute host
  path that does not exist inside nsjail's isolated filesystem

Fix pyproject.toml dependency parsing with extras syntax:
- Replace single regex /\[([\s\S]*?)\]/ with a bracket-aware loop so
  dependencies containing extras (e.g. "pkg[redis,hiredis]") are parsed
  correctly instead of truncating at the first ] inside the extras bracket

Fix isPyprojectSimpleScript for module-based packages:
- Check [project.scripts] entry point module roots (e.g. "plane_mcp" from
  "plane_mcp.__main__:main") against the filesystem so packages that use a
  module directory instead of a src/ layout are correctly identified as
  installable rather than simple scripts
diff --git a/services/satellite/.env.example b/services/satellite/.env.example
@@ -105,14 +105,30 @@ EVENT_FLUSH_TIMEOUT_MS=5000
 # Only active when NODE_ENV=production and platform is Linux
 #
 # Defaults based on empirical testing with npx and Node.js V8:
-# - 2048MB memory: Absolute minimum for V8 initialization (cannot be reduced)
+# - "inf" virtual memory (rlimit_as): Node.js v24 WASM (undici HTTP parser) needs ~10GB virtual space
+# - 512MB physical memory (cgroup): Precise control over actual RAM usage per process
 # - 1000 processes: npm spawns many child processes during package installation
 # - 1024 file descriptors: Adequate for file I/O operations
 # - 50MB file size: Prevents oversized downloads while accommodating 99% of npm packages
 # - 100MB tmpfs: Sufficient for npm cache and temporary operations
-
-# Memory limit per process in MB (default: 2048, V8 minimum requirement)
-NSJAIL_MEMORY_LIMIT_MB=2048
+#
+# Cgroup v2 limits (auto-detected at startup):
+# When running as a systemd service with Delegate=yes, the satellite auto-detects
+# the delegated cgroup path and enables physical memory + PID limits via cgroup v2.
+# If cgroups are unavailable, falls back to rlimit-only mode gracefully.
+
+# Virtual address space limit per process (default: "inf")
+# Accepts MB number (e.g. "4096") or nsjail keywords: "inf", "soft", "hard"
+# "inf" is required for Node.js v24 — WASM (undici) reserves ~10GB virtual address space
+NSJAIL_MEMORY_LIMIT_MB=inf
+
+# Cgroup physical memory limit in bytes (default: 536870912 = 512MB)
+# Only applied when cgroup v2 is available with a delegated subtree
+NSJAIL_CGROUP_MEM_MAX_BYTES=536870912
+
+# Cgroup max PIDs per process (default: 1000)
+# Only applied when cgroup v2 is available with a delegated subtree
+NSJAIL_CGROUP_PIDS_MAX=1000
 
 # CPU time limit per process in seconds (default: 60)
 NSJAIL_CPU_TIME_LIMIT_SECONDS=60
diff --git a/services/satellite/src/config/nsjail.ts b/services/satellite/src/config/nsjail.ts
@@ -24,16 +24,19 @@ function parseSizeToBytes(sizeStr: string): number {
  * These limits apply only in production on Linux platforms when nsjail isolation is enabled
  *
  * Defaults based on empirical testing with package managers (npm, uvx) and runtime requirements:
- * - 2048MB virtual memory (RLIMIT_AS): Fallback for systems without cgroup support
+ * - "inf" virtual memory (RLIMIT_AS): Node.js v24 WASM (undici HTTP parser) needs ~10GB virtual space
  * - 512MB physical memory (cgroup_mem_max): Precise control over actual memory usage
  * - 1000 processes: Adequate for package managers which spawn many child processes
  * - 1024 file descriptors: Adequate for file I/O operations
  * - 50MB file size: Prevents oversized downloads while accommodating most packages
  * - 100MB tmpfs: Sufficient for package manager cache operations
  */
 export const nsjailConfig = {
-  /** Memory limit per MCP server process in MB (default: 2048, sufficient for most runtimes) */
-  memoryLimitMB: parseInt(process.env.NSJAIL_MEMORY_LIMIT_MB || '2048', 10),
+  /** Virtual address space limit per MCP process.
+   *  Accepts MB number (e.g. "4096") or nsjail keywords: "inf", "soft", "hard".
+   *  Default "inf" — Node.js v24 fetch (undici) uses WASM which reserves ~10GB
+   *  virtual address space. This is virtual memory only, not physical RAM. */
+  memoryLimitMB: process.env.NSJAIL_MEMORY_LIMIT_MB || 'inf',
 
   /** Cgroup physical memory limit in bytes (default: 512MB = 536870912 bytes) */
   cgroupMemMaxBytes: parseInt(process.env.NSJAIL_CGROUP_MEM_MAX_BYTES || '536870912', 10),
diff --git a/services/satellite/src/events/registry.ts b/services/satellite/src/events/registry.ts
@@ -281,6 +281,7 @@ export function isValidEventType(type: string): type is EventType {
     'mcp.server.respawned',
     'mcp.server.status_changed',
     'mcp.server.logs',
+    'mcp.server.log_rate_limit_exceeded',
     'mcp.request.logs',
     'mcp.tools.discovered',
     'mcp.tools.updated',
diff --git a/services/satellite/src/lib/cgroup-detector.ts b/services/satellite/src/lib/cgroup-detector.ts
@@ -0,0 +1,82 @@
+import { readFileSync, writeFileSync, mkdirSync, existsSync, accessSync, constants } from 'fs';
+
+export interface CgroupInfo {
+  available: boolean;
+  version: 'v2' | 'v1' | 'none';
+  mountPath: string | null;      // e.g. "/sys/fs/cgroup/system.slice/deploystack-satellite.service"
+  reason?: string;                // why unavailable
+}
+
+/**
+ * Detect cgroup v2 availability and find the delegated cgroup path.
+ *
+ * When running as a systemd service with Delegate=yes, the process owns a
+ * subtree under /sys/fs/cgroup (e.g. /sys/fs/cgroup/system.slice/deploystack-satellite.service).
+ * nsjail must use this delegated path (not the root) to create per-process cgroups.
+ *
+ * Cgroup v2 has a "no internal process" constraint: a cgroup cannot both have
+ * member processes AND enable controllers for children. To satisfy this, we move
+ * the satellite process into a "satellite" child cgroup, leaving the service
+ * cgroup root free for nsjail to write cgroup.subtree_control.
+ */
+export function detectCgroupV2(): CgroupInfo {
+  // 1. Check cgroup v2 is mounted
+  if (!existsSync('/sys/fs/cgroup/cgroup.controllers')) {
+    return { available: false, version: 'none', mountPath: null, reason: 'cgroup v2 not mounted' };
+  }
+
+  // 2. Read our cgroup path from /proc/self/cgroup
+  //    Format: "0::/system.slice/deploystack-satellite.service"
+  try {
+    const content = readFileSync('/proc/self/cgroup', 'utf-8').trim();
+    const match = content.match(/0::(.+)/);
+    if (!match || match[1] === '/') {
+      return { available: false, version: 'v2', mountPath: null, reason: 'process is in root cgroup (no delegation)' };
+    }
+
+    const serviceCgroupRelative = match[1];
+    const serviceCgroupPath = `/sys/fs/cgroup${serviceCgroupRelative}`;
+
+    // 3. Verify the delegated path exists
+    if (!existsSync(serviceCgroupPath)) {
+      return { available: false, version: 'v2', mountPath: null, reason: `delegated path does not exist: ${serviceCgroupPath}` };
+    }
+
+    // 4. Verify write access to the delegated path
+    try {
+      accessSync(serviceCgroupPath, constants.W_OK);
+    } catch {
+      return { available: false, version: 'v2', mountPath: null, reason: `no write access to ${serviceCgroupPath}` };
+    }
+
+    // 5. Move ourselves into a child cgroup to satisfy the "no internal process" constraint.
+    //    cgroup v2 requires that a cgroup with controllers enabled for children has no
+    //    direct member processes. We create "satellite" child and move our PID there,
+    //    freeing the service root for nsjail's subtree_control writes.
+    const satelliteChildPath = `${serviceCgroupPath}/satellite`;
+    try {
+      if (!existsSync(satelliteChildPath)) {
+        mkdirSync(satelliteChildPath);
+      }
+      // Move our process into the child cgroup by writing our PID to cgroup.procs
+      writeFileSync(`${satelliteChildPath}/cgroup.procs`, String(process.pid));
+    } catch (err) {
+      const msg = err instanceof Error ? err.message : String(err);
+      return { available: false, version: 'v2', mountPath: null, reason: `failed to move process to child cgroup: ${msg}` };
+    }
+
+    // 6. Enable memory and pids controllers on the service cgroup root
+    //    This writes "+memory +pids" to cgroup.subtree_control so nsjail's
+    //    per-process child cgroups can enforce limits.
+    try {
+      writeFileSync(`${serviceCgroupPath}/cgroup.subtree_control`, '+memory +pids');
+    } catch (err) {
+      const msg = err instanceof Error ? err.message : String(err);
+      return { available: false, version: 'v2', mountPath: null, reason: `failed to enable controllers on subtree_control: ${msg}` };
+    }
+
+    return { available: true, version: 'v2', mountPath: serviceCgroupPath };
+  } catch {
+    return { available: false, version: 'v2', mountPath: null, reason: 'failed to read /proc/self/cgroup' };
+  }
+}
diff --git a/services/satellite/src/process/github-deployment.ts b/services/satellite/src/process/github-deployment.ts
@@ -118,7 +118,7 @@ export class GitHubDeploymentHandler {
       emitBuildResult(this.logBuffer, metadata, 'npm install', result);
 
       if (result.code !== 0) {
-        throw new Error(`npm install failed with code ${result.code}: ${result.stderr.substring(0, 200)}`);
+        throw new Error(`npm install failed with code ${result.code}: ${result.stderr.substring(0, 1000)}`);
       }
 
       this.logger.info({
@@ -182,7 +182,7 @@ export class GitHubDeploymentHandler {
               stderr: stderr.substring(0, 500) // Limit stderr output
             }, `npm install failed with code ${code}`);
 
-            reject(new Error(`npm install failed with code ${code}: ${stderr.substring(0, 200)}`));
+            reject(new Error(`npm install failed with code ${code}: ${stderr.substring(0, 1000)}`));
           }
         });
 
@@ -252,7 +252,7 @@ export class GitHubDeploymentHandler {
         emitBuildResult(this.logBuffer, metadata, 'npm build', result);
 
         if (result.code !== 0) {
-          throw new Error(`npm run build failed with code ${result.code}: ${result.stderr.substring(0, 200)}`);
+          throw new Error(`npm run build failed with code ${result.code}: ${result.stderr.substring(0, 1000)}`);
         }
 
         this.logger.info({
@@ -316,7 +316,7 @@ export class GitHubDeploymentHandler {
                 stderr: stderr.substring(0, 500)
               }, `npm run build failed with code ${code}`);
 
-              reject(new Error(`npm run build failed with code ${code}: ${stderr.substring(0, 200)}`));
+              reject(new Error(`npm run build failed with code ${code}: ${stderr.substring(0, 1000)}`));
             }
           });
 
@@ -522,7 +522,7 @@ export class GitHubDeploymentHandler {
           message: `[${command}] ${result.stderr.substring(0, 500)}`,
           timestamp: new Date().toISOString()
         });
-        throw new Error(`${command} ${args.join(' ')} failed with code ${result.code}: ${result.stderr.substring(0, 200)}`);
+        throw new Error(`${command} ${args.join(' ')} failed with code ${result.code}: ${result.stderr.substring(0, 1000)}`);
       }
 
       // Step 2: For simple scripts, install dependencies into the venv
@@ -582,7 +582,7 @@ export class GitHubDeploymentHandler {
         emitBuildResult(this.logBuffer, metadata, 'uv pip', depsResult);
 
         if (depsResult.code !== 0) {
-          throw new Error(`uv pip install failed with code ${depsResult.code}: ${depsResult.stderr.substring(0, 200)}`);
+          throw new Error(`uv pip install failed with code ${depsResult.code}: ${depsResult.stderr.substring(0, 1000)}`);
         }
       }
 
@@ -649,7 +649,7 @@ export class GitHubDeploymentHandler {
                 exit_code: code,
                 stderr: stderr.substring(0, 500)
               }, `${cmd} failed with code ${code}`);
-              reject(new Error(`${cmd} failed with code ${code}: ${stderr.substring(0, 200)}`));
+              reject(new Error(`${cmd} failed with code ${code}: ${stderr.substring(0, 1000)}`));
             }
           });
 
@@ -674,29 +674,14 @@ export class GitHubDeploymentHandler {
         let pipArgs: string[];
 
         if (hasPyproject) {
-          // Parse dependencies from pyproject.toml and install them directly
+          // Parse dependencies from pyproject.toml using extracted helper
           const pyprojectPath = path.join(tempDir, 'pyproject.toml');
-          const pyprojectContent = await fs.promises.readFile(pyprojectPath, 'utf8');
-
-          // Extract dependencies array from [project] section
-          const depsMatch = pyprojectContent.match(/dependencies\s*=\s*\[([\s\S]*?)\]/);
-          if (depsMatch) {
-            const depsContent = depsMatch[1];
-            // Parse individual dependencies (handle both "pkg" and 'pkg' quotes)
-            const deps = depsContent
-              .split(',')
-              .map(d => d.trim())
-              .filter(d => d.length > 0)
-              .map(d => d.replace(/^["']|["']$/g, '')); // Remove quotes
-
-            if (deps.length > 0) {
-              pipArgs = ['pip', 'install', ...deps];
-            } else {
-              // No dependencies, skip
-              pipArgs = [];
-            }
+          const deps = await parsePyprojectDependencies(pyprojectPath);
+
+          if (deps.length > 0) {
+            pipArgs = ['pip', 'install', ...deps];
           } else {
-            // No dependencies section
+            // No dependencies, skip
             pipArgs = [];
           }
         } else {
@@ -734,7 +719,7 @@ export class GitHubDeploymentHandler {
    * Resolve Python package entry point from pyproject.toml or __main__.py
    * Delegates to resolvePythonEntryPoint() from python-helpers.ts
    */
-  async resolvePythonPackageEntry(tempDir: string): Promise<{ command: string; entryPoint: string }> {
+  async resolvePythonPackageEntry(tempDir: string): Promise<{ command: string; entryPoint: string; moduleEntryPoint?: string }> {
     this.logger.debug({
       operation: 'python_entry_resolve_start',
       temp_dir: tempDir
@@ -932,7 +917,7 @@ export class GitHubDeploymentHandler {
 
     // Fetch GitHub App installation token (may be null for public repos)
     const tokenResult = await this.backendClient.fetchGitHubToken(config.installation_id);
-    const githubToken = tokenResult?.token || undefined;
+    const githubToken = tokenResult?.token || '';
 
     if (githubToken) {
       this.logger.debug({
@@ -1038,15 +1023,12 @@ export class GitHubDeploymentHandler {
       await this.installPythonDependencies(deploymentDir, config.installation_id, config.team_id, config.user_id);
 
       // Resolve Python package entry point
-      const { command, entryPoint } = await this.resolvePythonPackageEntry(deploymentDir);
+      const { command, entryPoint, moduleEntryPoint } = await this.resolvePythonPackageEntry(deploymentDir);
 
       // Determine the correct command and args based on entry point type
       let updatedConfig: MCPServerConfig;
 
       if (command === 'python3') {
-        // Running with system python3 interpreter - make script path relative
-        const relativeEntryPoint = path.relative(deploymentDir, entryPoint);
-
         // Activate venv by setting PYTHONPATH environment variable
         // This allows system python3 to find packages in .venv/lib/python3.x/site-packages
         //
@@ -1071,10 +1053,37 @@ export class GitHubDeploymentHandler {
           is_production: isProduction
         }, `Activated Python venv via PYTHONPATH: ${sitePackagesPath}`);
 
+        // Determine args: prefer module entry point over script path.
+        // Module entry point (e.g., "plane_mcp.__main__:main") is used via `python3 -m`
+        // to avoid running the venv shebang script, which contains an absolute host path
+        // that doesn't exist inside nsjail's isolated filesystem.
+        let pythonArgs: string[];
+        if (moduleEntryPoint) {
+          // Extract module path from "module.path:func" → "module.path"
+          // Then invoke as: python3 -m module.path
+          // (The module must support __main__ execution, which all [project.scripts] entries do)
+          const colonIdx = moduleEntryPoint.lastIndexOf(':');
+          const modulePath = colonIdx !== -1
+            ? moduleEntryPoint.substring(0, colonIdx)
+            : moduleEntryPoint;
+          pythonArgs = ['-m', modulePath];
+
+          this.logger.info({
+            operation: 'python_module_entry',
+            module_entry_point: moduleEntryPoint,
+            module_path: modulePath,
+            python_args: pythonArgs
+          }, `Using module entry point: python3 -m ${modulePath}`);
+        } else {
+          // Fall back to running script file directly (relative path, nsjail prepends /app)
+          const relativeEntryPoint = path.relative(deploymentDir, entryPoint);
+          pythonArgs = [relativeEntryPoint];
+        }
+
         updatedConfig = {
           ...config,
           command: 'python3',
-          args: [relativeEntryPoint],
+          args: pythonArgs,
           temp_dir: deploymentDir,
           env: activatedEnv
         };
diff --git a/services/satellite/src/process/manager.ts b/services/satellite/src/process/manager.ts
@@ -374,10 +374,16 @@ export class ProcessManager extends EventEmitter {
           const trimmedLine = line.trim();
           if (!trimmedLine) continue;
 
-          // Check rate limit BEFORE processing
+          // Pre-filter nsjail INFO lines before rate limiting (infrastructure noise that
+          // would be discarded anyway — don't waste rate limit slots on them)
+          const nsjailPreCheck = parseNsjailLog(trimmedLine);
+          if (nsjailPreCheck && nsjailPreCheck.level === 'I') {
+            continue;
+          }
+
+          // Check rate limit (only for lines we'll actually buffer)
           const rateLimitResult = rateLimiter.shouldAcceptLog(trimmedLine, currentTime);
           if (!rateLimitResult.accept) {
-            // Log dropped - skip this line
             continue;
           }
 
@@ -386,16 +392,11 @@ export class ProcessManager extends EventEmitter {
             ? rateLimitResult.truncatedMessage!
             : trimmedLine;
 
-          // Check if this is an nsjail log
+          // Re-parse nsjail for WARNING/ERROR/FATAL (truncation may have changed finalMessage)
           const nsjailLog = parseNsjailLog(finalMessage);
 
           if (nsjailLog) {
-            // nsjail log detected - filter out INFO level (infrastructure noise)
-            if (nsjailLog.level === 'I') {
-              // Skip nsjail INFO logs (Mount, Uid map, Jail parameters, etc.)
-              continue;
-            }
-            // Keep nsjail WARNING/ERROR/FATAL logs with correct level mapping
+            // nsjail WARNING/ERROR/FATAL - map to correct level
             const level: 'warn' | 'error' = nsjailLog.level === 'W' ? 'warn' : 'error';
             this.logBuffer.add({
               installation_id: config.installation_id,
@@ -436,6 +437,25 @@ export class ProcessManager extends EventEmitter {
         signal: signal
       }, `MCP server exited: ${config.installation_name} (code: ${code}, signal: ${signal})`);
 
+      // Send exit notification to user-facing logs on non-zero exit (crash)
+      if (code !== 0 && code !== null) {
+        const exitMessage = signal
+          ? `Server process terminated by signal ${signal} (exit code: ${code})`
+          : `Server process exited with error (exit code: ${code})`;
+
+        this.logBuffer.add({
+          installation_id: config.installation_id,
+          team_id: config.team_id,
+          user_id: config.user_id,
+          level: 'error',
+          message: exitMessage,
+          timestamp: new Date().toISOString()
+        });
+
+        // Flush immediately so crash logs reach the backend without waiting for 3s interval
+        this.logBuffer.flush();
+      }
+
       this.emit('processExit', processInfo, code, signal);
     });
 
diff --git a/services/satellite/src/process/nsjail-spawner.ts b/services/satellite/src/process/nsjail-spawner.ts
diff --git a/services/satellite/src/utils/python-helpers.ts b/services/satellite/src/utils/python-helpers.ts
