From 98d98cc05691f20f8503dae2572cd0f97c6d5570 Mon Sep 17 00:00:00 2001 From: Shirone Date: Sun, 25 Jan 2026 13:58:21 +0100 Subject: [PATCH 1/5] fix(ui): fix spinner visibility in github issue validation button The spinner component in the GitHub issue validation button was blended into the button's primary background color, making it invisible. This was caused by the spinner using the default 'primary' variant which applies text-primary color, matching the button's background. Changed the spinner to use the 'foreground' variant which applies text-primary-foreground for proper contrast against the primary background. This follows the existing pattern already implemented in the worktree panel components. Fixes #697 --- .../views/github-issues-view/components/issue-detail-panel.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/ui/src/components/views/github-issues-view/components/issue-detail-panel.tsx b/apps/ui/src/components/views/github-issues-view/components/issue-detail-panel.tsx index cc62a7fe..dbf38e96 100644 --- a/apps/ui/src/components/views/github-issues-view/components/issue-detail-panel.tsx +++ b/apps/ui/src/components/views/github-issues-view/components/issue-detail-panel.tsx @@ -87,7 +87,7 @@ export function IssueDetailPanel({ if (isValidating) { return ( ); From 80ef21c8d048f4350fad6fe3d291fc7b699b4e74 Mon Sep 17 00:00:00 2001 From: Shirone Date: Sun, 25 Jan 2026 14:07:17 +0100 Subject: [PATCH 2/5] refactor(ui): use Button loading prop instead of manual Spinner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address PR #698 review feedback from Gemini Code Assist to use the Button component's built-in loading prop instead of manually rendering Spinner components. The Button component already handles spinner display with correct variant selection (foreground for default/destructive buttons, primary for others), so this simplifies the code and aligns with component abstractions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- .../components/issue-detail-panel.tsx | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/apps/ui/src/components/views/github-issues-view/components/issue-detail-panel.tsx b/apps/ui/src/components/views/github-issues-view/components/issue-detail-panel.tsx index dbf38e96..c44dbbe2 100644 --- a/apps/ui/src/components/views/github-issues-view/components/issue-detail-panel.tsx +++ b/apps/ui/src/components/views/github-issues-view/components/issue-detail-panel.tsx @@ -86,8 +86,7 @@ export function IssueDetailPanel({ {(() => { if (isValidating) { return ( - ); @@ -335,15 +334,9 @@ export function IssueDetailPanel({ className="w-full" onClick={loadMore} disabled={loadingMore} + loading={loadingMore} > - {loadingMore ? ( - <> - - Loading... - - ) : ( - 'Load More Comments' - )} + {loadingMore ? 'Loading...' : 'Load More Comments'} )} From 08dc90b37858870787a6c6dcb401457f72abdfb8 Mon Sep 17 00:00:00 2001 From: Shirone Date: Sun, 25 Jan 2026 14:09:14 +0100 Subject: [PATCH 3/5] refactor(ui): remove redundant disabled props when using Button loading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Button component internally sets disabled when loading=true, so explicit disabled props are redundant and can be removed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- .../views/github-issues-view/components/issue-detail-panel.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apps/ui/src/components/views/github-issues-view/components/issue-detail-panel.tsx b/apps/ui/src/components/views/github-issues-view/components/issue-detail-panel.tsx index c44dbbe2..e81eac9b 100644 --- a/apps/ui/src/components/views/github-issues-view/components/issue-detail-panel.tsx +++ b/apps/ui/src/components/views/github-issues-view/components/issue-detail-panel.tsx @@ -86,7 +86,7 @@ export function IssueDetailPanel({ {(() => { if (isValidating) { return ( - ); @@ -333,7 +333,6 @@ export function IssueDetailPanel({ size="sm" className="w-full" onClick={loadMore} - disabled={loadingMore} loading={loadingMore} > {loadingMore ? 'Loading...' : 'Load More Comments'} From 011ac404bbb32792cb99282054d9375c3e9ab04e Mon Sep 17 00:00:00 2001 From: Shirone Date: Sun, 25 Jan 2026 14:38:39 +0100 Subject: [PATCH 4/5] fix: Prevent features from getting stuck in in_progress after server restart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add graceful shutdown handler that marks running features as 'interrupted' before server exit (SIGTERM/SIGINT) - Add 30-second shutdown timeout to prevent hanging on exit - Add orphan detection to identify features with missing branches - Add isFeatureRunning() for idempotent resume checks - Improve resumeInterruptedFeatures() to handle features without saved context - Add 'interrupted' status to FeatureStatusWithPipeline type - Replace console.log with proper logger in auto-mode-service - Add comprehensive unit tests for all new functionality (15 new tests) Fixes #696 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- apps/server/src/index.ts | 41 +- apps/server/src/routes/features/index.ts | 10 +- .../server/src/routes/features/routes/list.ts | 34 +- apps/server/src/services/auto-mode-service.ts | 336 +++++++++++++-- .../unit/services/auto-mode-service.test.ts | 400 ++++++++++++++++++ libs/types/src/pipeline.ts | 1 + 6 files changed, 779 insertions(+), 43 deletions(-) diff --git a/apps/server/src/index.ts b/apps/server/src/index.ts index a5f7cbcb..06040100 100644 --- a/apps/server/src/index.ts +++ b/apps/server/src/index.ts @@ -326,7 +326,10 @@ app.get('/api/health/detailed', createDetailedHandler()); app.use('/api/fs', createFsRoutes(events)); app.use('/api/agent', createAgentRoutes(agentService, events)); app.use('/api/sessions', createSessionsRoutes(agentService)); -app.use('/api/features', createFeaturesRoutes(featureLoader, settingsService, events)); +app.use( + '/api/features', + createFeaturesRoutes(featureLoader, settingsService, events, autoModeService) +); app.use('/api/auto-mode', createAutoModeRoutes(autoModeService)); app.use('/api/enhance-prompt', createEnhancePromptRoutes(settingsService)); app.use('/api/worktree', createWorktreeRoutes(events, settingsService)); @@ -769,21 +772,39 @@ process.on('uncaughtException', (error: Error) => { process.exit(1); }); -// Graceful shutdown -process.on('SIGTERM', () => { - logger.info('SIGTERM received, shutting down...'); +// Graceful shutdown timeout (30 seconds) +const SHUTDOWN_TIMEOUT_MS = 30000; + +// Graceful shutdown helper +const gracefulShutdown = async (signal: string) => { + logger.info(`${signal} received, shutting down...`); + + // Set up a force-exit timeout to prevent hanging + const forceExitTimeout = setTimeout(() => { + logger.error(`Shutdown timed out after ${SHUTDOWN_TIMEOUT_MS}ms, forcing exit`); + process.exit(1); + }, SHUTDOWN_TIMEOUT_MS); + + // Mark all running features as interrupted before shutdown + // This ensures they can be resumed when the server restarts + try { + await autoModeService.markAllRunningFeaturesInterrupted(`${signal} signal received`); + } catch (error) { + logger.error('Failed to mark running features as interrupted:', error); + } + terminalService.cleanup(); server.close(() => { + clearTimeout(forceExitTimeout); logger.info('Server closed'); process.exit(0); }); +}; + +process.on('SIGTERM', () => { + gracefulShutdown('SIGTERM'); }); process.on('SIGINT', () => { - logger.info('SIGINT received, shutting down...'); - terminalService.cleanup(); - server.close(() => { - logger.info('Server closed'); - process.exit(0); - }); + gracefulShutdown('SIGINT'); }); diff --git a/apps/server/src/routes/features/index.ts b/apps/server/src/routes/features/index.ts index e4fed9d4..8c7dbb06 100644 --- a/apps/server/src/routes/features/index.ts +++ b/apps/server/src/routes/features/index.ts @@ -5,6 +5,7 @@ import { Router } from 'express'; import { FeatureLoader } from '../../services/feature-loader.js'; import type { SettingsService } from '../../services/settings-service.js'; +import type { AutoModeService } from '../../services/auto-mode-service.js'; import type { EventEmitter } from '../../lib/events.js'; import { validatePathParams } from '../../middleware/validate-paths.js'; import { createListHandler } from './routes/list.js'; @@ -22,11 +23,16 @@ import { createImportHandler, createConflictCheckHandler } from './routes/import export function createFeaturesRoutes( featureLoader: FeatureLoader, settingsService?: SettingsService, - events?: EventEmitter + events?: EventEmitter, + autoModeService?: AutoModeService ): Router { const router = Router(); - router.post('/list', validatePathParams('projectPath'), createListHandler(featureLoader)); + router.post( + '/list', + validatePathParams('projectPath'), + createListHandler(featureLoader, autoModeService) + ); router.post('/get', validatePathParams('projectPath'), createGetHandler(featureLoader)); router.post( '/create', diff --git a/apps/server/src/routes/features/routes/list.ts b/apps/server/src/routes/features/routes/list.ts index 00127fc9..7920db73 100644 --- a/apps/server/src/routes/features/routes/list.ts +++ b/apps/server/src/routes/features/routes/list.ts @@ -1,12 +1,19 @@ /** * POST /list endpoint - List all features for a project + * + * Also performs orphan detection when a project is loaded to identify + * features whose branches no longer exist. This runs on every project load/switch. */ import type { Request, Response } from 'express'; import { FeatureLoader } from '../../../services/feature-loader.js'; +import type { AutoModeService } from '../../../services/auto-mode-service.js'; import { getErrorMessage, logError } from '../common.js'; +import { createLogger } from '@automaker/utils'; -export function createListHandler(featureLoader: FeatureLoader) { +const logger = createLogger('FeaturesListRoute'); + +export function createListHandler(featureLoader: FeatureLoader, autoModeService?: AutoModeService) { return async (req: Request, res: Response): Promise => { try { const { projectPath } = req.body as { projectPath: string }; @@ -17,6 +24,31 @@ export function createListHandler(featureLoader: FeatureLoader) { } const features = await featureLoader.getAll(projectPath); + + // Run orphan detection in background when project is loaded + // This detects features whose branches no longer exist (e.g., after merge/delete) + // We don't await this to keep the list response fast + if (autoModeService) { + autoModeService + .detectOrphanedFeatures(projectPath) + .then((orphanedFeatures) => { + if (orphanedFeatures.length > 0) { + logger.info( + `[ProjectLoad] Detected ${orphanedFeatures.length} orphaned feature(s) in ${projectPath}` + ); + for (const { feature, missingBranch } of orphanedFeatures) { + logger.info( + `[ProjectLoad] Orphaned: ${feature.title || feature.id} - branch "${missingBranch}" no longer exists` + ); + } + } + }) + .catch((error: unknown) => { + const errorMessage = error instanceof Error ? error.message : String(error); + logger.warn(`[ProjectLoad] Failed to detect orphaned features: ${errorMessage}`); + }); + } + res.json({ success: true, features }); } catch (error) { logError(error, 'List features failed'); diff --git a/apps/server/src/services/auto-mode-service.ts b/apps/server/src/services/auto-mode-service.ts index 8715278b..64ab7ee6 100644 --- a/apps/server/src/services/auto-mode-service.ts +++ b/apps/server/src/services/auto-mode-service.ts @@ -335,6 +335,19 @@ export class AutoModeService { this.settingsService = settingsService ?? null; } + /** + * Acquire a slot in the runningFeatures map for a feature. + * Implements reference counting via leaseCount to support nested calls + * (e.g., resumeFeature -> executeFeature). + * + * @param params.featureId - ID of the feature to track + * @param params.projectPath - Path to the project + * @param params.isAutoMode - Whether this is an auto-mode execution + * @param params.allowReuse - If true, allows incrementing leaseCount for already-running features + * @param params.abortController - Optional abort controller to use + * @returns The RunningFeature entry (existing or newly created) + * @throws Error if feature is already running and allowReuse is false + */ private acquireRunningFeature(params: { featureId: string; projectPath: string; @@ -347,7 +360,7 @@ export class AutoModeService { if (!params.allowReuse) { throw new Error('already running'); } - existing.leaseCount = (existing.leaseCount ?? 1) + 1; + existing.leaseCount += 1; return existing; } @@ -366,6 +379,14 @@ export class AutoModeService { return entry; } + /** + * Release a slot in the runningFeatures map for a feature. + * Decrements leaseCount and only removes the entry when it reaches zero, + * unless force option is used. + * + * @param featureId - ID of the feature to release + * @param options.force - If true, immediately removes the entry regardless of leaseCount + */ private releaseRunningFeature(featureId: string, options?: { force?: boolean }): void { const entry = this.runningFeatures.get(featureId); if (!entry) { @@ -377,7 +398,7 @@ export class AutoModeService { return; } - entry.leaseCount = (entry.leaseCount ?? 1) - 1; + entry.leaseCount -= 1; if (entry.leaseCount <= 0) { this.runningFeatures.delete(featureId); } @@ -1628,7 +1649,17 @@ Complete the pipeline step instructions above. Review the previous work and appl } /** - * Resume a feature (continues from saved context) + * Resume a feature (continues from saved context or starts fresh if no context) + * + * This method handles interrupted features regardless of whether they have saved context: + * - With context: Continues from where the agent left off using the saved agent-output.md + * - Without context: Starts fresh execution (feature was interrupted before any agent output) + * - Pipeline features: Delegates to resumePipelineFeature for specialized handling + * + * @param projectPath - Path to the project + * @param featureId - ID of the feature to resume + * @param useWorktrees - Whether to use git worktrees for isolation + * @param _calledInternally - Internal flag to prevent double-tracking when called from other methods */ async resumeFeature( projectPath: string, @@ -1637,6 +1668,15 @@ Complete the pipeline step instructions above. Review the previous work and appl /** Internal flag: set to true when called from a method that already tracks the feature */ _calledInternally = false ): Promise { + // Idempotent check: if feature is already being resumed/running, skip silently + // This prevents race conditions when multiple callers try to resume the same feature + if (!_calledInternally && this.isFeatureRunning(featureId)) { + logger.info( + `[AutoMode] Feature ${featureId} is already being resumed/running, skipping duplicate resume request` + ); + return; + } + this.acquireRunningFeature({ featureId, projectPath, @@ -1651,6 +1691,10 @@ Complete the pipeline step instructions above. Review the previous work and appl throw new Error(`Feature ${featureId} not found`); } + logger.info( + `[AutoMode] Resuming feature ${featureId} (${feature.title}) - current status: ${feature.status}` + ); + // Check if feature is stuck in a pipeline step const pipelineInfo = await this.detectPipelineStatus( projectPath, @@ -1661,6 +1705,9 @@ Complete the pipeline step instructions above. Review the previous work and appl if (pipelineInfo.isPipeline) { // Feature stuck in pipeline - use pipeline resume // Pass _alreadyTracked to prevent double-tracking + logger.info( + `[AutoMode] Feature ${featureId} is in pipeline step ${pipelineInfo.stepId}, using pipeline resume` + ); return await this.resumePipelineFeature(projectPath, feature, useWorktrees, pipelineInfo); } @@ -1674,17 +1721,44 @@ Complete the pipeline step instructions above. Review the previous work and appl await secureFs.access(contextPath); hasContext = true; } catch { - // No context + // No context - feature was interrupted before any agent output was saved } if (hasContext) { // Load previous context and continue // executeFeatureWithContext -> executeFeature will see feature is already tracked const context = (await secureFs.readFile(contextPath, 'utf-8')) as string; + logger.info( + `[AutoMode] Resuming feature ${featureId} with saved context (${context.length} chars)` + ); + + // Emit event for UI notification + this.emitAutoModeEvent('auto_mode_feature_resuming', { + featureId, + featureName: feature.title, + projectPath, + hasContext: true, + message: `Resuming feature "${feature.title}" from saved context`, + }); + return await this.executeFeatureWithContext(projectPath, featureId, context, useWorktrees); } - // No context, start fresh - executeFeature will see feature is already tracked + // No context - feature was interrupted before any agent output was saved + // Start fresh execution instead of leaving the feature stuck + logger.info( + `[AutoMode] Feature ${featureId} has no saved context - starting fresh execution` + ); + + // Emit event for UI notification + this.emitAutoModeEvent('auto_mode_feature_resuming', { + featureId, + featureName: feature.title, + projectPath, + hasContext: false, + message: `Starting fresh execution for interrupted feature "${feature.title}" (no previous context found)`, + }); + return await this.executeFeature(projectPath, featureId, useWorktrees, false, undefined, { _calledInternally: true, }); @@ -1828,8 +1902,8 @@ Complete the pipeline step instructions above. Review the previous work and appl // Check if the current step is excluded // If so, use getNextStatus to find the appropriate next step if (excludedStepIds.has(currentStep.id)) { - console.log( - `[AutoMode] Current step ${currentStep.id} is excluded for feature ${featureId}, finding next valid step` + logger.info( + `Current step ${currentStep.id} is excluded for feature ${featureId}, finding next valid step` ); const nextStatus = pipelineService.getNextStatus( `pipeline_${currentStep.id}`, @@ -1884,8 +1958,8 @@ Complete the pipeline step instructions above. Review the previous work and appl // Use the filtered steps for counting const sortedSteps = allSortedSteps.filter((step) => !excludedStepIds.has(step.id)); - console.log( - `[AutoMode] Resuming pipeline for feature ${featureId} from step ${startFromStepIndex + 1}/${sortedSteps.length}` + logger.info( + `Resuming pipeline for feature ${featureId} from step ${startFromStepIndex + 1}/${sortedSteps.length}` ); const runningEntry = this.acquireRunningFeature({ @@ -1908,11 +1982,9 @@ Complete the pipeline step instructions above. Review the previous work and appl if (useWorktrees && branchName) { worktreePath = await this.findExistingWorktreeForBranch(projectPath, branchName); if (worktreePath) { - console.log(`[AutoMode] Using worktree for branch "${branchName}": ${worktreePath}`); + logger.info(`Using worktree for branch "${branchName}": ${worktreePath}`); } else { - console.warn( - `[AutoMode] Worktree for branch "${branchName}" not found, using project path` - ); + logger.warn(`Worktree for branch "${branchName}" not found, using project path`); } } @@ -1964,7 +2036,7 @@ Complete the pipeline step instructions above. Review the previous work and appl const finalStatus = feature.skipTests ? 'waiting_approval' : 'verified'; await this.updateFeatureStatus(projectPath, featureId, finalStatus); - console.log('[AutoMode] Pipeline resume completed successfully'); + logger.info(`Pipeline resume completed successfully for feature ${featureId}`); this.emitAutoModeEvent('auto_mode_feature_complete', { featureId, @@ -1987,7 +2059,7 @@ Complete the pipeline step instructions above. Review the previous work and appl projectPath, }); } else { - console.error(`[AutoMode] Pipeline resume failed for feature ${featureId}:`, error); + logger.error(`Pipeline resume failed for feature ${featureId}:`, error); await this.updateFeatureStatus(projectPath, featureId, 'backlog'); this.emitAutoModeEvent('auto_mode_error', { featureId, @@ -3015,6 +3087,70 @@ Format your response as a structured markdown document.`; } } + /** + * Mark a feature as interrupted due to server restart or other interruption. + * + * This is a convenience helper that updates the feature status to 'interrupted', + * indicating the feature was in progress but execution was disrupted (e.g., server + * restart, process crash, or manual stop). Features with this status can be + * resumed later using the resume functionality. + * + * @param projectPath - Path to the project + * @param featureId - ID of the feature to mark as interrupted + * @param reason - Optional reason for the interruption (logged for debugging) + */ + async markFeatureInterrupted( + projectPath: string, + featureId: string, + reason?: string + ): Promise { + if (reason) { + logger.info(`Marking feature ${featureId} as interrupted: ${reason}`); + } else { + logger.info(`Marking feature ${featureId} as interrupted`); + } + + await this.updateFeatureStatus(projectPath, featureId, 'interrupted'); + } + + /** + * Mark all currently running features as interrupted. + * + * This method is called during graceful server shutdown to ensure that all + * features currently being executed are properly marked as 'interrupted'. + * This allows them to be detected and resumed when the server restarts. + * + * @param reason - Optional reason for the interruption (logged for debugging) + * @returns Promise that resolves when all features have been marked as interrupted + */ + async markAllRunningFeaturesInterrupted(reason?: string): Promise { + const runningCount = this.runningFeatures.size; + + if (runningCount === 0) { + logger.info('No running features to mark as interrupted'); + return; + } + + const logReason = reason || 'server shutdown'; + logger.info(`Marking ${runningCount} running feature(s) as interrupted due to: ${logReason}`); + + const markPromises: Promise[] = []; + + for (const [featureId, runningFeature] of this.runningFeatures) { + markPromises.push( + this.markFeatureInterrupted(runningFeature.projectPath, featureId, logReason).catch( + (error) => { + logger.error(`Failed to mark feature ${featureId} as interrupted:`, error); + } + ) + ); + } + + await Promise.all(markPromises); + + logger.info(`Finished marking ${runningCount} feature(s) as interrupted`); + } + private isFeatureFinished(feature: Feature): boolean { const isCompleted = feature.status === 'completed' || feature.status === 'verified'; @@ -3030,6 +3166,18 @@ Format your response as a structured markdown document.`; return isCompleted; } + /** + * Check if a feature is currently running (being executed or resumed). + * This is used for idempotent checks to prevent race conditions when + * multiple callers try to resume the same feature simultaneously. + * + * @param featureId - The ID of the feature to check + * @returns true if the feature is currently running, false otherwise + */ + isFeatureRunning(featureId: string): boolean { + return this.runningFeatures.has(featureId); + } + /** * Update the planSpec of a feature */ @@ -4544,7 +4692,9 @@ After generating the revised spec, output: try { const entries = await secureFs.readdir(featuresDir, { withFileTypes: true }); - const interruptedFeatures: Feature[] = []; + // Track features with and without context separately for better logging + const featuresWithContext: Feature[] = []; + const featuresWithoutContext: Feature[] = []; for (const entry of entries) { if (entry.isDirectory()) { @@ -4569,48 +4719,71 @@ After generating the revised spec, output: feature.status === 'in_progress' || (feature.status && feature.status.startsWith('pipeline_')) ) { - // Verify it has existing context (agent-output.md) + // Check if context (agent-output.md) exists const featureDir = getFeatureDir(projectPath, feature.id); const contextPath = path.join(featureDir, 'agent-output.md'); try { await secureFs.access(contextPath); - interruptedFeatures.push(feature); + featuresWithContext.push(feature); logger.info( - `Found interrupted feature: ${feature.id} (${feature.title}) - status: ${feature.status}` + `Found interrupted feature with context: ${feature.id} (${feature.title}) - status: ${feature.status}` ); } catch { - // No context file, skip this feature - it will be restarted fresh - logger.info(`Interrupted feature ${feature.id} has no context, will restart fresh`); + // No context file - feature was interrupted before any agent output + // Still include it for resumption (will start fresh) + featuresWithoutContext.push(feature); + logger.info( + `Found interrupted feature without context: ${feature.id} (${feature.title}) - status: ${feature.status} (will restart fresh)` + ); } } } } - if (interruptedFeatures.length === 0) { + // Combine all interrupted features (with and without context) + const allInterruptedFeatures = [...featuresWithContext, ...featuresWithoutContext]; + + if (allInterruptedFeatures.length === 0) { logger.info('No interrupted features found'); return; } - logger.info(`Found ${interruptedFeatures.length} interrupted feature(s) to resume`); + logger.info( + `Found ${allInterruptedFeatures.length} interrupted feature(s) to resume ` + + `(${featuresWithContext.length} with context, ${featuresWithoutContext.length} without context)` + ); - // Emit event to notify UI + // Emit event to notify UI with context information this.emitAutoModeEvent('auto_mode_resuming_features', { - message: `Resuming ${interruptedFeatures.length} interrupted feature(s) after server restart`, + message: `Resuming ${allInterruptedFeatures.length} interrupted feature(s) after server restart`, projectPath, - featureIds: interruptedFeatures.map((f) => f.id), - features: interruptedFeatures.map((f) => ({ + featureIds: allInterruptedFeatures.map((f) => f.id), + features: allInterruptedFeatures.map((f) => ({ id: f.id, title: f.title, status: f.status, branchName: f.branchName ?? null, + hasContext: featuresWithContext.some((fc) => fc.id === f.id), })), }); // Resume each interrupted feature - for (const feature of interruptedFeatures) { + for (const feature of allInterruptedFeatures) { try { - logger.info(`Resuming feature: ${feature.id} (${feature.title})`); - // Use resumeFeature which will detect the existing context and continue + // Idempotent check: skip if feature is already being resumed (prevents race conditions) + if (this.isFeatureRunning(feature.id)) { + logger.info( + `Feature ${feature.id} (${feature.title}) is already being resumed, skipping` + ); + continue; + } + + const hasContext = featuresWithContext.some((fc) => fc.id === feature.id); + logger.info( + `Resuming feature: ${feature.id} (${feature.title}) - ${hasContext ? 'continuing from context' : 'starting fresh'}` + ); + // Use resumeFeature which will detect the existing context and continue, + // or start fresh if no context exists await this.resumeFeature(projectPath, feature.id, true); } catch (error) { logger.error(`Failed to resume feature ${feature.id}:`, error); @@ -4810,4 +4983,107 @@ After generating the revised spec, output: console.warn(`[AutoMode] Failed to extract learnings from feature ${feature.id}:`, error); } } + + /** + * Detect orphaned features - features whose branchName points to a branch that no longer exists. + * + * Orphaned features can occur when: + * - A feature branch is deleted after merge + * - A worktree is manually removed + * - A branch is force-deleted + * + * @param projectPath - Path to the project + * @returns Array of orphaned features with their missing branch names + */ + async detectOrphanedFeatures( + projectPath: string + ): Promise> { + const orphanedFeatures: Array<{ feature: Feature; missingBranch: string }> = []; + + try { + // Get all features for this project + const allFeatures = await this.featureLoader.getAll(projectPath); + + // Get features that have a branchName set (excludes main branch features) + const featuresWithBranches = allFeatures.filter( + (f) => f.branchName && f.branchName.trim() !== '' + ); + + if (featuresWithBranches.length === 0) { + logger.debug('[detectOrphanedFeatures] No features with branch names found'); + return orphanedFeatures; + } + + // Get all existing branches (local) + const existingBranches = await this.getExistingBranches(projectPath); + + // Get current/primary branch (features with null branchName are implicitly on this) + const primaryBranch = await getCurrentBranch(projectPath); + + // Check each feature with a branchName + for (const feature of featuresWithBranches) { + const branchName = feature.branchName!; + + // Skip if the branchName matches the primary branch (implicitly valid) + if (primaryBranch && branchName === primaryBranch) { + continue; + } + + // Check if the branch exists + if (!existingBranches.has(branchName)) { + orphanedFeatures.push({ + feature, + missingBranch: branchName, + }); + logger.info( + `[detectOrphanedFeatures] Found orphaned feature: ${feature.id} (${feature.title}) - branch "${branchName}" no longer exists` + ); + } + } + + if (orphanedFeatures.length > 0) { + logger.info( + `[detectOrphanedFeatures] Found ${orphanedFeatures.length} orphaned feature(s) in ${projectPath}` + ); + } else { + logger.debug('[detectOrphanedFeatures] No orphaned features found'); + } + + return orphanedFeatures; + } catch (error) { + logger.error('[detectOrphanedFeatures] Error detecting orphaned features:', error); + return orphanedFeatures; + } + } + + /** + * Get all existing local branches for a project + * @param projectPath - Path to the git repository + * @returns Set of branch names + */ + private async getExistingBranches(projectPath: string): Promise> { + const branches = new Set(); + + try { + // Use git for-each-ref to get all local branches + const { stdout } = await execAsync( + 'git for-each-ref --format="%(refname:short)" refs/heads/', + { cwd: projectPath } + ); + + const branchLines = stdout.trim().split('\n'); + for (const branch of branchLines) { + const trimmed = branch.trim(); + if (trimmed) { + branches.add(trimmed); + } + } + + logger.debug(`[getExistingBranches] Found ${branches.size} local branches`); + } catch (error) { + logger.error('[getExistingBranches] Failed to get branches:', error); + } + + return branches; + } } diff --git a/apps/server/tests/unit/services/auto-mode-service.test.ts b/apps/server/tests/unit/services/auto-mode-service.test.ts index 3dda13e2..1de26bae 100644 --- a/apps/server/tests/unit/services/auto-mode-service.test.ts +++ b/apps/server/tests/unit/services/auto-mode-service.test.ts @@ -315,4 +315,404 @@ describe('auto-mode-service.ts', () => { expect(duration).toBeLessThan(40); }); }); + + describe('detectOrphanedFeatures', () => { + // Helper to mock featureLoader.getAll + const mockFeatureLoaderGetAll = (svc: AutoModeService, mockFn: ReturnType) => { + (svc as any).featureLoader = { getAll: mockFn }; + }; + + // Helper to mock getExistingBranches + const mockGetExistingBranches = (svc: AutoModeService, branches: string[]) => { + (svc as any).getExistingBranches = vi.fn().mockResolvedValue(new Set(branches)); + }; + + it('should return empty array when no features have branch names', async () => { + const getAllMock = vi.fn().mockResolvedValue([ + { id: 'f1', title: 'Feature 1', description: 'desc', category: 'test' }, + { id: 'f2', title: 'Feature 2', description: 'desc', category: 'test' }, + ] satisfies Feature[]); + mockFeatureLoaderGetAll(service, getAllMock); + mockGetExistingBranches(service, ['main', 'develop']); + + const result = await service.detectOrphanedFeatures('/test/project'); + + expect(result).toEqual([]); + }); + + it('should return empty array when all feature branches exist', async () => { + const getAllMock = vi.fn().mockResolvedValue([ + { + id: 'f1', + title: 'Feature 1', + description: 'desc', + category: 'test', + branchName: 'feature-1', + }, + { + id: 'f2', + title: 'Feature 2', + description: 'desc', + category: 'test', + branchName: 'feature-2', + }, + ] satisfies Feature[]); + mockFeatureLoaderGetAll(service, getAllMock); + mockGetExistingBranches(service, ['main', 'feature-1', 'feature-2']); + + const result = await service.detectOrphanedFeatures('/test/project'); + + expect(result).toEqual([]); + }); + + it('should detect orphaned features with missing branches', async () => { + const features: Feature[] = [ + { + id: 'f1', + title: 'Feature 1', + description: 'desc', + category: 'test', + branchName: 'feature-1', + }, + { + id: 'f2', + title: 'Feature 2', + description: 'desc', + category: 'test', + branchName: 'deleted-branch', + }, + { id: 'f3', title: 'Feature 3', description: 'desc', category: 'test' }, // No branch + ]; + const getAllMock = vi.fn().mockResolvedValue(features); + mockFeatureLoaderGetAll(service, getAllMock); + mockGetExistingBranches(service, ['main', 'feature-1']); // deleted-branch not in list + + const result = await service.detectOrphanedFeatures('/test/project'); + + expect(result).toHaveLength(1); + expect(result[0].feature.id).toBe('f2'); + expect(result[0].missingBranch).toBe('deleted-branch'); + }); + + it('should detect multiple orphaned features', async () => { + const features: Feature[] = [ + { + id: 'f1', + title: 'Feature 1', + description: 'desc', + category: 'test', + branchName: 'orphan-1', + }, + { + id: 'f2', + title: 'Feature 2', + description: 'desc', + category: 'test', + branchName: 'orphan-2', + }, + { + id: 'f3', + title: 'Feature 3', + description: 'desc', + category: 'test', + branchName: 'valid-branch', + }, + ]; + const getAllMock = vi.fn().mockResolvedValue(features); + mockFeatureLoaderGetAll(service, getAllMock); + mockGetExistingBranches(service, ['main', 'valid-branch']); + + const result = await service.detectOrphanedFeatures('/test/project'); + + expect(result).toHaveLength(2); + expect(result.map((r) => r.feature.id)).toContain('f1'); + expect(result.map((r) => r.feature.id)).toContain('f2'); + }); + + it('should return empty array when getAll throws error', async () => { + const getAllMock = vi.fn().mockRejectedValue(new Error('Failed to load features')); + mockFeatureLoaderGetAll(service, getAllMock); + + const result = await service.detectOrphanedFeatures('/test/project'); + + expect(result).toEqual([]); + }); + + it('should ignore empty branchName strings', async () => { + const features: Feature[] = [ + { id: 'f1', title: 'Feature 1', description: 'desc', category: 'test', branchName: '' }, + { id: 'f2', title: 'Feature 2', description: 'desc', category: 'test', branchName: ' ' }, + ]; + const getAllMock = vi.fn().mockResolvedValue(features); + mockFeatureLoaderGetAll(service, getAllMock); + mockGetExistingBranches(service, ['main']); + + const result = await service.detectOrphanedFeatures('/test/project'); + + expect(result).toEqual([]); + }); + + it('should skip features whose branchName matches the primary branch', async () => { + const features: Feature[] = [ + { id: 'f1', title: 'Feature 1', description: 'desc', category: 'test', branchName: 'main' }, + { + id: 'f2', + title: 'Feature 2', + description: 'desc', + category: 'test', + branchName: 'orphaned', + }, + ]; + const getAllMock = vi.fn().mockResolvedValue(features); + mockFeatureLoaderGetAll(service, getAllMock); + mockGetExistingBranches(service, ['main', 'develop']); + // Mock getCurrentBranch to return 'main' + (service as any).getCurrentBranch = vi.fn().mockResolvedValue('main'); + + const result = await service.detectOrphanedFeatures('/test/project'); + + // Only f2 should be orphaned (orphaned branch doesn't exist) + expect(result).toHaveLength(1); + expect(result[0].feature.id).toBe('f2'); + }); + }); + + describe('markFeatureInterrupted', () => { + // Helper to mock updateFeatureStatus + const mockUpdateFeatureStatus = (svc: AutoModeService, mockFn: ReturnType) => { + (svc as any).updateFeatureStatus = mockFn; + }; + + it('should call updateFeatureStatus with interrupted status', async () => { + const updateMock = vi.fn().mockResolvedValue(undefined); + mockUpdateFeatureStatus(service, updateMock); + + await service.markFeatureInterrupted('/test/project', 'feature-123'); + + expect(updateMock).toHaveBeenCalledWith('/test/project', 'feature-123', 'interrupted'); + }); + + it('should call updateFeatureStatus with reason when provided', async () => { + const updateMock = vi.fn().mockResolvedValue(undefined); + mockUpdateFeatureStatus(service, updateMock); + + await service.markFeatureInterrupted('/test/project', 'feature-123', 'server shutdown'); + + expect(updateMock).toHaveBeenCalledWith('/test/project', 'feature-123', 'interrupted'); + }); + + it('should propagate errors from updateFeatureStatus', async () => { + const updateMock = vi.fn().mockRejectedValue(new Error('Update failed')); + mockUpdateFeatureStatus(service, updateMock); + + await expect(service.markFeatureInterrupted('/test/project', 'feature-123')).rejects.toThrow( + 'Update failed' + ); + }); + }); + + describe('markAllRunningFeaturesInterrupted', () => { + // Helper to access private runningFeatures Map + const getRunningFeaturesMap = (svc: AutoModeService) => + (svc as any).runningFeatures as Map< + string, + { featureId: string; projectPath: string; isAutoMode: boolean } + >; + + // Helper to mock updateFeatureStatus + const mockUpdateFeatureStatus = (svc: AutoModeService, mockFn: ReturnType) => { + (svc as any).updateFeatureStatus = mockFn; + }; + + it('should do nothing when no features are running', async () => { + const updateMock = vi.fn().mockResolvedValue(undefined); + mockUpdateFeatureStatus(service, updateMock); + + await service.markAllRunningFeaturesInterrupted(); + + expect(updateMock).not.toHaveBeenCalled(); + }); + + it('should mark a single running feature as interrupted', async () => { + const runningFeaturesMap = getRunningFeaturesMap(service); + runningFeaturesMap.set('feature-1', { + featureId: 'feature-1', + projectPath: '/project/path', + isAutoMode: true, + }); + + const updateMock = vi.fn().mockResolvedValue(undefined); + mockUpdateFeatureStatus(service, updateMock); + + await service.markAllRunningFeaturesInterrupted(); + + expect(updateMock).toHaveBeenCalledWith('/project/path', 'feature-1', 'interrupted'); + }); + + it('should mark multiple running features as interrupted', async () => { + const runningFeaturesMap = getRunningFeaturesMap(service); + runningFeaturesMap.set('feature-1', { + featureId: 'feature-1', + projectPath: '/project-a', + isAutoMode: true, + }); + runningFeaturesMap.set('feature-2', { + featureId: 'feature-2', + projectPath: '/project-b', + isAutoMode: false, + }); + runningFeaturesMap.set('feature-3', { + featureId: 'feature-3', + projectPath: '/project-a', + isAutoMode: true, + }); + + const updateMock = vi.fn().mockResolvedValue(undefined); + mockUpdateFeatureStatus(service, updateMock); + + await service.markAllRunningFeaturesInterrupted(); + + expect(updateMock).toHaveBeenCalledTimes(3); + expect(updateMock).toHaveBeenCalledWith('/project-a', 'feature-1', 'interrupted'); + expect(updateMock).toHaveBeenCalledWith('/project-b', 'feature-2', 'interrupted'); + expect(updateMock).toHaveBeenCalledWith('/project-a', 'feature-3', 'interrupted'); + }); + + it('should mark features in parallel', async () => { + const runningFeaturesMap = getRunningFeaturesMap(service); + for (let i = 1; i <= 5; i++) { + runningFeaturesMap.set(`feature-${i}`, { + featureId: `feature-${i}`, + projectPath: `/project-${i}`, + isAutoMode: true, + }); + } + + const callOrder: string[] = []; + const updateMock = vi.fn().mockImplementation(async (_path: string, featureId: string) => { + callOrder.push(featureId); + await new Promise((resolve) => setTimeout(resolve, 10)); + }); + mockUpdateFeatureStatus(service, updateMock); + + const startTime = Date.now(); + await service.markAllRunningFeaturesInterrupted(); + const duration = Date.now() - startTime; + + expect(updateMock).toHaveBeenCalledTimes(5); + // If executed in parallel, total time should be ~10ms + // If sequential, it would be ~50ms (5 * 10ms) + expect(duration).toBeLessThan(40); + }); + + it('should continue marking other features when one fails', async () => { + const runningFeaturesMap = getRunningFeaturesMap(service); + runningFeaturesMap.set('feature-1', { + featureId: 'feature-1', + projectPath: '/project-a', + isAutoMode: true, + }); + runningFeaturesMap.set('feature-2', { + featureId: 'feature-2', + projectPath: '/project-b', + isAutoMode: false, + }); + + const updateMock = vi + .fn() + .mockResolvedValueOnce(undefined) + .mockRejectedValueOnce(new Error('Failed to update')); + mockUpdateFeatureStatus(service, updateMock); + + // Should not throw even though one feature failed + await expect(service.markAllRunningFeaturesInterrupted()).resolves.not.toThrow(); + + expect(updateMock).toHaveBeenCalledTimes(2); + }); + + it('should use provided reason in logging', async () => { + const runningFeaturesMap = getRunningFeaturesMap(service); + runningFeaturesMap.set('feature-1', { + featureId: 'feature-1', + projectPath: '/project/path', + isAutoMode: true, + }); + + const updateMock = vi.fn().mockResolvedValue(undefined); + mockUpdateFeatureStatus(service, updateMock); + + await service.markAllRunningFeaturesInterrupted('manual stop'); + + expect(updateMock).toHaveBeenCalledWith('/project/path', 'feature-1', 'interrupted'); + }); + + it('should use default reason when none provided', async () => { + const runningFeaturesMap = getRunningFeaturesMap(service); + runningFeaturesMap.set('feature-1', { + featureId: 'feature-1', + projectPath: '/project/path', + isAutoMode: true, + }); + + const updateMock = vi.fn().mockResolvedValue(undefined); + mockUpdateFeatureStatus(service, updateMock); + + await service.markAllRunningFeaturesInterrupted(); + + expect(updateMock).toHaveBeenCalledWith('/project/path', 'feature-1', 'interrupted'); + }); + }); + + describe('isFeatureRunning', () => { + // Helper to access private runningFeatures Map + const getRunningFeaturesMap = (svc: AutoModeService) => + (svc as any).runningFeatures as Map< + string, + { featureId: string; projectPath: string; isAutoMode: boolean } + >; + + it('should return false when no features are running', () => { + expect(service.isFeatureRunning('feature-123')).toBe(false); + }); + + it('should return true when the feature is running', () => { + const runningFeaturesMap = getRunningFeaturesMap(service); + runningFeaturesMap.set('feature-123', { + featureId: 'feature-123', + projectPath: '/project/path', + isAutoMode: true, + }); + + expect(service.isFeatureRunning('feature-123')).toBe(true); + }); + + it('should return false for non-running feature when others are running', () => { + const runningFeaturesMap = getRunningFeaturesMap(service); + runningFeaturesMap.set('feature-other', { + featureId: 'feature-other', + projectPath: '/project/path', + isAutoMode: true, + }); + + expect(service.isFeatureRunning('feature-123')).toBe(false); + }); + + it('should correctly track multiple running features', () => { + const runningFeaturesMap = getRunningFeaturesMap(service); + runningFeaturesMap.set('feature-1', { + featureId: 'feature-1', + projectPath: '/project-a', + isAutoMode: true, + }); + runningFeaturesMap.set('feature-2', { + featureId: 'feature-2', + projectPath: '/project-b', + isAutoMode: false, + }); + + expect(service.isFeatureRunning('feature-1')).toBe(true); + expect(service.isFeatureRunning('feature-2')).toBe(true); + expect(service.isFeatureRunning('feature-3')).toBe(false); + }); + }); }); diff --git a/libs/types/src/pipeline.ts b/libs/types/src/pipeline.ts index 23798d0b..05a4b4aa 100644 --- a/libs/types/src/pipeline.ts +++ b/libs/types/src/pipeline.ts @@ -22,6 +22,7 @@ export type PipelineStatus = `pipeline_${string}`; export type FeatureStatusWithPipeline = | 'backlog' | 'in_progress' + | 'interrupted' | 'waiting_approval' | 'verified' | 'completed' From ef779daedf185ac2b7b00b79f492b660469e9f21 Mon Sep 17 00:00:00 2001 From: Shirone Date: Sun, 25 Jan 2026 14:57:23 +0100 Subject: [PATCH 5/5] refactor: Improve error handling and status preservation in auto-mode service - Simplified the graceful shutdown process by removing redundant error handling for marking features as interrupted, as it is now managed internally. - Updated orphan detection logging to streamline the process and enhance clarity. - Added logic to preserve specific pipeline statuses when marking features as interrupted, ensuring correct resumption of features after a server restart. - Enhanced unit tests to cover new behavior for preserving pipeline statuses and handling various feature states. --- apps/server/src/index.ts | 7 +- .../server/src/routes/features/routes/list.ts | 25 ++-- apps/server/src/services/auto-mode-service.ts | 16 +++ .../unit/services/auto-mode-service.test.ts | 129 +++++++++++++++++- 4 files changed, 156 insertions(+), 21 deletions(-) diff --git a/apps/server/src/index.ts b/apps/server/src/index.ts index 06040100..ab3d60f5 100644 --- a/apps/server/src/index.ts +++ b/apps/server/src/index.ts @@ -787,11 +787,8 @@ const gracefulShutdown = async (signal: string) => { // Mark all running features as interrupted before shutdown // This ensures they can be resumed when the server restarts - try { - await autoModeService.markAllRunningFeaturesInterrupted(`${signal} signal received`); - } catch (error) { - logger.error('Failed to mark running features as interrupted:', error); - } + // Note: markAllRunningFeaturesInterrupted handles errors internally and never rejects + await autoModeService.markAllRunningFeaturesInterrupted(`${signal} signal received`); terminalService.cleanup(); server.close(() => { diff --git a/apps/server/src/routes/features/routes/list.ts b/apps/server/src/routes/features/routes/list.ts index 7920db73..40c35966 100644 --- a/apps/server/src/routes/features/routes/list.ts +++ b/apps/server/src/routes/features/routes/list.ts @@ -28,25 +28,20 @@ export function createListHandler(featureLoader: FeatureLoader, autoModeService? // Run orphan detection in background when project is loaded // This detects features whose branches no longer exist (e.g., after merge/delete) // We don't await this to keep the list response fast + // Note: detectOrphanedFeatures handles errors internally and always resolves if (autoModeService) { - autoModeService - .detectOrphanedFeatures(projectPath) - .then((orphanedFeatures) => { - if (orphanedFeatures.length > 0) { + autoModeService.detectOrphanedFeatures(projectPath).then((orphanedFeatures) => { + if (orphanedFeatures.length > 0) { + logger.info( + `[ProjectLoad] Detected ${orphanedFeatures.length} orphaned feature(s) in ${projectPath}` + ); + for (const { feature, missingBranch } of orphanedFeatures) { logger.info( - `[ProjectLoad] Detected ${orphanedFeatures.length} orphaned feature(s) in ${projectPath}` + `[ProjectLoad] Orphaned: ${feature.title || feature.id} - branch "${missingBranch}" no longer exists` ); - for (const { feature, missingBranch } of orphanedFeatures) { - logger.info( - `[ProjectLoad] Orphaned: ${feature.title || feature.id} - branch "${missingBranch}" no longer exists` - ); - } } - }) - .catch((error: unknown) => { - const errorMessage = error instanceof Error ? error.message : String(error); - logger.warn(`[ProjectLoad] Failed to detect orphaned features: ${errorMessage}`); - }); + } + }); } res.json({ success: true, features }); diff --git a/apps/server/src/services/auto-mode-service.ts b/apps/server/src/services/auto-mode-service.ts index 64ab7ee6..d6aa180b 100644 --- a/apps/server/src/services/auto-mode-service.ts +++ b/apps/server/src/services/auto-mode-service.ts @@ -3095,6 +3095,10 @@ Format your response as a structured markdown document.`; * restart, process crash, or manual stop). Features with this status can be * resumed later using the resume functionality. * + * Note: Features with pipeline_* statuses are preserved rather than overwritten + * to 'interrupted'. This ensures that resumePipelineFeature() can pick up from + * the correct pipeline step after a restart. + * * @param projectPath - Path to the project * @param featureId - ID of the feature to mark as interrupted * @param reason - Optional reason for the interruption (logged for debugging) @@ -3104,6 +3108,18 @@ Format your response as a structured markdown document.`; featureId: string, reason?: string ): Promise { + // Load the feature to check its current status + const feature = await this.loadFeature(projectPath, featureId); + const currentStatus = feature?.status; + + // Preserve pipeline_* statuses so resumePipelineFeature can resume from the correct step + if (currentStatus && currentStatus.startsWith('pipeline_')) { + logger.info( + `Feature ${featureId} was in ${currentStatus}; preserving pipeline status for resume` + ); + return; + } + if (reason) { logger.info(`Marking feature ${featureId} as interrupted: ${reason}`); } else { diff --git a/apps/server/tests/unit/services/auto-mode-service.test.ts b/apps/server/tests/unit/services/auto-mode-service.test.ts index 1de26bae..7f3f9af0 100644 --- a/apps/server/tests/unit/services/auto-mode-service.test.ts +++ b/apps/server/tests/unit/services/auto-mode-service.test.ts @@ -483,8 +483,15 @@ describe('auto-mode-service.ts', () => { (svc as any).updateFeatureStatus = mockFn; }; - it('should call updateFeatureStatus with interrupted status', async () => { + // Helper to mock loadFeature + const mockLoadFeature = (svc: AutoModeService, mockFn: ReturnType) => { + (svc as any).loadFeature = mockFn; + }; + + it('should call updateFeatureStatus with interrupted status for non-pipeline features', async () => { + const loadMock = vi.fn().mockResolvedValue({ id: 'feature-123', status: 'in_progress' }); const updateMock = vi.fn().mockResolvedValue(undefined); + mockLoadFeature(service, loadMock); mockUpdateFeatureStatus(service, updateMock); await service.markFeatureInterrupted('/test/project', 'feature-123'); @@ -493,7 +500,9 @@ describe('auto-mode-service.ts', () => { }); it('should call updateFeatureStatus with reason when provided', async () => { + const loadMock = vi.fn().mockResolvedValue({ id: 'feature-123', status: 'in_progress' }); const updateMock = vi.fn().mockResolvedValue(undefined); + mockLoadFeature(service, loadMock); mockUpdateFeatureStatus(service, updateMock); await service.markFeatureInterrupted('/test/project', 'feature-123', 'server shutdown'); @@ -502,13 +511,73 @@ describe('auto-mode-service.ts', () => { }); it('should propagate errors from updateFeatureStatus', async () => { + const loadMock = vi.fn().mockResolvedValue({ id: 'feature-123', status: 'in_progress' }); const updateMock = vi.fn().mockRejectedValue(new Error('Update failed')); + mockLoadFeature(service, loadMock); mockUpdateFeatureStatus(service, updateMock); await expect(service.markFeatureInterrupted('/test/project', 'feature-123')).rejects.toThrow( 'Update failed' ); }); + + it('should preserve pipeline_implementation status instead of marking as interrupted', async () => { + const loadMock = vi + .fn() + .mockResolvedValue({ id: 'feature-123', status: 'pipeline_implementation' }); + const updateMock = vi.fn().mockResolvedValue(undefined); + mockLoadFeature(service, loadMock); + mockUpdateFeatureStatus(service, updateMock); + + await service.markFeatureInterrupted('/test/project', 'feature-123', 'server shutdown'); + + // updateFeatureStatus should NOT be called for pipeline statuses + expect(updateMock).not.toHaveBeenCalled(); + }); + + it('should preserve pipeline_testing status instead of marking as interrupted', async () => { + const loadMock = vi.fn().mockResolvedValue({ id: 'feature-123', status: 'pipeline_testing' }); + const updateMock = vi.fn().mockResolvedValue(undefined); + mockLoadFeature(service, loadMock); + mockUpdateFeatureStatus(service, updateMock); + + await service.markFeatureInterrupted('/test/project', 'feature-123'); + + expect(updateMock).not.toHaveBeenCalled(); + }); + + it('should preserve pipeline_review status instead of marking as interrupted', async () => { + const loadMock = vi.fn().mockResolvedValue({ id: 'feature-123', status: 'pipeline_review' }); + const updateMock = vi.fn().mockResolvedValue(undefined); + mockLoadFeature(service, loadMock); + mockUpdateFeatureStatus(service, updateMock); + + await service.markFeatureInterrupted('/test/project', 'feature-123'); + + expect(updateMock).not.toHaveBeenCalled(); + }); + + it('should mark feature as interrupted when loadFeature returns null', async () => { + const loadMock = vi.fn().mockResolvedValue(null); + const updateMock = vi.fn().mockResolvedValue(undefined); + mockLoadFeature(service, loadMock); + mockUpdateFeatureStatus(service, updateMock); + + await service.markFeatureInterrupted('/test/project', 'feature-123'); + + expect(updateMock).toHaveBeenCalledWith('/test/project', 'feature-123', 'interrupted'); + }); + + it('should mark feature as interrupted for pending status', async () => { + const loadMock = vi.fn().mockResolvedValue({ id: 'feature-123', status: 'pending' }); + const updateMock = vi.fn().mockResolvedValue(undefined); + mockLoadFeature(service, loadMock); + mockUpdateFeatureStatus(service, updateMock); + + await service.markFeatureInterrupted('/test/project', 'feature-123'); + + expect(updateMock).toHaveBeenCalledWith('/test/project', 'feature-123', 'interrupted'); + }); }); describe('markAllRunningFeaturesInterrupted', () => { @@ -524,6 +593,11 @@ describe('auto-mode-service.ts', () => { (svc as any).updateFeatureStatus = mockFn; }; + // Helper to mock loadFeature + const mockLoadFeature = (svc: AutoModeService, mockFn: ReturnType) => { + (svc as any).loadFeature = mockFn; + }; + it('should do nothing when no features are running', async () => { const updateMock = vi.fn().mockResolvedValue(undefined); mockUpdateFeatureStatus(service, updateMock); @@ -541,7 +615,9 @@ describe('auto-mode-service.ts', () => { isAutoMode: true, }); + const loadMock = vi.fn().mockResolvedValue({ id: 'feature-1', status: 'in_progress' }); const updateMock = vi.fn().mockResolvedValue(undefined); + mockLoadFeature(service, loadMock); mockUpdateFeatureStatus(service, updateMock); await service.markAllRunningFeaturesInterrupted(); @@ -567,7 +643,9 @@ describe('auto-mode-service.ts', () => { isAutoMode: true, }); + const loadMock = vi.fn().mockResolvedValue({ status: 'in_progress' }); const updateMock = vi.fn().mockResolvedValue(undefined); + mockLoadFeature(service, loadMock); mockUpdateFeatureStatus(service, updateMock); await service.markAllRunningFeaturesInterrupted(); @@ -588,11 +666,13 @@ describe('auto-mode-service.ts', () => { }); } + const loadMock = vi.fn().mockResolvedValue({ status: 'in_progress' }); const callOrder: string[] = []; const updateMock = vi.fn().mockImplementation(async (_path: string, featureId: string) => { callOrder.push(featureId); await new Promise((resolve) => setTimeout(resolve, 10)); }); + mockLoadFeature(service, loadMock); mockUpdateFeatureStatus(service, updateMock); const startTime = Date.now(); @@ -618,10 +698,12 @@ describe('auto-mode-service.ts', () => { isAutoMode: false, }); + const loadMock = vi.fn().mockResolvedValue({ status: 'in_progress' }); const updateMock = vi .fn() .mockResolvedValueOnce(undefined) .mockRejectedValueOnce(new Error('Failed to update')); + mockLoadFeature(service, loadMock); mockUpdateFeatureStatus(service, updateMock); // Should not throw even though one feature failed @@ -638,7 +720,9 @@ describe('auto-mode-service.ts', () => { isAutoMode: true, }); + const loadMock = vi.fn().mockResolvedValue({ id: 'feature-1', status: 'in_progress' }); const updateMock = vi.fn().mockResolvedValue(undefined); + mockLoadFeature(service, loadMock); mockUpdateFeatureStatus(service, updateMock); await service.markAllRunningFeaturesInterrupted('manual stop'); @@ -654,13 +738,56 @@ describe('auto-mode-service.ts', () => { isAutoMode: true, }); + const loadMock = vi.fn().mockResolvedValue({ id: 'feature-1', status: 'in_progress' }); const updateMock = vi.fn().mockResolvedValue(undefined); + mockLoadFeature(service, loadMock); mockUpdateFeatureStatus(service, updateMock); await service.markAllRunningFeaturesInterrupted(); expect(updateMock).toHaveBeenCalledWith('/project/path', 'feature-1', 'interrupted'); }); + + it('should preserve pipeline statuses for running features', async () => { + const runningFeaturesMap = getRunningFeaturesMap(service); + runningFeaturesMap.set('feature-1', { + featureId: 'feature-1', + projectPath: '/project-a', + isAutoMode: true, + }); + runningFeaturesMap.set('feature-2', { + featureId: 'feature-2', + projectPath: '/project-b', + isAutoMode: false, + }); + runningFeaturesMap.set('feature-3', { + featureId: 'feature-3', + projectPath: '/project-c', + isAutoMode: true, + }); + + // feature-1 has in_progress (should be interrupted) + // feature-2 has pipeline_testing (should be preserved) + // feature-3 has pipeline_implementation (should be preserved) + const loadMock = vi + .fn() + .mockImplementation(async (_projectPath: string, featureId: string) => { + if (featureId === 'feature-1') return { id: 'feature-1', status: 'in_progress' }; + if (featureId === 'feature-2') return { id: 'feature-2', status: 'pipeline_testing' }; + if (featureId === 'feature-3') + return { id: 'feature-3', status: 'pipeline_implementation' }; + return null; + }); + const updateMock = vi.fn().mockResolvedValue(undefined); + mockLoadFeature(service, loadMock); + mockUpdateFeatureStatus(service, updateMock); + + await service.markAllRunningFeaturesInterrupted(); + + // Only feature-1 should be marked as interrupted + expect(updateMock).toHaveBeenCalledTimes(1); + expect(updateMock).toHaveBeenCalledWith('/project-a', 'feature-1', 'interrupted'); + }); }); describe('isFeatureRunning', () => {