diff --git a/apps/server/container.ts b/apps/server/container.ts index daa6151..bfa0c55 100644 --- a/apps/server/container.ts +++ b/apps/server/container.ts @@ -250,6 +250,7 @@ export async function createContainer(options?: ContainerOptions): Promise ({ ensureProjectClone: vi.fn().mockResolvedValue('/tmp/test-workspace/clones/test'), })); + +vi.mock('./quality-review.js', () => ({ + shouldRunQualityReview: vi.fn(), + runQualityReview: vi.fn(), + computeQualifyingFiles: vi.fn(), +})); 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'; @@ -110,6 +117,23 @@ function createMocks() { const eventBus = createMockEventBus(); + const agentManager = { + spawn: vi.fn().mockResolvedValue({ id: 'review-agent-1' }), + stop: vi.fn(), + list: vi.fn(), + resume: vi.fn(), + delete: vi.fn(), + }; + + const agentRepository = { + findById: vi.fn().mockResolvedValue({ id: 'a1', mode: 'execute' }), + findByTaskId: vi.fn().mockResolvedValue(null), + findAll: vi.fn().mockResolvedValue([]), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + }; + return { branchManager, phaseRepository, @@ -120,6 +144,8 @@ function createMocks() { dispatchManager, conflictResolutionService, eventBus, + agentManager, + agentRepository, }; } @@ -135,6 +161,8 @@ function createOrchestrator(mocks: ReturnType) { mocks.conflictResolutionService, mocks.eventBus, '/tmp/test-workspace', + mocks.agentManager as any, + mocks.agentRepository as any, ); orchestrator.start(); return orchestrator; @@ -370,3 +398,87 @@ describe('ExecutionOrchestrator', () => { }); }); }); + +describe('handleAgentStopped — quality review integration', () => { + let mocks: ReturnType; + + 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); + + 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(); + }); +}); diff --git a/apps/server/execution/orchestrator.ts b/apps/server/execution/orchestrator.ts index 138aaa3..5cf9581 100644 --- a/apps/server/execution/orchestrator.ts +++ b/apps/server/execution/orchestrator.ts @@ -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 } 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'); @@ -49,6 +51,7 @@ export class ExecutionOrchestrator { private conflictResolutionService: ConflictResolutionService, private eventBus: EventBus, private workspaceRoot: string, + private agentManager: AgentManager, private agentRepository?: AgentRepository, ) {} @@ -108,15 +111,53 @@ export class ExecutionOrchestrator { private async handleAgentStopped(event: AgentStoppedEvent): Promise { 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'); + if (!this.agentRepository) { + // No agent repository — skip quality review, complete task directly + log.warn({ taskId, agentId }, 'agentRepository not available; skipping quality review'); + await this.dispatchManager.completeTask(taskId, agentId); + log.info({ taskId, agentId, reason }, 'task auto-completed on agent stop'); + } else { + // Get repoPath from first project in initiative (for branch diffing) + const repoPath = await this.getRepoPathForTask(taskId); + + const result = await shouldRunQualityReview({ + agentId, + taskId, + stopReason: reason, + agentRepository: this.agentRepository, + taskRepository: this.taskRepository, + initiativeRepository: this.initiativeRepository, + phaseRepository: this.phaseRepository, + branchManager: this.branchManager, + repoPath, + }); + + if (result.run) { + const task = await this.taskRepository.findById(taskId); + const initiative = await this.initiativeRepository.findById(task!.initiativeId!); + const phase = await this.phaseRepository.findById(task!.phaseId!); + const initBranch = initiative!.branch!; + await runQualityReview({ + taskId, + taskBranch: taskBranchName(initBranch, taskId), + baseBranch: phaseBranchName(initBranch, phase!.name), + initiativeId: task!.initiativeId!, + qualifyingFiles: result.qualifyingFiles, + taskRepository: this.taskRepository, + agentManager: this.agentManager, + log, + }); + } else { + 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 +165,14 @@ export class ExecutionOrchestrator { this.scheduleDispatch(); } + private async getRepoPathForTask(taskId: string): Promise { + const task = await this.taskRepository.findById(taskId); + if (!task?.initiativeId) return this.workspaceRoot; + const projects = await this.projectRepository.findProjectsByInitiativeId(task.initiativeId); + if (!projects.length) return this.workspaceRoot; + return ensureProjectClone(projects[0], this.workspaceRoot); + } + private async handleAgentCrashed(event: AgentCrashedEvent): Promise { const { taskId, agentId, error } = event.payload; if (!taskId) return; diff --git a/apps/server/execution/quality-review.test.ts b/apps/server/execution/quality-review.test.ts new file mode 100644 index 0000000..3aec70b --- /dev/null +++ b/apps/server/execution/quality-review.test.ts @@ -0,0 +1,434 @@ +/** + * Quality Review Tests + * + * Tests for the quality-review dispatch hook that intercepts agent:stopped + * events and spawns a review agent when conditions are met. + */ + +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 { PhaseRepository } from '../db/repositories/phase-repository.js'; +import type { AgentManager } from '../agent/types.js'; +import type { createModuleLogger } from '../logger/index.js'; + +type Logger = ReturnType; + +// --------------------------------------------------------------------------- +// Mock helpers +// --------------------------------------------------------------------------- + +function createBranchManagerMock(): 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(), + } as unknown as BranchManager; +} + +function createAgentRepositoryMock(): AgentRepository { + return { + findById: vi.fn().mockResolvedValue({ id: 'a1', mode: 'execute' }), + findByTaskId: vi.fn(), + findAll: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + } as unknown as AgentRepository; +} + +function createTaskRepositoryMock(): TaskRepository { + return { + findById: vi.fn().mockResolvedValue({ + id: 't1', + status: 'in_progress', + initiativeId: 'i1', + phaseId: 'p1', + }), + findByPhaseId: vi.fn(), + findByInitiativeId: vi.fn(), + create: vi.fn(), + update: vi.fn().mockResolvedValue(undefined), + delete: vi.fn(), + } as unknown as TaskRepository; +} + +function createInitiativeRepositoryMock(): InitiativeRepository { + return { + findById: vi.fn().mockResolvedValue({ + id: 'i1', + qualityReview: true, + branch: 'cw/test', + }), + findAll: vi.fn(), + findByStatus: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + } as unknown as InitiativeRepository; +} + +function createPhaseRepositoryMock(): PhaseRepository { + return { + findById: vi.fn().mockResolvedValue({ id: 'p1', name: 'impl', initiativeId: 'i1' }), + findByInitiativeId: vi.fn(), + create: vi.fn(), + update: vi.fn(), + delete: vi.fn(), + } as unknown as PhaseRepository; +} + +function createAgentManagerMock(): AgentManager { + return { + spawn: vi.fn().mockResolvedValue({ id: 'review-agent-1' }), + stop: vi.fn(), + list: vi.fn(), + resume: vi.fn(), + delete: vi.fn(), + getStatus: vi.fn(), + answerQuestion: vi.fn(), + spawnWithLifecycle: vi.fn(), + } as unknown as AgentManager; +} + +function createLoggerMock(): Logger { + return { + info: vi.fn(), + error: vi.fn(), + warn: vi.fn(), + debug: vi.fn(), + trace: vi.fn(), + fatal: vi.fn(), + child: vi.fn(), + } as unknown as Logger; +} + +// --------------------------------------------------------------------------- +// computeQualifyingFiles +// --------------------------------------------------------------------------- + +describe('computeQualifyingFiles', () => { + let branchManager: BranchManager; + + beforeEach(() => { + branchManager = createBranchManagerMock(); + }); + + it('includes .ts files', async () => { + vi.mocked(branchManager.diffBranchesStat).mockResolvedValue([ + { path: 'src/foo.ts', status: 'modified', additions: 5, deletions: 2 }, + ]); + expect(await computeQualifyingFiles(branchManager, '/repo', 'task-branch', 'base')).toEqual(['src/foo.ts']); + }); + + it('includes .tsx and .js files', async () => { + vi.mocked(branchManager.diffBranchesStat).mockResolvedValue([ + { path: 'src/Comp.tsx', status: 'modified', additions: 3, deletions: 1 }, + { path: 'src/util.js', status: 'added', additions: 10, deletions: 0 }, + ]); + const result = await computeQualifyingFiles(branchManager, '/repo', 'task-branch', 'base'); + expect(result).toContain('src/Comp.tsx'); + expect(result).toContain('src/util.js'); + }); + + it('excludes *.gen.ts files', async () => { + vi.mocked(branchManager.diffBranchesStat).mockResolvedValue([ + { path: 'src/routeTree.gen.ts', status: 'modified', additions: 1, deletions: 0 }, + ]); + expect(await computeQualifyingFiles(branchManager, '/repo', 'branch', 'base')).toEqual([]); + }); + + it('excludes files starting with dist/', async () => { + vi.mocked(branchManager.diffBranchesStat).mockResolvedValue([ + { path: 'dist/index.js', status: 'modified', additions: 1, deletions: 0 }, + ]); + expect(await computeQualifyingFiles(branchManager, '/repo', 'branch', 'base')).toEqual([]); + }); + + it('excludes files containing /dist/', async () => { + vi.mocked(branchManager.diffBranchesStat).mockResolvedValue([ + { path: 'packages/foo/dist/bar.js', status: 'modified', additions: 1, deletions: 0 }, + ]); + expect(await computeQualifyingFiles(branchManager, '/repo', 'branch', 'base')).toEqual([]); + }); + + it('returns empty array when diffBranchesStat throws', async () => { + vi.mocked(branchManager.diffBranchesStat).mockRejectedValue(new Error('branch not found')); + expect(await computeQualifyingFiles(branchManager, '/repo', 'branch', 'base')).toEqual([]); + }); + + it('returns only qualifying files from a mixed set', async () => { + vi.mocked(branchManager.diffBranchesStat).mockResolvedValue([ + { path: 'src/foo.ts', status: 'modified', additions: 5, deletions: 2 }, + { path: 'src/routeTree.gen.ts', status: 'modified', additions: 1, deletions: 0 }, + { path: 'dist/bundle.js', status: 'modified', additions: 1, deletions: 0 }, + { path: 'src/bar.tsx', status: 'added', additions: 10, deletions: 0 }, + { path: 'README.md', status: 'modified', additions: 2, deletions: 0 }, + ]); + const result = await computeQualifyingFiles(branchManager, '/repo', 'branch', 'base'); + expect(result).toEqual(['src/foo.ts', 'src/bar.tsx']); + }); +}); + +// --------------------------------------------------------------------------- +// shouldRunQualityReview +// --------------------------------------------------------------------------- + +describe('shouldRunQualityReview', () => { + let branchManager: BranchManager; + let agentRepository: AgentRepository; + let taskRepository: TaskRepository; + let initiativeRepository: InitiativeRepository; + let phaseRepository: PhaseRepository; + + // Base params where all conditions pass + let params: Parameters[0]; + + beforeEach(() => { + branchManager = createBranchManagerMock(); + agentRepository = createAgentRepositoryMock(); + taskRepository = createTaskRepositoryMock(); + initiativeRepository = createInitiativeRepositoryMock(); + phaseRepository = createPhaseRepositoryMock(); + + // Default diffBranchesStat returns qualifying file + vi.mocked(branchManager.diffBranchesStat).mockResolvedValue([ + { path: 'src/foo.ts', status: 'modified', additions: 5, deletions: 2 }, + ]); + + params = { + agentId: 'a1', + taskId: 't1', + stopReason: 'task_complete', + agentRepository, + taskRepository, + initiativeRepository, + phaseRepository, + branchManager, + repoPath: '/repo', + }; + }); + + it('returns false when stopReason is not task_complete', async () => { + const result = await shouldRunQualityReview({ ...params, stopReason: 'error' }); + expect(result).toEqual({ run: false, qualifyingFiles: [] }); + }); + + it('returns false when agent is not found', async () => { + vi.mocked(agentRepository.findById).mockResolvedValue(undefined as any); + const result = await shouldRunQualityReview(params); + expect(result).toEqual({ run: false, qualifyingFiles: [] }); + }); + + it('returns false when agent mode is errand', async () => { + vi.mocked(agentRepository.findById).mockResolvedValue({ id: 'a1', mode: 'errand' } as any); + const result = await shouldRunQualityReview(params); + expect(result).toEqual({ run: false, qualifyingFiles: [] }); + }); + + it('returns false when task is not found', async () => { + vi.mocked(taskRepository.findById).mockResolvedValue(undefined as any); + const result = await shouldRunQualityReview(params); + expect(result).toEqual({ run: false, qualifyingFiles: [] }); + }); + + it('returns false when task status is quality_review (recursion guard)', async () => { + vi.mocked(taskRepository.findById).mockResolvedValue({ + id: 't1', + status: 'quality_review', + initiativeId: 'i1', + phaseId: 'p1', + } as any); + const result = await shouldRunQualityReview(params); + expect(result).toEqual({ run: false, qualifyingFiles: [] }); + }); + + it('returns false when task status is not in_progress', async () => { + vi.mocked(taskRepository.findById).mockResolvedValue({ + id: 't1', + status: 'pending', + initiativeId: 'i1', + phaseId: 'p1', + } as any); + const result = await shouldRunQualityReview(params); + expect(result).toEqual({ run: false, qualifyingFiles: [] }); + }); + + it('returns false when task has no initiativeId', async () => { + vi.mocked(taskRepository.findById).mockResolvedValue({ + id: 't1', + status: 'in_progress', + initiativeId: null, + phaseId: 'p1', + } as any); + const result = await shouldRunQualityReview(params); + expect(result).toEqual({ run: false, qualifyingFiles: [] }); + }); + + it('returns false when initiative is not found', async () => { + vi.mocked(initiativeRepository.findById).mockResolvedValue(undefined as any); + const result = await shouldRunQualityReview(params); + expect(result).toEqual({ run: false, qualifyingFiles: [] }); + }); + + it('returns false when initiative qualityReview is false', async () => { + vi.mocked(initiativeRepository.findById).mockResolvedValue({ + id: 'i1', + qualityReview: false, + branch: 'cw/test', + } as any); + const result = await shouldRunQualityReview(params); + expect(result).toEqual({ run: false, qualifyingFiles: [] }); + }); + + it('returns false when task has no phaseId', async () => { + vi.mocked(taskRepository.findById).mockResolvedValue({ + id: 't1', + status: 'in_progress', + initiativeId: 'i1', + phaseId: null, + } as any); + const result = await shouldRunQualityReview(params); + expect(result).toEqual({ run: false, qualifyingFiles: [] }); + }); + + it('returns false when phase is not found', async () => { + vi.mocked(phaseRepository.findById).mockResolvedValue(undefined as any); + const result = await shouldRunQualityReview(params); + expect(result).toEqual({ run: false, qualifyingFiles: [] }); + }); + + it('returns false when initiative has no branch', async () => { + vi.mocked(initiativeRepository.findById).mockResolvedValue({ + id: 'i1', + qualityReview: true, + branch: null, + } as any); + const result = await shouldRunQualityReview(params); + expect(result).toEqual({ run: false, qualifyingFiles: [] }); + }); + + it('returns false when no qualifying files in changeset', async () => { + vi.mocked(branchManager.diffBranchesStat).mockResolvedValue([ + { path: 'src/routeTree.gen.ts', status: 'modified', additions: 1, deletions: 0 }, + ]); + const result = await shouldRunQualityReview(params); + expect(result).toEqual({ run: false, qualifyingFiles: [] }); + }); + + it('returns true with qualifying files when all conditions pass', async () => { + const result = await shouldRunQualityReview(params); + expect(result.run).toBe(true); + expect(result.qualifyingFiles).toContain('src/foo.ts'); + }); +}); + +// --------------------------------------------------------------------------- +// runQualityReview +// --------------------------------------------------------------------------- + +describe('runQualityReview', () => { + let taskRepository: TaskRepository; + let agentManager: AgentManager; + let log: Logger; + + let params: Parameters[0]; + + beforeEach(() => { + taskRepository = createTaskRepositoryMock(); + agentManager = createAgentManagerMock(); + log = createLoggerMock(); + + params = { + taskId: 't1', + taskBranch: 'cw/test-task-t1', + baseBranch: 'cw/test-phase-impl', + initiativeId: 'i1', + qualifyingFiles: ['src/foo.ts', 'src/bar.ts'], + taskRepository, + agentManager, + log, + }; + }); + + it('transitions task to quality_review before spawning', async () => { + await runQualityReview(params); + expect(taskRepository.update).toHaveBeenCalledWith('t1', { status: 'quality_review' }); + // update called BEFORE spawn + const updateOrder = vi.mocked(taskRepository.update).mock.invocationCallOrder[0]!; + const spawnOrder = vi.mocked(agentManager.spawn).mock.invocationCallOrder[0]!; + expect(updateOrder).toBeLessThan(spawnOrder); + }); + + it('spawns agent with mode execute on the task branch', async () => { + await runQualityReview(params); + expect(agentManager.spawn).toHaveBeenCalledWith( + expect.objectContaining({ + mode: 'execute', + branchName: 'cw/test-task-t1', + baseBranch: 'cw/test-phase-impl', + }), + ); + }); + + it('includes qualifying files in the prompt', async () => { + await runQualityReview(params); + const spawnArgs = vi.mocked(agentManager.spawn).mock.calls[0]![0]; + expect(spawnArgs.prompt).toContain('src/foo.ts'); + expect(spawnArgs.prompt).toContain('src/bar.ts'); + expect(spawnArgs.prompt).toContain('/simplify'); + }); + + it('spawns with taskId and initiativeId', async () => { + await runQualityReview(params); + expect(agentManager.spawn).toHaveBeenCalledWith( + expect.objectContaining({ + taskId: 't1', + initiativeId: 'i1', + }), + ); + }); + + it('on spawn failure: marks task completed and does not throw', async () => { + vi.mocked(agentManager.spawn).mockRejectedValue(new Error('spawn failed')); + await expect(runQualityReview(params)).resolves.toBeUndefined(); + // Last call to update should set status to completed + const updateCalls = vi.mocked(taskRepository.update).mock.calls; + const lastCall = updateCalls[updateCalls.length - 1]!; + expect(lastCall).toEqual(['t1', { status: 'completed' }]); + }); + + it('logs info after successful spawn', async () => { + await runQualityReview(params); + expect(log.info).toHaveBeenCalledWith( + expect.objectContaining({ taskId: 't1', reviewAgentId: 'review-agent-1' }), + expect.any(String), + ); + }); + + it('logs error on spawn failure', async () => { + vi.mocked(agentManager.spawn).mockRejectedValue(new Error('spawn failed')); + await runQualityReview(params); + expect(log.error).toHaveBeenCalledWith( + expect.objectContaining({ taskId: 't1' }), + expect.any(String), + ); + }); +}); diff --git a/apps/server/execution/quality-review.ts b/apps/server/execution/quality-review.ts new file mode 100644 index 0000000..b8a3a6b --- /dev/null +++ b/apps/server/execution/quality-review.ts @@ -0,0 +1,175 @@ +/** + * Quality Review Dispatch Hook + * + * Intercepts agent:stopped events and, when conditions are met, spawns + * a fresh execute-mode agent to run /simplify on changed files before + * the task reaches 'completed' status. + */ + +import type { BranchManager } from '../git/branch-manager.js'; +import type { AgentManager } from '../agent/types.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 { PhaseRepository } from '../db/repositories/phase-repository.js'; +import { phaseBranchName, taskBranchName } from '../git/branch-naming.js'; +import type { createModuleLogger } from '../logger/index.js'; + +type Logger = ReturnType; + +// --------------------------------------------------------------------------- +// computeQualifyingFiles +// --------------------------------------------------------------------------- + +/** + * Returns the list of .ts/.tsx/.js files changed between taskBranch and baseBranch, + * excluding generated files and dist artifacts. + */ +export async function computeQualifyingFiles( + branchManager: BranchManager, + repoPath: string, + taskBranch: string, + baseBranch: string, +): Promise { + try { + const entries = await branchManager.diffBranchesStat(repoPath, baseBranch, taskBranch); + return entries + .map((e) => e.path) + .filter( + (p) => + /\.(ts|tsx|js)$/.test(p) && + !p.endsWith('.gen.ts') && + !p.startsWith('dist/') && + !p.includes('/dist/'), + ); + } catch { + return []; + } +} + +// --------------------------------------------------------------------------- +// shouldRunQualityReview +// --------------------------------------------------------------------------- + +interface ShouldRunParams { + agentId: string; + taskId: string; + stopReason: string; + agentRepository: AgentRepository; + taskRepository: TaskRepository; + initiativeRepository: InitiativeRepository; + phaseRepository: PhaseRepository; + branchManager: BranchManager; + repoPath: string; +} + +/** + * Evaluates whether a quality review should be run for the stopped agent. + * Returns `{ run: true, qualifyingFiles }` only when all conditions pass. + * Short-circuits on first false condition. + */ +export async function shouldRunQualityReview( + params: ShouldRunParams, +): Promise<{ run: boolean; qualifyingFiles: string[] }> { + const { + agentId, + taskId, + stopReason, + agentRepository, + taskRepository, + initiativeRepository, + phaseRepository, + branchManager, + repoPath, + } = params; + + const NO = { run: false, qualifyingFiles: [] }; + + // 1. Only act on task_complete stops + if (stopReason !== 'task_complete') return NO; + + // 2. Agent must be in execute mode (guards against errand agents) + const agent = await agentRepository.findById(agentId); + if (!agent || agent.mode !== 'execute') return NO; + + // 3. Task must be in_progress; quality_review is the recursion guard + 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 !== true) return NO; + + // 6. Compute branch names from task context + if (!task.phaseId) return NO; + const phase = await phaseRepository.findById(task.phaseId); + if (!phase) return NO; + + const initBranch = initiative.branch; + if (!initBranch) return NO; + + const base = phaseBranchName(initBranch, phase.name); + const branch = taskBranchName(initBranch, task.id); + + // 7. Must have qualifying files in the changeset + const qualifyingFiles = await computeQualifyingFiles(branchManager, repoPath, branch, base); + if (qualifyingFiles.length === 0) return NO; + + return { run: true, qualifyingFiles }; +} + +// --------------------------------------------------------------------------- +// runQualityReview +// --------------------------------------------------------------------------- + +interface RunQualityReviewParams { + taskId: string; + taskBranch: string; + baseBranch: string; + initiativeId: string; + qualifyingFiles: string[]; + taskRepository: TaskRepository; + agentManager: AgentManager; + log: Logger; +} + +/** + * Transitions the task to quality_review and spawns a fresh execute-mode + * agent to run /simplify on the changed files. + * + * On spawn failure: marks task completed and returns (never throws). + */ +export async function runQualityReview(params: RunQualityReviewParams): Promise { + const { taskId, taskBranch, baseBranch, initiativeId, qualifyingFiles, taskRepository, agentManager, log } = params; + + // 1. Transition BEFORE spawning + await taskRepository.update(taskId, { status: 'quality_review' }); + + // 2. Build prompt + const fileList = qualifyingFiles.map((f) => `- ${f}`).join('\n'); + const reviewPrompt = `Run /simplify to review and fix code quality in this branch.\n\nFiles changed in this task:\n${fileList}`; + + // 3. Spawn fresh execute-mode agent on the same task branch + try { + const reviewAgent = await agentManager.spawn({ + taskId, + initiativeId, + prompt: reviewPrompt, + mode: 'execute', + baseBranch, + branchName: taskBranch, + }); + + // 4. Log success + log.info({ taskId, reviewAgentId: reviewAgent.id }, 'quality review agent spawned'); + } catch (err) { + // 5. On spawn failure: mark completed and return — never block task completion + log.error({ taskId, err }, 'quality review spawn failed; marking task completed'); + await taskRepository.update(taskId, { status: 'completed' }); + } +} diff --git a/docs/dispatch-events.md b/docs/dispatch-events.md index 5d1b4e9..fea1ddb 100644 --- a/docs/dispatch-events.md +++ b/docs/dispatch-events.md @@ -113,7 +113,7 @@ InitiativeChangesRequestedEvent { initiativeId, phaseId, taskId } | Event | Action | |-------|--------| | `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. | | `task:completed` | Merge task branch (if branch exists), check phase completion, dispatch next queued task |