mirror of
https://github.com/AutoMaker-Org/automaker.git
synced 2026-02-02 20:43:36 +00:00
fix: Prevent features from getting stuck in in_progress after server restart
- Add graceful shutdown handler that marks running features as 'interrupted' before server exit (SIGTERM/SIGINT) - Add 30-second shutdown timeout to prevent hanging on exit - Add orphan detection to identify features with missing branches - Add isFeatureRunning() for idempotent resume checks - Improve resumeInterruptedFeatures() to handle features without saved context - Add 'interrupted' status to FeatureStatusWithPipeline type - Replace console.log with proper logger in auto-mode-service - Add comprehensive unit tests for all new functionality (15 new tests) Fixes #696 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Shirone
@@ -315,4 +315,404 @@ describe('auto-mode-service.ts', () => {
|
||||
expect(duration).toBeLessThan(40);
|
||||
});
|
||||
});
|
||||
|
||||
describe('detectOrphanedFeatures', () => {
|
||||
// Helper to mock featureLoader.getAll
|
||||
const mockFeatureLoaderGetAll = (svc: AutoModeService, mockFn: ReturnType<typeof vi.fn>) => {
|
||||
(svc as any).featureLoader = { getAll: mockFn };
|
||||
};
|
||||
|
||||
// Helper to mock getExistingBranches
|
||||
const mockGetExistingBranches = (svc: AutoModeService, branches: string[]) => {
|
||||
(svc as any).getExistingBranches = vi.fn().mockResolvedValue(new Set(branches));
|
||||
};
|
||||
|
||||
it('should return empty array when no features have branch names', async () => {
|
||||
const getAllMock = vi.fn().mockResolvedValue([
|
||||
{ id: 'f1', title: 'Feature 1', description: 'desc', category: 'test' },
|
||||
{ id: 'f2', title: 'Feature 2', description: 'desc', category: 'test' },
|
||||
] satisfies Feature[]);
|
||||
mockFeatureLoaderGetAll(service, getAllMock);
|
||||
mockGetExistingBranches(service, ['main', 'develop']);
|
||||
|
||||
const result = await service.detectOrphanedFeatures('/test/project');
|
||||
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
|
||||
it('should return empty array when all feature branches exist', async () => {
|
||||
const getAllMock = vi.fn().mockResolvedValue([
|
||||
{
|
||||
id: 'f1',
|
||||
title: 'Feature 1',
|
||||
description: 'desc',
|
||||
category: 'test',
|
||||
branchName: 'feature-1',
|
||||
},
|
||||
{
|
||||
id: 'f2',
|
||||
title: 'Feature 2',
|
||||
description: 'desc',
|
||||
category: 'test',
|
||||
branchName: 'feature-2',
|
||||
},
|
||||
] satisfies Feature[]);
|
||||
mockFeatureLoaderGetAll(service, getAllMock);
|
||||
mockGetExistingBranches(service, ['main', 'feature-1', 'feature-2']);
|
||||
|
||||
const result = await service.detectOrphanedFeatures('/test/project');
|
||||
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
|
||||
it('should detect orphaned features with missing branches', async () => {
|
||||
const features: Feature[] = [
|
||||
{
|
||||
id: 'f1',
|
||||
title: 'Feature 1',
|
||||
description: 'desc',
|
||||
category: 'test',
|
||||
branchName: 'feature-1',
|
||||
},
|
||||
{
|
||||
id: 'f2',
|
||||
title: 'Feature 2',
|
||||
description: 'desc',
|
||||
category: 'test',
|
||||
branchName: 'deleted-branch',
|
||||
},
|
||||
{ id: 'f3', title: 'Feature 3', description: 'desc', category: 'test' }, // No branch
|
||||
];
|
||||
const getAllMock = vi.fn().mockResolvedValue(features);
|
||||
mockFeatureLoaderGetAll(service, getAllMock);
|
||||
mockGetExistingBranches(service, ['main', 'feature-1']); // deleted-branch not in list
|
||||
|
||||
const result = await service.detectOrphanedFeatures('/test/project');
|
||||
|
||||
expect(result).toHaveLength(1);
|
||||
expect(result[0].feature.id).toBe('f2');
|
||||
expect(result[0].missingBranch).toBe('deleted-branch');
|
||||
});
|
||||
|
||||
it('should detect multiple orphaned features', async () => {
|
||||
const features: Feature[] = [
|
||||
{
|
||||
id: 'f1',
|
||||
title: 'Feature 1',
|
||||
description: 'desc',
|
||||
category: 'test',
|
||||
branchName: 'orphan-1',
|
||||
},
|
||||
{
|
||||
id: 'f2',
|
||||
title: 'Feature 2',
|
||||
description: 'desc',
|
||||
category: 'test',
|
||||
branchName: 'orphan-2',
|
||||
},
|
||||
{
|
||||
id: 'f3',
|
||||
title: 'Feature 3',
|
||||
description: 'desc',
|
||||
category: 'test',
|
||||
branchName: 'valid-branch',
|
||||
},
|
||||
];
|
||||
const getAllMock = vi.fn().mockResolvedValue(features);
|
||||
mockFeatureLoaderGetAll(service, getAllMock);
|
||||
mockGetExistingBranches(service, ['main', 'valid-branch']);
|
||||
|
||||
const result = await service.detectOrphanedFeatures('/test/project');
|
||||
|
||||
expect(result).toHaveLength(2);
|
||||
expect(result.map((r) => r.feature.id)).toContain('f1');
|
||||
expect(result.map((r) => r.feature.id)).toContain('f2');
|
||||
});
|
||||
|
||||
it('should return empty array when getAll throws error', async () => {
|
||||
const getAllMock = vi.fn().mockRejectedValue(new Error('Failed to load features'));
|
||||
mockFeatureLoaderGetAll(service, getAllMock);
|
||||
|
||||
const result = await service.detectOrphanedFeatures('/test/project');
|
||||
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
|
||||
it('should ignore empty branchName strings', async () => {
|
||||
const features: Feature[] = [
|
||||
{ id: 'f1', title: 'Feature 1', description: 'desc', category: 'test', branchName: '' },
|
||||
{ id: 'f2', title: 'Feature 2', description: 'desc', category: 'test', branchName: ' ' },
|
||||
];
|
||||
const getAllMock = vi.fn().mockResolvedValue(features);
|
||||
mockFeatureLoaderGetAll(service, getAllMock);
|
||||
mockGetExistingBranches(service, ['main']);
|
||||
|
||||
const result = await service.detectOrphanedFeatures('/test/project');
|
||||
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
|
||||
it('should skip features whose branchName matches the primary branch', async () => {
|
||||
const features: Feature[] = [
|
||||
{ id: 'f1', title: 'Feature 1', description: 'desc', category: 'test', branchName: 'main' },
|
||||
{
|
||||
id: 'f2',
|
||||
title: 'Feature 2',
|
||||
description: 'desc',
|
||||
category: 'test',
|
||||
branchName: 'orphaned',
|
||||
},
|
||||
];
|
||||
const getAllMock = vi.fn().mockResolvedValue(features);
|
||||
mockFeatureLoaderGetAll(service, getAllMock);
|
||||
mockGetExistingBranches(service, ['main', 'develop']);
|
||||
// Mock getCurrentBranch to return 'main'
|
||||
(service as any).getCurrentBranch = vi.fn().mockResolvedValue('main');
|
||||
|
||||
const result = await service.detectOrphanedFeatures('/test/project');
|
||||
|
||||
// Only f2 should be orphaned (orphaned branch doesn't exist)
|
||||
expect(result).toHaveLength(1);
|
||||
expect(result[0].feature.id).toBe('f2');
|
||||
});
|
||||
});
|
||||
|
||||
describe('markFeatureInterrupted', () => {
|
||||
// Helper to mock updateFeatureStatus
|
||||
const mockUpdateFeatureStatus = (svc: AutoModeService, mockFn: ReturnType<typeof vi.fn>) => {
|
||||
(svc as any).updateFeatureStatus = mockFn;
|
||||
};
|
||||
|
||||
it('should call updateFeatureStatus with interrupted status', async () => {
|
||||
const updateMock = vi.fn().mockResolvedValue(undefined);
|
||||
mockUpdateFeatureStatus(service, updateMock);
|
||||
|
||||
await service.markFeatureInterrupted('/test/project', 'feature-123');
|
||||
|
||||
expect(updateMock).toHaveBeenCalledWith('/test/project', 'feature-123', 'interrupted');
|
||||
});
|
||||
|
||||
it('should call updateFeatureStatus with reason when provided', async () => {
|
||||
const updateMock = vi.fn().mockResolvedValue(undefined);
|
||||
mockUpdateFeatureStatus(service, updateMock);
|
||||
|
||||
await service.markFeatureInterrupted('/test/project', 'feature-123', 'server shutdown');
|
||||
|
||||
expect(updateMock).toHaveBeenCalledWith('/test/project', 'feature-123', 'interrupted');
|
||||
});
|
||||
|
||||
it('should propagate errors from updateFeatureStatus', async () => {
|
||||
const updateMock = vi.fn().mockRejectedValue(new Error('Update failed'));
|
||||
mockUpdateFeatureStatus(service, updateMock);
|
||||
|
||||
await expect(service.markFeatureInterrupted('/test/project', 'feature-123')).rejects.toThrow(
|
||||
'Update failed'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('markAllRunningFeaturesInterrupted', () => {
|
||||
// Helper to access private runningFeatures Map
|
||||
const getRunningFeaturesMap = (svc: AutoModeService) =>
|
||||
(svc as any).runningFeatures as Map<
|
||||
string,
|
||||
{ featureId: string; projectPath: string; isAutoMode: boolean }
|
||||
>;
|
||||
|
||||
// Helper to mock updateFeatureStatus
|
||||
const mockUpdateFeatureStatus = (svc: AutoModeService, mockFn: ReturnType<typeof vi.fn>) => {
|
||||
(svc as any).updateFeatureStatus = mockFn;
|
||||
};
|
||||
|
||||
it('should do nothing when no features are running', async () => {
|
||||
const updateMock = vi.fn().mockResolvedValue(undefined);
|
||||
mockUpdateFeatureStatus(service, updateMock);
|
||||
|
||||
await service.markAllRunningFeaturesInterrupted();
|
||||
|
||||
expect(updateMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should mark a single running feature as interrupted', async () => {
|
||||
const runningFeaturesMap = getRunningFeaturesMap(service);
|
||||
runningFeaturesMap.set('feature-1', {
|
||||
featureId: 'feature-1',
|
||||
projectPath: '/project/path',
|
||||
isAutoMode: true,
|
||||
});
|
||||
|
||||
const updateMock = vi.fn().mockResolvedValue(undefined);
|
||||
mockUpdateFeatureStatus(service, updateMock);
|
||||
|
||||
await service.markAllRunningFeaturesInterrupted();
|
||||
|
||||
expect(updateMock).toHaveBeenCalledWith('/project/path', 'feature-1', 'interrupted');
|
||||
});
|
||||
|
||||
it('should mark multiple running features as interrupted', async () => {
|
||||
const runningFeaturesMap = getRunningFeaturesMap(service);
|
||||
runningFeaturesMap.set('feature-1', {
|
||||
featureId: 'feature-1',
|
||||
projectPath: '/project-a',
|
||||
isAutoMode: true,
|
||||
});
|
||||
runningFeaturesMap.set('feature-2', {
|
||||
featureId: 'feature-2',
|
||||
projectPath: '/project-b',
|
||||
isAutoMode: false,
|
||||
});
|
||||
runningFeaturesMap.set('feature-3', {
|
||||
featureId: 'feature-3',
|
||||
projectPath: '/project-a',
|
||||
isAutoMode: true,
|
||||
});
|
||||
|
||||
const updateMock = vi.fn().mockResolvedValue(undefined);
|
||||
mockUpdateFeatureStatus(service, updateMock);
|
||||
|
||||
await service.markAllRunningFeaturesInterrupted();
|
||||
|
||||
expect(updateMock).toHaveBeenCalledTimes(3);
|
||||
expect(updateMock).toHaveBeenCalledWith('/project-a', 'feature-1', 'interrupted');
|
||||
expect(updateMock).toHaveBeenCalledWith('/project-b', 'feature-2', 'interrupted');
|
||||
expect(updateMock).toHaveBeenCalledWith('/project-a', 'feature-3', 'interrupted');
|
||||
});
|
||||
|
||||
it('should mark features in parallel', async () => {
|
||||
const runningFeaturesMap = getRunningFeaturesMap(service);
|
||||
for (let i = 1; i <= 5; i++) {
|
||||
runningFeaturesMap.set(`feature-${i}`, {
|
||||
featureId: `feature-${i}`,
|
||||
projectPath: `/project-${i}`,
|
||||
isAutoMode: true,
|
||||
});
|
||||
}
|
||||
|
||||
const callOrder: string[] = [];
|
||||
const updateMock = vi.fn().mockImplementation(async (_path: string, featureId: string) => {
|
||||
callOrder.push(featureId);
|
||||
await new Promise((resolve) => setTimeout(resolve, 10));
|
||||
});
|
||||
mockUpdateFeatureStatus(service, updateMock);
|
||||
|
||||
const startTime = Date.now();
|
||||
await service.markAllRunningFeaturesInterrupted();
|
||||
const duration = Date.now() - startTime;
|
||||
|
||||
expect(updateMock).toHaveBeenCalledTimes(5);
|
||||
// If executed in parallel, total time should be ~10ms
|
||||
// If sequential, it would be ~50ms (5 * 10ms)
|
||||
expect(duration).toBeLessThan(40);
|
||||
});
|
||||
|
||||
it('should continue marking other features when one fails', async () => {
|
||||
const runningFeaturesMap = getRunningFeaturesMap(service);
|
||||
runningFeaturesMap.set('feature-1', {
|
||||
featureId: 'feature-1',
|
||||
projectPath: '/project-a',
|
||||
isAutoMode: true,
|
||||
});
|
||||
runningFeaturesMap.set('feature-2', {
|
||||
featureId: 'feature-2',
|
||||
projectPath: '/project-b',
|
||||
isAutoMode: false,
|
||||
});
|
||||
|
||||
const updateMock = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce(undefined)
|
||||
.mockRejectedValueOnce(new Error('Failed to update'));
|
||||
mockUpdateFeatureStatus(service, updateMock);
|
||||
|
||||
// Should not throw even though one feature failed
|
||||
await expect(service.markAllRunningFeaturesInterrupted()).resolves.not.toThrow();
|
||||
|
||||
expect(updateMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('should use provided reason in logging', async () => {
|
||||
const runningFeaturesMap = getRunningFeaturesMap(service);
|
||||
runningFeaturesMap.set('feature-1', {
|
||||
featureId: 'feature-1',
|
||||
projectPath: '/project/path',
|
||||
isAutoMode: true,
|
||||
});
|
||||
|
||||
const updateMock = vi.fn().mockResolvedValue(undefined);
|
||||
mockUpdateFeatureStatus(service, updateMock);
|
||||
|
||||
await service.markAllRunningFeaturesInterrupted('manual stop');
|
||||
|
||||
expect(updateMock).toHaveBeenCalledWith('/project/path', 'feature-1', 'interrupted');
|
||||
});
|
||||
|
||||
it('should use default reason when none provided', async () => {
|
||||
const runningFeaturesMap = getRunningFeaturesMap(service);
|
||||
runningFeaturesMap.set('feature-1', {
|
||||
featureId: 'feature-1',
|
||||
projectPath: '/project/path',
|
||||
isAutoMode: true,
|
||||
});
|
||||
|
||||
const updateMock = vi.fn().mockResolvedValue(undefined);
|
||||
mockUpdateFeatureStatus(service, updateMock);
|
||||
|
||||
await service.markAllRunningFeaturesInterrupted();
|
||||
|
||||
expect(updateMock).toHaveBeenCalledWith('/project/path', 'feature-1', 'interrupted');
|
||||
});
|
||||
});
|
||||
|
||||
describe('isFeatureRunning', () => {
|
||||
// Helper to access private runningFeatures Map
|
||||
const getRunningFeaturesMap = (svc: AutoModeService) =>
|
||||
(svc as any).runningFeatures as Map<
|
||||
string,
|
||||
{ featureId: string; projectPath: string; isAutoMode: boolean }
|
||||
>;
|
||||
|
||||
it('should return false when no features are running', () => {
|
||||
expect(service.isFeatureRunning('feature-123')).toBe(false);
|
||||
});
|
||||
|
||||
it('should return true when the feature is running', () => {
|
||||
const runningFeaturesMap = getRunningFeaturesMap(service);
|
||||
runningFeaturesMap.set('feature-123', {
|
||||
featureId: 'feature-123',
|
||||
projectPath: '/project/path',
|
||||
isAutoMode: true,
|
||||
});
|
||||
|
||||
expect(service.isFeatureRunning('feature-123')).toBe(true);
|
||||
});
|
||||
|
||||
it('should return false for non-running feature when others are running', () => {
|
||||
const runningFeaturesMap = getRunningFeaturesMap(service);
|
||||
runningFeaturesMap.set('feature-other', {
|
||||
featureId: 'feature-other',
|
||||
projectPath: '/project/path',
|
||||
isAutoMode: true,
|
||||
});
|
||||
|
||||
expect(service.isFeatureRunning('feature-123')).toBe(false);
|
||||
});
|
||||
|
||||
it('should correctly track multiple running features', () => {
|
||||
const runningFeaturesMap = getRunningFeaturesMap(service);
|
||||
runningFeaturesMap.set('feature-1', {
|
||||
featureId: 'feature-1',
|
||||
projectPath: '/project-a',
|
||||
isAutoMode: true,
|
||||
});
|
||||
runningFeaturesMap.set('feature-2', {
|
||||
featureId: 'feature-2',
|
||||
projectPath: '/project-b',
|
||||
isAutoMode: false,
|
||||
});
|
||||
|
||||
expect(service.isFeatureRunning('feature-1')).toBe(true);
|
||||
expect(service.isFeatureRunning('feature-2')).toBe(true);
|
||||
expect(service.isFeatureRunning('feature-3')).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user