Remove task-level approval system
Task-level approval (requiresApproval, mergeRequiresApproval, pending_approval status) was redundant with executionMode (yolo vs review_per_phase) and blocked the orchestrator's phase completion flow. Tasks now complete directly; phase-level review via executionMode is the right granularity. Removed: schema columns (left in DB, removed from Drizzle), TaskPendingApprovalEvent, approveTask/listPendingApprovals procedures, findPendingApproval repository method, and all frontend approval UI.
This commit is contained in:
@@ -48,7 +48,6 @@ describe('writeInputFiles', () => {
|
||||
id: 'init-1',
|
||||
name: 'Test Initiative',
|
||||
status: 'active',
|
||||
mergeRequiresApproval: true,
|
||||
branch: 'cw/test-initiative',
|
||||
executionMode: 'review_per_phase',
|
||||
createdAt: new Date('2026-01-01'),
|
||||
|
||||
@@ -132,7 +132,6 @@ export async function writeInputFiles(options: WriteInputFilesOptions): Promise<
|
||||
id: ini.id,
|
||||
name: ini.name,
|
||||
status: ini.status,
|
||||
mergeRequiresApproval: ini.mergeRequiresApproval,
|
||||
branch: ini.branch,
|
||||
},
|
||||
'',
|
||||
|
||||
@@ -336,7 +336,7 @@ export function createCli(serverHandler?: (port?: number) => Promise<void>): Com
|
||||
}
|
||||
|
||||
// Count by status
|
||||
const pending = tasks.filter(t => t.status === 'pending' || t.status === 'pending_approval').length;
|
||||
const pending = tasks.filter(t => t.status === 'pending').length;
|
||||
const inProgress = tasks.filter(t => t.status === 'in_progress').length;
|
||||
const completed = tasks.filter(t => t.status === 'completed').length;
|
||||
const blocked = tasks.filter(t => t.status === 'blocked').length;
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
* Implements TaskRepository interface using Drizzle ORM.
|
||||
*/
|
||||
|
||||
import { eq, asc, and } from 'drizzle-orm';
|
||||
import { eq, asc } from 'drizzle-orm';
|
||||
import { nanoid } from 'nanoid';
|
||||
import type { DrizzleDatabase } from '../../index.js';
|
||||
import { tasks, taskDependencies, type Task } from '../../schema.js';
|
||||
@@ -12,7 +12,6 @@ import type {
|
||||
TaskRepository,
|
||||
CreateTaskData,
|
||||
UpdateTaskData,
|
||||
PendingApprovalFilters,
|
||||
} from '../task-repository.js';
|
||||
|
||||
/**
|
||||
@@ -77,26 +76,6 @@ export class DrizzleTaskRepository implements TaskRepository {
|
||||
.orderBy(asc(tasks.order));
|
||||
}
|
||||
|
||||
async findPendingApproval(filters?: PendingApprovalFilters): Promise<Task[]> {
|
||||
const conditions = [eq(tasks.status, 'pending_approval')];
|
||||
|
||||
if (filters?.initiativeId) {
|
||||
conditions.push(eq(tasks.initiativeId, filters.initiativeId));
|
||||
}
|
||||
if (filters?.phaseId) {
|
||||
conditions.push(eq(tasks.phaseId, filters.phaseId));
|
||||
}
|
||||
if (filters?.category) {
|
||||
conditions.push(eq(tasks.category, filters.category));
|
||||
}
|
||||
|
||||
return this.db
|
||||
.select()
|
||||
.from(tasks)
|
||||
.where(and(...conditions))
|
||||
.orderBy(asc(tasks.createdAt));
|
||||
}
|
||||
|
||||
async update(id: string, data: UpdateTaskData): Promise<Task> {
|
||||
const [updated] = await this.db
|
||||
.update(tasks)
|
||||
|
||||
@@ -22,7 +22,6 @@ export type {
|
||||
TaskRepository,
|
||||
CreateTaskData,
|
||||
UpdateTaskData,
|
||||
PendingApprovalFilters,
|
||||
} from './task-repository.js';
|
||||
|
||||
export type {
|
||||
|
||||
@@ -20,15 +20,6 @@ export type CreateTaskData = Omit<NewTask, 'id' | 'createdAt' | 'updatedAt'> & {
|
||||
*/
|
||||
export type UpdateTaskData = Partial<CreateTaskData>;
|
||||
|
||||
/**
|
||||
* Filters for finding pending approval tasks.
|
||||
*/
|
||||
export interface PendingApprovalFilters {
|
||||
initiativeId?: string;
|
||||
phaseId?: string;
|
||||
category?: TaskCategory;
|
||||
}
|
||||
|
||||
/**
|
||||
* Task Repository Port
|
||||
*
|
||||
@@ -70,13 +61,6 @@ export interface TaskRepository {
|
||||
*/
|
||||
findByPhaseId(phaseId: string): Promise<Task[]>;
|
||||
|
||||
/**
|
||||
* Find all tasks with status 'pending_approval'.
|
||||
* Optional filters by initiative, phase, or category.
|
||||
* Returns tasks ordered by createdAt.
|
||||
*/
|
||||
findPendingApproval(filters?: PendingApprovalFilters): Promise<Task[]>;
|
||||
|
||||
/**
|
||||
* Update a task.
|
||||
* Throws if task not found.
|
||||
|
||||
@@ -22,9 +22,6 @@ export const initiatives = sqliteTable('initiatives', {
|
||||
status: text('status', { enum: ['active', 'completed', 'archived', 'pending_review'] })
|
||||
.notNull()
|
||||
.default('active'),
|
||||
mergeRequiresApproval: integer('merge_requires_approval', { mode: 'boolean' })
|
||||
.notNull()
|
||||
.default(true),
|
||||
branch: text('branch'), // Auto-generated initiative branch (e.g., 'cw/user-auth')
|
||||
executionMode: text('execution_mode', { enum: ['yolo', 'review_per_phase'] })
|
||||
.notNull()
|
||||
@@ -153,11 +150,10 @@ export const tasks = sqliteTable('tasks', {
|
||||
.notNull()
|
||||
.default('medium'),
|
||||
status: text('status', {
|
||||
enum: ['pending_approval', 'pending', 'in_progress', 'completed', 'blocked'],
|
||||
enum: ['pending', 'in_progress', 'completed', 'blocked'],
|
||||
})
|
||||
.notNull()
|
||||
.default('pending'),
|
||||
requiresApproval: integer('requires_approval', { mode: 'boolean' }), // null = inherit from initiative
|
||||
order: integer('order').notNull().default(0),
|
||||
summary: text('summary'), // Agent result summary — propagated to dependent tasks as context
|
||||
createdAt: integer('created_at', { mode: 'timestamp' }).notNull(),
|
||||
|
||||
@@ -13,7 +13,6 @@ import type {
|
||||
TaskCompletedEvent,
|
||||
TaskBlockedEvent,
|
||||
TaskDispatchedEvent,
|
||||
TaskPendingApprovalEvent,
|
||||
} from '../events/index.js';
|
||||
import type { AgentManager, AgentResult, AgentInfo } from '../agent/types.js';
|
||||
import type { TaskRepository } from '../db/repositories/task-repository.js';
|
||||
@@ -172,7 +171,6 @@ export class DefaultDispatchManager implements DispatchManager {
|
||||
|
||||
/**
|
||||
* Mark a task as complete.
|
||||
* If the task requires approval, sets status to 'pending_approval' instead.
|
||||
* Updates task status and removes from queue.
|
||||
*
|
||||
* @param taskId - ID of the task to complete
|
||||
@@ -184,78 +182,15 @@ export class DefaultDispatchManager implements DispatchManager {
|
||||
throw new Error(`Task not found: ${taskId}`);
|
||||
}
|
||||
|
||||
// Determine if approval is required
|
||||
const requiresApproval = await this.taskRequiresApproval(task);
|
||||
|
||||
// Store agent result summary on the task for propagation to dependent tasks
|
||||
await this.storeAgentSummary(taskId, agentId);
|
||||
|
||||
if (requiresApproval) {
|
||||
// Set to pending_approval instead of completed
|
||||
await this.taskRepository.update(taskId, { status: 'pending_approval' });
|
||||
|
||||
// Remove from queue
|
||||
this.taskQueue.delete(taskId);
|
||||
|
||||
log.info({ taskId, category: task.category }, 'task pending approval');
|
||||
|
||||
// Emit TaskPendingApprovalEvent
|
||||
const event: TaskPendingApprovalEvent = {
|
||||
type: 'task:pending_approval',
|
||||
timestamp: new Date(),
|
||||
payload: {
|
||||
taskId,
|
||||
agentId: agentId ?? '',
|
||||
category: task.category,
|
||||
name: task.name,
|
||||
},
|
||||
};
|
||||
this.eventBus.emit(event);
|
||||
} else {
|
||||
// Complete directly
|
||||
await this.taskRepository.update(taskId, { status: 'completed' });
|
||||
|
||||
// Remove from queue
|
||||
this.taskQueue.delete(taskId);
|
||||
|
||||
log.info({ taskId }, 'task completed');
|
||||
|
||||
// Emit TaskCompletedEvent
|
||||
const event: TaskCompletedEvent = {
|
||||
type: 'task:completed',
|
||||
timestamp: new Date(),
|
||||
payload: {
|
||||
taskId,
|
||||
agentId: agentId ?? '',
|
||||
success: true,
|
||||
message: 'Task completed',
|
||||
},
|
||||
};
|
||||
this.eventBus.emit(event);
|
||||
}
|
||||
|
||||
// Also remove from blocked if it was there
|
||||
this.blockedTasks.delete(taskId);
|
||||
}
|
||||
|
||||
/**
|
||||
* Approve a task that is pending approval.
|
||||
* Sets status to 'completed' and emits completion event.
|
||||
*/
|
||||
async approveTask(taskId: string): Promise<void> {
|
||||
const task = await this.taskRepository.findById(taskId);
|
||||
if (!task) {
|
||||
throw new Error(`Task not found: ${taskId}`);
|
||||
}
|
||||
|
||||
if (task.status !== 'pending_approval') {
|
||||
throw new Error(`Task ${taskId} is not pending approval (status: ${task.status})`);
|
||||
}
|
||||
|
||||
// Complete the task
|
||||
await this.taskRepository.update(taskId, { status: 'completed' });
|
||||
|
||||
log.info({ taskId }, 'task approved and completed');
|
||||
// Remove from queue
|
||||
this.taskQueue.delete(taskId);
|
||||
|
||||
log.info({ taskId }, 'task completed');
|
||||
|
||||
// Emit TaskCompletedEvent
|
||||
const event: TaskCompletedEvent = {
|
||||
@@ -263,12 +198,15 @@ export class DefaultDispatchManager implements DispatchManager {
|
||||
timestamp: new Date(),
|
||||
payload: {
|
||||
taskId,
|
||||
agentId: '',
|
||||
agentId: agentId ?? '',
|
||||
success: true,
|
||||
message: 'Task approved',
|
||||
message: 'Task completed',
|
||||
},
|
||||
};
|
||||
this.eventBus.emit(event);
|
||||
|
||||
// Also remove from blocked if it was there
|
||||
this.blockedTasks.delete(taskId);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -563,26 +501,4 @@ export class DefaultDispatchManager implements DispatchManager {
|
||||
return { phases, tasks: implementationTasks, pages };
|
||||
}
|
||||
|
||||
/**
|
||||
* Determine if a task requires approval before being marked complete.
|
||||
* Checks task-level override first, then falls back to initiative setting.
|
||||
*/
|
||||
private async taskRequiresApproval(task: Task): Promise<boolean> {
|
||||
// Task-level override takes precedence
|
||||
if (task.requiresApproval !== null) {
|
||||
return task.requiresApproval;
|
||||
}
|
||||
|
||||
// Fall back to initiative setting if we have initiative access
|
||||
if (this.initiativeRepository && task.initiativeId) {
|
||||
const initiative = await this.initiativeRepository.findById(task.initiativeId);
|
||||
if (initiative) {
|
||||
return initiative.mergeRequiresApproval;
|
||||
}
|
||||
}
|
||||
|
||||
// If task has a phaseId but no initiativeId, we could traverse up but for now default to false
|
||||
// Default: no approval required
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -49,7 +49,6 @@ function createMockDispatchManager(): DispatchManager {
|
||||
getNextDispatchable: vi.fn().mockResolvedValue(null),
|
||||
dispatchNext: vi.fn().mockResolvedValue({ success: false, reason: 'mock' }),
|
||||
completeTask: vi.fn(),
|
||||
approveTask: vi.fn(),
|
||||
blockTask: vi.fn(),
|
||||
getQueueState: vi.fn().mockResolvedValue({ queued: [], ready: [], blocked: [] }),
|
||||
};
|
||||
|
||||
@@ -86,7 +86,6 @@ export interface DispatchManager {
|
||||
|
||||
/**
|
||||
* Mark a task as complete.
|
||||
* If the task requires approval, sets status to 'pending_approval' instead.
|
||||
* Triggers re-evaluation of dependent tasks.
|
||||
*
|
||||
* @param taskId - ID of the completed task
|
||||
@@ -94,14 +93,6 @@ export interface DispatchManager {
|
||||
*/
|
||||
completeTask(taskId: string, agentId?: string): Promise<void>;
|
||||
|
||||
/**
|
||||
* Approve a task that is pending approval.
|
||||
* Sets status to 'completed' and emits completion event.
|
||||
*
|
||||
* @param taskId - ID of the task to approve
|
||||
*/
|
||||
approveTask(taskId: string): Promise<void>;
|
||||
|
||||
/**
|
||||
* Mark a task as blocked.
|
||||
* Task will not be dispatched until unblocked.
|
||||
|
||||
6
apps/server/drizzle/0030_remove_task_approval.sql
Normal file
6
apps/server/drizzle/0030_remove_task_approval.sql
Normal file
@@ -0,0 +1,6 @@
|
||||
-- Migrate any pending_approval tasks to completed (unblock the pipeline)
|
||||
UPDATE tasks SET status = 'completed' WHERE status = 'pending_approval';
|
||||
|
||||
-- Note: SQLite cannot drop columns. The merge_requires_approval column on
|
||||
-- initiatives and requires_approval column on tasks are left in the DB but
|
||||
-- removed from the Drizzle schema. Drizzle ignores extra DB columns.
|
||||
@@ -211,6 +211,13 @@
|
||||
"when": 1772064000000,
|
||||
"tag": "0029_add_project_last_fetched_at",
|
||||
"breakpoints": true
|
||||
},
|
||||
{
|
||||
"idx": 30,
|
||||
"version": "6",
|
||||
"when": 1772150400000,
|
||||
"tag": "0030_remove_task_approval",
|
||||
"breakpoints": true
|
||||
}
|
||||
]
|
||||
}
|
||||
@@ -32,7 +32,6 @@ export type {
|
||||
TaskDispatchedEvent,
|
||||
TaskCompletedEvent,
|
||||
TaskBlockedEvent,
|
||||
TaskPendingApprovalEvent,
|
||||
PhaseQueuedEvent,
|
||||
PhaseStartedEvent,
|
||||
PhaseCompletedEvent,
|
||||
|
||||
@@ -272,16 +272,6 @@ export interface TaskBlockedEvent extends DomainEvent {
|
||||
};
|
||||
}
|
||||
|
||||
export interface TaskPendingApprovalEvent extends DomainEvent {
|
||||
type: 'task:pending_approval';
|
||||
payload: {
|
||||
taskId: string;
|
||||
agentId: string;
|
||||
category: string;
|
||||
name: string;
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Phase Events
|
||||
*/
|
||||
@@ -647,7 +637,6 @@ export type DomainEventMap =
|
||||
| TaskDispatchedEvent
|
||||
| TaskCompletedEvent
|
||||
| TaskBlockedEvent
|
||||
| TaskPendingApprovalEvent
|
||||
| PhaseQueuedEvent
|
||||
| PhaseStartedEvent
|
||||
| PhaseCompletedEvent
|
||||
|
||||
@@ -631,7 +631,6 @@ export function createTestHarness(): TestHarness {
|
||||
description,
|
||||
category: 'detail',
|
||||
type: 'auto',
|
||||
requiresApproval: true,
|
||||
});
|
||||
},
|
||||
|
||||
|
||||
@@ -69,7 +69,7 @@ export function printDetailResult(phase: Phase, tasks: Task[]): void {
|
||||
console.log(`\n[DETAIL] Phase "${phase.name}" → ${tasks.length} task(s)`);
|
||||
console.log(THIN);
|
||||
tasks.forEach((t, i) => {
|
||||
const flags = [t.category, t.type, t.requiresApproval ? 'approval-required' : 'auto'].join(', ');
|
||||
const flags = [t.category, t.type].join(', ');
|
||||
line(`${i + 1}. ${t.name} [${flags}]`);
|
||||
if (t.description) {
|
||||
line(` ${t.description.slice(0, 120)}`);
|
||||
|
||||
@@ -204,7 +204,6 @@ export function initiativeProcedures(publicProcedure: ProcedureBuilder) {
|
||||
updateInitiativeConfig: publicProcedure
|
||||
.input(z.object({
|
||||
initiativeId: z.string().min(1),
|
||||
mergeRequiresApproval: z.boolean().optional(),
|
||||
executionMode: z.enum(['yolo', 'review_per_phase']).optional(),
|
||||
branch: z.string().nullable().optional(),
|
||||
}))
|
||||
|
||||
@@ -38,7 +38,7 @@ export function taskProcedures(publicProcedure: ProcedureBuilder) {
|
||||
updateTaskStatus: publicProcedure
|
||||
.input(z.object({
|
||||
id: z.string().min(1),
|
||||
status: z.enum(['pending_approval', 'pending', 'in_progress', 'completed', 'blocked']),
|
||||
status: z.enum(['pending', 'in_progress', 'completed', 'blocked']),
|
||||
}))
|
||||
.mutation(async ({ ctx, input }) => {
|
||||
const taskRepository = requireTaskRepository(ctx);
|
||||
@@ -59,7 +59,6 @@ export function taskProcedures(publicProcedure: ProcedureBuilder) {
|
||||
description: z.string().optional(),
|
||||
category: z.enum(['execute', 'research', 'discuss', 'plan', 'detail', 'refine', 'verify', 'merge', 'review']).optional(),
|
||||
type: z.enum(['auto', 'checkpoint:human-verify', 'checkpoint:decision', 'checkpoint:human-action']).optional(),
|
||||
requiresApproval: z.boolean().nullable().optional(),
|
||||
}))
|
||||
.mutation(async ({ ctx, input }) => {
|
||||
const taskRepository = requireTaskRepository(ctx);
|
||||
@@ -79,7 +78,6 @@ export function taskProcedures(publicProcedure: ProcedureBuilder) {
|
||||
description: input.description ?? null,
|
||||
category: input.category ?? 'execute',
|
||||
type: input.type ?? 'auto',
|
||||
requiresApproval: input.requiresApproval ?? null,
|
||||
status: 'pending',
|
||||
});
|
||||
}),
|
||||
@@ -91,7 +89,6 @@ export function taskProcedures(publicProcedure: ProcedureBuilder) {
|
||||
description: z.string().optional(),
|
||||
category: z.enum(['execute', 'research', 'discuss', 'plan', 'detail', 'refine', 'verify', 'merge', 'review']).optional(),
|
||||
type: z.enum(['auto', 'checkpoint:human-verify', 'checkpoint:decision', 'checkpoint:human-action']).optional(),
|
||||
requiresApproval: z.boolean().nullable().optional(),
|
||||
}))
|
||||
.mutation(async ({ ctx, input }) => {
|
||||
const taskRepository = requireTaskRepository(ctx);
|
||||
@@ -111,22 +108,10 @@ export function taskProcedures(publicProcedure: ProcedureBuilder) {
|
||||
description: input.description ?? null,
|
||||
category: input.category ?? 'execute',
|
||||
type: input.type ?? 'auto',
|
||||
requiresApproval: input.requiresApproval ?? null,
|
||||
status: 'pending',
|
||||
});
|
||||
}),
|
||||
|
||||
listPendingApprovals: publicProcedure
|
||||
.input(z.object({
|
||||
initiativeId: z.string().optional(),
|
||||
phaseId: z.string().optional(),
|
||||
category: z.enum(['execute', 'research', 'discuss', 'plan', 'detail', 'refine', 'verify', 'merge', 'review']).optional(),
|
||||
}).optional())
|
||||
.query(async ({ ctx, input }) => {
|
||||
const taskRepository = requireTaskRepository(ctx);
|
||||
return taskRepository.findPendingApproval(input);
|
||||
}),
|
||||
|
||||
listInitiativeTasks: publicProcedure
|
||||
.input(z.object({ initiativeId: z.string().min(1) }))
|
||||
.query(async ({ ctx, input }) => {
|
||||
@@ -196,12 +181,5 @@ export function taskProcedures(publicProcedure: ProcedureBuilder) {
|
||||
return edges;
|
||||
}),
|
||||
|
||||
approveTask: publicProcedure
|
||||
.input(z.object({ taskId: z.string().min(1) }))
|
||||
.mutation(async ({ ctx, input }) => {
|
||||
const dispatchManager = requireDispatchManager(ctx);
|
||||
await dispatchManager.approveTask(input.taskId);
|
||||
return { success: true };
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user