mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 01:50:13 +01:00
fix(agents): refuse agent purge when cost history references it (#19022)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A board user can permanently delete (purge) a terminated agent > - The purge deletes the agent's heartbeat runs, but cost and finance events still point to those runs > - Postgres then rejects the delete with a foreign key error, and the API returns a 500 > - Cost and finance events are accounting history. The purge must not remove them > - This pull request makes the purge check for this history first and return a clear 409 > - The benefit is a clear, expected response instead of a 500, and accounting history stays intact ## Linked Issues or Issue Description **What happened?** Permanent agent deletion failed with `Failed query: delete from "heartbeat_runs" where "heartbeat_runs"."agent_id" = $1`. The cause is `update or delete on table "heartbeat_runs" violates foreign key constraint "cost_events_heartbeat_run_id_heartbeat_runs_id_fk" on table "cost_events"`. The server reported this as an unexpected error. Sentry: PAPERCLIP-BACKEND-4Z. **Steps to reproduce** 1. Run an agent so that a heartbeat run records a cost event. 2. Terminate the agent. 3. Permanently delete (purge) the agent. **Expected behavior** The purge stops before it deletes anything. It returns a clear conflict, and the agent stays terminated. This matches the existing `agent_case_history_retained` behavior. **Actual behavior** The transaction fails on the `heartbeat_runs` delete with a raw foreign key error (500). Related, older approaches: #9130 and #1987 tried to delete cost and finance rows during agent removal. This PR keeps the ledger instead. ## What Changed - `deleteAgentDependencies` now checks for `cost_events` and `finance_events` rows that reference the agent or one of its heartbeat runs. It does this before any destructive cleanup. - If such rows exist, it throws `409 agent_cost_history_retained`. Termination stays available. - The activity-log cleanup reuses the same agent-runs subquery. - New embedded-Postgres regression test `server/src/services/agent-cost-history-deletion.test.ts`. It covers a cost event on a run, a cost event attributed to the agent, a finance event on a run, and a normal purge with no cost history. ## Verification - Without the fix, 3 of the 4 new tests fail with the exact Sentry error. Examples: `cost_events_heartbeat_run_id_heartbeat_runs_id_fk`, `cost_events_agent_id_agents_id_fk`, `finance_events_heartbeat_run_id_heartbeat_runs_id_fk`. - With the fix, all 4 pass. - Related suites pass (8 files, 249 tests): `lifecycle.test.ts`, `execution-admission/adapters/postgres.test.ts`, `decision-models.test.ts`, `prompt-injection-prevention.test.ts`, `fast-responses.test.ts`, `agent-timer-deadlines.test.ts`, `case-deletion.test.ts`, and the new test. - `tsc --noEmit` in `server/` reports 0 errors. ## Risks - Behavior change: agents with recorded cost or finance history can no longer be purged. Before this change, these purges already failed with a 500, so no purge that used to succeed is now blocked. - No schema change and no migration. - Docs follow-up: `doc/SPEC-implementation.md`, `doc/DEVELOPING.md` and `maps/agent-access-map/agents.md` describe `agent_case_history_retained`. They do not yet mention the new `agent_cost_history_retained` code. I can add that in this PR if reviewers want it. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), agentic coding with tool use and code execution. ## 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] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [ ] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
08f3e7ebec
commit
db95539d46
2 files changed
+73
-2
No files matched your search
@@ -0,0 +1,61 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { eq } from "drizzle-orm";
|
||||
import { agents, companies, costEvents, createDb, financeEvents, heartbeatRuns } from "@paperclipai/db";
|
||||
import { afterAll, beforeAll, describe, expect, it } from "vitest";
|
||||
import { startEmbeddedPostgresTestDatabase } from "../__tests__/helpers/embedded-postgres.js";
|
||||
import { createAgentLifecycle } from "./agent-lifecycle.js";
|
||||
|
||||
describe("agent purge with retained cost history", () => {
|
||||
let temporary: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>>;
|
||||
let db: ReturnType<typeof createDb>;
|
||||
beforeAll(async () => {
|
||||
temporary = await startEmbeddedPostgresTestDatabase("agent-cost-deletion-");
|
||||
db = createDb(temporary.connectionString);
|
||||
});
|
||||
afterAll(async () => { await temporary?.cleanup(); });
|
||||
|
||||
async function fixture() {
|
||||
const companyId = randomUUID(), agentId = randomUUID();
|
||||
await db.insert(companies).values({ id: companyId, name: "Cost deletion", issuePrefix: `C${companyId.slice(0, 6)}` });
|
||||
await db.insert(agents).values({ id: agentId, companyId, name: "Billed worker", status: "terminated", lifecycleState: "terminated" });
|
||||
const [run] = await db.insert(heartbeatRuns).values({ companyId, agentId, status: "succeeded" }).returning();
|
||||
return { companyId, agentId, run: run! };
|
||||
}
|
||||
const cost = (companyId: string, values: Partial<typeof costEvents.$inferInsert>) => db.insert(costEvents).values({
|
||||
companyId, usageKind: "decision", provider: "anthropic", model: "claude", costCents: 1, occurredAt: new Date(), ...values,
|
||||
}).returning();
|
||||
|
||||
async function expectRetained(f: Awaited<ReturnType<typeof fixture>>) {
|
||||
await expect(createAgentLifecycle(db).purgeAgent(f.agentId)).rejects.toMatchObject({ status: 409,
|
||||
details: { code: "agent_cost_history_retained" } });
|
||||
expect(await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, f.run.id))).toHaveLength(1);
|
||||
expect(await db.select().from(agents).where(eq(agents.id, f.agentId))).toMatchObject([{ lifecycleState: "terminated" }]);
|
||||
}
|
||||
|
||||
it("refuses before cleanup when a cost event references one of the agent's runs", async () => {
|
||||
const f = await fixture();
|
||||
const [event] = await cost(f.companyId, { heartbeatRunId: f.run.id });
|
||||
await expectRetained(f);
|
||||
expect(await db.select().from(costEvents).where(eq(costEvents.id, event!.id))).toEqual([event]);
|
||||
});
|
||||
|
||||
it("refuses when a cost event is attributed to the agent", async () => {
|
||||
const f = await fixture();
|
||||
await cost(f.companyId, { usageKind: "agent", agentId: f.agentId });
|
||||
await expectRetained(f);
|
||||
});
|
||||
|
||||
it("refuses when a finance event references one of the agent's runs", async () => {
|
||||
const f = await fixture();
|
||||
const [event] = await db.insert(financeEvents).values({ companyId: f.companyId, heartbeatRunId: f.run.id,
|
||||
eventKind: "usage", biller: "anthropic", amountCents: 1, occurredAt: new Date() }).returning();
|
||||
await expectRetained(f);
|
||||
expect(await db.select().from(financeEvents).where(eq(financeEvents.id, event!.id))).toEqual([event]);
|
||||
});
|
||||
|
||||
it("still purges an agent whose runs have no cost history", async () => {
|
||||
const f = await fixture();
|
||||
expect(await createAgentLifecycle(db).purgeAgent(f.agentId)).toMatchObject({ id: f.agentId });
|
||||
expect(await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, f.run.id))).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
@@ -10,7 +10,7 @@ import { notifyInstructionCommit } from "./instruction-commit-notifications.js";
|
||||
import { notifyScheduledRetryCancellation } from "./scheduled-retry-cancellation-notifications.js";
|
||||
import { and, eq, or, sql } from "drizzle-orm";
|
||||
import { agents, agentApiKeys, agentRuntimeState, agentTaskSessions, agentWakeupRequests, activityLog,
|
||||
budgetReservations, heartbeatRunEvents, heartbeatRuns, issueExecutionDecisions, issues, issueComments,
|
||||
budgetReservations, costEvents, financeEvents, heartbeatRunEvents, heartbeatRuns, issueExecutionDecisions, issues, issueComments,
|
||||
caseHandoffs, principalPermissionGrants, type Db } from "@paperclipai/db";
|
||||
import { conflict } from "../errors.js";
|
||||
import { issueThreadInteractionService } from "./issue-thread-interactions.js";
|
||||
@@ -47,6 +47,16 @@ export async function deleteAgentDependencies(tx: Db, companyId: string, id: str
|
||||
if (fastResponseHold) throw conflict("Wait for active fast responses or resolve their unknown charges in Costs before deleting this agent", {
|
||||
code: "agent_fast_response_accounting_pending",
|
||||
});
|
||||
const agentRuns = sql`select ${heartbeatRuns.id} from ${heartbeatRuns} where ${heartbeatRuns.agentId} = ${id}`;
|
||||
const [retainedCost] = await tx.select({ id: costEvents.id }).from(costEvents).where(and(
|
||||
eq(costEvents.companyId, companyId), or(eq(costEvents.agentId, id), sql`${costEvents.heartbeatRunId} in (${agentRuns})`),
|
||||
)).limit(1);
|
||||
const [retainedFinance] = await tx.select({ id: financeEvents.id }).from(financeEvents).where(and(
|
||||
eq(financeEvents.companyId, companyId), or(eq(financeEvents.agentId, id), sql`${financeEvents.heartbeatRunId} in (${agentRuns})`),
|
||||
)).limit(1);
|
||||
if (retainedCost || retainedFinance) throw conflict("Cost history requires this agent record. Keep the agent terminated instead of permanently deleting it", {
|
||||
code: "agent_cost_history_retained",
|
||||
});
|
||||
await notifyAttentionRetentionWork(tx, { companyId });
|
||||
await notifyExternalConversationSettlement(tx, { companyId });
|
||||
await issueThreadInteractionService(tx).cancelPendingForDeletedAddressee(companyId, id);
|
||||
@@ -63,7 +73,7 @@ export async function deleteAgentDependencies(tx: Db, companyId: string, id: str
|
||||
await tx.delete(activityLog).where(
|
||||
or(
|
||||
eq(activityLog.agentId, id),
|
||||
sql`${activityLog.runId} in (select ${heartbeatRuns.id} from ${heartbeatRuns} where ${heartbeatRuns.agentId} = ${id})`,
|
||||
sql`${activityLog.runId} in (${agentRuns})`,
|
||||
),
|
||||
);
|
||||
await tx.delete(issueExecutionDecisions).where(eq(issueExecutionDecisions.actorAgentId, id));
|
||||
|
||||
Reference in new issue
Block a user