From e5682d22fa8f7c0dd01ddb5e9cc42d941db6fe43 Mon Sep 17 00:00:00 2001 From: Dotta Date: Tue, 27 Oct 2026 05:16:18 -0500 Subject: [PATCH] fix: retain approving budget authority and guard built-in provisioning Co-Authored-By: Paperclip --- maps/agent-access-map/agents.md | 2 +- .../agent-permissions-routes.test.ts | 7 +++++++ .../__tests__/built-in-agent-routes.test.ts | 8 ++++++++ server/src/__tests__/built-in-agents.test.ts | 8 ++++++++ server/src/lib/agent-records.ts | 1 + .../__tests__/lifecycle.test.ts | 20 ++++++++++++++++--- .../agent-lifecycle/adapters/records.ts | 4 ++-- server/src/routes/agents.ts | 4 +++- server/src/routes/built-in-agents.ts | 4 +++- server/src/services/built-in-agents.ts | 14 +++++++------ 10 files changed, 58 insertions(+), 14 deletions(-) diff --git a/maps/agent-access-map/agents.md b/maps/agent-access-map/agents.md index a0c36a72553..be4d8f4078d 100644 --- a/maps/agent-access-map/agents.md +++ b/maps/agent-access-map/agents.md @@ -132,7 +132,7 @@ CLI. Policy: Agent creation/configuration uses action grants, resource policies and change consent; some controls require a board actor. -The legacy `budgetMonthlyCents` field requires a board actor for updates or budget-changing rollbacks. Agent-authored rollbacks with an unchanged amount do not write budget policy or policy audit. Board mutations retain the actual operator identity. +Changes to the legacy `budgetMonthlyCents` field and built-in provision budget overrides require a board actor, including budget-changing rollbacks. Agent-authored rollbacks with an unchanged amount do not write budget policy or policy audit. Board mutations retain the actual operator identity. The roster intent preserves unrelated settings, rejects stale model/connection previews and harness changes, and checks the full execution configuration under diff --git a/server/src/__tests__/agent-permissions-routes.test.ts b/server/src/__tests__/agent-permissions-routes.test.ts index 126874a32c2..31cf885ae1b 100644 --- a/server/src/__tests__/agent-permissions-routes.test.ts +++ b/server/src/__tests__/agent-permissions-routes.test.ts @@ -2344,6 +2344,7 @@ describe("agent permission routes", () => { }); it.each([0, 5000])("budget authority: rejects agent policy changes through the legacy field (%i)", async amount => { + mockAgentService.getById.mockResolvedValue({ ...baseAgent, budgetMonthlyCents: 500 }); const app = createApp({ type: "agent", agentId, companyId, source: "agent_key", runId: "run-1" }); const response = await requestApp(app, url => request(url).patch(`/api/agents/${agentId}`).send({ budgetMonthlyCents: amount })); expect(response.status).toBe(403); @@ -2351,6 +2352,12 @@ describe("agent permission routes", () => { expect(mockBudgetService.upsertPolicy).not.toHaveBeenCalled(); expect(mockLogActivity).not.toHaveBeenCalled(); }); + it("budget authority: accepts an unchanged legacy amount with an unrelated title edit", async () => { + const app = createApp({ type: "agent", agentId, companyId, source: "agent_key" }); + const response = await requestApp(app, url => request(url).patch(`/api/agents/${agentId}`).send({ budgetMonthlyCents: 0, title: "New title" })); + expect(response.status).toBe(200); + expect(mockAgentService.update).toHaveBeenCalled(); + }); it.each(["agents", "agent-hires"])("budget authority: rejects agent immediate creation budgets through %s", async route => { mockAccessService.hasPermission.mockResolvedValue(true); const app = createApp({ type: "agent", agentId, companyId, source: "agent_key" }); diff --git a/server/src/__tests__/built-in-agent-routes.test.ts b/server/src/__tests__/built-in-agent-routes.test.ts index e51166f2ba4..c07fc56985d 100644 --- a/server/src/__tests__/built-in-agent-routes.test.ts +++ b/server/src/__tests__/built-in-agent-routes.test.ts @@ -204,6 +204,14 @@ describe("built-in agent routes", () => { })); }); + it.each([0, 5000])("rejects an agent budget override through built-in provision (%s)", async budgetMonthlyCents => { + const app = await createApp({ type: "agent", agentId: "agent-1", companyId, source: "agent_key" }); + const response = await request(app).post(`/api/companies/${companyId}/built-in-agents/briefs/provision`).send({ budgetMonthlyCents }); + expect(response.status).toBe(403); + expect(mockBuiltInAgentService.provision).not.toHaveBeenCalled(); + expect(mockLogActivity).not.toHaveBeenCalled(); + }); + it("denies provision when agents:create is not allowed", async () => { mockAccessService.decide.mockResolvedValue(denyDecision()); const app = await createApp({ diff --git a/server/src/__tests__/built-in-agents.test.ts b/server/src/__tests__/built-in-agents.test.ts index c43f911d6e7..e6989b1798b 100644 --- a/server/src/__tests__/built-in-agents.test.ts +++ b/server/src/__tests__/built-in-agents.test.ts @@ -266,6 +266,14 @@ describeEmbeddedPostgres("built-in agents", () => { ])).toThrow("defaultAdapterType must be allowed"); }); + it("retains the actual operator on a provisioned existing agent budget", async () => { + const companyId = await seedCompany({ requireApproval: false }); + const svc = builtInAgentService(db); + const original = await svc.ensure(companyId, "briefs"); + await svc.provision(companyId, "briefs", { budgetMonthlyCents: 5000 }, { requestedByUserId: "budget-operator" }); + expect(await db.select().from(budgetPolicies).where(eq(budgetPolicies.scopeId, original.agentId!))).toMatchObject([{ amount: 5000, createdByUserId: "budget-operator" }]); + }); + it.each([{ platform: "linux", architecture: "x64" }, { platform: "linux", architecture: "arm64" }, { platform: "darwin", architecture: "arm64" }, { platform: "darwin", architecture: "x64" }])("lazily provisions one agent per company/key and updates the same row on setup ($platform/$architecture)", async ({ platform, architecture }) => { runnerTarget.platform = platform; runnerTarget.architecture = architecture; diff --git a/server/src/lib/agent-records.ts b/server/src/lib/agent-records.ts index 25c0ed7c9f5..a27f564ee20 100644 --- a/server/src/lib/agent-records.ts +++ b/server/src/lib/agent-records.ts @@ -88,6 +88,7 @@ export type AgentConfigurationPatch = Partial; export interface CreateAgentOptions { + initialBudgetPolicyUserId?: string; setupOnly?: boolean; setupAgentId?: string; runnerResolved?: boolean; diff --git a/server/src/modules/agent-lifecycle/__tests__/lifecycle.test.ts b/server/src/modules/agent-lifecycle/__tests__/lifecycle.test.ts index 9e5a3e7b856..e2de5993e6e 100644 --- a/server/src/modules/agent-lifecycle/__tests__/lifecycle.test.ts +++ b/server/src/modules/agent-lifecycle/__tests__/lifecycle.test.ts @@ -299,14 +299,28 @@ const support = await getEmbeddedPostgresTestSupport(); }); it("saves the immediate hire budget and actual operator audit in its transaction", async () => { - const agent = await createAgentLifecycle(db).requestHire(companyId, { name: "Capped hire", adapterType: "process", budgetMonthlyCents: 5000 }, { createdByUserId: "operator-user" }); + const agent = await createAgentLifecycle(db).requestHire(companyId, { name: "Capped hire", adapterType: "process", budgetMonthlyCents: 5000 }, { createdByUserId: "operator-user", initialBudgetPolicyUserId: "operator-user" }); expect(await db.select().from(budgetPolicies).where(eq(budgetPolicies.scopeId, agent.id))).toMatchObject([{ amount: 5000 }]); const audit = await db.select().from(activityLog).where(and(eq(activityLog.companyId, companyId), eq(activityLog.action, "budget.policy_upserted"))); expect(audit).toMatchObject([{ actorType: "user", actorId: "operator-user", agentId: null }]); }); + it("preserves trusted no-user provisioning without inventing initial board authority", async () => { + const agent = await createAgentLifecycle(db).requestHire(companyId, { name: "Internal declared budget", budgetMonthlyCents: 5000 }); + expect(agent.budgetMonthlyCents).toBe(5000); + expect(await db.select().from(budgetPolicies).where(eq(budgetPolicies.scopeId, agent.id))).toHaveLength(0); + expect(await db.select().from(activityLog).where(and(eq(activityLog.companyId, companyId), eq(activityLog.action, "budget.policy_upserted")))).toHaveLength(0); + }); + + it("credits only the approving operator for a positive-budget approval without a precreated agent", async () => { + const approval = await approvalService(db).create(companyId, { type: "hire_agent", requestedByUserId: "requester", payload: { name: "Approved cap", role: "engineer", budgetMonthlyCents: 5000 } }); + await approvalService(db).approve(approval.id, "approver"); + expect(await db.select().from(budgetPolicies).where(eq(budgetPolicies.companyId, companyId))).toMatchObject([{ amount: 5000, createdByUserId: "approver" }]); + expect(await db.select().from(activityLog).where(and(eq(activityLog.companyId, companyId), eq(activityLog.action, "budget.policy_upserted")))).toMatchObject([{ actorType: "user", actorId: "approver" }]); + }); + it("does not activate a proposed hire budget before human approval", async () => { - const agent = await createAgentLifecycle(db).requestHire(companyId, { name: "Proposed cap", status: "pending_approval", budgetMonthlyCents: 5000 }, { createdByUserId: "operator-user" }); + const agent = await createAgentLifecycle(db).requestHire(companyId, { name: "Proposed cap", status: "pending_approval", budgetMonthlyCents: 5000 }, { createdByUserId: "operator-user", initialBudgetPolicyUserId: "operator-user" }); expect(agent.budgetMonthlyCents).toBe(5000); expect(await db.select().from(budgetPolicies).where(eq(budgetPolicies.scopeId, agent.id))).toHaveLength(0); }); @@ -318,7 +332,7 @@ const support = await getEmbeddedPostgresTestSupport(); throw new Error("Initial budget failed"); }); const lifecycle = createLifecycleCommands(db, { ...effects, setAgentBudget }); - await expect(lifecycle.requestHire(companyId, { name: "Budget rollback", adapterType: "process", budgetMonthlyCents: 5000 }, { createdByUserId: "operator-user" })) + await expect(lifecycle.requestHire(companyId, { name: "Budget rollback", adapterType: "process", budgetMonthlyCents: 5000 }, { createdByUserId: "operator-user", initialBudgetPolicyUserId: "operator-user" })) .rejects.toThrow("Initial budget failed"); expect(setAgentBudget).toHaveBeenCalledWith(expect.anything(), expect.any(Array), companyId, expect.any(String), 5000, "operator-user"); expect(await db.select().from(agents).where(eq(agents.companyId, companyId))).toHaveLength(0); diff --git a/server/src/modules/agent-lifecycle/adapters/records.ts b/server/src/modules/agent-lifecycle/adapters/records.ts index c568f0826fa..e76c0fd5589 100644 --- a/server/src/modules/agent-lifecycle/adapters/records.ts +++ b/server/src/modules/agent-lifecycle/adapters/records.ts @@ -59,8 +59,8 @@ export function agentRecords(db: Db, effects: LifecycleEffects) { .then((rows) => rows[0]); await effects.initializeHire(txDb, created, options); if (!options?.setupOnly && status !== "pending_approval" && status !== "terminated" - && created.budgetMonthlyCents > 0 && options?.createdByUserId) { - await effects.setAgentBudget(txDb, publications, companyId, created.id, created.budgetMonthlyCents, options.createdByUserId); + && created.budgetMonthlyCents > 0 && options?.initialBudgetPolicyUserId) { + await effects.setAgentBudget(txDb, publications, companyId, created.id, created.budgetMonthlyCents, options.initialBudgetPolicyUserId); } if (!options?.setupOnly && created.status !== "pending_approval" && created.status !== "terminated") { await effects.recordCreation(txDb, companyId, created.id); diff --git a/server/src/routes/agents.ts b/server/src/routes/agents.ts index f1ae00f1abc..13b700599fe 100644 --- a/server/src/routes/agents.ts +++ b/server/src/routes/agents.ts @@ -4566,6 +4566,7 @@ export function agentRoutes( { ...(setupAgentId ? { setupAgentId } : {}), createdByUserId: req.actor.type === "board" ? req.actor.userId : null, + ...(req.actor.type === "board" && req.body.budgetMonthlyCents > 0 ? { initialBudgetPolicyUserId: getActorInfo(req).actorId } : {}), runnerResolved: true, responsibleUserId: responsibleUserForAiRequest(req), aiConnectionInstall: managedHireConnection ? { ...managedHireConnection, createdByUserId: responsibleUserForAiRequest(req) } : undefined, @@ -4849,6 +4850,7 @@ export function agentRoutes( { ...(setupId ? { setupOnly: true } : {}), createdByUserId: req.actor.type === "board" ? req.actor.userId : null, + ...(req.actor.type === "board" && req.body.budgetMonthlyCents > 0 ? { initialBudgetPolicyUserId: getActorInfo(req).actorId } : {}), runnerResolved: true, responsibleUserId: responsibleUserForAiRequest(req), aiConnectionInstall: managedConnection ? { ...managedConnection, createdByUserId: responsibleUserForAiRequest(req) } : undefined, @@ -5385,7 +5387,7 @@ export function agentRoutes( const id = req.params.id as string; const existing = await getAccessibleResource(req, res, svc.getById(id), "Agent not found"); if (!existing) return; - if (hasOwn(req.body as object, "budgetMonthlyCents")) assertBoard(req); + if (hasOwn(req.body as object, "budgetMonthlyCents") && req.body.budgetMonthlyCents !== existing.budgetMonthlyCents) assertBoard(req); if (hasOwn(req.body as object, "permissions")) { res.status(422).json({ error: "Use /api/agents/:id/permissions for permission changes" }); diff --git a/server/src/routes/built-in-agents.ts b/server/src/routes/built-in-agents.ts index c881ae9c93f..087bda77c85 100644 --- a/server/src/routes/built-in-agents.ts +++ b/server/src/routes/built-in-agents.ts @@ -6,7 +6,7 @@ import { forbidden, notFound } from "../errors.js"; import { accessService, instanceSettingsService, logActivity } from "../services/index.js"; import { builtInAgentService } from "../services/built-in-agents.js"; import { authorizationDeniedDetails } from "../services/authorization.js"; -import { assertCompanyAccess, getActorInfo } from "./authz.js"; +import { assertBoard, assertCompanyAccess, getActorInfo } from "./authz.js"; import type { BuiltInAgentState } from "../services/built-in-agents.js"; const WEEKDAY_LABELS = ["Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"]; @@ -186,8 +186,10 @@ export function builtInAgentRoutes(db: Db) { await assertBuiltInAgentsEnabled(); await assertCanProvisionBuiltInAgents(req, companyId); const actor = getActorInfo(req); + if (req.body.budgetMonthlyCents !== undefined) assertBoard(req); const result = await svc.provision(companyId, key, req.body, { requestedByAgentId: actor.actorType === "agent" ? actor.actorId : null, + ...(actor.runId ? { sourceRunId: actor.runId } : {}), requestedByUserId: actor.actorType === "user" ? actor.actorId : null, }); const { state, approval } = result; diff --git a/server/src/services/built-in-agents.ts b/server/src/services/built-in-agents.ts index bdadbe6c1c5..3e015da3d54 100644 --- a/server/src/services/built-in-agents.ts +++ b/server/src/services/built-in-agents.ts @@ -75,6 +75,7 @@ export interface BuiltInAgentProvisionInput { } export interface BuiltInAgentProvisionActor { + sourceRunId?: string | null; requestedByAgentId?: string | null; requestedByUserId?: string | null; } @@ -1729,7 +1730,7 @@ export function builtInAgentService(db: Db) { companyId: string, key: string, input: BuiltInAgentProvisionInput = {}, - options: { isRaceRetry?: boolean; createdByUserId?: string | null } = {}, + options: { isRaceRetry?: boolean; createdByUserId?: string | null; createdByAgentId?: string | null; sourceRunId?: string | null } = {}, ) { const definition = requireBuiltInAgentDefinition(key); await ensureCompany(companyId); @@ -1797,7 +1798,7 @@ export function builtInAgentService(db: Db) { } const updated = await agentSvc.update(existing.id, patch, { allowBuiltInAgentMetadata: true, - recordRevision: { source: "built-in-agent:ensure" }, + recordRevision: { source: "built-in-agent:ensure", createdByUserId: options.createdByUserId, createdByAgentId: options.createdByAgentId, sourceRunId: options.sourceRunId }, }); if (!updated) throw notFound("Built-in agent not found"); if (existingPendingApproval) { @@ -1832,13 +1833,14 @@ export function builtInAgentService(db: Db) { permissions: definition.defaultPermissions ?? {}, spentMonthlyCents: 0, lastHeartbeatAt: null, - }, { allowBuiltInAgentMetadata: true, createdByUserId: options.createdByUserId ?? null }) as Agent; + }, { allowBuiltInAgentMetadata: true, createdByUserId: options.createdByUserId ?? null, + ...(options.createdByUserId && resolvedInput.budgetMonthlyCents ? { initialBudgetPolicyUserId: options.createdByUserId } : {}) }) as Agent; } catch (error) { // Lost the provisioning race: a concurrent writer inserted the row first // and the partial unique index rejected ours. Re-run once; the winning // row now exists, so we take the update path instead of inserting again. if (!options.isRaceRetry && isBuiltInAgentMarkerConflict(error)) { - return ensure(companyId, key, input, { isRaceRetry: true, createdByUserId: options.createdByUserId }); + return ensure(companyId, key, input, { isRaceRetry: true, createdByUserId: options.createdByUserId, createdByAgentId: options.createdByAgentId, sourceRunId: options.sourceRunId }); } throw error; } @@ -1871,7 +1873,7 @@ export function builtInAgentService(db: Db) { const definition = requireBuiltInAgentDefinition(key); const company = await ensureCompany(companyId); if (!company.requireBoardApprovalForNewAgents) { - return { state: await ensure(companyId, key, input, { createdByUserId: actor.requestedByUserId ?? null }), approval: null }; + return { state: await ensure(companyId, key, input, { createdByUserId: actor.requestedByUserId ?? null, createdByAgentId: actor.requestedByAgentId ?? null, sourceRunId: actor.sourceRunId }), approval: null }; } await assertKnownBuiltInAgentModel(definition, input); @@ -1903,7 +1905,7 @@ export function builtInAgentService(db: Db) { // it does when board approval is not required, instead of dead-ending on a // fresh board-approval requirement the operator can never satisfy. if (providesAdapterSetup && (existing.metadata?.paperclipBuiltInSetupRequired === true || !hasCompleteAdapterConfig(existing.adapterType, existing.adapterConfig))) { - return { state: await ensure(companyId, key, input), approval: null }; + return { state: await ensure(companyId, key, input, { createdByUserId: actor.requestedByUserId ?? null, createdByAgentId: actor.requestedByAgentId ?? null, sourceRunId: actor.sourceRunId }), approval: null }; } // Changing the adapter of an already-configured (`ready`/`paused`)