fix: PR review error visibility and gh CLI resolution in bundled apps

- Surface review errors in UI instead of silently falling back to "Not Reviewed" - Thread reviewError from store through hook → GitHubPRs → PRDetail → ReviewStatusTree - Fix error payload to include prNumber so store updates correct PR key - Use CLI tool manager (getToolInfo) instead of `which gh` in validateGitHubModule so bundled Electron apps can find gh via Homebrew/augmented PATH - Pass GITHUB_CLI_PATH in subprocess env via getRunnerEnv - Use resolved gh path for `gh auth status` check - Add Sentry breadcrumbs and error capture for gh CLI resolution diagnostics - Add i18n keys for retryReview (en + fr) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-18 21:23:42 +01:00
parent 4a6df82792
commit 732fc1cd3f
10 changed files with 119 additions and 27 deletions
@@ -40,6 +40,8 @@ Auto Claude is a desktop application (+ CLI) where users describe a goal and AI

 **PR target** — Always target the `develop` branch for PRs to AndyMik90/Auto-Claude, NOT `main`.

+**No console.log for debugging production issues** — `console.log` output is not visible in bundled/packaged versions of the Electron app. Use Sentry for error tracking and diagnostics in production. Reserve `console.log` for development only.
+
 ## Work Approach

 **Investigate before speculating** — Always read the actual code before proposing root causes. Spawn agents to grep and read relevant source files before forming any hypothesis. Never guess at causes without evidence from the codebase.
@@ -1499,6 +1499,19 @@ async function runPRReview(
  // Build environment with project settings
  const subprocessEnv = await getRunnerEnv(getClaudeMdEnv(project));

+  safeBreadcrumb({
+    category: 'github.pr-review',
+    message: `Subprocess env for PR #${prNumber} review`,
+    level: 'info',
+    data: {
+      prNumber,
+      hasGITHUB_CLI_PATH: !!subprocessEnv.GITHUB_CLI_PATH,
+      GITHUB_CLI_PATH: subprocessEnv.GITHUB_CLI_PATH ?? 'NOT SET',
+      hasGITHUB_TOKEN: !!subprocessEnv.GITHUB_TOKEN,
+      hasPYTHONPATH: !!subprocessEnv.PYTHONPATH,
+    },
+  });
+
  // Create operation ID for this review
  const reviewKey = getReviewKey(project.id, prNumber);

@@ -2028,7 +2041,7 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v
        },
        projectId
      );
-      sendError(error instanceof Error ? error.message : "Failed to run PR review");
+      sendError({ prNumber, error: error instanceof Error ? error.message : "Failed to run PR review" });
    }
  });

@@ -3015,6 +3028,19 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v
          // Build environment with project settings
          const followupEnv = await getRunnerEnv(getClaudeMdEnv(project));

