From 45eebe960f0a03fc2ba5617d6f3d1cb26bc4368d Mon Sep 17 00:00:00 2001 From: AndyMik90 Date: Sat, 14 Feb 2026 00:39:04 +0100 Subject: [PATCH 01/15] auto-claude: subtask-1-1 - Create XState v5 PR review state machine and state utils Co-Authored-By: Claude Opus 4.6 --- .../src/shared/state-machines/index.ts | 9 + .../state-machines/pr-review-machine.ts | 177 ++++++++++++++++++ .../state-machines/pr-review-state-utils.ts | 52 +++++ 3 files changed, 238 insertions(+) create mode 100644 apps/frontend/src/shared/state-machines/pr-review-machine.ts create mode 100644 apps/frontend/src/shared/state-machines/pr-review-state-utils.ts diff --git a/apps/frontend/src/shared/state-machines/index.ts b/apps/frontend/src/shared/state-machines/index.ts index 68cd1f3a..1c9a29fc 100644 --- a/apps/frontend/src/shared/state-machines/index.ts +++ b/apps/frontend/src/shared/state-machines/index.ts @@ -7,3 +7,12 @@ export { mapStateToLegacy, } from './task-state-utils'; export type { TaskStateName } from './task-state-utils'; + +export { prReviewMachine } from './pr-review-machine'; +export type { PRReviewContext, PRReviewEvent } from './pr-review-machine'; +export { + PR_REVIEW_STATE_NAMES, + PR_REVIEW_SETTLED_STATES, + mapPRReviewStateToLegacy, +} from './pr-review-state-utils'; +export type { PRReviewStateName } from './pr-review-state-utils'; diff --git a/apps/frontend/src/shared/state-machines/pr-review-machine.ts b/apps/frontend/src/shared/state-machines/pr-review-machine.ts new file mode 100644 index 00000000..fd14698a --- /dev/null +++ b/apps/frontend/src/shared/state-machines/pr-review-machine.ts @@ -0,0 +1,177 @@ +import { assign, createMachine } from 'xstate'; +import type { PRReviewProgress, PRReviewResult } from '../../preload/api/modules/github-api'; + +export interface PRReviewContext { + prNumber: number | null; + projectId: string | null; + startedAt: string | null; + isFollowup: boolean; + progress: PRReviewProgress | null; + result: PRReviewResult | null; + previousResult: PRReviewResult | null; + error: string | null; + isExternalReview: boolean; +} + +export type PRReviewEvent = + | { type: 'START_REVIEW'; prNumber: number; projectId: string } + | { type: 'START_FOLLOWUP_REVIEW'; prNumber: number; projectId: string; previousResult: PRReviewResult } + | { type: 'SET_PROGRESS'; progress: PRReviewProgress } + | { type: 'REVIEW_COMPLETE'; result: PRReviewResult } + | { type: 'REVIEW_ERROR'; error: string } + | { type: 'CANCEL_REVIEW' } + | { type: 'DETECT_EXTERNAL_REVIEW' } + | { type: 'CLEAR_REVIEW' }; + +const initialContext: PRReviewContext = { + prNumber: null, + projectId: null, + startedAt: null, + isFollowup: false, + progress: null, + result: null, + previousResult: null, + error: null, + isExternalReview: false, +}; + +export const prReviewMachine = createMachine( + { + id: 'prReview', + initial: 'idle', + types: {} as { + context: PRReviewContext; + events: PRReviewEvent; + }, + context: { ...initialContext }, + states: { + idle: { + on: { + START_REVIEW: { + target: 'reviewing', + actions: 'setReviewStart', + }, + START_FOLLOWUP_REVIEW: { + target: 'reviewing', + actions: 'setFollowupReviewStart', + }, + }, + }, + reviewing: { + on: { + START_REVIEW: { + target: 'reviewing', + guard: 'isNotAlreadyReviewing', + }, + SET_PROGRESS: { + actions: 'setProgress', + }, + REVIEW_COMPLETE: { + target: 'completed', + actions: 'setResult', + }, + REVIEW_ERROR: { + target: 'error', + actions: 'setError', + }, + CANCEL_REVIEW: { + target: 'error', + actions: 'setCancelledError', + }, + DETECT_EXTERNAL_REVIEW: { + target: 'externalReview', + actions: 'setExternalReview', + }, + }, + }, + externalReview: { + on: { + REVIEW_COMPLETE: { + target: 'completed', + actions: 'setResult', + }, + REVIEW_ERROR: { + target: 'error', + actions: 'setError', + }, + }, + }, + completed: { + on: { + START_REVIEW: { + target: 'reviewing', + actions: 'setReviewStart', + }, + START_FOLLOWUP_REVIEW: { + target: 'reviewing', + actions: 'setFollowupReviewStart', + }, + CLEAR_REVIEW: { + target: 'idle', + actions: 'clearContext', + }, + }, + }, + error: { + on: { + START_REVIEW: { + target: 'reviewing', + actions: 'setReviewStart', + }, + CLEAR_REVIEW: { + target: 'idle', + actions: 'clearContext', + }, + }, + }, + }, + }, + { + guards: { + isNotAlreadyReviewing: () => false, + }, + actions: { + setReviewStart: assign({ + prNumber: ({ event }) => (event as { prNumber: number }).prNumber, + projectId: ({ event }) => (event as { projectId: string }).projectId, + startedAt: () => new Date().toISOString(), + isFollowup: () => false, + progress: () => null, + result: () => null, + previousResult: () => null, + error: () => null, + isExternalReview: () => false, + }), + setFollowupReviewStart: assign({ + prNumber: ({ event }) => (event as { prNumber: number }).prNumber, + projectId: ({ event }) => (event as { projectId: string }).projectId, + startedAt: () => new Date().toISOString(), + isFollowup: () => true, + progress: () => null, + result: () => null, + previousResult: ({ event }) => (event as { previousResult: PRReviewResult }).previousResult, + error: () => null, + isExternalReview: () => false, + }), + setProgress: assign({ + progress: ({ event }) => (event as { progress: PRReviewProgress }).progress, + }), + setResult: assign({ + result: ({ event }) => (event as { result: PRReviewResult }).result, + progress: () => null, + }), + setError: assign({ + error: ({ event }) => (event as { error: string }).error, + progress: () => null, + }), + setCancelledError: assign({ + error: () => 'Review cancelled by user', + progress: () => null, + }), + setExternalReview: assign({ + isExternalReview: () => true, + }), + clearContext: assign(() => ({ ...initialContext })), + }, + } +); diff --git a/apps/frontend/src/shared/state-machines/pr-review-state-utils.ts b/apps/frontend/src/shared/state-machines/pr-review-state-utils.ts new file mode 100644 index 00000000..a45f41a4 --- /dev/null +++ b/apps/frontend/src/shared/state-machines/pr-review-state-utils.ts @@ -0,0 +1,52 @@ +/** + * Shared XState PR review state utilities. + * + * Provides type-safe state names, settled states, and legacy status conversion + * derived from the PR review machine definition. + */ + +/** + * All XState PR review state names. + * + * IMPORTANT: These must match the state keys in pr-review-machine.ts. + * If you add/remove a state in the machine, update this array. + */ +export const PR_REVIEW_STATE_NAMES = [ + 'idle', 'reviewing', 'externalReview', 'completed', 'error' +] as const; + +export type PRReviewStateName = typeof PR_REVIEW_STATE_NAMES[number]; + +/** + * XState states where the PR review has "settled" — the review lifecycle + * has reached a terminal or resting state. Progress events should not + * overwrite these states. + */ +export const PR_REVIEW_SETTLED_STATES: ReadonlySet = new Set([ + 'completed', 'error' +]); + +/** + * Legacy review status values used by the existing Zustand store. + */ +type LegacyPRReviewStatus = 'idle' | 'reviewing' | 'completed' | 'error'; + +/** + * Convert XState PR review state to legacy status for backward compatibility. + */ +export function mapPRReviewStateToLegacy(state: string): LegacyPRReviewStatus { + switch (state) { + case 'idle': + return 'idle'; + case 'reviewing': + return 'reviewing'; + case 'externalReview': + return 'reviewing'; + case 'completed': + return 'completed'; + case 'error': + return 'error'; + default: + return 'idle'; + } +} From f3e910598b480ed1c4762a904166423749d9b968 Mon Sep 17 00:00:00 2001 From: AndyMik90 Date: Sat, 14 Feb 2026 00:41:26 +0100 Subject: [PATCH 02/15] auto-claude: subtask-1-2 - Write comprehensive unit tests for the PR review state machine and utilities Co-Authored-By: Claude Opus 4.6 --- .../__tests__/pr-review-machine.test.ts | 312 ++++++++++++++++++ .../__tests__/pr-review-state-utils.test.ts | 55 +++ 2 files changed, 367 insertions(+) create mode 100644 apps/frontend/src/shared/state-machines/__tests__/pr-review-machine.test.ts create mode 100644 apps/frontend/src/shared/state-machines/__tests__/pr-review-state-utils.test.ts diff --git a/apps/frontend/src/shared/state-machines/__tests__/pr-review-machine.test.ts b/apps/frontend/src/shared/state-machines/__tests__/pr-review-machine.test.ts new file mode 100644 index 00000000..b7946beb --- /dev/null +++ b/apps/frontend/src/shared/state-machines/__tests__/pr-review-machine.test.ts @@ -0,0 +1,312 @@ +import { describe, it, expect } from 'vitest'; +import { createActor } from 'xstate'; +import { prReviewMachine, type PRReviewEvent } from '../pr-review-machine'; + +/** + * Helper to run a sequence of events and get the final state + */ +function runEvents(events: PRReviewEvent[]) { + const actor = createActor(prReviewMachine); + actor.start(); + + for (const event of events) { + actor.send(event); + } + + const snapshot = actor.getSnapshot(); + actor.stop(); + return snapshot; +} + +const mockResult = { + summary: 'Test review', + rating: 'approve' as const, + comments: [], + filesReviewed: 3, + issuesFound: 0, +}; + +const mockProgress = { + phase: 'analyzing' as const, + message: 'Analyzing files...', + percentage: 50, +}; + +describe('prReviewMachine', () => { + describe('initial state', () => { + it('should start in idle state', () => { + const actor = createActor(prReviewMachine); + actor.start(); + expect(actor.getSnapshot().value).toBe('idle'); + actor.stop(); + }); + + it('should have null context initially', () => { + const actor = createActor(prReviewMachine); + actor.start(); + const ctx = actor.getSnapshot().context; + expect(ctx.prNumber).toBeNull(); + expect(ctx.projectId).toBeNull(); + expect(ctx.progress).toBeNull(); + expect(ctx.result).toBeNull(); + expect(ctx.previousResult).toBeNull(); + expect(ctx.error).toBeNull(); + expect(ctx.isFollowup).toBe(false); + expect(ctx.isExternalReview).toBe(false); + actor.stop(); + }); + }); + + describe('happy path: idle -> reviewing -> completed', () => { + it('should transition through the standard review flow', () => { + const snapshot = runEvents([ + { type: 'START_REVIEW', prNumber: 42, projectId: 'proj-1' }, + { type: 'REVIEW_COMPLETE', result: mockResult }, + ]); + + expect(snapshot.value).toBe('completed'); + expect(snapshot.context.result).toEqual(mockResult); + expect(snapshot.context.prNumber).toBe(42); + expect(snapshot.context.projectId).toBe('proj-1'); + }); + }); + + describe('follow-up review: completed -> reviewing (with previousResult)', () => { + it('should preserve previousResult when starting follow-up from completed', () => { + const snapshot = runEvents([ + { type: 'START_REVIEW', prNumber: 42, projectId: 'proj-1' }, + { type: 'REVIEW_COMPLETE', result: mockResult }, + { type: 'START_FOLLOWUP_REVIEW', prNumber: 42, projectId: 'proj-1', previousResult: mockResult }, + ]); + + expect(snapshot.value).toBe('reviewing'); + expect(snapshot.context.isFollowup).toBe(true); + expect(snapshot.context.previousResult).toEqual(mockResult); + expect(snapshot.context.result).toBeNull(); + }); + }); + + describe('error handling: reviewing -> error', () => { + it('should transition to error on REVIEW_ERROR', () => { + const snapshot = runEvents([ + { type: 'START_REVIEW', prNumber: 42, projectId: 'proj-1' }, + { type: 'REVIEW_ERROR', error: 'API failure' }, + ]); + + expect(snapshot.value).toBe('error'); + expect(snapshot.context.error).toBe('API failure'); + expect(snapshot.context.progress).toBeNull(); + }); + }); + + describe('cancel flow: reviewing -> error', () => { + it('should set cancelled error message on CANCEL_REVIEW', () => { + const snapshot = runEvents([ + { type: 'START_REVIEW', prNumber: 42, projectId: 'proj-1' }, + { type: 'CANCEL_REVIEW' }, + ]); + + expect(snapshot.value).toBe('error'); + expect(snapshot.context.error).toBe('Review cancelled by user'); + }); + }); + + describe('external review: reviewing -> externalReview -> completed', () => { + it('should transition through external review flow', () => { + const snapshot = runEvents([ + { type: 'START_REVIEW', prNumber: 42, projectId: 'proj-1' }, + { type: 'DETECT_EXTERNAL_REVIEW' }, + { type: 'REVIEW_COMPLETE', result: mockResult }, + ]); + + expect(snapshot.value).toBe('completed'); + expect(snapshot.context.isExternalReview).toBe(true); + expect(snapshot.context.result).toEqual(mockResult); + }); + + it('should transition to error from externalReview', () => { + const snapshot = runEvents([ + { type: 'START_REVIEW', prNumber: 42, projectId: 'proj-1' }, + { type: 'DETECT_EXTERNAL_REVIEW' }, + { type: 'REVIEW_ERROR', error: 'External review failed' }, + ]); + + expect(snapshot.value).toBe('error'); + expect(snapshot.context.error).toBe('External review failed'); + }); + }); + + describe('retry after error: error -> reviewing -> completed', () => { + it('should allow starting a new review from error state', () => { + const snapshot = runEvents([ + { type: 'START_REVIEW', prNumber: 42, projectId: 'proj-1' }, + { type: 'REVIEW_ERROR', error: 'Failed' }, + { type: 'START_REVIEW', prNumber: 42, projectId: 'proj-1' }, + { type: 'REVIEW_COMPLETE', result: mockResult }, + ]); + + expect(snapshot.value).toBe('completed'); + expect(snapshot.context.error).toBeNull(); + expect(snapshot.context.result).toEqual(mockResult); + }); + }); + + describe('clear review', () => { + it('should clear from completed to idle', () => { + const snapshot = runEvents([ + { type: 'START_REVIEW', prNumber: 42, projectId: 'proj-1' }, + { type: 'REVIEW_COMPLETE', result: mockResult }, + { type: 'CLEAR_REVIEW' }, + ]); + + expect(snapshot.value).toBe('idle'); + expect(snapshot.context.prNumber).toBeNull(); + expect(snapshot.context.result).toBeNull(); + }); + + it('should clear from error to idle', () => { + const snapshot = runEvents([ + { type: 'START_REVIEW', prNumber: 42, projectId: 'proj-1' }, + { type: 'REVIEW_ERROR', error: 'Failed' }, + { type: 'CLEAR_REVIEW' }, + ]); + + expect(snapshot.value).toBe('idle'); + expect(snapshot.context.error).toBeNull(); + }); + }); + + describe('guard: reject START_REVIEW when already reviewing', () => { + it('should stay in reviewing when START_REVIEW is sent again', () => { + const snapshot = runEvents([ + { type: 'START_REVIEW', prNumber: 42, projectId: 'proj-1' }, + { type: 'START_REVIEW', prNumber: 99, projectId: 'proj-2' }, + ]); + + expect(snapshot.value).toBe('reviewing'); + expect(snapshot.context.prNumber).toBe(42); + }); + }); + + describe('guard: reject SET_PROGRESS when not in reviewing state', () => { + it('should ignore SET_PROGRESS in idle state', () => { + const snapshot = runEvents([ + { type: 'SET_PROGRESS', progress: mockProgress }, + ]); + + expect(snapshot.value).toBe('idle'); + expect(snapshot.context.progress).toBeNull(); + }); + + it('should ignore SET_PROGRESS in completed state', () => { + const snapshot = runEvents([ + { type: 'START_REVIEW', prNumber: 42, projectId: 'proj-1' }, + { type: 'REVIEW_COMPLETE', result: mockResult }, + { type: 'SET_PROGRESS', progress: mockProgress }, + ]); + + expect(snapshot.value).toBe('completed'); + expect(snapshot.context.progress).toBeNull(); + }); + + it('should ignore SET_PROGRESS in error state', () => { + const snapshot = runEvents([ + { type: 'START_REVIEW', prNumber: 42, projectId: 'proj-1' }, + { type: 'REVIEW_ERROR', error: 'Failed' }, + { type: 'SET_PROGRESS', progress: mockProgress }, + ]); + + expect(snapshot.value).toBe('error'); + expect(snapshot.context.progress).toBeNull(); + }); + }); + + describe('context updates', () => { + it('should store progress during review', () => { + const snapshot = runEvents([ + { type: 'START_REVIEW', prNumber: 42, projectId: 'proj-1' }, + { type: 'SET_PROGRESS', progress: mockProgress }, + ]); + + expect(snapshot.context.progress).toEqual(mockProgress); + }); + + it('should store result on completion', () => { + const snapshot = runEvents([ + { type: 'START_REVIEW', prNumber: 42, projectId: 'proj-1' }, + { type: 'REVIEW_COMPLETE', result: mockResult }, + ]); + + expect(snapshot.context.result).toEqual(mockResult); + expect(snapshot.context.progress).toBeNull(); + }); + + it('should store error on failure', () => { + const snapshot = runEvents([ + { type: 'START_REVIEW', prNumber: 42, projectId: 'proj-1' }, + { type: 'REVIEW_ERROR', error: 'Something broke' }, + ]); + + expect(snapshot.context.error).toBe('Something broke'); + expect(snapshot.context.progress).toBeNull(); + }); + + it('should set startedAt on review start', () => { + const snapshot = runEvents([ + { type: 'START_REVIEW', prNumber: 42, projectId: 'proj-1' }, + ]); + + expect(snapshot.context.startedAt).toBeTruthy(); + }); + }); + + describe('follow-up context', () => { + it('should preserve previousResult in follow-up review', () => { + const snapshot = runEvents([ + { type: 'START_REVIEW', prNumber: 42, projectId: 'proj-1' }, + { type: 'REVIEW_COMPLETE', result: mockResult }, + { type: 'START_FOLLOWUP_REVIEW', prNumber: 42, projectId: 'proj-1', previousResult: mockResult }, + { type: 'REVIEW_COMPLETE', result: { ...mockResult, summary: 'Follow-up review' } }, + ]); + + expect(snapshot.value).toBe('completed'); + expect(snapshot.context.previousResult).toEqual(mockResult); + expect(snapshot.context.result?.summary).toBe('Follow-up review'); + }); + + it('should clear previousResult on normal START_REVIEW from completed', () => { + const snapshot = runEvents([ + { type: 'START_REVIEW', prNumber: 42, projectId: 'proj-1' }, + { type: 'REVIEW_COMPLETE', result: mockResult }, + { type: 'START_REVIEW', prNumber: 43, projectId: 'proj-1' }, + ]); + + expect(snapshot.context.previousResult).toBeNull(); + expect(snapshot.context.isFollowup).toBe(false); + }); + }); + + describe('stale events', () => { + it('should reject SET_PROGRESS in completed state', () => { + const snapshot = runEvents([ + { type: 'START_REVIEW', prNumber: 42, projectId: 'proj-1' }, + { type: 'REVIEW_COMPLETE', result: mockResult }, + { type: 'SET_PROGRESS', progress: mockProgress }, + ]); + + expect(snapshot.value).toBe('completed'); + expect(snapshot.context.progress).toBeNull(); + }); + + it('should reject SET_PROGRESS in error state', () => { + const snapshot = runEvents([ + { type: 'START_REVIEW', prNumber: 42, projectId: 'proj-1' }, + { type: 'REVIEW_ERROR', error: 'Failed' }, + { type: 'SET_PROGRESS', progress: mockProgress }, + ]); + + expect(snapshot.value).toBe('error'); + expect(snapshot.context.progress).toBeNull(); + }); + }); +}); diff --git a/apps/frontend/src/shared/state-machines/__tests__/pr-review-state-utils.test.ts b/apps/frontend/src/shared/state-machines/__tests__/pr-review-state-utils.test.ts new file mode 100644 index 00000000..39c6f80f --- /dev/null +++ b/apps/frontend/src/shared/state-machines/__tests__/pr-review-state-utils.test.ts @@ -0,0 +1,55 @@ +import { describe, it, expect } from 'vitest'; +import { + PR_REVIEW_STATE_NAMES, + PR_REVIEW_SETTLED_STATES, + mapPRReviewStateToLegacy, +} from '../pr-review-state-utils'; + +describe('pr-review-state-utils', () => { + describe('PR_REVIEW_STATE_NAMES', () => { + it('should contain all expected state names', () => { + expect(PR_REVIEW_STATE_NAMES).toEqual([ + 'idle', 'reviewing', 'externalReview', 'completed', 'error', + ]); + }); + }); + + describe('PR_REVIEW_SETTLED_STATES', () => { + it('should contain completed and error', () => { + expect(PR_REVIEW_SETTLED_STATES.has('completed')).toBe(true); + expect(PR_REVIEW_SETTLED_STATES.has('error')).toBe(true); + }); + + it('should not contain active states', () => { + expect(PR_REVIEW_SETTLED_STATES.has('idle')).toBe(false); + expect(PR_REVIEW_SETTLED_STATES.has('reviewing')).toBe(false); + expect(PR_REVIEW_SETTLED_STATES.has('externalReview')).toBe(false); + }); + }); + + describe('mapPRReviewStateToLegacy', () => { + it('should map idle to idle', () => { + expect(mapPRReviewStateToLegacy('idle')).toBe('idle'); + }); + + it('should map reviewing to reviewing', () => { + expect(mapPRReviewStateToLegacy('reviewing')).toBe('reviewing'); + }); + + it('should map externalReview to reviewing', () => { + expect(mapPRReviewStateToLegacy('externalReview')).toBe('reviewing'); + }); + + it('should map completed to completed', () => { + expect(mapPRReviewStateToLegacy('completed')).toBe('completed'); + }); + + it('should map error to error', () => { + expect(mapPRReviewStateToLegacy('error')).toBe('error'); + }); + + it('should map unknown states to idle', () => { + expect(mapPRReviewStateToLegacy('unknown')).toBe('idle'); + }); + }); +}); From b4264ae95be954e6956e51aa7c5c829d3b303fad Mon Sep 17 00:00:00 2001 From: AndyMik90 Date: Sat, 14 Feb 2026 00:43:33 +0100 Subject: [PATCH 03/15] auto-claude: subtask-2-1 - Add IPC channel constant and preload API listener for PR review state changes --- .../src/preload/api/modules/github-api.ts | 25 +++++++++++++++++++ apps/frontend/src/shared/constants/ipc.ts | 1 + 2 files changed, 26 insertions(+) diff --git a/apps/frontend/src/preload/api/modules/github-api.ts b/apps/frontend/src/preload/api/modules/github-api.ts index bca2a8f1..7c836589 100644 --- a/apps/frontend/src/preload/api/modules/github-api.ts +++ b/apps/frontend/src/preload/api/modules/github-api.ts @@ -308,6 +308,9 @@ export interface GitHubAPI { onPRReviewError: ( callback: (projectId: string, error: { prNumber: number; error: string }) => void ) => IpcListenerCleanup; + onPRReviewStateChange: ( + callback: (key: string, state: PRReviewStatePayload) => void + ) => IpcListenerCleanup; onPRLogsUpdated: ( callback: (projectId: string, data: { prNumber: number; entryCount: number }) => void ) => IpcListenerCleanup; @@ -456,6 +459,23 @@ export interface PRReviewProgress { message: string; } +/** + * PR review state payload (emitted on state machine transitions) + */ +export interface PRReviewStatePayload { + state: string; + prNumber: number; + projectId: string; + isReviewing: boolean; + startedAt: string | null; + progress: PRReviewProgress | null; + result: PRReviewResult | null; + previousResult: PRReviewResult | null; + error: string | null; + isExternalReview: boolean; + isFollowup: boolean; +} + /** * PR review log entry type */ @@ -780,6 +800,11 @@ export const createGitHubAPI = (): GitHubAPI => ({ ): IpcListenerCleanup => createIpcListener(IPC_CHANNELS.GITHUB_PR_REVIEW_ERROR, callback), + onPRReviewStateChange: ( + callback: (key: string, state: PRReviewStatePayload) => void + ): IpcListenerCleanup => + createIpcListener(IPC_CHANNELS.GITHUB_PR_REVIEW_STATE_CHANGE, callback), + onPRLogsUpdated: ( callback: (projectId: string, data: { prNumber: number; entryCount: number }) => void ): IpcListenerCleanup => diff --git a/apps/frontend/src/shared/constants/ipc.ts b/apps/frontend/src/shared/constants/ipc.ts index 4c628608..8af598b8 100644 --- a/apps/frontend/src/shared/constants/ipc.ts +++ b/apps/frontend/src/shared/constants/ipc.ts @@ -413,6 +413,7 @@ export const IPC_CHANNELS = { GITHUB_PR_REVIEW_PROGRESS: 'github:pr:reviewProgress', GITHUB_PR_REVIEW_COMPLETE: 'github:pr:reviewComplete', GITHUB_PR_REVIEW_ERROR: 'github:pr:reviewError', + GITHUB_PR_REVIEW_STATE_CHANGE: 'github:pr:reviewStateChange', GITHUB_PR_LOGS_UPDATED: 'github:pr:logsUpdated', // GitHub PR Logs (for viewing AI review logs) From bfdbdb5f229a330dd1c84a40d1d069cd84cd7136 Mon Sep 17 00:00:00 2001 From: AndyMik90 Date: Sat, 14 Feb 2026 00:46:04 +0100 Subject: [PATCH 04/15] auto-claude: subtask-2-2 - Create PRReviewStateManager class in Electron main process Co-Authored-By: Claude Opus 4.6 --- .../src/main/pr-review-state-manager.ts | 162 ++++++++++++++++++ 1 file changed, 162 insertions(+) create mode 100644 apps/frontend/src/main/pr-review-state-manager.ts diff --git a/apps/frontend/src/main/pr-review-state-manager.ts b/apps/frontend/src/main/pr-review-state-manager.ts new file mode 100644 index 00000000..01ba68fb --- /dev/null +++ b/apps/frontend/src/main/pr-review-state-manager.ts @@ -0,0 +1,162 @@ +import { createActor } from 'xstate'; +import type { ActorRefFrom } from 'xstate'; +import type { BrowserWindow } from 'electron'; +import { prReviewMachine, type PRReviewEvent, type PRReviewContext } from '../shared/state-machines'; +import type { PRReviewProgress, PRReviewResult } from '../preload/api/modules/github-api'; +import { IPC_CHANNELS } from '../shared/constants'; +import { safeSendToRenderer } from './ipc-handlers/utils'; + +type PRReviewActor = ActorRefFrom; + +/** + * Build a deduplication key from snapshot state + relevant context fields. + * PR reviews need to emit even when state stays the same but context changes + * (e.g., progress updates within 'reviewing' state). + */ +function buildContextKey(snapshot: { context: PRReviewContext }): string { + const ctx = snapshot.context; + const progressKey = ctx.progress + ? `${ctx.progress.phase}:${ctx.progress.progress}:${ctx.progress.message}` + : 'none'; + const resultKey = ctx.result ? ctx.result.overallStatus : 'none'; + const errorKey = ctx.error ?? 'none'; + return `${progressKey}|${resultKey}|${errorKey}`; +} + +export class PRReviewStateManager { + private actors = new Map(); + private lastStateByPR = new Map(); + private getMainWindow: () => BrowserWindow | null; + + constructor(getMainWindow: () => BrowserWindow | null) { + this.getMainWindow = getMainWindow; + } + + handleStartReview(projectId: string, prNumber: number): void { + const actor = this.getOrCreateActor(projectId, prNumber); + actor.send({ type: 'START_REVIEW', prNumber, projectId } satisfies PRReviewEvent); + } + + handleStartFollowupReview(projectId: string, prNumber: number, previousResult?: PRReviewResult): void { + const actor = this.getOrCreateActor(projectId, prNumber); + if (previousResult) { + actor.send({ type: 'START_FOLLOWUP_REVIEW', prNumber, projectId, previousResult } satisfies PRReviewEvent); + } else { + actor.send({ type: 'START_REVIEW', prNumber, projectId } satisfies PRReviewEvent); + } + } + + handleProgress(projectId: string, prNumber: number, progress: PRReviewProgress): void { + const actor = this.getActor(projectId, prNumber); + if (!actor) return; + actor.send({ type: 'SET_PROGRESS', progress } satisfies PRReviewEvent); + } + + handleComplete(projectId: string, prNumber: number, result: PRReviewResult): void { + const actor = this.getActor(projectId, prNumber); + if (!actor) return; + + // Detect external review (result arrives with 'in_progress' status from outside) + if (result.overallStatus === 'in_progress') { + actor.send({ type: 'DETECT_EXTERNAL_REVIEW' } satisfies PRReviewEvent); + } else { + actor.send({ type: 'REVIEW_COMPLETE', result } satisfies PRReviewEvent); + } + } + + handleError(projectId: string, prNumber: number, error: string): void { + const actor = this.getActor(projectId, prNumber); + if (!actor) return; + actor.send({ type: 'REVIEW_ERROR', error } satisfies PRReviewEvent); + } + + handleCancel(projectId: string, prNumber: number): void { + const actor = this.getActor(projectId, prNumber); + if (!actor) return; + actor.send({ type: 'CANCEL_REVIEW' } satisfies PRReviewEvent); + } + + handleClearReview(projectId: string, prNumber: number): void { + const key = this.getKey(projectId, prNumber); + const actor = this.actors.get(key); + if (actor) { + actor.send({ type: 'CLEAR_REVIEW' } satisfies PRReviewEvent); + actor.stop(); + this.actors.delete(key); + } + this.lastStateByPR.delete(key); + } + + handleAuthChange(): void { + for (const [key, actor] of this.actors) { + actor.stop(); + // Emit cleared (idle) state to renderer for each PR + this.emitStateToRenderer(key, null); + } + this.actors.clear(); + this.lastStateByPR.clear(); + } + + getState(projectId: string, prNumber: number): ReturnType | null { + const actor = this.getActor(projectId, prNumber); + if (!actor) return null; + return actor.getSnapshot(); + } + + clearAll(): void { + for (const [, actor] of this.actors) { + actor.stop(); + } + this.actors.clear(); + this.lastStateByPR.clear(); + } + + // --------------------------------------------------------------------------- + // Private + // --------------------------------------------------------------------------- + + private getOrCreateActor(projectId: string, prNumber: number): PRReviewActor { + const key = this.getKey(projectId, prNumber); + const existing = this.actors.get(key); + if (existing) return existing; + + const actor = createActor(prReviewMachine); + + actor.subscribe((snapshot) => { + const stateValue = String(snapshot.value); + const contextKey = buildContextKey(snapshot); + const currentKey = `${stateValue}:${contextKey}`; + if (this.lastStateByPR.get(key) === currentKey) return; + this.lastStateByPR.set(key, currentKey); + this.emitStateToRenderer(key, snapshot); + }); + + actor.start(); + this.actors.set(key, actor); + return actor; + } + + private getActor(projectId: string, prNumber: number): PRReviewActor | null { + return this.actors.get(this.getKey(projectId, prNumber)) ?? null; + } + + private getKey(projectId: string, prNumber: number): string { + return `${projectId}:${prNumber}`; + } + + private emitStateToRenderer( + key: string, + snapshot: ReturnType | null + ): void { + const stateValue = snapshot ? String(snapshot.value) : 'idle'; + const context = snapshot ? snapshot.context : null; + + safeSendToRenderer( + this.getMainWindow, + IPC_CHANNELS.GITHUB_PR_REVIEW_STATE_CHANGE, + key, + stateValue, + context + ); + } +} From 8c0b0257a2d4ab35a49c835804a86273ddde4e75 Mon Sep 17 00:00:00 2001 From: AndyMik90 Date: Sat, 14 Feb 2026 00:48:16 +0100 Subject: [PATCH 05/15] auto-claude: subtask-2-3 - Write unit tests for PRReviewStateManager. File: apps/frontend/src/main/__tests__/pr-review-state-manager.test.ts --- .../__tests__/pr-review-state-manager.test.ts | 296 ++++++++++++++++++ 1 file changed, 296 insertions(+) create mode 100644 apps/frontend/src/main/__tests__/pr-review-state-manager.test.ts diff --git a/apps/frontend/src/main/__tests__/pr-review-state-manager.test.ts b/apps/frontend/src/main/__tests__/pr-review-state-manager.test.ts new file mode 100644 index 00000000..3661fe4d --- /dev/null +++ b/apps/frontend/src/main/__tests__/pr-review-state-manager.test.ts @@ -0,0 +1,296 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { PRReviewStateManager } from '../pr-review-state-manager'; +import type { PRReviewResult, PRReviewProgress } from '../../preload/api/modules/github-api'; + +// Mock dependencies +const mockSafeSendToRenderer = vi.fn(); +vi.mock('../ipc-handlers/utils', () => ({ + safeSendToRenderer: (...args: unknown[]) => mockSafeSendToRenderer(...args) +})); + +function createMockGetMainWindow() { + return vi.fn(() => ({ id: 1 }) as unknown as Electron.BrowserWindow); +} + +function createMockProgress(overrides: Partial = {}): PRReviewProgress { + return { + phase: 'analyzing', + progress: 50, + message: 'Analyzing files...', + ...overrides + } as PRReviewProgress; +} + +function createMockResult(overrides: Partial = {}): PRReviewResult { + return { + overallStatus: 'approved', + summary: 'Looks good', + ...overrides + } as PRReviewResult; +} + +describe('PRReviewStateManager', () => { + let manager: PRReviewStateManager; + const projectId = 'project-1'; + const prNumber = 42; + + beforeEach(() => { + manager = new PRReviewStateManager(createMockGetMainWindow()); + vi.clearAllMocks(); + }); + + afterEach(() => { + manager.clearAll(); + }); + + describe('actor lifecycle', () => { + it('should create actor on first handleStartReview call', () => { + manager.handleStartReview(projectId, prNumber); + const snapshot = manager.getState(projectId, prNumber); + expect(snapshot).not.toBeNull(); + }); + + it('should reuse existing actor for same PR key', () => { + manager.handleStartReview(projectId, prNumber); + const snapshot1 = manager.getState(projectId, prNumber); + // Calling again should not create a new actor + manager.handleStartReview(projectId, prNumber); + const snapshot2 = manager.getState(projectId, prNumber); + expect(snapshot1).not.toBeNull(); + expect(snapshot2).not.toBeNull(); + }); + + it('should create separate actors for different PRs', () => { + manager.handleStartReview(projectId, 1); + manager.handleStartReview(projectId, 2); + const snapshot1 = manager.getState(projectId, 1); + const snapshot2 = manager.getState(projectId, 2); + expect(snapshot1).not.toBeNull(); + expect(snapshot2).not.toBeNull(); + }); + + it('should start actor before events are sent', () => { + manager.handleStartReview(projectId, prNumber); + const snapshot = manager.getState(projectId, prNumber); + // If actor wasn't started, getSnapshot would fail or return unexpected state + expect(snapshot).not.toBeNull(); + expect(String(snapshot!.value)).toBe('reviewing'); + }); + }); + + describe('event routing', () => { + it('should transition to reviewing on handleStartReview', () => { + manager.handleStartReview(projectId, prNumber); + const snapshot = manager.getState(projectId, prNumber); + expect(String(snapshot!.value)).toBe('reviewing'); + }); + + it('should send START_FOLLOWUP_REVIEW with previousResult', () => { + const previousResult = createMockResult(); + manager.handleStartFollowupReview(projectId, prNumber, previousResult); + const snapshot = manager.getState(projectId, prNumber); + expect(String(snapshot!.value)).toBe('reviewing'); + expect(snapshot!.context.isFollowup).toBe(true); + expect(snapshot!.context.previousResult).toBe(previousResult); + }); + + it('should send START_REVIEW when handleStartFollowupReview has no previousResult', () => { + manager.handleStartFollowupReview(projectId, prNumber); + const snapshot = manager.getState(projectId, prNumber); + expect(String(snapshot!.value)).toBe('reviewing'); + expect(snapshot!.context.isFollowup).toBe(false); + }); + + it('should update context on handleProgress', () => { + manager.handleStartReview(projectId, prNumber); + const progress = createMockProgress(); + manager.handleProgress(projectId, prNumber, progress); + const snapshot = manager.getState(projectId, prNumber); + expect(snapshot!.context.progress).toEqual(progress); + }); + + it('should ignore handleProgress for unknown PR', () => { + // Should not throw + manager.handleProgress(projectId, 999, createMockProgress()); + expect(manager.getState(projectId, 999)).toBeNull(); + }); + + it('should transition to completed on handleComplete', () => { + manager.handleStartReview(projectId, prNumber); + const result = createMockResult(); + manager.handleComplete(projectId, prNumber, result); + const snapshot = manager.getState(projectId, prNumber); + expect(String(snapshot!.value)).toBe('completed'); + expect(snapshot!.context.result).toEqual(result); + }); + + it('should send DETECT_EXTERNAL_REVIEW when overallStatus is in_progress', () => { + manager.handleStartReview(projectId, prNumber); + const result = createMockResult({ overallStatus: 'in_progress' }); + manager.handleComplete(projectId, prNumber, result); + const snapshot = manager.getState(projectId, prNumber); + expect(String(snapshot!.value)).toBe('externalReview'); + }); + + it('should transition to error on handleError', () => { + manager.handleStartReview(projectId, prNumber); + manager.handleError(projectId, prNumber, 'Something went wrong'); + const snapshot = manager.getState(projectId, prNumber); + expect(String(snapshot!.value)).toBe('error'); + expect(snapshot!.context.error).toBe('Something went wrong'); + }); + + it('should transition to error on handleCancel', () => { + manager.handleStartReview(projectId, prNumber); + manager.handleCancel(projectId, prNumber); + const snapshot = manager.getState(projectId, prNumber); + expect(String(snapshot!.value)).toBe('error'); + }); + }); + + describe('state emission', () => { + it('should emit state changes to renderer via safeSendToRenderer', () => { + manager.handleStartReview(projectId, prNumber); + expect(mockSafeSendToRenderer).toHaveBeenCalled(); + }); + + it('should use GITHUB_PR_REVIEW_STATE_CHANGE IPC channel', () => { + manager.handleStartReview(projectId, prNumber); + expect(mockSafeSendToRenderer).toHaveBeenCalledWith( + expect.any(Function), + 'github:pr:reviewStateChange', + expect.any(String), + expect.any(String), + expect.anything() + ); + }); + + it('should include state value and context in payload', () => { + manager.handleStartReview(projectId, prNumber); + // Find the call that emits 'reviewing' state + const reviewingCall = mockSafeSendToRenderer.mock.calls.find( + (call: unknown[]) => call[3] === 'reviewing' + ); + expect(reviewingCall).toBeDefined(); + expect(reviewingCall![2]).toBe(`${projectId}:${prNumber}`); + expect(reviewingCall![4]).toEqual(expect.objectContaining({ prNumber, projectId })); + }); + + it('should use projectId:prNumber as key format', () => { + manager.handleStartReview(projectId, prNumber); + const calls = mockSafeSendToRenderer.mock.calls; + const prCall = calls.find((call: unknown[]) => call[2] === `${projectId}:${prNumber}`); + expect(prCall).toBeDefined(); + }); + }); + + describe('deduplication', () => { + it('should NOT emit duplicate IPC for same state + same context', () => { + manager.handleStartReview(projectId, prNumber); + const callCountAfterStart = mockSafeSendToRenderer.mock.calls.length; + + // Sending START_REVIEW again won't transition (guard prevents it), so no new emission + manager.handleStartReview(projectId, prNumber); + expect(mockSafeSendToRenderer.mock.calls.length).toBe(callCountAfterStart); + }); + + it('should emit for same state but different context (progress update)', () => { + manager.handleStartReview(projectId, prNumber); + const callCountAfterStart = mockSafeSendToRenderer.mock.calls.length; + + manager.handleProgress(projectId, prNumber, createMockProgress({ progress: 25, message: 'Step 1' })); + expect(mockSafeSendToRenderer.mock.calls.length).toBeGreaterThan(callCountAfterStart); + + const callCountAfterProgress1 = mockSafeSendToRenderer.mock.calls.length; + manager.handleProgress(projectId, prNumber, createMockProgress({ progress: 75, message: 'Step 2' })); + expect(mockSafeSendToRenderer.mock.calls.length).toBeGreaterThan(callCountAfterProgress1); + }); + + it('should always emit for different state transitions', () => { + manager.handleStartReview(projectId, prNumber); + const callCountAfterStart = mockSafeSendToRenderer.mock.calls.length; + + manager.handleComplete(projectId, prNumber, createMockResult()); + expect(mockSafeSendToRenderer.mock.calls.length).toBeGreaterThan(callCountAfterStart); + }); + }); + + describe('cleanup', () => { + it('should stop actor and remove from map on handleClearReview', () => { + manager.handleStartReview(projectId, prNumber); + expect(manager.getState(projectId, prNumber)).not.toBeNull(); + + manager.handleClearReview(projectId, prNumber); + expect(manager.getState(projectId, prNumber)).toBeNull(); + }); + + it('should stop ALL actors and clear maps on handleAuthChange', () => { + manager.handleStartReview(projectId, 1); + manager.handleStartReview(projectId, 2); + + manager.handleAuthChange(); + + expect(manager.getState(projectId, 1)).toBeNull(); + expect(manager.getState(projectId, 2)).toBeNull(); + }); + + it('should emit cleared state to renderer on handleAuthChange', () => { + manager.handleStartReview(projectId, 1); + manager.handleStartReview(projectId, 2); + mockSafeSendToRenderer.mockClear(); + + manager.handleAuthChange(); + + // Should emit idle/null state for each PR + expect(mockSafeSendToRenderer).toHaveBeenCalledTimes(2); + for (const call of mockSafeSendToRenderer.mock.calls) { + expect(call[3]).toBe('idle'); // stateValue + expect(call[4]).toBeNull(); // context + } + }); + + it('should stop all actors on clearAll', () => { + manager.handleStartReview(projectId, 1); + manager.handleStartReview(projectId, 2); + + manager.clearAll(); + + expect(manager.getState(projectId, 1)).toBeNull(); + expect(manager.getState(projectId, 2)).toBeNull(); + }); + }); + + describe('concurrent PRs', () => { + it('should support multiple PRs with independent actors', () => { + manager.handleStartReview(projectId, 1); + manager.handleStartReview(projectId, 2); + + manager.handleComplete(projectId, 1, createMockResult()); + + expect(String(manager.getState(projectId, 1)!.value)).toBe('completed'); + expect(String(manager.getState(projectId, 2)!.value)).toBe('reviewing'); + }); + + it('should route events to correct actor by key', () => { + manager.handleStartReview(projectId, 1); + manager.handleStartReview(projectId, 2); + + manager.handleError(projectId, 2, 'Error on PR 2'); + + expect(String(manager.getState(projectId, 1)!.value)).toBe('reviewing'); + expect(String(manager.getState(projectId, 2)!.value)).toBe('error'); + expect(manager.getState(projectId, 2)!.context.error).toBe('Error on PR 2'); + }); + + it('should not affect other PRs when clearing one', () => { + manager.handleStartReview(projectId, 1); + manager.handleStartReview(projectId, 2); + + manager.handleClearReview(projectId, 1); + + expect(manager.getState(projectId, 1)).toBeNull(); + expect(manager.getState(projectId, 2)).not.toBeNull(); + expect(String(manager.getState(projectId, 2)!.value)).toBe('reviewing'); + }); + }); +}); From df819b39900766d0e878865683d68245003dedf1 Mon Sep 17 00:00:00 2001 From: AndyMik90 Date: Sat, 14 Feb 2026 00:52:31 +0100 Subject: [PATCH 06/15] auto-claude: subtask-3-1 - Refactor pr-handlers.ts to use PRReviewStateManager Replace all 6 createIPCCommunicators() calls for review lifecycle events with PRReviewStateManager routing. The manager's XState actors now handle all state transitions and emit on GITHUB_PR_REVIEW_STATE_CHANGE channel. Changes: - Create PRReviewStateManager instance in registerPRHandlers() - GITHUB_PR_REVIEW handler: route start/progress/complete/error through manager - GITHUB_PR_FOLLOWUP_REVIEW handler: route through manager with previous result - GITHUB_PR_REVIEW_CANCEL handler: call manager.handleCancel() after kill - sendReviewStateUpdate(): use manager.handleComplete() instead of direct IPC - runPRReview(): accept manager param, use for progress callbacks - Remove unused createIPCCommunicators import Co-Authored-By: Claude Opus 4.6 --- .../main/ipc-handlers/github/pr-handlers.ts | 138 ++++++------------ 1 file changed, 45 insertions(+), 93 deletions(-) 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 1f63b6c4..e56810f9 100644 --- a/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts @@ -26,7 +26,7 @@ import { getMemoryService, getDefaultDbPath } from "../../memory-service"; import type { Project, AppSettings } from "../../../shared/types"; import { createContextLogger } from "./utils/logger"; import { withProjectOrNull } from "./utils/project-middleware"; -import { createIPCCommunicators } from "./utils/ipc-communicator"; +import { PRReviewStateManager } from "../../pr-review-state-manager"; import { getRunnerEnv } from "./utils/runner-env"; import { runPythonSubprocess, @@ -1384,7 +1384,7 @@ function sendReviewStateUpdate( project: Project, prNumber: number, projectId: string, - getMainWindow: () => BrowserWindow | null, + prReviewStateManager: PRReviewStateManager, context: string ): void { try { @@ -1393,18 +1393,8 @@ function sendReviewStateUpdate( debugLog("Could not retrieve updated review result for UI notification", { prNumber, context }); return; } - const mainWindow = getMainWindow(); - if (!mainWindow) return; - const { sendComplete } = createIPCCommunicators( - mainWindow, - { - progress: IPC_CHANNELS.GITHUB_PR_REVIEW_PROGRESS, - error: IPC_CHANNELS.GITHUB_PR_REVIEW_ERROR, - complete: IPC_CHANNELS.GITHUB_PR_REVIEW_COMPLETE, - }, - projectId - ); - sendComplete(updatedResult); + // Route through state manager so the XState actor emits the state change + prReviewStateManager.handleComplete(projectId, prNumber, updatedResult); debugLog(`Sent PR review state update ${context}`, { prNumber }); } catch (uiError) { debugLog("Failed to send UI update (non-critical)", { @@ -1445,7 +1435,8 @@ function getGitHubPRSettings(): { model: string; thinkingLevel: string } { async function runPRReview( project: Project, prNumber: number, - mainWindow: BrowserWindow + mainWindow: BrowserWindow, + prReviewStateManager: PRReviewStateManager ): Promise { // Comprehensive validation of GitHub module const validation = await validateGitHubModule(project); @@ -1456,15 +1447,9 @@ async function runPRReview( const backendPath = validation.backendPath!; - const { sendProgress } = createIPCCommunicators( - mainWindow, - { - progress: IPC_CHANNELS.GITHUB_PR_REVIEW_PROGRESS, - error: IPC_CHANNELS.GITHUB_PR_REVIEW_ERROR, - complete: IPC_CHANNELS.GITHUB_PR_REVIEW_COMPLETE, - }, - project.id - ); + const sendProgress = (progress: PRReviewProgress): void => { + prReviewStateManager.handleProgress(project.id, prNumber, progress); + }; const { model, thinkingLevel } = getGitHubPRSettings(); const args = buildRunnerArgs( @@ -1682,6 +1667,9 @@ async function fetchPRsFromGraphQL( export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): void { debugLog("Registering PR handlers"); + // Create the XState-based PR review state manager + const prReviewStateManager = new PRReviewStateManager(getMainWindow); + // List open PRs - fetches up to 100 open PRs at once, returns hasNextPage and endCursor from API ipcMain.handle( IPC_CHANNELS.GITHUB_PR_LIST, @@ -1887,20 +1875,14 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v const reviewKey = getReviewKey(projectId, prNumber); + // Notify state manager that review is starting + prReviewStateManager.handleStartReview(projectId, prNumber); + try { await withProjectOrNull(projectId, async (project) => { - const { sendProgress, sendComplete } = createIPCCommunicators< - PRReviewProgress, - PRReviewResult - >( - mainWindow, - { - progress: IPC_CHANNELS.GITHUB_PR_REVIEW_PROGRESS, - error: IPC_CHANNELS.GITHUB_PR_REVIEW_ERROR, - complete: IPC_CHANNELS.GITHUB_PR_REVIEW_COMPLETE, - }, - projectId - ); + const sendProgress = (progress: PRReviewProgress): void => { + prReviewStateManager.handleProgress(projectId, prNumber, progress); + }; // Check if already running — notify renderer so it can display ongoing logs if (runningReviews.has(reviewKey)) { @@ -1978,31 +1960,18 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v message: "Fetching PR data...", }); - 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; - } + const result = await runPRReview(project, prNumber, mainWindow, prReviewStateManager); debugLog("PR review completed", { prNumber, findingsCount: result.findings.length }); sendProgress({ phase: "complete", prNumber, progress: 100, - message: "Review complete!", + message: result.overallStatus === "in_progress" ? "Review already in progress" : "Review complete!", }); - sendComplete(result); + // Route through manager — handles external review detection internally + prReviewStateManager.handleComplete(projectId, prNumber, result); } finally { // Clean up in case we exit before runPRReview was called (e.g., cancelled during CI wait) // runPRReview also has its own cleanup, but delete is idempotent @@ -2019,16 +1988,7 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v prNumber, error: error instanceof Error ? error.message : error, }); - const { sendError } = createIPCCommunicators( - mainWindow, - { - progress: IPC_CHANNELS.GITHUB_PR_REVIEW_PROGRESS, - error: IPC_CHANNELS.GITHUB_PR_REVIEW_ERROR, - complete: IPC_CHANNELS.GITHUB_PR_REVIEW_COMPLETE, - }, - projectId - ); - sendError(error instanceof Error ? error.message : "Failed to run PR review"); + prReviewStateManager.handleError(projectId, prNumber, error instanceof Error ? error.message : "Failed to run PR review"); } }); @@ -2233,7 +2193,7 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v } // Send state update event to refresh UI immediately (non-blocking) - sendReviewStateUpdate(project, prNumber, projectId, getMainWindow, "after posting"); + sendReviewStateUpdate(project, prNumber, projectId, prReviewStateManager, "after posting"); return true; } catch (error) { @@ -2272,7 +2232,7 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v debugLog("Marked review as posted", { prNumber }); // Send state update event to refresh UI immediately (non-blocking) - sendReviewStateUpdate(project, prNumber, projectId, getMainWindow, "after marking posted"); + sendReviewStateUpdate(project, prNumber, projectId, prReviewStateManager, "after marking posted"); return true; } catch (error) { @@ -2390,7 +2350,7 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v } // Send state update event to refresh UI immediately (non-blocking) - sendReviewStateUpdate(project, prNumber, projectId, getMainWindow, "after deletion"); + sendReviewStateUpdate(project, prNumber, projectId, prReviewStateManager, "after deletion"); return true; } catch (error) { @@ -2502,6 +2462,8 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v ciWaitAbortControllers.delete(reviewKey); } runningReviews.delete(reviewKey); + // Notify state manager of cancellation + prReviewStateManager.handleCancel(projectId, prNumber); debugLog("CI wait cancelled", { reviewKey }); return true; } @@ -2522,6 +2484,8 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v // Clean up the registry runningReviews.delete(reviewKey); + // Notify state manager of cancellation + prReviewStateManager.handleCancel(projectId, prNumber); debugLog("Review process cancelled", { reviewKey }); return true; } catch (error) { @@ -2917,25 +2881,24 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v return; } + // Get previous result for followup context + const previousResult = await withProjectOrNull(projectId, async (project) => { + return getReviewResult(project, prNumber) ?? undefined; + }) ?? undefined; + + // Notify state manager that followup review is starting + prReviewStateManager.handleStartFollowupReview(projectId, prNumber, previousResult); + try { await withProjectOrNull(projectId, async (project) => { - const { sendProgress, sendError, sendComplete } = createIPCCommunicators< - PRReviewProgress, - PRReviewResult - >( - mainWindow, - { - progress: IPC_CHANNELS.GITHUB_PR_REVIEW_PROGRESS, - error: IPC_CHANNELS.GITHUB_PR_REVIEW_ERROR, - complete: IPC_CHANNELS.GITHUB_PR_REVIEW_COMPLETE, - }, - projectId - ); + const sendProgress = (progress: PRReviewProgress): void => { + prReviewStateManager.handleProgress(projectId, prNumber, progress); + }; // Comprehensive validation of GitHub module const validation = await validateGitHubModule(project); if (!validation.valid) { - sendError({ prNumber, error: validation.error || "GitHub module validation failed" }); + prReviewStateManager.handleError(projectId, prNumber, validation.error || "GitHub module validation failed"); return; } @@ -3103,7 +3066,8 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v message: "Follow-up review complete!", }); - sendComplete(result.data!); + // Route through state manager + prReviewStateManager.handleComplete(projectId, prNumber, result.data!); } finally { // Always clean up registry, whether we exit normally or via error runningReviews.delete(reviewKey); @@ -3116,19 +3080,7 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v prNumber, error: error instanceof Error ? error.message : error, }); - const { sendError } = createIPCCommunicators( - mainWindow, - { - progress: IPC_CHANNELS.GITHUB_PR_REVIEW_PROGRESS, - error: IPC_CHANNELS.GITHUB_PR_REVIEW_ERROR, - complete: IPC_CHANNELS.GITHUB_PR_REVIEW_COMPLETE, - }, - projectId - ); - sendError({ - prNumber, - error: error instanceof Error ? error.message : "Failed to run follow-up review", - }); + prReviewStateManager.handleError(projectId, prNumber, error instanceof Error ? error.message : "Failed to run follow-up review"); } } ); From a0c8e89fee3658b3a0b5aacecf46fbea4fbd01f4 Mon Sep 17 00:00:00 2001 From: AndyMik90 Date: Sat, 14 Feb 2026 00:55:56 +0100 Subject: [PATCH 07/15] auto-claude: subtask-4-1 - Rewrite pr-review-store.ts as thin XState state subscriber Replace 6 imperative review lifecycle actions with a single handlePRReviewStateChange handler that maps XState state/context to the existing PRReviewState interface shape. Update initializePRReviewListeners to use onPRReviewStateChange IPC channel. Remove exported startPRReview/startFollowupReview helpers (now direct IPC calls from hooks). Co-Authored-By: Claude Opus 4.6 --- .../src/renderer/stores/github/index.ts | 4 +- .../renderer/stores/github/pr-review-store.ts | 285 ++++-------------- 2 files changed, 55 insertions(+), 234 deletions(-) diff --git a/apps/frontend/src/renderer/stores/github/index.ts b/apps/frontend/src/renderer/stores/github/index.ts index c20fa48b..ff241439 100644 --- a/apps/frontend/src/renderer/stores/github/index.ts +++ b/apps/frontend/src/renderer/stores/github/index.ts @@ -23,9 +23,7 @@ export { export { usePRReviewStore, initializePRReviewListeners, - cleanupPRReviewListeners, - startPRReview, - startFollowupReview + cleanupPRReviewListeners } from './pr-review-store'; import { initializePRReviewListeners as _initPRReviewListeners } from './pr-review-store'; import { cleanupPRReviewListeners as _cleanupPRReviewListeners } from './pr-review-store'; diff --git a/apps/frontend/src/renderer/stores/github/pr-review-store.ts b/apps/frontend/src/renderer/stores/github/pr-review-store.ts index b67ce18e..bb9b2f6b 100644 --- a/apps/frontend/src/renderer/stores/github/pr-review-store.ts +++ b/apps/frontend/src/renderer/stores/github/pr-review-store.ts @@ -2,7 +2,8 @@ import { create } from 'zustand'; import type { PRReviewProgress, PRReviewResult, - NewCommitsCheck + NewCommitsCheck, + PRReviewStatePayload } from '../../../preload/api/modules/github-api'; import type { ChecksStatus, @@ -44,12 +45,10 @@ interface PRReviewStoreState { // Key: `${projectId}:${prNumber}` prReviews: Record; - // Actions - startPRReview: (projectId: string, prNumber: number) => void; - startFollowupReview: (projectId: string, prNumber: number) => void; - setPRReviewProgress: (projectId: string, progress: PRReviewProgress) => void; - setPRReviewResult: (projectId: string, result: PRReviewResult, options?: { preserveNewCommitsCheck?: boolean }) => void; - setPRReviewError: (projectId: string, prNumber: number, error: string) => void; + // XState state change handler + handlePRReviewStateChange: (key: string, payload: PRReviewStatePayload) => void; + + // Kept actions (not managed by XState) setNewCommitsCheck: (projectId: string, prNumber: number, check: NewCommitsCheck) => void; clearPRReview: (projectId: string, prNumber: number) => void; /** Update PR status from polling (CI checks, reviews, mergeability) */ @@ -61,8 +60,6 @@ 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; @@ -80,147 +77,49 @@ export const usePRReviewStore = create((set, get) => ({ // Initial state prReviews: {}, - // Actions - startPRReview: (projectId: string, prNumber: number) => set((state) => { - const key = `${projectId}:${prNumber}`; - const existing = state.prReviews[key]; - return { - prReviews: { - ...state.prReviews, - [key]: { - prNumber, - projectId, - isReviewing: true, - startedAt: new Date().toISOString(), - progress: null, - result: null, - 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: false - } - } - }; - }), + // XState state change handler — maps XState state/context back to PRReviewState shape + handlePRReviewStateChange: (key: string, payload: PRReviewStatePayload) => { + const isCompleted = payload.state === 'completed'; - startFollowupReview: (projectId: string, prNumber: number) => set((state) => { - const key = `${projectId}:${prNumber}`; - const existing = state.prReviews[key]; + set((state) => { + const existing = state.prReviews[key]; - // Log warning if starting follow-up without a previous result - if (!existing?.result) { - console.warn( - `[PRReviewStore] Starting follow-up review for PR #${prNumber} without a previous result. ` + - `This may indicate the follow-up was triggered incorrectly.` - ); + const updated: PRReviewState = { + prNumber: payload.prNumber, + projectId: payload.projectId, + isReviewing: payload.state === 'reviewing' || payload.state === 'externalReview', + startedAt: payload.startedAt, + progress: payload.progress, + result: payload.result, + previousResult: payload.previousResult, + error: payload.error, + isExternalReview: payload.isExternalReview, + // Preserve polling data — not managed by XState + checksStatus: existing?.checksStatus ?? null, + reviewsStatus: existing?.reviewsStatus ?? null, + mergeableState: existing?.mergeableState ?? null, + lastPolled: existing?.lastPolled ?? null, + // Preserve newCommitsCheck unless review completed (it was just reviewed) + newCommitsCheck: isCompleted ? null : (existing?.newCommitsCheck ?? null), + }; + + return { + prReviews: { + ...state.prReviews, + [key]: updated, + }, + }; + }); + + // Trigger registered refresh callbacks when review completes + if (isCompleted) { + refreshCallbacks.forEach(callback => { + Promise.resolve(callback()).catch(error => { + console.error('[PRReviewStore] Error in refresh callback:', error); + }); + }); } - - return { - prReviews: { - ...state.prReviews, - [key]: { - prNumber, - projectId, - isReviewing: true, - startedAt: new Date().toISOString(), - progress: null, - result: null, - previousResult: existing?.result ?? null, // Preserve for follow-up continuity - error: null, - newCommitsCheck: existing?.newCommitsCheck ?? null, - checksStatus: existing?.checksStatus ?? null, - reviewsStatus: existing?.reviewsStatus ?? null, - mergeableState: existing?.mergeableState ?? null, - lastPolled: existing?.lastPolled ?? null, - isExternalReview: false - } - } - }; - }), - - setPRReviewProgress: (projectId: string, progress: PRReviewProgress) => set((state) => { - const key = `${projectId}:${progress.prNumber}`; - const existing = state.prReviews[key]; - return { - prReviews: { - ...state.prReviews, - [key]: { - prNumber: progress.prNumber, - projectId, - isReviewing: true, - startedAt: existing?.startedAt ?? null, - progress, - 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: existing?.isExternalReview ?? false - } - } - }; - }), - - setPRReviewResult: (projectId: string, result: PRReviewResult, options?: { preserveNewCommitsCheck?: boolean }) => set((state) => { - const key = `${projectId}:${result.prNumber}`; - const existing = state.prReviews[key]; - return { - prReviews: { - ...state.prReviews, - [key]: { - prNumber: result.prNumber, - projectId, - isReviewing: false, - startedAt: existing?.startedAt ?? null, - progress: null, - result, - previousResult: existing?.previousResult ?? null, - error: result.error ?? null, - // Clear new commits check when review completes (it was just reviewed) - // BUT preserve it during preload/refresh to avoid race condition - newCommitsCheck: options?.preserveNewCommitsCheck ? (existing?.newCommitsCheck ?? null) : null, - checksStatus: existing?.checksStatus ?? null, - reviewsStatus: existing?.reviewsStatus ?? null, - mergeableState: existing?.mergeableState ?? null, - lastPolled: existing?.lastPolled ?? null, - isExternalReview: existing?.isExternalReview ?? false - } - } - }; - }), - - setPRReviewError: (projectId: string, prNumber: number, error: string) => set((state) => { - const key = `${projectId}:${prNumber}`; - const existing = state.prReviews[key]; - return { - prReviews: { - ...state.prReviews, - [key]: { - prNumber, - projectId, - isReviewing: false, - startedAt: existing?.startedAt ?? null, - progress: null, - result: existing?.result ?? null, - previousResult: existing?.previousResult ?? null, - error, - newCommitsCheck: existing?.newCommitsCheck ?? null, - checksStatus: existing?.checksStatus ?? null, - reviewsStatus: existing?.reviewsStatus ?? null, - mergeableState: existing?.mergeableState ?? null, - lastPolled: existing?.lastPolled ?? null, - isExternalReview: existing?.isExternalReview ?? false - } - } - }; - }), + }, setNewCommitsCheck: (projectId: string, prNumber: number, check: NewCommitsCheck) => set((state) => { const key = `${projectId}:${prNumber}`; @@ -332,32 +231,6 @@ export const usePRReviewStore = create((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(); @@ -398,50 +271,18 @@ export function initializePRReviewListeners(): void { const store = usePRReviewStore.getState(); // Check if GitHub PR Review API is available - if (!window.electronAPI?.github?.onPRReviewProgress) { + if (!window.electronAPI?.github?.onPRReviewStateChange) { console.warn('[GitHub PR Store] GitHub PR Review API not available, skipping listener setup'); return; } - // Listen for PR review progress events - // Each on* method returns a cleanup function — capture them for proper teardown - const cleanupProgress = window.electronAPI.github.onPRReviewProgress( - (projectId: string, progress: PRReviewProgress) => { - store.setPRReviewProgress(projectId, progress); + // Listen for XState state changes — single handler replaces progress/complete/error listeners + const cleanupStateChange = window.electronAPI.github.onPRReviewStateChange( + (key: string, payload: PRReviewStatePayload) => { + store.handlePRReviewStateChange(key, payload); } ); - cleanupFunctions.push(cleanupProgress); - - // 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 => { - Promise.resolve(callback()).catch(error => { - console.error('[PRReviewStore] Error in refresh callback:', error); - }); - }); - } - ); - cleanupFunctions.push(cleanupComplete); - - // Listen for PR review error events - const cleanupError = window.electronAPI.github.onPRReviewError( - (projectId: string, data: { prNumber: number; error: string }) => { - store.setPRReviewError(projectId, data.prNumber, data.error); - } - ); - cleanupFunctions.push(cleanupError); + cleanupFunctions.push(cleanupStateChange); // Listen for GitHub auth changes - clear all PR review state when account changes const cleanupAuthChanged = window.electronAPI.github.onGitHubAuthChanged( @@ -457,7 +298,7 @@ export function initializePRReviewListeners(): void { cleanupFunctions.push(cleanupAuthChanged); // Listen for PR status polling updates (CI checks, reviews, mergeability) - window.electronAPI.github.onPRStatusUpdate( + const cleanupStatusUpdate = window.electronAPI.github.onPRStatusUpdate( (update: PRStatusUpdate) => { const { projectId, statuses } = update; for (const status of statuses) { @@ -470,6 +311,7 @@ export function initializePRReviewListeners(): void { } } ); + cleanupFunctions.push(cleanupStatusUpdate); prReviewListenersInitialized = true; } @@ -490,22 +332,3 @@ export function cleanupPRReviewListeners(): void { refreshCallbacks.clear(); prReviewListenersInitialized = false; } - -/** - * Start a PR review and track it in the store - */ -export function startPRReview(projectId: string, prNumber: number): void { - const store = usePRReviewStore.getState(); - store.startPRReview(projectId, prNumber); - window.electronAPI.github.runPRReview(projectId, prNumber); -} - -/** - * Start a follow-up PR review and track it in the store - * Uses startFollowupReview action to preserve previous result for continuity - */ -export function startFollowupReview(projectId: string, prNumber: number): void { - const store = usePRReviewStore.getState(); - store.startFollowupReview(projectId, prNumber); - window.electronAPI.github.runFollowupReview(projectId, prNumber); -} From 181fe50be7660a347d303cc81890232193c55ba2 Mon Sep 17 00:00:00 2001 From: AndyMik90 Date: Sat, 14 Feb 2026 00:58:35 +0100 Subject: [PATCH 08/15] auto-claude: subtask-4-2 - Update useGitHubPRs hook for new XState-driven architecture - Remove imports of deleted startPRReview/startFollowupReview store functions - runReview/runFollowupReview now call IPC directly (main process handles XState) - cancelReview no longer mutates store directly (state flows back via IPC) - Add setLoadedReviewResult action to pr-review-store for disk-loaded reviews - Replace all setPRReviewResult calls with setLoadedReviewResult Co-Authored-By: Claude Opus 4.6 --- .../github-prs/hooks/useGitHubPRs.ts | 32 +++++++------------ .../renderer/stores/github/pr-review-store.ts | 32 +++++++++++++++++++ 2 files changed, 44 insertions(+), 20 deletions(-) diff --git a/apps/frontend/src/renderer/components/github-prs/hooks/useGitHubPRs.ts b/apps/frontend/src/renderer/components/github-prs/hooks/useGitHubPRs.ts index bdaca8bd..9ed57644 100644 --- a/apps/frontend/src/renderer/components/github-prs/hooks/useGitHubPRs.ts +++ b/apps/frontend/src/renderer/components/github-prs/hooks/useGitHubPRs.ts @@ -7,8 +7,6 @@ import type { } from "../../../../preload/api/modules/github-api"; import { usePRReviewStore, - startPRReview as storeStartPRReview, - startFollowupReview as storeStartFollowupReview, } from "../../../stores/github"; // Re-export types for consumers @@ -204,7 +202,7 @@ export function useGitHubPRs( // Update store with loaded results for (const reviewResult of Object.values(batchReviews)) { if (reviewResult) { - usePRReviewStore.getState().setPRReviewResult(projectId, reviewResult, { + usePRReviewStore.getState().setLoadedReviewResult(projectId, reviewResult, { preserveNewCommitsCheck: true, }); } @@ -422,7 +420,7 @@ export function useGitHubPRs( // Preserve newCommitsCheck when loading existing review from disk usePRReviewStore .getState() - .setPRReviewResult(projectId, result, { preserveNewCommitsCheck: true }); + .setLoadedReviewResult(projectId, result, { preserveNewCommitsCheck: true }); // Always check for new commits when selecting a reviewed PR // This ensures fresh data even if we have a cached check from earlier in the session @@ -510,7 +508,7 @@ export function useGitHubPRs( // Update store with loaded results for (const reviewResult of Object.values(batchReviews)) { if (reviewResult) { - usePRReviewStore.getState().setPRReviewResult(requestProjectId, reviewResult, { + usePRReviewStore.getState().setLoadedReviewResult(requestProjectId, reviewResult, { preserveNewCommitsCheck: true, }); } @@ -534,8 +532,8 @@ export function useGitHubPRs( (prNumber: number) => { if (!projectId) return; - // Use the store function which handles both state and IPC - storeStartPRReview(projectId, prNumber); + // Main process handles XState state transition and subprocess launch + window.electronAPI.github.runPRReview(projectId, prNumber); }, [projectId] ); @@ -544,8 +542,8 @@ export function useGitHubPRs( (prNumber: number) => { if (!projectId) return; - // Use the store function which handles both state and IPC - storeStartFollowupReview(projectId, prNumber); + // Main process handles XState state transition and subprocess launch + window.electronAPI.github.runFollowupReview(projectId, prNumber); }, [projectId] ); @@ -575,15 +573,9 @@ export function useGitHubPRs( if (!projectId) return false; try { + // Main process kills subprocess and sends CANCEL_REVIEW to XState + // State update flows back via IPC (onPRReviewStateChange) const success = await window.electronAPI.github.cancelPRReview(projectId, prNumber); - // Always update store state to exit the "reviewing" state - // Use different messages based on whether the process was found and killed - const message = success - ? "Review cancelled by user" - : "Review stopped - process not found"; - usePRReviewStore - .getState() - .setPRReviewError(projectId, prNumber, message); return success; } catch (err) { setError(err instanceof Error ? err.message : "Failed to cancel review"); @@ -615,7 +607,7 @@ export function useGitHubPRs( // Preserve newCommitsCheck - posting doesn't change whether there are new commits usePRReviewStore .getState() - .setPRReviewResult(projectId, result, { preserveNewCommitsCheck: true }); + .setLoadedReviewResult(projectId, result, { preserveNewCommitsCheck: true }); } } return success; @@ -697,7 +689,7 @@ export function useGitHubPRs( const existingState = getPRReviewState(projectId, prNumber); if (existingState?.result) { // If we have the result loaded, update it with hasPostedFindings and postedAt - usePRReviewStore.getState().setPRReviewResult( + usePRReviewStore.getState().setLoadedReviewResult( projectId, { ...existingState.result, hasPostedFindings: true, postedAt }, { preserveNewCommitsCheck: true } @@ -706,7 +698,7 @@ export function useGitHubPRs( // If result not loaded yet (race condition), reload from disk to get updated state const result = await window.electronAPI.github.getPRReview(projectId, prNumber); if (result) { - usePRReviewStore.getState().setPRReviewResult( + usePRReviewStore.getState().setLoadedReviewResult( projectId, result, { preserveNewCommitsCheck: true } diff --git a/apps/frontend/src/renderer/stores/github/pr-review-store.ts b/apps/frontend/src/renderer/stores/github/pr-review-store.ts index bb9b2f6b..ff4de82a 100644 --- a/apps/frontend/src/renderer/stores/github/pr-review-store.ts +++ b/apps/frontend/src/renderer/stores/github/pr-review-store.ts @@ -49,6 +49,8 @@ interface PRReviewStoreState { handlePRReviewStateChange: (key: string, payload: PRReviewStatePayload) => void; // Kept actions (not managed by XState) + /** Load a review result from disk into the store (not triggered by XState) */ + setLoadedReviewResult: (projectId: string, result: PRReviewResult, options?: { preserveNewCommitsCheck?: boolean }) => void; setNewCommitsCheck: (projectId: string, prNumber: number, check: NewCommitsCheck) => void; clearPRReview: (projectId: string, prNumber: number) => void; /** Update PR status from polling (CI checks, reviews, mergeability) */ @@ -121,6 +123,36 @@ export const usePRReviewStore = create((set, get) => ({ } }, + setLoadedReviewResult: (projectId: string, result: PRReviewResult, options?: { preserveNewCommitsCheck?: boolean }) => set((state) => { + const key = `${projectId}:${result.prNumber}`; + const existing = state.prReviews[key]; + // Don't overwrite active review state from XState + if (existing?.isReviewing) { + return state; + } + return { + prReviews: { + ...state.prReviews, + [key]: { + prNumber: result.prNumber, + projectId, + isReviewing: false, + startedAt: null, + progress: null, + result, + previousResult: existing?.previousResult ?? null, + error: null, + newCommitsCheck: options?.preserveNewCommitsCheck ? (existing?.newCommitsCheck ?? null) : null, + checksStatus: existing?.checksStatus ?? null, + reviewsStatus: existing?.reviewsStatus ?? null, + mergeableState: existing?.mergeableState ?? null, + lastPolled: existing?.lastPolled ?? null, + isExternalReview: false, + }, + }, + }; + }), + setNewCommitsCheck: (projectId: string, prNumber: number, check: NewCommitsCheck) => set((state) => { const key = `${projectId}:${prNumber}`; const existing = state.prReviews[key]; From 16f3194ca0f44f41083dc5b52ae3789e33bfcd78 Mon Sep 17 00:00:00 2001 From: AndyMik90 Date: Sat, 14 Feb 2026 01:03:13 +0100 Subject: [PATCH 09/15] auto-claude: subtask-5-1 - Fix type errors and verify full test suite - Remove obsolete setPRReviewResult calls in PRDetail.tsx (now handled by XState) - Add missing onPRReviewStateChange to browser-mock.ts - Fix pr-review-machine.test.ts mock data to match actual PRReviewResult/PRReviewProgress types - All 130 test files pass (3278 tests), typecheck clean, lint clean Co-Authored-By: Claude Opus 4.6 --- .../components/github-prs/components/PRDetail.tsx | 15 ++++----------- apps/frontend/src/renderer/lib/browser-mock.ts | 1 + .../__tests__/pr-review-machine.test.ts | 13 ++++++++----- 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/apps/frontend/src/renderer/components/github-prs/components/PRDetail.tsx b/apps/frontend/src/renderer/components/github-prs/components/PRDetail.tsx index 3098ba6f..a8366bad 100644 --- a/apps/frontend/src/renderer/components/github-prs/components/PRDetail.tsx +++ b/apps/frontend/src/renderer/components/github-prs/components/PRDetail.tsx @@ -420,16 +420,8 @@ export function PRDetail({ 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', - }); + // XState now handles external review state - timeout is managed by the state machine + console.warn('[PRDetail] External review polling timed out after 30 minutes'); return; } @@ -440,7 +432,8 @@ export function PRDetail({ // 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); + // XState now handles review completion via IPC events - no need to manually set result + console.log('[PRDetail] External review completed, result handled by XState'); } } } catch { diff --git a/apps/frontend/src/renderer/lib/browser-mock.ts b/apps/frontend/src/renderer/lib/browser-mock.ts index e933054e..44a04fc8 100644 --- a/apps/frontend/src/renderer/lib/browser-mock.ts +++ b/apps/frontend/src/renderer/lib/browser-mock.ts @@ -225,6 +225,7 @@ const browserMockAPI: ElectronAPI = { onPRReviewProgress: () => () => {}, onPRReviewComplete: () => () => {}, onPRReviewError: () => () => {}, + onPRReviewStateChange: () => () => {}, onPRLogsUpdated: () => () => {}, batchAutoFix: () => {}, getBatches: async () => [], diff --git a/apps/frontend/src/shared/state-machines/__tests__/pr-review-machine.test.ts b/apps/frontend/src/shared/state-machines/__tests__/pr-review-machine.test.ts index b7946beb..4df160de 100644 --- a/apps/frontend/src/shared/state-machines/__tests__/pr-review-machine.test.ts +++ b/apps/frontend/src/shared/state-machines/__tests__/pr-review-machine.test.ts @@ -19,17 +19,20 @@ function runEvents(events: PRReviewEvent[]) { } const mockResult = { + prNumber: 42, + repo: 'test/repo', + success: true, + findings: [], summary: 'Test review', - rating: 'approve' as const, - comments: [], - filesReviewed: 3, - issuesFound: 0, + overallStatus: 'approve' as const, + reviewedAt: new Date().toISOString(), }; const mockProgress = { phase: 'analyzing' as const, + prNumber: 42, + progress: 50, message: 'Analyzing files...', - percentage: 50, }; describe('prReviewMachine', () => { From b3d6bbdeda1ed3fd765bc800abadf6916e95fd3a Mon Sep 17 00:00:00 2001 From: AndyMik90 Date: Sat, 14 Feb 2026 08:49:57 +0100 Subject: [PATCH 10/15] =?UTF-8?q?fix:=20resolve=20IPC=20serialization,=20a?= =?UTF-8?q?uth=20change=20wiring,=20and=20error=E2=86=92followup=20transit?= =?UTF-8?q?ion=20(qa-requested)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: - Build PRReviewStatePayload object in emitStateToRenderer instead of sending raw args - Wire up handleAuthChange via ipcMain listener for GITHUB_AUTH_CHANGED - Add START_FOLLOWUP_REVIEW transition to error state in pr-review-machine - Update state manager tests to verify PRReviewStatePayload shape Verified: - All 3279 tests pass - TypeScript compilation clean QA Fix Session: 1 Co-Authored-By: Claude Opus 4.6 --- .../__tests__/pr-review-state-manager.test.ts | 29 ++++++++++++++----- .../ipc-handlers/github/oauth-handlers.ts | 2 ++ .../main/ipc-handlers/github/pr-handlers.ts | 5 ++++ .../src/main/pr-review-state-manager.ts | 21 +++++++++++--- .../__tests__/pr-review-machine.test.ts | 14 +++++++++ .../state-machines/pr-review-machine.ts | 4 +++ 6 files changed, 64 insertions(+), 11 deletions(-) diff --git a/apps/frontend/src/main/__tests__/pr-review-state-manager.test.ts b/apps/frontend/src/main/__tests__/pr-review-state-manager.test.ts index 3661fe4d..02f7135b 100644 --- a/apps/frontend/src/main/__tests__/pr-review-state-manager.test.ts +++ b/apps/frontend/src/main/__tests__/pr-review-state-manager.test.ts @@ -160,20 +160,35 @@ describe('PRReviewStateManager', () => { expect.any(Function), 'github:pr:reviewStateChange', expect.any(String), - expect.any(String), - expect.anything() + expect.objectContaining({ state: expect.any(String) }) ); }); - it('should include state value and context in payload', () => { + it('should emit PRReviewStatePayload with correct shape', () => { manager.handleStartReview(projectId, prNumber); // Find the call that emits 'reviewing' state const reviewingCall = mockSafeSendToRenderer.mock.calls.find( - (call: unknown[]) => call[3] === 'reviewing' + (call: unknown[]) => { + const payload = call[3] as Record | undefined; + return payload && typeof payload === 'object' && payload.state === 'reviewing'; + } ); expect(reviewingCall).toBeDefined(); expect(reviewingCall![2]).toBe(`${projectId}:${prNumber}`); - expect(reviewingCall![4]).toEqual(expect.objectContaining({ prNumber, projectId })); + const payload = reviewingCall![3] as Record; + expect(payload).toEqual(expect.objectContaining({ + state: 'reviewing', + prNumber, + projectId, + isReviewing: true, + startedAt: expect.any(String), + progress: null, + result: null, + previousResult: null, + error: null, + isExternalReview: false, + isFollowup: false, + })); }); it('should use projectId:prNumber as key format', () => { @@ -244,8 +259,8 @@ describe('PRReviewStateManager', () => { // Should emit idle/null state for each PR expect(mockSafeSendToRenderer).toHaveBeenCalledTimes(2); for (const call of mockSafeSendToRenderer.mock.calls) { - expect(call[3]).toBe('idle'); // stateValue - expect(call[4]).toBeNull(); // context + const payload = call[3] as Record; + expect(payload).toEqual(expect.objectContaining({ state: 'idle' })); } }); diff --git a/apps/frontend/src/main/ipc-handlers/github/oauth-handlers.ts b/apps/frontend/src/main/ipc-handlers/github/oauth-handlers.ts index 773feb15..e1b40b1d 100644 --- a/apps/frontend/src/main/ipc-handlers/github/oauth-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/github/oauth-handlers.ts @@ -51,6 +51,8 @@ function sendAuthChangedToRenderer(oldUsername: string | null, newUsername: stri for (const win of windows) { win.webContents.send(IPC_CHANNELS.GITHUB_AUTH_CHANGED, payload); } + // Emit on ipcMain so main-process listeners (e.g., PRReviewStateManager) can react + ipcMain.emit(IPC_CHANNELS.GITHUB_AUTH_CHANGED, payload); } /** 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 e56810f9..35ef0bc7 100644 --- a/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts @@ -1670,6 +1670,11 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v // Create the XState-based PR review state manager const prReviewStateManager = new PRReviewStateManager(getMainWindow); + // Clear all PR review actors when GitHub auth changes (account swap) + ipcMain.on(IPC_CHANNELS.GITHUB_AUTH_CHANGED, () => { + prReviewStateManager.handleAuthChange(); + }); + // List open PRs - fetches up to 100 open PRs at once, returns hasNextPage and endCursor from API ipcMain.handle( IPC_CHANNELS.GITHUB_PR_LIST, diff --git a/apps/frontend/src/main/pr-review-state-manager.ts b/apps/frontend/src/main/pr-review-state-manager.ts index 01ba68fb..301a3c4a 100644 --- a/apps/frontend/src/main/pr-review-state-manager.ts +++ b/apps/frontend/src/main/pr-review-state-manager.ts @@ -2,7 +2,7 @@ import { createActor } from 'xstate'; import type { ActorRefFrom } from 'xstate'; import type { BrowserWindow } from 'electron'; import { prReviewMachine, type PRReviewEvent, type PRReviewContext } from '../shared/state-machines'; -import type { PRReviewProgress, PRReviewResult } from '../preload/api/modules/github-api'; +import type { PRReviewProgress, PRReviewResult, PRReviewStatePayload } from '../preload/api/modules/github-api'; import { IPC_CHANNELS } from '../shared/constants'; import { safeSendToRenderer } from './ipc-handlers/utils'; @@ -149,14 +149,27 @@ export class PRReviewStateManager { snapshot: ReturnType | null ): void { const stateValue = snapshot ? String(snapshot.value) : 'idle'; - const context = snapshot ? snapshot.context : null; + const ctx = snapshot?.context ?? null; + + const payload: PRReviewStatePayload = { + state: stateValue, + prNumber: ctx?.prNumber ?? 0, + projectId: ctx?.projectId ?? '', + isReviewing: stateValue === 'reviewing' || stateValue === 'externalReview', + startedAt: ctx?.startedAt ?? null, + progress: ctx?.progress ?? null, + result: ctx?.result ?? null, + previousResult: ctx?.previousResult ?? null, + error: ctx?.error ?? null, + isExternalReview: ctx?.isExternalReview ?? false, + isFollowup: ctx?.isFollowup ?? false, + }; safeSendToRenderer( this.getMainWindow, IPC_CHANNELS.GITHUB_PR_REVIEW_STATE_CHANGE, key, - stateValue, - context + payload ); } } diff --git a/apps/frontend/src/shared/state-machines/__tests__/pr-review-machine.test.ts b/apps/frontend/src/shared/state-machines/__tests__/pr-review-machine.test.ts index 4df160de..f2e5d548 100644 --- a/apps/frontend/src/shared/state-machines/__tests__/pr-review-machine.test.ts +++ b/apps/frontend/src/shared/state-machines/__tests__/pr-review-machine.test.ts @@ -152,6 +152,20 @@ describe('prReviewMachine', () => { expect(snapshot.context.error).toBeNull(); expect(snapshot.context.result).toEqual(mockResult); }); + + it('should allow starting a follow-up review from error state with previousResult preserved', () => { + const snapshot = runEvents([ + { type: 'START_REVIEW', prNumber: 42, projectId: 'proj-1' }, + { type: 'REVIEW_COMPLETE', result: mockResult }, + { type: 'START_FOLLOWUP_REVIEW', prNumber: 42, projectId: 'proj-1', previousResult: mockResult }, + { type: 'REVIEW_ERROR', error: 'Follow-up failed' }, + { type: 'START_FOLLOWUP_REVIEW', prNumber: 42, projectId: 'proj-1', previousResult: mockResult }, + ]); + + expect(snapshot.value).toBe('reviewing'); + expect(snapshot.context.previousResult).toEqual(mockResult); + expect(snapshot.context.isFollowup).toBe(true); + }); }); describe('clear review', () => { diff --git a/apps/frontend/src/shared/state-machines/pr-review-machine.ts b/apps/frontend/src/shared/state-machines/pr-review-machine.ts index fd14698a..8ae075a8 100644 --- a/apps/frontend/src/shared/state-machines/pr-review-machine.ts +++ b/apps/frontend/src/shared/state-machines/pr-review-machine.ts @@ -118,6 +118,10 @@ export const prReviewMachine = createMachine( target: 'reviewing', actions: 'setReviewStart', }, + START_FOLLOWUP_REVIEW: { + target: 'reviewing', + actions: 'setFollowupReviewStart', + }, CLEAR_REVIEW: { target: 'idle', actions: 'clearContext', From 1b65a7ba2357aea8e8fae54ef16d54e1be4802f7 Mon Sep 17 00:00:00 2001 From: AndyMik90 Date: Sat, 14 Feb 2026 10:25:15 +0100 Subject: [PATCH 11/15] fix: resolve XState gaps in external review polling, cancel/clear handlers, and result updates - Add CANCEL_REVIEW and CLEAR_REVIEW handlers to externalReview state so users can cancel or clear while an external review is in progress - Add REVIEW_COMPLETE self-transition in completed state so sendReviewStateUpdate can update result context (e.g., after posting findings or marking as posted) - Add notifyExternalReviewComplete IPC channel so renderer polling can notify the main process when an external review finishes on disk or times out, transitioning the XState actor to completed/error instead of leaving it stuck in externalReview - Wire PRDetail.tsx external review polling to use the new IPC notification Co-Authored-By: Claude Opus 4.6 --- .../src/main/ipc-handlers/github/pr-handlers.ts | 15 +++++++++++++++ .../src/preload/api/modules/github-api.ts | 7 +++++++ .../components/github-prs/components/PRDetail.tsx | 7 ++++--- apps/frontend/src/renderer/lib/browser-mock.ts | 1 + apps/frontend/src/shared/constants/ipc.ts | 1 + .../shared/state-machines/pr-review-machine.ts | 11 +++++++++++ 6 files changed, 39 insertions(+), 3 deletions(-) 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 35ef0bc7..ddb79edb 100644 --- a/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts @@ -2503,6 +2503,21 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v } ); + // Notify main process about external review completion or timeout + // Called by renderer when its polling detects an external review has finished on disk + ipcMain.handle( + IPC_CHANNELS.GITHUB_PR_NOTIFY_EXTERNAL_REVIEW_COMPLETE, + async (_, projectId: string, prNumber: number, result: PRReviewResult | null): Promise => { + debugLog("notifyExternalReviewComplete handler called", { projectId, prNumber, hasResult: !!result }); + if (result) { + prReviewStateManager.handleComplete(projectId, prNumber, result); + } else { + // Timeout — no result found within polling window + prReviewStateManager.handleError(projectId, prNumber, "External review timed out after 30 minutes"); + } + } + ); + // Check for new commits since last review ipcMain.handle( IPC_CHANNELS.GITHUB_PR_CHECK_NEW_COMMITS, diff --git a/apps/frontend/src/preload/api/modules/github-api.ts b/apps/frontend/src/preload/api/modules/github-api.ts index 7c836589..c2115eb1 100644 --- a/apps/frontend/src/preload/api/modules/github-api.ts +++ b/apps/frontend/src/preload/api/modules/github-api.ts @@ -285,6 +285,9 @@ export interface GitHubAPI { getPRReview: (projectId: string, prNumber: number) => Promise; getPRReviewsBatch: (projectId: string, prNumbers: number[]) => Promise>; + // External review notification (renderer tells main process about external review completion/timeout) + notifyExternalReviewComplete: (projectId: string, prNumber: number, result: PRReviewResult | null) => Promise; + // Follow-up review operations checkNewCommits: (projectId: string, prNumber: number) => Promise; checkMergeReadiness: (projectId: string, prNumber: number) => Promise; @@ -760,6 +763,10 @@ export const createGitHubAPI = (): GitHubAPI => ({ getPRReviewsBatch: (projectId: string, prNumbers: number[]): Promise> => invokeIpc(IPC_CHANNELS.GITHUB_PR_GET_REVIEWS_BATCH, projectId, prNumbers), + // External review notification + notifyExternalReviewComplete: (projectId: string, prNumber: number, result: PRReviewResult | null): Promise => + invokeIpc(IPC_CHANNELS.GITHUB_PR_NOTIFY_EXTERNAL_REVIEW_COMPLETE, projectId, prNumber, result), + // Follow-up review operations checkNewCommits: (projectId: string, prNumber: number): Promise => invokeIpc(IPC_CHANNELS.GITHUB_PR_CHECK_NEW_COMMITS, projectId, prNumber), diff --git a/apps/frontend/src/renderer/components/github-prs/components/PRDetail.tsx b/apps/frontend/src/renderer/components/github-prs/components/PRDetail.tsx index a8366bad..7e3950ed 100644 --- a/apps/frontend/src/renderer/components/github-prs/components/PRDetail.tsx +++ b/apps/frontend/src/renderer/components/github-prs/components/PRDetail.tsx @@ -420,8 +420,9 @@ export function PRDetail({ const pollForCompletion = async () => { // Timeout: stop polling after 30 minutes to avoid indefinite polling if (Date.now() - pollStart > MAX_POLL_DURATION_MS) { - // XState now handles external review state - timeout is managed by the state machine console.warn('[PRDetail] External review polling timed out after 30 minutes'); + // Notify main process so the XState actor transitions to error state + await window.electronAPI.github.notifyExternalReviewComplete(projectId, pr.number, null); return; } @@ -432,8 +433,8 @@ export function PRDetail({ // 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)) { - // XState now handles review completion via IPC events - no need to manually set result - console.log('[PRDetail] External review completed, result handled by XState'); + // Notify main process so the XState actor transitions to completed state + await window.electronAPI.github.notifyExternalReviewComplete(projectId, pr.number, result); } } } catch { diff --git a/apps/frontend/src/renderer/lib/browser-mock.ts b/apps/frontend/src/renderer/lib/browser-mock.ts index 44a04fc8..d33756c9 100644 --- a/apps/frontend/src/renderer/lib/browser-mock.ts +++ b/apps/frontend/src/renderer/lib/browser-mock.ts @@ -214,6 +214,7 @@ const browserMockAPI: ElectronAPI = { markReviewPosted: async () => true, getPRReview: async () => null, getPRReviewsBatch: async () => ({}), + notifyExternalReviewComplete: async () => {}, deletePRReview: async () => true, checkNewCommits: async () => ({ hasNewCommits: false, newCommitCount: 0 }), checkMergeReadiness: async () => ({ isDraft: false, mergeable: 'UNKNOWN' as const, isBehind: false, ciStatus: 'none' as const, blockers: [] }), diff --git a/apps/frontend/src/shared/constants/ipc.ts b/apps/frontend/src/shared/constants/ipc.ts index 8af598b8..65bc51a4 100644 --- a/apps/frontend/src/shared/constants/ipc.ts +++ b/apps/frontend/src/shared/constants/ipc.ts @@ -408,6 +408,7 @@ export const IPC_CHANNELS = { GITHUB_PR_CHECK_MERGE_READINESS: 'github:pr:checkMergeReadiness', GITHUB_PR_MARK_REVIEW_POSTED: 'github:pr:markReviewPosted', GITHUB_PR_UPDATE_BRANCH: 'github:pr:updateBranch', + GITHUB_PR_NOTIFY_EXTERNAL_REVIEW_COMPLETE: 'github:pr:notifyExternalReviewComplete', // GitHub PR Review events (main -> renderer) GITHUB_PR_REVIEW_PROGRESS: 'github:pr:reviewProgress', diff --git a/apps/frontend/src/shared/state-machines/pr-review-machine.ts b/apps/frontend/src/shared/state-machines/pr-review-machine.ts index 8ae075a8..43039031 100644 --- a/apps/frontend/src/shared/state-machines/pr-review-machine.ts +++ b/apps/frontend/src/shared/state-machines/pr-review-machine.ts @@ -94,6 +94,14 @@ export const prReviewMachine = createMachine( target: 'error', actions: 'setError', }, + CANCEL_REVIEW: { + target: 'error', + actions: 'setCancelledError', + }, + CLEAR_REVIEW: { + target: 'idle', + actions: 'clearContext', + }, }, }, completed: { @@ -106,6 +114,9 @@ export const prReviewMachine = createMachine( target: 'reviewing', actions: 'setFollowupReviewStart', }, + REVIEW_COMPLETE: { + actions: 'setResult', + }, CLEAR_REVIEW: { target: 'idle', actions: 'clearContext', From 947fc3c6f802ce10c9a8d055681e73bd7e7a0e64 Mon Sep 17 00:00:00 2001 From: AndyMik90 Date: Sat, 14 Feb 2026 11:25:15 +0100 Subject: [PATCH 12/15] =?UTF-8?q?fix:=20address=20follow-up=20review=20fin?= =?UTF-8?q?dings=20=E2=80=94=20unhandled=20rejection,=20auth=20payload,=20?= =?UTF-8?q?polling=20guard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Wrap timeout-branch IPC call in try/catch to prevent unhandled promise rejection in the Electron renderer when notifyExternalReviewComplete fails - Add `notified` flag to short-circuit duplicate polling notifications before React cleanup fires - Use last snapshot context in handleAuthChange so emitted cleared-state payload contains real projectId/prNumber instead of zeros - Move handleStartReview after duplicate-review check so state machine isn't transitioned for already-running reviews Co-Authored-By: Claude Opus 4.6 --- .../main/ipc-handlers/github/pr-handlers.ts | 6 ++-- .../src/main/pr-review-state-manager.ts | 32 ++++++++++++++++++- .../github-prs/components/PRDetail.tsx | 15 +++++++-- 3 files changed, 47 insertions(+), 6 deletions(-) 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 ddb79edb..8cb38141 100644 --- a/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts @@ -1880,9 +1880,6 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v const reviewKey = getReviewKey(projectId, prNumber); - // Notify state manager that review is starting - prReviewStateManager.handleStartReview(projectId, prNumber); - try { await withProjectOrNull(projectId, async (project) => { const sendProgress = (progress: PRReviewProgress): void => { @@ -1901,6 +1898,9 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v return; } + // Notify state manager that review is starting (after duplicate check) + prReviewStateManager.handleStartReview(projectId, prNumber); + // Register as running BEFORE CI wait to prevent race conditions // Use CI_WAIT_PLACEHOLDER sentinel until real process is spawned runningReviews.set(reviewKey, CI_WAIT_PLACEHOLDER); diff --git a/apps/frontend/src/main/pr-review-state-manager.ts b/apps/frontend/src/main/pr-review-state-manager.ts index 301a3c4a..8db89d80 100644 --- a/apps/frontend/src/main/pr-review-state-manager.ts +++ b/apps/frontend/src/main/pr-review-state-manager.ts @@ -89,9 +89,12 @@ export class PRReviewStateManager { handleAuthChange(): void { for (const [key, actor] of this.actors) { + // Capture the last known snapshot before stopping so the emitted payload + // contains the real projectId/prNumber instead of zeros. + const snapshot = actor.getSnapshot(); actor.stop(); // Emit cleared (idle) state to renderer for each PR - this.emitStateToRenderer(key, null); + this.emitClearedState(key, snapshot?.context ?? null); } this.actors.clear(); this.lastStateByPR.clear(); @@ -172,4 +175,31 @@ export class PRReviewStateManager { payload ); } + + /** + * Emit a cleared (idle) state using context from the last snapshot + * so the payload contains the real projectId/prNumber. + */ + private emitClearedState(key: string, ctx: PRReviewContext | null): void { + const payload: PRReviewStatePayload = { + state: 'idle', + prNumber: ctx?.prNumber ?? 0, + projectId: ctx?.projectId ?? '', + isReviewing: false, + startedAt: null, + progress: null, + result: null, + previousResult: null, + error: null, + isExternalReview: false, + isFollowup: false, + }; + + safeSendToRenderer( + this.getMainWindow, + IPC_CHANNELS.GITHUB_PR_REVIEW_STATE_CHANGE, + key, + payload + ); + } } diff --git a/apps/frontend/src/renderer/components/github-prs/components/PRDetail.tsx b/apps/frontend/src/renderer/components/github-prs/components/PRDetail.tsx index 7e3950ed..655c6e77 100644 --- a/apps/frontend/src/renderer/components/github-prs/components/PRDetail.tsx +++ b/apps/frontend/src/renderer/components/github-prs/components/PRDetail.tsx @@ -417,12 +417,22 @@ export function PRDetail({ const MAX_POLL_DURATION_MS = 30 * 60 * 1000; // 30 minutes const pollStart = Date.now(); + let notified = false; + const pollForCompletion = async () => { + // Skip if we already notified (prevents duplicate notifications before React cleanup) + if (notified) return; + // Timeout: stop polling after 30 minutes to avoid indefinite polling if (Date.now() - pollStart > MAX_POLL_DURATION_MS) { console.warn('[PRDetail] External review polling timed out after 30 minutes'); - // Notify main process so the XState actor transitions to error state - await window.electronAPI.github.notifyExternalReviewComplete(projectId, pr.number, null); + notified = true; + try { + // Notify main process so the XState actor transitions to error state + await window.electronAPI.github.notifyExternalReviewComplete(projectId, pr.number, null); + } catch { + // Non-critical — state manager timeout is a best-effort notification + } return; } @@ -433,6 +443,7 @@ export function PRDetail({ // 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)) { + notified = true; // Notify main process so the XState actor transitions to completed state await window.electronAPI.github.notifyExternalReviewComplete(projectId, pr.number, result); } From 58fec15cd23701ba2adebdec50d55a8aa506a119 Mon Sep 17 00:00:00 2001 From: AndyMik90 Date: Sat, 14 Feb 2026 15:20:23 +0100 Subject: [PATCH 13/15] fix: prevent OOM, orphaned agents, and unbounded growth during overnight builds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Move handleStartFollowupReview after runningReviews.has() duplicate check to prevent XState actor getting stuck in reviewing state when follow-up is already running (mirroring the regular review handler pattern) - Add CLEAR_REVIEW handler to reviewing state so handleClearReview works correctly when an active review is cleared - Remove dead-code START_REVIEW transition and always-false isNotAlreadyReviewing guard from reviewing state — duplicate prevention is handled at the IPC layer Co-Authored-By: Claude Opus 4.6 --- .../main/ipc-handlers/github/pr-handlers.ts | 29 +++++++++---------- .../__tests__/pr-review-machine.test.ts | 2 +- .../state-machines/pr-review-machine.ts | 11 +++---- 3 files changed, 19 insertions(+), 23 deletions(-) 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 8cb38141..a7601013 100644 --- a/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts @@ -2901,20 +2901,26 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v return; } - // Get previous result for followup context - const previousResult = await withProjectOrNull(projectId, async (project) => { - return getReviewResult(project, prNumber) ?? undefined; - }) ?? undefined; - - // Notify state manager that followup review is starting - prReviewStateManager.handleStartFollowupReview(projectId, prNumber, previousResult); - try { await withProjectOrNull(projectId, async (project) => { const sendProgress = (progress: PRReviewProgress): void => { prReviewStateManager.handleProgress(projectId, prNumber, progress); }; + const reviewKey = getReviewKey(projectId, prNumber); + + // Check if already running — before notifying state manager to avoid stuck reviewing state + if (runningReviews.has(reviewKey)) { + debugLog("Follow-up review already running", { reviewKey }); + return; + } + + // Get previous result for followup context + const previousResult = getReviewResult(project, prNumber) ?? undefined; + + // Notify state manager that followup review is starting (after duplicate check) + prReviewStateManager.handleStartFollowupReview(projectId, prNumber, previousResult); + // Comprehensive validation of GitHub module const validation = await validateGitHubModule(project); if (!validation.valid) { @@ -2923,13 +2929,6 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v } const backendPath = validation.backendPath!; - const reviewKey = getReviewKey(projectId, prNumber); - - // Check if already running - if (runningReviews.has(reviewKey)) { - debugLog("Follow-up review already running", { reviewKey }); - return; - } // Register as running BEFORE CI wait to prevent race conditions // Use CI_WAIT_PLACEHOLDER sentinel until real process is spawned diff --git a/apps/frontend/src/shared/state-machines/__tests__/pr-review-machine.test.ts b/apps/frontend/src/shared/state-machines/__tests__/pr-review-machine.test.ts index f2e5d548..6a7b1446 100644 --- a/apps/frontend/src/shared/state-machines/__tests__/pr-review-machine.test.ts +++ b/apps/frontend/src/shared/state-machines/__tests__/pr-review-machine.test.ts @@ -193,7 +193,7 @@ describe('prReviewMachine', () => { }); }); - describe('guard: reject START_REVIEW when already reviewing', () => { + describe('reject START_REVIEW when already reviewing', () => { it('should stay in reviewing when START_REVIEW is sent again', () => { const snapshot = runEvents([ { type: 'START_REVIEW', prNumber: 42, projectId: 'proj-1' }, diff --git a/apps/frontend/src/shared/state-machines/pr-review-machine.ts b/apps/frontend/src/shared/state-machines/pr-review-machine.ts index 43039031..17240cea 100644 --- a/apps/frontend/src/shared/state-machines/pr-review-machine.ts +++ b/apps/frontend/src/shared/state-machines/pr-review-machine.ts @@ -59,10 +59,6 @@ export const prReviewMachine = createMachine( }, reviewing: { on: { - START_REVIEW: { - target: 'reviewing', - guard: 'isNotAlreadyReviewing', - }, SET_PROGRESS: { actions: 'setProgress', }, @@ -78,6 +74,10 @@ export const prReviewMachine = createMachine( target: 'error', actions: 'setCancelledError', }, + CLEAR_REVIEW: { + target: 'idle', + actions: 'clearContext', + }, DETECT_EXTERNAL_REVIEW: { target: 'externalReview', actions: 'setExternalReview', @@ -142,9 +142,6 @@ export const prReviewMachine = createMachine( }, }, { - guards: { - isNotAlreadyReviewing: () => false, - }, actions: { setReviewStart: assign({ prNumber: ({ event }) => (event as { prNumber: number }).prNumber, From 4a151aeb54440b96f05a78e156470eddb6112058 Mon Sep 17 00:00:00 2001 From: AndyMik90 Date: Sun, 15 Feb 2026 14:09:08 +0100 Subject: [PATCH 14/15] =?UTF-8?q?fix:=20address=20low-severity=20review=20?= =?UTF-8?q?findings=20=E2=80=94=20stale=20progress,=20dead=20code,=20missi?= =?UTF-8?q?ng=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Clear stale progress data when transitioning to externalReview state - Capture snapshot before CLEAR_REVIEW so emitted payload has real context - Add user feedback for duplicate follow-up review requests - Remove dead clearPRReview code from store (zero callers) - Add test coverage for CLEAR_REVIEW from reviewing/externalReview states - Clarify ipcMain.emit comment re: EventEmitter semantics Co-Authored-By: Claude Opus 4.6 --- .../ipc-handlers/github/oauth-handlers.ts | 4 +++- .../main/ipc-handlers/github/pr-handlers.ts | 10 ++++++-- .../src/main/pr-review-state-manager.ts | 3 +++ .../renderer/stores/github/pr-review-store.ts | 6 ----- .../__tests__/pr-review-machine.test.ts | 23 +++++++++++++++++++ .../state-machines/pr-review-machine.ts | 1 + 6 files changed, 38 insertions(+), 9 deletions(-) diff --git a/apps/frontend/src/main/ipc-handlers/github/oauth-handlers.ts b/apps/frontend/src/main/ipc-handlers/github/oauth-handlers.ts index e1b40b1d..37adb6bb 100644 --- a/apps/frontend/src/main/ipc-handlers/github/oauth-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/github/oauth-handlers.ts @@ -51,7 +51,9 @@ function sendAuthChangedToRenderer(oldUsername: string | null, newUsername: stri for (const win of windows) { win.webContents.send(IPC_CHANNELS.GITHUB_AUTH_CHANGED, payload); } - // Emit on ipcMain so main-process listeners (e.g., PRReviewStateManager) can react + // Uses EventEmitter.emit (not IPC send) so main-process listeners can react. + // The listener (PRReviewStateManager) intentionally ignores all args — it only + // needs the event signal, not the payload. ipcMain.emit(IPC_CHANNELS.GITHUB_AUTH_CHANGED, payload); } 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 a7601013..0fc2e874 100644 --- a/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts @@ -2909,9 +2909,15 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v const reviewKey = getReviewKey(projectId, prNumber); - // Check if already running — before notifying state manager to avoid stuck reviewing state + // Check if already running — notify renderer so it can display ongoing logs if (runningReviews.has(reviewKey)) { - debugLog("Follow-up review already running", { reviewKey }); + debugLog("Follow-up review already running, notifying renderer", { reviewKey }); + sendProgress({ + phase: "analyzing", + prNumber, + progress: 50, + message: "Follow-up review is already in progress. Reconnecting to ongoing review...", + }); return; } diff --git a/apps/frontend/src/main/pr-review-state-manager.ts b/apps/frontend/src/main/pr-review-state-manager.ts index 8db89d80..ad79f1b9 100644 --- a/apps/frontend/src/main/pr-review-state-manager.ts +++ b/apps/frontend/src/main/pr-review-state-manager.ts @@ -80,9 +80,12 @@ export class PRReviewStateManager { const key = this.getKey(projectId, prNumber); const actor = this.actors.get(key); if (actor) { + // Capture snapshot before clearing so the emitted payload has real context + const snapshot = actor.getSnapshot(); actor.send({ type: 'CLEAR_REVIEW' } satisfies PRReviewEvent); actor.stop(); this.actors.delete(key); + this.emitClearedState(key, snapshot?.context ?? null); } this.lastStateByPR.delete(key); } diff --git a/apps/frontend/src/renderer/stores/github/pr-review-store.ts b/apps/frontend/src/renderer/stores/github/pr-review-store.ts index ff4de82a..b790ee24 100644 --- a/apps/frontend/src/renderer/stores/github/pr-review-store.ts +++ b/apps/frontend/src/renderer/stores/github/pr-review-store.ts @@ -52,7 +52,6 @@ interface PRReviewStoreState { /** Load a review result from disk into the store (not triggered by XState) */ setLoadedReviewResult: (projectId: string, result: PRReviewResult, options?: { preserveNewCommitsCheck?: boolean }) => void; setNewCommitsCheck: (projectId: string, prNumber: number, check: NewCommitsCheck) => void; - clearPRReview: (projectId: string, prNumber: number) => void; /** Update PR status from polling (CI checks, reviews, mergeability) */ setPRStatus: (projectId: string, prNumber: number, status: { checksStatus: ChecksStatus; @@ -191,11 +190,6 @@ export const usePRReviewStore = create((set, get) => ({ }; }), - clearPRReview: (projectId: string, prNumber: number) => set((state) => { - const key = `${projectId}:${prNumber}`; - const { [key]: _, ...rest } = state.prReviews; - return { prReviews: rest }; - }), setPRStatus: (projectId: string, prNumber: number, status: { checksStatus: ChecksStatus; diff --git a/apps/frontend/src/shared/state-machines/__tests__/pr-review-machine.test.ts b/apps/frontend/src/shared/state-machines/__tests__/pr-review-machine.test.ts index 6a7b1446..f5b5319d 100644 --- a/apps/frontend/src/shared/state-machines/__tests__/pr-review-machine.test.ts +++ b/apps/frontend/src/shared/state-machines/__tests__/pr-review-machine.test.ts @@ -191,6 +191,29 @@ describe('prReviewMachine', () => { expect(snapshot.value).toBe('idle'); expect(snapshot.context.error).toBeNull(); }); + + it('should clear from reviewing to idle', () => { + const snapshot = runEvents([ + { type: 'START_REVIEW', prNumber: 42, projectId: 'proj-1' }, + { type: 'CLEAR_REVIEW' }, + ]); + + expect(snapshot.value).toBe('idle'); + expect(snapshot.context.prNumber).toBeNull(); + expect(snapshot.context.projectId).toBeNull(); + }); + + it('should clear from externalReview to idle', () => { + const snapshot = runEvents([ + { type: 'START_REVIEW', prNumber: 42, projectId: 'proj-1' }, + { type: 'DETECT_EXTERNAL_REVIEW' }, + { type: 'CLEAR_REVIEW' }, + ]); + + expect(snapshot.value).toBe('idle'); + expect(snapshot.context.prNumber).toBeNull(); + expect(snapshot.context.isExternalReview).toBe(false); + }); }); describe('reject START_REVIEW when already reviewing', () => { diff --git a/apps/frontend/src/shared/state-machines/pr-review-machine.ts b/apps/frontend/src/shared/state-machines/pr-review-machine.ts index 17240cea..189002eb 100644 --- a/apps/frontend/src/shared/state-machines/pr-review-machine.ts +++ b/apps/frontend/src/shared/state-machines/pr-review-machine.ts @@ -182,6 +182,7 @@ export const prReviewMachine = createMachine( }), setExternalReview: assign({ isExternalReview: () => true, + progress: () => null, }), clearContext: assign(() => ({ ...initialContext })), }, From 191736df1ace73f1ce6ad54d36acea8988cbbf35 Mon Sep 17 00:00:00 2001 From: AndyMik90 Date: Tue, 17 Feb 2026 13:37:40 +0100 Subject: [PATCH 15/15] fix: address PR review findings for XState PR review refactor - handleComplete uses getOrCreateActor so late-arriving results after auth change or restart are processed instead of silently dropped - GITHUB_AUTH_CHANGED handler now kills running review subprocesses and aborts CI wait controllers before clearing XState actors - Duplicate review detection reads actual progress from actor snapshot instead of hardcoding progress=50 - handleClearReview stops actor directly without sending CLEAR_REVIEW event, preventing double IPC emission to renderer Co-Authored-By: Claude Opus 4.6 --- .../__tests__/pr-review-state-manager.test.ts | 22 ++++++++++++++++ .../main/ipc-handlers/github/pr-handlers.ts | 26 +++++++++++++++++-- .../src/main/pr-review-state-manager.ts | 17 +++++++++--- 3 files changed, 59 insertions(+), 6 deletions(-) diff --git a/apps/frontend/src/main/__tests__/pr-review-state-manager.test.ts b/apps/frontend/src/main/__tests__/pr-review-state-manager.test.ts index 02f7135b..03d12646 100644 --- a/apps/frontend/src/main/__tests__/pr-review-state-manager.test.ts +++ b/apps/frontend/src/main/__tests__/pr-review-state-manager.test.ts @@ -124,6 +124,15 @@ describe('PRReviewStateManager', () => { expect(snapshot!.context.result).toEqual(result); }); + it('should create actor for handleComplete on unknown PR (late-arriving result)', () => { + const result = createMockResult(); + // No handleStartReview called — handleComplete should create the actor + manager.handleComplete(projectId, prNumber, result); + const snapshot = manager.getState(projectId, prNumber); + expect(snapshot).not.toBeNull(); + expect(snapshot!.context.result).toEqual(result); + }); + it('should send DETECT_EXTERNAL_REVIEW when overallStatus is in_progress', () => { manager.handleStartReview(projectId, prNumber); const result = createMockResult({ overallStatus: 'in_progress' }); @@ -239,6 +248,19 @@ describe('PRReviewStateManager', () => { expect(manager.getState(projectId, prNumber)).toBeNull(); }); + it('should emit exactly one cleared state IPC on handleClearReview (no double emission)', () => { + manager.handleStartReview(projectId, prNumber); + mockSafeSendToRenderer.mockClear(); + + manager.handleClearReview(projectId, prNumber); + + // Should emit exactly 1 cleared state, not 2 (no double emission from + // sending CLEAR_REVIEW to actor subscription + manual emitClearedState) + expect(mockSafeSendToRenderer).toHaveBeenCalledTimes(1); + const payload = mockSafeSendToRenderer.mock.calls[0][3] as Record; + expect(payload).toEqual(expect.objectContaining({ state: 'idle' })); + }); + it('should stop ALL actors and clear maps on handleAuthChange', () => { manager.handleStartReview(projectId, 1); manager.handleStartReview(projectId, 2); 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 0fc2e874..3c000f48 100644 --- a/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/github/pr-handlers.ts @@ -1672,6 +1672,24 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v // Clear all PR review actors when GitHub auth changes (account swap) ipcMain.on(IPC_CHANNELS.GITHUB_AUTH_CHANGED, () => { + // Cancel all running review subprocesses and CI wait controllers + for (const [reviewKey, entry] of runningReviews) { + if (entry === CI_WAIT_PLACEHOLDER) { + const abortController = ciWaitAbortControllers.get(reviewKey); + if (abortController) { + abortController.abort(); + ciWaitAbortControllers.delete(reviewKey); + } + } else { + try { + entry.kill("SIGTERM"); + } catch { + // Process may have already exited + } + } + } + runningReviews.clear(); + ciWaitAbortControllers.clear(); prReviewStateManager.handleAuthChange(); }); @@ -1889,10 +1907,12 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v // Check if already running — notify renderer so it can display ongoing logs if (runningReviews.has(reviewKey)) { debugLog("Review already running, notifying renderer", { reviewKey }); + const currentSnapshot = prReviewStateManager.getState(projectId, prNumber); + const currentProgress = currentSnapshot?.context?.progress?.progress ?? 50; sendProgress({ phase: "analyzing", prNumber, - progress: 50, + progress: currentProgress, message: "Review is already in progress. Reconnecting to ongoing review...", }); return; @@ -2912,10 +2932,12 @@ export function registerPRHandlers(getMainWindow: () => BrowserWindow | null): v // Check if already running — notify renderer so it can display ongoing logs if (runningReviews.has(reviewKey)) { debugLog("Follow-up review already running, notifying renderer", { reviewKey }); + const currentSnapshot = prReviewStateManager.getState(projectId, prNumber); + const currentProgress = currentSnapshot?.context?.progress?.progress ?? 50; sendProgress({ phase: "analyzing", prNumber, - progress: 50, + progress: currentProgress, message: "Follow-up review is already in progress. Reconnecting to ongoing review...", }); return; diff --git a/apps/frontend/src/main/pr-review-state-manager.ts b/apps/frontend/src/main/pr-review-state-manager.ts index ad79f1b9..78a9c688 100644 --- a/apps/frontend/src/main/pr-review-state-manager.ts +++ b/apps/frontend/src/main/pr-review-state-manager.ts @@ -53,8 +53,16 @@ export class PRReviewStateManager { } handleComplete(projectId: string, prNumber: number, result: PRReviewResult): void { - const actor = this.getActor(projectId, prNumber); - if (!actor) return; + // Use getOrCreateActor so late-arriving results (e.g. after auth change or + // app restart) still get processed instead of silently dropped. + const actor = this.getOrCreateActor(projectId, prNumber); + + // If the actor is in idle state (freshly created for a late-arriving result), + // transition to reviewing first so REVIEW_COMPLETE is accepted. + const snapshot = actor.getSnapshot(); + if (String(snapshot.value) === 'idle') { + actor.send({ type: 'START_REVIEW', prNumber, projectId } satisfies PRReviewEvent); + } // Detect external review (result arrives with 'in_progress' status from outside) if (result.overallStatus === 'in_progress') { @@ -80,9 +88,10 @@ export class PRReviewStateManager { const key = this.getKey(projectId, prNumber); const actor = this.actors.get(key); if (actor) { - // Capture snapshot before clearing so the emitted payload has real context + // Capture snapshot before stopping so the emitted payload has real context. + // Don't send CLEAR_REVIEW to the actor — that would trigger the subscription + // and cause a duplicate IPC emission alongside emitClearedState below. const snapshot = actor.getSnapshot(); - actor.send({ type: 'CLEAR_REVIEW' } satisfies PRReviewEvent); actor.stop(); this.actors.delete(key); this.emitClearedState(key, snapshot?.context ?? null);