mirror of
https://github.com/AutoMaker-Org/automaker.git
synced 2026-03-20 11:03:08 +00:00
Feature: worktree view customization and stability fixes (#805)
* Changes from feature/worktree-view-customization * Feature: Git sync, set-tracking, and push divergence handling (#796) * 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. * Changes from feature/worktree-view-customization * refactor: Remove unused worktree swap and highlight props * refactor: Consolidate feature completion logic and improve thinking level defaults * feat: Increase max turn limit to 10000 - Update DEFAULT_MAX_TURNS from 1000 to 10000 in settings-helpers.ts and agent-executor.ts - Update MAX_ALLOWED_TURNS from 2000 to 10000 in settings-helpers.ts - Update UI clamping logic from 2000 to 10000 in app-store.ts - Update fallback values from 1000 to 10000 in use-settings-sync.ts - Update default value from 1000 to 10000 in DEFAULT_GLOBAL_SETTINGS - Update documentation to reflect new range: 1-10000 Allows agents to perform up to 10000 turns for complex feature execution. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * feat: Add model resolution, improve session handling, and enhance UI stability * refactor: Remove unused sync and tracking branch props from worktree components * feat: Add PR number update functionality to worktrees. Address pr feedback * feat: Optimize Gemini CLI startup and add tool result tracking * refactor: Improve error handling and simplify worktree task cleanup --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -188,6 +188,125 @@ describe('agent-service.ts', () => {
|
||||
expect(mockEvents.emit).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should emit tool_result events from provider stream', async () => {
|
||||
const mockProvider = {
|
||||
getName: () => 'gemini',
|
||||
executeQuery: async function* () {
|
||||
yield {
|
||||
type: 'assistant',
|
||||
message: {
|
||||
role: 'assistant',
|
||||
content: [
|
||||
{
|
||||
type: 'tool_use',
|
||||
name: 'Read',
|
||||
tool_use_id: 'tool-1',
|
||||
input: { file_path: 'README.md' },
|
||||
},
|
||||
],
|
||||
},
|
||||
};
|
||||
yield {
|
||||
type: 'assistant',
|
||||
message: {
|
||||
role: 'assistant',
|
||||
content: [
|
||||
{
|
||||
type: 'tool_result',
|
||||
tool_use_id: 'tool-1',
|
||||
content: 'File contents here',
|
||||
},
|
||||
],
|
||||
},
|
||||
};
|
||||
yield {
|
||||
type: 'result',
|
||||
subtype: 'success',
|
||||
};
|
||||
},
|
||||
};
|
||||
|
||||
vi.mocked(ProviderFactory.getProviderForModel).mockReturnValue(mockProvider as any);
|
||||
|
||||
vi.mocked(promptBuilder.buildPromptWithImages).mockResolvedValue({
|
||||
content: 'Hello',
|
||||
hasImages: false,
|
||||
});
|
||||
|
||||
await service.sendMessage({
|
||||
sessionId: 'session-1',
|
||||
message: 'Hello',
|
||||
});
|
||||
|
||||
expect(mockEvents.emit).toHaveBeenCalledWith(
|
||||
'agent:stream',
|
||||
expect.objectContaining({
|
||||
sessionId: 'session-1',
|
||||
type: 'tool_result',
|
||||
tool: {
|
||||
name: 'Read',
|
||||
input: {
|
||||
toolUseId: 'tool-1',
|
||||
content: 'File contents here',
|
||||
},
|
||||
},
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
it('should emit tool_result with unknown tool name for unregistered tool_use_id', async () => {
|
||||
const mockProvider = {
|
||||
getName: () => 'gemini',
|
||||
executeQuery: async function* () {
|
||||
// Yield tool_result WITHOUT a preceding tool_use (unregistered tool_use_id)
|
||||
yield {
|
||||
type: 'assistant',
|
||||
message: {
|
||||
role: 'assistant',
|
||||
content: [
|
||||
{
|
||||
type: 'tool_result',
|
||||
tool_use_id: 'unregistered-id',
|
||||
content: 'Some result content',
|
||||
},
|
||||
],
|
||||
},
|
||||
};
|
||||
yield {
|
||||
type: 'result',
|
||||
subtype: 'success',
|
||||
};
|
||||
},
|
||||
};
|
||||
|
||||
vi.mocked(ProviderFactory.getProviderForModel).mockReturnValue(mockProvider as any);
|
||||
|
||||
vi.mocked(promptBuilder.buildPromptWithImages).mockResolvedValue({
|
||||
content: 'Hello',
|
||||
hasImages: false,
|
||||
});
|
||||
|
||||
await service.sendMessage({
|
||||
sessionId: 'session-1',
|
||||
message: 'Hello',
|
||||
});
|
||||
|
||||
expect(mockEvents.emit).toHaveBeenCalledWith(
|
||||
'agent:stream',
|
||||
expect.objectContaining({
|
||||
sessionId: 'session-1',
|
||||
type: 'tool_result',
|
||||
tool: {
|
||||
name: 'unknown',
|
||||
input: {
|
||||
toolUseId: 'unregistered-id',
|
||||
content: 'Some result content',
|
||||
},
|
||||
},
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
it('should handle images in message', async () => {
|
||||
const mockProvider = {
|
||||
getName: () => 'claude',
|
||||
|
||||
@@ -116,6 +116,7 @@ describe('EventHookService', () => {
|
||||
|
||||
mockEmitter.simulateEvent('auto-mode:event', {
|
||||
type: 'auto_mode_feature_complete',
|
||||
executionMode: 'auto',
|
||||
featureId: 'feat-1',
|
||||
featureName: 'Test Feature',
|
||||
passes: true,
|
||||
@@ -144,6 +145,7 @@ describe('EventHookService', () => {
|
||||
|
||||
mockEmitter.simulateEvent('auto-mode:event', {
|
||||
type: 'auto_mode_feature_complete',
|
||||
executionMode: 'auto',
|
||||
featureId: 'feat-1',
|
||||
featureName: 'Test Feature',
|
||||
passes: false,
|
||||
@@ -171,6 +173,7 @@ describe('EventHookService', () => {
|
||||
|
||||
mockEmitter.simulateEvent('auto-mode:event', {
|
||||
type: 'auto_mode_feature_complete',
|
||||
executionMode: 'auto',
|
||||
featureId: 'feat-1',
|
||||
featureName: 'Test Feature',
|
||||
passes: true,
|
||||
@@ -200,6 +203,7 @@ describe('EventHookService', () => {
|
||||
|
||||
mockEmitter.simulateEvent('auto-mode:event', {
|
||||
type: 'auto_mode_feature_complete',
|
||||
executionMode: 'auto',
|
||||
featureId: 'feat-1',
|
||||
featureName: 'Test Feature',
|
||||
passes: false,
|
||||
@@ -217,6 +221,55 @@ describe('EventHookService', () => {
|
||||
// Error field should be populated for error triggers
|
||||
expect(storeCall.error).toBe('Feature stopped by user');
|
||||
});
|
||||
|
||||
it('should ignore feature complete events without explicit auto execution mode', async () => {
|
||||
service.initialize(
|
||||
mockEmitter,
|
||||
mockSettingsService,
|
||||
mockEventHistoryService,
|
||||
mockFeatureLoader
|
||||
);
|
||||
|
||||
mockEmitter.simulateEvent('auto-mode:event', {
|
||||
type: 'auto_mode_feature_complete',
|
||||
featureId: 'feat-1',
|
||||
featureName: 'Manual Feature',
|
||||
passes: true,
|
||||
message: 'Manually verified',
|
||||
projectPath: '/test/project',
|
||||
});
|
||||
|
||||
await new Promise((resolve) => setTimeout(resolve, 50));
|
||||
expect(mockEventHistoryService.storeEvent).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('event mapping - feature:completed', () => {
|
||||
it('should map manual completion to feature_success', async () => {
|
||||
service.initialize(
|
||||
mockEmitter,
|
||||
mockSettingsService,
|
||||
mockEventHistoryService,
|
||||
mockFeatureLoader
|
||||
);
|
||||
|
||||
mockEmitter.simulateEvent('feature:completed', {
|
||||
featureId: 'feat-1',
|
||||
featureName: 'Manual Feature',
|
||||
projectPath: '/test/project',
|
||||
passes: true,
|
||||
executionMode: 'manual',
|
||||
});
|
||||
|
||||
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);
|
||||
});
|
||||
});
|
||||
|
||||
describe('event mapping - auto_mode_error', () => {
|
||||
@@ -400,6 +453,7 @@ describe('EventHookService', () => {
|
||||
|
||||
mockEmitter.simulateEvent('auto-mode:event', {
|
||||
type: 'auto_mode_feature_complete',
|
||||
executionMode: 'auto',
|
||||
featureId: 'feat-1',
|
||||
featureName: 'Test Feature',
|
||||
passes: true,
|
||||
@@ -420,7 +474,6 @@ describe('EventHookService', () => {
|
||||
it('should NOT execute error hooks when feature completes successfully', async () => {
|
||||
// This is the key regression test for the bug:
|
||||
// "Error event hook fired when a feature completes successfully"
|
||||
const errorHookCommand = vi.fn();
|
||||
const hooks = [
|
||||
{
|
||||
id: 'hook-error',
|
||||
@@ -444,6 +497,7 @@ describe('EventHookService', () => {
|
||||
|
||||
mockEmitter.simulateEvent('auto-mode:event', {
|
||||
type: 'auto_mode_feature_complete',
|
||||
executionMode: 'auto',
|
||||
featureId: 'feat-1',
|
||||
featureName: 'Test Feature',
|
||||
passes: true,
|
||||
@@ -480,6 +534,7 @@ describe('EventHookService', () => {
|
||||
|
||||
mockEmitter.simulateEvent('auto-mode:event', {
|
||||
type: 'auto_mode_feature_complete',
|
||||
executionMode: 'auto',
|
||||
featureId: 'feat-1',
|
||||
passes: true,
|
||||
message: 'Done',
|
||||
@@ -507,6 +562,7 @@ describe('EventHookService', () => {
|
||||
|
||||
mockEmitter.simulateEvent('auto-mode:event', {
|
||||
type: 'auto_mode_feature_complete',
|
||||
executionMode: 'auto',
|
||||
featureId: 'feat-1',
|
||||
featureName: 'Fallback Name',
|
||||
passes: true,
|
||||
@@ -617,6 +673,7 @@ describe('EventHookService', () => {
|
||||
// First: auto_mode_feature_complete fires (auto-mode path)
|
||||
mockEmitter.simulateEvent('auto-mode:event', {
|
||||
type: 'auto_mode_feature_complete',
|
||||
executionMode: 'auto',
|
||||
featureId: 'feat-1',
|
||||
featureName: 'Auto Feature',
|
||||
passes: true,
|
||||
@@ -690,6 +747,7 @@ describe('EventHookService', () => {
|
||||
// Auto-mode completion for feat-1
|
||||
mockEmitter.simulateEvent('auto-mode:event', {
|
||||
type: 'auto_mode_feature_complete',
|
||||
executionMode: 'auto',
|
||||
featureId: 'feat-1',
|
||||
passes: true,
|
||||
message: 'Done',
|
||||
@@ -757,6 +815,7 @@ describe('EventHookService', () => {
|
||||
|
||||
mockEmitter.simulateEvent('auto-mode:event', {
|
||||
type: 'auto_mode_feature_complete',
|
||||
executionMode: 'auto',
|
||||
featureId: 'feat-1',
|
||||
passes: false,
|
||||
message: 'Feature stopped by user',
|
||||
|
||||
@@ -1269,6 +1269,34 @@ describe('execution-service.ts', () => {
|
||||
|
||||
expect(mockConcurrencyManager.release).toHaveBeenCalledWith('feature-1', { force: true });
|
||||
});
|
||||
|
||||
it('immediately updates feature status to interrupted before subprocess terminates', async () => {
|
||||
const runningFeature = createRunningFeature('feature-1');
|
||||
vi.mocked(mockConcurrencyManager.getRunningFeature).mockReturnValue(runningFeature);
|
||||
|
||||
await service.stopFeature('feature-1');
|
||||
|
||||
// Should update to 'interrupted' immediately so the UI reflects the stop
|
||||
// without waiting for the CLI subprocess to fully terminate
|
||||
expect(mockUpdateFeatureStatusFn).toHaveBeenCalledWith(
|
||||
'/test/project',
|
||||
'feature-1',
|
||||
'interrupted'
|
||||
);
|
||||
});
|
||||
|
||||
it('still aborts and releases even if status update fails', async () => {
|
||||
const runningFeature = createRunningFeature('feature-1');
|
||||
const abortSpy = vi.spyOn(runningFeature.abortController, 'abort');
|
||||
vi.mocked(mockConcurrencyManager.getRunningFeature).mockReturnValue(runningFeature);
|
||||
vi.mocked(mockUpdateFeatureStatusFn).mockRejectedValueOnce(new Error('disk error'));
|
||||
|
||||
const result = await service.stopFeature('feature-1');
|
||||
|
||||
expect(result).toBe(true);
|
||||
expect(abortSpy).toHaveBeenCalled();
|
||||
expect(mockConcurrencyManager.release).toHaveBeenCalledWith('feature-1', { force: true });
|
||||
});
|
||||
});
|
||||
|
||||
describe('worktree resolution', () => {
|
||||
|
||||
@@ -740,8 +740,11 @@ describe('settings-service.ts', () => {
|
||||
// Legacy fields should be migrated to phaseModels with canonical IDs
|
||||
expect(settings.phaseModels.enhancementModel).toEqual({ model: 'claude-haiku' });
|
||||
expect(settings.phaseModels.validationModel).toEqual({ model: 'claude-opus' });
|
||||
// Other fields should use defaults (canonical IDs)
|
||||
expect(settings.phaseModels.specGenerationModel).toEqual({ model: 'claude-opus' });
|
||||
// Other fields should use defaults (canonical IDs) - specGenerationModel includes thinkingLevel from DEFAULT_PHASE_MODELS
|
||||
expect(settings.phaseModels.specGenerationModel).toEqual({
|
||||
model: 'claude-opus',
|
||||
thinkingLevel: 'adaptive',
|
||||
});
|
||||
});
|
||||
|
||||
it('should use default phase models when none are configured', async () => {
|
||||
@@ -755,10 +758,13 @@ describe('settings-service.ts', () => {
|
||||
|
||||
const settings = await settingsService.getGlobalSettings();
|
||||
|
||||
// Should use DEFAULT_PHASE_MODELS (with canonical IDs)
|
||||
// Should use DEFAULT_PHASE_MODELS (with canonical IDs) - specGenerationModel includes thinkingLevel from DEFAULT_PHASE_MODELS
|
||||
expect(settings.phaseModels.enhancementModel).toEqual({ model: 'claude-sonnet' });
|
||||
expect(settings.phaseModels.fileDescriptionModel).toEqual({ model: 'claude-haiku' });
|
||||
expect(settings.phaseModels.specGenerationModel).toEqual({ model: 'claude-opus' });
|
||||
expect(settings.phaseModels.specGenerationModel).toEqual({
|
||||
model: 'claude-opus',
|
||||
thinkingLevel: 'adaptive',
|
||||
});
|
||||
});
|
||||
|
||||
it('should deep merge phaseModels on update', async () => {
|
||||
|
||||
Reference in New Issue
Block a user