Merge branch 'cw/continuous-code-quality-task-_Om84Be00WJgnctOvmkyo' into cw/continuous-code-quality-phase-quality-review-dispatch-hook

# Conflicts:
#	apps/server/execution/quality-review.test.ts
#	apps/server/execution/quality-review.ts
This commit is contained in:
Lukas May
2026-03-06 22:04:50 +01:00
4 changed files with 167 additions and 5 deletions

View File

@@ -250,6 +250,7 @@ export async function createContainer(options?: ContainerOptions): Promise<Conta
conflictResolutionService, conflictResolutionService,
eventBus, eventBus,
workspaceRoot, workspaceRoot,
agentManager,
repos.agentRepository, repos.agentRepository,
); );
executionOrchestrator.start(); executionOrchestrator.start();

View File

@@ -8,10 +8,17 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
import { ExecutionOrchestrator } from './orchestrator.js'; import { ExecutionOrchestrator } from './orchestrator.js';
import { ensureProjectClone } from '../git/project-clones.js'; import { ensureProjectClone } from '../git/project-clones.js';
import type { BranchManager } from '../git/branch-manager.js'; import type { BranchManager } from '../git/branch-manager.js';
import { shouldRunQualityReview, runQualityReview } from './quality-review.js';
vi.mock('../git/project-clones.js', () => ({ vi.mock('../git/project-clones.js', () => ({
ensureProjectClone: vi.fn().mockResolvedValue('/tmp/test-workspace/clones/test'), ensureProjectClone: vi.fn().mockResolvedValue('/tmp/test-workspace/clones/test'),
})); }));
vi.mock('./quality-review.js', () => ({
shouldRunQualityReview: vi.fn(),
runQualityReview: vi.fn(),
computeQualifyingFiles: vi.fn(),
}));
import type { PhaseRepository } from '../db/repositories/phase-repository.js'; import type { PhaseRepository } from '../db/repositories/phase-repository.js';
import type { TaskRepository } from '../db/repositories/task-repository.js'; import type { TaskRepository } from '../db/repositories/task-repository.js';
import type { InitiativeRepository } from '../db/repositories/initiative-repository.js'; import type { InitiativeRepository } from '../db/repositories/initiative-repository.js';
@@ -110,6 +117,23 @@ function createMocks() {
const eventBus = createMockEventBus(); 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 { return {
branchManager, branchManager,
phaseRepository, phaseRepository,
@@ -120,6 +144,8 @@ function createMocks() {
dispatchManager, dispatchManager,
conflictResolutionService, conflictResolutionService,
eventBus, eventBus,
agentManager,
agentRepository,
}; };
} }
@@ -135,6 +161,8 @@ function createOrchestrator(mocks: ReturnType<typeof createMocks>) {
mocks.conflictResolutionService, mocks.conflictResolutionService,
mocks.eventBus, mocks.eventBus,
'/tmp/test-workspace', '/tmp/test-workspace',
mocks.agentManager as any,
mocks.agentRepository as any,
); );
orchestrator.start(); orchestrator.start();
return orchestrator; return orchestrator;
@@ -370,3 +398,87 @@ describe('ExecutionOrchestrator', () => {
}); });
}); });
}); });
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);
createOrchestrator(mocks);
mocks.eventBus.emit({
type: 'agent:stopped',
timestamp: new Date(),
payload: { taskId: 't1', reason: 'task_complete', agentId: 'a1' },
});
await vi.waitFor(() => {
expect(runQualityReview).toHaveBeenCalledWith(
expect.objectContaining({ taskId: 't1', qualifyingFiles: ['src/foo.ts'] }),
);
});
expect(mocks.dispatchManager.completeTask).not.toHaveBeenCalled();
});
it('calls completeTask and skips runQualityReview when shouldRunQualityReview returns run:false', async () => {
vi.mocked(shouldRunQualityReview).mockResolvedValue({ run: false, qualifyingFiles: [] });
vi.mocked(runQualityReview).mockResolvedValue(undefined);
createOrchestrator(mocks);
mocks.eventBus.emit({
type: 'agent:stopped',
timestamp: new Date(),
payload: { taskId: 't1', reason: 'task_complete', agentId: 'a1' },
});
await vi.waitFor(() => {
expect(mocks.dispatchManager.completeTask).toHaveBeenCalledWith('t1', 'a1');
});
expect(runQualityReview).not.toHaveBeenCalled();
});
it('skips both paths for user_requested reason', async () => {
createOrchestrator(mocks);
mocks.eventBus.emit({
type: 'agent:stopped',
timestamp: new Date(),
payload: { taskId: 't1', reason: 'user_requested', agentId: 'a1' },
});
// Wait for scheduleDispatch to be triggered (dispatchNext is called in the cycle)
await vi.waitFor(() => expect(mocks.dispatchManager.dispatchNext).toHaveBeenCalled());
expect(shouldRunQualityReview).not.toHaveBeenCalled();
expect(mocks.dispatchManager.completeTask).not.toHaveBeenCalled();
});
});

