From 3cbc4e7df769e9bc42362f7ab8d193a8e26dd963 Mon Sep 17 00:00:00 2001 From: Dotta Date: Tue, 7 Jul 2026 21:26:44 +0000 Subject: [PATCH] Fix built-in agent approval gating --- .../__tests__/built-in-agent-routes.test.ts | 60 +++++++++++- server/src/__tests__/built-in-agents.test.ts | 84 ++++++++++++++++ server/src/routes/built-in-agents.ts | 27 +++++- server/src/services/built-in-agents.ts | 97 ++++++++++++++++++- 4 files changed, 256 insertions(+), 12 deletions(-) diff --git a/server/src/__tests__/built-in-agent-routes.test.ts b/server/src/__tests__/built-in-agent-routes.test.ts index 1c96f8badbf..ceb4e78fc34 100644 --- a/server/src/__tests__/built-in-agent-routes.test.ts +++ b/server/src/__tests__/built-in-agent-routes.test.ts @@ -12,6 +12,7 @@ const mockAccessService = vi.hoisted(() => ({ const mockBuiltInAgentService = vi.hoisted(() => ({ list: vi.fn(), ensure: vi.fn(), + provision: vi.fn(), reset: vi.fn(), })); @@ -96,6 +97,7 @@ describe("built-in agent routes", () => { mockAccessService.decide.mockResolvedValue(allowDecision()); mockBuiltInAgentService.list.mockResolvedValue([builtInState()]); mockBuiltInAgentService.ensure.mockResolvedValue(builtInState()); + mockBuiltInAgentService.provision.mockResolvedValue({ state: builtInState(), approval: null }); mockBuiltInAgentService.reset.mockResolvedValue(builtInState()); }); @@ -149,10 +151,15 @@ describe("built-in agent routes", () => { action: "agents:create", resource: { type: "company", companyId }, }); - expect(mockBuiltInAgentService.ensure).toHaveBeenCalledWith(companyId, "briefs", { - adapterType: "codex_local", - adapterConfig: { model: "gpt-5.4" }, - }); + expect(mockBuiltInAgentService.provision).toHaveBeenCalledWith( + companyId, + "briefs", + { + adapterType: "codex_local", + adapterConfig: { model: "gpt-5.4" }, + }, + { requestedByAgentId: null, requestedByUserId: "board-user" }, + ); expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ companyId, actorType: "user", @@ -179,6 +186,7 @@ describe("built-in agent routes", () => { expect(res.status, JSON.stringify(res.body)).toBe(403); expect(res.body.details).toMatchObject({ reason: "deny_missing_grant" }); expect(mockBuiltInAgentService.ensure).not.toHaveBeenCalled(); + expect(mockBuiltInAgentService.provision).not.toHaveBeenCalled(); }); it("rejects provision bodies with unknown fields", async () => { @@ -196,6 +204,50 @@ describe("built-in agent routes", () => { expect(res.status, JSON.stringify(res.body)).toBe(400); expect(mockBuiltInAgentService.ensure).not.toHaveBeenCalled(); + expect(mockBuiltInAgentService.provision).not.toHaveBeenCalled(); + }); + + it("returns pending hire approvals instead of provisioning immediately when company policy requires it", async () => { + const approval = { + id: "approval-1", + status: "pending", + type: "hire_agent", + }; + mockBuiltInAgentService.provision.mockResolvedValue({ + state: builtInState({ + status: "pending_approval", + agent: { ...builtInState().agent, status: "pending_approval" }, + }), + approval, + }); + const app = await createApp({ + type: "agent", + agentId: "manager-agent", + companyId, + source: "agent_key", + }); + + const res = await request(app) + .post(`/api/companies/${companyId}/built-in-agents/briefs/provision`) + .send({ adapterType: "codex_local", adapterConfig: { model: "gpt-5.4" } }); + + expect(res.status, JSON.stringify(res.body)).toBe(202); + expect(res.body.status).toBe("pending_approval"); + expect(res.body.approval).toMatchObject({ id: "approval-1", status: "pending", type: "hire_agent" }); + expect(mockBuiltInAgentService.ensure).not.toHaveBeenCalled(); + expect(mockBuiltInAgentService.provision).toHaveBeenCalledWith( + companyId, + "briefs", + { adapterType: "codex_local", adapterConfig: { model: "gpt-5.4" } }, + { requestedByAgentId: "manager-agent", requestedByUserId: null }, + ); + expect(mockLogActivity).toHaveBeenCalledWith(expect.anything(), expect.objectContaining({ + companyId, + actorType: "agent", + actorId: "manager-agent", + action: "approval.created", + entityId: "approval-1", + })); }); it("resets registry defaults through the same agents:create gate", async () => { diff --git a/server/src/__tests__/built-in-agents.test.ts b/server/src/__tests__/built-in-agents.test.ts index 05c09a5c489..ed47f6c04a3 100644 --- a/server/src/__tests__/built-in-agents.test.ts +++ b/server/src/__tests__/built-in-agents.test.ts @@ -5,6 +5,7 @@ import { activityLog, agentConfigRevisions, agents, + approvals, companies, createDb, } from "@paperclipai/db"; @@ -14,6 +15,7 @@ import { } from "./helpers/embedded-postgres.js"; import { HttpError } from "../errors.ts"; import { agentService } from "../services/agents.ts"; +import { approvalService } from "../services/approvals.ts"; import { builtInAgentService, deriveBuiltInAgentStatus, @@ -48,6 +50,7 @@ describeEmbeddedPostgres("built-in agents", () => { afterEach(async () => { await db.delete(agentConfigRevisions); await db.delete(activityLog); + await db.delete(approvals); await db.delete(agents); await db.delete(companies); }); @@ -132,6 +135,87 @@ describeEmbeddedPostgres("built-in agents", () => { expect(rows).toHaveLength(1); }); + it("routes policy-gated built-in provisioning through a pending hire approval", async () => { + const companyId = await seedCompany(); + const builtIns = builtInAgentService(db); + + const result = await builtIns.provision(companyId, "briefs", { + adapterType: "process", + adapterConfig: { command: "echo safe" }, + }, { requestedByUserId: "board-user" }); + + expect(result.state).toMatchObject({ + status: "pending_approval", + agent: { + companyId, + name: "Briefs Agent", + status: "pending_approval", + adapterType: "process", + adapterConfig: { command: "echo safe" }, + }, + }); + expect(result.approval).toMatchObject({ + companyId, + type: "hire_agent", + status: "pending", + requestedByUserId: "board-user", + requestedByAgentId: null, + payload: { + name: "Briefs Agent", + role: "general", + adapterType: "process", + adapterConfig: { command: "echo safe" }, + agentId: result.state.agentId, + sourceBuiltInAgentKey: "briefs", + featureKeys: ["briefs"], + }, + }); + + const rowsBeforeApproval = await db.select().from(agents).where(eq(agents.companyId, companyId)); + expect(rowsBeforeApproval).toHaveLength(1); + expect(rowsBeforeApproval[0]).toMatchObject({ status: "pending_approval" }); + + await expect(builtIns.requireBuiltInAgent(companyId, "briefs")).rejects.toMatchObject({ + status: 412, + details: { code: "built_in_agent_not_configured", status: "pending_approval" }, + }); + + await approvalService(db).approve(result.approval!.id, "board-user", "Approved built-in agent"); + + await expect(builtIns.get(companyId, "briefs")).resolves.toMatchObject({ + status: "ready", + agentId: result.state.agentId, + agent: { status: "idle", adapterType: "process", adapterConfig: { command: "echo safe" } }, + }); + }); + + it("blocks policy-gated built-in reconfiguration instead of applying adapter overrides immediately", async () => { + const companyId = await seedCompany(); + const builtIns = builtInAgentService(db); + const ready = await builtIns.ensure(companyId, "briefs", { + adapterType: "codex_local", + adapterConfig: { model: "gpt-5.4" }, + }); + + await expect(builtIns.provision(companyId, "briefs", { + adapterType: "process", + adapterConfig: { command: "echo bypass" }, + })).rejects.toMatchObject({ + status: 409, + details: { + code: "built_in_agent_reconfiguration_requires_approval", + key: "briefs", + agentId: ready.agentId, + }, + }); + + await expect(builtIns.get(companyId, "briefs")).resolves.toMatchObject({ + status: "ready", + agentId: ready.agentId, + agent: { adapterType: "codex_local", adapterConfig: { model: "gpt-5.4" } }, + }); + }); + it("rejects adapter types outside the built-in definition allowlist", async () => { const companyId = await seedCompany(); diff --git a/server/src/routes/built-in-agents.ts b/server/src/routes/built-in-agents.ts index bda6f5f55cb..158741177eb 100644 --- a/server/src/routes/built-in-agents.ts +++ b/server/src/routes/built-in-agents.ts @@ -28,10 +28,11 @@ export function builtInAgentRoutes(db: Db) { req: Request, input: { companyId: string; - action: "built_in_agent.provision_requested" | "built_in_agent.reset"; + action: "built_in_agent.provision_requested" | "built_in_agent.reset" | "approval.created"; key: string; agentId: string | null; status: string; + approvalId?: string | null; }, ) { const actor = getActorInfo(req); @@ -40,13 +41,14 @@ export function builtInAgentRoutes(db: Db) { actorType: actor.actorType, actorId: actor.actorId, action: input.action, - entityType: "agent", - entityId: input.agentId ?? input.key, + entityType: input.action === "approval.created" ? "approval" : "agent", + entityId: input.action === "approval.created" ? input.approvalId ?? input.key : input.agentId ?? input.key, ...(actor.agentId ? { agentId: actor.agentId } : {}), ...(actor.runId ? { runId: actor.runId } : {}), details: { key: input.key, status: input.status, + approvalId: input.approvalId ?? null, }, }); } @@ -64,7 +66,12 @@ export function builtInAgentRoutes(db: Db) { const companyId = req.params.companyId as string; const key = req.params.key as string; await assertCanProvisionBuiltInAgents(req, companyId); - const state = await svc.ensure(companyId, key, req.body); + const actor = getActorInfo(req); + const result = await svc.provision(companyId, key, req.body, { + requestedByAgentId: actor.actorType === "agent" ? actor.actorId : null, + requestedByUserId: actor.actorType === "user" ? actor.actorId : null, + }); + const { state, approval } = result; await logBuiltInAgentMutation(req, { companyId, action: "built_in_agent.provision_requested", @@ -72,7 +79,17 @@ export function builtInAgentRoutes(db: Db) { agentId: state.agentId, status: state.status, }); - res.json(state); + if (approval) { + await logBuiltInAgentMutation(req, { + companyId, + action: "approval.created", + key, + agentId: state.agentId, + status: approval.status, + approvalId: approval.id, + }); + } + res.status(approval ? 202 : 200).json({ ...state, approval }); }, ); diff --git a/server/src/services/built-in-agents.ts b/server/src/services/built-in-agents.ts index 122ef4a56ff..05f1275a777 100644 --- a/server/src/services/built-in-agents.ts +++ b/server/src/services/built-in-agents.ts @@ -1,16 +1,17 @@ import { and, eq, ne } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; import { agents, companies } from "@paperclipai/db"; -import type { Agent } from "@paperclipai/shared"; +import type { Agent, Approval } from "@paperclipai/shared"; import { conflict, HttpError, notFound, unprocessable } from "../errors.js"; import { logActivity } from "./activity-log.js"; import { agentService } from "./agents.js"; +import { approvalService } from "./approvals.js"; import { readBuiltInAgentMarker, withBuiltInAgentMarker, } from "./built-in-agent-metadata.js"; -export type BuiltInAgentStatus = "not_provisioned" | "needs_setup" | "ready" | "paused"; +export type BuiltInAgentStatus = "not_provisioned" | "pending_approval" | "needs_setup" | "ready" | "paused"; export interface BuiltInAgentDefinition { key: string; @@ -36,6 +37,16 @@ export interface BuiltInAgentProvisionInput { adapterConfig?: Record; } +export interface BuiltInAgentProvisionActor { + requestedByAgentId?: string | null; + requestedByUserId?: string | null; +} + +export interface BuiltInAgentProvisionResult { + state: BuiltInAgentState; + approval: Approval | null; +} + export interface RequiredBuiltInAgentWarning { code: "built_in_agent_paused"; key: string; @@ -191,6 +202,7 @@ function hasCompleteAdapterConfig(adapterType: string, adapterConfig: unknown) { export function deriveBuiltInAgentStatus(agent: Pick | null): BuiltInAgentStatus { if (!agent) return "not_provisioned"; + if (agent.status === "pending_approval") return "pending_approval"; if (agent.status === "paused" || agent.pausedAt) return "paused"; return hasCompleteAdapterConfig(agent.adapterType, agent.adapterConfig) ? "ready" : "needs_setup"; } @@ -233,14 +245,19 @@ function rowIsBuiltInAgent(row: typeof agents.$inferSelect, key: string) { export function builtInAgentService(db: Db) { const agentSvc = agentService(db); + const approvalSvc = approvalService(db); async function ensureCompany(companyId: string) { const company = await db - .select({ id: companies.id }) + .select({ + id: companies.id, + requireBoardApprovalForNewAgents: companies.requireBoardApprovalForNewAgents, + }) .from(companies) .where(eq(companies.id, companyId)) .then((rows) => rows[0] ?? null); if (!company) throw notFound("Company not found"); + return company; } async function findMarkedRows(companyId: string, key: string) { @@ -332,6 +349,79 @@ export function builtInAgentService(db: Db) { return state(definition, created); } + async function provision( + companyId: string, + key: string, + input: BuiltInAgentProvisionInput = {}, + actor: BuiltInAgentProvisionActor = {}, + ): Promise { + const definition = requireBuiltInAgentDefinition(key); + const company = await ensureCompany(companyId); + if (!company.requireBoardApprovalForNewAgents) { + return { state: await ensure(companyId, key, input), approval: null }; + } + + const existing = await findSingleAgent(companyId, definition); + if (existing) { + if (existing.status === "pending_approval") { + const approval = await approvalSvc.findOpenHireApprovalForAgent(companyId, existing.id); + return { + state: state(definition, existing), + approval: approval as Approval | null, + }; + } + + if (input.adapterType !== undefined || input.adapterConfig !== undefined) { + throw conflict("Built-in agent adapter changes require board approval before they can be applied.", { + code: "built_in_agent_reconfiguration_requires_approval", + key: definition.key, + agentId: existing.id, + }); + } + + return { state: state(definition, existing), approval: null }; + } + + const pending = await agentSvc.create(companyId, { + ...definitionPatch(definition, input), + status: "pending_approval", + metadata: builtInMetadata(definition), + runtimeConfig: {}, + permissions: {}, + spentMonthlyCents: 0, + lastHeartbeatAt: null, + }, { allowBuiltInAgentMetadata: true }) as Agent; + + const approval = await approvalSvc.create(companyId, { + type: "hire_agent", + requestedByAgentId: actor.requestedByAgentId ?? null, + requestedByUserId: actor.requestedByUserId ?? null, + status: "pending", + payload: { + name: pending.name, + role: pending.role, + title: pending.title, + icon: pending.icon, + reportsTo: pending.reportsTo, + capabilities: pending.capabilities, + adapterType: pending.adapterType, + adapterConfig: pending.adapterConfig, + runtimeConfig: pending.runtimeConfig, + budgetMonthlyCents: pending.budgetMonthlyCents, + metadata: pending.metadata, + agentId: pending.id, + sourceBuiltInAgentKey: definition.key, + featureKeys: definition.featureKeys, + }, + decisionNote: null, + decidedByUserId: null, + decidedAt: null, + updatedAt: new Date(), + }) as Approval; + + return { state: state(definition, pending), approval }; + } + async function list(companyId: string) { await ensureCompany(companyId); return Promise.all(DEFINITIONS.map(async (definition) => state(definition, await findSingleAgent(companyId, definition)))); @@ -387,6 +477,7 @@ export function builtInAgentService(db: Db) { definitions: listBuiltInAgentDefinitions, get, ensure, + provision, list, reset, requireBuiltInAgent,