fix(issues): make DELETE /api/issues/:id succeed for issues with dependents (#11331)

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - The server provides issue APIs and the database stores issue child
rows
> - The issue delete endpoint removes the parent issue before dependent
rows
> - Several issue foreign keys had no delete policy, so PostgreSQL
returned a foreign-key error
> - This pull request adds safe cascade and set-null policies and a
clear conflict response
> - The benefit is reliable issue deletion with a useful error when a
restricted audit row still blocks deletion

## Linked Issues or Issue Description

Fixes #7728
Fixes #4660
Fixes #7991
Fixes #4627
Fixes #5086

**What happened?**

`DELETE /api/issues/:id` returned HTTP 500 when dependent comments,
thread interactions, read states, inbox archives, feedback votes, or
ledger rows referenced the issue. The database raised SQLSTATE 23503
because several foreign keys had no delete policy.

**Expected behavior**

The endpoint must remove dependent rows that have no meaning without the
issue. It must keep ledger rows with a null issue reference. It must
return HTTP 409 when a restricted decision audit row still references
the issue.

**Steps to reproduce**

1. Create an issue.
2. Add a comment or thread interaction that references the issue.
3. Send `DELETE /api/issues/:id`.
4. Observe the HTTP 500 response.

**Paperclip version or commit**

Commit `1f8f456f8340823fe2bd891ae8933d942f190b7b`.

**Deployment mode**

Local dev with embedded PGlite or external PostgreSQL.

## What Changed

- Add `CASCADE` to five issue child foreign keys.
- Add `SET NULL` to the finance and cost event issue foreign keys.
- Keep decision audit references restricted.
- Map SQLSTATE 23503 from the issue delete service to HTTP 409.
- Add migration 0217 for the seven changed tables.
- Add regression tests for cascade deletion and restricted decision
references.

## Verification

- Run `pnpm --filter @paperclipai/db typecheck`.
- Run `pnpm --filter @paperclipai/server typecheck`.
- Run `npx vitest run src/__tests__/issue-remove-cascade.test.ts` from
`server/`.
- The regression test applies migration 0217 to a fresh embedded
PostgreSQL database.

## Risks

- Migration 0217 changes only seven foreign keys that reference
`issues.id`.
- Cascade deletion removes child rows that cannot exist without the
parent issue.
- Set-null preserves finance and cost ledger rows.
- Decision audit rows remain protected, so the endpoint can return HTTP
409.

## Model Used

Codex, based on GPT-5, with tool use and code-review support. The
implementation author used an AI coding agent. This PR handoff uses the
same model family to validate the commit and manage the pull request.

## 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
- [x] 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

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Nicky Leach 2026-08-13 12:02:15 -07:00 committed by GitHub
parent 062271e934
commit 44694328a3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 39588 additions and 13 deletions

View File

@ -0,0 +1,21 @@
ALTER TABLE "cost_events" DROP CONSTRAINT "cost_events_issue_id_issues_id_fk";
--> statement-breakpoint
ALTER TABLE "feedback_votes" DROP CONSTRAINT "feedback_votes_issue_id_issues_id_fk";
--> statement-breakpoint
ALTER TABLE "finance_events" DROP CONSTRAINT "finance_events_issue_id_issues_id_fk";
--> statement-breakpoint
ALTER TABLE "issue_comments" DROP CONSTRAINT "issue_comments_issue_id_issues_id_fk";
--> statement-breakpoint
ALTER TABLE "issue_inbox_archives" DROP CONSTRAINT "issue_inbox_archives_issue_id_issues_id_fk";
--> statement-breakpoint
ALTER TABLE "issue_read_states" DROP CONSTRAINT "issue_read_states_issue_id_issues_id_fk";
--> statement-breakpoint
ALTER TABLE "issue_thread_interactions" DROP CONSTRAINT "issue_thread_interactions_issue_id_issues_id_fk";
--> statement-breakpoint
ALTER TABLE "cost_events" ADD CONSTRAINT "cost_events_issue_id_issues_id_fk" FOREIGN KEY ("issue_id") REFERENCES "public"."issues"("id") ON DELETE set null ON UPDATE no action;--> statement-breakpoint
ALTER TABLE "feedback_votes" ADD CONSTRAINT "feedback_votes_issue_id_issues_id_fk" FOREIGN KEY ("issue_id") REFERENCES "public"."issues"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint
ALTER TABLE "finance_events" ADD CONSTRAINT "finance_events_issue_id_issues_id_fk" FOREIGN KEY ("issue_id") REFERENCES "public"."issues"("id") ON DELETE set null ON UPDATE no action;--> statement-breakpoint
ALTER TABLE "issue_comments" ADD CONSTRAINT "issue_comments_issue_id_issues_id_fk" FOREIGN KEY ("issue_id") REFERENCES "public"."issues"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint
ALTER TABLE "issue_inbox_archives" ADD CONSTRAINT "issue_inbox_archives_issue_id_issues_id_fk" FOREIGN KEY ("issue_id") REFERENCES "public"."issues"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint
ALTER TABLE "issue_read_states" ADD CONSTRAINT "issue_read_states_issue_id_issues_id_fk" FOREIGN KEY ("issue_id") REFERENCES "public"."issues"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint
ALTER TABLE "issue_thread_interactions" ADD CONSTRAINT "issue_thread_interactions_issue_id_issues_id_fk" FOREIGN KEY ("issue_id") REFERENCES "public"."issues"("id") ON DELETE cascade ON UPDATE no action;

File diff suppressed because it is too large Load Diff

View File

@ -1506,6 +1506,13 @@
"when": 1786467952628,
"tag": "0216_company_onboarding_seeds",
"breakpoints": true
},
{
"idx": 217,
"version": "7",
"when": 1786643974972,
"tag": "0217_yielding_starbolt",
"breakpoints": true
}
]
}
}

