From 750d330ef3fdb809f3d22147c8a2bd109e83df2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sondre=20Engebr=C3=A5ten?= Date: Fri, 13 Feb 2026 13:58:49 +0100 Subject: [PATCH] fix(issues): resolve race conditions and main thread blocking - IPC-7: Wrap all enrichment read-modify-write cycles in withEnrichmentFileLock across 5 handler files (10 call sites); remove inner lock from writeEnrichmentFile to prevent deadlock - IPC-4: Replace single activeTriageProcess variable with Map keyed by projectId:operation for concurrent enrich/split tracking - IPC-10: Add concurrency guard in triage-handlers preventing duplicate Python subprocess runs per project - IPC-11/12: Replace execFileSync with async execFile in bulk-handlers and label-sync-handlers to unblock main thread Phase 3 of alpha stability audit. Co-Authored-By: Claude Opus 4.6 --- .../ipc-handlers/github/ai-triage-handlers.ts | 139 +++++----- .../main/ipc-handlers/github/bulk-handlers.ts | 6 +- .../github/create-spec-handler.ts | 37 +-- .../github/enrichment-handlers.ts | 109 ++++---- .../github/enrichment-persistence.ts | 240 ++++++++++-------- .../github/label-sync-handlers.ts | 18 +- .../ipc-handlers/github/mutation-handlers.ts | 93 +++---- .../ipc-handlers/github/triage-handlers.ts | 39 ++- 8 files changed, 379 insertions(+), 302 deletions(-) diff --git a/apps/frontend/src/main/ipc-handlers/github/ai-triage-handlers.ts b/apps/frontend/src/main/ipc-handlers/github/ai-triage-handlers.ts index 48ddb651..ed3cf92e 100644 --- a/apps/frontend/src/main/ipc-handlers/github/ai-triage-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/github/ai-triage-handlers.ts @@ -32,7 +32,7 @@ import { } from './utils/subprocess-runner'; import { MAX_SPLIT_SUB_ISSUES } from '../../../shared/constants/ai-triage'; import { createDefaultProgressiveTrust } from '../../../shared/types/ai-triage'; -import { readEnrichmentFile, writeEnrichmentFile, appendTransition } from './enrichment-persistence'; +import { readEnrichmentFile, writeEnrichmentFile, withEnrichmentFileLock, appendTransition } from './enrichment-persistence'; import { createDefaultEnrichment } from '../../../shared/types/enrichment'; import type { TriageCategory } from '../../../shared/types/enrichment'; import type { @@ -67,9 +67,9 @@ function getGitHubDir(projectPath: string): string { return path.join(projectPath, '.auto-claude', 'github'); } -// Track active subprocess for cancellation +// Track active subprocesses for cancellation, keyed by operation type (e.g. 'enrich', 'split') import type { ChildProcess } from 'child_process'; -let activeTriageProcess: ChildProcess | null = null; +const activeTriageProcesses = new Map(); /** * Register AI triage handlers @@ -79,16 +79,19 @@ export function registerAITriageHandlers( ): void { debugLog('Registering AI Triage handlers'); - // Cancel active triage subprocess + // Cancel active triage subprocesses (kills all tracked operations) ipcMain.handle( IPC_CHANNELS.GITHUB_TRIAGE_CANCEL, async () => { - if (activeTriageProcess && !activeTriageProcess.killed) { - activeTriageProcess.kill('SIGTERM'); - activeTriageProcess = null; - return { cancelled: true }; + let cancelled = false; + for (const [key, proc] of activeTriageProcesses) { + if (!proc.killed) { + proc.kill('SIGTERM'); + cancelled = true; + } + activeTriageProcesses.delete(key); } - return { cancelled: false }; + return { cancelled }; }, ); @@ -155,10 +158,15 @@ export function registerAITriageHandlers( mainWindow.webContents.send(IPC_CHANNELS.CLAUDE_AUTH_FAILURE, authFailureInfo); }, }); - activeTriageProcess = childProcess; + const processKey = `${projectId}:enrich`; + activeTriageProcesses.set(processKey, childProcess); - const result = await promise; - activeTriageProcess = null; + let result; + try { + result = await promise; + } finally { + activeTriageProcesses.delete(processKey); + } if (!result.success) { sendError(result.error ?? 'Enrichment failed'); @@ -170,24 +178,26 @@ export function registerAITriageHandlers( // Persist enrichment data to local file try { - const enrichmentFile = await readEnrichmentFile(project.path); - const key = String(issueNumber); - const existing = enrichmentFile.issues[key] ?? createDefaultEnrichment(issueNumber); - enrichmentFile.issues[key] = { - ...existing, - enrichment: { - problem: enrichmentResult.problem, - goal: enrichmentResult.goal, - scopeIn: enrichmentResult.scopeIn, - scopeOut: enrichmentResult.scopeOut, - acceptanceCriteria: enrichmentResult.acceptanceCriteria, - technicalContext: enrichmentResult.technicalContext, - risksEdgeCases: enrichmentResult.risksEdgeCases, - }, - completenessScore: enrichmentResult.confidence, - updatedAt: new Date().toISOString(), - }; - await writeEnrichmentFile(project.path, enrichmentFile); + await withEnrichmentFileLock(project.path, async () => { + const enrichmentFile = await readEnrichmentFile(project.path); + const key = String(issueNumber); + const existing = enrichmentFile.issues[key] ?? createDefaultEnrichment(issueNumber); + enrichmentFile.issues[key] = { + ...existing, + enrichment: { + problem: enrichmentResult.problem, + goal: enrichmentResult.goal, + scopeIn: enrichmentResult.scopeIn, + scopeOut: enrichmentResult.scopeOut, + acceptanceCriteria: enrichmentResult.acceptanceCriteria, + technicalContext: enrichmentResult.technicalContext, + risksEdgeCases: enrichmentResult.risksEdgeCases, + }, + completenessScore: enrichmentResult.confidence, + updatedAt: new Date().toISOString(), + }; + await writeEnrichmentFile(project.path, enrichmentFile); + }); } catch (persistErr) { debugLog('Failed to persist enrichment result', { issueNumber, @@ -263,10 +273,15 @@ export function registerAITriageHandlers( mainWindow.webContents.send(IPC_CHANNELS.CLAUDE_AUTH_FAILURE, authFailureInfo); }, }); - activeTriageProcess = splitProcess; + const processKey = `${projectId}:split`; + activeTriageProcesses.set(processKey, splitProcess); - const result = await promise; - activeTriageProcess = null; + let result; + try { + result = await promise; + } finally { + activeTriageProcesses.delete(processKey); + } if (!result.success) { sendError(result.error ?? 'Split analysis failed'); @@ -367,35 +382,37 @@ export function registerAITriageHandlers( // Persist triage result to enrichment file try { - const enrichmentFile = await readEnrichmentFile(project.path); - const key = String(item.issueNumber); - const existing = enrichmentFile.issues[key] ?? createDefaultEnrichment(item.issueNumber); - enrichmentFile.issues[key] = { - ...existing, - triageResult: { - category: item.result.category as TriageCategory, - confidence: item.result.confidence, - labelsToAdd: item.result.labelsToAdd, - labelsToRemove: item.result.labelsToRemove, - isDuplicate: item.result.isDuplicate, - duplicateOf: item.result.duplicateOf, - isSpam: item.result.isSpam, - suggestedBreakdown: item.result.suggestedBreakdown, - comment: item.result.comment, - triagedAt: item.result.triagedAt, - }, - updatedAt: new Date().toISOString(), - }; - await writeEnrichmentFile(project.path, enrichmentFile); + await withEnrichmentFileLock(project.path, async () => { + const enrichmentFile = await readEnrichmentFile(project.path); + const key = String(item.issueNumber); + const existing = enrichmentFile.issues[key] ?? createDefaultEnrichment(item.issueNumber); + enrichmentFile.issues[key] = { + ...existing, + triageResult: { + category: item.result.category as TriageCategory, + confidence: item.result.confidence, + labelsToAdd: item.result.labelsToAdd, + labelsToRemove: item.result.labelsToRemove, + isDuplicate: item.result.isDuplicate, + duplicateOf: item.result.duplicateOf, + isSpam: item.result.isSpam, + suggestedBreakdown: item.result.suggestedBreakdown, + comment: item.result.comment, + triagedAt: item.result.triagedAt, + }, + updatedAt: new Date().toISOString(), + }; + await writeEnrichmentFile(project.path, enrichmentFile); - // Append audit trail transition - await appendTransition(project.path, { - issueNumber: item.issueNumber, - from: existing.triageState, - to: 'triage', - actor: 'ai-triage', - reason: `AI triage applied: ${item.result.category} (confidence: ${item.result.confidence})`, - timestamp: new Date().toISOString(), + // Append audit trail transition + await appendTransition(project.path, { + issueNumber: item.issueNumber, + from: existing.triageState, + to: 'triage', + actor: 'ai-triage', + reason: `AI triage applied: ${item.result.category} (confidence: ${item.result.confidence})`, + timestamp: new Date().toISOString(), + }); }); } catch (persistErr) { debugLog('Failed to persist triage result', { diff --git a/apps/frontend/src/main/ipc-handlers/github/bulk-handlers.ts b/apps/frontend/src/main/ipc-handlers/github/bulk-handlers.ts index 6adc998f..637cc34f 100644 --- a/apps/frontend/src/main/ipc-handlers/github/bulk-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/github/bulk-handlers.ts @@ -5,7 +5,8 @@ import { ipcMain } from 'electron'; import type { BrowserWindow } from 'electron'; -import { execFileSync } from 'child_process'; +import { execFile } from 'child_process'; +import { promisify } from 'util'; import { IPC_CHANNELS } from '../../../shared/constants/ipc'; import { BULK_INTER_ITEM_DELAY } from '../../../shared/constants/mutations'; import type { @@ -18,6 +19,7 @@ import { withProject } from './utils/project-middleware'; import { getAugmentedEnv } from '../../env-utils'; import { createContextLogger } from './utils/logger'; +const execFileAsync = promisify(execFile); const logger = createContextLogger('GitHub Bulk Operations'); function sleep(ms: number): Promise { @@ -108,7 +110,7 @@ export function registerBulkHandlers( skipped++; } else { try { - execFileSync('gh', args, { + await execFileAsync('gh', args, { cwd: project.path, env: getAugmentedEnv(), }); diff --git a/apps/frontend/src/main/ipc-handlers/github/create-spec-handler.ts b/apps/frontend/src/main/ipc-handlers/github/create-spec-handler.ts index 9cdc2fdc..4889842e 100644 --- a/apps/frontend/src/main/ipc-handlers/github/create-spec-handler.ts +++ b/apps/frontend/src/main/ipc-handlers/github/create-spec-handler.ts @@ -13,6 +13,7 @@ import { isValidTransition } from '../../../shared/constants/enrichment'; import { readEnrichmentFile, writeEnrichmentFile, + withEnrichmentFileLock, appendTransition, } from './enrichment-persistence'; import { createSpecForIssue, buildInvestigationTask, buildIssueContext } from './spec-utils'; @@ -134,27 +135,29 @@ async function transitionToInProgress( issueNumber: number, ): Promise { try { - const data = await readEnrichmentFile(projectPath); - const key = String(issueNumber); - const enrichment = data.issues[key]; + await withEnrichmentFileLock(projectPath, async () => { + const data = await readEnrichmentFile(projectPath); + const key = String(issueNumber); + const enrichment = data.issues[key]; - if (!enrichment) return; + if (!enrichment) return; - const from = enrichment.triageState; - if (from === 'in_progress') return; // Already there - if (!isValidTransition(from, 'in_progress')) return; + const from = enrichment.triageState; + if (from === 'in_progress') return; // Already there + if (!isValidTransition(from, 'in_progress')) return; - enrichment.triageState = 'in_progress'; - enrichment.updatedAt = new Date().toISOString(); - data.issues[key] = enrichment; + enrichment.triageState = 'in_progress'; + enrichment.updatedAt = new Date().toISOString(); + data.issues[key] = enrichment; - await writeEnrichmentFile(projectPath, data); - await appendTransition(projectPath, { - issueNumber, - from, - to: 'in_progress', - actor: 'user', - timestamp: enrichment.updatedAt, + await writeEnrichmentFile(projectPath, data); + await appendTransition(projectPath, { + issueNumber, + from, + to: 'in_progress', + actor: 'user', + timestamp: enrichment.updatedAt, + }); }); } catch (error) { logger.debug(`Failed to transition enrichment to in_progress for #${issueNumber}`, error); diff --git a/apps/frontend/src/main/ipc-handlers/github/enrichment-handlers.ts b/apps/frontend/src/main/ipc-handlers/github/enrichment-handlers.ts index 2b89791e..21b70cc6 100644 --- a/apps/frontend/src/main/ipc-handlers/github/enrichment-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/github/enrichment-handlers.ts @@ -12,6 +12,7 @@ import type { GitHubIssue } from '../../../shared/types/integrations'; import { readEnrichmentFile, writeEnrichmentFile, + withEnrichmentFileLock, appendTransition, bootstrapFromGitHub, reconcileWithGitHub, @@ -52,13 +53,15 @@ export function registerEnrichmentHandlers( IPC_CHANNELS.GITHUB_ENRICHMENT_SAVE, async (_, projectId: string, enrichment: IssueEnrichment) => { return withProject(projectId, async (project) => { - const data = await readEnrichmentFile(project.path); - data.issues[String(enrichment.issueNumber)] = { - ...enrichment, - updatedAt: new Date().toISOString(), - }; - await writeEnrichmentFile(project.path, data); - return true; + return withEnrichmentFileLock(project.path, async () => { + const data = await readEnrichmentFile(project.path); + data.issues[String(enrichment.issueNumber)] = { + ...enrichment, + updatedAt: new Date().toISOString(), + }; + await writeEnrichmentFile(project.path, data); + return true; + }); }); }, ); @@ -74,61 +77,63 @@ export function registerEnrichmentHandlers( resolution?: Resolution, ) => { return withProject(projectId, async (project) => { - const data = await readEnrichmentFile(project.path); - const key = String(issueNumber); - const enrichment = data.issues[key]; + return withEnrichmentFileLock(project.path, async () => { + const data = await readEnrichmentFile(project.path); + const key = String(issueNumber); + const enrichment = data.issues[key]; - if (!enrichment) { - throw new Error(`No enrichment found for issue #${issueNumber}`); - } - - const from = enrichment.triageState; - - // Validate transition (blocked state unblock handled specially) - if (from === 'blocked' && enrichment.previousState) { - // Unblock: return to previousState - enrichment.triageState = enrichment.previousState; - enrichment.previousState = undefined; - } else if (to === 'blocked') { - // Block: save current state as previousState - if (!isValidTransition(from, to)) { - throw new Error(`Invalid transition: ${from} → ${to}`); - } - enrichment.previousState = from; - enrichment.triageState = 'blocked'; - } else { - if (!isValidTransition(from, to)) { - throw new Error(`Invalid transition: ${from} → ${to}`); + if (!enrichment) { + throw new Error(`No enrichment found for issue #${issueNumber}`); } - // Require resolution when transitioning to done - if (to === 'done' && !resolution) { - throw new Error('Resolution is required when transitioning to done'); - } + const from = enrichment.triageState; - enrichment.triageState = to; - if (to === 'done') { - enrichment.resolution = resolution; + // Validate transition (blocked state unblock handled specially) + if (from === 'blocked' && enrichment.previousState) { + // Unblock: return to previousState + enrichment.triageState = enrichment.previousState; + enrichment.previousState = undefined; + } else if (to === 'blocked') { + // Block: save current state as previousState + if (!isValidTransition(from, to)) { + throw new Error(`Invalid transition: ${from} → ${to}`); + } + enrichment.previousState = from; + enrichment.triageState = 'blocked'; } else { - enrichment.resolution = undefined; + if (!isValidTransition(from, to)) { + throw new Error(`Invalid transition: ${from} → ${to}`); + } + + // Require resolution when transitioning to done + if (to === 'done' && !resolution) { + throw new Error('Resolution is required when transitioning to done'); + } + + enrichment.triageState = to; + if (to === 'done') { + enrichment.resolution = resolution; + } else { + enrichment.resolution = undefined; + } } - } - enrichment.updatedAt = new Date().toISOString(); - data.issues[key] = enrichment; + enrichment.updatedAt = new Date().toISOString(); + data.issues[key] = enrichment; - await writeEnrichmentFile(project.path, data); + await writeEnrichmentFile(project.path, data); - await appendTransition(project.path, { - issueNumber, - from, - to: enrichment.triageState, - actor: 'user', - resolution: enrichment.resolution, - timestamp: enrichment.updatedAt, + await appendTransition(project.path, { + issueNumber, + from, + to: enrichment.triageState, + actor: 'user', + resolution: enrichment.resolution, + timestamp: enrichment.updatedAt, + }); + + return enrichment; }); - - return enrichment; }); }, ); diff --git a/apps/frontend/src/main/ipc-handlers/github/enrichment-persistence.ts b/apps/frontend/src/main/ipc-handlers/github/enrichment-persistence.ts index 706955db..b0da4743 100644 --- a/apps/frontend/src/main/ipc-handlers/github/enrichment-persistence.ts +++ b/apps/frontend/src/main/ipc-handlers/github/enrichment-persistence.ts @@ -99,14 +99,24 @@ export async function writeEnrichmentFile( await mkdir(dir, { recursive: true }); - await withEnrichmentLock(filePath, async () => { - await writeJsonWithRetry(filePath, data, { - indent: 2, - maxRetries: isWindows() ? 5 : 3, - }); + await writeJsonWithRetry(filePath, data, { + indent: 2, + maxRetries: isWindows() ? 5 : 3, }); } +/** + * Wrap an entire read-modify-write cycle on the enrichment file in a single lock. + * Callers MUST use this instead of separate read + write calls to prevent lost updates. + */ +export async function withEnrichmentFileLock( + projectPath: string, + operation: () => Promise, +): Promise { + const filePath = getEnrichmentFilePath(projectPath); + return withEnrichmentLock(filePath, operation); +} + // ============================================ // Read / Append Transitions // ============================================ @@ -216,53 +226,55 @@ export async function bootstrapFromGitHub( projectPath: string, issues: GitHubIssue[], ): Promise { - const enrichmentFile = await readEnrichmentFile(projectPath); - const now = new Date().toISOString(); + return withEnrichmentFileLock(projectPath, async () => { + const enrichmentFile = await readEnrichmentFile(projectPath); + const now = new Date().toISOString(); - for (const issue of issues) { - const key = String(issue.number); + for (const issue of issues) { + const key = String(issue.number); - // Skip issues that already have enrichment - if (enrichmentFile.issues[key]) continue; + // Skip issues that already have enrichment + if (enrichmentFile.issues[key]) continue; - const enrichment = createDefaultEnrichment(issue.number); + const enrichment = createDefaultEnrichment(issue.number); - // Infer state from GitHub issue data - if (issue.state === 'closed') { - enrichment.triageState = 'done'; - enrichment.resolution = 'completed'; - } else if (issue.assignees.length > 0) { - enrichment.triageState = 'in_progress'; - } - - // Extract priority from labels - for (const label of issue.labels) { - const name = label.name.toLowerCase(); - if (name === 'priority:critical' || name === 'critical') { - enrichment.priority = 'critical'; - } else if (name === 'priority:high' || name === 'high') { - enrichment.priority = 'high'; - } else if (name === 'priority:medium' || name === 'medium') { - enrichment.priority = 'medium'; - } else if (name === 'priority:low' || name === 'low') { - enrichment.priority = 'low'; + // Infer state from GitHub issue data + if (issue.state === 'closed') { + enrichment.triageState = 'done'; + enrichment.resolution = 'completed'; + } else if (issue.assignees.length > 0) { + enrichment.triageState = 'in_progress'; } + + // Extract priority from labels + for (const label of issue.labels) { + const name = label.name.toLowerCase(); + if (name === 'priority:critical' || name === 'critical') { + enrichment.priority = 'critical'; + } else if (name === 'priority:high' || name === 'high') { + enrichment.priority = 'high'; + } else if (name === 'priority:medium' || name === 'medium') { + enrichment.priority = 'medium'; + } else if (name === 'priority:low' || name === 'low') { + enrichment.priority = 'low'; + } + } + + enrichmentFile.issues[key] = enrichment; + + // Log bootstrap transition + await appendTransition(projectPath, { + issueNumber: issue.number, + from: 'new', + to: enrichment.triageState, + actor: 'bootstrap', + timestamp: now, + }); } - enrichmentFile.issues[key] = enrichment; - - // Log bootstrap transition - await appendTransition(projectPath, { - issueNumber: issue.number, - from: 'new', - to: enrichment.triageState, - actor: 'bootstrap', - timestamp: now, - }); - } - - await writeEnrichmentFile(projectPath, enrichmentFile); - return enrichmentFile; + await writeEnrichmentFile(projectPath, enrichmentFile); + return enrichmentFile; + }); } // ============================================ @@ -277,51 +289,53 @@ export async function reconcileWithGitHub( projectPath: string, issues: GitHubIssue[], ): Promise { - const enrichmentFile = await readEnrichmentFile(projectPath); - const now = new Date().toISOString(); + return withEnrichmentFileLock(projectPath, async () => { + const enrichmentFile = await readEnrichmentFile(projectPath); + const now = new Date().toISOString(); - for (const issue of issues) { - const key = String(issue.number); - const enrichment = enrichmentFile.issues[key]; - if (!enrichment) continue; + for (const issue of issues) { + const key = String(issue.number); + const enrichment = enrichmentFile.issues[key]; + if (!enrichment) continue; - // Closed on GitHub but not done in enrichment → mark done - if (issue.state === 'closed' && enrichment.triageState !== 'done') { - const from = enrichment.triageState; - enrichment.triageState = 'done'; - enrichment.resolution = enrichment.resolution ?? 'completed'; - enrichment.updatedAt = now; + // Closed on GitHub but not done in enrichment → mark done + if (issue.state === 'closed' && enrichment.triageState !== 'done') { + const from = enrichment.triageState; + enrichment.triageState = 'done'; + enrichment.resolution = enrichment.resolution ?? 'completed'; + enrichment.updatedAt = now; - await appendTransition(projectPath, { - issueNumber: issue.number, - from, - to: 'done', - actor: 'auto-reconcile', - reason: 'GitHub state diverged', - resolution: enrichment.resolution, - timestamp: now, - }); + await appendTransition(projectPath, { + issueNumber: issue.number, + from, + to: 'done', + actor: 'auto-reconcile', + reason: 'GitHub state diverged', + resolution: enrichment.resolution, + timestamp: now, + }); + } + + // Open on GitHub but done in enrichment → reopen to ready (GAP-2) + if (issue.state === 'open' && enrichment.triageState === 'done') { + enrichment.triageState = 'ready'; + enrichment.resolution = undefined; + enrichment.updatedAt = now; + + await appendTransition(projectPath, { + issueNumber: issue.number, + from: 'done', + to: 'ready', + actor: 'auto-reconcile', + reason: 'GitHub state diverged', + timestamp: now, + }); + } } - // Open on GitHub but done in enrichment → reopen to ready (GAP-2) - if (issue.state === 'open' && enrichment.triageState === 'done') { - enrichment.triageState = 'ready'; - enrichment.resolution = undefined; - enrichment.updatedAt = now; - - await appendTransition(projectPath, { - issueNumber: issue.number, - from: 'done', - to: 'ready', - actor: 'auto-reconcile', - reason: 'GitHub state diverged', - timestamp: now, - }); - } - } - - await writeEnrichmentFile(projectPath, enrichmentFile); - return enrichmentFile; + await writeEnrichmentFile(projectPath, enrichmentFile); + return enrichmentFile; + }); } // ============================================ @@ -337,37 +351,39 @@ export async function runGarbageCollection( return { pruned: 0, orphaned: 0 }; } - const enrichmentFile = await readEnrichmentFile(projectPath); - const currentSet = new Set(currentIssueNumbers.map(String)); - const now = new Date(); - let pruned = 0; - let orphaned = 0; + return withEnrichmentFileLock(projectPath, async () => { + const enrichmentFile = await readEnrichmentFile(projectPath); + const currentSet = new Set(currentIssueNumbers.map(String)); + const now = new Date(); + let pruned = 0; + let orphaned = 0; - for (const [key, enrichment] of Object.entries(enrichmentFile.issues)) { - if (!currentSet.has(key)) { - // Mark as orphaned if not already - if (!(enrichment as IssueEnrichment & { _orphanedAt?: string })._orphanedAt) { - (enrichment as IssueEnrichment & { _orphanedAt?: string })._orphanedAt = now.toISOString(); - orphaned++; - } else { - // Check if orphan is old enough to prune - const orphanedAt = new Date( - (enrichment as IssueEnrichment & { _orphanedAt?: string })._orphanedAt!, - ); - const daysSinceOrphan = (now.getTime() - orphanedAt.getTime()) / (1000 * 60 * 60 * 24); - if (daysSinceOrphan > 30) { - delete enrichmentFile.issues[key]; - pruned++; - } else { + for (const [key, enrichment] of Object.entries(enrichmentFile.issues)) { + if (!currentSet.has(key)) { + // Mark as orphaned if not already + if (!(enrichment as IssueEnrichment & { _orphanedAt?: string })._orphanedAt) { + (enrichment as IssueEnrichment & { _orphanedAt?: string })._orphanedAt = now.toISOString(); orphaned++; + } else { + // Check if orphan is old enough to prune + const orphanedAt = new Date( + (enrichment as IssueEnrichment & { _orphanedAt?: string })._orphanedAt!, + ); + const daysSinceOrphan = (now.getTime() - orphanedAt.getTime()) / (1000 * 60 * 60 * 24); + if (daysSinceOrphan > 30) { + delete enrichmentFile.issues[key]; + pruned++; + } else { + orphaned++; + } } + } else { + // Not orphaned — clear orphan marker if present + delete (enrichment as IssueEnrichment & { _orphanedAt?: string })._orphanedAt; } - } else { - // Not orphaned — clear orphan marker if present - delete (enrichment as IssueEnrichment & { _orphanedAt?: string })._orphanedAt; } - } - await writeEnrichmentFile(projectPath, enrichmentFile); - return { pruned, orphaned }; + await writeEnrichmentFile(projectPath, enrichmentFile); + return { pruned, orphaned }; + }); } diff --git a/apps/frontend/src/main/ipc-handlers/github/label-sync-handlers.ts b/apps/frontend/src/main/ipc-handlers/github/label-sync-handlers.ts index 0796154a..109f5606 100644 --- a/apps/frontend/src/main/ipc-handlers/github/label-sync-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/github/label-sync-handlers.ts @@ -6,7 +6,8 @@ import { ipcMain } from 'electron'; import type { BrowserWindow } from 'electron'; -import { execFileSync } from 'child_process'; +import { execFile } from 'child_process'; +import { promisify } from 'util'; import path from 'node:path'; import fs from 'node:fs'; import { withProject } from './utils/project-middleware'; @@ -22,6 +23,7 @@ import { import type { WorkflowState } from '../../../shared/types/enrichment'; import type { LabelSyncConfig, LabelSyncResult } from '../../../shared/types/label-sync'; +const execFileAsync = promisify(execFile); const logger = createContextLogger('Label Sync'); function getConfigPath(projectPath: string): string { @@ -63,7 +65,7 @@ export function registerLabelSyncHandlers( for (const label of labels) { try { - execFileSync('gh', [ + await execFileAsync('gh', [ 'label', 'create', label.name, '--color', label.color, '--description', label.description, @@ -104,7 +106,7 @@ export function registerLabelSyncHandlers( for (const [issueNumber, enrichment] of Object.entries(data.issues)) { const label = getLabelForState(enrichment.triageState as WorkflowState); try { - execFileSync('gh', [ + await execFileAsync('gh', [ 'issue', 'edit', issueNumber, '--remove-label', label, ], { env, cwd: project.path, encoding: 'utf-8' }); @@ -117,7 +119,7 @@ export function registerLabelSyncHandlers( const labels = getWorkflowLabels(); for (const label of labels) { try { - execFileSync('gh', [ + await execFileAsync('gh', [ 'label', 'delete', label.name, '--yes', ], { env, cwd: project.path, encoding: 'utf-8' }); } catch { @@ -142,7 +144,7 @@ export function registerLabelSyncHandlers( // Check current labels to avoid unnecessary API calls (GAP-1 fix) try { - const labelsJson = execFileSync('gh', [ + const { stdout: labelsJson } = await execFileAsync('gh', [ 'issue', 'view', String(issueNumber), '--json', 'labels', '--jq', '.labels', @@ -166,7 +168,7 @@ export function registerLabelSyncHandlers( args.push('--add-label', targetLabel); - execFileSync('gh', args, { env, cwd: project.path, encoding: 'utf-8' }); + await execFileAsync('gh', args, { env, cwd: project.path, encoding: 'utf-8' }); return { synced: true }; } catch (error) { return { error: error instanceof Error ? error.message : 'Sync failed' }; @@ -208,7 +210,7 @@ export function registerLabelSyncHandlers( try { // Get current labels - const labelsJson = execFileSync('gh', [ + const { stdout: labelsJson } = await execFileAsync('gh', [ 'issue', 'view', String(issueNumber), '--json', 'labels', '--jq', '.labels', @@ -228,7 +230,7 @@ export function registerLabelSyncHandlers( } args.push('--add-label', targetLabel); - execFileSync('gh', args, { env, cwd: project.path, encoding: 'utf-8' }); + await execFileAsync('gh', args, { env, cwd: project.path, encoding: 'utf-8' }); synced++; } catch (error) { logger.debug('Bulk sync error for issue', { issueNumber, error }); diff --git a/apps/frontend/src/main/ipc-handlers/github/mutation-handlers.ts b/apps/frontend/src/main/ipc-handlers/github/mutation-handlers.ts index 4dcc91bc..b2311de8 100644 --- a/apps/frontend/src/main/ipc-handlers/github/mutation-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/github/mutation-handlers.ts @@ -27,6 +27,7 @@ import { isValidTransition } from '../../../shared/constants/enrichment'; import { readEnrichmentFile, writeEnrichmentFile, + withEnrichmentFileLock, appendTransition, } from './enrichment-persistence'; import { withProject } from './utils/project-middleware'; @@ -65,35 +66,37 @@ async function transitionEnrichmentOnClose( issueNumber: number, ): Promise { try { - const data = await readEnrichmentFile(projectPath); - const key = String(issueNumber); - const enrichment = data.issues[key]; + await withEnrichmentFileLock(projectPath, async () => { + const data = await readEnrichmentFile(projectPath); + const key = String(issueNumber); + const enrichment = data.issues[key]; - if (!enrichment) return; + if (!enrichment) return; - const from = enrichment.triageState; + const from = enrichment.triageState; - // Only transition if closing is valid from current state - if (from === 'done') return; // Already done - if (!isValidTransition(from, 'done') && from !== 'blocked') return; + // Only transition if closing is valid from current state + if (from === 'done') return; // Already done + if (!isValidTransition(from, 'done') && from !== 'blocked') return; - enrichment.previousState = undefined; - enrichment.triageState = 'done'; - enrichment.resolution = 'completed'; - enrichment.updatedAt = new Date().toISOString(); - data.issues[key] = enrichment; + enrichment.previousState = undefined; + enrichment.triageState = 'done'; + enrichment.resolution = 'completed'; + enrichment.updatedAt = new Date().toISOString(); + data.issues[key] = enrichment; - await writeEnrichmentFile(projectPath, data); - await appendTransition(projectPath, { - issueNumber, - from, - to: 'done', - actor: 'user', - resolution: 'completed', - timestamp: enrichment.updatedAt, + await writeEnrichmentFile(projectPath, data); + await appendTransition(projectPath, { + issueNumber, + from, + to: 'done', + actor: 'user', + resolution: 'completed', + timestamp: enrichment.updatedAt, + }); + + logger.debug(`Auto-transitioned issue #${issueNumber} from ${from} to done`); }); - - logger.debug(`Auto-transitioned issue #${issueNumber} from ${from} to done`); } catch (error) { logger.debug(`Failed to auto-transition enrichment on close for #${issueNumber}`, error); } @@ -108,33 +111,35 @@ async function transitionEnrichmentOnReopen( issueNumber: number, ): Promise { try { - const data = await readEnrichmentFile(projectPath); - const key = String(issueNumber); - const enrichment = data.issues[key]; + await withEnrichmentFileLock(projectPath, async () => { + const data = await readEnrichmentFile(projectPath); + const key = String(issueNumber); + const enrichment = data.issues[key]; - if (!enrichment) return; + if (!enrichment) return; - const from = enrichment.triageState; + const from = enrichment.triageState; - // Only transition if currently done - if (from !== 'done') return; - if (!isValidTransition('done', 'ready')) return; + // Only transition if currently done + if (from !== 'done') return; + if (!isValidTransition('done', 'ready')) return; - enrichment.triageState = 'ready'; - enrichment.resolution = undefined; - enrichment.updatedAt = new Date().toISOString(); - data.issues[key] = enrichment; + enrichment.triageState = 'ready'; + enrichment.resolution = undefined; + enrichment.updatedAt = new Date().toISOString(); + data.issues[key] = enrichment; - await writeEnrichmentFile(projectPath, data); - await appendTransition(projectPath, { - issueNumber, - from: 'done', - to: 'ready', - actor: 'user', - timestamp: enrichment.updatedAt, + await writeEnrichmentFile(projectPath, data); + await appendTransition(projectPath, { + issueNumber, + from: 'done', + to: 'ready', + actor: 'user', + timestamp: enrichment.updatedAt, + }); + + logger.debug(`Auto-transitioned issue #${issueNumber} from done to ready`); }); - - logger.debug(`Auto-transitioned issue #${issueNumber} from done to ready`); } catch (error) { logger.debug(`Failed to auto-transition enrichment on reopen for #${issueNumber}`, error); } diff --git a/apps/frontend/src/main/ipc-handlers/github/triage-handlers.ts b/apps/frontend/src/main/ipc-handlers/github/triage-handlers.ts index f0865d03..89aaaccf 100644 --- a/apps/frontend/src/main/ipc-handlers/github/triage-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/github/triage-handlers.ts @@ -30,9 +30,14 @@ import { buildRunnerArgs, } from './utils/subprocess-runner'; +import type { ChildProcess } from 'child_process'; + // Debug logging const { debug: debugLog } = createContextLogger('GitHub Triage'); +// Track active triage runs per project to prevent concurrent subprocess spawns +const activeTriageRuns = new Map(); + /** * Triage categories */ @@ -259,7 +264,7 @@ async function runTriage( const subprocessEnv = await getRunnerEnv(); - const { promise } = runPythonSubprocess({ + const { process: triageProcess, promise } = runPythonSubprocess({ pythonPath: getPythonPath(backendPath), args, cwd: backendPath, @@ -288,13 +293,19 @@ async function runTriage( }, }); - const result = await promise; + activeTriageRuns.set(project.id, triageProcess); - if (!result.success) { - throw new Error(result.error ?? 'Triage failed'); + try { + const result = await promise; + + if (!result.success) { + throw new Error(result.error ?? 'Triage failed'); + } + + return result.data!; + } finally { + activeTriageRuns.delete(project.id); } - - return result.data!; } /** @@ -357,6 +368,22 @@ export function registerTriageHandlers( return; } + // Concurrency guard: reject if triage is already running for this project + if (activeTriageRuns.has(projectId)) { + debugLog('Triage already running for project, rejecting', { projectId }); + const { sendError } = createIPCCommunicators( + mainWindow, + { + progress: IPC_CHANNELS.GITHUB_TRIAGE_PROGRESS, + error: IPC_CHANNELS.GITHUB_TRIAGE_ERROR, + complete: IPC_CHANNELS.GITHUB_TRIAGE_COMPLETE, + }, + projectId + ); + sendError('Triage is already running for this project'); + return; + } + try { await withProjectOrNull(projectId, async (project) => { const { sendProgress, sendError: _sendError, sendComplete } = createIPCCommunicators(