fix(sentry): enable Sentry for Python subprocesses and add diagnostic instrumentation (#1804)

* fix(sentry): enable Sentry for Python subprocesses and add diagnostic instrumentation

Sentry was broken for PR review (and all GitHub runner) subprocesses due to
two bugs: getRunnerEnv() didn't include getSentryEnvForSubprocess(), and
Python's init_sentry() required sys.frozen which is always False for the
non-frozen interpreter. Also adds a 120s health-check timeout to detect
subprocess hangs, Sentry breadcrumbs to PR review lifecycle, and forces
unbuffered Python output for reliable progress streaming.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(sentry): remove dead should_enable guard and add missing breadcrumb levels

The dsn_explicitly_set check was always True after the early return for
empty DSN, making should_enable always True and the gating block
unreachable dead code. Simplified to just a clear comment explaining
that DSN presence is sufficient to enable Sentry.

Also added missing level field to two safeBreadcrumb calls in PR review
handlers to match the established project convention.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(sentry): clean up dead code, sanitize stderr, and add follow-up review instrumentation

- Remove dead force_enable parameter from init_sentry() (no callers use it)
- Fix misleading SENTRY_DEV comment — Python backend no longer reads it
- Remove SENTRY_DEV pass-through from getSentryEnvForSubprocess()
- Add sanitizeForSentry() to redact potential secrets (tokens, API keys)
  from subprocess stderr before sending to Sentry
- Add safeBreadcrumb and safeCaptureException to follow-up review handler
  for parity with the initial review handler

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

apps/backend/core/sentry.py

@@ -186,14 +186,12 @@ def _before_send(event: dict, hint: dict) -> dict | None:
 
 def init_sentry(
     component: str = "backend",
-    force_enable: bool = False,
 ) -> bool:
     """
     Initialize Sentry for the Python backend.
 
     Args:
         component: Component name for tagging (e.g., "backend", "github-runner")
-        force_enable: Force enable even without packaged app detection
 
     Returns:
         True if Sentry was initialized, False otherwise
@@ -212,20 +210,11 @@ def init_sentry(
         logger.debug("[Sentry] No SENTRY_DSN configured - error reporting disabled")
         return False
 
-    # Check if we should enable Sentry
-    # Enable if:
-    # - Running from packaged app (detected by __compiled__ or frozen)
-    # - SENTRY_DEV=true is set
-    # - force_enable is True
+    # DSN is present (checked above), so Sentry should be enabled.
+    # The Electron main process only passes SENTRY_DSN to subprocesses in
+    # production builds, so its presence is sufficient to gate activation.
+    # In dev, set SENTRY_DSN in your environment to opt-in.
     is_packaged = getattr(sys, "frozen", False) or hasattr(sys, "__compiled__")
-    sentry_dev = os.environ.get("SENTRY_DEV", "").lower() in ("true", "1", "yes")
-    should_enable = is_packaged or sentry_dev or force_enable
-
-    if not should_enable:
-        logger.debug(
-            "[Sentry] Development mode - error reporting disabled (set SENTRY_DEV=true to enable)"
-        )
-        return False
 
     try:
         import sentry_sdk

apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts

@@ -36,6 +36,8 @@ import {
   buildRunnerArgs,
 } from "./utils/subprocess-runner";
 import { getPRStatusPoller } from "../../services/pr-status-poller";
+import { safeBreadcrumb, safeCaptureException } from "../../sentry";
+import { sanitizeForSentry } from "../../../shared/utils/sentry-privacy";
 import type {
   StartPollingRequest,
   StopPollingRequest,
@@ -1462,6 +1464,20 @@ async function runPRReview(
 
   debugLog("Spawning PR review process", { args, model, thinkingLevel });
 
+  safeBreadcrumb({
+    category: 'pr-review',
+    message: 'Spawning PR review subprocess',
+    level: 'info',
+    data: {
+      pythonPath: getPythonPath(backendPath),
+      runnerPath: getRunnerPath(backendPath),
+      cwd: backendPath,
+      model,
+      thinkingLevel,
+      prNumber,
+    },
+  });
+
   // Create log collector for this review
   const config = getGitHubConfig(project);
   const repo = config?.repo || project.name || "unknown";
@@ -1525,9 +1541,22 @@ async function runPRReview(
     // Wait for the process to complete
     const result = await promise;
 
+    safeBreadcrumb({
+      category: 'pr-review',
+      message: `PR review subprocess exited`,
+      level: result.success ? 'info' : 'error',
+      data: { exitCode: result.exitCode, success: result.success, prNumber },
+    });
+
     if (!result.success) {
       // Finalize logs with failure
       logCollector.finalize(false);
+
+      safeCaptureException(
+        new Error(`PR review subprocess failed: ${result.error ?? 'unknown error'}`),
+        { extra: { exitCode: result.exitCode, prNumber, stderr: sanitizeForSentry(result.stderr.slice(0, 500)) } }
+      );
+
       throw new Error(result.error ?? "Review failed");
     }
 
@@ -2907,6 +2936,20 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v
 
           debugLog("Spawning follow-up review process", { args, model, thinkingLevel });
 
+          safeBreadcrumb({
+            category: 'pr-review',
+            message: 'Spawning follow-up PR review subprocess',
+            level: 'info',
+            data: {
+              pythonPath: getPythonPath(backendPath),
+              runnerPath: getRunnerPath(backendPath),
+              cwd: backendPath,
+              model,
+              thinkingLevel,
+              prNumber,
+            },
+          });
+
           // Create log collector for this follow-up review (config already declared above)
           const repo = config?.repo || project.name || "unknown";
           const logCollector = new PRLogCollector(project, prNumber, repo, true, mainWindow);
@@ -2964,9 +3007,22 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v
 
             const result = await promise;
 
+            safeBreadcrumb({
+              category: 'pr-review',
+              message: 'Follow-up PR review subprocess exited',
+              level: result.success ? 'info' : 'error',
+              data: { exitCode: result.exitCode, success: result.success, prNumber },
+            });
+
             if (!result.success) {
               // Finalize logs with failure
               logCollector.finalize(false);
+
+              safeCaptureException(
+                new Error(`Follow-up PR review subprocess failed: ${result.error ?? 'unknown error'}`),
+                { extra: { exitCode: result.exitCode, prNumber, stderr: sanitizeForSentry(result.stderr.slice(0, 500)) } }
+              );
+
               throw new Error(result.error ?? "Follow-up review failed");
             }
 

apps/frontend/src/main/ipc-handlers/github/utils/runner-env.ts

@@ -3,6 +3,7 @@ import { getAPIProfileEnv } from '../../../services/profile';
 import { getBestAvailableProfileEnv } from '../../../rate-limit-detector';
 import { pythonEnvManager } from '../../../python-env-manager';
 import { getGitHubTokenForSubprocess } from '../utils';
+import { getSentryEnvForSubprocess } from '../../../sentry';
 
 /**
  * Get environment variables for Python runner subprocesses.
@@ -48,6 +49,7 @@ export async function getRunnerEnv(
     ...oauthModeClearVars,
     ...profileEnv,  // OAuth token from profile manager (fixes #563, rate-limit aware)
     ...githubEnv,  // Fresh GitHub token from gh CLI (fixes #151)
-    ...extraEnv,
+    ...getSentryEnvForSubprocess(),  // Sentry DSN + sample rates for Python subprocess
+    ...extraEnv,  // extraEnv last so callers can still override
   };
 }

apps/frontend/src/main/ipc-handlers/github/utils/subprocess-runner.ts

@@ -20,6 +20,7 @@ import { isWindows, isMacOS } from '../../../platform';
 import { getEffectiveSourcePath } from '../../../updater/path-resolver';
 import { pythonEnvManager, getConfiguredPythonPath } from '../../../python-env-manager';
 import { getTaskkillExePath, getWhereExePath } from '../../../utils/windows-paths';
+import { safeCaptureException } from '../../../sentry';
 
 const execAsync = promisify(exec);
 const execFileAsync = promisify(execFile);
@@ -214,6 +215,17 @@ export function runPythonSubprocess<T = unknown>(
     let killedDueToAuthFailure = false; // Track if subprocess was killed due to auth failure
     let billingFailureEmitted = false; // Track if we've already emitted a billing failure
     let killedDueToBillingFailure = false; // Track if subprocess was killed due to billing failure
+    let receivedOutput = false; // Track if any stdout/stderr has been received
+
+    // Health-check: report to Sentry if no output received within 120 seconds
+    const healthCheckTimeout = setTimeout(() => {
+      if (!receivedOutput) {
+        safeCaptureException(
+          new Error('[SubprocessRunner] No output received from subprocess after 120s'),
+          { extra: { pythonPath: options.pythonPath, args: options.args, cwd: options.cwd, envKeys: options.env ? Object.keys(options.env) : [] } }
+        );
+      }
+    }, 120_000);
 
     // Default progress pattern: [ 30%] message OR [30%] message
     const progressPattern = options.progressPattern ?? /\[\s*(\d+)%\]\s*(.+)/;
@@ -337,6 +349,7 @@ export function runPythonSubprocess<T = unknown>(
     };
 
     child.stdout.on('data', (data: Buffer) => {
+      receivedOutput = true;
       const text = data.toString('utf-8');
       stdout += text;
 
@@ -364,6 +377,7 @@ export function runPythonSubprocess<T = unknown>(
     });
 
     child.stderr.on('data', (data: Buffer) => {
+      receivedOutput = true;
       const text = data.toString('utf-8');
       stderr += text;
 
@@ -382,6 +396,7 @@ export function runPythonSubprocess<T = unknown>(
     });
 
     child.on('close', (code: number | null) => {
+      clearTimeout(healthCheckTimeout);
       // Treat null exit code (killed with SIGKILL) as failure, not success
       const exitCode = code ?? -1;
 
@@ -461,6 +476,7 @@ export function runPythonSubprocess<T = unknown>(
     });
 
     child.on('error', (err: Error) => {
+      clearTimeout(healthCheckTimeout);
       options.onError?.(err.message);
       resolve({
         success: false,

apps/frontend/src/main/python-env-manager.ts

@@ -753,6 +753,8 @@ if sys.version_info >= (3, 12):
       ...windowsEnv,
       // Don't write bytecode - not needed and avoids permission issues
       PYTHONDONTWRITEBYTECODE: '1',
+      // Force unbuffered stdout/stderr so progress updates reach Electron immediately
+      PYTHONUNBUFFERED: '1',
       // Use UTF-8 encoding
       PYTHONIOENCODING: 'utf-8',
       PYTHONUTF8: '1',

apps/frontend/src/main/sentry.ts

@@ -222,7 +222,5 @@ export function getSentryEnvForSubprocess(): Record<string, string> {
     SENTRY_DSN: dsn,
     SENTRY_TRACES_SAMPLE_RATE: String(getTracesSampleRate()),
     SENTRY_PROFILES_SAMPLE_RATE: String(getProfilesSampleRate()),
-    // Pass SENTRY_DEV so Python backend also enables Sentry in dev mode
-    ...(process.env.SENTRY_DEV ? { SENTRY_DEV: process.env.SENTRY_DEV } : {}),
   };
 }

apps/frontend/src/shared/utils/sentry-privacy.ts

@@ -74,6 +74,31 @@ export function maskUserPaths(text: string): string {
   return text;
 }
 
+/**
+ * Sanitize text for safe inclusion in Sentry reports.
+ * Masks user paths and redacts potential secrets (tokens, keys, credentials).
+ */
+export function sanitizeForSentry(text: string): string {
+  if (!text) return text;
+
+  text = maskUserPaths(text);
+
+  // Redact common secret patterns (API keys, tokens, auth headers)
+  // Bearer/token auth
+  text = text.replace(/\b(Bearer|token|Token)\s+[A-Za-z0-9\-_.]+/gi, '$1 [REDACTED]');
+  // API keys / secrets in key=value or key: value format
+  text = text.replace(
+    /\b(api[_-]?key|api[_-]?secret|auth[_-]?token|access[_-]?token|refresh[_-]?token|secret[_-]?key|password|credential|private[_-]?key)[=:]\s*\S+/gi,
+    '$1=[REDACTED]'
+  );
+  // Anthropic API key format
+  text = text.replace(/\bsk-ant-[A-Za-z0-9\-_]{20,}/g, '[REDACTED_KEY]');
+  // Generic long hex/base64 tokens (40+ chars, likely secrets)
+  text = text.replace(/\b[A-Za-z0-9+/]{40,}={0,2}\b/g, '[REDACTED_TOKEN]');
+
+  return text;
+}
+
 /**
  * Recursively mask paths in an object
  * Handles nested objects and arrays