00e426ac00
When merge_and_push failed at the push step, the local defaultBranch ref was left pointing at the merge commit. This made the three-dot diff (defaultBranch...initiativeBranch) return empty because main already contained all changes — causing the review tab to show "no changes." Now mergeBranch returns the previous ref, and approveInitiative restores it on push failure. Also repaired the corrupted clone state.
370 lines
14 KiB
TypeScript
370 lines
14 KiB
TypeScript
/**
|
|
* ExecutionOrchestrator Tests
|
|
*
|
|
* Tests phase completion transitions, especially when initiative has no branch.
|
|
*/
|
|
|
|
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
|
import { ExecutionOrchestrator } from './orchestrator.js';
|
|
import { ensureProjectClone } from '../git/project-clones.js';
|
|
import type { BranchManager } from '../git/branch-manager.js';
|
|
|
|
vi.mock('../git/project-clones.js', () => ({
|
|
ensureProjectClone: vi.fn().mockResolvedValue('/tmp/test-workspace/clones/test'),
|
|
}));
|
|
import type { PhaseRepository } from '../db/repositories/phase-repository.js';
|
|
import type { TaskRepository } from '../db/repositories/task-repository.js';
|
|
import type { InitiativeRepository } from '../db/repositories/initiative-repository.js';
|
|
import type { ProjectRepository } from '../db/repositories/project-repository.js';
|
|
import type { DispatchManager, PhaseDispatchManager } from '../dispatch/types.js';
|
|
import type { ConflictResolutionService } from '../coordination/conflict-resolution-service.js';
|
|
import type { EventBus, TaskCompletedEvent, DomainEvent } from '../events/types.js';
|
|
|
|
function createMockEventBus(): EventBus & { handlers: Map<string, Function[]>; emitted: DomainEvent[] } {
|
|
const handlers = new Map<string, Function[]>();
|
|
const emitted: DomainEvent[] = [];
|
|
return {
|
|
handlers,
|
|
emitted,
|
|
emit: vi.fn((event: DomainEvent) => {
|
|
emitted.push(event);
|
|
const fns = handlers.get(event.type) ?? [];
|
|
for (const fn of fns) fn(event);
|
|
}),
|
|
on: vi.fn((type: string, handler: Function) => {
|
|
const fns = handlers.get(type) ?? [];
|
|
fns.push(handler);
|
|
handlers.set(type, fns);
|
|
}),
|
|
off: vi.fn(),
|
|
once: vi.fn(),
|
|
};
|
|
}
|
|
|
|
function createMocks() {
|
|
const branchManager: BranchManager = {
|
|
ensureBranch: vi.fn(),
|
|
mergeBranch: vi.fn().mockResolvedValue({ success: true, message: 'merged', previousRef: 'abc000' }),
|
|
diffBranches: vi.fn().mockResolvedValue(''),
|
|
deleteBranch: vi.fn(),
|
|
branchExists: vi.fn().mockResolvedValue(true),
|
|
remoteBranchExists: vi.fn().mockResolvedValue(false),
|
|
listCommits: vi.fn().mockResolvedValue([]),
|
|
diffCommit: vi.fn().mockResolvedValue(''),
|
|
getMergeBase: vi.fn().mockResolvedValue('abc123'),
|
|
pushBranch: vi.fn(),
|
|
checkMergeability: vi.fn().mockResolvedValue({ mergeable: true }),
|
|
fetchRemote: vi.fn(),
|
|
fastForwardBranch: vi.fn(),
|
|
updateRef: vi.fn(),
|
|
};
|
|
|
|
const phaseRepository = {
|
|
findById: vi.fn(),
|
|
findByInitiativeId: vi.fn().mockResolvedValue([]),
|
|
update: vi.fn().mockImplementation(async (id: string, data: any) => ({ id, ...data })),
|
|
create: vi.fn(),
|
|
} as unknown as PhaseRepository;
|
|
|
|
const taskRepository = {
|
|
findById: vi.fn(),
|
|
findByPhaseId: vi.fn().mockResolvedValue([]),
|
|
findByInitiativeId: vi.fn().mockResolvedValue([]),
|
|
} as unknown as TaskRepository;
|
|
|
|
const initiativeRepository = {
|
|
findById: vi.fn(),
|
|
findByStatus: vi.fn().mockResolvedValue([]),
|
|
update: vi.fn(),
|
|
} as unknown as InitiativeRepository;
|
|
|
|
const projectRepository = {
|
|
findProjectsByInitiativeId: vi.fn().mockResolvedValue([]),
|
|
} as unknown as ProjectRepository;
|
|
|
|
const phaseDispatchManager: PhaseDispatchManager = {
|
|
queuePhase: vi.fn(),
|
|
getNextDispatchablePhase: vi.fn().mockResolvedValue(null),
|
|
dispatchNextPhase: vi.fn().mockResolvedValue({ success: false, phaseId: '', reason: 'none' }),
|
|
completePhase: vi.fn(),
|
|
blockPhase: vi.fn(),
|
|
getPhaseQueueState: vi.fn().mockResolvedValue({ queued: [], ready: [], blocked: [] }),
|
|
};
|
|
|
|
const dispatchManager = {
|
|
queue: vi.fn(),
|
|
getNextDispatchable: vi.fn().mockResolvedValue(null),
|
|
dispatchNext: vi.fn().mockResolvedValue({ success: false, taskId: '' }),
|
|
completeTask: vi.fn(),
|
|
blockTask: vi.fn(),
|
|
retryBlockedTask: vi.fn(),
|
|
getQueueState: vi.fn().mockResolvedValue({ queued: [], ready: [], blocked: [] }),
|
|
} as unknown as DispatchManager;
|
|
|
|
const conflictResolutionService: ConflictResolutionService = {
|
|
handleConflict: vi.fn(),
|
|
};
|
|
|
|
const eventBus = createMockEventBus();
|
|
|
|
return {
|
|
branchManager,
|
|
phaseRepository,
|
|
taskRepository,
|
|
initiativeRepository,
|
|
projectRepository,
|
|
phaseDispatchManager,
|
|
dispatchManager,
|
|
conflictResolutionService,
|
|
eventBus,
|
|
};
|
|
}
|
|
|
|
function createOrchestrator(mocks: ReturnType<typeof createMocks>) {
|
|
const orchestrator = new ExecutionOrchestrator(
|
|
mocks.branchManager,
|
|
mocks.phaseRepository,
|
|
mocks.taskRepository,
|
|
mocks.initiativeRepository,
|
|
mocks.projectRepository,
|
|
mocks.phaseDispatchManager,
|
|
mocks.dispatchManager,
|
|
mocks.conflictResolutionService,
|
|
mocks.eventBus,
|
|
'/tmp/test-workspace',
|
|
);
|
|
orchestrator.start();
|
|
return orchestrator;
|
|
}
|
|
|
|
function emitTaskCompleted(eventBus: ReturnType<typeof createMockEventBus>, taskId: string) {
|
|
const event: TaskCompletedEvent = {
|
|
type: 'task:completed',
|
|
timestamp: new Date(),
|
|
payload: { taskId, agentId: 'agent-1', success: true, message: 'done' },
|
|
};
|
|
eventBus.emit(event);
|
|
}
|
|
|
|
describe('ExecutionOrchestrator', () => {
|
|
let mocks: ReturnType<typeof createMocks>;
|
|
|
|
beforeEach(() => {
|
|
mocks = createMocks();
|
|
});
|
|
|
|
describe('phase completion when initiative has no branch', () => {
|
|
it('should transition phase to pending_review in review mode even without a branch', async () => {
|
|
const task = {
|
|
id: 'task-1',
|
|
phaseId: 'phase-1',
|
|
initiativeId: 'init-1',
|
|
category: 'execute',
|
|
status: 'completed',
|
|
};
|
|
const phase = { id: 'phase-1', initiativeId: 'init-1', name: 'Phase 1', status: 'in_progress' };
|
|
const initiative = { id: 'init-1', branch: null, executionMode: 'review_per_phase' };
|
|
|
|
vi.mocked(mocks.taskRepository.findById).mockResolvedValue(task as any);
|
|
vi.mocked(mocks.phaseRepository.findById).mockResolvedValue(phase as any);
|
|
vi.mocked(mocks.initiativeRepository.findById).mockResolvedValue(initiative as any);
|
|
vi.mocked(mocks.taskRepository.findByPhaseId).mockResolvedValue([task] as any);
|
|
|
|
createOrchestrator(mocks);
|
|
|
|
emitTaskCompleted(mocks.eventBus, 'task-1');
|
|
|
|
// Allow async handler to complete
|
|
await vi.waitFor(() => {
|
|
expect(mocks.phaseRepository.update).toHaveBeenCalledWith('phase-1', { status: 'pending_review' });
|
|
});
|
|
});
|
|
|
|
it('should complete phase in yolo mode even without a branch', async () => {
|
|
const task = {
|
|
id: 'task-1',
|
|
phaseId: 'phase-1',
|
|
initiativeId: 'init-1',
|
|
category: 'execute',
|
|
status: 'completed',
|
|
};
|
|
const phase = { id: 'phase-1', initiativeId: 'init-1', name: 'Phase 1', status: 'in_progress' };
|
|
const initiative = { id: 'init-1', branch: null, executionMode: 'yolo' };
|
|
|
|
vi.mocked(mocks.taskRepository.findById).mockResolvedValue(task as any);
|
|
vi.mocked(mocks.phaseRepository.findById).mockResolvedValue(phase as any);
|
|
vi.mocked(mocks.initiativeRepository.findById).mockResolvedValue(initiative as any);
|
|
vi.mocked(mocks.initiativeRepository.findByStatus).mockResolvedValue([]);
|
|
vi.mocked(mocks.taskRepository.findByPhaseId).mockResolvedValue([task] as any);
|
|
vi.mocked(mocks.phaseRepository.findByInitiativeId).mockResolvedValue([phase] as any);
|
|
|
|
createOrchestrator(mocks);
|
|
|
|
emitTaskCompleted(mocks.eventBus, 'task-1');
|
|
|
|
await vi.waitFor(() => {
|
|
expect(mocks.phaseDispatchManager.completePhase).toHaveBeenCalledWith('phase-1');
|
|
});
|
|
|
|
// Should NOT have attempted any branch merges
|
|
expect(mocks.branchManager.mergeBranch).not.toHaveBeenCalled();
|
|
});
|
|
});
|
|
|
|
describe('phase completion when merge fails', () => {
|
|
it('should still check phase completion even if task merge throws', async () => {
|
|
const task = {
|
|
id: 'task-1',
|
|
phaseId: 'phase-1',
|
|
initiativeId: 'init-1',
|
|
category: 'execute',
|
|
status: 'completed',
|
|
};
|
|
const phase = { id: 'phase-1', initiativeId: 'init-1', name: 'Phase 1', status: 'in_progress' };
|
|
const initiative = { id: 'init-1', branch: 'cw/test', executionMode: 'review_per_phase' };
|
|
const project = { id: 'proj-1', name: 'test', url: 'https://example.com', defaultBranch: 'main' };
|
|
|
|
vi.mocked(mocks.taskRepository.findById).mockResolvedValue(task as any);
|
|
vi.mocked(mocks.phaseRepository.findById).mockResolvedValue(phase as any);
|
|
vi.mocked(mocks.initiativeRepository.findById).mockResolvedValue(initiative as any);
|
|
vi.mocked(mocks.taskRepository.findByPhaseId).mockResolvedValue([task] as any);
|
|
vi.mocked(mocks.projectRepository.findProjectsByInitiativeId).mockResolvedValue([project] as any);
|
|
// Merge fails
|
|
vi.mocked(mocks.branchManager.mergeBranch).mockResolvedValue({
|
|
success: false,
|
|
message: 'conflict',
|
|
conflicts: ['file.ts'],
|
|
});
|
|
|
|
createOrchestrator(mocks);
|
|
|
|
emitTaskCompleted(mocks.eventBus, 'task-1');
|
|
|
|
// Phase should still transition despite merge failure
|
|
await vi.waitFor(() => {
|
|
expect(mocks.phaseRepository.update).toHaveBeenCalledWith('phase-1', { status: 'pending_review' });
|
|
});
|
|
});
|
|
});
|
|
|
|
describe('phase completion with branch (normal flow)', () => {
|
|
it('should merge task branch and transition phase when all tasks done', async () => {
|
|
const task = {
|
|
id: 'task-1',
|
|
phaseId: 'phase-1',
|
|
initiativeId: 'init-1',
|
|
category: 'execute',
|
|
status: 'completed',
|
|
};
|
|
const phase = { id: 'phase-1', initiativeId: 'init-1', name: 'Phase 1', status: 'in_progress' };
|
|
const initiative = { id: 'init-1', branch: 'cw/test', executionMode: 'review_per_phase' };
|
|
const project = { id: 'proj-1', name: 'test', url: 'https://example.com', defaultBranch: 'main' };
|
|
|
|
vi.mocked(mocks.taskRepository.findById).mockResolvedValue(task as any);
|
|
vi.mocked(mocks.phaseRepository.findById).mockResolvedValue(phase as any);
|
|
vi.mocked(mocks.initiativeRepository.findById).mockResolvedValue(initiative as any);
|
|
vi.mocked(mocks.taskRepository.findByPhaseId).mockResolvedValue([task] as any);
|
|
vi.mocked(mocks.projectRepository.findProjectsByInitiativeId).mockResolvedValue([project] as any);
|
|
vi.mocked(mocks.branchManager.branchExists).mockResolvedValue(true);
|
|
vi.mocked(mocks.branchManager.mergeBranch).mockResolvedValue({ success: true, message: 'ok' });
|
|
|
|
createOrchestrator(mocks);
|
|
|
|
emitTaskCompleted(mocks.eventBus, 'task-1');
|
|
|
|
await vi.waitFor(() => {
|
|
expect(mocks.phaseRepository.update).toHaveBeenCalledWith('phase-1', { status: 'pending_review' });
|
|
});
|
|
});
|
|
|
|
it('should not transition phase when some tasks are still pending', async () => {
|
|
const task1 = {
|
|
id: 'task-1',
|
|
phaseId: 'phase-1',
|
|
initiativeId: 'init-1',
|
|
category: 'execute',
|
|
status: 'completed',
|
|
};
|
|
const task2 = {
|
|
id: 'task-2',
|
|
phaseId: 'phase-1',
|
|
initiativeId: 'init-1',
|
|
category: 'execute',
|
|
status: 'pending',
|
|
};
|
|
const phase = { id: 'phase-1', initiativeId: 'init-1', name: 'Phase 1', status: 'in_progress' };
|
|
const initiative = { id: 'init-1', branch: 'cw/test', executionMode: 'review_per_phase' };
|
|
|
|
vi.mocked(mocks.taskRepository.findById).mockResolvedValue(task1 as any);
|
|
vi.mocked(mocks.phaseRepository.findById).mockResolvedValue(phase as any);
|
|
vi.mocked(mocks.initiativeRepository.findById).mockResolvedValue(initiative as any);
|
|
vi.mocked(mocks.taskRepository.findByPhaseId).mockResolvedValue([task1, task2] as any);
|
|
vi.mocked(mocks.projectRepository.findProjectsByInitiativeId).mockResolvedValue([]);
|
|
|
|
createOrchestrator(mocks);
|
|
|
|
emitTaskCompleted(mocks.eventBus, 'task-1');
|
|
|
|
// Give the async handler time to run
|
|
await new Promise((r) => setTimeout(r, 50));
|
|
|
|
expect(mocks.phaseRepository.update).not.toHaveBeenCalled();
|
|
expect(mocks.phaseDispatchManager.completePhase).not.toHaveBeenCalled();
|
|
});
|
|
});
|
|
|
|
describe('approveInitiative', () => {
|
|
function setupApproveTest(mocks: ReturnType<typeof createMocks>) {
|
|
const initiative = { id: 'init-1', branch: 'cw/test', status: 'pending_review' };
|
|
const project = { id: 'proj-1', name: 'test', url: 'https://example.com', defaultBranch: 'main' };
|
|
vi.mocked(mocks.initiativeRepository.findById).mockResolvedValue(initiative as any);
|
|
vi.mocked(mocks.projectRepository.findProjectsByInitiativeId).mockResolvedValue([project] as any);
|
|
vi.mocked(mocks.branchManager.branchExists).mockResolvedValue(true);
|
|
vi.mocked(mocks.branchManager.mergeBranch).mockResolvedValue({ success: true, message: 'ok', previousRef: 'abc000' });
|
|
return { initiative, project };
|
|
}
|
|
|
|
it('should roll back merge when push fails', async () => {
|
|
setupApproveTest(mocks);
|
|
vi.mocked(mocks.branchManager.pushBranch).mockRejectedValue(new Error('non-fast-forward'));
|
|
|
|
const orchestrator = createOrchestrator(mocks);
|
|
|
|
await expect(orchestrator.approveInitiative('init-1', 'merge_and_push')).rejects.toThrow('non-fast-forward');
|
|
|
|
// Should have rolled back the merge by restoring the previous ref
|
|
expect(mocks.branchManager.updateRef).toHaveBeenCalledWith(
|
|
expect.any(String),
|
|
'main',
|
|
'abc000',
|
|
);
|
|
|
|
// Should NOT have marked initiative as completed
|
|
expect(mocks.initiativeRepository.update).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it('should complete initiative when push succeeds', async () => {
|
|
setupApproveTest(mocks);
|
|
|
|
const orchestrator = createOrchestrator(mocks);
|
|
|
|
await orchestrator.approveInitiative('init-1', 'merge_and_push');
|
|
|
|
expect(mocks.branchManager.updateRef).not.toHaveBeenCalled();
|
|
expect(mocks.initiativeRepository.update).toHaveBeenCalledWith('init-1', { status: 'completed' });
|
|
});
|
|
|
|
it('should not attempt rollback for push_branch strategy', async () => {
|
|
setupApproveTest(mocks);
|
|
vi.mocked(mocks.branchManager.pushBranch).mockRejectedValue(new Error('auth failed'));
|
|
|
|
const orchestrator = createOrchestrator(mocks);
|
|
|
|
await expect(orchestrator.approveInitiative('init-1', 'push_branch')).rejects.toThrow('auth failed');
|
|
|
|
// No merge happened, so no rollback needed
|
|
expect(mocks.branchManager.updateRef).not.toHaveBeenCalled();
|
|
});
|
|
});
|
|
});
|