Merge branch 'cw/continuous-code-quality' into cw-merge-1772833031033
This commit is contained in:
@@ -52,6 +52,7 @@ describe('writeInputFiles', () => {
|
||||
status: 'active',
|
||||
branch: 'cw/test-initiative',
|
||||
executionMode: 'review_per_phase',
|
||||
qualityReview: false,
|
||||
createdAt: new Date('2026-01-01'),
|
||||
updatedAt: new Date('2026-01-02'),
|
||||
};
|
||||
|
||||
@@ -251,6 +251,7 @@ export async function createContainer(options?: ContainerOptions): Promise<Conta
|
||||
eventBus,
|
||||
workspaceRoot,
|
||||
repos.agentRepository,
|
||||
agentManager,
|
||||
);
|
||||
executionOrchestrator.start();
|
||||
log.info('execution orchestrator started');
|
||||
|
||||
@@ -147,4 +147,32 @@ describe('DrizzleInitiativeRepository', () => {
|
||||
expect(archived[0].name).toBe('Archived');
|
||||
});
|
||||
});
|
||||
|
||||
describe('qualityReview', () => {
|
||||
it('defaults to false on create', async () => {
|
||||
const initiative = await repo.create({ name: 'QR Test' });
|
||||
expect(initiative.qualityReview).toBe(false);
|
||||
});
|
||||
|
||||
it('can be set to true via update', async () => {
|
||||
const created = await repo.create({ name: 'QR Toggle' });
|
||||
const updated = await repo.update(created.id, { qualityReview: true });
|
||||
expect(updated.qualityReview).toBe(true);
|
||||
});
|
||||
|
||||
it('round-trips through findById', async () => {
|
||||
const created = await repo.create({ name: 'QR Round-trip' });
|
||||
await repo.update(created.id, { qualityReview: true });
|
||||
const found = await repo.findById(created.id);
|
||||
expect(found!.qualityReview).toBe(true);
|
||||
});
|
||||
|
||||
it('round-trips false value', async () => {
|
||||
const created = await repo.create({ name: 'QR False' });
|
||||
await repo.update(created.id, { qualityReview: true });
|
||||
await repo.update(created.id, { qualityReview: false });
|
||||
const found = await repo.findById(created.id);
|
||||
expect(found!.qualityReview).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -196,4 +196,38 @@ describe('DrizzleTaskRepository', () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('quality_review status', () => {
|
||||
it('can set status to quality_review', async () => {
|
||||
const task = await taskRepo.create({
|
||||
phaseId: testPhaseId,
|
||||
name: 'QR Status Task',
|
||||
order: 99,
|
||||
});
|
||||
const updated = await taskRepo.update(task.id, { status: 'quality_review' });
|
||||
expect(updated.status).toBe('quality_review');
|
||||
});
|
||||
|
||||
it('round-trips quality_review through findById', async () => {
|
||||
const task = await taskRepo.create({
|
||||
phaseId: testPhaseId,
|
||||
name: 'QR Round-trip Task',
|
||||
order: 100,
|
||||
});
|
||||
await taskRepo.update(task.id, { status: 'quality_review' });
|
||||
const found = await taskRepo.findById(task.id);
|
||||
expect(found!.status).toBe('quality_review');
|
||||
});
|
||||
|
||||
it('can transition from quality_review to completed', async () => {
|
||||
const task = await taskRepo.create({
|
||||
phaseId: testPhaseId,
|
||||
name: 'QR to Completed',
|
||||
order: 101,
|
||||
});
|
||||
await taskRepo.update(task.id, { status: 'quality_review' });
|
||||
const completed = await taskRepo.update(task.id, { status: 'completed' });
|
||||
expect(completed.status).toBe('completed');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -26,6 +26,7 @@ export const initiatives = sqliteTable('initiatives', {
|
||||
executionMode: text('execution_mode', { enum: ['yolo', 'review_per_phase'] })
|
||||
.notNull()
|
||||
.default('review_per_phase'),
|
||||
qualityReview: integer('quality_review', { mode: 'boolean' }).notNull().default(false),
|
||||
createdAt: integer('created_at', { mode: 'timestamp' }).notNull(),
|
||||
updatedAt: integer('updated_at', { mode: 'timestamp' }).notNull(),
|
||||
});
|
||||
@@ -151,7 +152,7 @@ export const tasks = sqliteTable('tasks', {
|
||||
.notNull()
|
||||
.default('medium'),
|
||||
status: text('status', {
|
||||
enum: ['pending', 'in_progress', 'completed', 'blocked'],
|
||||
enum: ['pending', 'in_progress', 'quality_review', 'completed', 'blocked'],
|
||||
})
|
||||
.notNull()
|
||||
.default('pending'),
|
||||
|
||||
1
apps/server/drizzle/0037_worthless_princess_powerful.sql
Normal file
1
apps/server/drizzle/0037_worthless_princess_powerful.sql
Normal file
@@ -0,0 +1 @@
|
||||
ALTER TABLE `initiatives` ADD `quality_review` integer DEFAULT false NOT NULL;
|
||||
1
apps/server/drizzle/0038_worthless_princess_powerful.sql
Normal file
1
apps/server/drizzle/0038_worthless_princess_powerful.sql
Normal file
@@ -0,0 +1 @@
|
||||
ALTER TABLE `initiatives` ADD `quality_review` integer DEFAULT false NOT NULL;
|
||||
@@ -238,6 +238,13 @@
|
||||
"notNull": false,
|
||||
"autoincrement": false
|
||||
},
|
||||
"prompt": {
|
||||
"name": "prompt",
|
||||
"type": "text",
|
||||
"primaryKey": false,
|
||||
"notNull": false,
|
||||
"autoincrement": false
|
||||
},
|
||||
"exit_code": {
|
||||
"name": "exit_code",
|
||||
"type": "integer",
|
||||
|
||||
2037
apps/server/drizzle/meta/0038_snapshot.json
Normal file
2037
apps/server/drizzle/meta/0038_snapshot.json
Normal file
File diff suppressed because it is too large
Load Diff
@@ -267,6 +267,13 @@
|
||||
"when": 1772828694292,
|
||||
"tag": "0037_eager_devos",
|
||||
"breakpoints": true
|
||||
},
|
||||
{
|
||||
"idx": 38,
|
||||
"version": "6",
|
||||
"when": 1772829916655,
|
||||
"tag": "0038_worthless_princess_powerful",
|
||||
"breakpoints": true
|
||||
}
|
||||
]
|
||||
}
|
||||
@@ -8,17 +8,26 @@ 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';
|
||||
import type { AgentManager } from '../agent/types.js';
|
||||
import type { AgentRepository } from '../db/repositories/agent-repository.js';
|
||||
|
||||
vi.mock('../git/project-clones.js', () => ({
|
||||
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 { 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';
|
||||
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[] } {
|
||||
const handlers = new Map<string, Function[]>();
|
||||
@@ -108,6 +117,33 @@ function createMocks() {
|
||||
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();
|
||||
|
||||
return {
|
||||
@@ -119,11 +155,13 @@ function createMocks() {
|
||||
phaseDispatchManager,
|
||||
dispatchManager,
|
||||
conflictResolutionService,
|
||||
agentRepository,
|
||||
agentManager,
|
||||
eventBus,
|
||||
};
|
||||
}
|
||||
|
||||
function createOrchestrator(mocks: ReturnType<typeof createMocks>) {
|
||||
function createOrchestrator(mocks: ReturnType<typeof createMocks>, opts: { withAgentManager?: boolean; withAgentRepository?: boolean } = {}) {
|
||||
const orchestrator = new ExecutionOrchestrator(
|
||||
mocks.branchManager,
|
||||
mocks.phaseRepository,
|
||||
@@ -135,6 +173,8 @@ function createOrchestrator(mocks: ReturnType<typeof createMocks>) {
|
||||
mocks.conflictResolutionService,
|
||||
mocks.eventBus,
|
||||
'/tmp/test-workspace',
|
||||
opts.withAgentRepository !== false ? mocks.agentRepository : undefined,
|
||||
opts.withAgentManager !== false ? mocks.agentManager : undefined,
|
||||
);
|
||||
orchestrator.start();
|
||||
return orchestrator;
|
||||
@@ -369,4 +409,176 @@ describe('ExecutionOrchestrator', () => {
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -18,12 +18,14 @@ 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 { AgentRepository } from '../db/repositories/agent-repository.js';
|
||||
import type { AgentManager } from '../agent/types.js';
|
||||
import type { DispatchManager, PhaseDispatchManager } from '../dispatch/types.js';
|
||||
import type { ConflictResolutionService } from '../coordination/conflict-resolution-service.js';
|
||||
import { phaseBranchName, taskBranchName, isPlanningCategory } from '../git/branch-naming.js';
|
||||
import { ensureProjectClone } from '../git/project-clones.js';
|
||||
import { createModuleLogger } from '../logger/index.js';
|
||||
import { phaseMetaCache, fileDiffCache } from '../review/diff-cache.js';
|
||||
import { shouldRunQualityReview, runQualityReview } from './quality-review.js';
|
||||
|
||||
const log = createModuleLogger('execution-orchestrator');
|
||||
|
||||
@@ -50,6 +52,7 @@ export class ExecutionOrchestrator {
|
||||
private eventBus: EventBus,
|
||||
private workspaceRoot: string,
|
||||
private agentRepository?: AgentRepository,
|
||||
private agentManager?: AgentManager,
|
||||
) {}
|
||||
|
||||
/**
|
||||
@@ -108,15 +111,17 @@ export class ExecutionOrchestrator {
|
||||
private async handleAgentStopped(event: AgentStoppedEvent): Promise<void> {
|
||||
const { taskId, reason, agentId } = event.payload;
|
||||
|
||||
// Auto-complete task for successful agent completions, not manual stops
|
||||
if (taskId && reason !== 'user_requested') {
|
||||
try {
|
||||
await this.dispatchManager.completeTask(taskId, agentId);
|
||||
log.info({ taskId, agentId, reason }, 'task auto-completed on agent stop');
|
||||
const result = await this.tryQualityReview(taskId, agentId, reason);
|
||||
if (!result.reviewStarted) {
|
||||
await this.dispatchManager.completeTask(taskId, agentId);
|
||||
log.info({ taskId, agentId, reason }, 'task auto-completed on agent stop');
|
||||
}
|
||||
} catch (err) {
|
||||
log.warn(
|
||||
{ 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();
|
||||
}
|
||||
|
||||
/**
|
||||
* 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> {
|
||||
const { taskId, agentId, error } = event.payload;
|
||||
if (!taskId) return;
|
||||
|
||||
582
apps/server/execution/quality-review.test.ts
Normal file
582
apps/server/execution/quality-review.test.ts
Normal 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' });
|
||||
});
|
||||
});
|
||||
152
apps/server/execution/quality-review.ts
Normal file
152
apps/server/execution/quality-review.ts
Normal 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' });
|
||||
}
|
||||
}
|
||||
97
apps/server/trpc/routers/initiative.test.ts
Normal file
97
apps/server/trpc/routers/initiative.test.ts
Normal file
@@ -0,0 +1,97 @@
|
||||
/**
|
||||
* Integration tests for initiative tRPC router — qualityReview field.
|
||||
*
|
||||
* Verifies that updateInitiativeConfig accepts and persists qualityReview.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import { router, publicProcedure, createCallerFactory } from '../trpc.js';
|
||||
import { initiativeProcedures } from './initiative.js';
|
||||
import type { TRPCContext } from '../context.js';
|
||||
import { createTestDatabase } from '../../db/repositories/drizzle/test-helpers.js';
|
||||
import { DrizzleInitiativeRepository } from '../../db/repositories/drizzle/index.js';
|
||||
|
||||
// =============================================================================
|
||||
// Mock ensureProjectClone — prevents actual git cloning
|
||||
// =============================================================================
|
||||
|
||||
vi.mock('../../git/project-clones.js', () => ({
|
||||
ensureProjectClone: vi.fn().mockResolvedValue('/fake/clone/path'),
|
||||
getProjectCloneDir: vi.fn().mockReturnValue('repos/fake-project-id'),
|
||||
}));
|
||||
|
||||
// =============================================================================
|
||||
// Test router
|
||||
// =============================================================================
|
||||
|
||||
const testRouter = router({
|
||||
...initiativeProcedures(publicProcedure),
|
||||
});
|
||||
|
||||
const createCaller = createCallerFactory(testRouter);
|
||||
|
||||
// =============================================================================
|
||||
// Setup helper
|
||||
// =============================================================================
|
||||
|
||||
function createMockEventBus(): TRPCContext['eventBus'] {
|
||||
return {
|
||||
emit: vi.fn(),
|
||||
on: vi.fn(),
|
||||
off: vi.fn(),
|
||||
once: vi.fn(),
|
||||
};
|
||||
}
|
||||
|
||||
async function setup() {
|
||||
const db = createTestDatabase();
|
||||
const initiativeRepo = new DrizzleInitiativeRepository(db);
|
||||
const ctx: TRPCContext = {
|
||||
eventBus: createMockEventBus(),
|
||||
serverStartedAt: null,
|
||||
processCount: 0,
|
||||
initiativeRepository: initiativeRepo,
|
||||
};
|
||||
const caller = createCaller(ctx);
|
||||
const initiative = await initiativeRepo.create({ name: 'Test Initiative' });
|
||||
return { caller, initiativeRepo, initiative };
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
// Tests
|
||||
// =============================================================================
|
||||
|
||||
describe('updateInitiativeConfig — qualityReview', () => {
|
||||
it('sets qualityReview to true', async () => {
|
||||
const { caller, initiative } = await setup();
|
||||
const result = await caller.updateInitiativeConfig({
|
||||
initiativeId: initiative.id,
|
||||
qualityReview: true,
|
||||
});
|
||||
expect(result.qualityReview).toBe(true);
|
||||
});
|
||||
|
||||
it('sets qualityReview to false', async () => {
|
||||
const { caller, initiative, initiativeRepo } = await setup();
|
||||
// First set it to true
|
||||
await initiativeRepo.update(initiative.id, { qualityReview: true });
|
||||
// Now flip it back
|
||||
const result = await caller.updateInitiativeConfig({
|
||||
initiativeId: initiative.id,
|
||||
qualityReview: false,
|
||||
});
|
||||
expect(result.qualityReview).toBe(false);
|
||||
});
|
||||
|
||||
it('does not change qualityReview when omitted', async () => {
|
||||
const { caller, initiative, initiativeRepo } = await setup();
|
||||
// Set to true first
|
||||
await initiativeRepo.update(initiative.id, { qualityReview: true });
|
||||
// Update without qualityReview
|
||||
const result = await caller.updateInitiativeConfig({
|
||||
initiativeId: initiative.id,
|
||||
executionMode: 'yolo',
|
||||
});
|
||||
expect(result.qualityReview).toBe(true); // unchanged
|
||||
});
|
||||
});
|
||||
@@ -221,6 +221,7 @@ export function initiativeProcedures(publicProcedure: ProcedureBuilder) {
|
||||
initiativeId: z.string().min(1),
|
||||
executionMode: z.enum(['yolo', 'review_per_phase']).optional(),
|
||||
branch: z.string().nullable().optional(),
|
||||
qualityReview: z.boolean().optional(),
|
||||
}))
|
||||
.mutation(async ({ ctx, input }) => {
|
||||
const repo = requireInitiativeRepository(ctx);
|
||||
|
||||
Reference in New Issue
Block a user