mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 17:45:53 +01:00
Fix built-in agent approval gating
This commit is contained in:
1 parent
f1f629dbd0
commit
3cbc4e7df7
4 files changed
+256
-12
No files matched your search
@@ -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 () => {
|
||||
|
||||
@@ -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();
|
||||
|
||||
|
||||
@@ -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 });
|
||||
},
|
||||
);
|
||||
|
||||
|
||||
@@ -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<string, unknown>;
|
||||
}
|
||||
|
||||
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<Agent, "adapterType" | "adapterConfig" | "status" | "pausedAt"> | 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<BuiltInAgentProvisionResult> {
|
||||
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,
|
||||
|
||||
Reference in new issue
Block a user