feat: add quality-review service with qualifying file detection and agent spawning
Adds apps/server/execution/quality-review.ts with three exported functions:
- computeQualifyingFiles: diffs task branch vs base, filters out *.gen.ts and dist/ paths
- shouldRunQualityReview: evaluates all six guard conditions (task_complete, execute mode,
in_progress status, initiative membership, qualityReview flag, non-empty changeset)
and returns { run, qualifyingFiles } to avoid recomputing the diff in the orchestrator
- runQualityReview: transitions task to quality_review, spawns execute-mode review agent
on the task branch, logs the review agent ID, and falls back to completed on spawn failure
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
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' });
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user