mirror of
https://github.com/AutoMaker-Org/automaker.git
synced 2026-03-18 22:33:08 +00:00
Add quick-add feature with improved workflows (#802)
* Changes from feature/quick-add * feat: Clarify system prompt and improve error handling across services. Address PR Feedback * feat: Improve PR description parsing and refactor event handling * feat: Add context options to pipeline orchestrator initialization * fix: Deduplicate React and handle CJS interop for use-sync-external-store Resolve "Cannot read properties of null (reading 'useState')" errors by deduplicating React/react-dom and ensuring use-sync-external-store is bundled together with React to prevent CJS packages from resolving to different React instances.
This commit is contained in:
218
apps/server/tests/unit/routes/backlog-plan/generate-plan.test.ts
Normal file
218
apps/server/tests/unit/routes/backlog-plan/generate-plan.test.ts
Normal file
@@ -0,0 +1,218 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import type { BacklogPlanResult, ProviderMessage } from '@automaker/types';
|
||||
|
||||
const {
|
||||
mockGetAll,
|
||||
mockExecuteQuery,
|
||||
mockSaveBacklogPlan,
|
||||
mockSetRunningState,
|
||||
mockSetRunningDetails,
|
||||
mockGetPromptCustomization,
|
||||
mockGetAutoLoadClaudeMdSetting,
|
||||
mockGetUseClaudeCodeSystemPromptSetting,
|
||||
} = vi.hoisted(() => ({
|
||||
mockGetAll: vi.fn(),
|
||||
mockExecuteQuery: vi.fn(),
|
||||
mockSaveBacklogPlan: vi.fn(),
|
||||
mockSetRunningState: vi.fn(),
|
||||
mockSetRunningDetails: vi.fn(),
|
||||
mockGetPromptCustomization: vi.fn(),
|
||||
mockGetAutoLoadClaudeMdSetting: vi.fn(),
|
||||
mockGetUseClaudeCodeSystemPromptSetting: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('@/services/feature-loader.js', () => ({
|
||||
FeatureLoader: class {
|
||||
getAll = mockGetAll;
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock('@/providers/provider-factory.js', () => ({
|
||||
ProviderFactory: {
|
||||
getProviderForModel: vi.fn(() => ({
|
||||
executeQuery: mockExecuteQuery,
|
||||
})),
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock('@/routes/backlog-plan/common.js', () => ({
|
||||
logger: {
|
||||
debug: vi.fn(),
|
||||
info: vi.fn(),
|
||||
warn: vi.fn(),
|
||||
error: vi.fn(),
|
||||
},
|
||||
setRunningState: mockSetRunningState,
|
||||
setRunningDetails: mockSetRunningDetails,
|
||||
getErrorMessage: (error: unknown) => (error instanceof Error ? error.message : String(error)),
|
||||
saveBacklogPlan: mockSaveBacklogPlan,
|
||||
}));
|
||||
|
||||
vi.mock('@/lib/settings-helpers.js', () => ({
|
||||
getPromptCustomization: mockGetPromptCustomization,
|
||||
getAutoLoadClaudeMdSetting: mockGetAutoLoadClaudeMdSetting,
|
||||
getUseClaudeCodeSystemPromptSetting: mockGetUseClaudeCodeSystemPromptSetting,
|
||||
getPhaseModelWithOverrides: vi.fn(),
|
||||
}));
|
||||
|
||||
import { generateBacklogPlan } from '@/routes/backlog-plan/generate-plan.js';
|
||||
|
||||
function createMockEvents() {
|
||||
return {
|
||||
emit: vi.fn(),
|
||||
};
|
||||
}
|
||||
|
||||
describe('generateBacklogPlan', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
|
||||
mockGetAll.mockResolvedValue([]);
|
||||
mockGetPromptCustomization.mockResolvedValue({
|
||||
backlogPlan: {
|
||||
systemPrompt: 'System instructions',
|
||||
userPromptTemplate:
|
||||
'Current features:\n{{currentFeatures}}\n\nUser request:\n{{userRequest}}',
|
||||
},
|
||||
});
|
||||
mockGetAutoLoadClaudeMdSetting.mockResolvedValue(false);
|
||||
mockGetUseClaudeCodeSystemPromptSetting.mockResolvedValue(true);
|
||||
});
|
||||
|
||||
it('salvages valid streamed JSON when Claude process exits with code 1', async () => {
|
||||
const partialResult: BacklogPlanResult = {
|
||||
changes: [
|
||||
{
|
||||
type: 'add',
|
||||
feature: {
|
||||
title: 'Add signup form',
|
||||
description: 'Create signup UI and validation',
|
||||
category: 'frontend',
|
||||
},
|
||||
reason: 'Required for user onboarding',
|
||||
},
|
||||
],
|
||||
summary: 'Adds signup feature to the backlog',
|
||||
dependencyUpdates: [],
|
||||
};
|
||||
|
||||
const responseJson = JSON.stringify(partialResult);
|
||||
|
||||
async function* streamWithExitError(): AsyncGenerator<ProviderMessage> {
|
||||
yield {
|
||||
type: 'assistant',
|
||||
message: {
|
||||
role: 'assistant',
|
||||
content: [{ type: 'text', text: responseJson }],
|
||||
},
|
||||
};
|
||||
throw new Error('Claude Code process exited with code 1');
|
||||
}
|
||||
|
||||
mockExecuteQuery.mockReturnValueOnce(streamWithExitError());
|
||||
|
||||
const events = createMockEvents();
|
||||
const abortController = new AbortController();
|
||||
|
||||
const result = await generateBacklogPlan(
|
||||
'/tmp/project',
|
||||
'Please add a signup feature',
|
||||
events as any,
|
||||
abortController,
|
||||
undefined,
|
||||
'claude-opus'
|
||||
);
|
||||
|
||||
expect(mockExecuteQuery).toHaveBeenCalledTimes(1);
|
||||
expect(result).toEqual(partialResult);
|
||||
expect(mockSaveBacklogPlan).toHaveBeenCalledWith(
|
||||
'/tmp/project',
|
||||
expect.objectContaining({
|
||||
prompt: 'Please add a signup feature',
|
||||
model: 'claude-opus-4-6',
|
||||
result: partialResult,
|
||||
})
|
||||
);
|
||||
expect(events.emit).toHaveBeenCalledWith('backlog-plan:event', {
|
||||
type: 'backlog_plan_complete',
|
||||
result: partialResult,
|
||||
});
|
||||
expect(mockSetRunningState).toHaveBeenCalledWith(false, null);
|
||||
expect(mockSetRunningDetails).toHaveBeenCalledWith(null);
|
||||
});
|
||||
|
||||
it('prefers parseable provider result over longer non-JSON accumulated text on exit', async () => {
|
||||
const recoveredResult: BacklogPlanResult = {
|
||||
changes: [
|
||||
{
|
||||
type: 'add',
|
||||
feature: {
|
||||
title: 'Add reset password flow',
|
||||
description: 'Implement reset password request and token validation UI',
|
||||
category: 'frontend',
|
||||
},
|
||||
reason: 'Supports account recovery',
|
||||
},
|
||||
],
|
||||
summary: 'Adds password reset capability',
|
||||
dependencyUpdates: [],
|
||||
};
|
||||
|
||||
const validProviderResult = JSON.stringify(recoveredResult);
|
||||
const invalidAccumulatedText = `${validProviderResult}\n\nAdditional commentary that breaks raw JSON parsing.`;
|
||||
|
||||
async function* streamWithResultThenExit(): AsyncGenerator<ProviderMessage> {
|
||||
yield {
|
||||
type: 'assistant',
|
||||
message: {
|
||||
role: 'assistant',
|
||||
content: [{ type: 'text', text: invalidAccumulatedText }],
|
||||
},
|
||||
};
|
||||
yield {
|
||||
type: 'result',
|
||||
subtype: 'success',
|
||||
duration_ms: 10,
|
||||
duration_api_ms: 10,
|
||||
is_error: false,
|
||||
num_turns: 1,
|
||||
result: validProviderResult,
|
||||
session_id: 'session-1',
|
||||
total_cost_usd: 0,
|
||||
usage: {
|
||||
input_tokens: 10,
|
||||
cache_creation_input_tokens: 0,
|
||||
cache_read_input_tokens: 0,
|
||||
output_tokens: 10,
|
||||
server_tool_use: {
|
||||
web_search_requests: 0,
|
||||
},
|
||||
service_tier: 'standard',
|
||||
},
|
||||
};
|
||||
throw new Error('Claude Code process exited with code 1');
|
||||
}
|
||||
|
||||
mockExecuteQuery.mockReturnValueOnce(streamWithResultThenExit());
|
||||
|
||||
const events = createMockEvents();
|
||||
const abortController = new AbortController();
|
||||
|
||||
const result = await generateBacklogPlan(
|
||||
'/tmp/project',
|
||||
'Add password reset support',
|
||||
events as any,
|
||||
abortController,
|
||||
undefined,
|
||||
'claude-opus'
|
||||
);
|
||||
|
||||
expect(result).toEqual(recoveredResult);
|
||||
expect(mockSaveBacklogPlan).toHaveBeenCalledWith(
|
||||
'/tmp/project',
|
||||
expect.objectContaining({
|
||||
result: recoveredResult,
|
||||
})
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -524,6 +524,202 @@ describe('EventHookService', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('event mapping - feature_status_changed (non-auto-mode completion)', () => {
|
||||
it('should trigger feature_success when status changes to verified', async () => {
|
||||
mockFeatureLoader = createMockFeatureLoader({
|
||||
'feat-1': { title: 'Manual Feature' },
|
||||
});
|
||||
|
||||
service.initialize(
|
||||
mockEmitter,
|
||||
mockSettingsService,
|
||||
mockEventHistoryService,
|
||||
mockFeatureLoader
|
||||
);
|
||||
|
||||
mockEmitter.simulateEvent('auto-mode:event', {
|
||||
type: 'feature_status_changed',
|
||||
featureId: 'feat-1',
|
||||
projectPath: '/test/project',
|
||||
status: 'verified',
|
||||
});
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(mockEventHistoryService.storeEvent).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
const storeCall = (mockEventHistoryService.storeEvent as ReturnType<typeof vi.fn>).mock
|
||||
.calls[0][0];
|
||||
expect(storeCall.trigger).toBe('feature_success');
|
||||
expect(storeCall.featureName).toBe('Manual Feature');
|
||||
expect(storeCall.passes).toBe(true);
|
||||
});
|
||||
|
||||
it('should trigger feature_success when status changes to waiting_approval', async () => {
|
||||
mockFeatureLoader = createMockFeatureLoader({
|
||||
'feat-1': { title: 'Manual Feature' },
|
||||
});
|
||||
|
||||
service.initialize(
|
||||
mockEmitter,
|
||||
mockSettingsService,
|
||||
mockEventHistoryService,
|
||||
mockFeatureLoader
|
||||
);
|
||||
|
||||
mockEmitter.simulateEvent('auto-mode:event', {
|
||||
type: 'feature_status_changed',
|
||||
featureId: 'feat-1',
|
||||
projectPath: '/test/project',
|
||||
status: 'waiting_approval',
|
||||
});
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(mockEventHistoryService.storeEvent).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
const storeCall = (mockEventHistoryService.storeEvent as ReturnType<typeof vi.fn>).mock
|
||||
.calls[0][0];
|
||||
expect(storeCall.trigger).toBe('feature_success');
|
||||
expect(storeCall.passes).toBe(true);
|
||||
expect(storeCall.featureName).toBe('Manual Feature');
|
||||
});
|
||||
|
||||
it('should NOT trigger hooks for non-completion status changes', async () => {
|
||||
service.initialize(
|
||||
mockEmitter,
|
||||
mockSettingsService,
|
||||
mockEventHistoryService,
|
||||
mockFeatureLoader
|
||||
);
|
||||
|
||||
mockEmitter.simulateEvent('auto-mode:event', {
|
||||
type: 'feature_status_changed',
|
||||
featureId: 'feat-1',
|
||||
projectPath: '/test/project',
|
||||
status: 'in_progress',
|
||||
});
|
||||
|
||||
// Give it time to process
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
|
||||
expect(mockEventHistoryService.storeEvent).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should NOT double-fire hooks when auto_mode_feature_complete already fired', async () => {
|
||||
service.initialize(
|
||||
mockEmitter,
|
||||
mockSettingsService,
|
||||
mockEventHistoryService,
|
||||
mockFeatureLoader
|
||||
);
|
||||
|
||||
// First: auto_mode_feature_complete fires (auto-mode path)
|
||||
mockEmitter.simulateEvent('auto-mode:event', {
|
||||
type: 'auto_mode_feature_complete',
|
||||
featureId: 'feat-1',
|
||||
featureName: 'Auto Feature',
|
||||
passes: true,
|
||||
message: 'Feature completed',
|
||||
projectPath: '/test/project',
|
||||
});
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(mockEventHistoryService.storeEvent).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
// Then: feature_status_changed fires for the same feature
|
||||
mockEmitter.simulateEvent('auto-mode:event', {
|
||||
type: 'feature_status_changed',
|
||||
featureId: 'feat-1',
|
||||
projectPath: '/test/project',
|
||||
status: 'verified',
|
||||
});
|
||||
|
||||
// Give it time to process
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
|
||||
// Should still only have been called once (from auto_mode_feature_complete)
|
||||
expect(mockEventHistoryService.storeEvent).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('should NOT double-fire hooks when auto_mode_error already fired for feature', async () => {
|
||||
service.initialize(
|
||||
mockEmitter,
|
||||
mockSettingsService,
|
||||
mockEventHistoryService,
|
||||
mockFeatureLoader
|
||||
);
|
||||
|
||||
// First: auto_mode_error fires for a feature
|
||||
mockEmitter.simulateEvent('auto-mode:event', {
|
||||
type: 'auto_mode_error',
|
||||
featureId: 'feat-1',
|
||||
error: 'Something failed',
|
||||
errorType: 'execution',
|
||||
projectPath: '/test/project',
|
||||
});
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(mockEventHistoryService.storeEvent).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
// Then: feature_status_changed fires for the same feature (e.g., reset to backlog)
|
||||
mockEmitter.simulateEvent('auto-mode:event', {
|
||||
type: 'feature_status_changed',
|
||||
featureId: 'feat-1',
|
||||
projectPath: '/test/project',
|
||||
status: 'verified', // unlikely after error, but tests the dedup
|
||||
});
|
||||
|
||||
// Give it time to process
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
|
||||
// Should still only have been called once
|
||||
expect(mockEventHistoryService.storeEvent).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('should fire hooks for different features independently', async () => {
|
||||
service.initialize(
|
||||
mockEmitter,
|
||||
mockSettingsService,
|
||||
mockEventHistoryService,
|
||||
mockFeatureLoader
|
||||
);
|
||||
|
||||
// Auto-mode completion for feat-1
|
||||
mockEmitter.simulateEvent('auto-mode:event', {
|
||||
type: 'auto_mode_feature_complete',
|
||||
featureId: 'feat-1',
|
||||
passes: true,
|
||||
message: 'Done',
|
||||
projectPath: '/test/project',
|
||||
});
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(mockEventHistoryService.storeEvent).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
// Manual completion for feat-2 (different feature)
|
||||
mockEmitter.simulateEvent('auto-mode:event', {
|
||||
type: 'feature_status_changed',
|
||||
featureId: 'feat-2',
|
||||
projectPath: '/test/project',
|
||||
status: 'verified',
|
||||
});
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(mockEventHistoryService.storeEvent).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
// feat-2 should have triggered feature_success
|
||||
const secondCall = (mockEventHistoryService.storeEvent as ReturnType<typeof vi.fn>).mock
|
||||
.calls[1][0];
|
||||
expect(secondCall.trigger).toBe('feature_success');
|
||||
expect(secondCall.featureId).toBe('feat-2');
|
||||
});
|
||||
});
|
||||
|
||||
describe('error context for error events', () => {
|
||||
it('should use payload.error when available for error triggers', async () => {
|
||||
service.initialize(
|
||||
|
||||
@@ -34,6 +34,7 @@ import { getFeatureDir } from '@automaker/platform';
|
||||
import {
|
||||
getPromptCustomization,
|
||||
getAutoLoadClaudeMdSetting,
|
||||
getUseClaudeCodeSystemPromptSetting,
|
||||
filterClaudeMdFromContext,
|
||||
} from '../../../src/lib/settings-helpers.js';
|
||||
import { extractSummary } from '../../../src/services/spec-parser.js';
|
||||
@@ -67,6 +68,7 @@ vi.mock('../../../src/lib/settings-helpers.js', () => ({
|
||||
},
|
||||
}),
|
||||
getAutoLoadClaudeMdSetting: vi.fn().mockResolvedValue(true),
|
||||
getUseClaudeCodeSystemPromptSetting: vi.fn().mockResolvedValue(true),
|
||||
filterClaudeMdFromContext: vi.fn().mockReturnValue('context prompt'),
|
||||
}));
|
||||
|
||||
@@ -230,6 +232,7 @@ describe('execution-service.ts', () => {
|
||||
},
|
||||
} as Awaited<ReturnType<typeof getPromptCustomization>>);
|
||||
vi.mocked(getAutoLoadClaudeMdSetting).mockResolvedValue(true);
|
||||
vi.mocked(getUseClaudeCodeSystemPromptSetting).mockResolvedValue(true);
|
||||
vi.mocked(filterClaudeMdFromContext).mockReturnValue('context prompt');
|
||||
|
||||
// Re-setup spec-parser mock
|
||||
|
||||
@@ -57,6 +57,7 @@ vi.mock('../../../src/lib/settings-helpers.js', () => ({
|
||||
},
|
||||
}),
|
||||
getAutoLoadClaudeMdSetting: vi.fn().mockResolvedValue(true),
|
||||
getUseClaudeCodeSystemPromptSetting: vi.fn().mockResolvedValue(true),
|
||||
filterClaudeMdFromContext: vi.fn().mockReturnValue('context prompt'),
|
||||
}));
|
||||
|
||||
|
||||
Reference in New Issue
Block a user