View File

@ -12,7 +12,7 @@ export const costEvents = pgTable(
id: uuid("id").primaryKey().defaultRandom(),
companyId: uuid("company_id").notNull().references(() => companies.id),
agentId: uuid("agent_id").notNull().references(() => agents.id),
issueId: uuid("issue_id").references(() => issues.id),
issueId: uuid("issue_id").references(() => issues.id, { onDelete: "set null" }),
projectId: uuid("project_id").references(() => projects.id),
goalId: uuid("goal_id").references(() => goals.id),
heartbeatRunId: uuid("heartbeat_run_id").references(() => heartbeatRuns.id),

View File

@ -7,7 +7,7 @@ export const feedbackVotes = pgTable(
{
id: uuid("id").primaryKey().defaultRandom(),
companyId: uuid("company_id").notNull().references(() => companies.id),
issueId: uuid("issue_id").notNull().references(() => issues.id),
issueId: uuid("issue_id").notNull().references(() => issues.id, { onDelete: "cascade" }),
targetType: text("target_type").notNull(),
targetId: text("target_id").notNull(),
authorUserId: text("author_user_id").notNull(),

View File

@ -13,7 +13,7 @@ export const financeEvents = pgTable(
id: uuid("id").primaryKey().defaultRandom(),
companyId: uuid("company_id").notNull().references(() => companies.id),
agentId: uuid("agent_id").references(() => agents.id),
issueId: uuid("issue_id").references(() => issues.id),
issueId: uuid("issue_id").references(() => issues.id, { onDelete: "set null" }),
projectId: uuid("project_id").references(() => projects.id),
goalId: uuid("goal_id").references(() => goals.id),
heartbeatRunId: uuid("heartbeat_run_id").references(() => heartbeatRuns.id),

View File

@ -17,7 +17,7 @@ export const issueComments = pgTable(
{
id: uuid("id").primaryKey().defaultRandom(),
companyId: uuid("company_id").notNull().references(() => companies.id),
issueId: uuid("issue_id").notNull().references(() => issues.id),
issueId: uuid("issue_id").notNull().references(() => issues.id, { onDelete: "cascade" }),
authorAgentId: uuid("author_agent_id").references(() => agents.id),
authorUserId: text("author_user_id"),
onBehalfOfUserId: text("on_behalf_of_user_id").references(() => authUsers.id, { onDelete: "set null" }),

View File

@ -10,7 +10,7 @@ export const issueInboxArchives = pgTable(
{
id: uuid("id").primaryKey().defaultRandom(),
companyId: uuid("company_id").notNull().references(() => companies.id),
issueId: uuid("issue_id").notNull().references(() => issues.id),
issueId: uuid("issue_id").notNull().references(() => issues.id, { onDelete: "cascade" }),
userId: text("user_id").notNull(),
archivedByActorType: text("archived_by_actor_type").$type<"user" | "agent">().notNull().default("user"),
// Agent-attributed writes must set both IDs; SET NULL preserves rows if referenced records are deleted.

View File

@ -7,7 +7,7 @@ export const issueReadStates = pgTable(
{
id: uuid("id").primaryKey().defaultRandom(),
companyId: uuid("company_id").notNull().references(() => companies.id),
issueId: uuid("issue_id").notNull().references(() => issues.id),
issueId: uuid("issue_id").notNull().references(() => issues.id, { onDelete: "cascade" }),
userId: text("user_id").notNull(),
lastReadAt: timestamp("last_read_at", { withTimezone: true }).notNull().defaultNow(),
createdAt: timestamp("created_at", { withTimezone: true }).notNull().defaultNow(),

View File

@ -16,7 +16,7 @@ export const issueThreadInteractions = pgTable(
{
id: uuid("id").primaryKey().defaultRandom(),
companyId: uuid("company_id").notNull().references(() => companies.id),
issueId: uuid("issue_id").notNull().references(() => issues.id),
issueId: uuid("issue_id").notNull().references(() => issues.id, { onDelete: "cascade" }),
kind: text("kind").notNull(),
status: text("status").notNull().default("pending"),
continuationPolicy: text("continuation_policy").notNull().default("wake_assignee"),

View File

@ -0,0 +1,248 @@
import { randomUUID } from "node:crypto";
import { eq } from "drizzle-orm";
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
import {
agents,
companies,
costEvents,
createDb,
decisions,
feedbackVotes,
financeEvents,
goals,
heartbeatRuns,
issueComments,
issueInboxArchives,
issueReadStates,
issueThreadInteractions,
issues,
} from "@paperclipai/db";
import {
getEmbeddedPostgresTestSupport,
startEmbeddedPostgresTestDatabase,
} from "./helpers/embedded-postgres.js";
import { HttpError } from "../errors.js";
import { issueService } from "../services/issues.js";
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
// These tests protect DELETE /api/issues/:id. A foreign key to issues.id
// without a delete policy raised SQLSTATE 23503 and the route returned a bare
// 500. The dependent rows that have meaning only with their parent issue now
// cascade. The decisions table stays restricted on purpose, so the delete must
// return a typed 409 instead of a 500.
describeEmbeddedPostgres("issueService.remove referential integrity", () => {
let db!: ReturnType<typeof createDb>;
let svc!: ReturnType<typeof issueService>;
let tempDb: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | null = null;
beforeAll(async () => {
tempDb = await startEmbeddedPostgresTestDatabase("paperclip-issue-remove-cascade-");
db = createDb(tempDb.connectionString);
svc = issueService(db);
}, 20_000);
afterEach(async () => {
await db.delete(decisions);
await db.delete(issueThreadInteractions);
await db.delete(issueComments);
await db.delete(issueReadStates);
await db.delete(issueInboxArchives);
await db.delete(feedbackVotes);
// finance_events references cost_events, so delete finance rows first.
await db.delete(financeEvents);
await db.delete(costEvents);
await db.delete(heartbeatRuns);
await db.delete(issues);
await db.delete(goals);
await db.delete(agents);
await db.delete(companies);
});
afterAll(async () => {
await tempDb?.cleanup();
});
async function seedCompanyAgentRun() {
const companyId = randomUUID();
const goalId = randomUUID();
const agentId = randomUUID();
const runId = randomUUID();
await db.insert(companies).values({
id: companyId,
name: "Paperclip",
issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`,
requireBoardApprovalForNewAgents: false,
});
await db.insert(goals).values({
id: goalId,
companyId,
title: "Delete safety",
level: "task",
status: "active",
});
await db.insert(agents).values({
id: agentId,
companyId,
name: "Origin",
role: "engineer",
status: "active",
adapterType: "codex_local",
adapterConfig: {},
runtimeConfig: {},
permissions: {},
});
await db.insert(heartbeatRuns).values({
id: runId,
companyId,
agentId,
invocationSource: "manual",
status: "succeeded",
});
return { companyId, goalId, agentId, runId };
}
async function seedIssue(companyId: string, goalId: string) {
const issueId = randomUUID();
await db.insert(issues).values({
id: issueId,
companyId,
goalId,
title: "Deletable issue",
status: "in_progress",
priority: "medium",
});
return issueId;
}
it("deletes an issue and cascades its dependent rows", async () => {
const { companyId, goalId, runId } = await seedCompanyAgentRun();
const issueId = await seedIssue(companyId, goalId);
await db.insert(feedbackVotes).values({
id: randomUUID(),
companyId,
issueId,
targetType: "comment",
targetId: "target-1",
authorUserId: "board-user",
vote: "up",
});
await db.insert(issueComments).values({
id: randomUUID(),
companyId,
issueId,
authorUserId: "board-user",
body: "First comment.",
});
await db.insert(issueThreadInteractions).values({
id: randomUUID(),
companyId,
issueId,
kind: "request_confirmation",
sourceRunId: runId,
payload: {
version: 1,
prompt: "Confirm the change.",
detailsMarkdown: "Details.",
target: { type: "custom", key: "target", revisionId: "v1" },
},
});
await db.insert(issueReadStates).values({
id: randomUUID(),
companyId,
issueId,
userId: "board-user",
});
await db.insert(issueInboxArchives).values({
id: randomUUID(),
companyId,
issueId,
userId: "board-user",
});
const removed = await svc.remove(issueId);
expect(removed).not.toBeNull();
expect(removed?.id).toBe(issueId);
expect(await db.select().from(issues).where(eq(issues.id, issueId))).toHaveLength(0);
expect(await db.select().from(issueComments).where(eq(issueComments.issueId, issueId))).toHaveLength(0);
expect(
await db.select().from(issueThreadInteractions).where(eq(issueThreadInteractions.issueId, issueId)),
).toHaveLength(0);
expect(await db.select().from(issueReadStates).where(eq(issueReadStates.issueId, issueId))).toHaveLength(0);
expect(await db.select().from(issueInboxArchives).where(eq(issueInboxArchives.issueId, issueId))).toHaveLength(0);
expect(await db.select().from(feedbackVotes).where(eq(feedbackVotes.issueId, issueId))).toHaveLength(0);
});
it("deletes an issue and keeps ledger rows with issueId set to null", async () => {
const { companyId, goalId, agentId } = await seedCompanyAgentRun();
const issueId = await seedIssue(companyId, goalId);
const costEventId = randomUUID();
await db.insert(costEvents).values({
id: costEventId,
companyId,
agentId,
issueId,
provider: "anthropic",
model: "claude-opus-4-8",
costCents: 100,
occurredAt: new Date(),
});
const financeEventId = randomUUID();
await db.insert(financeEvents).values({
id: financeEventId,
companyId,
agentId,
issueId,
costEventId,
eventKind: "usage",
biller: "anthropic",
amountCents: 100,
occurredAt: new Date(),
});
const removed = await svc.remove(issueId);
expect(removed?.id).toBe(issueId);
expect(await db.select().from(issues).where(eq(issues.id, issueId))).toHaveLength(0);
// The ledger rows must survive the delete with a detached issueId.
const costRows = await db.select().from(costEvents).where(eq(costEvents.id, costEventId));
expect(costRows).toHaveLength(1);
expect(costRows[0]?.issueId).toBeNull();
const financeRows = await db.select().from(financeEvents).where(eq(financeEvents.id, financeEventId));
expect(financeRows).toHaveLength(1);
expect(financeRows[0]?.issueId).toBeNull();
});
it("returns a 409 conflict when a restricted decisions row references the issue", async () => {
const { companyId, goalId, agentId, runId } = await seedCompanyAgentRun();
const issueId = await seedIssue(companyId, goalId);
await db.insert(decisions).values({
id: randomUUID(),
companyId,
originAgentId: agentId,
originIssueId: issueId,
originRunId: runId,
title: "Ledger decision",
body: "A decision that must survive issue deletion.",
options: [],
expiresAt: new Date(Date.now() + 60_000),
signedSpec: "signed",
targetSnapshots: {},
});
await expect(svc.remove(issueId)).rejects.toBeInstanceOf(HttpError);
await expect(svc.remove(issueId)).rejects.toMatchObject({ status: 409 });
// The delete failed, so the issue row must still exist.
expect(await db.select().from(issues).where(eq(issues.id, issueId))).toHaveLength(1);
});
});

View File

@ -1,4 +1,5 @@
const UNIQUE_VIOLATION = "23505";
const FOREIGN_KEY_VIOLATION = "23503";
const MAX_CAUSE_DEPTH = 4;
/**
@ -31,3 +32,21 @@ export function isUniqueViolation(error: unknown, constraintName?: string): bool
}
return false;
}
/**
* Recognizes a Postgres foreign-key-constraint violation (SQLSTATE 23503).
*
* A delete that leaves an orphan reference raises this code. Drizzle wraps the
* driver failure in its own `Failed query: ...` error, so the Postgres error
* that carries the code is reachable only through `cause`. This helper walks
* the `cause` chain, the same way `isUniqueViolation` does.
*/
export function isForeignKeyViolation(error: unknown): boolean {
let current: unknown = error;
for (let depth = 0; depth < MAX_CAUSE_DEPTH && current && typeof current === "object"; depth += 1) {
const candidate = current as { code?: unknown; cause?: unknown };
if (candidate.code === FOREIGN_KEY_VIOLATION) return true;
current = candidate.cause;
}
return false;
}

View File

@ -66,6 +66,7 @@ import {
normalizeIssueIdentifier as normalizeIssueReferenceIdentifier,
} from "@paperclipai/shared";
import { conflict, HttpError, notFound, unprocessable } from "../errors.js";
import { isForeignKeyViolation } from "../db-errors.js";
import { logger } from "../middleware/logger.js";
import { parseObject } from "../adapters/utils.js";
import {
@ -7941,11 +7942,23 @@ export function issueService(db: Db) {
.from(issueDocuments)
.where(eq(issueDocuments.issueId, id));
const removedIssue = await tx
.delete(issues)
.where(eq(issues.id, id))
.returning()
.then((rows) => rows[0] ?? null);
let removedIssue;
try {
removedIssue = await tx
.delete(issues)
.where(eq(issues.id, id))
.returning()
.then((rows) => rows[0] ?? null);
} catch (err) {
// A foreign key to issues.id without a delete policy blocks the delete
// and raises SQLSTATE 23503. Map it to a clear 409 instead of a bare
// 500. This also covers the decisions table, whose NOT NULL references
// to issues.id stay restricted on purpose.
if (isForeignKeyViolation(err)) {
throw conflict("Issue cannot be deleted because another record still references it.");
}
throw err;
}
if (removedIssue && attachmentAssetIds.length > 0) {
await tx