Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
Copy Markdown
Contributor

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
Loading