Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion apps/server/src/services/auto-mode/compat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand All @@ -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();
Expand All @@ -37,6 +39,7 @@ export class AutoModeServiceCompat {
settingsService,
featureLoader,
sharedServices,
claudeUsageService: claudeUsageService ?? null,
};
}

Expand Down
155 changes: 122 additions & 33 deletions apps/server/src/services/auto-mode/facade.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -447,19 +487,29 @@ 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;
}
}

/**
* Stop the auto mode loop for this project/worktree
* @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;
}
}

/**
Expand Down Expand Up @@ -500,24 +550,34 @@ 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;
}
}

/**
* Stop a specific feature
* @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;
}
}

/**
Expand Down Expand Up @@ -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'
);
}
Comment on lines +646 to +657
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Two concerns with imagePaths persistence.

  1. p.split('/').pop() will fail on Windows paths (backslash separators). Use path.basename(p) instead — path is already imported at line 14.

  2. Calling updateFeatureStatus with the existing status solely to persist imagePaths is a side-effect-heavy workaround: it will overwrite updatedAt, clear justFinishedAt, and emit a feature_status_changed event. The image paths are set on the in-memory feature object, but updateFeatureStatus re-reads from disk, so these imagePaths will be lost — the method loads its own copy of the feature from disk and never sees the mutation at line 647.

🐛 Fix: persist imagePaths via a dedicated write or updateFeaturePlanSpec

The current approach mutates the local feature object's imagePaths but then calls updateFeatureStatus which re-reads the feature from disk — so the in-memory imagePaths are never persisted. You need to either:

  • Write the feature directly with atomicWriteJson after updating imagePaths, or
  • Pass imagePaths through the options parameter to executeFeature so it can attach them at execution time.
-      // 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'
-        );
-      }

Depending on how executeFeature / ExecutionService consumes image paths, either pass them via options or persist with a dedicated helper.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/server/src/services/auto-mode/facade.ts` around lines 646 - 657, The
code currently sets feature.imagePaths using p.split('/') which breaks on
Windows and then calls this.featureStateManager.updateFeatureStatus which
re-reads the feature from disk so the mutated in-memory imagePaths are never
persisted and causes unwanted side effects; replace p.split('/').pop() with
path.basename(p) and persist the new imagePaths explicitly instead of calling
updateFeatureStatus: after setting feature.imagePaths call the existing
atomicWriteJson helper (or whichever project-level atomic write utility is used)
to write the feature JSON file for this.projectPath/featureId so the imagePaths
are saved without changing timestamps or emitting status events, or
alternatively pass imagePaths through the executeFeature/options path (e.g., via
the ExecutionService/executeFeature options) if that flow is already supported.


// 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) {
Expand All @@ -582,8 +673,6 @@ export class AutoModeServiceFacade {
});
}
throw error;
} finally {
this.concurrencyManager.release(featureId);
}
}

Expand Down
1 change: 1 addition & 0 deletions apps/server/src/services/auto-mode/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export type {
WorktreeCapacityInfo,
RunningAgentInfo,
OrphanedFeatureInfo,
FacadeError,
GlobalAutoModeOperations,
} from './types.js';

Expand Down
20 changes: 20 additions & 0 deletions apps/server/src/services/auto-mode/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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.
Expand Down
37 changes: 31 additions & 6 deletions apps/server/src/services/claude-usage-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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');
Expand All @@ -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;
Expand Down Expand Up @@ -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, '')
Expand Down
Loading