Merge branch 'cw/continuous-code-quality-phase-quality-review-dispatch-hook' into cw-merge-1772831549281

This commit is contained in:
Lukas May
2026-03-06 22:12:29 +01:00
6 changed files with 1025 additions and 7 deletions

View File

@@ -251,6 +251,7 @@ export async function createContainer(options?: ContainerOptions): Promise<Conta
eventBus, eventBus,
workspaceRoot, workspaceRoot,
repos.agentRepository, repos.agentRepository,
agentManager,
); );
executionOrchestrator.start(); executionOrchestrator.start();
log.info('execution orchestrator started'); log.info('execution orchestrator started');

View File

@@ -8,17 +8,26 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
import { ExecutionOrchestrator } from './orchestrator.js'; import { ExecutionOrchestrator } from './orchestrator.js';
import { ensureProjectClone } from '../git/project-clones.js'; import { ensureProjectClone } from '../git/project-clones.js';
import type { BranchManager } from '../git/branch-manager.js'; import type { BranchManager } from '../git/branch-manager.js';
import type { AgentManager } from '../agent/types.js';
import type { AgentRepository } from '../db/repositories/agent-repository.js';
vi.mock('../git/project-clones.js', () => ({ vi.mock('../git/project-clones.js', () => ({
ensureProjectClone: vi.fn().mockResolvedValue('/tmp/test-workspace/clones/test'), ensureProjectClone: vi.fn().mockResolvedValue('/tmp/test-workspace/clones/test'),
})); }));
vi.mock('./quality-review.js', () => ({
shouldRunQualityReview: vi.fn().mockResolvedValue({ run: false, qualifyingFiles: [] }),
runQualityReview: vi.fn().mockResolvedValue(undefined),
computeQualifyingFiles: vi.fn().mockResolvedValue([]),
}));
import type { PhaseRepository } from '../db/repositories/phase-repository.js'; import type { PhaseRepository } from '../db/repositories/phase-repository.js';
import type { TaskRepository } from '../db/repositories/task-repository.js'; import type { TaskRepository } from '../db/repositories/task-repository.js';
import type { InitiativeRepository } from '../db/repositories/initiative-repository.js'; import type { InitiativeRepository } from '../db/repositories/initiative-repository.js';
import type { ProjectRepository } from '../db/repositories/project-repository.js'; import type { ProjectRepository } from '../db/repositories/project-repository.js';
import type { DispatchManager, PhaseDispatchManager } from '../dispatch/types.js'; import type { DispatchManager, PhaseDispatchManager } from '../dispatch/types.js';
import type { ConflictResolutionService } from '../coordination/conflict-resolution-service.js'; import type { ConflictResolutionService } from '../coordination/conflict-resolution-service.js';
import type { EventBus, TaskCompletedEvent, DomainEvent } from '../events/types.js'; import type { EventBus, TaskCompletedEvent, AgentStoppedEvent, DomainEvent } from '../events/types.js';
import { shouldRunQualityReview, runQualityReview } from './quality-review.js';
function createMockEventBus(): EventBus & { handlers: Map<string, Function[]>; emitted: DomainEvent[] } { function createMockEventBus(): EventBus & { handlers: Map<string, Function[]>; emitted: DomainEvent[] } {
const handlers = new Map<string, Function[]>(); const handlers = new Map<string, Function[]>();
@@ -108,6 +117,33 @@ function createMocks() {
handleConflict: vi.fn(), handleConflict: vi.fn(),
}; };
const agentRepository = {
create: vi.fn(),
findById: vi.fn().mockResolvedValue(null),
findByName: vi.fn(),
findByTaskId: vi.fn(),
findBySessionId: vi.fn(),
findAll: vi.fn(),
findByStatus: vi.fn(),
update: vi.fn(),
delete: vi.fn(),
} as unknown as AgentRepository;
const agentManager = {
spawn: vi.fn().mockResolvedValue({ id: 'review-agent-1', name: 'review-agent' }),
stop: vi.fn(),
list: vi.fn().mockResolvedValue([]),
get: vi.fn(),
getByName: vi.fn(),
resume: vi.fn(),
getResult: vi.fn(),
getPendingQuestions: vi.fn(),
delete: vi.fn(),
dismiss: vi.fn(),
resumeForConversation: vi.fn(),
sendUserMessage: vi.fn(),
} as unknown as AgentManager;
const eventBus = createMockEventBus(); const eventBus = createMockEventBus();
return { return {
@@ -119,11 +155,13 @@ function createMocks() {
phaseDispatchManager, phaseDispatchManager,
dispatchManager, dispatchManager,
conflictResolutionService, conflictResolutionService,
agentRepository,
agentManager,
eventBus, eventBus,
}; };
} }
function createOrchestrator(mocks: ReturnType<typeof createMocks>) { function createOrchestrator(mocks: ReturnType<typeof createMocks>, opts: { withAgentManager?: boolean; withAgentRepository?: boolean } = {}) {
const orchestrator = new ExecutionOrchestrator( const orchestrator = new ExecutionOrchestrator(
mocks.branchManager, mocks.branchManager,
mocks.phaseRepository, mocks.phaseRepository,
@@ -135,6 +173,8 @@ function createOrchestrator(mocks: ReturnType<typeof createMocks>) {
mocks.conflictResolutionService, mocks.conflictResolutionService,
mocks.eventBus, mocks.eventBus,
'/tmp/test-workspace', '/tmp/test-workspace',
opts.withAgentRepository !== false ? mocks.agentRepository : undefined,
opts.withAgentManager !== false ? mocks.agentManager : undefined,
); );
orchestrator.start(); orchestrator.start();
return orchestrator; return orchestrator;
@@ -369,4 +409,176 @@ describe('ExecutionOrchestrator', () => {
expect(mocks.branchManager.updateRef).not.toHaveBeenCalled(); expect(mocks.branchManager.updateRef).not.toHaveBeenCalled();
}); });
}); });
describe('handleAgentStopped quality review hook', () => {
function emitAgentStopped(eventBus: ReturnType<typeof createMockEventBus>, payload: { taskId?: string; agentId: string; reason: AgentStoppedEvent['payload']['reason'] }) {
const event: AgentStoppedEvent = {
type: 'agent:stopped',
timestamp: new Date(),
payload: { taskId: payload.taskId ?? null, agentId: payload.agentId, name: 'test-agent', reason: payload.reason },
};
eventBus.emit(event);
}
function setupQualityReviewMocks() {
const task = { id: 'task-1', phaseId: 'phase-1', initiativeId: 'init-1', category: 'execute', status: 'in_progress' };
const initiative = { id: 'init-1', branch: 'cw/test-initiative', executionMode: 'yolo', qualityReview: true };
const phase = { id: 'phase-1', initiativeId: 'init-1', name: 'Phase 1', status: 'in_progress' };
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.initiativeRepository.findById).mockResolvedValue(initiative as any);
vi.mocked(mocks.phaseRepository.findById).mockResolvedValue(phase as any);
vi.mocked(mocks.projectRepository.findProjectsByInitiativeId).mockResolvedValue([project] as any);
vi.mocked(ensureProjectClone).mockResolvedValue('/tmp/test-workspace/clones/test');
}
beforeEach(() => {
vi.mocked(shouldRunQualityReview).mockClear().mockResolvedValue({ run: false, qualifyingFiles: [] });
vi.mocked(runQualityReview).mockClear().mockResolvedValue(undefined);
});
it('should not call shouldRunQualityReview when reason is user_requested', async () => {
createOrchestrator(mocks);
emitAgentStopped(mocks.eventBus, { taskId: 'task-1', agentId: 'agent-1', reason: 'user_requested' });
await new Promise((r) => setTimeout(r, 50));
expect(shouldRunQualityReview).not.toHaveBeenCalled();
expect(mocks.dispatchManager.completeTask).not.toHaveBeenCalled();
});
it('should call dispatchManager.completeTask when shouldRunQualityReview returns run: false', async () => {
setupQualityReviewMocks();
vi.mocked(shouldRunQualityReview).mockResolvedValue({ run: false, qualifyingFiles: [] });
createOrchestrator(mocks);
emitAgentStopped(mocks.eventBus, { taskId: 'task-1', agentId: 'agent-1', reason: 'task_complete' });
await vi.waitFor(() => {
expect(mocks.dispatchManager.completeTask).toHaveBeenCalledWith('task-1', 'agent-1');
});
expect(runQualityReview).not.toHaveBeenCalled();
});
it('should call runQualityReview and NOT call completeTask when shouldRunQualityReview returns run: true', async () => {
setupQualityReviewMocks();
vi.mocked(shouldRunQualityReview).mockResolvedValue({ run: true, qualifyingFiles: ['src/foo.ts'] });
createOrchestrator(mocks);
emitAgentStopped(mocks.eventBus, { taskId: 'task-1', agentId: 'agent-1', reason: 'task_complete' });
await vi.waitFor(() => {
expect(runQualityReview).toHaveBeenCalledWith(
expect.objectContaining({
taskId: 'task-1',
qualifyingFiles: ['src/foo.ts'],
taskRepository: mocks.taskRepository,
}),
);
});
expect(mocks.dispatchManager.completeTask).not.toHaveBeenCalled();
});
it('should fall back to completeTask when agentRepository is not available', async () => {
createOrchestrator(mocks, { withAgentRepository: false, withAgentManager: false });
emitAgentStopped(mocks.eventBus, { taskId: 'task-1', agentId: 'agent-1', reason: 'task_complete' });
await vi.waitFor(() => {
expect(mocks.dispatchManager.completeTask).toHaveBeenCalledWith('task-1', 'agent-1');
});
expect(shouldRunQualityReview).not.toHaveBeenCalled();
});
});
});
describe('handleAgentStopped — quality review integration', () => {
let mocks: ReturnType<typeof createMocks>;
beforeEach(() => {
mocks = createMocks();
vi.mocked(shouldRunQualityReview).mockReset();
vi.mocked(runQualityReview).mockReset();
});
it('calls runQualityReview and skips completeTask when shouldRunQualityReview returns run:true', async () => {
vi.mocked(shouldRunQualityReview).mockResolvedValue({
run: true,
qualifyingFiles: ['src/foo.ts'],
});
vi.mocked(runQualityReview).mockResolvedValue(undefined);
// Provide task data for re-fetch inside runQualityReview branch
vi.mocked(mocks.taskRepository.findById).mockResolvedValue({
id: 't1',
status: 'in_progress',
initiativeId: 'i1',
phaseId: 'p1',
} as any);
vi.mocked(mocks.initiativeRepository.findById).mockResolvedValue({
id: 'i1',
branch: 'cw/test',
qualityReview: true,
} as any);
vi.mocked(mocks.phaseRepository.findById).mockResolvedValue({
id: 'p1',
name: 'impl',
initiativeId: 'i1',
} as any);
vi.mocked(mocks.projectRepository.findProjectsByInitiativeId).mockResolvedValue([
{ id: 'proj-1', name: 'test', url: 'https://example.com', defaultBranch: 'main' },
] as any);
createOrchestrator(mocks);
mocks.eventBus.emit({
type: 'agent:stopped',
timestamp: new Date(),
payload: { taskId: 't1', reason: 'task_complete', agentId: 'a1' },
});
await vi.waitFor(() => {
expect(runQualityReview).toHaveBeenCalledWith(
expect.objectContaining({ taskId: 't1', qualifyingFiles: ['src/foo.ts'] }),
);
});
expect(mocks.dispatchManager.completeTask).not.toHaveBeenCalled();
});
it('calls completeTask and skips runQualityReview when shouldRunQualityReview returns run:false', async () => {
vi.mocked(shouldRunQualityReview).mockResolvedValue({ run: false, qualifyingFiles: [] });
vi.mocked(runQualityReview).mockResolvedValue(undefined);
createOrchestrator(mocks);
mocks.eventBus.emit({
type: 'agent:stopped',
timestamp: new Date(),
payload: { taskId: 't1', reason: 'task_complete', agentId: 'a1' },
});
await vi.waitFor(() => {
expect(mocks.dispatchManager.completeTask).toHaveBeenCalledWith('t1', 'a1');
});
expect(runQualityReview).not.toHaveBeenCalled();
});
it('skips both paths for user_requested reason', async () => {
createOrchestrator(mocks);
mocks.eventBus.emit({
type: 'agent:stopped',
timestamp: new Date(),
payload: { taskId: 't1', reason: 'user_requested', agentId: 'a1' },
});
// Wait for scheduleDispatch to be triggered (dispatchNext is called in the cycle)
await vi.waitFor(() => expect(mocks.dispatchManager.dispatchNext).toHaveBeenCalled());
expect(shouldRunQualityReview).not.toHaveBeenCalled();
expect(mocks.dispatchManager.completeTask).not.toHaveBeenCalled();
});
}); });

