auto-claude: 216-display-ongoing-pr-review-logs-in-progress (#1807)

* auto-claude: subtask-1-1 - Add 'in_progress_since' optional field to PRReviewResult

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

* auto-claude: subtask-1-2 - Return in_progress result from orchestrator skip logic

When BotDetector detects a review is already running, return a PRReviewResult
with overall_status='in_progress' and in_progress_since timestamp extracted
from BotDetector state. Critically, this result is NOT saved to disk to avoid
overwriting the partial result being written by the active review.

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

* auto-claude: subtask-2-1 - Add isExternalReview field to PRReviewState

Add 'isExternalReview' boolean field to PRReviewState interface (default false).
Add 'setExternalReviewInProgress' action that sets isReviewing=true and
isExternalReview=true with a startedAt timestamp. All existing actions
properly handle the new field.

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

* auto-claude: subtask-3-1 - Notify renderer when PR review is already in progress

Instead of silently returning when a review is already running, send a
progress message so the renderer can reconnect and display ongoing logs.

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

* auto-claude: subtask-3-2 - Handle backend 'in_progress' result after runPRReview

Add 'in_progress' to PRReviewResult.overallStatus type union. When
runPRReview returns an in_progress result (review already running
externally), send it as a completed event so the renderer can detect
it and activate external review polling instead of showing a misleading
"no issues found" state.

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

* auto-claude: subtask-4-1 - Detect in_progress review status and poll for completion

When the backend reports an already-running review (overallStatus === 'in_progress'),
the IPC listener now calls setExternalReviewInProgress() instead of setPRReviewResult().
This activates log polling automatically. A new completion-detection useEffect in
PRDetail polls getPRReview() every 3s to detect when the external review finishes.

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

* auto-claude: subtask-4-2 - Update ReviewStatusTree for external review messaging

- Add isExternalReview prop to ReviewStatusTreeProps
- Hide cancel button when review is running externally
- Show 'Review started in another session' label for external reviews
- Show 'External review detected' as status header for external reviews
- Pass isExternalReview from PRDetail to ReviewStatusTree

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

* auto-claude: subtask-4-3 - Add i18n translation keys for PR review in-progress states

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

* Fix PR review findings: stale polling, dead i18n keys, unwired field

- Fix critical bug: polling now compares reviewedAt vs startedAt to
  reject stale disk results from previous reviews (in-progress results
  are intentionally not saved to disk)
- Replace dynamic import with static import of usePRReviewStore via
  barrel export for consistency with rest of codebase
- Remove unused i18n keys (reviewInProgressStartedAgo,
  cannotCancelExternalReview) from en and fr locale files
- Wire up inProgressSince field in TypeScript interfaces and mapper
  so backend data is no longer silently dropped
- Add startedAt to useEffect dependency array

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

* Fix follow-up review findings: unreachable in_progress, polling timeout, timestamps

- Fix unreachable in_progress detection: Python runner now outputs
  __RESULT_JSON__ marker to stdout for in_progress results (which are
  not saved to disk), and onComplete parses stdout before falling back
  to disk read
- Add 30-minute polling timeout so external review polling doesn't run
  indefinitely if the external process crashes
- Add immediate first poll before setInterval to eliminate 3s delay
- Pass backend's inProgressSince timestamp to setExternalReviewInProgress
  instead of always using new Date(), preventing valid completed results
  from being rejected by the staleness check

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

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Andy
2026-02-13 14:57:29 +01:00
committed by GitHub
parent f1b8cd3a7a
commit a4870fa0c3
12 changed files with 216 additions and 16 deletions
+7
View File
@@ -572,6 +572,9 @@ class PRReviewResult:
) # IDs of posted findings
posted_at: str | None = None # Timestamp when findings were posted
# In-progress review tracking
in_progress_since: str | None = None # ISO timestamp when active review started
def to_dict(self) -> dict:
return {
"pr_number": self.pr_number,
@@ -603,6 +606,8 @@ class PRReviewResult:
"has_posted_findings": self.has_posted_findings,
"posted_finding_ids": self.posted_finding_ids,
"posted_at": self.posted_at,
# In-progress review tracking
"in_progress_since": self.in_progress_since,
}
@classmethod
@@ -650,6 +655,8 @@ class PRReviewResult:
has_posted_findings=data.get("has_posted_findings", False),
posted_finding_ids=data.get("posted_finding_ids", []),
posted_at=data.get("posted_at"),
# In-progress review tracking
in_progress_since=data.get("in_progress_since"),
)
async def save(self, github_dir: Path) -> None:
+21 -1
View File
@@ -395,8 +395,28 @@ class GitHubOrchestrator:
else:
# No existing review found, create skip result
return await self._create_skip_result(pr_number, skip_reason)
elif "Review already in progress" in skip_reason:
# Return an in-progress result WITHOUT saving to disk
# to avoid overwriting the partial result being written by the active review
started_at = self.bot_detector.state.in_progress_reviews.get(
str(pr_number)
)
safe_print(
f"[BOT DETECTION] Review in progress for PR #{pr_number} "
f"(started: {started_at})",
flush=True,
)
return PRReviewResult(
pr_number=pr_number,
repo=self.config.repo,
success=True,
findings=[],
summary="Review in progress",
overall_status="in_progress",
in_progress_since=started_at,
)
else:
# For other skip reasons (bot-authored, cooling off, in-progress), create a skip result
# For other skip reasons (bot-authored, cooling off), create a skip result
return await self._create_skip_result(pr_number, skip_reason)
# Mark review as started (prevents concurrent reviews)
+6
View File
@@ -235,6 +235,12 @@ async def cmd_review_pr(args) -> int:
safe_print(f"[DEBUG] review_pr returned, success={result.success}")
if result.success:
# For in_progress results (not saved to disk), output JSON so the frontend
# can parse it from stdout instead of relying on the disk file.
if result.overall_status == "in_progress":
safe_print(f"__RESULT_JSON__:{json.dumps(result.to_dict())}")
return 0
safe_print(f"\n{'=' * 60}")
safe_print(f"PR #{result.pr_number} Review Complete")
safe_print(f"{'=' * 60}")
@@ -278,7 +278,7 @@ export interface PRReviewResult {
success: boolean;
findings: PRReviewFinding[];
summary: string;
overallStatus: "approve" | "request_changes" | "comment";
overallStatus: "approve" | "request_changes" | "comment" | "in_progress";
reviewId?: number;
reviewedAt: string;
error?: string;
@@ -294,6 +294,8 @@ export interface PRReviewResult {
hasPostedFindings?: boolean;
postedFindingIds?: string[];
postedAt?: string;
// In-progress review tracking
inProgressSince?: string;
}
/**
@@ -1356,6 +1358,8 @@ function getReviewResult(project: Project, prNumber: number): PRReviewResult | n
hasPostedFindings: data.has_posted_findings ?? false,
postedFindingIds: data.posted_finding_ids ?? [],
postedAt: data.posted_at,
// In-progress review tracking
inProgressSince: data.in_progress_since,
};
} catch {
// File doesn't exist or couldn't be read
@@ -1514,7 +1518,32 @@ async function runPRReview(
debugLog("Auth failure detected in PR review", authFailureInfo);
mainWindow.webContents.send(IPC_CHANNELS.CLAUDE_AUTH_FAILURE, authFailureInfo);
},
onComplete: () => {
onComplete: (stdout: string) => {
// Check stdout for in_progress JSON marker (not saved to disk by backend)
const inProgressMarker = "__RESULT_JSON__:";
for (const line of stdout.split("\n")) {
if (line.startsWith(inProgressMarker)) {
try {
const data = JSON.parse(line.slice(inProgressMarker.length));
if (data.overall_status === "in_progress") {
debugLog("In-progress result parsed from stdout", { prNumber });
return {
prNumber: data.pr_number,
repo: data.repo,
success: data.success,
findings: [],
summary: data.summary ?? "",
overallStatus: "in_progress" as const,
reviewedAt: data.reviewed_at ?? new Date().toISOString(),
inProgressSince: data.in_progress_since,
};
}
} catch {
debugLog("Failed to parse __RESULT_JSON__ line", { line });
}
}
}
// Load the result from disk
const reviewResult = getReviewResult(project, prNumber);
if (!reviewResult) {
@@ -1864,9 +1893,15 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v
projectId
);
// Check if already running
// Check if already running — notify renderer so it can display ongoing logs
if (runningReviews.has(reviewKey)) {
debugLog("Review already running", { reviewKey });
debugLog("Review already running, notifying renderer", { reviewKey });
sendProgress({
phase: "analyzing",
prNumber,
progress: 50,
message: "Review is already in progress. Reconnecting to ongoing review...",
});
return;
}
@@ -1936,6 +1971,20 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v
const result = await runPRReview(project, prNumber, mainWindow);
if (result.overallStatus === "in_progress") {
// Review is already running externally (detected by BotDetector).
// Send the result as-is so the renderer can activate external review polling.
debugLog("PR review already in progress externally", { prNumber });
sendProgress({
phase: "complete",
prNumber,
progress: 100,
message: "Review already in progress",
});
sendComplete(result);
return;
}
debugLog("PR review completed", { prNumber, findingsCount: result.findings.length });
sendProgress({
phase: "complete",
@@ -387,7 +387,7 @@ export interface PRReviewResult {
success: boolean;
findings: PRReviewFinding[];
summary: string;
overallStatus: 'approve' | 'request_changes' | 'comment';
overallStatus: 'approve' | 'request_changes' | 'comment' | 'in_progress';
reviewId?: number;
reviewedAt: string;
error?: string;
@@ -403,6 +403,8 @@ export interface PRReviewResult {
hasPostedFindings?: boolean;
postedFindingIds?: string[];
postedAt?: string;
// In-progress review tracking
inProgressSince?: string;
}
/**
@@ -66,6 +66,7 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps)
reviewProgress,
startedAt,
isReviewing,
isExternalReview,
previousReviewResult,
hasMore,
selectPR,
@@ -269,6 +270,7 @@ export function GitHubPRs({ onOpenSettings, isActive = false }: GitHubPRsProps)
reviewProgress={reviewProgress}
startedAt={startedAt}
isReviewing={isReviewing}
isExternalReview={isExternalReview}
initialNewCommitsCheck={storedNewCommitsCheck}
isActive={isActive}
isLoadingFiles={isLoadingPRDetails}
@@ -35,6 +35,7 @@ import { PRLogs } from './PRLogs';
import type { PRData, PRReviewResult, PRReviewProgress } from '../hooks/useGitHubPRs';
import type { NewCommitsCheck, MergeReadiness, PRLogs as PRLogsType, WorkflowsAwaitingApprovalResult } from '../../../../preload/api/modules/github-api';
import { usePRReviewStore } from '../../../stores/github';
interface PRDetailProps {
pr: PRData;
@@ -44,6 +45,7 @@ interface PRDetailProps {
reviewProgress: PRReviewProgress | null;
startedAt: string | null;
isReviewing: boolean;
isExternalReview?: boolean;
initialNewCommitsCheck?: NewCommitsCheck | null;
isActive?: boolean;
isLoadingFiles?: boolean;
@@ -78,6 +80,7 @@ export function PRDetail({
reviewProgress,
startedAt,
isReviewing,
isExternalReview = false,
initialNewCommitsCheck,
isActive: _isActive = false,
isLoadingFiles = false,
@@ -398,6 +401,59 @@ export function PRDetail({
};
}, [isReviewing, onGetLogs]);
/**
* Completion detection for external (in-progress) reviews
*
* When the backend reports overallStatus === 'in_progress', the store sets
* isExternalReview = true and isReviewing = true. This effect polls the
* review result file every 3 seconds to detect when the external review
* finishes. Once a completed result is found (overallStatus !== 'in_progress'),
* we update the store which will set isReviewing = false and display the result.
*/
useEffect(() => {
if (!isReviewing || !isExternalReview) return;
const POLL_INTERVAL_MS = 3000;
const MAX_POLL_DURATION_MS = 30 * 60 * 1000; // 30 minutes
const pollStart = Date.now();
const pollForCompletion = async () => {
// Timeout: stop polling after 30 minutes to avoid indefinite polling
if (Date.now() - pollStart > MAX_POLL_DURATION_MS) {
usePRReviewStore.getState().setPRReviewResult(projectId, {
prNumber: pr.number,
repo: '',
success: false,
findings: [],
summary: '',
overallStatus: 'comment',
reviewedAt: new Date().toISOString(),
error: 'External review polling timed out after 30 minutes',
});
return;
}
try {
const result = await window.electronAPI.github.getPRReview(projectId, pr.number);
if (result && result.overallStatus !== 'in_progress') {
// Only accept results that were produced AFTER we detected the external review.
// Otherwise this is a stale result from a previous review still on disk
// (in-progress results are intentionally NOT saved to disk).
if (startedAt && result.reviewedAt && new Date(result.reviewedAt) > new Date(startedAt)) {
usePRReviewStore.getState().setPRReviewResult(projectId, result);
}
}
} catch {
// Ignore errors — transient file read failures shouldn't stop polling
}
};
// Poll immediately, then every 3 seconds
pollForCompletion();
const interval = setInterval(pollForCompletion, POLL_INTERVAL_MS);
return () => clearInterval(interval);
}, [isReviewing, isExternalReview, projectId, pr.number, startedAt]);
/**
* Fallback mechanism: Load logs after review completes if not already loaded
*
@@ -1067,6 +1123,7 @@ ${t('prReview.blockedStatusMessageFooter')}`;
<ReviewStatusTree
status={prStatus.status}
isReviewing={isReviewing}
isExternalReview={isExternalReview}
startedAt={startedAt}
reviewResult={reviewResult}
previousReviewResult={previousReviewResult}
@@ -21,6 +21,7 @@ export type ReviewStatus =
export interface ReviewStatusTreeProps {
status: ReviewStatus;
isReviewing: boolean;
isExternalReview?: boolean;
startedAt: string | null;
reviewResult: PRReviewResult | null;
previousReviewResult: PRReviewResult | null;
@@ -39,6 +40,7 @@ export interface ReviewStatusTreeProps {
export function ReviewStatusTree({
status,
isReviewing,
isExternalReview = false,
startedAt,
reviewResult,
previousReviewResult,
@@ -137,7 +139,9 @@ export function ReviewStatusTree({
if (isReviewing) {
steps.push({
id: 'analysis',
label: t('prReview.analysisInProgress'),
label: isExternalReview
? t('prReview.reviewStartedExternally')
: t('prReview.analysisInProgress'),
status: 'current',
date: null
});
@@ -255,7 +259,7 @@ export function ReviewStatusTree({
// Status label - explicitly handle all statuses
const getStatusLabel = (): string => {
if (isReviewing) return t('prReview.aiReviewInProgress');
if (isReviewing) return isExternalReview ? t('prReview.externalReviewDetected') : t('prReview.aiReviewInProgress');
switch (status) {
case 'ready_to_merge':
return t('prReview.readyToMerge');
@@ -279,7 +283,7 @@ export function ReviewStatusTree({
<CollapsibleCard
title={statusLabel}
icon={<div className={statusDotColor} />}
headerAction={isReviewing ? (
headerAction={isReviewing && !isExternalReview ? (
<Button
variant="ghost"
size="sm"
@@ -32,6 +32,7 @@ interface UseGitHubPRsResult {
reviewProgress: PRReviewProgress | null;
startedAt: string | null;
isReviewing: boolean;
isExternalReview: boolean;
previousReviewResult: PRReviewResult | null;
isConnected: boolean;
repoFullName: string | null;
@@ -113,6 +114,7 @@ export function useGitHubPRs(
const reviewResult = selectedPRReviewState?.result ?? null;
const reviewProgress = selectedPRReviewState?.progress ?? null;
const isReviewing = selectedPRReviewState?.isReviewing ?? false;
const isExternalReview = selectedPRReviewState?.isExternalReview ?? false;
const previousReviewResult = selectedPRReviewState?.previousResult ?? null;
const startedAt = selectedPRReviewState?.startedAt ?? null;
@@ -727,6 +729,7 @@ export function useGitHubPRs(
reviewProgress,
startedAt,
isReviewing,
isExternalReview,
previousReviewResult,
isConnected,
repoFullName,
@@ -35,6 +35,8 @@ interface PRReviewState {
mergeableState: MergeableState | null;
/** Timestamp of last status poll (ISO 8601 string) */
lastPolled: string | null;
/** Whether this review was initiated externally (e.g., from PR list) rather than from detail view */
isExternalReview: boolean;
}
interface PRReviewStoreState {
@@ -59,6 +61,8 @@ interface PRReviewStoreState {
}) => void;
/** Clear PR status fields for a specific PR */
clearPRStatus: (projectId: string, prNumber: number) => void;
/** Start an external review (from PR list) - sets isReviewing and isExternalReview */
setExternalReviewInProgress: (projectId: string, prNumber: number, inProgressSince?: string) => void;
// Selectors
getPRReviewState: (projectId: string, prNumber: number) => PRReviewState | null;
@@ -96,7 +100,8 @@ export const usePRReviewStore = create<PRReviewStoreState>((set, get) => ({
checksStatus: existing?.checksStatus ?? null,
reviewsStatus: existing?.reviewsStatus ?? null,
mergeableState: existing?.mergeableState ?? null,
lastPolled: existing?.lastPolled ?? null
lastPolled: existing?.lastPolled ?? null,
isExternalReview: false
}
}
};
@@ -130,7 +135,8 @@ export const usePRReviewStore = create<PRReviewStoreState>((set, get) => ({
checksStatus: existing?.checksStatus ?? null,
reviewsStatus: existing?.reviewsStatus ?? null,
mergeableState: existing?.mergeableState ?? null,
lastPolled: existing?.lastPolled ?? null
lastPolled: existing?.lastPolled ?? null,
isExternalReview: false
}
}
};
@@ -155,7 +161,8 @@ export const usePRReviewStore = create<PRReviewStoreState>((set, get) => ({
checksStatus: existing?.checksStatus ?? null,
reviewsStatus: existing?.reviewsStatus ?? null,
mergeableState: existing?.mergeableState ?? null,
lastPolled: existing?.lastPolled ?? null
lastPolled: existing?.lastPolled ?? null,
isExternalReview: existing?.isExternalReview ?? false
}
}
};
@@ -182,7 +189,8 @@ export const usePRReviewStore = create<PRReviewStoreState>((set, get) => ({
checksStatus: existing?.checksStatus ?? null,
reviewsStatus: existing?.reviewsStatus ?? null,
mergeableState: existing?.mergeableState ?? null,
lastPolled: existing?.lastPolled ?? null
lastPolled: existing?.lastPolled ?? null,
isExternalReview: existing?.isExternalReview ?? false
}
}
};
@@ -207,7 +215,8 @@ export const usePRReviewStore = create<PRReviewStoreState>((set, get) => ({
checksStatus: existing?.checksStatus ?? null,
reviewsStatus: existing?.reviewsStatus ?? null,
mergeableState: existing?.mergeableState ?? null,
lastPolled: existing?.lastPolled ?? null
lastPolled: existing?.lastPolled ?? null,
isExternalReview: existing?.isExternalReview ?? false
}
}
};
@@ -234,7 +243,8 @@ export const usePRReviewStore = create<PRReviewStoreState>((set, get) => ({
checksStatus: null,
reviewsStatus: null,
mergeableState: null,
lastPolled: null
lastPolled: null,
isExternalReview: false
}
}
};
@@ -282,7 +292,8 @@ export const usePRReviewStore = create<PRReviewStoreState>((set, get) => ({
checksStatus: status.checksStatus,
reviewsStatus: status.reviewsStatus,
mergeableState: status.mergeableState,
lastPolled: status.lastPolled
lastPolled: status.lastPolled,
isExternalReview: false
}
}
};
@@ -321,6 +332,32 @@ export const usePRReviewStore = create<PRReviewStoreState>((set, get) => ({
};
}),
setExternalReviewInProgress: (projectId: string, prNumber: number, inProgressSince?: string) => set((state) => {
const key = `${projectId}:${prNumber}`;
const existing = state.prReviews[key];
return {
prReviews: {
...state.prReviews,
[key]: {
prNumber,
projectId,
isReviewing: true,
startedAt: inProgressSince || new Date().toISOString(),
progress: null,
result: existing?.result ?? null,
previousResult: existing?.previousResult ?? null,
error: null,
newCommitsCheck: existing?.newCommitsCheck ?? null,
checksStatus: existing?.checksStatus ?? null,
reviewsStatus: existing?.reviewsStatus ?? null,
mergeableState: existing?.mergeableState ?? null,
lastPolled: existing?.lastPolled ?? null,
isExternalReview: true
}
}
};
}),
// Selectors
getPRReviewState: (projectId: string, prNumber: number) => {
const { prReviews } = get();
@@ -378,6 +415,15 @@ export function initializePRReviewListeners(): void {
// Listen for PR review completion events
const cleanupComplete = window.electronAPI.github.onPRReviewComplete(
(projectId: string, result: PRReviewResult) => {
// When the backend detects an already-running review (e.g., started from another
// client or the PR list), it returns overallStatus === 'in_progress' instead of
// a real result. Transition to external-review-in-progress so the log polling
// activates and the UI shows the ongoing review.
if (result.overallStatus === 'in_progress') {
store.setExternalReviewInProgress(projectId, result.prNumber, result.inProgressSince);
return;
}
store.setPRReviewResult(projectId, result);
// Trigger all registered refresh callbacks when review completes
refreshCallbacks.forEach(callback => {
@@ -292,6 +292,8 @@
"newIssue": "{{count}} new issue",
"newIssue_plural": "{{count}} new issues",
"reviewFailed": "Review Failed",
"externalReviewDetected": "External Review Detected",
"reviewStartedExternally": "This review was started from another session",
"description": "Description",
"noDescription": "No description provided.",
"followupReviewDetails": "Follow-up Review Details",
@@ -292,6 +292,8 @@
"newIssue": "{{count}} nouveau problème",
"newIssue_plural": "{{count}} nouveaux problèmes",
"reviewFailed": "Révision échouée",
"externalReviewDetected": "Révision externe détectée",
"reviewStartedExternally": "Cette révision a été lancée depuis une autre session",
"description": "Description",
"noDescription": "Aucune description fournie.",
"followupReviewDetails": "Détails de la révision de suivi",