diff --git a/packages/db/src/__tests__/policies.test.ts b/packages/db/src/__tests__/policies.test.ts index 6e29f2e..6ba0762 100644 --- a/packages/db/src/__tests__/policies.test.ts +++ b/packages/db/src/__tests__/policies.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, beforeEach } from "vitest"; import { getDb } from "../connection.js"; -import { insertPolicy, listPolicies, getPolicy, updatePolicy, getPolicyStats, insertPolicyResult, getPolicyResults } from "../policies.js"; +import { insertPolicy, listPolicies, getPolicy, updatePolicy, getPolicyStats, insertPolicyResult, getPolicyResults, deletePolicy } from "../policies.js"; import { insertRun } from "../runs.js"; import { policyResults } from "../schema.js"; import type { AgentOpsDb } from "../connection.js"; @@ -423,4 +423,119 @@ describe("Policies repository", () => { expect(phases).toContain("run-complete"); }); }); + + describe("deletePolicy", () => { + function makeRun(id: string): Run { + return { + id: createRunId(id), + status: RunStatus.Completed, + goal: { + humanReadable: "Test", + structured: { type: "task", description: "Test", parameters: {} }, + }, + agents: [], + environment: { + repo: "test/repo", + branch: "main", + permissions: [], + sandbox: { enabled: false, isolationLevel: "none" }, + }, + actions: [], + artifacts: [], + metrics: { + tokenUsage: { input: 100, output: 50, total: 150 }, + wallTimeMs: 1000, + costUsd: 0.5, + flakeRate: 0, + }, + evaluations: [], + decisions: [], + createdAt: "2025-01-01T00:00:00.000Z", + updatedAt: "2025-01-01T00:00:00.000Z", + }; + } + + function seedPolicy(id: string): void { + insertPolicy(db, { + id: createPolicyId(id), + name: "Cost ceiling", + type: PolicyType.CostCeiling, + config: { type: PolicyType.CostCeiling, maxUsd: 10 }, + severity: PolicySeverity.Error, + enabled: true, + createdAt: "2025-01-01T00:00:00.000Z", + }); + } + + it("deletes a policy that has never been evaluated", () => { + seedPolicy("p_fresh"); + const { policyResults: deleted } = deletePolicy(db, createPolicyId("p_fresh")); + expect(deleted).toBe(0); + expect(getPolicy(db, createPolicyId("p_fresh"))).toBeNull(); + }); + + it("deletes an evaluated policy despite the FK on policy_results (regression)", () => { + // policy_results.policy_id is NOT NULL REFERENCES policies(id) and + // connections run with foreign_keys = ON. finalizeRun writes one + // result row per active policy on every completed run, so this is + // the normal state of any policy that has existed during a run — + // the old deletePolicy threw SQLITE_CONSTRAINT_FOREIGNKEY here. + seedPolicy("p_used"); + insertRun(db, makeRun("run_del_1")); + insertPolicyResult(db, { + id: "pr_del_1", + runId: "run_del_1", + policyId: "p_used", + passed: true, + message: "ok", + details: {}, + evaluatedAt: "2025-06-01T14:00:00.000Z", + }); + insertPolicyResult(db, { + id: "pr_del_2", + runId: "run_del_1", + policyId: "p_used", + passed: false, + message: "over budget", + details: {}, + evaluatedAt: "2025-06-02T14:00:00.000Z", + }); + + const { policyResults: deleted } = deletePolicy(db, createPolicyId("p_used")); + expect(deleted).toBe(2); + expect(getPolicy(db, createPolicyId("p_used"))).toBeNull(); + expect(getPolicyResults(db, createRunId("run_del_1"))).toHaveLength(0); + }); + + it("does not touch other policies' results", () => { + seedPolicy("p_keep"); + seedPolicy("p_drop"); + insertRun(db, makeRun("run_del_2")); + insertPolicyResult(db, { + id: "pr_keep_1", + runId: "run_del_2", + policyId: "p_keep", + passed: true, + message: "ok", + details: {}, + evaluatedAt: "2025-06-01T14:00:00.000Z", + }); + insertPolicyResult(db, { + id: "pr_drop_1", + runId: "run_del_2", + policyId: "p_drop", + passed: true, + message: "ok", + details: {}, + evaluatedAt: "2025-06-01T14:00:00.000Z", + }); + + const { policyResults: deleted } = deletePolicy(db, createPolicyId("p_drop")); + expect(deleted).toBe(1); + expect(getPolicy(db, createPolicyId("p_keep"))).not.toBeNull(); + const remaining = getPolicyResults(db, createRunId("run_del_2")); + expect(remaining).toHaveLength(1); + expect(remaining[0]!.policyId).toBe("p_keep"); + }); + }); }); diff --git a/packages/db/src/policies.ts b/packages/db/src/policies.ts index f59195b..eeaf549 100644 --- a/packages/db/src/policies.ts +++ b/packages/db/src/policies.ts @@ -217,8 +217,21 @@ export function getPolicyStats( export function deletePolicy( db: AgentOpsDb, id: PolicyId, -): void { - db.delete(policies).where(eq(policies.id, id as string)).run(); +): { policyResults: number } { + // policy_results.policy_id is NOT NULL REFERENCES policies(id) with no ON + // DELETE CASCADE, and finalizeRun writes one result row per active policy + // on every completed run — so after a single run, deleting the parent row + // alone throws an FK constraint error and the dashboard delete 500s. + // Delete children-first inside a transaction (same pattern as + // deleteOldRuns) and report how much evaluation history went with it. + return db.transaction((tx) => { + const prResult = tx + .delete(policyResults) + .where(eq(policyResults.policyId, id as string)) + .run() as { changes?: number }; + tx.delete(policies).where(eq(policies.id, id as string)).run(); + return { policyResults: prResult.changes ?? 0 }; + }); } export function getPolicyResultsForPolicy( diff --git a/packages/web/src/app/api/__tests__/auth-gaps.test.ts b/packages/web/src/app/api/__tests__/auth-gaps.test.ts index ea9ca06..0621305 100644 --- a/packages/web/src/app/api/__tests__/auth-gaps.test.ts +++ b/packages/web/src/app/api/__tests__/auth-gaps.test.ts @@ -626,4 +626,25 @@ describe("GET /api/policies/[id]/results scoping", () => { [ownerRun.id as string, otherRun.id as string].sort(), ); }); + + it("DELETE /api/policies/[id] succeeds for an evaluated policy (regression: FK 500)", async () => { + // finalizeRun writes a policy_results row per active policy on every + // completed run; the FK from policy_results.policy_id used to make the + // dashboard delete 500 for any policy that had ever been evaluated. + seedPolicy(); + const run = makeRun({ userId: owner.user.id }); + seedResult("pr_fk_regression", run.id as string); + + const res = await deletePolicyRoute( + authedRequest("http://localhost/api/policies/pol_test", { + method: "DELETE", + token: admin.token, + }), + withParams({ id: "pol_test" }), + ); + expect(res.status).toBe(200); + const body = (await jsonOf(res)) as { ok: boolean; deletedResults: number }; + expect(body.ok).toBe(true); + expect(body.deletedResults).toBe(1); + }); }); diff --git a/packages/web/src/app/api/policies/[id]/route.ts b/packages/web/src/app/api/policies/[id]/route.ts index cef22a7..755726d 100644 --- a/packages/web/src/app/api/policies/[id]/route.ts +++ b/packages/web/src/app/api/policies/[id]/route.ts @@ -182,13 +182,13 @@ export async function DELETE( return NextResponse.json({ error: "Policy not found" }, { status: 404 }); } - deletePolicy(database, policyId); + const { policyResults } = deletePolicy(database, policyId); recordAudit(request, user.id, AUDIT_ACTIONS.POLICY_DELETED, { targetType: "policy", targetId: policyId as string, - metadata: { name: existing.name, type: existing.type }, + metadata: { name: existing.name, type: existing.type, policyResults }, }); - return NextResponse.json({ ok: true }); + return NextResponse.json({ ok: true, deletedResults: policyResults }); } catch (error) { console.error("API error:", error); return NextResponse.json(