ros/automaker
1
0
Fork 0
mirror of https://github.com/AutoMaker-Org/automaker.git synced 2026-03-17 10:03:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat: Improve callback safety and remove unnecessary formatting in auto-mode facade

Browse Source
This commit is contained in:
gsxdsm
2026-02-16 13:14:55 -08:00
parent 0b03e70f1d
commit ab5d6a0e54
7 changed files with 80 additions and 118 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
apps/server/src/services/agent-executor.ts
View File

@@ -126,9 +126,7 @@ export class AgentExecutor {
const appendRawEvent = (event: unknown): void => {
if (!enableRawOutput) return;
try {
rawOutputLines.push(
JSON.stringify({ timestamp: new Date().toISOString(), event }, null, 4)
);
rawOutputLines.push(JSON.stringify({ timestamp: new Date().toISOString(), event }));
if (rawWriteTimeout) clearTimeout(rawWriteTimeout);
rawWriteTimeout = setTimeout(async () => {
try {
@@ -552,7 +550,7 @@ export class AgentExecutor {
});
let revText = '';
for await (const msg of provider.executeQuery(
this.buildExecOpts(options, revPrompt, sdkOptions?.maxTurns || 100)
this.buildExecOpts(options, revPrompt, sdkOptions?.maxTurns ?? 100)
)) {
if (msg.type === 'assistant' && msg.message?.content)
for (const b of msg.message.content)

101
apps/server/src/services/auto-mode/facade.ts
View File

@@ -143,9 +143,20 @@ export class AutoModeServiceFacade {
return prompt;
};
// Create placeholder callbacks - will be bound to facade methods after creation
// These use closures to capture the facade instance once created
// Create placeholder callbacks - will be bound to facade methods after creation.
// These use closures to capture the facade instance once created.
// INVARIANT: All callbacks passed to PipelineOrchestrator, AutoLoopCoordinator,
// and ExecutionService are invoked asynchronously (never during construction),
// so facadeInstance is guaranteed to be assigned before any callback runs.
let facadeInstance: AutoModeServiceFacade | null = null;
const getFacade = (): AutoModeServiceFacade => {
if (!facadeInstance) {
throw new Error(
'AutoModeServiceFacade not yet initialized — callback invoked during construction'
);
}
return facadeInstance;
};
// PipelineOrchestrator - runAgentFn is a stub; routes use AutoModeService directly
const pipelineOrchestrator = new PipelineOrchestrator(
@@ -162,7 +173,7 @@ export class AutoModeServiceFacade {
loadContextFiles,
buildFeaturePrompt,
(pPath, featureId, useWorktrees, _isAutoMode, _model, opts) =>
facadeInstance!.executeFeature(featureId, useWorktrees, false, undefined, opts),
getFacade().executeFeature(featureId, useWorktrees, false, undefined, opts),
// runAgentFn - delegates to AgentExecutor
async (
workDir: string,
@@ -227,7 +238,7 @@ export class AutoModeServiceFacade {
.replace(/\{\{taskName\}\}/g, task.description)
.replace(/\{\{taskIndex\}\}/g, String(taskIndex + 1))
.replace(/\{\{totalTasks\}\}/g, String(allTasks.length))
.replace(/\{\{taskDescription\}\}/g, task.description || task.name);
.replace(/\{\{taskDescription\}\}/g, task.description || `Task ${task.id}`);
if (feedback) {
taskPrompt = taskPrompt.replace(/\{\{userFeedback\}\}/g, feedback);
}
@@ -248,7 +259,7 @@ export class AutoModeServiceFacade {
settingsService,
// Callbacks
(pPath, featureId, useWorktrees, isAutoMode) =>
facadeInstance!.executeFeature(featureId, useWorktrees, isAutoMode),
getFacade().executeFeature(featureId, useWorktrees, isAutoMode),
async (pPath, branchName) => {
const features = await featureLoader.getAll(pPath);
// For main worktree (branchName === null), resolve the actual primary branch name
@@ -266,8 +277,8 @@ export class AutoModeServiceFacade {
);
},
(pPath, branchName, maxConcurrency) =>
facadeInstance!.saveExecutionStateForProject(branchName, maxConcurrency),
(pPath, branchName) => facadeInstance!.clearExecutionState(branchName),
getFacade().saveExecutionStateForProject(branchName, maxConcurrency),
(pPath, branchName) => getFacade().clearExecutionState(branchName),
(pPath) => featureStateManager.resetStuckFeatures(pPath),
(feature) =>
feature.status === 'completed' ||
@@ -375,16 +386,16 @@ export class AutoModeServiceFacade {
async () => {
/* recordLearnings - stub */
},
(pPath, featureId) => facadeInstance!.contextExists(featureId),
(pPath, featureId) => getFacade().contextExists(featureId),
(pPath, featureId, useWorktrees, _calledInternally) =>
facadeInstance!.resumeFeature(featureId, useWorktrees, _calledInternally),
getFacade().resumeFeature(featureId, useWorktrees, _calledInternally),
(errorInfo) =>
autoLoopCoordinator.trackFailureAndCheckPauseForProject(projectPath, null, errorInfo),
(errorInfo) => autoLoopCoordinator.signalShouldPauseForProject(projectPath, null, errorInfo),
() => {
/* recordSuccess - no-op */
},
(_pPath) => facadeInstance!.saveExecutionState(),
(_pPath) => getFacade().saveExecutionState(),
loadContextFiles
);
@@ -395,13 +406,7 @@ export class AutoModeServiceFacade {
settingsService,
// Callbacks
(pPath, featureId, useWorktrees, isAutoMode, providedWorktreePath, opts) =>
facadeInstance!.executeFeature(
featureId,
useWorktrees,
isAutoMode,
providedWorktreePath,
opts
),
getFacade().executeFeature(featureId, useWorktrees, isAutoMode, providedWorktreePath, opts),
(pPath, featureId) => featureStateManager.loadFeature(pPath, featureId),
(pPath, featureId, status) =>
pipelineOrchestrator.detectPipelineStatus(pPath, featureId, status),
@@ -547,7 +552,9 @@ export class AutoModeServiceFacade {
imagePaths?: string[],
useWorktrees = true
): Promise<void> {
// This method contains substantial logic - delegates most work to AgentExecutor
// 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({
@@ -555,56 +562,6 @@ export class AutoModeServiceFacade {
projectPath: this.projectPath,
isAutoMode: false,
});
const abortController = runningEntry.abortController;
const feature = await this.featureStateManager.loadFeature(this.projectPath, featureId);
let workDir = path.resolve(this.projectPath);
let worktreePath: string | null = null;
const branchName = feature?.branchName || `feature/${featureId}`;
if (useWorktrees && branchName) {
worktreePath = await this.worktreeResolver.findWorktreeForBranch(
this.projectPath,
branchName
);
if (worktreePath) {
workDir = worktreePath;
}
}
// Load previous context
const featureDir = getFeatureDir(this.projectPath, featureId);
const contextPath = path.join(featureDir, 'agent-output.md');
let previousContext = '';
try {
previousContext = (await secureFs.readFile(contextPath, 'utf-8')) as string;
} catch {
// No previous context
}
const prompts = await getPromptCustomization(this.settingsService, '[Facade]');
// Build follow-up prompt inline (no template in TaskExecutionPrompts)
let fullPrompt = `## Follow-up on Feature Implementation
${feature ? `**Feature ID:** ${feature.id}\n**Title:** ${feature.title || 'Untitled'}\n**Description:** ${feature.description}` : `**Feature ID:** ${featureId}`}
`;
if (previousContext) {
fullPrompt += `
## Previous Agent Work
The following is the output from the previous implementation attempt:
${previousContext}
`;
}
fullPrompt += `
## Follow-up Instructions
${prompt}
## Task
Address the follow-up instructions above. Review the previous work and make the requested changes or fixes.`;
try {
// NOTE: Facade does not have runAgent - this method requires AutoModeService
@@ -617,8 +574,8 @@ Address the follow-up instructions above. Review the previous work and make the
if (!errorInfo.isAbort) {
this.eventBus.emitAutoModeEvent('auto_mode_error', {
featureId,
featureName: feature?.title,
branchName: feature?.branchName ?? null,
featureName: undefined,
branchName: null,
error: errorInfo.message,
errorType: errorInfo.type,
projectPath: this.projectPath,
@@ -854,7 +811,9 @@ Address the follow-up instructions above. Review the previous work and make the
async checkWorktreeCapacity(featureId: string): Promise<WorktreeCapacityInfo> {
const feature = await this.featureStateManager.loadFeature(this.projectPath, featureId);
const rawBranchName = feature?.branchName ?? null;
const branchName = rawBranchName === 'main' ? null : rawBranchName;
// Normalize primary branch to null (works for main, master, or any default branch)
const primaryBranch = await this.worktreeResolver.getCurrentBranch(this.projectPath);
const branchName = rawBranchName === primaryBranch ? null : rawBranchName;
const maxAgents = await this.autoLoopCoordinator.resolveMaxConcurrency(
this.projectPath,

46
apps/server/src/services/feature-state-manager.ts
View File

@@ -123,23 +123,28 @@ export class FeatureStateManager {
});
// Create notifications for important status changes
const notificationService = getNotificationService();
if (status === 'waiting_approval') {
await notificationService.createNotification({
type: 'feature_waiting_approval',
title: 'Feature Ready for Review',
message: `"${feature.name || featureId}" is ready for your review and approval.`,
featureId,
projectPath,
});
} else if (status === 'verified') {
await notificationService.createNotification({
type: 'feature_verified',
title: 'Feature Verified',
message: `"${feature.name || featureId}" has been verified and is complete.`,
featureId,
projectPath,
});
// Wrapped in try-catch so failures don't block syncFeatureToAppSpec below
try {
const notificationService = getNotificationService();
if (status === 'waiting_approval') {
await notificationService.createNotification({
type: 'feature_waiting_approval',
title: 'Feature Ready for Review',
message: `"${feature.name || featureId}" is ready for your review and approval.`,
featureId,
projectPath,
});
} else if (status === 'verified') {
await notificationService.createNotification({
type: 'feature_verified',
title: 'Feature Verified',
message: `"${feature.name || featureId}" has been verified and is complete.`,
featureId,
projectPath,
});
}
} catch (notificationError) {
logger.warn(`Failed to create notification for feature ${featureId}:`, notificationError);
}
// Sync completed/verified features to app_spec.txt
@@ -334,7 +339,7 @@ export class FeatureStateManager {
Object.assign(feature.planSpec, updates);
// If content is being updated and it's different from old content, increment version
if (updates.content && updates.content !== oldContent) {
if (updates.content !== undefined && updates.content !== oldContent) {
feature.planSpec.version = (feature.planSpec.version || 0) + 1;
}
@@ -446,6 +451,11 @@ export class FeatureStateManager {
status,
tasks: feature.planSpec.tasks,
});
} else {
const availableIds = feature.planSpec.tasks.map((t) => t.id).join(', ');
logger.warn(
`[updateTaskStatus] Task ${taskId} not found in feature ${featureId} (${projectPath}). Available task IDs: [${availableIds}]`
);
}
} catch (error) {
logger.error(`Failed to update task ${taskId} status for ${featureId}:`, error);

23
apps/server/src/services/pipeline-orchestrator.ts
View File

@@ -362,7 +362,7 @@ export class PipelineOrchestrator {
await this.executePipeline(context);
// Re-fetch feature to check if executePipeline set a terminal status (e.g., merge_conflict)
const reloadedFeature = await this.featureLoader.getById(projectPath, featureId);
const reloadedFeature = await this.featureStateManager.loadFeature(projectPath, featureId);
const finalStatus = feature.skipTests ? 'waiting_approval' : 'verified';
// Only update status if not already in a terminal state
@@ -516,7 +516,7 @@ export class PipelineOrchestrator {
projectPath,
branchName,
worktreePath || projectPath,
targetBranch,
targetBranch || 'main',
{
deleteWorktreeAndBranch: false,
}
@@ -562,22 +562,33 @@ export class PipelineOrchestrator {
let passCount = 0;
let failCount = 0;
let inFailureContext = false;
for (const line of lines) {
const trimmed = line.trim();
if (trimmed.includes('FAIL') || trimmed.includes('FAILED')) {
const match = trimmed.match(/(?:FAIL|FAILED)\s+(.+)/);
if (match) failedTests.push(match[1].trim());
failCount++;
inFailureContext = true;
} else if (trimmed.includes('PASS') || trimmed.includes('PASSED')) {
passCount++;
inFailureContext = false;
}
if (trimmed.match(/^>\s+.*\.(test|spec)\./)) {
failedTests.push(trimmed.replace(/^>\s+/, ''));
}
if (
trimmed.includes('AssertionError') ||
trimmed.includes('toBe') ||
trimmed.includes('toEqual')
// Only capture assertion details when they appear in failure context
// or match explicit assertion error / expect patterns
if (trimmed.includes('AssertionError') || trimmed.includes('AssertionError')) {
failedTests.push(trimmed);
} else if (
inFailureContext &&
/expect\(.+\)\.(toBe|toEqual|toMatch|toThrow|toContain)\s*\(/.test(trimmed)
) {
failedTests.push(trimmed);
} else if (
inFailureContext &&
(trimmed.startsWith('Expected') || trimmed.startsWith('Received'))
) {
failedTests.push(trimmed);
}

1
apps/ui/eslint.config.mjs
View File

@@ -96,6 +96,7 @@ const eslintConfig = defineConfig([
setInterval: 'readonly',
clearTimeout: 'readonly',
clearInterval: 'readonly',
queueMicrotask: 'readonly',
// Node.js (for scripts and Electron)
process: 'readonly',
require: 'readonly',

17
apps/ui/src/store/app-store.ts
View File

@@ -7,7 +7,6 @@ import { createLogger } from '@automaker/utils/logger';
// Note: setItem/getItem moved to ./utils/theme-utils.ts
import { UI_SANS_FONT_OPTIONS, UI_MONO_FONT_OPTIONS } from '@/config/ui-font-options';
import type {
Feature as BaseFeature,
FeatureImagePath,
FeatureTextFilePath,
ModelAlias,
@@ -15,25 +14,11 @@ import type {
ThinkingLevel,
ReasoningEffort,
ModelProvider,
CursorModelId,
CodexModelId,
OpencodeModelId,
GeminiModelId,
CopilotModelId,
PhaseModelConfig,
PhaseModelKey,
PhaseModelEntry,
MCPServerConfig,
FeatureStatusWithPipeline,
PipelineConfig,
PipelineStep,
PromptCustomization,
ModelDefinition,
ServerLogLevel,
EventHook,
ClaudeApiProfile,
ClaudeCompatibleProvider,
SidebarStyle,
ParsedTask,
PlanSpec,
} from '@automaker/types';
@@ -2131,7 +2116,7 @@ export const useAppStore = create<AppState & AppActions>()((set, get) => ({
const updateSizes = (layout: TerminalPanelContent): TerminalPanelContent => {
if (layout.type === 'split') {
// Find matching panels and update sizes
const updatedPanels = layout.panels.map((panel, index) => {
const updatedPanels = layout.panels.map((panel, _index) => {
// Generate key for this panel
const panelKey =
panel.type === 'split'

4
apps/ui/src/store/types/state-types.ts
View File

@@ -2,8 +2,6 @@ import type { Project, TrashedProject } from '@/lib/electron';
import type {
ModelAlias,
PlanningMode,
ThinkingLevel,
ReasoningEffort,
ModelProvider,
CursorModelId,
CodexModelId,
@@ -33,7 +31,7 @@ import type {
BackgroundSettings,
} from './ui-types';
import type { ApiKeys } from './settings-types';
import type { ChatMessage, ChatSession, FeatureImage } from './chat-types';
import type { ChatMessage, ChatSession } from './chat-types';
import type { TerminalState, TerminalPanelContent, PersistedTerminalState } from './terminal-types';
import type { Feature, ProjectAnalysis } from './project-types';
import type { ClaudeUsage, CodexUsage } from './usage-types';