From 58fec15cd23701ba2adebdec50d55a8aa506a119 Mon Sep 17 00:00:00 2001 From: AndyMik90 Date: Sat, 14 Feb 2026 15:20:23 +0100 Subject: [PATCH] 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,