From dee770c2ab60b59ffee547d4ac1e7d20bb28b6a1 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Tue, 17 Feb 2026 10:32:20 -0800 Subject: [PATCH] refactor: Consolidate global settings fetching to avoid duplicate calls --- apps/server/src/index.ts | 67 ++-- .../src/services/feature-state-manager.ts | 363 ++++++++---------- apps/server/src/services/typed-event-bus.ts | 6 +- apps/ui/src/types/electron.d.ts | 6 +- libs/types/src/pipeline.ts | 1 + start-automaker.sh | 27 +- 6 files changed, 230 insertions(+), 240 deletions(-) diff --git a/apps/server/src/index.ts b/apps/server/src/index.ts index 54fcc247..a7ad979d 100644 --- a/apps/server/src/index.ts +++ b/apps/server/src/index.ts @@ -368,19 +368,31 @@ eventHookService.initialize(events, settingsService, eventHistoryService, featur logger.warn('Failed to check for legacy settings migration:', err); } - // Apply logging settings from saved settings + // Fetch global settings once and reuse for logging config and feature reconciliation + let globalSettings: Awaited> | null = null; try { - const settings = await settingsService.getGlobalSettings(); - if (settings.serverLogLevel && LOG_LEVEL_MAP[settings.serverLogLevel] !== undefined) { - setLogLevel(LOG_LEVEL_MAP[settings.serverLogLevel]); - logger.info(`Server log level set to: ${settings.serverLogLevel}`); - } - // Apply request logging setting (default true if not set) - const enableRequestLog = settings.enableRequestLogging ?? true; - setRequestLoggingEnabled(enableRequestLog); - logger.info(`HTTP request logging: ${enableRequestLog ? 'enabled' : 'disabled'}`); + globalSettings = await settingsService.getGlobalSettings(); } catch (err) { - logger.warn('Failed to load logging settings, using defaults'); + logger.warn('Failed to load global settings, using defaults'); + } + + // Apply logging settings from saved settings + if (globalSettings) { + try { + if ( + globalSettings.serverLogLevel && + LOG_LEVEL_MAP[globalSettings.serverLogLevel] !== undefined + ) { + setLogLevel(LOG_LEVEL_MAP[globalSettings.serverLogLevel]); + logger.info(`Server log level set to: ${globalSettings.serverLogLevel}`); + } + // Apply request logging setting (default true if not set) + const enableRequestLog = globalSettings.enableRequestLogging ?? true; + setRequestLoggingEnabled(enableRequestLog); + logger.info(`HTTP request logging: ${enableRequestLog ? 'enabled' : 'disabled'}`); + } catch (err) { + logger.warn('Failed to apply logging settings, using defaults'); + } } await agentService.initialize(); @@ -390,24 +402,25 @@ eventHookService.initialize(events, settingsService, eventHistoryService, featur // After any type of restart (clean, forced, crash), features may be stuck in // transient states (in_progress, interrupted, pipeline_*) that don't match reality. // Reconcile them back to resting states before the UI is served. - try { - const settings = await settingsService.getGlobalSettings(); - if (settings.projects && settings.projects.length > 0) { - let totalReconciled = 0; - for (const project of settings.projects) { - const count = await autoModeService.reconcileFeatureStates(project.path); - totalReconciled += count; - } - if (totalReconciled > 0) { - logger.info( - `[STARTUP] Reconciled ${totalReconciled} feature(s) across ${settings.projects.length} project(s)` - ); - } else { - logger.info('[STARTUP] Feature state reconciliation complete - no stale states found'); + if (globalSettings) { + try { + if (globalSettings.projects && globalSettings.projects.length > 0) { + let totalReconciled = 0; + for (const project of globalSettings.projects) { + const count = await autoModeService.reconcileFeatureStates(project.path); + totalReconciled += count; + } + if (totalReconciled > 0) { + logger.info( + `[STARTUP] Reconciled ${totalReconciled} feature(s) across ${globalSettings.projects.length} project(s)` + ); + } else { + logger.info('[STARTUP] Feature state reconciliation complete - no stale states found'); + } } + } catch (err) { + logger.warn('[STARTUP] Failed to reconcile feature states:', err); } - } catch (err) { - logger.warn('[STARTUP] Failed to reconcile feature states:', err); } // Bootstrap Codex model cache in background (don't block server startup) diff --git a/apps/server/src/services/feature-state-manager.ts b/apps/server/src/services/feature-state-manager.ts index 4ccc5e5c..1f8a4952 100644 --- a/apps/server/src/services/feature-state-manager.ts +++ b/apps/server/src/services/feature-state-manager.ts @@ -25,6 +25,7 @@ import { import { getFeatureDir, getFeaturesDir } from '@automaker/platform'; import * as secureFs from '../lib/secure-fs.js'; import type { EventEmitter } from '../lib/events.js'; +import type { AutoModeEventType } from './typed-event-bus.js'; import { getNotificationService } from './notification-service.js'; import { FeatureLoader } from './feature-loader.js'; @@ -267,6 +268,127 @@ export class FeatureStateManager { await this.updateFeatureStatus(projectPath, featureId, 'interrupted'); } + /** + * Shared helper that scans features in a project directory and resets any stuck + * in transient states (in_progress, interrupted, pipeline_*) back to resting states. + * + * Also resets: + * - generating planSpec status back to pending + * - in_progress tasks back to pending + * + * @param projectPath - The project path to scan + * @param callerLabel - Label for log messages (e.g., 'resetStuckFeatures', 'reconcileAllFeatureStates') + * @returns Object with reconciledFeatures (id + status info), reconciledCount, and scanned count + */ + private async scanAndResetFeatures( + projectPath: string, + callerLabel: string + ): Promise<{ + reconciledFeatures: Array<{ + id: string; + previousStatus: string | undefined; + newStatus: string | undefined; + }>; + reconciledFeatureIds: string[]; + reconciledCount: number; + scanned: number; + }> { + const featuresDir = getFeaturesDir(projectPath); + let scanned = 0; + let reconciledCount = 0; + const reconciledFeatureIds: string[] = []; + const reconciledFeatures: Array<{ + id: string; + previousStatus: string | undefined; + newStatus: string | undefined; + }> = []; + + try { + const entries = await secureFs.readdir(featuresDir, { withFileTypes: true }); + + for (const entry of entries) { + if (!entry.isDirectory()) continue; + + scanned++; + const featurePath = path.join(featuresDir, entry.name, 'feature.json'); + const result = await readJsonWithRecovery(featurePath, null, { + maxBackups: DEFAULT_BACKUP_COUNT, + autoRestore: true, + }); + + const feature = result.data; + if (!feature) continue; + + let needsUpdate = false; + const originalStatus = feature.status; + + // Reset features in active execution states back to a resting state + // After a server restart, no processes are actually running + const isActiveState = + originalStatus === 'in_progress' || + originalStatus === 'interrupted' || + (originalStatus != null && originalStatus.startsWith('pipeline_')); + + if (isActiveState) { + const hasApprovedPlan = feature.planSpec?.status === 'approved'; + feature.status = hasApprovedPlan ? 'ready' : 'backlog'; + needsUpdate = true; + logger.info( + `[${callerLabel}] Reset feature ${feature.id} from ${originalStatus} to ${feature.status}` + ); + } + + // Reset generating planSpec status back to pending (spec generation was interrupted) + if (feature.planSpec?.status === 'generating') { + feature.planSpec.status = 'pending'; + needsUpdate = true; + logger.info( + `[${callerLabel}] Reset feature ${feature.id} planSpec status from generating to pending` + ); + } + + // Reset any in_progress tasks back to pending (task execution was interrupted) + if (feature.planSpec?.tasks) { + for (const task of feature.planSpec.tasks) { + if (task.status === 'in_progress') { + task.status = 'pending'; + needsUpdate = true; + logger.info( + `[${callerLabel}] Reset task ${task.id} for feature ${feature.id} from in_progress to pending` + ); + // Clear currentTaskId if it points to this reverted task + if (feature.planSpec?.currentTaskId === task.id) { + feature.planSpec.currentTaskId = undefined; + logger.info( + `[${callerLabel}] Cleared planSpec.currentTaskId for feature ${feature.id} (was pointing to reverted task ${task.id})` + ); + } + } + } + } + + if (needsUpdate) { + feature.updatedAt = new Date().toISOString(); + await atomicWriteJson(featurePath, feature, { backupCount: DEFAULT_BACKUP_COUNT }); + reconciledCount++; + reconciledFeatureIds.push(feature.id); + reconciledFeatures.push({ + id: feature.id, + previousStatus: originalStatus, + newStatus: feature.status, + }); + } + } + } catch (error) { + // If features directory doesn't exist, that's fine + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { + logger.error(`[${callerLabel}] Error resetting features for ${projectPath}:`, error); + } + } + + return { reconciledFeatures, reconciledFeatureIds, reconciledCount, scanned }; + } + /** * Reset features that were stuck in transient states due to server crash. * Called when auto mode is enabled to clean up from previous session. @@ -281,108 +403,14 @@ export class FeatureStateManager { * @param projectPath - The project path to reset features for */ async resetStuckFeatures(projectPath: string): Promise { - const featuresDir = getFeaturesDir(projectPath); - let featuresScanned = 0; - let featuresReset = 0; + const { reconciledCount, scanned } = await this.scanAndResetFeatures( + projectPath, + 'resetStuckFeatures' + ); - try { - const entries = await secureFs.readdir(featuresDir, { withFileTypes: true }); - - for (const entry of entries) { - if (!entry.isDirectory()) continue; - - featuresScanned++; - const featurePath = path.join(featuresDir, entry.name, 'feature.json'); - const result = await readJsonWithRecovery(featurePath, null, { - maxBackups: DEFAULT_BACKUP_COUNT, - autoRestore: true, - }); - - const feature = result.data; - if (!feature) continue; - - let needsUpdate = false; - const originalStatus = feature.status; - - // Reset in_progress features back to ready/backlog - if (feature.status === 'in_progress') { - const hasApprovedPlan = feature.planSpec?.status === 'approved'; - feature.status = hasApprovedPlan ? 'ready' : 'backlog'; - needsUpdate = true; - logger.info( - `[resetStuckFeatures] Reset feature ${feature.id} from in_progress to ${feature.status}` - ); - } - - // Reset interrupted features back to ready/backlog - // These were marked interrupted during graceful shutdown but need to be reset - // so they appear in the correct column and can be re-executed - if (feature.status === 'interrupted') { - const hasApprovedPlan = feature.planSpec?.status === 'approved'; - feature.status = hasApprovedPlan ? 'ready' : 'backlog'; - needsUpdate = true; - logger.info( - `[resetStuckFeatures] Reset feature ${feature.id} from interrupted to ${feature.status}` - ); - } - - // Reset pipeline_* features back to ready/backlog - // After a server restart, pipeline execution cannot resume from the exact step, - // so these need to be reset to a clean state for re-execution - if (feature.status && feature.status.startsWith('pipeline_')) { - const hasApprovedPlan = feature.planSpec?.status === 'approved'; - feature.status = hasApprovedPlan ? 'ready' : 'backlog'; - needsUpdate = true; - logger.info( - `[resetStuckFeatures] Reset feature ${feature.id} from ${originalStatus} to ${feature.status}` - ); - } - - // Reset generating planSpec status back to pending (spec generation was interrupted) - if (feature.planSpec?.status === 'generating') { - feature.planSpec.status = 'pending'; - needsUpdate = true; - logger.info( - `[resetStuckFeatures] Reset feature ${feature.id} planSpec status from generating to pending` - ); - } - - // Reset any in_progress tasks back to pending (task execution was interrupted) - if (feature.planSpec?.tasks) { - for (const task of feature.planSpec.tasks) { - if (task.status === 'in_progress') { - task.status = 'pending'; - needsUpdate = true; - logger.info( - `[resetStuckFeatures] Reset task ${task.id} for feature ${feature.id} from in_progress to pending` - ); - // Clear currentTaskId if it points to this reverted task - if (feature.planSpec?.currentTaskId === task.id) { - feature.planSpec.currentTaskId = undefined; - logger.info( - `[resetStuckFeatures] Cleared planSpec.currentTaskId for feature ${feature.id} (was pointing to reverted task ${task.id})` - ); - } - } - } - } - - if (needsUpdate) { - feature.updatedAt = new Date().toISOString(); - await atomicWriteJson(featurePath, feature, { backupCount: DEFAULT_BACKUP_COUNT }); - featuresReset++; - } - } - - logger.info( - `[resetStuckFeatures] Scanned ${featuresScanned} features, reset ${featuresReset} features for ${projectPath}` - ); - } catch (error) { - // If features directory doesn't exist, that's fine - if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { - logger.error(`[resetStuckFeatures] Error resetting features for ${projectPath}:`, error); - } - } + logger.info( + `[resetStuckFeatures] Scanned ${scanned} features, reset ${reconciledCount} features for ${projectPath}` + ); } /** @@ -401,112 +429,35 @@ export class FeatureStateManager { async reconcileAllFeatureStates(projectPath: string): Promise { logger.info(`[reconcileAllFeatureStates] Starting reconciliation for ${projectPath}`); - const featuresDir = getFeaturesDir(projectPath); - let featuresScanned = 0; - let featuresReconciled = 0; - const reconciledFeatureIds: string[] = []; + const { reconciledFeatures, reconciledFeatureIds, reconciledCount, scanned } = + await this.scanAndResetFeatures(projectPath, 'reconcileAllFeatureStates'); - try { - const entries = await secureFs.readdir(featuresDir, { withFileTypes: true }); - - for (const entry of entries) { - if (!entry.isDirectory()) continue; - - featuresScanned++; - const featurePath = path.join(featuresDir, entry.name, 'feature.json'); - const result = await readJsonWithRecovery(featurePath, null, { - maxBackups: DEFAULT_BACKUP_COUNT, - autoRestore: true, - }); - - const feature = result.data; - if (!feature) continue; - - let needsUpdate = false; - const originalStatus = feature.status; - - // Reset features in active execution states back to a resting state - // After a server restart, no processes are actually running - const isActiveState = - feature.status === 'in_progress' || - feature.status === 'interrupted' || - (feature.status && feature.status.startsWith('pipeline_')); - - if (isActiveState) { - const hasApprovedPlan = feature.planSpec?.status === 'approved'; - feature.status = hasApprovedPlan ? 'ready' : 'backlog'; - needsUpdate = true; - logger.info( - `[reconcileAllFeatureStates] Reset feature ${feature.id} from ${originalStatus} to ${feature.status}` - ); - } - - // Reset generating planSpec status back to pending - if (feature.planSpec?.status === 'generating') { - feature.planSpec.status = 'pending'; - needsUpdate = true; - logger.info( - `[reconcileAllFeatureStates] Reset feature ${feature.id} planSpec from generating to pending` - ); - } - - // Reset any in_progress tasks back to pending - if (feature.planSpec?.tasks) { - for (const task of feature.planSpec.tasks) { - if (task.status === 'in_progress') { - task.status = 'pending'; - needsUpdate = true; - logger.info( - `[reconcileAllFeatureStates] Reset task ${task.id} for feature ${feature.id} from in_progress to pending` - ); - if (feature.planSpec?.currentTaskId === task.id) { - feature.planSpec.currentTaskId = undefined; - } - } - } - } - - if (needsUpdate) { - feature.updatedAt = new Date().toISOString(); - await atomicWriteJson(featurePath, feature, { backupCount: DEFAULT_BACKUP_COUNT }); - featuresReconciled++; - reconciledFeatureIds.push(feature.id); - - // Emit per-feature status change event so UI invalidates its cache - this.emitAutoModeEvent('feature_status_changed', { - featureId: feature.id, - projectPath, - status: feature.status, - previousStatus: originalStatus, - reason: 'server_restart_reconciliation', - }); - } - } - - // Emit a bulk reconciliation event for the UI - if (featuresReconciled > 0) { - this.emitAutoModeEvent('features_reconciled', { - projectPath, - reconciledCount: featuresReconciled, - reconciledFeatureIds, - message: `Reconciled ${featuresReconciled} feature(s) after server restart`, - }); - } - - logger.info( - `[reconcileAllFeatureStates] Scanned ${featuresScanned} features, reconciled ${featuresReconciled} for ${projectPath}` - ); - - return featuresReconciled; - } catch (error) { - if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { - logger.error( - `[reconcileAllFeatureStates] Error reconciling features for ${projectPath}:`, - error - ); - } - return 0; + // Emit per-feature status change events so UI invalidates its cache + for (const { id, previousStatus, newStatus } of reconciledFeatures) { + this.emitAutoModeEvent('feature_status_changed', { + featureId: id, + projectPath, + status: newStatus, + previousStatus, + reason: 'server_restart_reconciliation', + }); } + + // Emit a bulk reconciliation event for the UI + if (reconciledCount > 0) { + this.emitAutoModeEvent('features_reconciled', { + projectPath, + reconciledCount, + reconciledFeatureIds, + message: `Reconciled ${reconciledCount} feature(s) after server restart`, + }); + } + + logger.info( + `[reconcileAllFeatureStates] Scanned ${scanned} features, reconciled ${reconciledCount} for ${projectPath}` + ); + + return reconciledCount; } /** @@ -683,7 +634,7 @@ export class FeatureStateManager { * @param eventType - The event type (e.g., 'auto_mode_summary') * @param data - The event payload */ - private emitAutoModeEvent(eventType: string, data: Record): void { + private emitAutoModeEvent(eventType: AutoModeEventType, data: Record): void { // Wrap the event in auto-mode:event format expected by the client this.events.emit('auto-mode:event', { type: eventType, diff --git a/apps/server/src/services/typed-event-bus.ts b/apps/server/src/services/typed-event-bus.ts index 42600db5..09d1e9bc 100644 --- a/apps/server/src/services/typed-event-bus.ts +++ b/apps/server/src/services/typed-event-bus.ts @@ -40,11 +40,13 @@ export type AutoModeEventType = | 'plan_rejected' | 'plan_revision_requested' | 'plan_revision_warning' + | 'plan_spec_updated' | 'pipeline_step_started' | 'pipeline_step_complete' + | 'pipeline_test_failed' + | 'pipeline_merge_conflict' | 'feature_status_changed' - | 'features_reconciled' - | string; // Allow other strings for extensibility + | 'features_reconciled'; /** * TypedEventBus wraps an EventEmitter to provide type-safe event emission diff --git a/apps/ui/src/types/electron.d.ts b/apps/ui/src/types/electron.d.ts index e44f19dd..82ab237a 100644 --- a/apps/ui/src/types/electron.d.ts +++ b/apps/ui/src/types/electron.d.ts @@ -3,7 +3,7 @@ */ import type { ClaudeUsageResponse, CodexUsageResponse } from '@/store/app-store'; -import type { ParsedTask } from '@automaker/types'; +import type { ParsedTask, FeatureStatusWithPipeline } from '@automaker/types'; export interface ImageAttachment { id?: string; // Optional - may not be present in messages loaded from server @@ -364,8 +364,8 @@ export type AutoModeEvent = type: 'feature_status_changed'; featureId: string; projectPath?: string; - status: string; - previousStatus: string; + status: FeatureStatusWithPipeline; + previousStatus: FeatureStatusWithPipeline; reason?: string; } | { diff --git a/libs/types/src/pipeline.ts b/libs/types/src/pipeline.ts index 05a4b4aa..7190abbd 100644 --- a/libs/types/src/pipeline.ts +++ b/libs/types/src/pipeline.ts @@ -21,6 +21,7 @@ export type PipelineStatus = `pipeline_${string}`; export type FeatureStatusWithPipeline = | 'backlog' + | 'ready' | 'in_progress' | 'interrupted' | 'waiting_approval' diff --git a/start-automaker.sh b/start-automaker.sh index 6770db2c..497ad305 100755 --- a/start-automaker.sh +++ b/start-automaker.sh @@ -36,8 +36,24 @@ elif [[ "$OSTYPE" == "darwin"* ]]; then fi # Port configuration -DEFAULT_WEB_PORT=3007 -DEFAULT_SERVER_PORT=3008 +# Defaults can be overridden via AUTOMAKER_WEB_PORT and AUTOMAKER_SERVER_PORT env vars + +# Validate env-provided ports early (before colors are available) +if [ -n "$AUTOMAKER_WEB_PORT" ]; then + if ! [[ "$AUTOMAKER_WEB_PORT" =~ ^[0-9]+$ ]] || [ "$AUTOMAKER_WEB_PORT" -lt 1 ] || [ "$AUTOMAKER_WEB_PORT" -gt 65535 ]; then + echo "Error: AUTOMAKER_WEB_PORT must be a number between 1-65535, got '$AUTOMAKER_WEB_PORT'" + exit 1 + fi +fi +if [ -n "$AUTOMAKER_SERVER_PORT" ]; then + if ! [[ "$AUTOMAKER_SERVER_PORT" =~ ^[0-9]+$ ]] || [ "$AUTOMAKER_SERVER_PORT" -lt 1 ] || [ "$AUTOMAKER_SERVER_PORT" -gt 65535 ]; then + echo "Error: AUTOMAKER_SERVER_PORT must be a number between 1-65535, got '$AUTOMAKER_SERVER_PORT'" + exit 1 + fi +fi + +DEFAULT_WEB_PORT=${AUTOMAKER_WEB_PORT:-3007} +DEFAULT_SERVER_PORT=${AUTOMAKER_SERVER_PORT:-3008} PORT_SEARCH_MAX_ATTEMPTS=100 WEB_PORT=$DEFAULT_WEB_PORT SERVER_PORT=$DEFAULT_SERVER_PORT @@ -136,6 +152,9 @@ EXAMPLES: start-automaker.sh docker # Launch Docker dev container start-automaker.sh --version # Show version + AUTOMAKER_WEB_PORT=4000 AUTOMAKER_SERVER_PORT=4001 start-automaker.sh web + # Launch web mode on custom ports + KEYBOARD SHORTCUTS (in menu): Up/Down arrows Navigate between options Enter Select highlighted option @@ -146,6 +165,10 @@ HISTORY: Your last selected mode is remembered in: ~/.automaker_launcher_history Use --no-history to disable this feature +ENVIRONMENT VARIABLES: + AUTOMAKER_WEB_PORT Override default web/UI port (default: 3007) + AUTOMAKER_SERVER_PORT Override default API server port (default: 3008) + PLATFORMS: Linux, macOS, Windows (Git Bash, WSL, MSYS2, Cygwin)