mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-03 09:37:19 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Hiring a new agent creates a `hire_agent` approval record; until it is resolved, the agent sits in `pending_approval` and the open approval surfaces an "Approve/Reject" card in the inbox > - There are two places to act on that hire: the inbox approval card and the agent detail page's Approve/Terminate buttons > - The agent detail page only flipped the agent to `idle` via `activatePendingApproval`, never resolving the linked approval record > - So after approving (or terminating) from the detail page, the approval stayed `pending` and the inbox kept showing a stale "Approve/Reject" card for an agent that was already decided > - This pull request routes the detail-page approve through the shared `approvalsSvc.approve()` (which resolves the approval and runs activation, budget policy, and the hire-approved notification), and rejects the linked approval when a still-pending agent is terminated > - The benefit is a single source of truth: deciding a hire in one place clears it everywhere, so the inbox no longer asks you to approve an agent you already approved ## Linked Issues or Issue Description No public GitHub issue exists for this; describing in-PR (bug report). **What happened:** Create a new agent, then approve it from the agent detail page. The agent activates, but the inbox still shows a "Hire Agent" item with Approve/Reject buttons for it. **Expected:** Once a hire is approved or rejected anywhere in the UI, it should be resolved everywhere — the inbox should not re-ask you to approve an agent that is already decided. **Root cause:** `POST /agents/:id/approve` called `activatePendingApproval`, which only changes the agent's status. The linked `hire_agent` approval row stayed `pending`. The inbox lists approvals filtered to unresolved statuses, so the card persisted. Terminating a pending agent from the detail page had the mirror problem for the "reject" half. Related (not duplicates): #215 (join-request inbox badge), #1815 (approval detail button loading text). ## What Changed - Added `approvalService.findOpenHireApprovalForAgent(companyId, agentId)` to locate the open `hire_agent` approval for an agent. The company/type/open-status **and** `payload->>'agentId'` predicates all run in SQL (jsonb operator), so the DB returns only the relevant row instead of filtering in JS (`server/src/services/approvals.ts`). - `POST /agents/:id/approve` resolves the linked approval through the shared `approvalsSvc.approve()` — running activation, budget-policy upsert, and the hire-approved notification as one path — and falls back to direct `activatePendingApproval` only when no open approval exists (legacy agents created before approvals were tracked) (`server/src/routes/agents.ts`). - `POST /agents/:id/terminate` now branches the same way: when a still-`pending_approval` agent has an open hire approval, it delegates to `approvalsSvc.reject()` (which resolves the approval **and** terminates the agent internally) and re-reads the agent, otherwise it terminates directly. This avoids terminating the agent twice (`reject()` already calls `agentsSvc.terminate()`) (`server/src/routes/agents.ts`). - The `agent.approved` activity log records the resolved `approvalId` (or `null` on the fallback path) for traceability. ## Verification - `pnpm --filter @paperclipai/server typecheck` — clean. - Targeted server tests pass (57 tests): `npx vitest run src/__tests__/approvals-service.test.ts src/__tests__/agent-permissions-routes.test.ts` - `findOpenHireApprovalForAgent` returns the row the SQL filter yields / returns null when none matches. - Approving from the detail page resolves the linked approval via `approvalsSvc.approve()` and does not double-activate; legacy fallback still calls `activatePendingApproval`. - Terminating a still-pending agent with an open approval calls `approvalsSvc.reject()` and does **not** call `agentsSvc.terminate()` a second time; terminating with no open approval terminates directly. - Manual: create an agent → inbox shows the hire card → approve from the agent detail page → inbox card is gone and the agent is active. Same for terminate-while-pending clearing the card. ## Risks Low risk, server-only, no schema or migration changes. The fallback to `activatePendingApproval` preserves existing behavior for agents with no tracked approval record, so legacy agents still activate. The shared approval path is the same one the inbox card already uses, so approve/terminate-from-detail-page now match approve/reject-from-inbox exactly. ## Model Used Claude Opus 4.8 (claude-opus-4-8), extended thinking with tool use, via Claude Code. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots (server-only change, no UI diff) - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups (in progress) - [x] I will address all Greptile and reviewer comments before requesting merge
288 lines
9.8 KiB
TypeScript
288 lines
9.8 KiB
TypeScript
import { and, asc, eq, inArray, sql } from "drizzle-orm";
|
|
import type { Db } from "@paperclipai/db";
|
|
import { approvalComments, approvals } from "@paperclipai/db";
|
|
import { notFound, unprocessable } from "../errors.js";
|
|
import { redactCurrentUserText } from "../log-redaction.js";
|
|
import { agentService } from "./agents.js";
|
|
import { budgetService } from "./budgets.js";
|
|
import { notifyHireApproved } from "./hire-hook.js";
|
|
import { instanceSettingsService } from "./instance-settings.js";
|
|
|
|
export function approvalService(db: Db) {
|
|
const agentsSvc = agentService(db);
|
|
const budgets = budgetService(db);
|
|
const instanceSettings = instanceSettingsService(db);
|
|
const canResolveStatuses = new Set(["pending", "revision_requested"]);
|
|
const resolvableStatuses = Array.from(canResolveStatuses);
|
|
type ApprovalRecord = typeof approvals.$inferSelect;
|
|
type ResolutionResult = { approval: ApprovalRecord; applied: boolean };
|
|
|
|
function redactApprovalComment<T extends { body: string }>(comment: T, censorUsernameInLogs: boolean): T {
|
|
return {
|
|
...comment,
|
|
body: redactCurrentUserText(comment.body, { enabled: censorUsernameInLogs }),
|
|
};
|
|
}
|
|
|
|
async function getExistingApproval(id: string) {
|
|
const existing = await db
|
|
.select()
|
|
.from(approvals)
|
|
.where(eq(approvals.id, id))
|
|
.then((rows) => rows[0] ?? null);
|
|
if (!existing) throw notFound("Approval not found");
|
|
return existing;
|
|
}
|
|
|
|
async function resolveApproval(
|
|
id: string,
|
|
targetStatus: "approved" | "rejected",
|
|
decidedByUserId: string,
|
|
decisionNote: string | null | undefined,
|
|
): Promise<ResolutionResult> {
|
|
const existing = await getExistingApproval(id);
|
|
if (!canResolveStatuses.has(existing.status)) {
|
|
if (existing.status === targetStatus) {
|
|
return { approval: existing, applied: false };
|
|
}
|
|
throw unprocessable(
|
|
`Only pending or revision requested approvals can be ${targetStatus === "approved" ? "approved" : "rejected"}`,
|
|
);
|
|
}
|
|
|
|
const now = new Date();
|
|
const updated = await db
|
|
.update(approvals)
|
|
.set({
|
|
status: targetStatus,
|
|
decidedByUserId,
|
|
decisionNote: decisionNote ?? null,
|
|
decidedAt: now,
|
|
updatedAt: now,
|
|
})
|
|
.where(and(eq(approvals.id, id), inArray(approvals.status, resolvableStatuses)))
|
|
.returning()
|
|
.then((rows) => rows[0] ?? null);
|
|
|
|
if (updated) {
|
|
return { approval: updated, applied: true };
|
|
}
|
|
|
|
const latest = await getExistingApproval(id);
|
|
if (latest.status === targetStatus) {
|
|
return { approval: latest, applied: false };
|
|
}
|
|
|
|
throw unprocessable(
|
|
`Only pending or revision requested approvals can be ${targetStatus === "approved" ? "approved" : "rejected"}`,
|
|
);
|
|
}
|
|
|
|
return {
|
|
list: (companyId: string, status?: string) => {
|
|
const conditions = [eq(approvals.companyId, companyId)];
|
|
if (status) conditions.push(eq(approvals.status, status));
|
|
return db.select().from(approvals).where(and(...conditions));
|
|
},
|
|
|
|
getById: (id: string) =>
|
|
db
|
|
.select()
|
|
.from(approvals)
|
|
.where(eq(approvals.id, id))
|
|
.then((rows) => rows[0] ?? null),
|
|
|
|
findOpenHireApprovalForAgent: async (companyId: string, agentId: string) => {
|
|
const rows = await db
|
|
.select()
|
|
.from(approvals)
|
|
.where(
|
|
and(
|
|
eq(approvals.companyId, companyId),
|
|
eq(approvals.type, "hire_agent"),
|
|
inArray(approvals.status, resolvableStatuses),
|
|
sql`${approvals.payload} ->> 'agentId' = ${agentId}`,
|
|
),
|
|
);
|
|
return rows[0] ?? null;
|
|
},
|
|
|
|
create: (companyId: string, data: Omit<typeof approvals.$inferInsert, "companyId">) =>
|
|
db
|
|
.insert(approvals)
|
|
.values({ ...data, companyId })
|
|
.returning()
|
|
.then((rows) => rows[0]),
|
|
|
|
approve: async (id: string, decidedByUserId: string, decisionNote?: string | null) => {
|
|
const { approval: updated, applied } = await resolveApproval(
|
|
id,
|
|
"approved",
|
|
decidedByUserId,
|
|
decisionNote,
|
|
);
|
|
|
|
let hireApprovedAgentId: string | null = null;
|
|
const now = new Date();
|
|
if (applied && updated.type === "hire_agent") {
|
|
const payload = updated.payload as Record<string, unknown>;
|
|
const payloadAgentId = typeof payload.agentId === "string" ? payload.agentId : null;
|
|
if (payloadAgentId) {
|
|
await agentsSvc.activatePendingApproval(payloadAgentId);
|
|
hireApprovedAgentId = payloadAgentId;
|
|
} else {
|
|
const created = await agentsSvc.create(updated.companyId, {
|
|
name: String(payload.name ?? "New Agent"),
|
|
role: String(payload.role ?? "general"),
|
|
title: typeof payload.title === "string" ? payload.title : null,
|
|
reportsTo: typeof payload.reportsTo === "string" ? payload.reportsTo : null,
|
|
capabilities: typeof payload.capabilities === "string" ? payload.capabilities : null,
|
|
adapterType: String(payload.adapterType ?? "process"),
|
|
adapterConfig:
|
|
typeof payload.adapterConfig === "object" && payload.adapterConfig !== null
|
|
? (payload.adapterConfig as Record<string, unknown>)
|
|
: {},
|
|
budgetMonthlyCents:
|
|
typeof payload.budgetMonthlyCents === "number" ? payload.budgetMonthlyCents : 0,
|
|
metadata:
|
|
typeof payload.metadata === "object" && payload.metadata !== null
|
|
? (payload.metadata as Record<string, unknown>)
|
|
: null,
|
|
status: "idle",
|
|
spentMonthlyCents: 0,
|
|
permissions: undefined,
|
|
lastHeartbeatAt: null,
|
|
});
|
|
hireApprovedAgentId = created?.id ?? null;
|
|
}
|
|
if (hireApprovedAgentId) {
|
|
const budgetMonthlyCents =
|
|
typeof payload.budgetMonthlyCents === "number" ? payload.budgetMonthlyCents : 0;
|
|
if (budgetMonthlyCents > 0) {
|
|
await budgets.upsertPolicy(
|
|
updated.companyId,
|
|
{
|
|
scopeType: "agent",
|
|
scopeId: hireApprovedAgentId,
|
|
amount: budgetMonthlyCents,
|
|
windowKind: "calendar_month_utc",
|
|
},
|
|
decidedByUserId,
|
|
);
|
|
}
|
|
void notifyHireApproved(db, {
|
|
companyId: updated.companyId,
|
|
agentId: hireApprovedAgentId,
|
|
source: "approval",
|
|
sourceId: id,
|
|
approvedAt: now,
|
|
}).catch(() => {});
|
|
}
|
|
}
|
|
|
|
return { approval: updated, applied };
|
|
},
|
|
|
|
reject: async (id: string, decidedByUserId: string, decisionNote?: string | null) => {
|
|
const { approval: updated, applied } = await resolveApproval(
|
|
id,
|
|
"rejected",
|
|
decidedByUserId,
|
|
decisionNote,
|
|
);
|
|
|
|
if (applied && updated.type === "hire_agent") {
|
|
const payload = updated.payload as Record<string, unknown>;
|
|
const payloadAgentId = typeof payload.agentId === "string" ? payload.agentId : null;
|
|
if (payloadAgentId) {
|
|
await agentsSvc.terminate(payloadAgentId);
|
|
}
|
|
}
|
|
|
|
return { approval: updated, applied };
|
|
},
|
|
|
|
requestRevision: async (id: string, decidedByUserId: string, decisionNote?: string | null) => {
|
|
const existing = await getExistingApproval(id);
|
|
if (existing.status !== "pending") {
|
|
throw unprocessable("Only pending approvals can request revision");
|
|
}
|
|
|
|
const now = new Date();
|
|
return db
|
|
.update(approvals)
|
|
.set({
|
|
status: "revision_requested",
|
|
decidedByUserId,
|
|
decisionNote: decisionNote ?? null,
|
|
decidedAt: now,
|
|
updatedAt: now,
|
|
})
|
|
.where(eq(approvals.id, id))
|
|
.returning()
|
|
.then((rows) => rows[0]);
|
|
},
|
|
|
|
resubmit: async (id: string, payload?: Record<string, unknown>) => {
|
|
const existing = await getExistingApproval(id);
|
|
if (existing.status !== "revision_requested") {
|
|
throw unprocessable("Only revision requested approvals can be resubmitted");
|
|
}
|
|
|
|
const now = new Date();
|
|
return db
|
|
.update(approvals)
|
|
.set({
|
|
status: "pending",
|
|
payload: payload ?? existing.payload,
|
|
decisionNote: null,
|
|
decidedByUserId: null,
|
|
decidedAt: null,
|
|
updatedAt: now,
|
|
})
|
|
.where(eq(approvals.id, id))
|
|
.returning()
|
|
.then((rows) => rows[0]);
|
|
},
|
|
|
|
listComments: async (approvalId: string) => {
|
|
const existing = await getExistingApproval(approvalId);
|
|
const { censorUsernameInLogs } = await instanceSettings.getGeneral();
|
|
return db
|
|
.select()
|
|
.from(approvalComments)
|
|
.where(
|
|
and(
|
|
eq(approvalComments.approvalId, approvalId),
|
|
eq(approvalComments.companyId, existing.companyId),
|
|
),
|
|
)
|
|
.orderBy(asc(approvalComments.createdAt))
|
|
.then((comments) => comments.map((comment) => redactApprovalComment(comment, censorUsernameInLogs)));
|
|
},
|
|
|
|
addComment: async (
|
|
approvalId: string,
|
|
body: string,
|
|
actor: { agentId?: string; userId?: string },
|
|
) => {
|
|
const existing = await getExistingApproval(approvalId);
|
|
const currentUserRedactionOptions = {
|
|
enabled: (await instanceSettings.getGeneral()).censorUsernameInLogs,
|
|
};
|
|
const redactedBody = redactCurrentUserText(body, currentUserRedactionOptions);
|
|
return db
|
|
.insert(approvalComments)
|
|
.values({
|
|
companyId: existing.companyId,
|
|
approvalId,
|
|
authorAgentId: actor.agentId ?? null,
|
|
authorUserId: actor.userId ?? null,
|
|
body: redactedBody,
|
|
})
|
|
.returning()
|
|
.then((rows) => redactApprovalComment(rows[0], currentUserRedactionOptions.enabled));
|
|
},
|
|
};
|
|
}
|