+          safeBreadcrumb({
+            category: 'github.pr-review',
+            message: `Subprocess env for PR #${prNumber} follow-up review`,
+            level: 'info',
+            data: {
+              prNumber,
+              hasGITHUB_CLI_PATH: !!followupEnv.GITHUB_CLI_PATH,
+              GITHUB_CLI_PATH: followupEnv.GITHUB_CLI_PATH ?? 'NOT SET',
+              hasGITHUB_TOKEN: !!followupEnv.GITHUB_TOKEN,
+              hasPYTHONPATH: !!followupEnv.PYTHONPATH,
+            },
+          });
+
          const { process: childProcess, promise } = runPythonSubprocess<PRReviewResult>({
            pythonPath: getPythonPath(backendPath),
            args,
@@ -3,7 +3,8 @@ 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';
+import { getSentryEnvForSubprocess, safeBreadcrumb } from '../../../sentry';
+import { getToolInfo } from '../../../cli-tool-manager';

 /**
 * Get environment variables for Python runner subprocesses.
@@ -43,12 +44,30 @@ export async function getRunnerEnv(
  const githubToken = await getGitHubTokenForSubprocess();
  const githubEnv: Record<string, string> = githubToken ? { GITHUB_TOKEN: githubToken } : {};

+  // Resolve gh CLI path so Python subprocess can find it in bundled apps
+  // (bundled Electron apps have a stripped PATH that doesn't include Homebrew etc.)
+  const ghInfo = getToolInfo('gh');
+  const ghCliEnv: Record<string, string> = ghInfo.found && ghInfo.path ? { GITHUB_CLI_PATH: ghInfo.path } : {};
+  safeBreadcrumb({
+    category: 'github.runner-env',
+    message: `gh CLI for subprocess: found=${ghInfo.found}, path=${ghInfo.path ?? 'none'}, source=${ghInfo.source ?? 'none'}`,
+    level: ghInfo.found ? 'info' : 'warning',
+    data: {
+      found: ghInfo.found,
+      path: ghInfo.path ?? null,
+      source: ghInfo.source ?? null,
+      willSetGITHUB_CLI_PATH: !!(ghInfo.found && ghInfo.path),
+      hasGITHUB_TOKEN: !!githubToken,
+    },
+  });
+
  return {
    ...pythonEnv,  // Python environment including PYTHONPATH (fixes #139)
    ...apiProfileEnv,
    ...oauthModeClearVars,
    ...profileEnv,  // OAuth token from profile manager (fixes #563, rate-limit aware)
    ...githubEnv,  // Fresh GitHub token from gh CLI (fixes #151)
+    ...ghCliEnv,  // gh CLI path for bundled apps (Python backend uses GITHUB_CLI_PATH)
    ...getSentryEnvForSubprocess(),  // Sentry DSN + sample rates for Python subprocess
    ...extraEnv,  // extraEnv last so callers can still override
  };
@@ -20,7 +20,8 @@ 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';
+import { safeCaptureException, safeBreadcrumb } from '../../../sentry';
+import { getToolInfo } from '../../../cli-tool-manager';

 const execAsync = promisify(exec);
 const execFileAsync = promisify(execFile);
@@ -559,6 +560,7 @@ export interface GitHubModuleValidation {
  pythonEnvValid: boolean;
  error?: string;
  backendPath?: string;
+  ghCliPath?: string;
 }

 /**
@@ -622,33 +624,36 @@ export async function validateGitHubModule(project: Project): Promise<GitHubModu
    return result;
  }

-  // 2. Check gh CLI installation (cross-platform)
-  try {
-    if (isWindows()) {
-      await execFileAsync(getWhereExePath(), ['gh'], { timeout: 5000 });
-    } else {
-      await execAsync('which gh');
-    }
+  // 2. Check gh CLI installation (uses CLI tool manager for bundled app compatibility)
+  const ghInfo = getToolInfo('gh');
+  safeBreadcrumb({
+    category: 'github.validation',
+    message: `gh CLI lookup: found=${ghInfo.found}, path=${ghInfo.path ?? 'none'}, source=${ghInfo.source ?? 'none'}`,
+    level: ghInfo.found ? 'info' : 'warning',
+    data: { found: ghInfo.found, path: ghInfo.path ?? null, source: ghInfo.source ?? null },
+  });
+  if (ghInfo.found && ghInfo.path) {
    result.ghCliInstalled = true;
-  } catch (error: unknown) {
+    result.ghCliPath = ghInfo.path;
+  } else {
    result.ghCliInstalled = false;
-    const errCode = (error as NodeJS.ErrnoException).code;
-    if (errCode === 'ENOENT' && isWindows()) {
-      result.error = `System utility 'where.exe' not found. Check Windows installation.`;
-    } else {
-      const installInstructions = isWindows()
-        ? 'winget install --id GitHub.cli'
-        : isMacOS()
-          ? 'brew install gh'
-          : 'See https://cli.github.com/';
-      result.error = `GitHub CLI (gh) is not installed. Install it with:\n  ${installInstructions}`;
-    }
+    const installInstructions = isWindows()
+      ? 'winget install --id GitHub.cli'
+      : isMacOS()
+        ? 'brew install gh'
+        : 'See https://cli.github.com/';
+    result.error = `GitHub CLI (gh) is not installed. Install it with:\n  ${installInstructions}`;
+    safeCaptureException(new Error('gh CLI not found in bundled app'), {
+      tags: { component: 'github-validation' },
+      extra: { ghInfo, isPackaged: require('electron').app?.isPackaged ?? 'unknown' },
+    });
    return result;
  }

-  // 3. Check gh authentication
+  // 3. Check gh authentication (use resolved path for bundled app compatibility)
  try {
-    await execAsync('gh auth status 2>&1');
+    const ghPath = result.ghCliPath || 'gh';
+    await execAsync(`"${ghPath}" auth status 2>&1`);
    result.ghAuthenticated = true;
  } catch (error: any) {
    // gh auth status returns non-zero when not authenticated
@@ -68,6 +68,7 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps)
    isReviewing,
    isExternalReview,
    previousReviewResult,
+    reviewError,
    hasMore,
    selectPR,
    runReview,
@@ -271,6 +272,7 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps)
              startedAt={startedAt}
              isReviewing={isReviewing}
              isExternalReview={isExternalReview}
+              reviewError={reviewError}
              initialNewCommitsCheck={storedNewCommitsCheck}
              isActive={isActive}
              isLoadingFiles={isLoadingPRDetails}
@@ -46,6 +46,7 @@ interface PRDetailProps {
  startedAt: string | null;
  isReviewing: boolean;
  isExternalReview?: boolean;
+  reviewError?: string | null;
  initialNewCommitsCheck?: NewCommitsCheck | null;
  isActive?: boolean;
  isLoadingFiles?: boolean;
@@ -81,6 +82,7 @@ export function PRDetail({
  startedAt,
  isReviewing,
  isExternalReview = false,
+  reviewError: reviewErrorProp,
  initialNewCommitsCheck,
  isActive: _isActive = false,
  isLoadingFiles = false,
@@ -721,6 +723,16 @@ export function PRDetail({
      };
    }

+    if (reviewErrorProp && !reviewResult?.success) {
+      return {
+        status: 'not_reviewed',
+        label: t('prReview.reviewFailed'),
+        description: reviewErrorProp,
+        icon: <AlertCircle className="h-5 w-5" />,
+        color: 'bg-destructive/20 text-destructive border-destructive/50',
+      };
+    }
+
    if (!reviewResult || !reviewResult.success) {
      return {
        status: 'not_reviewed',
@@ -863,7 +875,7 @@ export function PRDetail({
      icon: <MessageSquare className="h-5 w-5" />,
      color: 'bg-primary/20 text-primary border-primary/50',
    };
-  }, [isReviewing, reviewProgress, reviewResult, postedFindingIds, isReadyToMerge, newCommitsCheck, t]);
+  }, [isReviewing, reviewProgress, reviewResult, reviewErrorProp, postedFindingIds, isReadyToMerge, newCommitsCheck, t]);

  const handlePostReview = async () => {
    const idsToPost = Array.from(selectedFindingIds);
@@ -1128,6 +1140,7 @@ ${t('prReview.blockedStatusMessageFooter')}`;
          reviewResult={reviewResult}
          previousReviewResult={previousReviewResult}
          postedCount={new Set([...postedFindingIds, ...(reviewResult?.postedFindingIds ?? [])]).size}
+          reviewError={reviewErrorProp}
          onRunReview={onRunReview}
          onRunFollowupReview={onRunFollowupReview}
          onCancelReview={onCancelReview}
@@ -1,5 +1,5 @@
 import { useState } from 'react';
-import { CheckCircle, Circle, CircleDot, Play, RefreshCw } from 'lucide-react';
+import { AlertCircle, CheckCircle, Circle, CircleDot, Play, RefreshCw } from 'lucide-react';
 import { useTranslation } from 'react-i18next';
 import { Button } from '../../ui/button';
 import { cn } from '../../../lib/utils';
@@ -26,6 +26,7 @@ export interface ReviewStatusTreeProps {
  reviewResult: PRReviewResult | null;
  previousReviewResult: PRReviewResult | null;
  postedCount: number;
+  reviewError?: string | null;
  onRunReview: () => void;
  onRunFollowupReview: () => void;
  onCancelReview: () => void;
@@ -45,6 +46,7 @@ export function ReviewStatusTree({
  reviewResult,
  previousReviewResult,
  postedCount,
+  reviewError,
  onRunReview,
  onRunFollowupReview,
  onCancelReview,
@@ -57,8 +59,26 @@ export function ReviewStatusTree({
  // Determine if this is a follow-up review in progress (for edge case handling)
  const isFollowupInProgress = isReviewing && (previousReviewResult !== null || reviewResult?.isFollowupReview);

-  // If not reviewed, show simple status
+  // If not reviewed, show simple status (with error if present)
  if (status === 'not_reviewed' && !isReviewing) {
+    if (reviewError) {
+      return (
+        <div className="flex flex-wrap items-center justify-between gap-y-3 p-4 border rounded-lg bg-card shadow-sm border-destructive/30">
+          <div className="flex items-center gap-3 min-w-0">
+            <div className="h-2.5 w-2.5 shrink-0 rounded-full bg-destructive" />
+            <div className="min-w-0">
+              <span className="font-medium text-destructive truncate block">{t('prReview.reviewFailed')}</span>
+              <span className="text-xs text-muted-foreground truncate block mt-0.5">{reviewError}</span>
+            </div>
+          </div>
+          <Button onClick={onRunReview} size="sm" variant="outline" className="gap-2 shrink-0 ml-auto sm:ml-0">
+            <RefreshCw className="h-3.5 w-3.5" />
+            {t('prReview.retryReview')}
+          </Button>
+        </div>
+      );
+    }
+
    return (
      <div className="flex flex-wrap items-center justify-between gap-y-3 p-4 border rounded-lg bg-card shadow-sm">
        <div className="flex items-center gap-3 min-w-0">
@@ -34,6 +34,7 @@ interface UseGitHubPRsResult {
  isReviewing: boolean;
  isExternalReview: boolean;
  previousReviewResult: PRReviewResult | null;
+  reviewError: string | null;
  isConnected: boolean;
  repoFullName: string | null;
  activePRReviews: number[]; // PR numbers currently being reviewed
@@ -117,6 +118,7 @@ export function useGitHubPRs(
  const isExternalReview = selectedPRReviewState?.isExternalReview ?? false;
  const previousReviewResult = selectedPRReviewState?.previousResult ?? null;
  const startedAt = selectedPRReviewState?.startedAt ?? null;
+  const reviewError = selectedPRReviewState?.error ?? null;

  // Get list of PR numbers currently being reviewed
  const activePRReviews = useMemo(() => {
@@ -731,6 +733,7 @@ export function useGitHubPRs(
    isReviewing,
    isExternalReview,
    previousReviewResult,
+    reviewError,
    isConnected,
    repoFullName,
    activePRReviews,
@@ -362,6 +362,7 @@
    "followup": "Follow-up",
    "initial": "Initial",
    "rerunFollowup": "Re-run follow-up review",
+    "retryReview": "Retry Review",
    "rerunReview": "Re-run review",
    "updateBranch": "Update Branch",
    "updatingBranch": "Updating...",
@@ -371,6 +371,7 @@
    "followup": "Suivi",
    "initial": "Initial",
    "rerunFollowup": "Relancer la revue de suivi",
+    "retryReview": "Réessayer la revue",
    "rerunReview": "Relancer la revue",
    "updateBranch": "Mettre à jour la branche",
    "updatingBranch": "Mise à jour...",