diff --git a/apps/backend/core/workspace/models.py b/apps/backend/core/workspace/models.py index 164e3418..039bf786 100644 --- a/apps/backend/core/workspace/models.py +++ b/apps/backend/core/workspace/models.py @@ -134,3 +134,141 @@ class MergeLock: self.lock_file.unlink() except Exception: pass # Best effort cleanup + + +class SpecNumberLockError(Exception): + """Raised when a spec number lock cannot be acquired.""" + + pass + + +class SpecNumberLock: + """ + Context manager for spec number coordination across main project and worktrees. + + Prevents race conditions when creating specs by: + 1. Acquiring an exclusive file lock + 2. Scanning ALL spec locations (main + worktrees) + 3. Finding global maximum spec number + 4. Allowing atomic spec directory creation + 5. Releasing lock + """ + + def __init__(self, project_dir: Path): + self.project_dir = project_dir + self.lock_dir = project_dir / ".auto-claude" / ".locks" + self.lock_file = self.lock_dir / "spec-numbering.lock" + self.acquired = False + self._global_max: int | None = None + + def __enter__(self) -> "SpecNumberLock": + """Acquire the spec numbering lock.""" + import os + import time + + self.lock_dir.mkdir(parents=True, exist_ok=True) + + max_wait = 30 # seconds + start_time = time.time() + + while True: + try: + # Try to create lock file exclusively (atomic operation) + fd = os.open( + str(self.lock_file), + os.O_CREAT | os.O_EXCL | os.O_WRONLY, + 0o644, + ) + os.close(fd) + + # Write our PID to the lock file + self.lock_file.write_text(str(os.getpid())) + self.acquired = True + return self + + except FileExistsError: + # Lock file exists - check if process is still running + if self.lock_file.exists(): + try: + pid = int(self.lock_file.read_text().strip()) + import os as _os + + try: + _os.kill(pid, 0) + is_running = True + except (OSError, ProcessLookupError): + is_running = False + + if not is_running: + # Stale lock - remove it + self.lock_file.unlink() + continue + except (ValueError, ProcessLookupError): + # Invalid PID or can't check - remove stale lock + self.lock_file.unlink() + continue + + # Active lock - wait or timeout + if time.time() - start_time >= max_wait: + raise SpecNumberLockError( + f"Could not acquire spec numbering lock after {max_wait}s" + ) + + time.sleep(0.1) # Shorter sleep for spec creation + + def __exit__(self, exc_type, exc_val, exc_tb): + """Release the spec numbering lock.""" + if self.acquired and self.lock_file.exists(): + try: + self.lock_file.unlink() + except Exception: + pass # Best effort cleanup + + def get_next_spec_number(self) -> int: + """ + Scan all spec locations and return the next available spec number. + + Must be called while lock is held. + + Returns: + Next available spec number (global max + 1) + """ + if not self.acquired: + raise SpecNumberLockError( + "Lock must be acquired before getting next spec number" + ) + + if self._global_max is not None: + return self._global_max + 1 + + max_number = 0 + + # 1. Scan main project specs + main_specs_dir = self.project_dir / ".auto-claude" / "specs" + max_number = max(max_number, self._scan_specs_dir(main_specs_dir)) + + # 2. Scan all worktree specs + worktrees_dir = self.project_dir / ".worktrees" + if worktrees_dir.exists(): + for worktree in worktrees_dir.iterdir(): + if worktree.is_dir(): + worktree_specs = worktree / ".auto-claude" / "specs" + max_number = max(max_number, self._scan_specs_dir(worktree_specs)) + + self._global_max = max_number + return max_number + 1 + + def _scan_specs_dir(self, specs_dir: Path) -> int: + """Scan a specs directory and return the highest spec number found.""" + if not specs_dir.exists(): + return 0 + + max_num = 0 + for folder in specs_dir.glob("[0-9][0-9][0-9]-*"): + try: + num = int(folder.name[:3]) + max_num = max(max_num, num) + except ValueError: + pass + + return max_num diff --git a/apps/backend/spec/pipeline/models.py b/apps/backend/spec/pipeline/models.py index f270e43f..f1a08303 100644 --- a/apps/backend/spec/pipeline/models.py +++ b/apps/backend/spec/pipeline/models.py @@ -5,15 +5,21 @@ Pipeline Models and Utilities Data structures, helper functions, and utilities for the spec creation pipeline. """ +from __future__ import annotations + import json import shutil from datetime import datetime, timedelta from pathlib import Path +from typing import TYPE_CHECKING from init import init_auto_claude_dir from task_logger import update_task_logger_path from ui import Icons, highlight, print_status +if TYPE_CHECKING: + from core.workspace.models import SpecNumberLock + def get_specs_dir(project_dir: Path, dev_mode: bool = False) -> Path: """Get the specs directory path. @@ -78,29 +84,37 @@ def cleanup_orphaned_pending_folders(specs_dir: Path) -> None: pass -def create_spec_dir(specs_dir: Path) -> Path: +def create_spec_dir(specs_dir: Path, lock: SpecNumberLock | None = None) -> Path: """Create a new spec directory with incremented number and placeholder name. Args: specs_dir: The parent specs directory + lock: Optional SpecNumberLock for coordinated numbering across worktrees. + If provided, uses global scan to prevent spec number collisions. + If None, uses local scan only (legacy behavior for single process). Returns: Path to the new spec directory """ - existing = list(specs_dir.glob("[0-9][0-9][0-9]-*")) - - if existing: - # Find the HIGHEST folder number - numbers = [] - for folder in existing: - try: - num = int(folder.name[:3]) - numbers.append(num) - except ValueError: - pass - next_num = max(numbers) + 1 if numbers else 1 + if lock is not None: + # Use global coordination via lock - scans main project + all worktrees + next_num = lock.get_next_spec_number() else: - next_num = 1 + # Legacy local scan (fallback for cases without lock) + existing = list(specs_dir.glob("[0-9][0-9][0-9]-*")) + + if existing: + # Find the HIGHEST folder number + numbers = [] + for folder in existing: + try: + num = int(folder.name[:3]) + numbers.append(num) + except ValueError: + pass + next_num = max(numbers) + 1 if numbers else 1 + else: + next_num = 1 # Start with placeholder - will be renamed after requirements gathering name = "pending" diff --git a/apps/backend/spec/pipeline/orchestrator.py b/apps/backend/spec/pipeline/orchestrator.py index ddef9f91..1f57cd16 100644 --- a/apps/backend/spec/pipeline/orchestrator.py +++ b/apps/backend/spec/pipeline/orchestrator.py @@ -10,6 +10,7 @@ from collections.abc import Callable from pathlib import Path from analysis.analyzers import analyze_project +from core.workspace.models import SpecNumberLock from phase_config import get_thinking_budget from prompts_pkg.project_context import should_refresh_project_index from review import run_review_checkpoint @@ -96,12 +97,16 @@ class SpecOrchestrator: if spec_dir: # Use provided spec directory (from UI) self.spec_dir = Path(spec_dir) + self.spec_dir.mkdir(parents=True, exist_ok=True) elif spec_name: self.spec_dir = self.specs_dir / spec_name + self.spec_dir.mkdir(parents=True, exist_ok=True) else: - self.spec_dir = create_spec_dir(self.specs_dir) - - self.spec_dir.mkdir(parents=True, exist_ok=True) + # Use lock for coordinated spec numbering across worktrees + with SpecNumberLock(self.project_dir) as lock: + self.spec_dir = create_spec_dir(self.specs_dir, lock) + # Create directory inside lock to ensure atomicity + self.spec_dir.mkdir(parents=True, exist_ok=True) self.validator = SpecValidator(self.spec_dir) # Agent runner (initialized when needed) diff --git a/apps/frontend/src/main/ipc-handlers/github/import-handlers.ts b/apps/frontend/src/main/ipc-handlers/github/import-handlers.ts index 1ae9f576..8a38619e 100644 --- a/apps/frontend/src/main/ipc-handlers/github/import-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/github/import-handlers.ts @@ -59,8 +59,8 @@ ${labelsString ? `**Labels:** ${labelsString}` : ''} ${issue.body || 'No description provided.'} `; - // Create spec directory and files - const specData = createSpecForIssue( + // Create spec directory and files (with coordinated numbering) + const specData = await createSpecForIssue( project, issue.number, issue.title, diff --git a/apps/frontend/src/main/ipc-handlers/github/investigation-handlers.ts b/apps/frontend/src/main/ipc-handlers/github/investigation-handlers.ts index 7a710a50..b5c39e65 100644 --- a/apps/frontend/src/main/ipc-handlers/github/investigation-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/github/investigation-handlers.ts @@ -140,8 +140,8 @@ export function registerInvestigateIssue( issueContext ); - // Create spec directory and files - const specData = createSpecForIssue( + // Create spec directory and files (with coordinated numbering) + const specData = await createSpecForIssue( project, issue.number, issue.title, diff --git a/apps/frontend/src/main/ipc-handlers/github/spec-utils.ts b/apps/frontend/src/main/ipc-handlers/github/spec-utils.ts index 66d750d8..1bf9d8d4 100644 --- a/apps/frontend/src/main/ipc-handlers/github/spec-utils.ts +++ b/apps/frontend/src/main/ipc-handlers/github/spec-utils.ts @@ -3,9 +3,10 @@ */ import path from 'path'; -import { existsSync, mkdirSync, readdirSync, writeFileSync } from 'fs'; +import { existsSync, mkdirSync, writeFileSync } from 'fs'; import { AUTO_BUILD_PATHS, getSpecsDir } from '../../../shared/constants'; import type { Project, TaskMetadata } from '../../../shared/types'; +import { withSpecNumberLock } from '../../utils/spec-number-lock'; export interface SpecCreationData { specId: string; @@ -14,32 +15,6 @@ export interface SpecCreationData { metadata: TaskMetadata; } -/** - * Find the next available spec number - */ -function getNextSpecNumber(specsDir: string): number { - if (!existsSync(specsDir)) { - return 1; - } - - const existingDirs = readdirSync(specsDir, { withFileTypes: true }) - .filter(d => d.isDirectory()) - .map(d => d.name); - - const existingNumbers = existingDirs - .map(name => { - const match = name.match(/^(\d+)/); - return match ? parseInt(match[1], 10) : 0; - }) - .filter(n => n > 0); - - if (existingNumbers.length > 0) { - return Math.max(...existingNumbers) + 1; - } - - return 1; -} - /** * Create a slug from a title */ @@ -105,15 +80,16 @@ function determineCategoryFromLabels(labels: string[]): 'feature' | 'bug_fix' | /** * Create a new spec directory and initial files + * Uses coordinated spec numbering to prevent collisions across worktrees */ -export function createSpecForIssue( +export async function createSpecForIssue( project: Project, issueNumber: number, issueTitle: string, taskDescription: string, githubUrl: string, labels: string[] = [] -): SpecCreationData { +): Promise { const specsBaseDir = getSpecsDir(project.autoBuildPath); const specsDir = path.join(project.path, specsBaseDir); @@ -121,63 +97,66 @@ export function createSpecForIssue( mkdirSync(specsDir, { recursive: true }); } - // Generate spec ID - const specNumber = getNextSpecNumber(specsDir); - const slugifiedTitle = slugifyTitle(issueTitle); - const specId = `${String(specNumber).padStart(3, '0')}-${slugifiedTitle}`; + // Use coordinated spec numbering with lock to prevent collisions + return await withSpecNumberLock(project.path, async (lock) => { + // Get next spec number from global scan (main + all worktrees) + const specNumber = lock.getNextSpecNumber(project.autoBuildPath); + const slugifiedTitle = slugifyTitle(issueTitle); + const specId = `${String(specNumber).padStart(3, '0')}-${slugifiedTitle}`; - // Create spec directory - const specDir = path.join(specsDir, specId); - mkdirSync(specDir, { recursive: true }); + // Create spec directory (inside lock to ensure atomicity) + const specDir = path.join(specsDir, specId); + mkdirSync(specDir, { recursive: true }); - // Create initial files - const now = new Date().toISOString(); + // Create initial files + const now = new Date().toISOString(); - // implementation_plan.json - const implementationPlan = { - feature: issueTitle, - description: taskDescription, - created_at: now, - updated_at: now, - status: 'pending', - phases: [] - }; - writeFileSync( - path.join(specDir, AUTO_BUILD_PATHS.IMPLEMENTATION_PLAN), - JSON.stringify(implementationPlan, null, 2) - ); + // implementation_plan.json + const implementationPlan = { + feature: issueTitle, + description: taskDescription, + created_at: now, + updated_at: now, + status: 'pending', + phases: [] + }; + writeFileSync( + path.join(specDir, AUTO_BUILD_PATHS.IMPLEMENTATION_PLAN), + JSON.stringify(implementationPlan, null, 2) + ); - // requirements.json - const requirements = { - task_description: taskDescription, - workflow_type: 'feature' - }; - writeFileSync( - path.join(specDir, AUTO_BUILD_PATHS.REQUIREMENTS), - JSON.stringify(requirements, null, 2) - ); + // requirements.json + const requirements = { + task_description: taskDescription, + workflow_type: 'feature' + }; + writeFileSync( + path.join(specDir, AUTO_BUILD_PATHS.REQUIREMENTS), + JSON.stringify(requirements, null, 2) + ); - // Determine category from GitHub issue labels - const category = determineCategoryFromLabels(labels); + // Determine category from GitHub issue labels + const category = determineCategoryFromLabels(labels); - // task_metadata.json - const metadata: TaskMetadata = { - sourceType: 'github', - githubIssueNumber: issueNumber, - githubUrl, - category - }; - writeFileSync( - path.join(specDir, 'task_metadata.json'), - JSON.stringify(metadata, null, 2) - ); + // task_metadata.json + const metadata: TaskMetadata = { + sourceType: 'github', + githubIssueNumber: issueNumber, + githubUrl, + category + }; + writeFileSync( + path.join(specDir, 'task_metadata.json'), + JSON.stringify(metadata, null, 2) + ); - return { - specId, - specDir, - taskDescription, - metadata - }; + return { + specId, + specDir, + taskDescription, + metadata + }; + }); } /** diff --git a/apps/frontend/src/main/ipc-handlers/ideation/task-converter.ts b/apps/frontend/src/main/ipc-handlers/ideation/task-converter.ts index c0895fba..34b593c8 100644 --- a/apps/frontend/src/main/ipc-handlers/ideation/task-converter.ts +++ b/apps/frontend/src/main/ipc-handlers/ideation/task-converter.ts @@ -3,7 +3,7 @@ */ import path from 'path'; -import { existsSync, mkdirSync, readdirSync, writeFileSync } from 'fs'; +import { existsSync, mkdirSync, writeFileSync } from 'fs'; import type { IpcMainInvokeEvent } from 'electron'; import { AUTO_BUILD_PATHS, getSpecsDir } from '../../../shared/constants'; import type { @@ -19,33 +19,7 @@ import type { import { projectStore } from '../../project-store'; import { readIdeationFile, writeIdeationFile, updateIdeationTimestamp } from './file-utils'; import type { RawIdea } from './types'; - -/** - * Find the next available spec number - */ -function findNextSpecNumber(specsDir: string): number { - if (!existsSync(specsDir)) { - return 1; - } - - try { - const existingSpecs = readdirSync(specsDir, { withFileTypes: true }) - .filter(d => d.isDirectory()) - .map(d => { - const match = d.name.match(/^(\d+)-/); - return match ? parseInt(match[1], 10) : 0; - }) - .filter(n => n > 0); - - if (existingSpecs.length > 0) { - return Math.max(...existingSpecs) + 1; - } - } catch { - // Use default 1 - } - - return 1; -} +import { withSpecNumberLock } from '../../utils/spec-number-lock'; /** * Create a slugified version of a title for use in directory names @@ -241,45 +215,48 @@ export async function convertIdeaToTask( mkdirSync(specsDir, { recursive: true }); } - // Find next spec number and create spec ID - const nextNum = findNextSpecNumber(specsDir); - const slugifiedTitle = slugifyTitle(idea.title); - const specId = `${String(nextNum).padStart(3, '0')}-${slugifiedTitle}`; - const specDir = path.join(specsDir, specId); + // Use coordinated spec numbering with lock to prevent collisions + return await withSpecNumberLock(project.path, async (lock) => { + // Get next spec number from global scan (main + all worktrees) + const nextNum = lock.getNextSpecNumber(project.autoBuildPath); + const slugifiedTitle = slugifyTitle(idea.title); + const specId = `${String(nextNum).padStart(3, '0')}-${slugifiedTitle}`; + const specDir = path.join(specsDir, specId); - // Build task description and metadata - const taskDescription = buildTaskDescription(idea); - const metadata = buildTaskMetadata(idea); + // Build task description and metadata + const taskDescription = buildTaskDescription(idea); + const metadata = buildTaskMetadata(idea); - // Create spec files - createSpecFiles(specDir, idea, taskDescription); + // Create spec files (inside lock to ensure atomicity) + createSpecFiles(specDir, idea, taskDescription); - // Save metadata - const metadataPath = path.join(specDir, 'task_metadata.json'); - writeFileSync(metadataPath, JSON.stringify(metadata, null, 2)); + // Save metadata + const metadataPath = path.join(specDir, 'task_metadata.json'); + writeFileSync(metadataPath, JSON.stringify(metadata, null, 2)); - // Update idea status to archived (converted ideas are archived) - idea.status = 'archived'; - idea.linked_task_id = specId; - updateIdeationTimestamp(ideation); - writeIdeationFile(ideationPath, ideation); + // Update idea status to archived (converted ideas are archived) + idea.status = 'archived'; + idea.linked_task_id = specId; + updateIdeationTimestamp(ideation); + writeIdeationFile(ideationPath, ideation); - // Create task object to return - const task: Task = { - id: specId, - specId: specId, - projectId, - title: idea.title, - description: taskDescription, - status: 'backlog', - subtasks: [], - logs: [], - metadata, - createdAt: new Date(), - updatedAt: new Date() - }; + // Create task object to return + const task: Task = { + id: specId, + specId: specId, + projectId, + title: idea.title, + description: taskDescription, + status: 'backlog', + subtasks: [], + logs: [], + metadata, + createdAt: new Date(), + updatedAt: new Date() + }; - return { success: true, data: task }; + return { success: true, data: task }; + }); } catch (error) { return { success: false, diff --git a/apps/frontend/src/main/utils/spec-number-lock.ts b/apps/frontend/src/main/utils/spec-number-lock.ts new file mode 100644 index 00000000..d7a57bea --- /dev/null +++ b/apps/frontend/src/main/utils/spec-number-lock.ts @@ -0,0 +1,225 @@ +/** + * Spec Number Lock - Distributed locking for spec number coordination + * + * Prevents race conditions when creating specs by: + * 1. Acquiring an exclusive file lock + * 2. Scanning ALL spec locations (main + worktrees) + * 3. Finding global maximum spec number + * 4. Allowing atomic spec directory creation + */ + +import { + existsSync, + mkdirSync, + readdirSync, + writeFileSync, + unlinkSync, + readFileSync +} from 'fs'; +import path from 'path'; + +export class SpecNumberLockError extends Error { + constructor(message: string) { + super(message); + this.name = 'SpecNumberLockError'; + } +} + +export class SpecNumberLock { + private projectDir: string; + private lockDir: string; + private lockFile: string; + private acquired: boolean = false; + private globalMax: number | null = null; + + constructor(projectDir: string) { + this.projectDir = projectDir; + this.lockDir = path.join(projectDir, '.auto-claude', '.locks'); + this.lockFile = path.join(this.lockDir, 'spec-numbering.lock'); + } + + /** + * Acquire the spec numbering lock + */ + async acquire(): Promise { + // Ensure lock directory exists + if (!existsSync(this.lockDir)) { + mkdirSync(this.lockDir, { recursive: true }); + } + + const maxWait = 30000; // 30 seconds in ms + const startTime = Date.now(); + + while (true) { + try { + // Try to create lock file exclusively using 'wx' flag + // This will throw if file already exists + if (!existsSync(this.lockFile)) { + writeFileSync(this.lockFile, String(process.pid), { flag: 'wx' }); + this.acquired = true; + return; + } + } catch (error: unknown) { + // EEXIST means file was created by another process between check and create + if ((error as NodeJS.ErrnoException).code !== 'EEXIST') { + throw error; + } + } + + // Lock file exists - check if holder is still running + if (existsSync(this.lockFile)) { + try { + const pidStr = readFileSync(this.lockFile, 'utf-8').trim(); + const pid = parseInt(pidStr, 10); + + if (!isNaN(pid) && !this.isProcessRunning(pid)) { + // Stale lock - remove it + try { + unlinkSync(this.lockFile); + continue; + } catch { + // Another process may have removed it + } + } + } catch { + // Invalid lock file - try to remove + try { + unlinkSync(this.lockFile); + continue; + } catch { + // Ignore removal errors + } + } + } + + // Check timeout + if (Date.now() - startTime >= maxWait) { + throw new SpecNumberLockError( + `Could not acquire spec numbering lock after ${maxWait / 1000}s` + ); + } + + // Wait before retry (100ms for quick turnaround) + await new Promise(resolve => setTimeout(resolve, 100)); + } + } + + /** + * Release the spec numbering lock + */ + release(): void { + if (this.acquired && existsSync(this.lockFile)) { + try { + unlinkSync(this.lockFile); + } catch { + // Best effort cleanup + } + this.acquired = false; + } + } + + /** + * Check if a process is still running + */ + private isProcessRunning(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } + } + + /** + * Get the next available spec number (must be called while lock is held) + */ + getNextSpecNumber(autoBuildPath?: string): number { + if (!this.acquired) { + throw new SpecNumberLockError( + 'Lock must be acquired before getting next spec number' + ); + } + + if (this.globalMax !== null) { + return this.globalMax + 1; + } + + let maxNumber = 0; + + // Determine specs directory base path + const specsBase = autoBuildPath || '.auto-claude'; + + // 1. Scan main project specs + const mainSpecsDir = path.join(this.projectDir, specsBase, 'specs'); + maxNumber = Math.max(maxNumber, this.scanSpecsDir(mainSpecsDir)); + + // 2. Scan all worktree specs + const worktreesDir = path.join(this.projectDir, '.worktrees'); + if (existsSync(worktreesDir)) { + try { + const worktrees = readdirSync(worktreesDir, { withFileTypes: true }); + for (const worktree of worktrees) { + if (worktree.isDirectory()) { + const worktreeSpecsDir = path.join( + worktreesDir, + worktree.name, + specsBase, + 'specs' + ); + maxNumber = Math.max(maxNumber, this.scanSpecsDir(worktreeSpecsDir)); + } + } + } catch { + // Ignore errors scanning worktrees + } + } + + this.globalMax = maxNumber; + return maxNumber + 1; + } + + /** + * Scan a specs directory and return the highest spec number found + */ + private scanSpecsDir(specsDir: string): number { + if (!existsSync(specsDir)) { + return 0; + } + + let maxNum = 0; + try { + const entries = readdirSync(specsDir, { withFileTypes: true }); + for (const entry of entries) { + if (entry.isDirectory()) { + const match = entry.name.match(/^(\d{3})-/); + if (match) { + const num = parseInt(match[1], 10); + if (!isNaN(num)) { + maxNum = Math.max(maxNum, num); + } + } + } + } + } catch { + // Ignore read errors + } + + return maxNum; + } +} + +/** + * Helper function to create a spec with coordinated numbering + */ +export async function withSpecNumberLock( + projectDir: string, + callback: (lock: SpecNumberLock) => T | Promise +): Promise { + const lock = new SpecNumberLock(projectDir); + try { + await lock.acquire(); + return await callback(lock); + } finally { + lock.release(); + } +}