mirror of
https://github.com/AutoMaker-Org/automaker.git
synced 2026-03-18 22:33:08 +00:00
feat: Add error handling to auto-mode facade and implement followUp feature. Fix Claude weekly usage indicator. Fix mobile card drag
This commit is contained in:
@@ -13,6 +13,7 @@ import { GlobalAutoModeService } from './global-service.js';
|
||||
import { AutoModeServiceFacade } from './facade.js';
|
||||
import type { SettingsService } from '../settings-service.js';
|
||||
import type { FeatureLoader } from '../feature-loader.js';
|
||||
import type { ClaudeUsageService } from '../claude-usage-service.js';
|
||||
import type { FacadeOptions, AutoModeStatus, RunningAgentInfo } from './types.js';
|
||||
|
||||
/**
|
||||
@@ -27,7 +28,8 @@ export class AutoModeServiceCompat {
|
||||
constructor(
|
||||
events: EventEmitter,
|
||||
settingsService: SettingsService | null,
|
||||
featureLoader: FeatureLoader
|
||||
featureLoader: FeatureLoader,
|
||||
claudeUsageService?: ClaudeUsageService | null
|
||||
) {
|
||||
this.globalService = new GlobalAutoModeService(events, settingsService, featureLoader);
|
||||
const sharedServices = this.globalService.getSharedServices();
|
||||
@@ -37,6 +39,7 @@ export class AutoModeServiceCompat {
|
||||
settingsService,
|
||||
featureLoader,
|
||||
sharedServices,
|
||||
claudeUsageService: claudeUsageService ?? null,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -38,6 +38,7 @@ import type { SettingsService } from '../settings-service.js';
|
||||
import type { EventEmitter } from '../../lib/events.js';
|
||||
import type {
|
||||
FacadeOptions,
|
||||
FacadeError,
|
||||
AutoModeStatus,
|
||||
ProjectAutoModeStatus,
|
||||
WorktreeCapacityInfo,
|
||||
@@ -89,6 +90,45 @@ export class AutoModeServiceFacade {
|
||||
private readonly settingsService: SettingsService | null
|
||||
) {}
|
||||
|
||||
/**
|
||||
* Classify and log an error at the facade boundary.
|
||||
* Emits an error event to the UI so failures are surfaced to the user.
|
||||
*
|
||||
* @param error - The caught error
|
||||
* @param method - The facade method name where the error occurred
|
||||
* @param featureId - Optional feature ID for context
|
||||
* @returns The classified FacadeError for structured consumption
|
||||
*/
|
||||
private handleFacadeError(error: unknown, method: string, featureId?: string): FacadeError {
|
||||
const errorInfo = classifyError(error);
|
||||
|
||||
// Log at the facade boundary for debugging
|
||||
logger.error(
|
||||
`[${method}] ${featureId ? `Feature ${featureId}: ` : ''}${errorInfo.message}`,
|
||||
error
|
||||
);
|
||||
|
||||
// Emit error event to UI unless it's an abort/cancellation
|
||||
if (!errorInfo.isAbort && !errorInfo.isCancellation) {
|
||||
this.eventBus.emitAutoModeEvent('auto_mode_error', {
|
||||
featureId: featureId ?? null,
|
||||
featureName: undefined,
|
||||
branchName: null,
|
||||
error: errorInfo.message,
|
||||
errorType: errorInfo.type,
|
||||
projectPath: this.projectPath,
|
||||
});
|
||||
}
|
||||
|
||||
return {
|
||||
method,
|
||||
errorType: errorInfo.type,
|
||||
message: errorInfo.message,
|
||||
featureId,
|
||||
projectPath: this.projectPath,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a new AutoModeServiceFacade instance for a specific project.
|
||||
*
|
||||
@@ -447,11 +487,16 @@ export class AutoModeServiceFacade {
|
||||
* @param maxConcurrency - Maximum concurrent features
|
||||
*/
|
||||
async startAutoLoop(branchName: string | null = null, maxConcurrency?: number): Promise<number> {
|
||||
return this.autoLoopCoordinator.startAutoLoopForProject(
|
||||
this.projectPath,
|
||||
branchName,
|
||||
maxConcurrency
|
||||
);
|
||||
try {
|
||||
return await this.autoLoopCoordinator.startAutoLoopForProject(
|
||||
this.projectPath,
|
||||
branchName,
|
||||
maxConcurrency
|
||||
);
|
||||
} catch (error) {
|
||||
this.handleFacadeError(error, 'startAutoLoop');
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -459,7 +504,12 @@ export class AutoModeServiceFacade {
|
||||
* @param branchName - The branch name, or null for main worktree
|
||||
*/
|
||||
async stopAutoLoop(branchName: string | null = null): Promise<number> {
|
||||
return this.autoLoopCoordinator.stopAutoLoopForProject(this.projectPath, branchName);
|
||||
try {
|
||||
return await this.autoLoopCoordinator.stopAutoLoopForProject(this.projectPath, branchName);
|
||||
} catch (error) {
|
||||
this.handleFacadeError(error, 'stopAutoLoop');
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -500,14 +550,19 @@ export class AutoModeServiceFacade {
|
||||
_calledInternally?: boolean;
|
||||
}
|
||||
): Promise<void> {
|
||||
return this.executionService.executeFeature(
|
||||
this.projectPath,
|
||||
featureId,
|
||||
useWorktrees,
|
||||
isAutoMode,
|
||||
providedWorktreePath,
|
||||
options
|
||||
);
|
||||
try {
|
||||
return await this.executionService.executeFeature(
|
||||
this.projectPath,
|
||||
featureId,
|
||||
useWorktrees,
|
||||
isAutoMode,
|
||||
providedWorktreePath,
|
||||
options
|
||||
);
|
||||
} catch (error) {
|
||||
this.handleFacadeError(error, 'executeFeature', featureId);
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -515,9 +570,14 @@ export class AutoModeServiceFacade {
|
||||
* @param featureId - ID of the feature to stop
|
||||
*/
|
||||
async stopFeature(featureId: string): Promise<boolean> {
|
||||
// Cancel any pending plan approval for this feature
|
||||
this.cancelPlanApproval(featureId);
|
||||
return this.executionService.stopFeature(featureId);
|
||||
try {
|
||||
// Cancel any pending plan approval for this feature
|
||||
this.cancelPlanApproval(featureId);
|
||||
return await this.executionService.stopFeature(featureId);
|
||||
} catch (error) {
|
||||
this.handleFacadeError(error, 'stopFeature', featureId);
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -552,23 +612,54 @@ export class AutoModeServiceFacade {
|
||||
imagePaths?: string[],
|
||||
useWorktrees = true
|
||||
): Promise<void> {
|
||||
// Stub: acquire concurrency slot then immediately throw.
|
||||
// Heavy I/O (loadFeature, worktree resolution, context reading, prompt building)
|
||||
// is deferred to the real AutoModeService.followUpFeature implementation.
|
||||
validateWorkingDirectory(this.projectPath);
|
||||
|
||||
const runningEntry = this.concurrencyManager.acquire({
|
||||
featureId,
|
||||
projectPath: this.projectPath,
|
||||
isAutoMode: false,
|
||||
});
|
||||
|
||||
try {
|
||||
// NOTE: Facade does not have runAgent - this method requires AutoModeService
|
||||
// Do NOT emit start events before throwing to prevent false start events
|
||||
throw new Error(
|
||||
'followUpFeature not fully implemented in facade - use AutoModeService.followUpFeature instead'
|
||||
);
|
||||
// Load feature to build the prompt context
|
||||
const feature = await this.featureStateManager.loadFeature(this.projectPath, featureId);
|
||||
if (!feature) throw new Error(`Feature ${featureId} not found`);
|
||||
|
||||
// Read previous agent output as context
|
||||
const featureDir = getFeatureDir(this.projectPath, featureId);
|
||||
let previousContext = '';
|
||||
try {
|
||||
previousContext = (await secureFs.readFile(
|
||||
path.join(featureDir, 'agent-output.md'),
|
||||
'utf-8'
|
||||
)) as string;
|
||||
} catch {
|
||||
// No previous context available - that's OK
|
||||
}
|
||||
|
||||
// Build the feature prompt section
|
||||
const featurePrompt = `## Feature Implementation Task\n\n**Feature ID:** ${feature.id}\n**Title:** ${feature.title || 'Untitled Feature'}\n**Description:** ${feature.description}\n`;
|
||||
|
||||
// Get the follow-up prompt template and build the continuation prompt
|
||||
const prompts = await getPromptCustomization(this.settingsService, '[Facade]');
|
||||
let continuationPrompt = prompts.autoMode.followUpPromptTemplate;
|
||||
continuationPrompt = continuationPrompt
|
||||
.replace(/\{\{featurePrompt\}\}/g, featurePrompt)
|
||||
.replace(/\{\{previousContext\}\}/g, previousContext)
|
||||
.replace(/\{\{followUpInstructions\}\}/g, prompt);
|
||||
|
||||
// Store image paths on the feature so executeFeature can pick them up
|
||||
if (imagePaths && imagePaths.length > 0) {
|
||||
feature.imagePaths = imagePaths.map((p) => ({
|
||||
path: p,
|
||||
filename: p.split('/').pop() || p,
|
||||
mimeType: 'image/*',
|
||||
}));
|
||||
await this.featureStateManager.updateFeatureStatus(
|
||||
this.projectPath,
|
||||
featureId,
|
||||
feature.status || 'in_progress'
|
||||
);
|
||||
}
|
||||
|
||||
// Delegate to executeFeature with the built continuation prompt
|
||||
await this.executeFeature(featureId, useWorktrees, false, undefined, {
|
||||
continuationPrompt,
|
||||
});
|
||||
} catch (error) {
|
||||
const errorInfo = classifyError(error);
|
||||
if (!errorInfo.isAbort) {
|
||||
@@ -582,8 +673,6 @@ export class AutoModeServiceFacade {
|
||||
});
|
||||
}
|
||||
throw error;
|
||||
} finally {
|
||||
this.concurrencyManager.release(featureId);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -58,6 +58,7 @@ export type {
|
||||
WorktreeCapacityInfo,
|
||||
RunningAgentInfo,
|
||||
OrphanedFeatureInfo,
|
||||
FacadeError,
|
||||
GlobalAutoModeOperations,
|
||||
} from './types.js';
|
||||
|
||||
|
||||
@@ -15,6 +15,7 @@ import type { ConcurrencyManager } from '../concurrency-manager.js';
|
||||
import type { AutoLoopCoordinator } from '../auto-loop-coordinator.js';
|
||||
import type { WorktreeResolver } from '../worktree-resolver.js';
|
||||
import type { TypedEventBus } from '../typed-event-bus.js';
|
||||
import type { ClaudeUsageService } from '../claude-usage-service.js';
|
||||
|
||||
// Re-export types from extracted services for route consumption
|
||||
export type { AutoModeConfig, ProjectAutoLoopState } from '../auto-loop-coordinator.js';
|
||||
@@ -55,6 +56,8 @@ export interface FacadeOptions {
|
||||
featureLoader?: FeatureLoader;
|
||||
/** Shared services for state sharing across facades (optional) */
|
||||
sharedServices?: SharedServices;
|
||||
/** ClaudeUsageService for checking usage limits before picking up features (optional) */
|
||||
claudeUsageService?: ClaudeUsageService | null;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -110,6 +113,23 @@ export interface OrphanedFeatureInfo {
|
||||
missingBranch: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Structured error object returned/emitted by facade methods.
|
||||
* Provides consistent error information for callers and UI consumers.
|
||||
*/
|
||||
export interface FacadeError {
|
||||
/** The facade method where the error originated */
|
||||
method: string;
|
||||
/** Classified error type from the error handler */
|
||||
errorType: import('@automaker/types').ErrorType;
|
||||
/** Human-readable error message */
|
||||
message: string;
|
||||
/** Feature ID if the error is associated with a specific feature */
|
||||
featureId?: string;
|
||||
/** Project path where the error occurred */
|
||||
projectPath: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Interface describing global auto-mode operations (not project-specific).
|
||||
* Used by routes that need global state access.
|
||||
|
||||
@@ -294,7 +294,16 @@ export class ClaudeUsageService {
|
||||
this.killPtyProcess(ptyProcess);
|
||||
}
|
||||
// Don't fail if we have data - return it instead
|
||||
if (output.includes('Current session')) {
|
||||
// Check cleaned output since raw output has ANSI codes between words
|
||||
// eslint-disable-next-line no-control-regex
|
||||
const cleanedForCheck = output
|
||||
.replace(/\x1B\[(\d+)C/g, (_m: string, n: string) => ' '.repeat(parseInt(n, 10)))
|
||||
.replace(/\x1B\[[0-9;?]*[A-Za-z@]/g, '');
|
||||
if (
|
||||
cleanedForCheck.includes('Current session') ||
|
||||
cleanedForCheck.includes('% used') ||
|
||||
cleanedForCheck.includes('% left')
|
||||
) {
|
||||
resolve(output);
|
||||
} else if (hasSeenTrustPrompt) {
|
||||
// Trust prompt was shown but we couldn't auto-approve it
|
||||
@@ -320,8 +329,13 @@ export class ClaudeUsageService {
|
||||
output += data;
|
||||
|
||||
// Strip ANSI codes for easier matching
|
||||
// Convert cursor forward (ESC[nC) to spaces first to preserve word boundaries,
|
||||
// then strip remaining ANSI sequences. Without this, the Claude CLI TUI output
|
||||
// like "Current week (all models)" becomes "Currentweek(allmodels)".
|
||||
// eslint-disable-next-line no-control-regex
|
||||
const cleanOutput = output.replace(/\x1B\[[0-9;]*[A-Za-z]/g, '');
|
||||
const cleanOutput = output
|
||||
.replace(/\x1B\[(\d+)C/g, (_match: string, n: string) => ' '.repeat(parseInt(n, 10)))
|
||||
.replace(/\x1B\[[0-9;?]*[A-Za-z@]/g, '');
|
||||
|
||||
// Check for specific authentication/permission errors
|
||||
// Must be very specific to avoid false positives from garbled terminal encoding
|
||||
@@ -356,7 +370,8 @@ export class ClaudeUsageService {
|
||||
const hasUsageIndicators =
|
||||
cleanOutput.includes('Current session') ||
|
||||
(cleanOutput.includes('Usage') && cleanOutput.includes('% left')) ||
|
||||
// Additional patterns for winpty - look for percentage patterns
|
||||
// Look for percentage patterns - allow optional whitespace between % and left/used
|
||||
// since cursor movement codes may or may not create spaces after stripping
|
||||
/\d+%\s*(left|used|remaining)/i.test(cleanOutput) ||
|
||||
cleanOutput.includes('Resets in') ||
|
||||
cleanOutput.includes('Current week');
|
||||
@@ -382,12 +397,15 @@ export class ClaudeUsageService {
|
||||
// Handle Trust Dialog - multiple variants:
|
||||
// - "Do you want to work in this folder?"
|
||||
// - "Ready to code here?" / "I'll need permission to work with your files"
|
||||
// - "Quick safety check" / "Yes, I trust this folder"
|
||||
// Since we are running in cwd (project dir), it is safe to approve.
|
||||
if (
|
||||
!hasApprovedTrust &&
|
||||
(cleanOutput.includes('Do you want to work in this folder?') ||
|
||||
cleanOutput.includes('Ready to code here') ||
|
||||
cleanOutput.includes('permission to work with your files'))
|
||||
cleanOutput.includes('permission to work with your files') ||
|
||||
cleanOutput.includes('trust this folder') ||
|
||||
cleanOutput.includes('safety check'))
|
||||
) {
|
||||
hasApprovedTrust = true;
|
||||
hasSeenTrustPrompt = true;
|
||||
@@ -471,10 +489,17 @@ export class ClaudeUsageService {
|
||||
* Handles CSI, OSC, and other common ANSI sequences
|
||||
*/
|
||||
private stripAnsiCodes(text: string): string {
|
||||
// First strip ANSI sequences (colors, etc) and handle CR
|
||||
// First, convert cursor movement sequences to whitespace to preserve word boundaries.
|
||||
// The Claude CLI TUI uses ESC[nC (cursor forward) instead of actual spaces between words.
|
||||
// Without this, "Current week (all models)" becomes "Currentweek(allmodels)" after stripping.
|
||||
// eslint-disable-next-line no-control-regex
|
||||
let clean = text
|
||||
// CSI sequences: ESC [ ... (letter or @)
|
||||
// Cursor forward (CSI n C): replace with n spaces to preserve word separation
|
||||
.replace(/\x1B\[(\d+)C/g, (_match, n) => ' '.repeat(parseInt(n, 10)))
|
||||
// Cursor movement (up/down/back/position): replace with newline or nothing
|
||||
.replace(/\x1B\[\d*[ABD]/g, '') // cursor up (A), down (B), back (D)
|
||||
.replace(/\x1B\[\d+;\d+[Hf]/g, '\n') // cursor position (H/f)
|
||||
// Now strip remaining CSI sequences (colors, modes, etc.)
|
||||
.replace(/\x1B\[[0-9;?]*[A-Za-z@]/g, '')
|
||||
// OSC sequences: ESC ] ... terminated by BEL, ST, or another ESC
|
||||
.replace(/\x1B\][^\x07\x1B]*(?:\x07|\x1B\\)?/g, '')
|
||||
|
||||
@@ -107,6 +107,47 @@ export class FeatureStateManager {
|
||||
// Badge will show for 2 minutes after this timestamp
|
||||
if (status === 'waiting_approval') {
|
||||
feature.justFinishedAt = new Date().toISOString();
|
||||
|
||||
// Finalize task statuses when feature is done:
|
||||
// - Mark any in_progress tasks as completed (agent finished but didn't explicitly complete them)
|
||||
// - Do NOT mark pending tasks as completed (they were never started)
|
||||
// - Clear currentTaskId since no task is actively running
|
||||
// This prevents cards in "waiting for review" from appearing to still have running tasks
|
||||
if (feature.planSpec?.tasks) {
|
||||
let tasksFinalized = 0;
|
||||
for (const task of feature.planSpec.tasks) {
|
||||
if (task.status === 'in_progress') {
|
||||
task.status = 'completed';
|
||||
tasksFinalized++;
|
||||
}
|
||||
}
|
||||
if (tasksFinalized > 0) {
|
||||
logger.info(
|
||||
`[updateFeatureStatus] Finalized ${tasksFinalized} in_progress tasks for feature ${featureId} moving to waiting_approval`
|
||||
);
|
||||
}
|
||||
// Update tasksCompleted count to reflect actual completed tasks
|
||||
feature.planSpec.tasksCompleted = feature.planSpec.tasks.filter(
|
||||
(t) => t.status === 'completed'
|
||||
).length;
|
||||
feature.planSpec.currentTaskId = undefined;
|
||||
}
|
||||
} else if (status === 'verified') {
|
||||
// Also finalize in_progress tasks when moving directly to verified (skipTests=false)
|
||||
// Do NOT mark pending tasks as completed - they were never started
|
||||
if (feature.planSpec?.tasks) {
|
||||
for (const task of feature.planSpec.tasks) {
|
||||
if (task.status === 'in_progress') {
|
||||
task.status = 'completed';
|
||||
}
|
||||
}
|
||||
feature.planSpec.tasksCompleted = feature.planSpec.tasks.filter(
|
||||
(t) => t.status === 'completed'
|
||||
).length;
|
||||
feature.planSpec.currentTaskId = undefined;
|
||||
}
|
||||
// Clear the timestamp when moving to other statuses
|
||||
feature.justFinishedAt = undefined;
|
||||
} else {
|
||||
// Clear the timestamp when moving to other statuses
|
||||
feature.justFinishedAt = undefined;
|
||||
|
||||
Reference in New Issue
Block a user