diff --git a/packages/db/src/migrations/0102_managed_sandbox_dedup_index.sql b/packages/db/src/migrations/0102_managed_sandbox_dedup_index.sql new file mode 100644 index 00000000..5f9c6da4 --- /dev/null +++ b/packages/db/src/migrations/0102_managed_sandbox_dedup_index.sql @@ -0,0 +1,33 @@ +-- One-shot dedup of any pre-existing managed sandbox rows duplicated per company. +-- Keeps the oldest row per (company_id) where driver='sandbox' AND metadata.managedByPaperclip is true. +-- Scoped to Paperclip-managed rows only: tenant-created sandbox envs (without the marker) are untouched. +DO $$ +DECLARE + deleted_count integer; +BEGIN + WITH ranked AS ( + SELECT + id, + row_number() OVER ( + PARTITION BY company_id + ORDER BY created_at ASC, id ASC + ) AS rn + FROM environments + WHERE driver = 'sandbox' + AND (metadata ->> 'managedByPaperclip')::boolean = true + ), + deleted AS ( + DELETE FROM environments + WHERE id IN (SELECT id FROM ranked WHERE rn > 1) + RETURNING 1 + ) + SELECT count(*) INTO deleted_count FROM deleted; + + IF deleted_count > 0 THEN + RAISE NOTICE 'environments_company_managed_sandbox_idx dedup removed % duplicate managed-sandbox row(s)', deleted_count; + END IF; +END $$; +--> statement-breakpoint +CREATE UNIQUE INDEX IF NOT EXISTS "environments_company_managed_sandbox_idx" + ON "environments" ("company_id") + WHERE driver = 'sandbox' AND (metadata ->> 'managedByPaperclip')::boolean = true; diff --git a/packages/db/src/migrations/meta/_journal.json b/packages/db/src/migrations/meta/_journal.json index 7e19d3b4..55ff7687 100644 --- a/packages/db/src/migrations/meta/_journal.json +++ b/packages/db/src/migrations/meta/_journal.json @@ -715,6 +715,13 @@ "when": 1781490000000, "tag": "0101_plugin_company_id_tenant_isolation", "breakpoints": true + }, + { + "idx": 102, + "version": "7", + "when": 1781490100000, + "tag": "0102_managed_sandbox_dedup_index", + "breakpoints": true } ] } \ No newline at end of file diff --git a/packages/db/src/schema/environments.ts b/packages/db/src/schema/environments.ts index 7bb308d8..e64e5c41 100644 --- a/packages/db/src/schema/environments.ts +++ b/packages/db/src/schema/environments.ts @@ -21,6 +21,11 @@ export const environments = pgTable( companyDriverIdx: uniqueIndex("environments_company_driver_idx") .on(table.companyId, table.driver) .where(sql`${table.driver} = 'local'`), + companyManagedSandboxIdx: uniqueIndex("environments_company_managed_sandbox_idx") + .on(table.companyId) + .where( + sql`${table.driver} = 'sandbox' AND (${table.metadata} ->> 'managedByPaperclip')::boolean = true`, + ), companyNameIdx: index("environments_company_name_idx").on(table.companyId, table.name), }), ); diff --git a/server/src/__tests__/environment-service.test.ts b/server/src/__tests__/environment-service.test.ts index 3232301d..37788d9c 100644 --- a/server/src/__tests__/environment-service.test.ts +++ b/server/src/__tests__/environment-service.test.ts @@ -300,6 +300,74 @@ describeEmbeddedPostgres("environmentService leases", () => { expect((rows[0]?.metadata as Record)?.managedKubernetesSandbox).toBe(true); }); + it("rejects a second managed-sandbox row for the same company at the DB level", async () => { + const companyId = randomUUID(); + await db.insert(companies).values({ + id: companyId, + name: "Acme", + status: "active", + createdAt: new Date(), + updatedAt: new Date(), + }); + + const now = new Date(); + await db.insert(environments).values({ + companyId, + name: "First", + driver: "sandbox", + status: "active", + config: { provider: "kubernetes" }, + metadata: { managedByPaperclip: true, managedKubernetesSandbox: true }, + createdAt: now, + updatedAt: now, + }); + + // Partial unique index environments_company_managed_sandbox_idx rejects a + // second row matching driver='sandbox' AND managedByPaperclip=true for the + // same company. This is the DB-level invariant that replaced the previous + // application-side post-insert convergence loop. + const secondInsert = db.insert(environments).values({ + companyId, + name: "Second", + driver: "sandbox", + status: "active", + config: { provider: "kubernetes" }, + metadata: { managedByPaperclip: true, managedKubernetesSandbox: true }, + createdAt: new Date(now.getTime() + 1), + updatedAt: new Date(now.getTime() + 1), + }); + let raisedConstraint: string | null = null; + try { + await secondInsert; + } catch (error) { + raisedConstraint = + (error as { constraint_name?: string; cause?: { constraint_name?: string } }) + ?.constraint_name ?? + (error as { cause?: { constraint_name?: string } })?.cause?.constraint_name ?? + "unknown"; + } + expect(raisedConstraint).toBe("environments_company_managed_sandbox_idx"); + + // Index does NOT cover tenant-created sandbox rows (no managedByPaperclip + // marker) — operators must be able to keep multiple tenant sandbox envs. + await db.insert(environments).values({ + companyId, + name: "Tenant Sandbox", + driver: "sandbox", + status: "active", + config: { provider: "fake" }, + metadata: { tenant: true }, + createdAt: new Date(now.getTime() + 2), + updatedAt: new Date(now.getTime() + 2), + }); + + const rows = await db + .select() + .from(environments) + .where(and(eq(environments.companyId, companyId), eq(environments.driver, "sandbox"))); + expect(rows).toHaveLength(2); + }); + it("does not treat a non-kubernetes sandbox environment as the managed k8s env", async () => { const companyId = randomUUID(); await db.insert(companies).values({ diff --git a/server/src/services/environments.ts b/server/src/services/environments.ts index e4287a3c..69aaa5c8 100644 --- a/server/src/services/environments.ts +++ b/server/src/services/environments.ts @@ -1,4 +1,4 @@ -import { and, asc, desc, eq, sql } from "drizzle-orm"; +import { and, desc, eq, sql } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; import { environmentLeases, environments } from "@paperclipai/db"; import { @@ -235,7 +235,12 @@ export function environmentService(db: Db) { return toEnvironment(updated); } - const row = await db + // The partial unique index `environments_company_managed_sandbox_idx` + // (added in migration 0102) enforces "at most one Paperclip-managed + // sandbox row per company" at the DB level. Use ON CONFLICT DO NOTHING + // keyed on that index so concurrent callers can race the INSERT — only + // one succeeds; losers re-read the surviving row. + const inserted = await db .insert(environments) .values({ companyId, @@ -248,26 +253,18 @@ export function environmentService(db: Db) { createdAt: now, updatedAt: now, }) + .onConflictDoNothing({ + target: [environments.companyId], + where: sql`${environments.driver} = 'sandbox' AND (${environments.metadata} ->> 'managedByPaperclip')::boolean = true`, + }) .returning() .then((rows) => rows[0] ?? null); - if (!row) { - throw new Error("Failed to ensure kubernetes environment"); - } + if (inserted) return toEnvironment(inserted); - // Concurrency: the schema's (companyId, driver) unique index is partial - // on driver='local' only, so there is no DB constraint stopping two - // simultaneous callers (e.g. concurrent heartbeats lazily provisioning a - // new company) from both inserting a managed k8s row. Until a partial - // unique index on (companyId, driver) WHERE the managed marker exists is - // added via migration (the proper long-term fix), converge here: re-read, - // deterministically prefer the oldest managed row, and delete our own - // insert if it lost the race. Both racers compute the same winner, so - // duplicates self-heal instead of persisting. const winner = await db .select() .from(environments) .where(and(eq(environments.companyId, companyId), eq(environments.driver, "sandbox"))) - .orderBy(asc(environments.createdAt), asc(environments.id)) .then( (rows) => rows.find( @@ -277,11 +274,10 @@ export function environmentService(db: Db) { ] === true, ) ?? null, ); - if (winner && winner.id !== row.id) { - await db.delete(environments).where(eq(environments.id, row.id)); - return toEnvironment(winner); + if (!winner) { + throw new Error("Failed to ensure kubernetes environment"); } - return toEnvironment(row); + return toEnvironment(winner); }, /**