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 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
56ca898d21
commit
750d330ef3
@@ -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<string, ChildProcess>();
|
||||
|
||||
/**
|
||||
* 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', {
|
||||
|
||||
@@ -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<void> {
|
||||
@@ -108,7 +110,7 @@ export function registerBulkHandlers(
|
||||
skipped++;
|
||||
} else {
|
||||
try {
|
||||
execFileSync('gh', args, {
|
||||
await execFileAsync('gh', args, {
|
||||
cwd: project.path,
|
||||
env: getAugmentedEnv(),
|
||||
});
|
||||
|
||||
@@ -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<void> {
|
||||
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);
|
||||
|
||||
@@ -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;
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
@@ -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<T>(
|
||||
projectPath: string,
|
||||
operation: () => Promise<T>,
|
||||
): Promise<T> {
|
||||
const filePath = getEnrichmentFilePath(projectPath);
|
||||
return withEnrichmentLock(filePath, operation);
|
||||
}
|
||||
|
||||
// ============================================
|
||||
// Read / Append Transitions
|
||||
// ============================================
|
||||
@@ -216,53 +226,55 @@ export async function bootstrapFromGitHub(
|
||||
projectPath: string,
|
||||
issues: GitHubIssue[],
|
||||
): Promise<EnrichmentFile> {
|
||||
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<EnrichmentFile> {
|
||||
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 };
|
||||
});
|
||||
}
|
||||
|
||||
@@ -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 });
|
||||
|
||||
@@ -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<void> {
|
||||
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<void> {
|
||||
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);
|
||||
}
|
||||
|
||||
@@ -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<string, ChildProcess>();
|
||||
|
||||
/**
|
||||
* Triage categories
|
||||
*/
|
||||
@@ -259,7 +264,7 @@ async function runTriage(
|
||||
|
||||
const subprocessEnv = await getRunnerEnv();
|
||||
|
||||
const { promise } = runPythonSubprocess<TriageResult[]>({
|
||||
const { process: triageProcess, promise } = runPythonSubprocess<TriageResult[]>({
|
||||
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<TriageProgress, TriageResult[]>(
|
||||
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<TriageProgress, TriageResult[]>(
|
||||
|
||||
Reference in New Issue
Block a user