Skip to content

Commit 732fc1c

Browse files
AndyMik90claude
andcommitted
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>
1 parent 4a6df82 commit 732fc1c

10 files changed

Lines changed: 119 additions & 27 deletions

File tree

CLAUDE.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ Auto Claude is a desktop application (+ CLI) where users describe a goal and AI
4040

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

43+
**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.
44+
4345
## Work Approach
4446

4547
**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.

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

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1499,6 +1499,19 @@ async function runPRReview(
14991499
// Build environment with project settings
15001500
const subprocessEnv = await getRunnerEnv(getClaudeMdEnv(project));
15011501

1502+
safeBreadcrumb({
1503+
category: 'github.pr-review',
1504+
message: `Subprocess env for PR #${prNumber} review`,
1505+
level: 'info',
1506+
data: {
1507+
prNumber,
1508+
hasGITHUB_CLI_PATH: !!subprocessEnv.GITHUB_CLI_PATH,
1509+
GITHUB_CLI_PATH: subprocessEnv.GITHUB_CLI_PATH ?? 'NOT SET',
1510+
hasGITHUB_TOKEN: !!subprocessEnv.GITHUB_TOKEN,
1511+
hasPYTHONPATH: !!subprocessEnv.PYTHONPATH,
1512+
},
1513+
});
1514+
15021515
// Create operation ID for this review
15031516
const reviewKey = getReviewKey(project.id, prNumber);
15041517

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

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

3031+
safeBreadcrumb({
3032+
category: 'github.pr-review',
3033+
message: `Subprocess env for PR #${prNumber} follow-up review`,
3034+
level: 'info',
3035+
data: {
3036+
prNumber,
3037+
hasGITHUB_CLI_PATH: !!followupEnv.GITHUB_CLI_PATH,
3038+
GITHUB_CLI_PATH: followupEnv.GITHUB_CLI_PATH ?? 'NOT SET',
3039+
hasGITHUB_TOKEN: !!followupEnv.GITHUB_TOKEN,
3040+
hasPYTHONPATH: !!followupEnv.PYTHONPATH,
3041+
},
3042+
});
3043+
30183044
const { process: childProcess, promise } = runPythonSubprocess<PRReviewResult>({
30193045
pythonPath: getPythonPath(backendPath),
30203046
args,

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

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ import { getAPIProfileEnv } from '../../../services/profile';
33
import { getBestAvailableProfileEnv } from '../../../rate-limit-detector';
44
import { pythonEnvManager } from '../../../python-env-manager';
55
import { getGitHubTokenForSubprocess } from '../utils';
6-
import { getSentryEnvForSubprocess } from '../../../sentry';
6+
import { getSentryEnvForSubprocess, safeBreadcrumb } from '../../../sentry';
7+
import { getToolInfo } from '../../../cli-tool-manager';
78

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

47+
// Resolve gh CLI path so Python subprocess can find it in bundled apps
48+
// (bundled Electron apps have a stripped PATH that doesn't include Homebrew etc.)
49+
const ghInfo = getToolInfo('gh');
50+
const ghCliEnv: Record<string, string> = ghInfo.found && ghInfo.path ? { GITHUB_CLI_PATH: ghInfo.path } : {};
51+
safeBreadcrumb({
52+
category: 'github.runner-env',
53+
message: `gh CLI for subprocess: found=${ghInfo.found}, path=${ghInfo.path ?? 'none'}, source=${ghInfo.source ?? 'none'}`,
54+
level: ghInfo.found ? 'info' : 'warning',
55+
data: {
56+
found: ghInfo.found,
57+
path: ghInfo.path ?? null,
58+
source: ghInfo.source ?? null,
59+
willSetGITHUB_CLI_PATH: !!(ghInfo.found && ghInfo.path),
60+
hasGITHUB_TOKEN: !!githubToken,
61+
},
62+
});
63+
4664
return {
4765
...pythonEnv, // Python environment including PYTHONPATH (fixes #139)
4866
...apiProfileEnv,
4967
...oauthModeClearVars,
5068
...profileEnv, // OAuth token from profile manager (fixes #563, rate-limit aware)
5169
...githubEnv, // Fresh GitHub token from gh CLI (fixes #151)
70+
...ghCliEnv, // gh CLI path for bundled apps (Python backend uses GITHUB_CLI_PATH)
5271
...getSentryEnvForSubprocess(), // Sentry DSN + sample rates for Python subprocess
5372
...extraEnv, // extraEnv last so callers can still override
5473
};

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

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@ import { isWindows, isMacOS } from '../../../platform';
2020
import { getEffectiveSourcePath } from '../../../updater/path-resolver';
2121
import { pythonEnvManager, getConfiguredPythonPath } from '../../../python-env-manager';
2222
import { getTaskkillExePath, getWhereExePath } from '../../../utils/windows-paths';
23-
import { safeCaptureException } from '../../../sentry';
23+
import { safeCaptureException, safeBreadcrumb } from '../../../sentry';
24+
import { getToolInfo } from '../../../cli-tool-manager';
2425

2526
const execAsync = promisify(exec);
2627
const execFileAsync = promisify(execFile);
@@ -559,6 +560,7 @@ export interface GitHubModuleValidation {
559560
pythonEnvValid: boolean;
560561
error?: string;
561562
backendPath?: string;
563+
ghCliPath?: string;
562564
}
563565

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

625-
// 2. Check gh CLI installation (cross-platform)
626-
try {
627-
if (isWindows()) {
628-
await execFileAsync(getWhereExePath(), ['gh'], { timeout: 5000 });
629-
} else {
630-
await execAsync('which gh');
631-
}
627+
// 2. Check gh CLI installation (uses CLI tool manager for bundled app compatibility)
628+
const ghInfo = getToolInfo('gh');
629+
safeBreadcrumb({
630+
category: 'github.validation',
631+
message: `gh CLI lookup: found=${ghInfo.found}, path=${ghInfo.path ?? 'none'}, source=${ghInfo.source ?? 'none'}`,
632+
level: ghInfo.found ? 'info' : 'warning',
633+
data: { found: ghInfo.found, path: ghInfo.path ?? null, source: ghInfo.source ?? null },
634+
});
635+
if (ghInfo.found && ghInfo.path) {
632636
result.ghCliInstalled = true;
633-
} catch (error: unknown) {
637+
result.ghCliPath = ghInfo.path;
638+
} else {
634639
result.ghCliInstalled = false;
635-
const errCode = (error as NodeJS.ErrnoException).code;
636-
if (errCode === 'ENOENT' && isWindows()) {
637-
result.error = `System utility 'where.exe' not found. Check Windows installation.`;
638-
} else {
639-
const installInstructions = isWindows()
640-
? 'winget install --id GitHub.cli'
641-
: isMacOS()
642-
? 'brew install gh'
643-
: 'See https://cli.github.com/';
644-
result.error = `GitHub CLI (gh) is not installed. Install it with:\n ${installInstructions}`;
645-
}
640+
const installInstructions = isWindows()
641+
? 'winget install --id GitHub.cli'
642+
: isMacOS()
643+
? 'brew install gh'
644+
: 'See https://cli.github.com/';
645+
result.error = `GitHub CLI (gh) is not installed. Install it with:\n ${installInstructions}`;
646+
safeCaptureException(new Error('gh CLI not found in bundled app'), {
647+
tags: { component: 'github-validation' },
648+
extra: { ghInfo, isPackaged: require('electron').app?.isPackaged ?? 'unknown' },
649+
});
646650
return result;
647651
}
648652

649-
// 3. Check gh authentication
653+
// 3. Check gh authentication (use resolved path for bundled app compatibility)
650654
try {
651-
await execAsync('gh auth status 2>&1');
655+
const ghPath = result.ghCliPath || 'gh';
656+
await execAsync(`"${ghPath}" auth status 2>&1`);
652657
result.ghAuthenticated = true;
653658
} catch (error: any) {
654659
// gh auth status returns non-zero when not authenticated

apps/frontend/src/renderer/components/github-prs/GitHubPRs.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps)
6868
isReviewing,
6969
isExternalReview,
7070
previousReviewResult,
71+
reviewError,
7172
hasMore,
7273
selectPR,
7374
runReview,
@@ -271,6 +272,7 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps)
271272
startedAt={startedAt}
272273
isReviewing={isReviewing}
273274
isExternalReview={isExternalReview}
275+
reviewError={reviewError}
274276
initialNewCommitsCheck={storedNewCommitsCheck}
275277
isActive={isActive}
276278
isLoadingFiles={isLoadingPRDetails}

apps/frontend/src/renderer/components/github-prs/components/PRDetail.tsx

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ interface PRDetailProps {
4646
startedAt: string | null;
4747
isReviewing: boolean;
4848
isExternalReview?: boolean;
49+
reviewError?: string | null;
4950
initialNewCommitsCheck?: NewCommitsCheck | null;
5051
isActive?: boolean;
5152
isLoadingFiles?: boolean;
@@ -81,6 +82,7 @@ export function PRDetail({
8182
startedAt,
8283
isReviewing,
8384
isExternalReview = false,
85+
reviewError: reviewErrorProp,
8486
initialNewCommitsCheck,
8587
isActive: _isActive = false,
8688
isLoadingFiles = false,
@@ -721,6 +723,16 @@ export function PRDetail({
721723
};
722724
}
723725

726+
if (reviewErrorProp && !reviewResult?.success) {
727+
return {
728+
status: 'not_reviewed',
729+
label: t('prReview.reviewFailed'),
730+
description: reviewErrorProp,
731+
icon: <AlertCircle className="h-5 w-5" />,
732+
color: 'bg-destructive/20 text-destructive border-destructive/50',
733+
};
734+
}
735+
724736
if (!reviewResult || !reviewResult.success) {
725737
return {
726738
status: 'not_reviewed',
@@ -863,7 +875,7 @@ export function PRDetail({
863875
icon: <MessageSquare className="h-5 w-5" />,
864876
color: 'bg-primary/20 text-primary border-primary/50',
865877
};
866-
}, [isReviewing, reviewProgress, reviewResult, postedFindingIds, isReadyToMerge, newCommitsCheck, t]);
878+
}, [isReviewing, reviewProgress, reviewResult, reviewErrorProp, postedFindingIds, isReadyToMerge, newCommitsCheck, t]);
867879

868880
const handlePostReview = async () => {
869881
const idsToPost = Array.from(selectedFindingIds);
@@ -1128,6 +1140,7 @@ ${t('prReview.blockedStatusMessageFooter')}`;
11281140
reviewResult={reviewResult}
11291141
previousReviewResult={previousReviewResult}
11301142
postedCount={new Set([...postedFindingIds, ...(reviewResult?.postedFindingIds ?? [])]).size}
1143+
reviewError={reviewErrorProp}
11311144
onRunReview={onRunReview}
11321145
onRunFollowupReview={onRunFollowupReview}
11331146
onCancelReview={onCancelReview}

apps/frontend/src/renderer/components/github-prs/components/ReviewStatusTree.tsx

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { useState } from 'react';
2-
import { CheckCircle, Circle, CircleDot, Play, RefreshCw } from 'lucide-react';
2+
import { AlertCircle, CheckCircle, Circle, CircleDot, Play, RefreshCw } from 'lucide-react';
33
import { useTranslation } from 'react-i18next';
44
import { Button } from '../../ui/button';
55
import { cn } from '../../../lib/utils';
@@ -26,6 +26,7 @@ export interface ReviewStatusTreeProps {
2626
reviewResult: PRReviewResult | null;
2727
previousReviewResult: PRReviewResult | null;
2828
postedCount: number;
29+
reviewError?: string | null;
2930
onRunReview: () => void;
3031
onRunFollowupReview: () => void;
3132
onCancelReview: () => void;
@@ -45,6 +46,7 @@ export function ReviewStatusTree({
4546
reviewResult,
4647
previousReviewResult,
4748
postedCount,
49+
reviewError,
4850
onRunReview,
4951
onRunFollowupReview,
5052
onCancelReview,
@@ -57,8 +59,26 @@ export function ReviewStatusTree({
5759
// Determine if this is a follow-up review in progress (for edge case handling)
5860
const isFollowupInProgress = isReviewing && (previousReviewResult !== null || reviewResult?.isFollowupReview);
5961

60-
// If not reviewed, show simple status
62+
// If not reviewed, show simple status (with error if present)
6163
if (status === 'not_reviewed' && !isReviewing) {
64+
if (reviewError) {
65+
return (
66+
<div className="flex flex-wrap items-center justify-between gap-y-3 p-4 border rounded-lg bg-card shadow-sm border-destructive/30">
67+
<div className="flex items-center gap-3 min-w-0">
68+
<div className="h-2.5 w-2.5 shrink-0 rounded-full bg-destructive" />
69+
<div className="min-w-0">
70+
<span className="font-medium text-destructive truncate block">{t('prReview.reviewFailed')}</span>
71+
<span className="text-xs text-muted-foreground truncate block mt-0.5">{reviewError}</span>
72+
</div>
73+
</div>
74+
<Button onClick={onRunReview} size="sm" variant="outline" className="gap-2 shrink-0 ml-auto sm:ml-0">
75+
<RefreshCw className="h-3.5 w-3.5" />
76+
{t('prReview.retryReview')}
77+
</Button>
78+
</div>
79+
);
80+
}
81+
6282
return (
6383
<div className="flex flex-wrap items-center justify-between gap-y-3 p-4 border rounded-lg bg-card shadow-sm">
6484
<div className="flex items-center gap-3 min-w-0">

apps/frontend/src/renderer/components/github-prs/hooks/useGitHubPRs.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ interface UseGitHubPRsResult {
3434
isReviewing: boolean;
3535
isExternalReview: boolean;
3636
previousReviewResult: PRReviewResult | null;
37+
reviewError: string | null;
3738
isConnected: boolean;
3839
repoFullName: string | null;
3940
activePRReviews: number[]; // PR numbers currently being reviewed
@@ -117,6 +118,7 @@ export function useGitHubPRs(
117118
const isExternalReview = selectedPRReviewState?.isExternalReview ?? false;
118119
const previousReviewResult = selectedPRReviewState?.previousResult ?? null;
119120
const startedAt = selectedPRReviewState?.startedAt ?? null;
121+
const reviewError = selectedPRReviewState?.error ?? null;
120122

121123
// Get list of PR numbers currently being reviewed
122124
const activePRReviews = useMemo(() => {
@@ -731,6 +733,7 @@ export function useGitHubPRs(
731733
isReviewing,
732734
isExternalReview,
733735
previousReviewResult,
736+
reviewError,
734737
isConnected,
735738
repoFullName,
736739
activePRReviews,

apps/frontend/src/shared/i18n/locales/en/common.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,7 @@
362362
"followup": "Follow-up",
363363
"initial": "Initial",
364364
"rerunFollowup": "Re-run follow-up review",
365+
"retryReview": "Retry Review",
365366
"rerunReview": "Re-run review",
366367
"updateBranch": "Update Branch",
367368
"updatingBranch": "Updating...",

apps/frontend/src/shared/i18n/locales/fr/common.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,7 @@
371371
"followup": "Suivi",
372372
"initial": "Initial",
373373
"rerunFollowup": "Relancer la revue de suivi",
374+
"retryReview": "Réessayer la revue",
374375
"rerunReview": "Relancer la revue",
375376
"updateBranch": "Mettre à jour la branche",
376377
"updatingBranch": "Mise à jour...",

0 commit comments

Comments
 (0)