diff --git a/apps/backend/core/sentry.py b/apps/backend/core/sentry.py index bd3414ad..453a246e 100644 --- a/apps/backend/core/sentry.py +++ b/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 diff --git a/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts b/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts index 9bb10ca7..746d474f 100644 --- a/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts +++ b/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"); } diff --git a/apps/frontend/src/main/ipc-handlers/github/utils/runner-env.ts b/apps/frontend/src/main/ipc-handlers/github/utils/runner-env.ts index a57bd2d7..a93bbb31 100644 --- a/apps/frontend/src/main/ipc-handlers/github/utils/runner-env.ts +++ b/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 }; } diff --git a/apps/frontend/src/main/ipc-handlers/github/utils/subprocess-runner.ts b/apps/frontend/src/main/ipc-handlers/github/utils/subprocess-runner.ts index 6bdf2338..16c17718 100644 --- a/apps/frontend/src/main/ipc-handlers/github/utils/subprocess-runner.ts +++ b/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( 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( }; child.stdout.on('data', (data: Buffer) => { + receivedOutput = true; const text = data.toString('utf-8'); stdout += text; @@ -364,6 +377,7 @@ export function runPythonSubprocess( }); child.stderr.on('data', (data: Buffer) => { + receivedOutput = true; const text = data.toString('utf-8'); stderr += text; @@ -382,6 +396,7 @@ export function runPythonSubprocess( }); 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( }); child.on('error', (err: Error) => { + clearTimeout(healthCheckTimeout); options.onError?.(err.message); resolve({ success: false, diff --git a/apps/frontend/src/main/python-env-manager.ts b/apps/frontend/src/main/python-env-manager.ts index d7f91c1b..5e98c959 100644 --- a/apps/frontend/src/main/python-env-manager.ts +++ b/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', diff --git a/apps/frontend/src/main/sentry.ts b/apps/frontend/src/main/sentry.ts index ad068d98..476933cd 100644 --- a/apps/frontend/src/main/sentry.ts +++ b/apps/frontend/src/main/sentry.ts @@ -222,7 +222,5 @@ export function getSentryEnvForSubprocess(): Record { 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 } : {}), }; } diff --git a/apps/frontend/src/shared/utils/sentry-privacy.ts b/apps/frontend/src/shared/utils/sentry-privacy.ts index 56be7ead..c6d9c5e2 100644 --- a/apps/frontend/src/shared/utils/sentry-privacy.ts +++ b/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