From 8074c5bd11713349da8bb38e1bec8291eaae69d3 Mon Sep 17 00:00:00 2001 From: Dotta Date: Mon, 6 Jul 2026 13:06:58 +0000 Subject: [PATCH] Fix Greptile findings on extracted work --- ...131_issue_work_product_identity_unique.sql | 16 ++++++++++ packages/db/src/migrations/meta/_journal.json | 7 +++++ packages/db/src/schema/issue_work_products.ts | 8 +++++ ...heartbeat-stale-queue-invalidation.test.ts | 7 +++-- .../__tests__/work-product-adoption.test.ts | 30 +++++++++++++++++++ server/src/services/heartbeat.ts | 2 +- server/src/services/work-product-adoption.ts | 21 +++++++------ 7 files changed, 78 insertions(+), 13 deletions(-) create mode 100644 packages/db/src/migrations/0131_issue_work_product_identity_unique.sql diff --git a/packages/db/src/migrations/0131_issue_work_product_identity_unique.sql b/packages/db/src/migrations/0131_issue_work_product_identity_unique.sql new file mode 100644 index 00000000000..85b563a1211 --- /dev/null +++ b/packages/db/src/migrations/0131_issue_work_product_identity_unique.sql @@ -0,0 +1,16 @@ +DELETE FROM "issue_work_products" loser +USING "issue_work_products" keeper +WHERE loser."company_id" = keeper."company_id" + AND loser."issue_id" = keeper."issue_id" + AND loser."type" = keeper."type" + AND loser."provider" = keeper."provider" + AND loser."external_id" = keeper."external_id" + AND ( + keeper."updated_at" > loser."updated_at" + OR ( + keeper."updated_at" = loser."updated_at" + AND keeper."id"::text > loser."id"::text + ) + );--> statement-breakpoint +CREATE UNIQUE INDEX IF NOT EXISTS "issue_work_products_company_issue_identity_uq" + ON "issue_work_products" USING btree ("company_id", "issue_id", "type", "provider", "external_id"); diff --git a/packages/db/src/migrations/meta/_journal.json b/packages/db/src/migrations/meta/_journal.json index e3e0473eb20..19569f5c472 100644 --- a/packages/db/src/migrations/meta/_journal.json +++ b/packages/db/src/migrations/meta/_journal.json @@ -918,6 +918,13 @@ "when": 1783025224120, "tag": "0130_run_responsible_user_invariant", "breakpoints": true + }, + { + "idx": 131, + "version": "7", + "when": 1783343000000, + "tag": "0131_issue_work_product_identity_unique", + "breakpoints": true } ] } diff --git a/packages/db/src/schema/issue_work_products.ts b/packages/db/src/schema/issue_work_products.ts index 8b568f9d1e5..5ac76862982 100644 --- a/packages/db/src/schema/issue_work_products.ts +++ b/packages/db/src/schema/issue_work_products.ts @@ -5,6 +5,7 @@ import { pgTable, text, timestamp, + uniqueIndex, uuid, } from "drizzle-orm/pg-core"; import type { SourceTrustMetadata } from "@paperclipai/shared"; @@ -58,6 +59,13 @@ export const issueWorkProducts = pgTable( table.provider, table.externalId, ), + companyIssueProviderExternalUq: uniqueIndex("issue_work_products_company_issue_identity_uq").on( + table.companyId, + table.issueId, + table.type, + table.provider, + table.externalId, + ), companyUpdatedIdx: index("issue_work_products_company_updated_idx").on( table.companyId, table.updatedAt, diff --git a/server/src/__tests__/heartbeat-stale-queue-invalidation.test.ts b/server/src/__tests__/heartbeat-stale-queue-invalidation.test.ts index dc5f83c59d0..32611ab4fab 100644 --- a/server/src/__tests__/heartbeat-stale-queue-invalidation.test.ts +++ b/server/src/__tests__/heartbeat-stale-queue-invalidation.test.ts @@ -394,7 +394,7 @@ describeEmbeddedPostgres("heartbeat stale queued-run invalidation", () => { expect(countExecuteCallsForRun(run!.id)).toBe(1); }); - it("skips generic timer wakes by default when there is no assigned issue work", async () => { + it("allows legacy generic timer wakes by default when no skip policy is set", async () => { const { agentId } = await seedCompanyAndAgent({ heartbeatConfig: { enabled: true, @@ -406,8 +406,9 @@ describeEmbeddedPostgres("heartbeat stale queued-run invalidation", () => { triggerDetail: "schedule", }); - expect(run).toBeNull(); - expect(mockAdapterExecute).not.toHaveBeenCalled(); + expect(run).not.toBeNull(); + await waitForCondition(async () => countExecuteCallsForRun(run!.id) > 0); + expect(countExecuteCallsForRun(run!.id)).toBe(1); }); it("allows explicit proactive generic timer wakes without assigned issue work", async () => { diff --git a/server/src/__tests__/work-product-adoption.test.ts b/server/src/__tests__/work-product-adoption.test.ts index 3ff4b748100..f49a09cc837 100644 --- a/server/src/__tests__/work-product-adoption.test.ts +++ b/server/src/__tests__/work-product-adoption.test.ts @@ -251,4 +251,34 @@ describeEmbeddedPostgres("work product adoption", () => { reviewState: "needs_board_review", }); }); + + it("deduplicates concurrent adoption for the same external work product", async () => { + await seedBase(); + + const input = { + db, + companyId, + issueId, + runId, + projectId, + executionWorkspaceId, + resultJson: { + pullRequestUrl: "https://github.com/paperclipai/paperclip/pull/11218", + }, + }; + + await Promise.all([ + adoptWorkProductsForRun(input), + adoptWorkProductsForRun(input), + ]); + + const rows = await db.select().from(issueWorkProducts).where(eq(issueWorkProducts.issueId, issueId)); + expect(rows).toHaveLength(1); + expect(rows[0]).toMatchObject({ + type: "pull_request", + provider: "github", + externalId: "paperclipai/paperclip#11218", + isPrimary: true, + }); + }); }); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 316885e6704..4c9774f82b8 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -8672,7 +8672,7 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {}) heartbeat.skipTimerWhenNoActionableWork ?? heartbeat.requireActionableTimerWork ?? heartbeat.issueOnlyTimer, - true, + false, ), maxDailyRuns: normalizeOptionalNonNegativeInteger( heartbeat.maxDailyRuns ?? heartbeat.dailyRunLimit ?? heartbeat.dailyRunCap ?? heartbeat.maxRunsPerDay, diff --git a/server/src/services/work-product-adoption.ts b/server/src/services/work-product-adoption.ts index 965a8b0c0b8..97332c307bd 100644 --- a/server/src/services/work-product-adoption.ts +++ b/server/src/services/work-product-adoption.ts @@ -472,22 +472,25 @@ export async function adoptWorkProductsForRun(input: { )); } - if (existing) { - await tx - .update(issueWorkProducts) - .set(values) - .where(eq(issueWorkProducts.id, existing.id)); - return "updated" as const; - } - await tx .insert(issueWorkProducts) .values({ ...values, createdByRunId: input.runId, createdAt: now, + }) + .onConflictDoUpdate({ + target: [ + issueWorkProducts.companyId, + issueWorkProducts.issueId, + issueWorkProducts.type, + issueWorkProducts.provider, + issueWorkProducts.externalId, + ], + set: values, }); - return "created" as const; + + return existing ? "updated" as const : "created" as const; }); if (outcome === "created") created += 1;