fix: prevent OOM, orphaned agents, and unbounded growth during overnight builds
- 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 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
947fc3c6f8
commit
58fec15cd2
@@ -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
|
||||
|
||||
@@ -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' },
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user