From 4342cf3e2f0d22a0b395f1afa956f8193850b548 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Thu, 16 Apr 2026 22:38:05 -0400 Subject: [PATCH] refactor(api-server): move approvals SQL into db.approvals domain methods Replace the ad-hoc getApproval helper and the filterable list query in routes-approvals.ts with db.approvals methods. Add findApprovalById (non-throwing lookup returning null) and listApprovalsForApi (accepts requestedByAgentId / relatedTaskId / statuses filters and builds the prepared statement inside the domain layer). --- src/main/api-server/routes-approvals.ts | 41 +++++++------------------ src/main/database-base.ts | 1 + src/main/database/approvals.ts | 14 +++++++++ 3 files changed, 26 insertions(+), 30 deletions(-) diff --git a/src/main/api-server/routes-approvals.ts b/src/main/api-server/routes-approvals.ts index c62fe24c..ac2f73fc 100644 --- a/src/main/api-server/routes-approvals.ts +++ b/src/main/api-server/routes-approvals.ts @@ -1,10 +1,8 @@ import { z } from "zod"; import log from "electron-log/main.js"; -import type { SQLInputValue } from "node:sqlite"; import { approvalInputSchema } from "@shared/contracts"; import { resolveApprovalDecision, shouldAutoApproveApproval } from "../approval-policy"; import { requireParam, type AgentApiServer } from "./server-core"; -import type { AppDatabase } from "../database"; const approvalListQuerySchema = z.object({ requestedByAgentId: z.string().uuid().optional(), @@ -12,21 +10,6 @@ const approvalListQuerySchema = z.object({ status: z.string().optional(), }); -type ApprovalRow = { - id: string; companyId: string; relatedTaskId: string | null; relatedAgentId: string | null; - requestedByAgentId: string | null; type: string; payloadSummary: string | null; - impactSummary: string | null; payloadJson: string | null; decisionNote: string | null; - state: string; createdAt: string; updatedAt: string; -}; - -function getApproval(db: AppDatabase, approvalId: string): ApprovalRow | null { - const rows = db.queryAll( - "select id, company_id as companyId, related_task_id as relatedTaskId, related_agent_id as relatedAgentId, requested_by_agent_id as requestedByAgentId, type, payload_summary as payloadSummary, impact_summary as impactSummary, payload_json as payloadJson, decision_note as decisionNote, state, created_at as createdAt, updated_at as updatedAt from approvals where id = ? limit 1", - approvalId, - ); - return rows[0] ?? null; -} - export function registerApprovalRoutes(server: AgentApiServer) { const db = server.db; const callbacks = server.callbacks; @@ -128,18 +111,16 @@ export function registerApprovalRoutes(server: AgentApiServer) { return; } const statuses = parsedQuery.data.status?.split(",").filter(Boolean) ?? null; - let sql = "select id, company_id as companyId, related_task_id as relatedTaskId, related_agent_id as relatedAgentId, requested_by_agent_id as requestedByAgentId, type, payload_summary as payloadSummary, impact_summary as impactSummary, payload_json as payloadJson, decision_note as decisionNote, state, created_at as createdAt, updated_at as updatedAt from approvals where company_id = ?"; - const sqlArgs: unknown[] = [companyId]; - if (parsedQuery.data.requestedByAgentId) { sql += " and requested_by_agent_id = ?"; sqlArgs.push(parsedQuery.data.requestedByAgentId); } - if (parsedQuery.data.relatedTaskId) { sql += " and related_task_id = ?"; sqlArgs.push(parsedQuery.data.relatedTaskId); } - if (statuses && statuses.length > 0) { sql += ` and state in (${statuses.map(() => "?").join(", ")})`; sqlArgs.push(...statuses); } - sql += " order by created_at desc"; - const approvals = db.queryAll(sql, ...(sqlArgs as SQLInputValue[])); + const approvals = db.listApprovalsForApi(companyId, { + requestedByAgentId: parsedQuery.data.requestedByAgentId, + relatedTaskId: parsedQuery.data.relatedTaskId, + statuses, + }); server.json(res, 200, approvals); }); server.addRoute("GET", "/api/approvals/:approvalId", (_req, res, params, token) => { - const approval = getApproval(db, requireParam(params, "approvalId")); + const approval = db.findApprovalById(requireParam(params, "approvalId")); if (!approval || approval.companyId !== token.companyId) { server.json(res, 404, { error: "Approval not found" }); return; @@ -153,7 +134,7 @@ export function registerApprovalRoutes(server: AgentApiServer) { server.addRoute("GET", "/api/approvals/:approvalId/comments", (_req, res, params, token) => { const approvalId = requireParam(params, "approvalId"); - const approval = getApproval(db, approvalId); + const approval = db.findApprovalById(approvalId); if (!approval) { server.json(res, 404, { error: "Approval not found" }); return; } if (approval.companyId !== token.companyId) { server.json(res, 403, { error: "Company mismatch" }); return; } const comments = db.listApprovalComments(approvalId, approval.companyId); @@ -161,7 +142,7 @@ export function registerApprovalRoutes(server: AgentApiServer) { }); server.addRoute("POST", "/api/approvals/:approvalId/comments", (_req, res, params, token, body) => { - const approval = getApproval(db, requireParam(params, "approvalId")); + const approval = db.findApprovalById(requireParam(params, "approvalId")); if (!approval) { server.json(res, 404, { error: "Approval not found" }); return; } if (approval.companyId !== token.companyId) { server.json(res, 403, { error: "Company mismatch" }); return; } const data = body as Record; @@ -185,7 +166,7 @@ export function registerApprovalRoutes(server: AgentApiServer) { }); server.addRoute("POST", "/api/approvals/:approvalId/resubmit", (_req, res, params, token) => { - const approval = getApproval(db, requireParam(params, "approvalId")); + const approval = db.findApprovalById(requireParam(params, "approvalId")); if (!approval) { server.json(res, 404, { error: "Approval not found" }); return; } if (approval.companyId !== token.companyId) { server.json(res, 403, { error: "Company mismatch" }); return; } if (approval.state !== "revision_requested") { @@ -203,7 +184,7 @@ export function registerApprovalRoutes(server: AgentApiServer) { server.addRoute("GET", "/api/approvals/:approvalId/tasks", (_req, res, params, token) => { const approvalId = requireParam(params, "approvalId"); - const approval = getApproval(db, approvalId); + const approval = db.findApprovalById(approvalId); if (!approval) { server.json(res, 404, { error: "Approval not found" }); return; } if (approval.companyId !== token.companyId) { server.json(res, 403, { error: "Company mismatch" }); return; } const taskIds = db.listTasksForApproval(approvalId); @@ -212,7 +193,7 @@ export function registerApprovalRoutes(server: AgentApiServer) { server.addRoute("POST", "/api/approvals/:approvalId/tasks", (_req, res, params, token, body) => { const approvalId = requireParam(params, "approvalId"); - const approval = getApproval(db, approvalId); + const approval = db.findApprovalById(approvalId); if (!approval) { server.json(res, 404, { error: "Approval not found" }); return; } if (approval.companyId !== token.companyId) { server.json(res, 403, { error: "Company mismatch" }); return; } const data = body as Record; diff --git a/src/main/database-base.ts b/src/main/database-base.ts index 87566079..f3101474 100644 --- a/src/main/database-base.ts +++ b/src/main/database-base.ts @@ -291,6 +291,7 @@ export class AppDatabaseBase { requestApproval(input: { companyId: string; relatedTaskId?: string | null; requestedByAgentId?: string | null; relatedAgentId?: string | null; type: string; payloadSummary: string; impactSummary: string; payloadJson?: string | null }) { return this.approvalsMod.requestApproval(input); } decideApproval(input: { companyId?: string | null; approvalId: string; state: string; decisionNote: string }) { return this.approvalsMod.decideApproval(input); } getApprovalRecord(id: string) { return this.approvalsMod.getApprovalRecord(id); } + findApprovalById(id: string) { return this.approvalsMod.findApprovalById(id); } deleteApproval(id: string, companyId: string) { this.approvalsMod.deleteApproval(id, companyId); } getBlockingApproval(companyId: string, agentId: string, taskId: string | null) { return this.approvalsMod.getBlockingApproval(companyId, agentId, taskId); } resubmitApproval(approvalId: string, companyId: string) { this.approvalsMod.resubmitApproval(approvalId, companyId); } diff --git a/src/main/database/approvals.ts b/src/main/database/approvals.ts index 1fefe595..3cfb85c5 100644 --- a/src/main/database/approvals.ts +++ b/src/main/database/approvals.ts @@ -23,6 +23,20 @@ export function buildApprovalMethods( return row; }, + /** + * Non-throwing lookup by id. Returns null when the approval does not + * exist. Used by approval routes that need to branch on 404 rather + * than catching an exception. + */ + findApprovalById(id: string): (ApprovalRecord & { payloadJson: string | null; relatedAgentId: string | null }) | null { + const row = getDb() + .prepare( + "select id, company_id as companyId, related_task_id as relatedTaskId, requested_by_agent_id as requestedByAgentId, related_agent_id as relatedAgentId, type, payload_summary as payloadSummary, impact_summary as impactSummary, payload_json as payloadJson, decision_note as decisionNote, state, created_at as createdAt, updated_at as updatedAt from approvals where id = ?", + ) + .get(id) as (ApprovalRecord & { payloadJson: string | null; relatedAgentId: string | null }) | undefined; + return row ?? null; + }, + requestApproval(input: { companyId: string; relatedTaskId?: string | null;