From 8804455c779ca8982b91f0b3ce95f00c81bc8316 Mon Sep 17 00:00:00 2001 From: Lukas May Date: Thu, 5 Mar 2026 17:09:48 +0100 Subject: [PATCH] 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. --- apps/server/agent/file-io.test.ts | 1 - apps/server/agent/file-io.ts | 1 - apps/server/cli/index.ts | 2 +- apps/server/db/repositories/drizzle/task.ts | 23 +--- apps/server/db/repositories/index.ts | 1 - .../server/db/repositories/task-repository.ts | 16 --- apps/server/db/schema.ts | 6 +- apps/server/dispatch/manager.ts | 102 ++---------------- apps/server/dispatch/phase-manager.test.ts | 1 - apps/server/dispatch/types.ts | 9 -- .../drizzle/0030_remove_task_approval.sql | 6 ++ apps/server/drizzle/meta/_journal.json | 7 ++ apps/server/events/index.ts | 1 - apps/server/events/types.ts | 11 -- apps/server/test/harness.ts | 1 - .../test/integration/full-flow/report.ts | 2 +- apps/server/trpc/routers/initiative.ts | 1 - apps/server/trpc/routers/task.ts | 24 +---- .../src/components/CreateInitiativeDialog.tsx | 1 - apps/web/src/components/InitiativeCard.tsx | 1 - apps/web/src/components/StatusDot.tsx | 1 - apps/web/src/components/TaskRow.tsx | 3 +- .../components/execution/TaskSlideOver.tsx | 40 +++---- apps/web/src/lib/invalidation.ts | 1 - docs/database.md | 8 +- docs/dispatch-events.md | 9 +- docs/server-api.md | 6 +- 27 files changed, 48 insertions(+), 237 deletions(-) create mode 100644 apps/server/drizzle/0030_remove_task_approval.sql diff --git a/apps/server/agent/file-io.test.ts b/apps/server/agent/file-io.test.ts index 42a66b9..20aec4d 100644 --- a/apps/server/agent/file-io.test.ts +++ b/apps/server/agent/file-io.test.ts @@ -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'), diff --git a/apps/server/agent/file-io.ts b/apps/server/agent/file-io.ts index 211befc..e6da8e1 100644 --- a/apps/server/agent/file-io.ts +++ b/apps/server/agent/file-io.ts @@ -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, }, '', diff --git a/apps/server/cli/index.ts b/apps/server/cli/index.ts index 2e117ec..79213ab 100644 --- a/apps/server/cli/index.ts +++ b/apps/server/cli/index.ts @@ -336,7 +336,7 @@ export function createCli(serverHandler?: (port?: number) => Promise): 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; diff --git a/apps/server/db/repositories/drizzle/task.ts b/apps/server/db/repositories/drizzle/task.ts index ea04f69..2240229 100644 --- a/apps/server/db/repositories/drizzle/task.ts +++ b/apps/server/db/repositories/drizzle/task.ts @@ -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 { - 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 { const [updated] = await this.db .update(tasks) diff --git a/apps/server/db/repositories/index.ts b/apps/server/db/repositories/index.ts index c9258d3..809214c 100644 --- a/apps/server/db/repositories/index.ts +++ b/apps/server/db/repositories/index.ts @@ -22,7 +22,6 @@ export type { TaskRepository, CreateTaskData, UpdateTaskData, - PendingApprovalFilters, } from './task-repository.js'; export type { diff --git a/apps/server/db/repositories/task-repository.ts b/apps/server/db/repositories/task-repository.ts index a621a1e..2131ac6 100644 --- a/apps/server/db/repositories/task-repository.ts +++ b/apps/server/db/repositories/task-repository.ts @@ -20,15 +20,6 @@ export type CreateTaskData = Omit & { */ export type UpdateTaskData = Partial; -/** - * 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; - /** - * Find all tasks with status 'pending_approval'. - * Optional filters by initiative, phase, or category. - * Returns tasks ordered by createdAt. - */ - findPendingApproval(filters?: PendingApprovalFilters): Promise; - /** * Update a task. * Throws if task not found. diff --git a/apps/server/db/schema.ts b/apps/server/db/schema.ts index 1b98e44..1e371db 100644 --- a/apps/server/db/schema.ts +++ b/apps/server/db/schema.ts @@ -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(), diff --git a/apps/server/dispatch/manager.ts b/apps/server/dispatch/manager.ts index 6ea5425..b799e57 100644 --- a/apps/server/dispatch/manager.ts +++ b/apps/server/dispatch/manager.ts @@ -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 { - 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 { - // 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; - } } diff --git a/apps/server/dispatch/phase-manager.test.ts b/apps/server/dispatch/phase-manager.test.ts index 0a9146d..246e141 100644 --- a/apps/server/dispatch/phase-manager.test.ts +++ b/apps/server/dispatch/phase-manager.test.ts @@ -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: [] }), }; diff --git a/apps/server/dispatch/types.ts b/apps/server/dispatch/types.ts index f2e8533..6c86111 100644 --- a/apps/server/dispatch/types.ts +++ b/apps/server/dispatch/types.ts @@ -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; - /** - * 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; - /** * Mark a task as blocked. * Task will not be dispatched until unblocked. diff --git a/apps/server/drizzle/0030_remove_task_approval.sql b/apps/server/drizzle/0030_remove_task_approval.sql new file mode 100644 index 0000000..1f68dbc --- /dev/null +++ b/apps/server/drizzle/0030_remove_task_approval.sql @@ -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. diff --git a/apps/server/drizzle/meta/_journal.json b/apps/server/drizzle/meta/_journal.json index b2df298..ac6687d 100644 --- a/apps/server/drizzle/meta/_journal.json +++ b/apps/server/drizzle/meta/_journal.json @@ -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 } ] } \ No newline at end of file diff --git a/apps/server/events/index.ts b/apps/server/events/index.ts index 2cd7ce9..e53920b 100644 --- a/apps/server/events/index.ts +++ b/apps/server/events/index.ts @@ -32,7 +32,6 @@ export type { TaskDispatchedEvent, TaskCompletedEvent, TaskBlockedEvent, - TaskPendingApprovalEvent, PhaseQueuedEvent, PhaseStartedEvent, PhaseCompletedEvent, diff --git a/apps/server/events/types.ts b/apps/server/events/types.ts index 6d0a7b9..fb6a8a6 100644 --- a/apps/server/events/types.ts +++ b/apps/server/events/types.ts @@ -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 diff --git a/apps/server/test/harness.ts b/apps/server/test/harness.ts index 7773c79..bb5c0ba 100644 --- a/apps/server/test/harness.ts +++ b/apps/server/test/harness.ts @@ -631,7 +631,6 @@ export function createTestHarness(): TestHarness { description, category: 'detail', type: 'auto', - requiresApproval: true, }); }, diff --git a/apps/server/test/integration/full-flow/report.ts b/apps/server/test/integration/full-flow/report.ts index 6fd01a3..7c447ac 100644 --- a/apps/server/test/integration/full-flow/report.ts +++ b/apps/server/test/integration/full-flow/report.ts @@ -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)}`); diff --git a/apps/server/trpc/routers/initiative.ts b/apps/server/trpc/routers/initiative.ts index 3161822..4b8fb66 100644 --- a/apps/server/trpc/routers/initiative.ts +++ b/apps/server/trpc/routers/initiative.ts @@ -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(), })) diff --git a/apps/server/trpc/routers/task.ts b/apps/server/trpc/routers/task.ts index e02f30c..48eedcc 100644 --- a/apps/server/trpc/routers/task.ts +++ b/apps/server/trpc/routers/task.ts @@ -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 }; - }), }; } diff --git a/apps/web/src/components/CreateInitiativeDialog.tsx b/apps/web/src/components/CreateInitiativeDialog.tsx index a025921..c7f05fa 100644 --- a/apps/web/src/components/CreateInitiativeDialog.tsx +++ b/apps/web/src/components/CreateInitiativeDialog.tsx @@ -51,7 +51,6 @@ export function CreateInitiativeDialog({ name: name.trim(), createdAt: new Date().toISOString(), updatedAt: new Date().toISOString(), - mergeRequiresApproval: true, branch: null, projects: [], activity: { state: 'idle' as const, phasesTotal: 0, phasesCompleted: 0 }, diff --git a/apps/web/src/components/InitiativeCard.tsx b/apps/web/src/components/InitiativeCard.tsx index b2b11ee..a4f7e18 100644 --- a/apps/web/src/components/InitiativeCard.tsx +++ b/apps/web/src/components/InitiativeCard.tsx @@ -17,7 +17,6 @@ export interface SerializedInitiative { id: string; name: string; status: "active" | "completed" | "archived"; - mergeRequiresApproval: boolean; branch: string | null; createdAt: string; updatedAt: string; diff --git a/apps/web/src/components/StatusDot.tsx b/apps/web/src/components/StatusDot.tsx index e72a570..f57b454 100644 --- a/apps/web/src/components/StatusDot.tsx +++ b/apps/web/src/components/StatusDot.tsx @@ -39,7 +39,6 @@ export function mapEntityStatus(rawStatus: string): StatusVariant { // Warning / needs attention case "waiting_for_input": - case "pending_approval": case "pending_review": case "approved": case "exhausted": diff --git a/apps/web/src/components/TaskRow.tsx b/apps/web/src/components/TaskRow.tsx index 6803a8c..2fdc909 100644 --- a/apps/web/src/components/TaskRow.tsx +++ b/apps/web/src/components/TaskRow.tsx @@ -15,8 +15,7 @@ export interface SerializedTask { type: "auto" | "checkpoint:human-verify" | "checkpoint:decision" | "checkpoint:human-action"; category: string; priority: "low" | "medium" | "high"; - status: "pending_approval" | "pending" | "in_progress" | "completed" | "blocked"; - requiresApproval: boolean | null; + status: "pending" | "in_progress" | "completed" | "blocked"; order: number; createdAt: string; updatedAt: string; diff --git a/apps/web/src/components/execution/TaskSlideOver.tsx b/apps/web/src/components/execution/TaskSlideOver.tsx index 3987be0..e2df6bf 100644 --- a/apps/web/src/components/execution/TaskSlideOver.tsx +++ b/apps/web/src/components/execution/TaskSlideOver.tsx @@ -1,6 +1,6 @@ import { useCallback, useEffect, useRef, useMemo } from "react"; import { motion, AnimatePresence } from "motion/react"; -import { X, Trash2, MessageCircle, CheckCircle2 } from "lucide-react"; +import { X, Trash2, MessageCircle } from "lucide-react"; import type { ChatTarget } from "@/components/chat/ChatSlideOver"; import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; @@ -20,7 +20,6 @@ interface TaskSlideOverProps { export function TaskSlideOver({ onOpenChat }: TaskSlideOverProps) { const { selectedEntry, setSelectedTaskId } = useExecutionContext(); const queueTaskMutation = trpc.queueTask.useMutation(); - const approveTaskMutation = trpc.approveTask.useMutation(); const deleteTaskMutation = trpc.deleteTask.useMutation(); const updateTaskMutation = trpc.updateTask.useMutation(); @@ -230,32 +229,17 @@ export function TaskSlideOver({ onOpenChat }: TaskSlideOverProps) { {/* Footer */}
- {task.status === "pending_approval" ? ( - - ) : ( - - )} +