diff --git a/packages/db/src/migrations/0171_issue_create_idempotency_keys.sql b/packages/db/src/migrations/0171_issue_create_idempotency_keys.sql new file mode 100644 index 0000000000..3eba8b3ce1 --- /dev/null +++ b/packages/db/src/migrations/0171_issue_create_idempotency_keys.sql @@ -0,0 +1,19 @@ +CREATE TABLE IF NOT EXISTS "issue_create_idempotency_keys" ( + "id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL, + "company_id" uuid NOT NULL REFERENCES "companies"("id") ON DELETE CASCADE, + "idempotency_key" text NOT NULL, + "issue_id" uuid NOT NULL REFERENCES "issues"("id") ON DELETE CASCADE, + "created_at" timestamp with time zone DEFAULT now() NOT NULL +);--> statement-breakpoint +CREATE UNIQUE INDEX IF NOT EXISTS "issue_create_idempotency_keys_company_key_uq" + ON "issue_create_idempotency_keys" USING btree ("company_id", "idempotency_key");--> statement-breakpoint +CREATE INDEX IF NOT EXISTS "issue_create_idempotency_keys_issue_idx" + ON "issue_create_idempotency_keys" USING btree ("issue_id");--> statement-breakpoint +CREATE INDEX IF NOT EXISTS "issues_open_normalized_title_created_idx" + ON "issues" USING btree ( + "company_id", + "parent_id", + lower(regexp_replace(btrim("title"), '\s+', ' ', 'g')), + "created_at" + ) + WHERE "hidden_at" is null and "status" not in ('done', 'cancelled'); diff --git a/packages/db/src/migrations/meta/_journal.json b/packages/db/src/migrations/meta/_journal.json index 80f6e4628c..08362a9f16 100644 --- a/packages/db/src/migrations/meta/_journal.json +++ b/packages/db/src/migrations/meta/_journal.json @@ -1184,6 +1184,13 @@ "when": 1784037600000, "tag": "0170_company_skill_policies", "breakpoints": true + }, + { + "idx": 171, + "version": "7", + "when": 1784160000000, + "tag": "0171_issue_create_idempotency_keys", + "breakpoints": true } ] } diff --git a/packages/db/src/schema/index.ts b/packages/db/src/schema/index.ts index c84a2ab120..636f3154d2 100644 --- a/packages/db/src/schema/index.ts +++ b/packages/db/src/schema/index.ts @@ -67,6 +67,7 @@ export { labels } from "./labels.js"; export { issueLabels } from "./issue_labels.js"; export { issueApprovals } from "./issue_approvals.js"; export { issueComments } from "./issue_comments.js"; +export { issueCreateIdempotencyKeys } from "./issue_create_idempotency_keys.js"; export { issueThreadInteractions } from "./issue_thread_interactions.js"; export { issueTreeHolds } from "./issue_tree_holds.js"; export { issueTreeHoldMembers } from "./issue_tree_hold_members.js"; diff --git a/packages/db/src/schema/issue_create_idempotency_keys.ts b/packages/db/src/schema/issue_create_idempotency_keys.ts new file mode 100644 index 0000000000..c428d9943d --- /dev/null +++ b/packages/db/src/schema/issue_create_idempotency_keys.ts @@ -0,0 +1,21 @@ +import { index, pgTable, text, timestamp, uniqueIndex, uuid } from "drizzle-orm/pg-core"; +import { companies } from "./companies.js"; +import { issues } from "./issues.js"; + +export const issueCreateIdempotencyKeys = pgTable( + "issue_create_idempotency_keys", + { + id: uuid("id").primaryKey().defaultRandom(), + companyId: uuid("company_id").notNull().references(() => companies.id, { onDelete: "cascade" }), + idempotencyKey: text("idempotency_key").notNull(), + issueId: uuid("issue_id").notNull().references(() => issues.id, { onDelete: "cascade" }), + createdAt: timestamp("created_at", { withTimezone: true }).notNull().defaultNow(), + }, + (table) => ({ + companyKeyIdx: uniqueIndex("issue_create_idempotency_keys_company_key_uq").on( + table.companyId, + table.idempotencyKey, + ), + issueIdx: index("issue_create_idempotency_keys_issue_idx").on(table.issueId), + }), +); diff --git a/packages/db/src/schema/issues.ts b/packages/db/src/schema/issues.ts index 9735c8700b..a7bc5a14fc 100644 --- a/packages/db/src/schema/issues.ts +++ b/packages/db/src/schema/issues.ts @@ -94,6 +94,14 @@ export const issues = pgTable( dueMonitorIdx: index("issues_company_monitor_due_idx").on(table.companyId, table.monitorNextCheckAt), companyUpdatedIdx: index("issues_company_updated_idx").on(table.companyId, table.updatedAt), companyCreatedIdx: index("issues_company_created_idx").on(table.companyId, table.createdAt), + openNormalizedTitleCreatedIdx: index("issues_open_normalized_title_created_idx") + .on( + table.companyId, + table.parentId, + sql`lower(regexp_replace(btrim(${table.title}), '\\s+', ' ', 'g'))`, + table.createdAt, + ) + .where(sql`${table.hiddenAt} is null and ${table.status} not in ('done', 'cancelled')`), companyPriorityIdx: index("issues_company_priority_idx").on(table.companyId, table.priority), identifierIdx: uniqueIndex("issues_identifier_idx").on(table.identifier), titleSearchIdx: index("issues_title_search_idx").using("gin", table.title.op("gin_trgm_ops")), diff --git a/packages/shared/src/validators/issue.ts b/packages/shared/src/validators/issue.ts index 711aa33629..a229f29918 100644 --- a/packages/shared/src/validators/issue.ts +++ b/packages/shared/src/validators/issue.ts @@ -410,11 +410,20 @@ const createIssueBaseSchema = z.object({ }).strict().optional().nullable(), }); +const createIssueDuplicateGuardSchema = { + idempotencyKey: z.string().trim().min(1).max(255).optional().nullable(), + allowDuplicate: z.boolean() + .describe("Bypasses recent-title duplicate detection; idempotency keys always replay their original issue") + .optional() + .default(false), +}; + export const createIssueInputSchema = createIssueBaseSchema.extend({ status: createIssueBaseSchema.shape.status.optional(), + ...createIssueDuplicateGuardSchema, }); -export const createIssueSchema = withCreateIssueStatusDefault(createIssueBaseSchema); +export const createIssueSchema = withCreateIssueStatusDefault(createIssueBaseSchema.extend(createIssueDuplicateGuardSchema)); export type CreateIssue = z.infer; diff --git a/server/src/__tests__/issue-create-deduplication-routes.test.ts b/server/src/__tests__/issue-create-deduplication-routes.test.ts new file mode 100644 index 0000000000..3958a6dd28 --- /dev/null +++ b/server/src/__tests__/issue-create-deduplication-routes.test.ts @@ -0,0 +1,283 @@ +import { randomUUID } from "node:crypto"; +import express from "express"; +import request from "supertest"; +import { eq } from "drizzle-orm"; +import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { + activityLog, + agents, + companies, + createDb, + heartbeatRuns, + issueCreateIdempotencyKeys, + issues, +} from "@paperclipai/db"; +import { + getEmbeddedPostgresTestSupport, + startEmbeddedPostgresTestDatabase, +} from "./helpers/embedded-postgres.js"; +import { actorMiddleware } from "../middleware/auth.js"; +import { errorHandler } from "../middleware/index.js"; +import { issueRoutes } from "../routes/issues.js"; +import { issueService } from "../services/issues.js"; + +const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); +const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; + +if (!embeddedPostgresSupport.supported) { + console.warn( + `Skipping embedded Postgres issue create deduplication route tests on this host: ${ + embeddedPostgresSupport.reason ?? "unsupported environment" + }`, + ); +} + +describeEmbeddedPostgres("issue create deduplication routes", () => { + let db!: ReturnType; + let tempDb: Awaited> | null = null; + + beforeAll(async () => { + tempDb = await startEmbeddedPostgresTestDatabase("paperclip-issue-create-deduplication-routes-"); + db = createDb(tempDb.connectionString); + }, 20_000); + + afterEach(async () => { + await db.delete(activityLog); + await db.delete(issueCreateIdempotencyKeys); + await db.delete(issues); + await db.delete(heartbeatRuns); + await db.delete(agents); + await db.delete(companies); + }); + + afterAll(async () => { + await tempDb?.cleanup(); + }); + + function createApp() { + const app = express(); + app.use(express.json()); + app.use(actorMiddleware(db, { deploymentMode: "local_trusted" })); + app.use("/api", issueRoutes(db, {} as any)); + app.use(errorHandler); + return app; + } + + async function seedCompany() { + const companyId = randomUUID(); + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: `D${companyId.replace(/-/g, "").slice(0, 5).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + }); + return companyId; + } + + async function seedParent(companyId: string) { + const [parent] = await db.insert(issues).values({ + companyId, + title: "Parent issue", + status: "todo", + priority: "medium", + }).returning(); + return parent; + } + + it("replays the existing issue for the same company idempotency key", async () => { + const companyId = await seedCompany(); + const parent = await seedParent(companyId); + const app = createApp(); + + const first = await request(app) + .post(`/api/companies/${companyId}/issues`) + .send({ parentId: parent.id, title: "Prepare release", idempotencyKey: "run-1:prepare-release" }) + .expect(201); + const replay = await request(app) + .post(`/api/companies/${companyId}/issues`) + .send({ + parentId: parent.id, + title: "Different retry payload", + idempotencyKey: "run-1:prepare-release", + allowDuplicate: true, + }) + .expect(200); + + expect(replay.body).toMatchObject({ + id: first.body.id, + title: "Prepare release", + deduplicated: true, + deduplicationReason: "idempotency_key", + }); + expect(await db.select().from(issueCreateIdempotencyKeys)).toHaveLength(1); + }); + + it("returns a recent open sibling whose normalized title matches", async () => { + const companyId = await seedCompany(); + const parent = await seedParent(companyId); + const app = createApp(); + + const first = await request(app) + .post(`/api/companies/${companyId}/issues`) + .send({ parentId: parent.id, title: "Create a single PR" }) + .expect(201); + const duplicate = await request(app) + .post(`/api/companies/${companyId}/issues`) + .send({ parentId: parent.id, title: " create a SINGLE pr " }) + .expect(200); + + expect(duplicate.body).toMatchObject({ + id: first.body.id, + deduplicated: true, + deduplicationReason: "recent_open_title", + }); + }); + + it("serializes keyed and title-only creates for the same issue", async () => { + const companyId = await seedCompany(); + const parent = await seedParent(companyId); + const app = createApp(); + + const [keyed, titleOnly] = await Promise.all([ + request(app) + .post(`/api/companies/${companyId}/issues`) + .send({ parentId: parent.id, title: "Coordinate launch", idempotencyKey: "run-2:coordinate-launch" }), + request(app) + .post(`/api/companies/${companyId}/issues`) + .send({ parentId: parent.id, title: "Coordinate launch" }), + ]); + + expect([keyed.status, titleOnly.status].sort()).toEqual([200, 201]); + expect(keyed.body.id).toBe(titleOnly.body.id); + expect([keyed, titleOnly].find((response) => response.status === 200)?.body).toMatchObject({ + deduplicated: true, + deduplicationReason: "recent_open_title", + }); + expect(await db.select().from(issues).where(eq(issues.parentId, parent.id))).toHaveLength(1); + expect(await db.select().from(issueCreateIdempotencyKeys)).toEqual([ + expect.objectContaining({ issueId: keyed.body.id, idempotencyKey: "run-2:coordinate-launch" }), + ]); + + const replay = await request(app) + .post(`/api/companies/${companyId}/issues`) + .send({ parentId: parent.id, title: "Different title", idempotencyKey: "run-2:coordinate-launch" }) + .expect(200); + expect(replay.body).toMatchObject({ + id: keyed.body.id, + deduplicated: true, + deduplicationReason: "idempotency_key", + }); + }); + + it("allows an explicit duplicate create", async () => { + const companyId = await seedCompany(); + const parent = await seedParent(companyId); + const app = createApp(); + + const first = await request(app) + .post(`/api/companies/${companyId}/issues`) + .send({ parentId: parent.id, title: "Investigate incident" }) + .expect(201); + const duplicate = await request(app) + .post(`/api/companies/${companyId}/issues`) + .send({ parentId: parent.id, title: "Investigate incident", allowDuplicate: true }) + .expect(201); + + expect(duplicate.body.id).not.toBe(first.body.id); + }); + + it("does not apply the route soft guard to internal service creates", async () => { + const companyId = await seedCompany(); + const parent = await seedParent(companyId); + const svc = issueService(db); + + const first = await svc.create(companyId, { + parentId: parent.id, + title: "System-generated follow-up", + status: "todo", + priority: "medium", + }); + const second = await svc.create(companyId, { + parentId: parent.id, + title: "System-generated follow-up", + status: "todo", + priority: "medium", + }); + + expect(second.id).not.toBe(first.id); + }); + + it("does not let closed or older issues block a recreate", async () => { + const companyId = await seedCompany(); + const parent = await seedParent(companyId); + const app = createApp(); + const oldIssueId = randomUUID(); + const closedIssueId = randomUUID(); + await db.insert(issues).values([ + { + id: oldIssueId, + companyId, + parentId: parent.id, + title: "Retry old work", + status: "todo", + priority: "medium", + createdAt: new Date(Date.now() - 49 * 60 * 60 * 1000), + }, + { + id: closedIssueId, + companyId, + parentId: parent.id, + title: "Retry closed work", + status: "done", + priority: "medium", + }, + ]); + + const recreatedOld = await request(app) + .post(`/api/companies/${companyId}/issues`) + .send({ parentId: parent.id, title: "Retry old work" }) + .expect(201); + const recreatedClosed = await request(app) + .post(`/api/companies/${companyId}/issues`) + .send({ parentId: parent.id, title: "Retry closed work" }) + .expect(201); + + expect(recreatedOld.body.id).not.toBe(oldIssueId); + expect(recreatedClosed.body.id).not.toBe(closedIssueId); + }); + + it("stores the request run header on manual creates", async () => { + const companyId = await seedCompany(); + const parent = await seedParent(companyId); + const app = createApp(); + const runId = randomUUID(); + const agentId = randomUUID(); + await db.insert(agents).values({ + id: agentId, + companyId, + name: "Creating agent", + role: "engineer", + status: "active", + adapterType: "codex_local", + adapterConfig: {}, + runtimeConfig: {}, + permissions: {}, + }); + await db.insert(heartbeatRuns).values({ + id: runId, + companyId, + agentId, + status: "running", + }); + + const response = await request(app) + .post(`/api/companies/${companyId}/issues`) + .set("X-Paperclip-Run-Id", runId) + .send({ parentId: parent.id, title: "Attributed create" }) + .expect(201); + const [created] = await db.select().from(issues).where(eq(issues.id, response.body.id)); + + expect(created.originKind).toBe("manual"); + expect(created.originRunId).toBe(runId); + }); +}); diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 85d93c7467..ea8443e8c5 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -6965,10 +6965,12 @@ export function issueRoutes( projectId: createBody.projectId ?? null, executionPolicy, }, actor); + let deduplicationReason: "idempotency_key" | "recent_open_title" | null = null; const issue = await svc.create(companyId, { ...createBody, ...(taskBridgeOriginForActor(req) ?? {}), id: issueId, + originRunId: createBody.originRunId ?? actor.runId, executionPolicy, ...(sourceTrust ? { sourceTrust } : {}), createdByAgentId: actor.agentId, @@ -6977,7 +6979,21 @@ export function issueRoutes( actorResponsibleUserId: authenticatedActorResponsibleUserId(req), trustExplicitResponsibleUserId: actor.actorType === "user", watchdogActorRunId: actor.runId, + onDeduplicated: (reason) => { + deduplicationReason = reason; + }, }); + if (deduplicationReason) { + const referenceSummary = await issueReferencesSvc.listIssueReferenceSummary(issue.id); + res.status(200).json({ + ...issue, + deduplicated: true, + deduplicationReason, + relatedWork: referenceSummary, + referencedIssueIdentifiers: referenceSummary.outbound.map((item) => item.issue.identifier ?? item.issue.id), + }); + return; + } await issueReferencesSvc.syncIssue(issue.id); await externalObjectsSvc.syncIssueSafely(issue.id); const referenceSummary = await issueReferencesSvc.listIssueReferenceSummary(issue.id); diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index 3d93671edf..ccbcada7f0 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -19,6 +19,7 @@ import { executionWorkspaces, issueApprovals, issueAttachments, + issueCreateIdempotencyKeys, issueInboxArchives, issueLabels, issueWatchdogs, @@ -575,6 +576,9 @@ type IssueCreateInput = Omit & { actorRunId?: string | null; actorResponsibleUserId?: string | null; trustExplicitResponsibleUserId?: boolean; + idempotencyKey?: string | null; + allowDuplicate?: boolean; + onDeduplicated?: (reason: "idempotency_key" | "recent_open_title") => void; }; type IssueChildCreateInput = IssueCreateInput & { acceptanceCriteria?: string[]; @@ -3636,6 +3640,10 @@ export function issueService(db: Db) { const instanceSettings = instanceSettingsService(db); const treeControlSvc = issueTreeControlService(db); + function normalizeCreateIssueTitle(title: string) { + return title.trim().replace(/\s+/g, " ").toLowerCase(); + } + async function getIssueByUuid(id: string) { const row = await db .select() @@ -5973,10 +5981,7 @@ export function issueService(db: Db) { }); }, - create: async ( - companyId: string, - data: IssueCreateInput, - ) => { + create: async (companyId: string, data: IssueCreateInput) => { const { labelIds: inputLabelIds, blockedByIssueIds, @@ -5987,6 +5992,9 @@ export function issueService(db: Db) { actorRunId, actorResponsibleUserId, trustExplicitResponsibleUserId, + idempotencyKey: rawIdempotencyKey, + allowDuplicate, + onDeduplicated, ...issueData } = data; const isolatedWorkspacesEnabled = (await instanceSettings.getExperimental()).enableIsolatedWorkspaces; @@ -6008,6 +6016,62 @@ export function issueService(db: Db) { throw unprocessable("in_progress issues require an assignee"); } return db.transaction(async (tx) => { + const idempotencyKey = rawIdempotencyKey?.trim() || null; + const normalizedTitle = normalizeCreateIssueTitle(issueData.title); + if (allowDuplicate === false) { + const titleGuardKey = + `issue-create:title:${companyId}:${issueData.parentId ?? "root"}:${normalizedTitle}`; + await tx.execute(sql`select pg_advisory_xact_lock(hashtextextended(${titleGuardKey}, 0))`); + } + if (idempotencyKey) { + const idempotencyGuardKey = `issue-create:idempotency:${companyId}:${idempotencyKey}`; + await tx.execute(sql`select pg_advisory_xact_lock(hashtextextended(${idempotencyGuardKey}, 0))`); + } + + let existingIssue: typeof issues.$inferSelect | undefined; + let deduplicationReason: "idempotency_key" | "recent_open_title" | null = null; + if (idempotencyKey) { + [existingIssue] = await tx + .select() + .from(issueCreateIdempotencyKeys) + .innerJoin(issues, eq(issueCreateIdempotencyKeys.issueId, issues.id)) + .where(and( + eq(issueCreateIdempotencyKeys.companyId, companyId), + eq(issueCreateIdempotencyKeys.idempotencyKey, idempotencyKey), + )) + .limit(1) + .then((rows) => rows.map((row) => row.issues)); + if (existingIssue) deduplicationReason = "idempotency_key"; + } + if (!existingIssue && allowDuplicate === false) { + [existingIssue] = await tx + .select() + .from(issues) + .where(and( + eq(issues.companyId, companyId), + issueData.parentId ? eq(issues.parentId, issueData.parentId) : isNull(issues.parentId), + isNull(issues.hiddenAt), + notInArray(issues.status, ["done", "cancelled"]), + gte(issues.createdAt, new Date(Date.now() - 48 * 60 * 60 * 1000)), + sql`lower(regexp_replace(btrim(${issues.title}), '\\s+', ' ', 'g')) = ${normalizedTitle}`, + )) + .orderBy(asc(issues.createdAt), asc(issues.id)) + .limit(1); + if (existingIssue) deduplicationReason = "recent_open_title"; + } + if (existingIssue) { + if (idempotencyKey) { + await tx + .insert(issueCreateIdempotencyKeys) + .values({ companyId, idempotencyKey, issueId: existingIssue.id }) + .onConflictDoNothing(); + } + if (deduplicationReason) onDeduplicated?.(deduplicationReason); + const [enriched] = await withIssueLabels(tx, [existingIssue]); + const [withRelations] = await withIssueRelationSummaries(companyId, [enriched], tx); + return withRelations; + } + const defaultCompanyGoal = await getDefaultCompanyGoal(tx, companyId); let projectWorkspaceId = issueData.projectWorkspaceId ?? null; let executionWorkspaceId = issueData.executionWorkspaceId ?? null; @@ -6193,6 +6257,13 @@ export function issueService(db: Db) { ); const [issue] = await tx.insert(issues).values(values).returning(); + if (idempotencyKey) { + await tx.insert(issueCreateIdempotencyKeys).values({ + companyId, + idempotencyKey, + issueId: issue.id, + }); + } if (watchdog) { await upsertIssueWatchdogForIssue(tx, companyId, issue.id, { agentId: watchdog.agentId,