View File

@@ -18,12 +18,14 @@ import type { TaskRepository } from '../db/repositories/task-repository.js';
import type { InitiativeRepository } from '../db/repositories/initiative-repository.js'; import type { InitiativeRepository } from '../db/repositories/initiative-repository.js';
import type { ProjectRepository } from '../db/repositories/project-repository.js'; import type { ProjectRepository } from '../db/repositories/project-repository.js';
import type { AgentRepository } from '../db/repositories/agent-repository.js'; import type { AgentRepository } from '../db/repositories/agent-repository.js';
import type { AgentManager } from '../agent/types.js';
import type { DispatchManager, PhaseDispatchManager } from '../dispatch/types.js'; import type { DispatchManager, PhaseDispatchManager } from '../dispatch/types.js';
import type { ConflictResolutionService } from '../coordination/conflict-resolution-service.js'; import type { ConflictResolutionService } from '../coordination/conflict-resolution-service.js';
import { phaseBranchName, taskBranchName } from '../git/branch-naming.js'; import { phaseBranchName, taskBranchName } from '../git/branch-naming.js';
import { ensureProjectClone } from '../git/project-clones.js'; import { ensureProjectClone } from '../git/project-clones.js';
import { createModuleLogger } from '../logger/index.js'; import { createModuleLogger } from '../logger/index.js';
import { phaseMetaCache, fileDiffCache } from '../review/diff-cache.js'; import { phaseMetaCache, fileDiffCache } from '../review/diff-cache.js';
import { shouldRunQualityReview, runQualityReview } from './quality-review.js';
const log = createModuleLogger('execution-orchestrator'); const log = createModuleLogger('execution-orchestrator');
@@ -49,6 +51,7 @@ export class ExecutionOrchestrator {
private conflictResolutionService: ConflictResolutionService, private conflictResolutionService: ConflictResolutionService,
private eventBus: EventBus, private eventBus: EventBus,
private workspaceRoot: string, private workspaceRoot: string,
private agentManager: AgentManager,
private agentRepository?: AgentRepository, private agentRepository?: AgentRepository,
) {} ) {}
@@ -108,15 +111,53 @@ export class ExecutionOrchestrator {
private async handleAgentStopped(event: AgentStoppedEvent): Promise<void> { private async handleAgentStopped(event: AgentStoppedEvent): Promise<void> {
const { taskId, reason, agentId } = event.payload; const { taskId, reason, agentId } = event.payload;
// Auto-complete task for successful agent completions, not manual stops
if (taskId && reason !== 'user_requested') { if (taskId && reason !== 'user_requested') {
try { try {
await this.dispatchManager.completeTask(taskId, agentId); if (!this.agentRepository) {
log.info({ taskId, agentId, reason }, 'task auto-completed on agent stop'); // 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) { } catch (err) {
log.warn( log.warn(
{ taskId, agentId, reason, err: err instanceof Error ? err.message : String(err) }, { taskId, agentId, reason, err: err instanceof Error ? err.message : String(err) },
'failed to auto-complete task on agent stop', 'failed to handle agent stop',
); );
} }
} }
@@ -124,6 +165,14 @@ export class ExecutionOrchestrator {
this.scheduleDispatch(); this.scheduleDispatch();
} }
private async getRepoPathForTask(taskId: string): Promise<string> {
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<void> { private async handleAgentCrashed(event: AgentCrashedEvent): Promise<void> {
const { taskId, agentId, error } = event.payload; const { taskId, agentId, error } = event.payload;
if (!taskId) return; if (!taskId) return;

View File

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