mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-27 19:35:13 +01:00
Freeze pending approval agent config
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
ac3156a6d4
commit
ad454e62f5
6 files changed
+263
-6
No files matched your search
@@ -0,0 +1,158 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
|
||||
import { eq } from "drizzle-orm";
|
||||
import {
|
||||
agents,
|
||||
approvals,
|
||||
activityLog,
|
||||
budgetPolicies,
|
||||
companies,
|
||||
createDb,
|
||||
} from "@paperclipai/db";
|
||||
import {
|
||||
getEmbeddedPostgresTestSupport,
|
||||
startEmbeddedPostgresTestDatabase,
|
||||
} from "./helpers/embedded-postgres.js";
|
||||
import { agentService } from "../services/agents.ts";
|
||||
import { approvalService } from "../services/approvals.ts";
|
||||
|
||||
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
|
||||
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
|
||||
|
||||
function issuePrefix(id: string) {
|
||||
return `T${id.replace(/-/g, "").slice(0, 6).toUpperCase()}`;
|
||||
}
|
||||
|
||||
if (!embeddedPostgresSupport.supported) {
|
||||
console.warn(
|
||||
`Skipping embedded Postgres pending approval agent tests on this host: ${embeddedPostgresSupport.reason ?? "unsupported environment"}`,
|
||||
);
|
||||
}
|
||||
|
||||
describeEmbeddedPostgres("pending approval agent config integrity", () => {
|
||||
let db!: ReturnType<typeof createDb>;
|
||||
let tempDb: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | null = null;
|
||||
|
||||
beforeAll(async () => {
|
||||
tempDb = await startEmbeddedPostgresTestDatabase("paperclip-pending-agent-config-");
|
||||
db = createDb(tempDb.connectionString);
|
||||
}, 20_000);
|
||||
|
||||
afterEach(async () => {
|
||||
await db.delete(activityLog);
|
||||
await db.delete(budgetPolicies);
|
||||
await db.delete(approvals);
|
||||
await db.delete(agents);
|
||||
await db.delete(companies);
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await tempDb?.cleanup();
|
||||
});
|
||||
|
||||
async function seedCompany() {
|
||||
const companyId = randomUUID();
|
||||
await db.insert(companies).values({
|
||||
id: companyId,
|
||||
name: "Paperclip",
|
||||
issuePrefix: issuePrefix(companyId),
|
||||
requireBoardApprovalForNewAgents: true,
|
||||
});
|
||||
return companyId;
|
||||
}
|
||||
|
||||
it("freezes generic pending hire config and reapplies the approval snapshot on activation", async () => {
|
||||
const companyId = await seedCompany();
|
||||
const agentSvc = agentService(db);
|
||||
const approvalSvc = approvalService(db);
|
||||
const pending = await agentSvc.create(companyId, {
|
||||
name: "Pending Coder",
|
||||
role: "engineer",
|
||||
title: "Software Engineer",
|
||||
icon: "code",
|
||||
capabilities: "Writes code",
|
||||
adapterType: "process",
|
||||
adapterConfig: { command: "echo safe" },
|
||||
runtimeConfig: { maxConcurrentRuns: 1 },
|
||||
budgetMonthlyCents: 1234,
|
||||
metadata: { source: "hire-form" },
|
||||
status: "pending_approval",
|
||||
spentMonthlyCents: 0,
|
||||
permissions: {},
|
||||
lastHeartbeatAt: null,
|
||||
});
|
||||
const approval = await approvalSvc.create(companyId, {
|
||||
type: "hire_agent",
|
||||
requestedByAgentId: null,
|
||||
requestedByUserId: "board-user",
|
||||
status: "pending",
|
||||
payload: {
|
||||
name: "Pending Coder",
|
||||
role: "engineer",
|
||||
title: "Software Engineer",
|
||||
icon: "code",
|
||||
reportsTo: null,
|
||||
capabilities: "Writes code",
|
||||
adapterType: "process",
|
||||
adapterConfig: { command: "echo safe" },
|
||||
runtimeConfig: { maxConcurrentRuns: 1 },
|
||||
budgetMonthlyCents: 1234,
|
||||
metadata: { source: "hire-form" },
|
||||
agentId: pending.id,
|
||||
},
|
||||
decisionNote: null,
|
||||
decidedByUserId: null,
|
||||
decidedAt: null,
|
||||
updatedAt: new Date(),
|
||||
});
|
||||
|
||||
await expect(agentSvc.update(pending.id, {
|
||||
name: "Tampered Coder",
|
||||
adapterConfig: { command: "echo malicious" },
|
||||
runtimeConfig: { maxConcurrentRuns: 99 },
|
||||
})).rejects.toMatchObject({
|
||||
status: 409,
|
||||
details: {
|
||||
code: "pending_approval_agent_config_frozen",
|
||||
agentId: pending.id,
|
||||
fields: ["name", "adapterConfig", "runtimeConfig"],
|
||||
},
|
||||
});
|
||||
await expect(agentSvc.updatePermissions(pending.id, {
|
||||
canCreateAgents: true,
|
||||
})).rejects.toMatchObject({
|
||||
status: 409,
|
||||
details: {
|
||||
code: "pending_approval_agent_config_frozen",
|
||||
agentId: pending.id,
|
||||
fields: ["permissions"],
|
||||
},
|
||||
});
|
||||
|
||||
await db
|
||||
.update(agents)
|
||||
.set({
|
||||
name: "Tampered Coder",
|
||||
adapterConfig: { command: "echo malicious" },
|
||||
runtimeConfig: { maxConcurrentRuns: 99 },
|
||||
metadata: { source: "tampered" },
|
||||
})
|
||||
.where(eq(agents.id, pending.id));
|
||||
|
||||
await approvalSvc.approve(approval.id, "board-user", "Approved generic hire");
|
||||
|
||||
await expect(agentSvc.getById(pending.id)).resolves.toMatchObject({
|
||||
status: "idle",
|
||||
name: "Pending Coder",
|
||||
role: "engineer",
|
||||
title: "Software Engineer",
|
||||
icon: "code",
|
||||
capabilities: "Writes code",
|
||||
adapterType: "process",
|
||||
adapterConfig: { command: "echo safe" },
|
||||
runtimeConfig: { maxConcurrentRuns: 1 },
|
||||
budgetMonthlyCents: 1234,
|
||||
metadata: { source: "hire-form" },
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -101,7 +101,7 @@ describe("approvalService resolution idempotency", () => {
|
||||
const result = await svc.approve("approval-1", "board", "ship it");
|
||||
|
||||
expect(result.applied).toBe(true);
|
||||
expect(mockAgentService.activatePendingApproval).toHaveBeenCalledWith("agent-1");
|
||||
expect(mockAgentService.activatePendingApproval).toHaveBeenCalledWith("agent-1", approved.payload);
|
||||
expect(mockNotifyHireApproved).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
|
||||
@@ -180,6 +180,26 @@ describeEmbeddedPostgres("built-in agents", () => {
|
||||
details: { code: "built_in_agent_not_configured", status: "pending_approval" },
|
||||
});
|
||||
|
||||
await expect(agentService(db).update(result.state.agentId!, {
|
||||
adapterType: "process",
|
||||
adapterConfig: { command: "echo tampered" },
|
||||
})).rejects.toMatchObject({
|
||||
status: 409,
|
||||
details: {
|
||||
code: "pending_approval_agent_config_frozen",
|
||||
agentId: result.state.agentId,
|
||||
fields: ["adapterConfig"],
|
||||
},
|
||||
});
|
||||
|
||||
await db
|
||||
.update(agents)
|
||||
.set({
|
||||
adapterType: "process",
|
||||
adapterConfig: { command: "echo tampered" },
|
||||
})
|
||||
.where(eq(agents.id, result.state.agentId!));
|
||||
|
||||
await approvalService(db).approve(result.approval!.id, "board-user", "Approved built-in agent");
|
||||
|
||||
await expect(builtIns.get(companyId, "briefs")).resolves.toMatchObject({
|
||||
|
||||
@@ -1271,7 +1271,9 @@ export function agentRoutes(
|
||||
delete nextAdapterConfig.bootstrapPromptTemplate;
|
||||
if (!hadLegacyPrompt) return agent;
|
||||
|
||||
const updated = await svc.update(agent.id, { adapterConfig: nextAdapterConfig });
|
||||
const updated = await svc.update(agent.id, { adapterConfig: nextAdapterConfig }, {
|
||||
allowPendingApprovalConfigUpdate: true,
|
||||
});
|
||||
return (updated as T | null) ?? { ...agent, adapterConfig: nextAdapterConfig };
|
||||
}
|
||||
|
||||
@@ -1286,7 +1288,9 @@ export function agentRoutes(
|
||||
delete nextAdapterConfig.promptTemplate;
|
||||
delete nextAdapterConfig.bootstrapPromptTemplate;
|
||||
|
||||
const updated = await svc.update(agent.id, { adapterConfig: nextAdapterConfig });
|
||||
const updated = await svc.update(agent.id, { adapterConfig: nextAdapterConfig }, {
|
||||
allowPendingApprovalConfigUpdate: true,
|
||||
});
|
||||
return (updated as T | null) ?? { ...agent, adapterConfig: nextAdapterConfig };
|
||||
}
|
||||
|
||||
|
||||
@@ -47,6 +47,7 @@ const CONFIG_REVISION_FIELDS = [
|
||||
"name",
|
||||
"role",
|
||||
"title",
|
||||
"icon",
|
||||
"reportsTo",
|
||||
"capabilities",
|
||||
"adapterType",
|
||||
@@ -70,6 +71,7 @@ interface RevisionMetadata {
|
||||
interface UpdateAgentOptions {
|
||||
recordRevision?: RevisionMetadata;
|
||||
allowBuiltInAgentMetadata?: boolean;
|
||||
allowPendingApprovalConfigUpdate?: boolean;
|
||||
}
|
||||
|
||||
interface CreateAgentOptions {
|
||||
@@ -113,6 +115,7 @@ function buildConfigSnapshot(
|
||||
name: row.name,
|
||||
role: row.role,
|
||||
title: row.title,
|
||||
icon: row.icon,
|
||||
reportsTo: row.reportsTo,
|
||||
capabilities: row.capabilities,
|
||||
adapterType: row.adapterType,
|
||||
@@ -135,6 +138,47 @@ function hasConfigPatchFields(data: Partial<typeof agents.$inferInsert>) {
|
||||
return CONFIG_REVISION_FIELDS.some((field) => Object.prototype.hasOwnProperty.call(data, field));
|
||||
}
|
||||
|
||||
function changedPendingApprovalConfigFields(
|
||||
existing: typeof agents.$inferSelect,
|
||||
data: Partial<typeof agents.$inferInsert>,
|
||||
) {
|
||||
return CONFIG_REVISION_FIELDS.filter((field) =>
|
||||
Object.prototype.hasOwnProperty.call(data, field) && !jsonEqual(data[field], existing[field]),
|
||||
);
|
||||
}
|
||||
|
||||
function configPatchFromApprovalPayload(payload: Record<string, unknown>) {
|
||||
const patch: Partial<typeof agents.$inferInsert> = {};
|
||||
if (typeof payload.name === "string") patch.name = payload.name;
|
||||
if (typeof payload.role === "string") patch.role = payload.role;
|
||||
if (Object.prototype.hasOwnProperty.call(payload, "title")) {
|
||||
patch.title = typeof payload.title === "string" ? payload.title : null;
|
||||
}
|
||||
if (Object.prototype.hasOwnProperty.call(payload, "icon")) {
|
||||
patch.icon = typeof payload.icon === "string" ? payload.icon : null;
|
||||
}
|
||||
if (Object.prototype.hasOwnProperty.call(payload, "reportsTo")) {
|
||||
patch.reportsTo = typeof payload.reportsTo === "string" ? payload.reportsTo : null;
|
||||
}
|
||||
if (Object.prototype.hasOwnProperty.call(payload, "capabilities")) {
|
||||
patch.capabilities = typeof payload.capabilities === "string" ? payload.capabilities : null;
|
||||
}
|
||||
if (typeof payload.adapterType === "string") patch.adapterType = payload.adapterType;
|
||||
if (isPlainRecord(payload.adapterConfig)) patch.adapterConfig = payload.adapterConfig;
|
||||
if (isPlainRecord(payload.runtimeConfig)) patch.runtimeConfig = payload.runtimeConfig;
|
||||
if (Object.prototype.hasOwnProperty.call(payload, "defaultEnvironmentId")) {
|
||||
patch.defaultEnvironmentId =
|
||||
typeof payload.defaultEnvironmentId === "string" ? payload.defaultEnvironmentId : null;
|
||||
}
|
||||
if (typeof payload.budgetMonthlyCents === "number") {
|
||||
patch.budgetMonthlyCents = payload.budgetMonthlyCents;
|
||||
}
|
||||
if (Object.prototype.hasOwnProperty.call(payload, "metadata")) {
|
||||
patch.metadata = isPlainRecord(payload.metadata) ? payload.metadata : null;
|
||||
}
|
||||
return patch;
|
||||
}
|
||||
|
||||
function parseFiniteNumberLike(value: unknown): number | null {
|
||||
if (typeof value === "number" && Number.isFinite(value)) return value;
|
||||
if (typeof value !== "string") return null;
|
||||
@@ -433,6 +477,16 @@ export function agentService(db: Db) {
|
||||
) {
|
||||
throw conflict("Pending approval agents cannot be activated directly");
|
||||
}
|
||||
if (existing.status === "pending_approval" && !options?.allowPendingApprovalConfigUpdate) {
|
||||
const changedFields = changedPendingApprovalConfigFields(existing as typeof agents.$inferSelect, data);
|
||||
if (changedFields.length > 0) {
|
||||
throw conflict("Pending approval agent configuration cannot be changed before board approval", {
|
||||
code: "pending_approval_agent_config_frozen",
|
||||
agentId: id,
|
||||
fields: changedFields,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
if (data.reportsTo !== undefined) {
|
||||
if (data.reportsTo) {
|
||||
@@ -712,12 +766,26 @@ export function agentService(db: Db) {
|
||||
});
|
||||
},
|
||||
|
||||
activatePendingApproval: async (id: string) => {
|
||||
activatePendingApproval: async (id: string, approvedPayload?: Record<string, unknown> | null) => {
|
||||
const activatedAgent = await db.transaction(async (tx) => {
|
||||
const txDb = tx as unknown as Db;
|
||||
const existing = await agentService(txDb).getById(id);
|
||||
if (!existing || existing.status !== "pending_approval") return null;
|
||||
const approvedPatch = approvedPayload ? configPatchFromApprovalPayload(approvedPayload) : {};
|
||||
let patch = { ...approvedPatch } as Partial<typeof agents.$inferInsert>;
|
||||
if (
|
||||
Object.prototype.hasOwnProperty.call(patch, "adapterConfig") &&
|
||||
isPlainRecord(patch.adapterConfig)
|
||||
) {
|
||||
patch.adapterConfig = await secretService(txDb).normalizeAdapterConfigForPersistence(
|
||||
existing.companyId,
|
||||
patch.adapterConfig,
|
||||
{ adapterType: (patch.adapterType ?? existing.adapterType) as string },
|
||||
);
|
||||
}
|
||||
const updated = await tx
|
||||
.update(agents)
|
||||
.set({ status: "idle", updatedAt: new Date() })
|
||||
.set({ ...patch, status: "idle", updatedAt: new Date() })
|
||||
.where(and(eq(agents.id, id), eq(agents.status, "pending_approval")))
|
||||
.returning()
|
||||
.then((rows) => rows[0] ?? null);
|
||||
@@ -741,6 +809,13 @@ export function agentService(db: Db) {
|
||||
updatePermissions: async (id: string, permissions: Record<string, unknown> & { canCreateAgents: boolean }) => {
|
||||
const existing = await getById(id);
|
||||
if (!existing) return null;
|
||||
if (existing.status === "pending_approval") {
|
||||
throw conflict("Pending approval agent permissions cannot be changed before board approval", {
|
||||
code: "pending_approval_agent_config_frozen",
|
||||
agentId: id,
|
||||
fields: ["permissions"],
|
||||
});
|
||||
}
|
||||
|
||||
const updated = await db
|
||||
.update(agents)
|
||||
|
||||
@@ -128,7 +128,7 @@ export function approvalService(db: Db) {
|
||||
const payload = updated.payload as Record<string, unknown>;
|
||||
const payloadAgentId = typeof payload.agentId === "string" ? payload.agentId : null;
|
||||
if (payloadAgentId) {
|
||||
await agentsSvc.activatePendingApproval(payloadAgentId);
|
||||
await agentsSvc.activatePendingApproval(payloadAgentId, payload);
|
||||
hireApprovedAgentId = payloadAgentId;
|
||||
} else {
|
||||
const created = await agentsSvc.create(updated.companyId, {
|
||||
|
||||
Reference in new issue
Block a user