View File

@@ -18,12 +18,14 @@ import type { TaskRepository } from '../db/repositories/task-repository.js';
import type { InitiativeRepository } from '../db/repositories/initiative-repository.js'; import type { InitiativeRepository } from '../db/repositories/initiative-repository.js';
import type { ProjectRepository } from '../db/repositories/project-repository.js'; import type { ProjectRepository } from '../db/repositories/project-repository.js';
import type { AgentRepository } from '../db/repositories/agent-repository.js'; import type { AgentRepository } from '../db/repositories/agent-repository.js';
import type { AgentManager } from '../agent/types.js';
import type { DispatchManager, PhaseDispatchManager } from '../dispatch/types.js'; import type { DispatchManager, PhaseDispatchManager } from '../dispatch/types.js';
import type { ConflictResolutionService } from '../coordination/conflict-resolution-service.js'; import type { ConflictResolutionService } from '../coordination/conflict-resolution-service.js';
import { phaseBranchName, taskBranchName } from '../git/branch-naming.js'; import { phaseBranchName, taskBranchName } from '../git/branch-naming.js';
import { ensureProjectClone } from '../git/project-clones.js'; import { ensureProjectClone } from '../git/project-clones.js';
import { createModuleLogger } from '../logger/index.js'; import { createModuleLogger } from '../logger/index.js';
import { phaseMetaCache, fileDiffCache } from '../review/diff-cache.js'; import { phaseMetaCache, fileDiffCache } from '../review/diff-cache.js';
import { shouldRunQualityReview, runQualityReview } from './quality-review.js';
const log = createModuleLogger('execution-orchestrator'); const log = createModuleLogger('execution-orchestrator');
@@ -50,6 +52,7 @@ export class ExecutionOrchestrator {
private eventBus: EventBus, private eventBus: EventBus,
private workspaceRoot: string, private workspaceRoot: string,
private agentRepository?: AgentRepository, private agentRepository?: AgentRepository,
private agentManager?: AgentManager,
) {} ) {}
/** /**
@@ -108,15 +111,17 @@ export class ExecutionOrchestrator {
private async handleAgentStopped(event: AgentStoppedEvent): Promise<void> { private async handleAgentStopped(event: AgentStoppedEvent): Promise<void> {
const { taskId, reason, agentId } = event.payload; const { taskId, reason, agentId } = event.payload;
// Auto-complete task for successful agent completions, not manual stops
if (taskId && reason !== 'user_requested') { if (taskId && reason !== 'user_requested') {
try { try {
const result = await this.tryQualityReview(taskId, agentId, reason);
if (!result.reviewStarted) {
await this.dispatchManager.completeTask(taskId, agentId); await this.dispatchManager.completeTask(taskId, agentId);
log.info({ taskId, agentId, reason }, 'task auto-completed on agent stop'); log.info({ taskId, agentId, reason }, 'task auto-completed on agent stop');
}
} catch (err) { } catch (err) {
log.warn( log.warn(
{ taskId, agentId, reason, err: err instanceof Error ? err.message : String(err) }, { taskId, agentId, reason, err: err instanceof Error ? err.message : String(err) },
'failed to auto-complete task on agent stop', 'failed to handle agent stop',
); );
} }
} }
@@ -124,6 +129,72 @@ export class ExecutionOrchestrator {
this.scheduleDispatch(); this.scheduleDispatch();
} }
/**
* Attempt to run quality review for a stopping agent.
* Returns { reviewStarted: true } if quality review was initiated (callers must NOT call completeTask).
* Returns { reviewStarted: false } if no review needed (caller should call completeTask).
*/
private async tryQualityReview(taskId: string, agentId: string, reason: string): Promise<{ reviewStarted: boolean }> {
if (!this.agentRepository || !this.agentManager) {
return { reviewStarted: false };
}
const task = await this.taskRepository.findById(taskId);
if (!task?.phaseId || !task.initiativeId) {
return { reviewStarted: false };
}
const initiative = await this.initiativeRepository.findById(task.initiativeId);
if (!initiative?.branch) {
return { reviewStarted: false };
}
const phase = await this.phaseRepository.findById(task.phaseId);
if (!phase) {
return { reviewStarted: false };
}
const taskBranch = taskBranchName(initiative.branch, taskId);
const baseBranch = phaseBranchName(initiative.branch, phase.name);
const projects = await this.projectRepository.findProjectsByInitiativeId(task.initiativeId);
if (projects.length === 0) {
return { reviewStarted: false };
}
const repoPath = await ensureProjectClone(projects[0], this.workspaceRoot);
const result = await shouldRunQualityReview({
agentId,
taskId,
stopReason: reason,
repoPath,
taskBranch,
baseBranch,
agentRepository: this.agentRepository,
taskRepository: this.taskRepository,
initiativeRepository: this.initiativeRepository,
branchManager: this.branchManager,
});
if (!result.run) {
return { reviewStarted: false };
}
await runQualityReview({
taskId,
taskBranch,
baseBranch,
initiativeId: task.initiativeId,
qualifyingFiles: result.qualifyingFiles,
taskRepository: this.taskRepository,
agentManager: this.agentManager,
log,
});
return { reviewStarted: true };
}
private async handleAgentCrashed(event: AgentCrashedEvent): Promise<void> { private async handleAgentCrashed(event: AgentCrashedEvent): Promise<void> {
const { taskId, agentId, error } = event.payload; const { taskId, agentId, error } = event.payload;
if (!taskId) return; if (!taskId) return;

View File

@@ -0,0 +1,582 @@
/**
* Quality Review Service Tests
*/
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { computeQualifyingFiles, shouldRunQualityReview, runQualityReview } from './quality-review.js';
import type { BranchManager } from '../git/branch-manager.js';
import type { AgentRepository } from '../db/repositories/agent-repository.js';
import type { TaskRepository } from '../db/repositories/task-repository.js';
import type { InitiativeRepository } from '../db/repositories/initiative-repository.js';
import type { AgentManager } from '../agent/types.js';
function makeBranchManager(overrides: Partial<BranchManager> = {}): BranchManager {
return {
ensureBranch: vi.fn(),
mergeBranch: vi.fn(),
diffBranches: vi.fn(),
diffBranchesStat: vi.fn().mockResolvedValue([]),
diffFileSingle: vi.fn(),
getHeadCommitHash: vi.fn(),
deleteBranch: vi.fn(),
branchExists: vi.fn(),
remoteBranchExists: vi.fn(),
listCommits: vi.fn(),
diffCommit: vi.fn(),
getMergeBase: vi.fn(),
pushBranch: vi.fn(),
checkMergeability: vi.fn(),
fetchRemote: vi.fn(),
fastForwardBranch: vi.fn(),
updateRef: vi.fn(),
...overrides,
};
}
function makeAgentRepository(overrides: Partial<AgentRepository> = {}): AgentRepository {
return {
create: vi.fn(),
findById: vi.fn().mockResolvedValue(null),
findByName: vi.fn(),
findByTaskId: vi.fn(),
findBySessionId: vi.fn(),
findAll: vi.fn(),
findByStatus: vi.fn(),
update: vi.fn(),
delete: vi.fn(),
...overrides,
} as unknown as AgentRepository;
}
function makeTaskRepository(overrides: Partial<TaskRepository> = {}): TaskRepository {
return {
create: vi.fn(),
findById: vi.fn().mockResolvedValue(null),
findByParentTaskId: vi.fn(),
findByInitiativeId: vi.fn(),
findByPhaseId: vi.fn(),
update: vi.fn().mockResolvedValue({}),
delete: vi.fn(),
createDependency: vi.fn(),
getDependencies: vi.fn(),
...overrides,
} as unknown as TaskRepository;
}
function makeInitiativeRepository(overrides: Partial<InitiativeRepository> = {}): InitiativeRepository {
return {
create: vi.fn(),
findById: vi.fn().mockResolvedValue(null),
findAll: vi.fn(),
findByStatus: vi.fn(),
update: vi.fn(),
findByProjectId: vi.fn(),
delete: vi.fn(),
...overrides,
} as unknown as InitiativeRepository;
}
function makeAgentManager(overrides: Partial<AgentManager> = {}): AgentManager {
return {
spawn: vi.fn().mockResolvedValue({ id: 'review-agent-1' }),
stop: vi.fn(),
list: vi.fn(),
get: vi.fn(),
getByName: vi.fn(),
resume: vi.fn(),
getResult: vi.fn(),
getPendingQuestions: vi.fn(),
delete: vi.fn(),
dismiss: vi.fn(),
resumeForConversation: vi.fn(),
sendUserMessage: vi.fn(),
...overrides,
} as unknown as AgentManager;
}
// ---------------------------------------------------------------------------
// computeQualifyingFiles
// ---------------------------------------------------------------------------
describe('computeQualifyingFiles', () => {
it('includes .ts files', async () => {
const branchManager = makeBranchManager({
diffBranchesStat: vi.fn().mockResolvedValue([
{ path: 'src/foo.ts', status: 'modified', additions: 5, deletions: 2 },
]),
});
const result = await computeQualifyingFiles('/repo', 'task-branch', 'main', branchManager);
expect(result).toEqual(['src/foo.ts']);
});
it('includes .tsx, .js, .css, .json and other non-excluded types', async () => {
const branchManager = makeBranchManager({
diffBranchesStat: vi.fn().mockResolvedValue([
{ path: 'src/App.tsx', status: 'modified', additions: 1, deletions: 0 },
{ path: 'src/utils.js', status: 'added', additions: 10, deletions: 0 },
{ path: 'src/style.css', status: 'modified', additions: 3, deletions: 1 },
{ path: 'config.json', status: 'modified', additions: 2, deletions: 2 },
]),
});
const result = await computeQualifyingFiles('/repo', 'task-branch', 'main', branchManager);
expect(result).toEqual(['src/App.tsx', 'src/utils.js', 'src/style.css', 'config.json']);
});
it('excludes files ending with .gen.ts', async () => {
const branchManager = makeBranchManager({
diffBranchesStat: vi.fn().mockResolvedValue([
{ path: 'src/foo.ts', status: 'modified', additions: 5, deletions: 2 },
{ path: 'src/routes.gen.ts', status: 'modified', additions: 100, deletions: 0 },
{ path: 'types.gen.ts', status: 'added', additions: 50, deletions: 0 },
]),
});
const result = await computeQualifyingFiles('/repo', 'task-branch', 'main', branchManager);
expect(result).toEqual(['src/foo.ts']);
});
it('excludes files under dist/', async () => {
const branchManager = makeBranchManager({
diffBranchesStat: vi.fn().mockResolvedValue([
{ path: 'src/foo.ts', status: 'modified', additions: 5, deletions: 2 },
{ path: 'dist/bundle.js', status: 'modified', additions: 1, deletions: 0 },
{ path: 'apps/server/dist/index.js', status: 'added', additions: 10, deletions: 0 },
{ path: 'packages/foo/dist/foo.js', status: 'modified', additions: 3, deletions: 0 },
]),
});
const result = await computeQualifyingFiles('/repo', 'task-branch', 'main', branchManager);
expect(result).toEqual(['src/foo.ts']);
});
it('returns empty array when diff throws', async () => {
const branchManager = makeBranchManager({
diffBranchesStat: vi.fn().mockRejectedValue(new Error('branch not found')),
});
const result = await computeQualifyingFiles('/repo', 'task-branch', 'main', branchManager);
expect(result).toEqual([]);
});
it('passes baseBranch as first branch arg and taskBranch as second to diffBranchesStat', async () => {
const diffSpy = vi.fn().mockResolvedValue([]);
const branchManager = makeBranchManager({ diffBranchesStat: diffSpy });
await computeQualifyingFiles('/repo', 'task-branch', 'main', branchManager);
expect(diffSpy).toHaveBeenCalledWith('/repo', 'main', 'task-branch');
});
});
// ---------------------------------------------------------------------------
// shouldRunQualityReview
// ---------------------------------------------------------------------------
describe('shouldRunQualityReview', () => {
const BASE_PARAMS = {
agentId: 'agent-1',
taskId: 'task-1',
stopReason: 'task_complete',
repoPath: '/repo',
taskBranch: 'cw/init-task-task-1',
baseBranch: 'main',
};
it('returns false when stopReason is not task_complete', async () => {
const agentRepository = makeAgentRepository();
const taskRepository = makeTaskRepository();
const initiativeRepository = makeInitiativeRepository();
const branchManager = makeBranchManager();
const result = await shouldRunQualityReview({
...BASE_PARAMS,
stopReason: 'error',
agentRepository,
taskRepository,
initiativeRepository,
branchManager,
});
expect(result).toEqual({ run: false, qualifyingFiles: [] });
// Should short-circuit: no repo lookups
expect(agentRepository.findById).not.toHaveBeenCalled();
});
it('returns false when agent mode is errand', async () => {
const agentRepository = makeAgentRepository({
findById: vi.fn().mockResolvedValue({ id: 'agent-1', mode: 'errand' }),
});
const taskRepository = makeTaskRepository();
const initiativeRepository = makeInitiativeRepository();
const branchManager = makeBranchManager();
const result = await shouldRunQualityReview({
...BASE_PARAMS,
agentRepository,
taskRepository,
initiativeRepository,
branchManager,
});
expect(result).toEqual({ run: false, qualifyingFiles: [] });
expect(taskRepository.findById).not.toHaveBeenCalled();
});
it('returns false when agent is not found', async () => {
const agentRepository = makeAgentRepository({
findById: vi.fn().mockResolvedValue(null),
});
const taskRepository = makeTaskRepository();
const initiativeRepository = makeInitiativeRepository();
const branchManager = makeBranchManager();
const result = await shouldRunQualityReview({
...BASE_PARAMS,
agentRepository,
taskRepository,
initiativeRepository,
branchManager,
});
expect(result).toEqual({ run: false, qualifyingFiles: [] });
});
it('returns false when task status is quality_review (recursion guard)', async () => {
const agentRepository = makeAgentRepository({
findById: vi.fn().mockResolvedValue({ id: 'agent-1', mode: 'execute' }),
});
const taskRepository = makeTaskRepository({
findById: vi.fn().mockResolvedValue({
id: 'task-1',
status: 'quality_review',
initiativeId: 'init-1',
}),
});
const initiativeRepository = makeInitiativeRepository();
const branchManager = makeBranchManager();
const result = await shouldRunQualityReview({
...BASE_PARAMS,
agentRepository,
taskRepository,
initiativeRepository,
branchManager,
});
expect(result).toEqual({ run: false, qualifyingFiles: [] });
expect(initiativeRepository.findById).not.toHaveBeenCalled();
});
it('returns false when task status is not in_progress and not quality_review', async () => {
const agentRepository = makeAgentRepository({
findById: vi.fn().mockResolvedValue({ id: 'agent-1', mode: 'execute' }),
});
const taskRepository = makeTaskRepository({
findById: vi.fn().mockResolvedValue({
id: 'task-1',
status: 'completed',
initiativeId: 'init-1',
}),
});
const initiativeRepository = makeInitiativeRepository();
const branchManager = makeBranchManager();
const result = await shouldRunQualityReview({
...BASE_PARAMS,
agentRepository,
taskRepository,
initiativeRepository,
branchManager,
});
expect(result).toEqual({ run: false, qualifyingFiles: [] });
});
it('returns false when task is not found', async () => {
const agentRepository = makeAgentRepository({
findById: vi.fn().mockResolvedValue({ id: 'agent-1', mode: 'execute' }),
});
const taskRepository = makeTaskRepository({
findById: vi.fn().mockResolvedValue(null),
});
const initiativeRepository = makeInitiativeRepository();
const branchManager = makeBranchManager();
const result = await shouldRunQualityReview({
...BASE_PARAMS,
agentRepository,
taskRepository,
initiativeRepository,
branchManager,
});
expect(result).toEqual({ run: false, qualifyingFiles: [] });
});
it('returns false when task has no initiativeId', async () => {
const agentRepository = makeAgentRepository({
findById: vi.fn().mockResolvedValue({ id: 'agent-1', mode: 'execute' }),
});
const taskRepository = makeTaskRepository({
findById: vi.fn().mockResolvedValue({
id: 'task-1',
status: 'in_progress',
initiativeId: null,
}),
});
const initiativeRepository = makeInitiativeRepository();
const branchManager = makeBranchManager();
const result = await shouldRunQualityReview({
...BASE_PARAMS,
agentRepository,
taskRepository,
initiativeRepository,
branchManager,
});
expect(result).toEqual({ run: false, qualifyingFiles: [] });
expect(initiativeRepository.findById).not.toHaveBeenCalled();
});
it('returns false when initiative.qualityReview is false', async () => {
const agentRepository = makeAgentRepository({
findById: vi.fn().mockResolvedValue({ id: 'agent-1', mode: 'execute' }),
});
const taskRepository = makeTaskRepository({
findById: vi.fn().mockResolvedValue({
id: 'task-1',
status: 'in_progress',
initiativeId: 'init-1',
}),
});
const initiativeRepository = makeInitiativeRepository({
findById: vi.fn().mockResolvedValue({ id: 'init-1', qualityReview: false }),
});
const branchManager = makeBranchManager({
diffBranchesStat: vi.fn().mockResolvedValue([
{ path: 'src/foo.ts', status: 'modified', additions: 5, deletions: 2 },
]),
});
const result = await shouldRunQualityReview({
...BASE_PARAMS,
agentRepository,
taskRepository,
initiativeRepository,
branchManager,
});
expect(result).toEqual({ run: false, qualifyingFiles: [] });
expect(branchManager.diffBranchesStat).not.toHaveBeenCalled();
});
it('returns false when initiative is not found', async () => {
const agentRepository = makeAgentRepository({
findById: vi.fn().mockResolvedValue({ id: 'agent-1', mode: 'execute' }),
});
const taskRepository = makeTaskRepository({
findById: vi.fn().mockResolvedValue({
id: 'task-1',
status: 'in_progress',
initiativeId: 'init-1',
}),
});
const initiativeRepository = makeInitiativeRepository({
findById: vi.fn().mockResolvedValue(null),
});
const branchManager = makeBranchManager();
const result = await shouldRunQualityReview({
...BASE_PARAMS,
agentRepository,
taskRepository,
initiativeRepository,
branchManager,
});
expect(result).toEqual({ run: false, qualifyingFiles: [] });
});
it('returns false when no qualifying files in changeset', async () => {
const agentRepository = makeAgentRepository({
findById: vi.fn().mockResolvedValue({ id: 'agent-1', mode: 'execute' }),
});
const taskRepository = makeTaskRepository({
findById: vi.fn().mockResolvedValue({
id: 'task-1',
status: 'in_progress',
initiativeId: 'init-1',
}),
});
const initiativeRepository = makeInitiativeRepository({
findById: vi.fn().mockResolvedValue({ id: 'init-1', qualityReview: true }),
});
const branchManager = makeBranchManager({
diffBranchesStat: vi.fn().mockResolvedValue([
{ path: 'dist/bundle.js', status: 'modified', additions: 10, deletions: 0 },
{ path: 'src/routes.gen.ts', status: 'added', additions: 50, deletions: 0 },
]),
});
const result = await shouldRunQualityReview({
...BASE_PARAMS,
agentRepository,
taskRepository,
initiativeRepository,
branchManager,
});
expect(result).toEqual({ run: false, qualifyingFiles: [] });
});
it('returns true with qualifying files when all conditions pass', async () => {
const agentRepository = makeAgentRepository({
findById: vi.fn().mockResolvedValue({ id: 'agent-1', mode: 'execute' }),
});
const taskRepository = makeTaskRepository({
findById: vi.fn().mockResolvedValue({
id: 'task-1',
status: 'in_progress',
initiativeId: 'init-1',
}),
});
const initiativeRepository = makeInitiativeRepository({
findById: vi.fn().mockResolvedValue({ id: 'init-1', qualityReview: true }),
});
const branchManager = makeBranchManager({
diffBranchesStat: vi.fn().mockResolvedValue([
{ path: 'src/foo.ts', status: 'modified', additions: 5, deletions: 2 },
{ path: 'src/bar.ts', status: 'added', additions: 20, deletions: 0 },
]),
});
const result = await shouldRunQualityReview({
...BASE_PARAMS,
agentRepository,
taskRepository,
initiativeRepository,
branchManager,
});
expect(result).toEqual({ run: true, qualifyingFiles: ['src/foo.ts', 'src/bar.ts'] });
});
});
// ---------------------------------------------------------------------------
// runQualityReview
// ---------------------------------------------------------------------------
describe('runQualityReview', () => {
const BASE_RUN_PARAMS = {
taskId: 'task-1',
taskBranch: 'cw/init-task-task-1',
baseBranch: 'main',
initiativeId: 'init-1',
qualifyingFiles: ['src/foo.ts', 'src/bar.ts'],
};
const makeLog = () => ({
info: vi.fn(),
error: vi.fn(),
warn: vi.fn(),
debug: vi.fn(),
trace: vi.fn(),
fatal: vi.fn(),
child: vi.fn(),
});
it('transitions task status to quality_review before spawning', async () => {
const taskRepository = makeTaskRepository();
const agentManager = makeAgentManager();
const log = makeLog();
await runQualityReview({
...BASE_RUN_PARAMS,
taskRepository,
agentManager,
log: log as any,
});
const updateCalls = vi.mocked(taskRepository.update).mock.calls;
expect(updateCalls[0]).toEqual(['task-1', { status: 'quality_review' }]);
// spawn should come after the update
expect(agentManager.spawn).toHaveBeenCalled();
});
it('calls agentManager.spawn with mode execute and correct branchName', async () => {
const taskRepository = makeTaskRepository();
const agentManager = makeAgentManager();
const log = makeLog();
await runQualityReview({
...BASE_RUN_PARAMS,
taskRepository,
agentManager,
log: log as any,
});
expect(agentManager.spawn).toHaveBeenCalledWith(
expect.objectContaining({
taskId: 'task-1',
initiativeId: 'init-1',
mode: 'execute',
baseBranch: 'main',
branchName: 'cw/init-task-task-1',
}),
);
});
it('prompt includes /simplify instruction and qualifying files', async () => {
const taskRepository = makeTaskRepository();
const agentManager = makeAgentManager();
const log = makeLog();
await runQualityReview({
...BASE_RUN_PARAMS,
taskRepository,
agentManager,
log: log as any,
});
const spawnCall = vi.mocked(agentManager.spawn).mock.calls[0][0];
expect(spawnCall.prompt).toContain('/simplify');
expect(spawnCall.prompt).toContain('src/foo.ts');
expect(spawnCall.prompt).toContain('src/bar.ts');
});
it('logs reviewAgentId at info level after spawn', async () => {
const taskRepository = makeTaskRepository();
const agentManager = makeAgentManager({
spawn: vi.fn().mockResolvedValue({ id: 'review-agent-42' }),
});
const log = makeLog();
await runQualityReview({
...BASE_RUN_PARAMS,
taskRepository,
agentManager,
log: log as any,
});
expect(log.info).toHaveBeenCalledWith(
expect.objectContaining({ taskId: 'task-1', reviewAgentId: 'review-agent-42' }),
expect.any(String),
);
});
it('on spawn failure: transitions task to completed and does not throw', async () => {
const taskRepository = makeTaskRepository();
const agentManager = makeAgentManager({
spawn: vi.fn().mockRejectedValue(new Error('spawn failed')),
});
const log = makeLog();
await expect(
runQualityReview({
...BASE_RUN_PARAMS,
taskRepository,
agentManager,
log: log as any,
}),
).resolves.toBeUndefined();
expect(log.error).toHaveBeenCalled();
expect(taskRepository.update).toHaveBeenCalledWith('task-1', { status: 'completed' });
});
});

View File

@@ -0,0 +1,152 @@
/**
* Quality Review Service
*
* Decides whether to run a quality review after a task agent completes,
* and orchestrates spawning the review agent.
*
* All dependencies are passed as function parameters (hexagonal DI pattern).
*/
import type { BranchManager } from '../git/branch-manager.js';
import type { AgentRepository } from '../db/repositories/agent-repository.js';
import type { TaskRepository } from '../db/repositories/task-repository.js';
import type { InitiativeRepository } from '../db/repositories/initiative-repository.js';
import type { AgentManager } from '../agent/types.js';
import type { Logger } from 'pino';
// ---------------------------------------------------------------------------
// computeQualifyingFiles
// ---------------------------------------------------------------------------
/**
* Compute source files in the diff between taskBranch and baseBranch that
* qualify for a quality review (excludes *.gen.ts and dist/ paths).
*
* Returns [] if the diff throws (treated as no qualifying files).
*/
export async function computeQualifyingFiles(
repoPath: string,
taskBranch: string,
baseBranch: string,
branchManager: BranchManager,
): Promise<string[]> {
try {
const entries = await branchManager.diffBranchesStat(repoPath, baseBranch, taskBranch);
return entries
.map((e) => e.path)
.filter((p) => !p.endsWith('.gen.ts'))
.filter((p) => !p.startsWith('dist/') && !p.includes('/dist/'));
} catch {
return [];
}
}
// ---------------------------------------------------------------------------
// shouldRunQualityReview
// ---------------------------------------------------------------------------
export interface QualityReviewCheckParams {
agentId: string;
taskId: string;
stopReason: string;
repoPath: string;
taskBranch: string;
baseBranch: string;
agentRepository: AgentRepository;
taskRepository: TaskRepository;
initiativeRepository: InitiativeRepository;
branchManager: BranchManager;
}
/**
* Determine whether a quality review should be run for the given agent stop event.
*
* Returns { run: true, qualifyingFiles } only when all six conditions pass:
* 1. stopReason === 'task_complete'
* 2. Agent mode is 'execute'
* 3. Task status is 'in_progress' (not 'quality_review' — recursion guard)
* 4. task.initiativeId is non-null
* 5. initiative.qualityReview === true
* 6. computeQualifyingFiles() returns a non-empty array
*/
export async function shouldRunQualityReview(
params: QualityReviewCheckParams,
): Promise<{ run: boolean; qualifyingFiles: string[] }> {
const { agentId, taskId, stopReason, repoPath, taskBranch, baseBranch, agentRepository, taskRepository, initiativeRepository, branchManager } = params;
const NO = { run: false, qualifyingFiles: [] };
// 1. Only fire on task_complete
if (stopReason !== 'task_complete') return NO;
// 2. Agent mode must be 'execute'
const agent = await agentRepository.findById(agentId);
if (!agent || agent.mode !== 'execute') return NO;
// 3. Task status must be 'in_progress' (recursion guard: skip if already quality_review)
const task = await taskRepository.findById(taskId);
if (!task) return NO;
if (task.status === 'quality_review') return NO;
if (task.status !== 'in_progress') return NO;
// 4. Task must belong to an initiative
if (!task.initiativeId) return NO;
// 5. Initiative must have qualityReview enabled
const initiative = await initiativeRepository.findById(task.initiativeId);
if (!initiative || !initiative.qualityReview) return NO;
// 6. Must have qualifying files in the changeset
const qualifyingFiles = await computeQualifyingFiles(repoPath, taskBranch, baseBranch, branchManager);
if (qualifyingFiles.length === 0) return NO;
return { run: true, qualifyingFiles };
}
// ---------------------------------------------------------------------------
// runQualityReview
// ---------------------------------------------------------------------------
export interface QualityReviewRunParams {
taskId: string;
taskBranch: string;
baseBranch: string;
initiativeId: string;
qualifyingFiles: string[];
taskRepository: TaskRepository;
agentManager: AgentManager;
log: Logger;
}
/**
* Spawn a quality review agent on the task branch.
*
* 1. Transitions task to 'quality_review'
* 2. Builds /simplify prompt with qualifying files
* 3. Spawns execute-mode agent on the same task branch
* 4. Logs the review agent ID
* 5. On spawn error: logs and transitions task to 'completed' — never throws
*/
export async function runQualityReview(params: QualityReviewRunParams): Promise<void> {
const { taskId, taskBranch, baseBranch, initiativeId, qualifyingFiles, taskRepository, agentManager, log } = params;
await taskRepository.update(taskId, { status: 'quality_review' });
const fileList = qualifyingFiles.join('\n');
const prompt = `Run /simplify to review and fix code quality in this branch.\n\n${fileList}`;
try {
const reviewAgent = await agentManager.spawn({
taskId,
initiativeId,
prompt,
mode: 'execute',
baseBranch,
branchName: taskBranch,
});
log.info({ taskId, reviewAgentId: reviewAgent.id }, 'quality review agent spawned');
} catch (err) {
log.error({ taskId, err: err instanceof Error ? err.message : String(err) }, 'quality review agent spawn failed');
await taskRepository.update(taskId, { status: 'completed' });
}
}

View File

@@ -113,7 +113,7 @@ InitiativeChangesRequestedEvent { initiativeId, phaseId, taskId }
| Event | Action | | Event | Action |
|-------|--------| |-------|--------|
| `phase:queued` | Dispatch ready phases → dispatch their tasks to idle agents | | `phase:queued` | Dispatch ready phases → dispatch their tasks to idle agents |
| `agent:stopped` | Auto-complete task (unless user_requested), re-dispatch queued tasks (freed agent slot) | | `agent:stopped` | When `task_complete`: check `shouldRunQualityReview()` — if conditions met, spawn quality-review agent and set task to `quality_review`; otherwise auto-complete task. Manual stops (`user_requested`) are skipped. Re-dispatch queued tasks after either path. |
| `agent:crashed` | Auto-retry crashed task up to `MAX_TASK_RETRIES` (3). Increments `retryCount`, resets status to `pending`, re-queues. Exceeding retries leaves task `in_progress` for manual intervention. | | `agent:crashed` | Auto-retry crashed task up to `MAX_TASK_RETRIES` (3). Increments `retryCount`, resets status to `pending`, re-queues. Exceeding retries leaves task `in_progress` for manual intervention. |
| `task:completed` | Merge task branch (if branch exists), check phase completion, dispatch next queued task | | `task:completed` | Merge task branch (if branch exists), check phase completion, dispatch next queued task |