From 7a1dd647aa5cfbc62ac6bec0295074acfb167730 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Wed, 22 Apr 2026 14:00:24 -0400 Subject: [PATCH] fix(api): make task updates transactional --- src/main/api-server/routes-tasks.ts | 124 ++++------ src/main/database-base.ts | 2 + src/main/database/task-review-guard.ts | 18 ++ src/main/database/task-status-rules.ts | 60 +++++ src/main/database/tasks-status.ts | 106 +-------- src/main/database/tasks-update.ts | 261 +++++++++++++++++++++ src/main/database/tasks.ts | 13 + tests/unit/task-update-transaction.test.ts | 178 ++++++++++++++ 8 files changed, 584 insertions(+), 178 deletions(-) create mode 100644 src/main/database/task-review-guard.ts create mode 100644 src/main/database/task-status-rules.ts create mode 100644 src/main/database/tasks-update.ts create mode 100644 tests/unit/task-update-transaction.test.ts diff --git a/src/main/api-server/routes-tasks.ts b/src/main/api-server/routes-tasks.ts index d961104f..a43f1f00 100644 --- a/src/main/api-server/routes-tasks.ts +++ b/src/main/api-server/routes-tasks.ts @@ -125,38 +125,6 @@ export function registerTaskRoutes(server: AgentApiServer) { return; } - // Deliverable review gate: intercept agent trying to advance a flagged task to "done". - // This MUST succeed — silently skipping review would let unreviewed work advance. - if (update.status === "done" && task.requiresUserReview) { - const docs = db.documentsMod.listImportantDocsForReview({ - companyId: task.companyId, - types: IMPORTANT_DOC_TYPES, - authorAgentId: task.assigneeAgentId ?? null, - taskId: task.id, - limit: 10, - }); - const reviewerAgent = db.entityCrud.getAgent(token.agentId); - const docTitles = docs.map((d) => d.title).join(", "); - db.approvalsMod.requestApproval({ - companyId: task.companyId, - relatedTaskId: task.id, - requestedByAgentId: token.agentId, - type: "deliverable_review", - payloadSummary: `Review deliverable: ${docTitles || task.title}`.slice(0, 280), - impactSummary: `${reviewerAgent.name} reviewed and approved. Awaiting your final review.`.slice(0, 400), - payloadJson: JSON.stringify({ documentIds: docs.map((d) => d.id), documents: docs }), - }); - db.tasks.setTaskRequiresUserReview(task.id, false); - if (task.assigneeAgentId) { - const prodAgent = db.entityCrud.getAgent(task.assigneeAgentId); - if (prodAgent.heartbeatEnabled) { - db.agents.setHeartbeat(task.assigneeAgentId, false, prodAgent.heartbeatIntervalSec || 120); - } - } - db.activityMod.addComment({ companyId: task.companyId, taskId: task.id, authorAgentId: null, authorName: "System", body: "Deliverable submitted for board review." }); - update.status = "in_review"; - } - // Validate status transition const effectiveStatus = update.status ?? task.status; const VALID_STATUSES = new Set(["backlog", "todo", "in_progress", "in_review", "done", "blocked", "cancelled"]); @@ -165,69 +133,63 @@ export function registerTaskRoutes(server: AgentApiServer) { return; } - // Apply status side-effect timestamps FIRST via advanceTaskStatus - // so started_at / completed_at / cancelled_at are set correctly. - // advanceTaskStatus checks previousStatus !== newStatus internally, - // so it must run before saveTask writes the new status. - if (update.status && update.status !== task.status) { - db.tasks.advanceTaskStatus(task.id, update.status); - } - - const input: Record = { - id: task.id, - companyId: task.companyId, - title: update.title ?? task.title, - description: update.description ?? task.description, - status: effectiveStatus, - priority: update.priority ?? task.priority, - projectId: task.projectId, - goalId: task.goalId, - parentId: task.parentId, - assigneeAgentId: newAssignee, - workspaceId: task.workspaceId, - requiresFilesystem: update.requiresFilesystem ?? task.requiresFilesystem, - }; - db.tasks.saveTask(input as Parameters[0]); - - if (update.comment) { - const agent = db.entityCrud.getAgent(token.agentId); - db.activityMod.addComment({ + const actorAgent = db.entityCrud.getAgent(token.agentId); + let reviewRequest: { reviewerAgentName: string; documents: Array<{ id: string; title: string } & Record> } | null = null; + if (effectiveStatus === "done" && task.requiresUserReview) { + const docs = db.documentsMod.listImportantDocsForReview({ companyId: task.companyId, + types: IMPORTANT_DOC_TYPES, + authorAgentId: task.assigneeAgentId ?? null, taskId: task.id, - authorAgentId: token.agentId, - authorName: agent.name, - body: update.comment, + limit: 10, }); + reviewRequest = { reviewerAgentName: actorAgent.name, documents: docs }; } - db.activityMod.addActivity({ + const updateResult = db.tasks.updateTaskWithReviewSideEffects({ + taskId: task.id, companyId: task.companyId, - actor: token.agentId, - action: `task.${update.status ? "status_changed" : "updated"}`, - entityType: "task", - entityId: task.id, - detail: update.comment ?? `Status → ${update.status ?? task.status}`, + actorAgentId: token.agentId, + actorName: actorAgent.name, + task: { + title: update.title ?? task.title, + description: update.description ?? task.description, + status: effectiveStatus, + priority: update.priority ?? task.priority, + projectId: task.projectId, + goalId: task.goalId, + parentId: task.parentId, + assigneeAgentId: newAssignee, + workspaceId: task.workspaceId, + requiresFilesystem: update.requiresFilesystem ?? task.requiresFilesystem, + }, + comment: update.comment, + reviewRequest, + activity: { + action: `task.${update.status ? "status_changed" : "updated"}`, + detail: update.comment ?? `Status -> ${update.status ?? task.status}`, + }, }); onDomainChanged(); // Fire automation rules for status changes - const previousStatus = task.status; - const newStatus = update.status ?? task.status; - if (update.status && previousStatus !== newStatus) { + const previousStatus = updateResult.previousStatus; + const newStatus = updateResult.newStatus; + if (update.status && updateResult.statusChanged) { callbacks.onTaskStatusChanged(task.id, task.companyId, previousStatus, newStatus); } - if (update.status && ["done", "in_review"].includes(update.status)) { - if (task.parentId) { + if (update.status && ["done", "in_review"].includes(newStatus)) { + if (updateResult.parentId) { try { - const parentTask = db.entityCrud.getTask(task.parentId); + const parentTask = db.entityCrud.getTask(updateResult.parentId); if (parentTask.assigneeAgentId && parentTask.assigneeAgentId !== token.agentId) { onWakeAgent(parentTask.assigneeAgentId, task.companyId, "subtask_completed"); } } catch (error) { log.warn( - `[api/tasks] parent task lookup failed for ${task.parentId}: ${error instanceof Error ? error.message : String(error)}`, + `[api/tasks] parent task lookup failed for ${updateResult.parentId}: ${error instanceof Error ? error.message : String(error)}`, ); } } else { @@ -242,16 +204,16 @@ export function registerTaskRoutes(server: AgentApiServer) { } } - if (update.status === "blocked") { + if (update.status && newStatus === "blocked") { const chain = db.agents.getChainOfCommand(token.agentId); if (chain.length > 0) { onWakeAgent(chain[0].id, task.companyId, "report_blocked"); } } - if (update.status && ["in_progress", "todo"].includes(update.status) && - task.assigneeAgentId && task.assigneeAgentId !== token.agentId) { - onWakeAgent(task.assigneeAgentId, task.companyId, "assignment"); + if (update.status && ["in_progress", "todo"].includes(newStatus) && + updateResult.newAssigneeAgentId && updateResult.newAssigneeAgentId !== token.agentId) { + onWakeAgent(updateResult.newAssigneeAgentId, task.companyId, "assignment"); } if (update.comment) { @@ -270,8 +232,8 @@ export function registerTaskRoutes(server: AgentApiServer) { } // Wake new assignee when task is reassigned - if (newAssignee && newAssignee !== task.assigneeAgentId && newAssignee !== token.agentId) { - onWakeAgent(newAssignee, task.companyId, "assignment"); + if (updateResult.newAssigneeAgentId && updateResult.newAssigneeAgentId !== updateResult.previousAssigneeAgentId && updateResult.newAssigneeAgentId !== token.agentId) { + onWakeAgent(updateResult.newAssigneeAgentId, task.companyId, "assignment"); } server.json(res, 200, { ok: true }); diff --git a/src/main/database-base.ts b/src/main/database-base.ts index 93196654..db4452fd 100644 --- a/src/main/database-base.ts +++ b/src/main/database-base.ts @@ -134,6 +134,8 @@ export class AppDatabaseBase { this.tasks = buildTaskMethods(getDb, ra, tx, { addComment: (input) => this.activityMod.addComment(input), addActivity: (input) => this.activityMod.addActivity(input), + requestApproval: (input) => this.approvalsMod.requestApproval(input), + setHeartbeat: (agentId, enabled, intervalSec) => this.agents.setHeartbeat(agentId, enabled, intervalSec), }); this.messages = buildMessageMethods(getDb, ra, { getAgent: (id) => this.entityCrud.getAgent(id), diff --git a/src/main/database/task-review-guard.ts b/src/main/database/task-review-guard.ts new file mode 100644 index 00000000..ab27ab04 --- /dev/null +++ b/src/main/database/task-review-guard.ts @@ -0,0 +1,18 @@ +import type { ReadAll } from "./helpers"; + +export function guardInReviewHasReviewer(readAll: ReadAll, taskId: string): void { + const row = readAll<{ requiresUserReview: number; reportsTo: string | null }>( + `select t.requires_user_review as requiresUserReview, + a.reports_to as reportsTo + from tasks t + left join agents a on a.id = t.assignee_agent_id + where t.id = ?`, + taskId, + )[0]; + if (!row) return; + if (row.requiresUserReview === 1) return; + if (row.reportsTo !== null) return; + throw new Error( + `Task ${taskId} has a top-level assignee and no reviewer: cannot enter in_review without requires_user_review=1.`, + ); +} diff --git a/src/main/database/task-status-rules.ts b/src/main/database/task-status-rules.ts new file mode 100644 index 00000000..c3c96254 --- /dev/null +++ b/src/main/database/task-status-rules.ts @@ -0,0 +1,60 @@ +export const VALID_TASK_STATUSES = new Set([ + "backlog", + "todo", + "in_progress", + "in_review", + "done", + "blocked", + "cancelled", +]); + +const STATUSES_THAT_CLEAR_LEASE = new Set([ + "done", + "cancelled", + "blocked", + "todo", + "backlog", +]); + +export function assertValidTaskStatus(status: string): void { + if (!VALID_TASK_STATUSES.has(status)) { + throw new Error(`Invalid task status: ${status}`); + } +} + +export function buildTaskStatusTransition( + newStatus: string, + timestamp: string, +): { sets: string[]; values: (string | null)[]; clearsLease: boolean } { + const sets: string[] = ["status = ?", "updated_at = ?"]; + const values: (string | null)[] = [newStatus, timestamp]; + + if (newStatus === "in_progress") { + sets.push("started_at = COALESCE(started_at, ?)"); + values.push(timestamp); + } + if (newStatus === "done") { + sets.push("completed_at = ?"); + values.push(timestamp); + } else { + sets.push("completed_at = NULL"); + } + if (newStatus === "cancelled") { + sets.push("cancelled_at = ?"); + values.push(timestamp); + } else { + sets.push("cancelled_at = NULL"); + } + if (newStatus !== "in_progress") { + sets.push("checkout_run_id = NULL"); + } + if (newStatus === "todo" || newStatus === "backlog") { + sets.push("active_run_id = NULL"); + } + + return { + sets, + values, + clearsLease: STATUSES_THAT_CLEAR_LEASE.has(newStatus), + }; +} diff --git a/src/main/database/tasks-status.ts b/src/main/database/tasks-status.ts index e6a79a18..e9007eaf 100644 --- a/src/main/database/tasks-status.ts +++ b/src/main/database/tasks-status.ts @@ -8,6 +8,8 @@ import { } from "./helpers"; import { chooseBestTaskAssignee } from "./task-assignment"; import { TASK_LEASE_TTL_MS, checkoutTaskInTransaction } from "./task-checkout"; +import { guardInReviewHasReviewer } from "./task-review-guard"; +import { assertValidTaskStatus, buildTaskStatusTransition } from "./task-status-rules"; export function buildTaskStatusMethods( getDb: () => DatabaseSync, @@ -18,20 +20,6 @@ export function buildTaskStatusMethods( addActivity: (input: { companyId: string; actor: string; action: string; entityType: string; entityId: string; detail: string }) => void; }, ) { - // Terminal phases and "returned to queue" phases release any existing - // lease: once a task is done/cancelled/blocked or bounced back to - // todo/backlog the execution grant no longer has a referent. We mirror - // that in `task_leases` explicitly — the status→phase trigger only - // touches the phase column and leaves the lease row behind, producing - // an orphan that the dev-mode consistency probe surfaces as a - // divergence. Production would silently drift instead. - const STATUSES_THAT_CLEAR_LEASE = new Set([ - "done", - "cancelled", - "blocked", - "todo", - "backlog", - ]); function incrementActiveSprintVelocity(companyId: string, delta: number): void { const ts = nowIso(); const today = ts.slice(0, 10); @@ -42,106 +30,30 @@ export function buildTaskStatusMethods( .run(delta, ts, companyId, today, today); } - const VALID_TASK_STATUSES = new Set([ - "backlog", - "todo", - "in_progress", - "in_review", - "done", - "blocked", - "cancelled", - ]); - - /** - * Guard against the "in_review dead letter": a task whose assignee is - * a top-level agent (reports_to IS NULL, the DB invariant CEO marker) - * and that does not require user review has no reachable reviewer — - * no manager to walk up to, and no human gate. Without this guard the - * CEO heartbeat loops forever re-proposing the same approval. - * - * The right root fix is to reject the transition at source: a caller - * that moves such a task into `in_review` must first set - * requires_user_review=1 (escalate to human) or route differently - * (advance to done, hire a reviewer, bounce to todo). Previous - * versions silently resurrected these tasks at every DB init; this - * replaces the startup side-effect with a clear runtime error. - */ - function guardInReviewHasReviewer(taskId: string): void { - const row = readAll<{ requiresUserReview: number; reportsTo: string | null }>( - `select t.requires_user_review as requiresUserReview, - a.reports_to as reportsTo - from tasks t - left join agents a on a.id = t.assignee_agent_id - where t.id = ?`, - taskId, - )[0]; - if (!row) return; - if (row.requiresUserReview === 1) return; - if (row.reportsTo !== null) return; - throw new Error( - `Task ${taskId} has a top-level assignee and no reviewer: cannot enter in_review without requires_user_review=1.`, - ); - } - return { advanceTaskStatus( taskId: string, newStatus: string, comment?: string, ): { previousStatus: string } { - if (!VALID_TASK_STATUSES.has(newStatus)) { - throw new Error(`Invalid task status: ${newStatus}`); - } + assertValidTaskStatus(newStatus); const task = getDb() .prepare("select status, company_id from tasks where id = ?") .get(taskId) as { status: string; company_id: string } | undefined; if (!task) throw new Error(`Task not found: ${taskId}`); const previousStatus = task.status; if (previousStatus === newStatus) return { previousStatus }; - if (newStatus === "in_review") guardInReviewHasReviewer(taskId); + if (newStatus === "in_review") guardInReviewHasReviewer(readAll, taskId); const ts = nowIso(); - - // Status side-effect timestamps (Paperclip pattern) - const timestampSets: string[] = ["status = ?", "updated_at = ?"]; - const timestampValues: (string | null)[] = [newStatus, ts]; - - if (newStatus === "in_progress") { - // Set startedAt only once (first time entering in_progress) - timestampSets.push("started_at = COALESCE(started_at, ?)"); - timestampValues.push(ts); - } - if (newStatus === "done") { - timestampSets.push("completed_at = ?"); - timestampValues.push(ts); - } else { - timestampSets.push("completed_at = NULL"); - } - if (newStatus === "cancelled") { - timestampSets.push("cancelled_at = ?"); - timestampValues.push(ts); - } else { - timestampSets.push("cancelled_at = NULL"); - } - if (newStatus !== "in_progress") { - timestampSets.push("checkout_run_id = NULL"); - } - // Returning a task to todo / backlog drops any active-run claim - // as well — heartbeat picks candidate tasks by !activeRunId, so - // leaving a stale pointer here (e.g. after startup recovery of a - // crashed run) silently hides the task from every future wake. - if (newStatus === "todo" || newStatus === "backlog") { - timestampSets.push("active_run_id = NULL"); - } - - const shouldClearLease = STATUSES_THAT_CLEAR_LEASE.has(newStatus); + const transition = buildTaskStatusTransition(newStatus, ts); transaction(() => { getDb() .prepare( - // eslint-disable-next-line no-restricted-syntax -- B2 Phase 1 grandfathered: advanceTaskStatus is the sanctioned dynamic-SQL helper; timestampSets[0] is always the literal "status = ?". Status→phase trigger plus the explicit DELETE FROM task_leases below keep the shadow surface coherent. - `UPDATE tasks SET ${timestampSets.join(", ")} WHERE id = ?`, + // eslint-disable-next-line no-restricted-syntax -- B2 Phase 1 grandfathered: advanceTaskStatus is the sanctioned dynamic-SQL helper; transition.sets[0] is always the literal "status = ?". Status→phase trigger plus the explicit DELETE FROM task_leases below keep the shadow surface coherent. + `UPDATE tasks SET ${transition.sets.join(", ")} WHERE id = ?`, ) - .run(...timestampValues, taskId); - if (shouldClearLease) { + .run(...transition.values, taskId); + if (transition.clearsLease) { // Drop any live lease row in the same transaction so no // caller observes the intermediate state of "task in terminal // or backlog phase, lease still present". `releaseTaskLock` diff --git a/src/main/database/tasks-update.ts b/src/main/database/tasks-update.ts new file mode 100644 index 00000000..8dd7012d --- /dev/null +++ b/src/main/database/tasks-update.ts @@ -0,0 +1,261 @@ +import type { DatabaseSync } from "node:sqlite"; +import type { ReadAll, Transaction } from "./helpers"; +import { normalizeOptionalId, nowIso } from "./helpers"; +import { guardInReviewHasReviewer } from "./task-review-guard"; +import { assertValidTaskStatus, buildTaskStatusTransition } from "./task-status-rules"; + +type TaskUpdatePatch = { + title: string; + description: string; + status: string; + priority: string; + projectId?: string | null; + goalId?: string | null; + parentId?: string | null; + assigneeAgentId?: string | null; + workspaceId?: string | null; + requiresFilesystem: boolean; +}; + +type ReviewDocument = { id: string; title: string } & Record; + +type TaskReviewRequest = { + reviewerAgentName: string; + documents: ReviewDocument[]; +}; + +type TaskUpdateActivity = { + action: string; + detail: string; +}; + +type ApprovalRequestInput = { + companyId: string; + relatedTaskId?: string | null; + requestedByAgentId?: string | null; + relatedAgentId?: string | null; + type: string; + payloadSummary: string; + impactSummary: string; + payloadJson?: string | null; +}; + +type TaskUpdateResult = { + previousStatus: string; + newStatus: string; + previousAssigneeAgentId: string | null; + newAssigneeAgentId: string | null; + parentId: string | null; + reviewRequested: boolean; + statusChanged: boolean; +}; + +export function buildTaskUpdateMethods( + getDb: () => DatabaseSync, + readAll: ReadAll, + transaction: Transaction, + deps: { + addComment: (input: { companyId: string; taskId: string; authorAgentId: string | null; authorName: string; body: string }) => void; + addActivity: (input: { companyId: string; actor: string; action: string; entityType: string; entityId: string; detail: string }) => void; + requestApproval: (input: ApprovalRequestInput) => string; + setHeartbeat: (agentId: string, enabled: boolean, intervalSec: number) => void; + }, +) { + function incrementActiveSprintVelocity(companyId: string, delta: number): void { + const ts = nowIso(); + const today = ts.slice(0, 10); + getDb() + .prepare( + "update sprints set completed_points = max(0, completed_points + ?), updated_at = ? where company_id = ? and status = 'active' and start_date <= ? and end_date >= ?", + ) + .run(delta, ts, companyId, today, today); + } + + function pauseAssigneeHeartbeat(agentId: string | null): void { + if (!agentId) return; + const agent = getDb() + .prepare("select heartbeat_enabled as heartbeatEnabled, heartbeat_interval_sec as heartbeatIntervalSec from agents where id = ?") + .get(agentId) as { heartbeatEnabled: number; heartbeatIntervalSec: number | null } | undefined; + if (!agent?.heartbeatEnabled) return; + deps.setHeartbeat(agentId, false, agent.heartbeatIntervalSec || 120); + } + + function requestDeliverableReview(input: { + companyId: string; + taskId: string; + actorAgentId: string; + taskTitle: string; + currentAssigneeAgentId: string | null; + reviewRequest: TaskReviewRequest; + }): void { + const docTitles = input.reviewRequest.documents.map((document) => document.title).join(", "); + deps.requestApproval({ + companyId: input.companyId, + relatedTaskId: input.taskId, + requestedByAgentId: input.actorAgentId, + type: "deliverable_review", + payloadSummary: `Review deliverable: ${docTitles || input.taskTitle}`.slice(0, 280), + impactSummary: `${input.reviewRequest.reviewerAgentName} reviewed and approved. Awaiting your final review.`.slice(0, 400), + payloadJson: JSON.stringify({ + documentIds: input.reviewRequest.documents.map((document) => document.id), + documents: input.reviewRequest.documents, + }), + }); + getDb() + .prepare("update tasks set requires_user_review = 0, updated_at = ? where id = ? and company_id = ?") + .run(nowIso(), input.taskId, input.companyId); + pauseAssigneeHeartbeat(input.currentAssigneeAgentId); + deps.addComment({ + companyId: input.companyId, + taskId: input.taskId, + authorAgentId: null, + authorName: "System", + body: "Deliverable submitted for board review.", + }); + } + + return { + updateTaskWithReviewSideEffects(input: { + taskId: string; + companyId: string; + actorAgentId: string; + actorName: string; + task: TaskUpdatePatch; + comment?: string; + reviewRequest?: TaskReviewRequest | null; + activity: TaskUpdateActivity; + }): TaskUpdateResult { + assertValidTaskStatus(input.task.status); + const assigneeAgentId = normalizeOptionalId(input.task.assigneeAgentId); + if (input.task.status === "in_progress" && !assigneeAgentId) { + throw new Error("Tasks in progress require an assignee."); + } + + return transaction(() => { + const current = getDb() + .prepare( + "select id, company_id as companyId, status, title, assignee_agent_id as assigneeAgentId, parent_id as parentId, requires_user_review as requiresUserReview from tasks where id = ? and company_id = ?", + ) + .get(input.taskId, input.companyId) as + | { + id: string; + companyId: string; + status: string; + title: string; + assigneeAgentId: string | null; + parentId: string | null; + requiresUserReview: number; + } + | undefined; + if (!current) { + throw new Error(`Task not found: ${input.taskId}`); + } + + const reviewRequested = + input.task.status === "done" && + current.requiresUserReview === 1 && + input.reviewRequest !== null && + input.reviewRequest !== undefined; + if (input.task.status === "done" && current.requiresUserReview === 1 && !input.reviewRequest) { + throw new Error(`Task ${input.taskId} requires a deliverable review request before it can be marked done.`); + } + const newStatus = reviewRequested ? "in_review" : input.task.status; + assertValidTaskStatus(newStatus); + + if (reviewRequested) { + requestDeliverableReview({ + companyId: input.companyId, + taskId: input.taskId, + actorAgentId: input.actorAgentId, + taskTitle: current.title, + currentAssigneeAgentId: current.assigneeAgentId, + reviewRequest: input.reviewRequest!, + }); + } + + const statusChanged = current.status !== newStatus; + if (statusChanged && newStatus === "in_review") { + guardInReviewHasReviewer(readAll, input.taskId); + } + + const ts = nowIso(); + const sets = [ + "project_id = ?", + "goal_id = ?", + "parent_id = ?", + "title = ?", + "description = ?", + "assignee_agent_id = ?", + "workspace_id = ?", + "priority = ?", + "requires_filesystem = ?", + ]; + const values: Array = [ + normalizeOptionalId(input.task.projectId), + normalizeOptionalId(input.task.goalId), + normalizeOptionalId(input.task.parentId), + input.task.title, + input.task.description, + assigneeAgentId, + normalizeOptionalId(input.task.workspaceId), + input.task.priority, + input.task.requiresFilesystem ? 1 : 0, + ]; + const transition = statusChanged ? buildTaskStatusTransition(newStatus, ts) : null; + if (transition) { + sets.push(...transition.sets); + values.push(...transition.values); + } else { + sets.push("status = ?", "updated_at = ?"); + values.push(newStatus, ts); + } + + const updateResult = getDb() + // eslint-disable-next-line no-restricted-syntax -- R-03 sanctioned domain update helper: SET clauses come from fixed literals only; status->phase trigger and explicit task_leases delete keep the dual-write surface coherent inside this transaction. + .prepare(`update tasks set ${sets.join(", ")} where id = ? and company_id = ?`) + .run(...values, input.taskId, input.companyId); + if (updateResult.changes === 0) { + throw new Error(`Task not found: ${input.taskId}`); + } + if (transition?.clearsLease) { + getDb() + .prepare("DELETE FROM task_leases WHERE task_id = ?") + .run(input.taskId); + } + if (newStatus === "done" && current.status !== "done") { + incrementActiveSprintVelocity(input.companyId, 1); + } + if (current.status === "done" && newStatus !== "done") { + incrementActiveSprintVelocity(input.companyId, -1); + } + if (input.comment) { + deps.addComment({ + companyId: input.companyId, + taskId: input.taskId, + authorAgentId: input.actorAgentId, + authorName: input.actorName, + body: input.comment, + }); + } + deps.addActivity({ + companyId: input.companyId, + actor: input.actorAgentId, + action: input.activity.action, + entityType: "task", + entityId: input.taskId, + detail: input.activity.detail, + }); + + return { + previousStatus: current.status, + newStatus, + previousAssigneeAgentId: current.assigneeAgentId, + newAssigneeAgentId: assigneeAgentId, + parentId: normalizeOptionalId(input.task.parentId), + reviewRequested, + statusChanged, + }; + }); + }, + }; +} diff --git a/src/main/database/tasks.ts b/src/main/database/tasks.ts index 11a87fb1..0afbd055 100644 --- a/src/main/database/tasks.ts +++ b/src/main/database/tasks.ts @@ -5,6 +5,7 @@ import { buildTaskQueryMethods } from "./task-queries"; import { buildTaskSessionMethods } from "./task-sessions-crud"; import { buildTaskStatusMethods } from "./tasks-status"; import { buildTasksPhaseMethods } from "./tasks-phase"; +import { buildTaskUpdateMethods } from "./tasks-update"; export function buildTaskMethods( getDb: () => DatabaseSync, @@ -13,6 +14,17 @@ export function buildTaskMethods( deps: { addComment: (input: { companyId: string; taskId: string; authorAgentId: string | null; authorName: string; body: string }) => void; addActivity: (input: { companyId: string; actor: string; action: string; entityType: string; entityId: string; detail: string }) => void; + requestApproval: (input: { + companyId: string; + relatedTaskId?: string | null; + requestedByAgentId?: string | null; + relatedAgentId?: string | null; + type: string; + payloadSummary: string; + impactSummary: string; + payloadJson?: string | null; + }) => string; + setHeartbeat: (agentId: string, enabled: boolean, intervalSec: number) => void; }, ) { return { @@ -20,6 +32,7 @@ export function buildTaskMethods( ...buildTaskQueryMethods(getDb, readAll, transaction), ...buildTaskSessionMethods(getDb, readAll, transaction), ...buildTaskStatusMethods(getDb, readAll, transaction, deps), + ...buildTaskUpdateMethods(getDb, readAll, transaction, deps), ...buildTasksPhaseMethods(getDb), }; } diff --git a/tests/unit/task-update-transaction.test.ts b/tests/unit/task-update-transaction.test.ts new file mode 100644 index 00000000..f4965f4c --- /dev/null +++ b/tests/unit/task-update-transaction.test.ts @@ -0,0 +1,178 @@ +import { join } from "node:path"; +import { tmpdir } from "node:os"; +import { mkdtemp, rm } from "node:fs/promises"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("electron", () => ({ + safeStorage: { + isEncryptionAvailable: () => false, + getSelectedStorageBackend: () => "basic_text", + encryptString: (value: string) => Buffer.from(value, "utf8"), + decryptString: (value: Buffer) => value.toString("utf8"), + }, +})); + +import { AppDatabase } from "../../src/main/database"; + +function seedCompany(db: AppDatabase) { + const companyId = db.company.saveCompany({ + name: "Review Co", + description: "Task update transaction tests", + status: "active", + }); + db.connectorsCrud.saveConnector({ + id: "openai-codex", + model: "gpt-5", + envBindingText: "", + notes: "fixture", + }); + const managerId = db.agents.saveAgent({ + companyId, + name: "Manager", + role: "VP Engineering", + title: "Manager", + department: "engineering", + status: "active", + reportsTo: null, + connectorId: "openai-codex", + workspaceId: null, + model: null, + capabilities: "Review", + budgetMonthlyUsd: 100, + }); + const workerId = db.agents.saveAgent({ + companyId, + name: "Worker", + role: "Engineer", + title: "Engineer", + department: "engineering", + status: "active", + reportsTo: managerId, + connectorId: "openai-codex", + workspaceId: null, + model: null, + capabilities: "Build", + budgetMonthlyUsd: 100, + }); + return { companyId, managerId, workerId }; +} + +function createReviewTask(db: AppDatabase, input: { companyId: string; assigneeAgentId: string }) { + const taskId = db.tasks.saveTask({ + companyId: input.companyId, + title: "Ship review packet", + description: "Prepare a review packet.", + assigneeAgentId: input.assigneeAgentId, + priority: "high", + status: "todo", + requiresFilesystem: false, + }); + db.tasks.setTaskRequiresUserReview(taskId, true); + return taskId; +} + +describe("updateTaskWithReviewSideEffects", () => { + let tempDir: string; + let db: AppDatabase; + + beforeEach(async () => { + tempDir = await mkdtemp(join(tmpdir(), "task-update-transaction-")); + db = new AppDatabase(tempDir); + db.init(); + }); + + afterEach(async () => { + db.close(); + await rm(tempDir, { recursive: true, force: true }); + }); + + it("rolls back review approval, flag clear, and comments when the status guard rejects", () => { + const { companyId, managerId } = seedCompany(db); + const taskId = createReviewTask(db, { companyId, assigneeAgentId: managerId }); + + expect(() => + db.tasks.updateTaskWithReviewSideEffects({ + taskId, + companyId, + actorAgentId: managerId, + actorName: "Manager", + task: { + title: "Ship review packet", + description: "Prepare a review packet.", + status: "done", + priority: "high", + projectId: null, + goalId: null, + parentId: null, + assigneeAgentId: managerId, + workspaceId: null, + requiresFilesystem: false, + }, + comment: "Ready for board review.", + reviewRequest: { + reviewerAgentName: "Manager", + documents: [], + }, + activity: { + action: "task.status_changed", + detail: "Ready for board review.", + }, + }), + ).toThrow(/cannot enter in_review/); + + const task = db.entityCrud.getTask(taskId); + expect(task.status).toBe("todo"); + expect(task.requiresUserReview).toBe(true); + expect(db.snap.listSnapshot().approvals).toHaveLength(0); + expect(db.activityMod.listComments(taskId)).toHaveLength(0); + }); + + it("commits review approval, flag clear, status, main row, comment, and activity together", () => { + const { companyId, managerId, workerId } = seedCompany(db); + const taskId = createReviewTask(db, { companyId, assigneeAgentId: workerId }); + + const result = db.tasks.updateTaskWithReviewSideEffects({ + taskId, + companyId, + actorAgentId: managerId, + actorName: "Manager", + task: { + title: "Ship review packet v2", + description: "Prepare a review packet with final notes.", + status: "done", + priority: "critical", + projectId: null, + goalId: null, + parentId: null, + assigneeAgentId: workerId, + workspaceId: null, + requiresFilesystem: false, + }, + comment: "Ready for board review.", + reviewRequest: { + reviewerAgentName: "Manager", + documents: [{ id: "doc-1", title: "Review Packet" }], + }, + activity: { + action: "task.status_changed", + detail: "Ready for board review.", + }, + }); + + const task = db.entityCrud.getTask(taskId); + const approvals = db.snap.listSnapshot().approvals; + expect(result.previousStatus).toBe("todo"); + expect(result.newStatus).toBe("in_review"); + expect(result.reviewRequested).toBe(true); + expect(task.status).toBe("in_review"); + expect(task.title).toBe("Ship review packet v2"); + expect(task.priority).toBe("critical"); + expect(task.requiresUserReview).toBe(false); + expect(approvals).toHaveLength(1); + expect(approvals[0]?.type).toBe("deliverable_review"); + expect(db.activityMod.listComments(taskId).map((comment) => comment.body)).toEqual([ + "Deliverable submitted for board review.", + "Ready for board review.", + ]); + }); +});