From bcfd0088bc9a5e273adb5ea90e3d69f21d456fd4 Mon Sep 17 00:00:00 2001 From: Matt Aitken Date: Thu, 17 Sep 2026 17:09:15 +0100 Subject: [PATCH 1/8] feat(webapp,core): bulk delete endpoint for environment variables POST /api/v1/projects/:projectRef/envvars/:slug/bulk-delete removes up to 1000 variables from one environment in a single call. The body can narrow the delete to values last written by a given source (onlyWrittenBy) or to branch values whose key also has a value on the parent environment (onlyShadowingParent); the response lists the keys deleted and the keys skipped. All value removal now goes through one helper that deletes the value rows, their secret references and secret store entries in a fixed number of statements for any number of keys, drops a variable left with no values, and skips a value that changed while the delete ran. The single-value delete uses the same path. --- .changeset/envvar-bulk-delete.md | 5 + ...s.$projectRef.envvars.$slug.bulk-delete.ts | 91 +++++++ .../environmentVariablesRepository.server.ts | 230 ++++++++++++++--- .../app/v3/environmentVariables/repository.ts | 25 +- .../environmentVariablesRepository.test.ts | 236 +++++++++++++++++- .../fixtures/environmentVariablesFixtures.ts | 2 + packages/core/src/v3/schemas/api.ts | 33 ++- 7 files changed, 586 insertions(+), 36 deletions(-) create mode 100644 .changeset/envvar-bulk-delete.md create mode 100644 apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.bulk-delete.ts diff --git a/.changeset/envvar-bulk-delete.md b/.changeset/envvar-bulk-delete.md new file mode 100644 index 00000000000..4b2837ffb2a --- /dev/null +++ b/.changeset/envvar-bulk-delete.md @@ -0,0 +1,5 @@ +--- +"@trigger.dev/core": patch +--- + +Add a bulk delete endpoint for environment variables, `POST /api/v1/projects/:projectRef/envvars/:slug/bulk-delete`, which removes up to 1000 variables from one environment in a single call and can be limited to values last written by a given source or to branch values that shadow a value on the parent environment. diff --git a/apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.bulk-delete.ts b/apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.bulk-delete.ts new file mode 100644 index 00000000000..c4e833fb15f --- /dev/null +++ b/apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.bulk-delete.ts @@ -0,0 +1,91 @@ +import type { ActionFunctionArgs } from "@remix-run/server-runtime"; +import { json } from "@remix-run/server-runtime"; +import { + BulkDeleteEnvironmentVariablesRequestBody, + type BulkDeleteEnvironmentVariablesResponseBody, +} from "@trigger.dev/core/v3"; +import { z } from "zod"; +import { + authenticatedEnvironmentForAuthentication, + branchNameFromRequest, +} from "~/services/apiAuth.server"; +import { + authenticateEnvVarApiRequest, + authorizeEnvVarApiRequest, +} from "~/services/environmentVariableApiAccess.server"; +import { logger } from "~/services/logger.server"; +import { EnvironmentVariablesRepository } from "~/v3/environmentVariables/environmentVariablesRepository.server"; + +const ParamsSchema = z.object({ + projectRef: z.string(), + slug: z.string(), +}); + +export async function action({ params, request }: ActionFunctionArgs) { + if (request.method.toUpperCase() !== "POST") { + return json({ error: "Method not allowed" }, { status: 405 }); + } + + const parsedParams = ParamsSchema.safeParse(params); + + if (!parsedParams.success) { + return json({ error: "Invalid params" }, { status: 400 }); + } + + try { + const authResult = await authenticateEnvVarApiRequest(request, "write"); + if (!authResult.ok) { + return json({ error: authResult.error }, { status: authResult.status }); + } + const authenticationResult = authResult.authentication; + + const environment = await authenticatedEnvironmentForAuthentication( + authenticationResult, + parsedParams.data.projectRef, + parsedParams.data.slug, + branchNameFromRequest(request) + ); + + const denied = await authorizeEnvVarApiRequest({ + request, + authType: authenticationResult.type, + ability: + authenticationResult.type === "apiKey" && authenticationResult.result.ok + ? authenticationResult.result.ability + : undefined, + organizationId: environment.organizationId, + projectId: environment.project.id, + envType: environment.type, + action: "write", + }); + if (denied) return denied; + + const rawBody = await request.json().catch(() => undefined); + const body = BulkDeleteEnvironmentVariablesRequestBody.safeParse(rawBody); + + if (!body.success) { + return json({ error: "Invalid request body", issues: body.error.issues }, { status: 400 }); + } + + const repository = new EnvironmentVariablesRepository(); + + const result: BulkDeleteEnvironmentVariablesResponseBody = await repository.deleteValues( + environment.project.id, + { + environmentId: environment.id, + keys: body.data.keys, + onlyWrittenBy: body.data.onlyWrittenBy, + onlyShadowingParent: body.data.onlyShadowingParent, + } + ); + + return json(result); + } catch (error) { + if (error instanceof Response) throw error; + logger.error("Failed to bulk delete environment variables", { + error, + projectRef: params.projectRef, + }); + return json({ error: "Internal Server Error" }, { status: 500 }); + } +} diff --git a/apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts b/apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts index 76cc5345149..04eb538f945 100644 --- a/apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts +++ b/apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts @@ -7,6 +7,7 @@ import { boundedIn, Prisma, type PrismaClient, + type PrismaClientOrTransaction, type RuntimeEnvironmentType, } from "@trigger.dev/database"; import { z } from "zod"; @@ -24,9 +25,13 @@ import { type CreateResult, type DeleteEnvironmentVariable, type DeleteEnvironmentVariableValue, + type DeleteEnvironmentVariableValues, + type DeleteEnvironmentVariableValuesResult, type EditEnvironmentVariable, type EditEnvironmentVariableValue, type EnvironmentVariable, + type EnvironmentVariableUpdater, + EnvironmentVariableUpdaterSchema, type EnvironmentVariableWithSecret, type ProjectEnvironmentVariable, type Repository, @@ -56,6 +61,97 @@ function parseSecretKey(key: string) { const SecretValue = z.object({ secret: z.string() }); +function isSameUpdater(stored: unknown, source: EnvironmentVariableUpdater): boolean { + const parsed = EnvironmentVariableUpdaterSchema.safeParse(stored); + if (!parsed.success) { + return false; + } + if (parsed.data.type === "user" && source.type === "user") { + return parsed.data.userId === source.userId; + } + if (parsed.data.type === "integration" && source.type === "integration") { + return parsed.data.integration === source.integration; + } + return false; +} + +export type EnvironmentVariableValueRow = { + id: string; + /** When set, the row is only removed if it still has this version. */ + version?: number; + variableId: string; + key: string; + secretReferenceKey?: string; +}; + +/** + * The single code path that removes value rows of one environment together with their secret + * store entries and secret references, using a fixed number of statements for any number of + * rows. A variable left with no values afterwards is removed as well. + */ +export async function deleteEnvironmentVariableValueRows( + tx: PrismaClientOrTransaction, + projectId: string, + environmentId: string, + rows: EnvironmentVariableValueRow[] +): Promise<{ deleted: EnvironmentVariableValueRow[]; skipped: EnvironmentVariableValueRow[] }> { + if (rows.length === 0) { + return { deleted: [], skipped: [] }; + } + + const removed = await tx.environmentVariableValue.deleteMany({ + where: { + OR: rows.map((row) => + row.version === undefined ? { id: row.id } : { id: row.id, version: row.version } + ), + }, + }); + + let deleted = rows; + let skipped: EnvironmentVariableValueRow[] = []; + if (removed.count < rows.length) { + const survivors = await tx.environmentVariableValue.findMany({ + where: { id: { in: boundedIn(rows.map((row) => row.id)) } }, + select: { id: true }, + }); + const survivorIds = new Set(survivors.map((value) => value.id)); + deleted = rows.filter((row) => !survivorIds.has(row.id)); + skipped = rows.filter((row) => survivorIds.has(row.id)); + } + + if (deleted.length === 0) { + return { deleted, skipped }; + } + + const referenceKeys = deleted.flatMap((row) => + row.secretReferenceKey ? [row.secretReferenceKey] : [] + ); + if (referenceKeys.length > 0) { + await tx.secretReference.deleteMany({ where: { key: { in: boundedIn(referenceKeys) } } }); + } + + await tx.secretStore.deleteMany({ + where: { + key: { in: boundedIn(deleted.map((row) => secretKey(projectId, environmentId, row.key))) }, + }, + }); + + const emptied = await tx.environmentVariable.findMany({ + where: { + id: { in: boundedIn(deleted.map((row) => row.variableId)) }, + values: { none: {} }, + }, + select: { id: true }, + }); + if (emptied.length > 0) { + await tx.environmentVariable.deleteMany({ + where: { id: { in: boundedIn(emptied.map((variable) => variable.id)) } }, + }); + } + + return { deleted, skipped }; +} + export class EnvironmentVariablesRepository implements Repository { constructor( private prismaClient: PrismaClient = prisma, @@ -870,11 +966,7 @@ export class EnvironmentVariablesRepository implements Repository { deletedAt: null, }, select: { - environments: { - select: { - id: true, - }, - }, + id: true, }, }); @@ -887,9 +979,11 @@ export class EnvironmentVariablesRepository implements Repository { id: true, key: true, values: { + where: { + environmentId: options.environmentId, + }, select: { id: true, - environmentId: true, valueReference: { select: { key: true, @@ -900,6 +994,7 @@ export class EnvironmentVariablesRepository implements Repository { }, where: { id: options.id, + projectId, }, }); @@ -907,39 +1002,22 @@ export class EnvironmentVariablesRepository implements Repository { return { success: false as const, error: "Environment variable not found" }; } - const value = environmentVariable.values.find((v) => v.environmentId === options.environmentId); + const value = environmentVariable.values[0]; if (!value) { return { success: false as const, error: "Environment variable value not found" }; } - // If this is the last value, delete the whole variable - if (environmentVariable.values.length === 1) { - return this.delete(projectId, { id: options.id }); - } - try { await $transaction(this.prismaClient, "delete env var value", async (tx) => { - const secretStore = getSecretStore("DATABASE", { - prismaClient: tx, - }); - - const key = secretKey(projectId, options.environmentId, environmentVariable.key); - await secretStore.deleteSecret(key); - - if (value.valueReference) { - await tx.secretReference.delete({ - where: { - key: value.valueReference.key, - }, - }); - } - - await tx.environmentVariableValue.delete({ - where: { + await deleteEnvironmentVariableValueRows(tx, projectId, options.environmentId, [ + { id: value.id, + variableId: environmentVariable.id, + key: environmentVariable.key, + secretReferenceKey: value.valueReference?.key, }, - }); + ]); }); return { @@ -952,6 +1030,98 @@ export class EnvironmentVariablesRepository implements Repository { }; } } + + async deleteValues( + projectId: string, + options: DeleteEnvironmentVariableValues + ): Promise { + const keys = Array.from(new Set(options.keys)); + if (keys.length === 0) { + return { deleted: [], skipped: [] }; + } + + const deletedKeys = await $transaction( + this.prismaClient, + "delete env var values", + async (tx) => { + let parentEnvironmentId: string | undefined; + if (options.onlyShadowingParent) { + const environment = await tx.runtimeEnvironment.findFirst({ + where: { id: options.environmentId, projectId }, + select: { parentEnvironmentId: true }, + }); + parentEnvironmentId = environment?.parentEnvironmentId ?? undefined; + if (!parentEnvironmentId) { + return []; + } + } + + const environmentIds = parentEnvironmentId + ? [options.environmentId, parentEnvironmentId] + : [options.environmentId]; + + const variables = await tx.environmentVariable.findMany({ + where: { + projectId, + key: { in: boundedIn(keys) }, + project: { deletedAt: null }, + }, + select: { + id: true, + key: true, + values: { + where: { environmentId: { in: boundedIn(environmentIds) } }, + select: { + id: true, + version: true, + environmentId: true, + lastUpdatedBy: true, + valueReference: { select: { key: true } }, + }, + }, + }, + }); + + const rows: EnvironmentVariableValueRow[] = []; + for (const variable of variables) { + const own = variable.values.find((v) => v.environmentId === options.environmentId); + if (!own) { + continue; + } + if (options.onlyWrittenBy && !isSameUpdater(own.lastUpdatedBy, options.onlyWrittenBy)) { + continue; + } + if ( + parentEnvironmentId && + !variable.values.some((v) => v.environmentId === parentEnvironmentId) + ) { + continue; + } + rows.push({ + id: own.id, + version: own.version, + variableId: variable.id, + key: variable.key, + secretReferenceKey: own.valueReference?.key, + }); + } + + const { deleted } = await deleteEnvironmentVariableValueRows( + tx, + projectId, + options.environmentId, + rows + ); + return deleted.map((row) => row.key); + } + ); + + const deleted = new Set(deletedKeys ?? []); + return { + deleted: keys.filter((key) => deleted.has(key)), + skipped: keys.filter((key) => !deleted.has(key)), + }; + } } // Derived from the slim AuthenticatedEnvironment so a full AE satisfies diff --git a/apps/webapp/app/v3/environmentVariables/repository.ts b/apps/webapp/app/v3/environmentVariables/repository.ts index 89f1000fc64..9f1a4044262 100644 --- a/apps/webapp/app/v3/environmentVariables/repository.ts +++ b/apps/webapp/app/v3/environmentVariables/repository.ts @@ -6,7 +6,7 @@ export const EnvironmentVariableKey = z .nonempty("Key is required") .regex(/^\w+$/, "Keys can only use alphanumeric characters and underscores"); -const EnvironmentVariableUpdaterSchema = z.discriminatedUnion("type", [ +export const EnvironmentVariableUpdaterSchema = z.discriminatedUnion("type", [ z.object({ type: z.literal("user"), userId: z.string(), @@ -63,6 +63,18 @@ export const DeleteEnvironmentVariableValue = z.object({ }); export type DeleteEnvironmentVariableValue = z.infer; +export type DeleteEnvironmentVariableValues = { + environmentId: string; + keys: string[]; + onlyWrittenBy?: EnvironmentVariableUpdater; + onlyShadowingParent?: boolean; +}; + +export type DeleteEnvironmentVariableValuesResult = { + deleted: string[]; + skipped: string[]; +}; + // Forms preserve explicit empty strings through their custom coercion. // A missing field is still invalid. export const EnvironmentVariableValue = z.string(); @@ -140,4 +152,15 @@ export interface Repository { ): Promise; delete(projectId: string, options: DeleteEnvironmentVariable): Promise; deleteValue(projectId: string, options: DeleteEnvironmentVariableValue): Promise; + /** + * Remove the environment's own values for `keys` in one transaction. A key is skipped when the + * environment holds no value for it, when `onlyWrittenBy` is set and the value was last written + * by someone else, when `onlyShadowingParent` is set and the parent environment holds no value + * for it, or when the value changed while the delete was running. A variable left with no + * values is removed entirely. + */ + deleteValues( + projectId: string, + options: DeleteEnvironmentVariableValues + ): Promise; } diff --git a/apps/webapp/test/environmentVariablesRepository.test.ts b/apps/webapp/test/environmentVariablesRepository.test.ts index 445f2436311..3b4e91edb1d 100644 --- a/apps/webapp/test/environmentVariablesRepository.test.ts +++ b/apps/webapp/test/environmentVariablesRepository.test.ts @@ -19,7 +19,10 @@ vi.mock("~/db.server", () => ({ import { postgresTest } from "@internal/testcontainers"; import { emptyEnvironmentVariableValuesEnabled } from "~/v3/environmentVariables/emptyValuesFlag.server"; -import { EnvironmentVariablesRepository } from "~/v3/environmentVariables/environmentVariablesRepository.server"; +import { + deleteEnvironmentVariableValueRows, + EnvironmentVariablesRepository, +} from "~/v3/environmentVariables/environmentVariablesRepository.server"; import { createEnvironmentVariable, createRuntimeEnvironment, @@ -761,3 +764,234 @@ postgresTest( ).toBe(false); } ); + +describe("EnvironmentVariablesRepository.deleteValues", () => { + const vercel = { type: "integration" as const, integration: "vercel" }; + + async function createBranchWithParent( + prisma: Parameters[0] + ) { + const { user, organization, project } = await createTestOrgProjectWithMember(prisma); + const parent = await createRuntimeEnvironment(prisma, { + projectId: project.id, + organizationId: organization.id, + type: "PREVIEW", + }); + const branch = await createRuntimeEnvironment(prisma, { + projectId: project.id, + organizationId: organization.id, + type: "PREVIEW", + parentEnvironmentId: parent.id, + }); + const repository = new EnvironmentVariablesRepository(prisma, prisma); + + const write = async ( + environmentId: string, + variables: Record, + lastUpdatedBy?: Parameters[1]["lastUpdatedBy"] + ) => { + const result = await repository.create(project.id, { + override: true, + environmentIds: [environmentId], + variables: Object.entries(variables).map(([key, value]) => ({ key, value })), + lastUpdatedBy, + }); + expect(result.success).toBe(true); + }; + + const ownKeys = async (environmentId: string) => { + const values = await prisma.environmentVariableValue.findMany({ + where: { environmentId, variable: { projectId: project.id } }, + select: { variable: { select: { key: true } } }, + }); + return values.map((v) => v.variable.key).sort(); + }; + + const variableKeys = async () => + (await prisma.environmentVariable.findMany({ where: { projectId: project.id } })) + .map((v) => v.key) + .sort(); + + const secretRows = async (environmentId: string) => { + const prefix = `environmentvariable:${project.id}:${environmentId}:`; + const store = await prisma.secretStore.findMany({ where: { key: { startsWith: prefix } } }); + const references = await prisma.secretReference.findMany({ + where: { key: { startsWith: prefix } }, + }); + return { + store: store.map((s) => s.key.slice(prefix.length)).sort(), + references: references.map((r) => r.key.slice(prefix.length)).sort(), + }; + }; + + return { user, project, parent, branch, repository, write, ownKeys, variableKeys, secretRows }; + } + + postgresTest("removes several values with their secrets and references", async ({ prisma }) => { + const { project, branch, repository, write, ownKeys, variableKeys, secretRows } = + await createBranchWithParent(prisma); + await write(branch.id, { A: "a", B: "b", C: "c" }, vercel); + + const result = await repository.deleteValues(project.id, { + environmentId: branch.id, + keys: ["A", "B", "A", "MISSING"], + }); + + expect(result).toEqual({ deleted: ["A", "B"], skipped: ["MISSING"] }); + expect(await ownKeys(branch.id)).toEqual(["C"]); + expect(await variableKeys()).toEqual(["C"]); + expect(await secretRows(branch.id)).toEqual({ store: ["C"], references: ["C"] }); + expect(await repository.getEnvironmentVariables(project.id, branch.id)).toEqual([ + { key: "C", value: "c" }, + ]); + }); + + postgresTest("onlyWrittenBy keeps values last written by anyone else", async ({ prisma }) => { + const { user, project, branch, repository, write, ownKeys } = + await createBranchWithParent(prisma); + const other = await prisma.user.create({ + data: { email: `${branch.id}@test.com`, authenticationMethod: "MAGIC_LINK" }, + }); + await write(branch.id, { MINE: "m" }, vercel); + await write(branch.id, { BY_OTHER: "o" }, { type: "integration", integration: "other" }); + await write(branch.id, { BY_NOBODY: "n" }); + await write(branch.id, { BY_ME: "u" }, { type: "user", userId: user.id }); + await write(branch.id, { BY_THEM: "t" }, { type: "user", userId: other.id }); + const keys = ["MINE", "BY_OTHER", "BY_NOBODY", "BY_ME", "BY_THEM"]; + + expect( + await repository.deleteValues(project.id, { + environmentId: branch.id, + keys, + onlyWrittenBy: vercel, + }) + ).toEqual({ deleted: ["MINE"], skipped: ["BY_OTHER", "BY_NOBODY", "BY_ME", "BY_THEM"] }); + expect( + await repository.deleteValues(project.id, { + environmentId: branch.id, + keys, + onlyWrittenBy: { type: "user", userId: user.id }, + }) + ).toEqual({ deleted: ["BY_ME"], skipped: ["MINE", "BY_OTHER", "BY_NOBODY", "BY_THEM"] }); + expect(await ownKeys(branch.id)).toEqual(["BY_NOBODY", "BY_OTHER", "BY_THEM"]); + }); + + postgresTest( + "onlyShadowingParent removes only values the parent also holds", + async ({ prisma }) => { + const { project, parent, branch, repository, write, ownKeys, variableKeys } = + await createBranchWithParent(prisma); + await write(branch.id, { SHARED: "branch-copy", BRANCH_ONLY: "branch" }, vercel); + await write(parent.id, { SHARED: "root", PARENT_ONLY: "root" }, vercel); + + const result = await repository.deleteValues(project.id, { + environmentId: branch.id, + keys: ["SHARED", "BRANCH_ONLY", "PARENT_ONLY"], + onlyShadowingParent: true, + }); + + expect(result).toEqual({ deleted: ["SHARED"], skipped: ["BRANCH_ONLY", "PARENT_ONLY"] }); + expect(await ownKeys(branch.id)).toEqual(["BRANCH_ONLY"]); + expect(await ownKeys(parent.id)).toEqual(["PARENT_ONLY", "SHARED"]); + expect(await variableKeys()).toEqual(["BRANCH_ONLY", "PARENT_ONLY", "SHARED"]); + expect( + Object.fromEntries( + (await repository.getEnvironmentVariables(project.id, branch.id, parent.id)).map( + ({ key, value }) => [key, value] + ) + ) + ).toEqual({ SHARED: "root", BRANCH_ONLY: "branch", PARENT_ONLY: "root" }); + + expect( + await repository.deleteValues(project.id, { + environmentId: parent.id, + keys: ["PARENT_ONLY"], + onlyShadowingParent: true, + }) + ).toEqual({ deleted: [], skipped: ["PARENT_ONLY"] }); + expect(await ownKeys(parent.id)).toEqual(["PARENT_ONLY", "SHARED"]); + } + ); + + postgresTest( + "removes a variable with its last value but keeps one the parent still uses", + async ({ prisma }) => { + const { project, parent, branch, repository, write, variableKeys } = + await createBranchWithParent(prisma); + await write(branch.id, { LAST: "l", BOTH: "b" }, vercel); + await write(parent.id, { BOTH: "root" }, vercel); + + expect( + await repository.deleteValues(project.id, { + environmentId: branch.id, + keys: ["LAST", "BOTH"], + }) + ).toEqual({ deleted: ["LAST", "BOTH"], skipped: [] }); + expect(await variableKeys()).toEqual(["BOTH"]); + expect(await repository.getEnvironmentVariables(project.id, parent.id)).toEqual([ + { key: "BOTH", value: "root" }, + ]); + } + ); + + postgresTest( + "skips a value whose version changed and cleans nothing for it", + async ({ prisma }) => { + const { project, branch, repository, write, ownKeys, secretRows, variableKeys } = + await createBranchWithParent(prisma); + await write(branch.id, { STALE: "s", FRESH: "f" }, vercel); + const values = await prisma.environmentVariableValue.findMany({ + where: { environmentId: branch.id }, + select: { + id: true, + version: true, + variableId: true, + variable: { select: { key: true } }, + valueReference: { select: { key: true } }, + }, + }); + const rows = values.map((value) => ({ + id: value.id, + version: value.variable.key === "STALE" ? value.version + 1 : value.version, + variableId: value.variableId, + key: value.variable.key, + secretReferenceKey: value.valueReference?.key, + })); + + const result = await prisma.$transaction((tx) => + deleteEnvironmentVariableValueRows(tx, project.id, branch.id, rows) + ); + + expect(result.deleted.map((r) => r.key)).toEqual(["FRESH"]); + expect(result.skipped.map((r) => r.key)).toEqual(["STALE"]); + expect(await ownKeys(branch.id)).toEqual(["STALE"]); + expect(await variableKeys()).toEqual(["STALE"]); + expect(await secretRows(branch.id)).toEqual({ store: ["STALE"], references: ["STALE"] }); + expect(await repository.getEnvironmentVariables(project.id, branch.id)).toEqual([ + { key: "STALE", value: "s" }, + ]); + } + ); + + postgresTest("does not reach into another project's variables", async ({ prisma }) => { + const mine = await createBranchWithParent(prisma); + const theirs = await createBranchWithParent(prisma); + await theirs.write(theirs.branch.id, { SHARED: "b" }, vercel); + await theirs.write(theirs.parent.id, { SHARED: "root" }, vercel); + + expect( + await mine.repository.deleteValues(mine.project.id, { + environmentId: theirs.branch.id, + keys: ["SHARED"], + }) + ).toEqual({ deleted: [], skipped: ["SHARED"] }); + expect( + await mine.repository.deleteValues(mine.project.id, { + environmentId: theirs.branch.id, + keys: ["SHARED"], + onlyShadowingParent: true, + }) + ).toEqual({ deleted: [], skipped: ["SHARED"] }); + expect(await theirs.ownKeys(theirs.branch.id)).toEqual(["SHARED"]); + }); +}); diff --git a/apps/webapp/test/fixtures/environmentVariablesFixtures.ts b/apps/webapp/test/fixtures/environmentVariablesFixtures.ts index 934a806a387..6d004537f7b 100644 --- a/apps/webapp/test/fixtures/environmentVariablesFixtures.ts +++ b/apps/webapp/test/fixtures/environmentVariablesFixtures.ts @@ -64,6 +64,7 @@ export async function createRuntimeEnvironment( apiKey?: string; rootApiKeyHiddenAt?: Date | null; slug?: string; + parentEnvironmentId?: string; } ) { const slug = options.slug ?? uniqueId("env"); @@ -73,6 +74,7 @@ export async function createRuntimeEnvironment( type: options.type, projectId: options.projectId, organizationId: options.organizationId, + parentEnvironmentId: options.parentEnvironmentId ?? null, orgMemberId: options.orgMemberId ?? null, rootApiKeyHiddenAt: options.rootApiKeyHiddenAt ?? null, apiKey: options.apiKey ?? uniqueId("api"), diff --git a/packages/core/src/v3/schemas/api.ts b/packages/core/src/v3/schemas/api.ts index 731032613d2..7c3432d40fd 100644 --- a/packages/core/src/v3/schemas/api.ts +++ b/packages/core/src/v3/schemas/api.ts @@ -1602,22 +1602,47 @@ export type UpdateEnvironmentVariableRequestBody = z.infer< typeof UpdateEnvironmentVariableRequestBody >; +export const EnvironmentVariableSource = discriminatedUnion("type", [ + z.object({ type: z.literal("user"), userId: z.string() }), + z.object({ type: z.literal("integration"), integration: z.string() }), +]); + +export type EnvironmentVariableSource = z.infer; + export const ImportEnvironmentVariablesRequestBody = z.object({ variables: z.record(z.string(), z.string()), parentVariables: z.record(z.string(), z.string()).optional(), override: z.boolean().optional(), // When omitted, variables default to non-secret (the DB default is false). isSecret: z.boolean().optional(), - source: discriminatedUnion("type", [ - z.object({ type: z.literal("user"), userId: z.string() }), - z.object({ type: z.literal("integration"), integration: z.string() }), - ]).optional(), + source: EnvironmentVariableSource.optional(), }); export type ImportEnvironmentVariablesRequestBody = z.infer< typeof ImportEnvironmentVariablesRequestBody >; +export const BulkDeleteEnvironmentVariablesRequestBody = z.object({ + keys: z.array(z.string()).min(1).max(1000), + /** Only remove values last written by this source. */ + onlyWrittenBy: EnvironmentVariableSource.optional(), + /** Only remove values whose key also has a value on the parent environment. */ + onlyShadowingParent: z.boolean().optional(), +}); + +export type BulkDeleteEnvironmentVariablesRequestBody = z.infer< + typeof BulkDeleteEnvironmentVariablesRequestBody +>; + +export const BulkDeleteEnvironmentVariablesResponseBody = z.object({ + deleted: z.array(z.string()), + skipped: z.array(z.string()), +}); + +export type BulkDeleteEnvironmentVariablesResponseBody = z.infer< + typeof BulkDeleteEnvironmentVariablesResponseBody +>; + export const EnvironmentVariableResponseBody = z.object({ success: z.boolean(), }); From 759240c8cb94cb389bde3788dd494ebf776e0045 Mon Sep 17 00:00:00 2001 From: Matt Aitken Date: Thu, 17 Sep 2026 17:22:17 +0100 Subject: [PATCH 2/8] fix(webapp): keep env var value deletes to a fixed statement shape The shared value delete pads its per-row OR arms to a power of two so a call site does not mint one prepared statement per distinct row count, folds the emptied-variable sweep into one conditional delete, and takes the environment id per row so a caller cannot mix environments and clear the wrong secret store entry. Adds behavioural tests for the single-value delete path (last value drops the variable, other environments keep theirs, a value without a secret reference deletes cleanly) and for skipped duplicate and already-removed keys in the bulk delete. --- .../environmentVariablesRepository.server.ts | 35 ++++---- .../environmentVariablesRepository.test.ts | 88 ++++++++++++++++++- 2 files changed, 101 insertions(+), 22 deletions(-) diff --git a/apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts b/apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts index 04eb538f945..edfd7455de8 100644 --- a/apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts +++ b/apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts @@ -80,19 +80,21 @@ export type EnvironmentVariableValueRow = { /** When set, the row is only removed if it still has this version. */ version?: number; variableId: string; + environmentId: string; key: string; secretReferenceKey?: string; }; /** - * The single code path that removes value rows of one environment together with their secret - * store entries and secret references, using a fixed number of statements for any number of - * rows. A variable left with no values afterwards is removed as well. + * The single code path that removes value rows together with their secret store entries and + * secret references, in a fixed number of statements for any number of rows. A variable left + * with no values afterwards is removed as well; under READ COMMITTED a value created for that + * variable at the same moment can still be swept away with it, a pre-existing window that this + * narrows but does not close. */ export async function deleteEnvironmentVariableValueRows( tx: PrismaClientOrTransaction, projectId: string, - environmentId: string, rows: EnvironmentVariableValueRow[] ): Promise<{ deleted: EnvironmentVariableValueRow[]; skipped: EnvironmentVariableValueRow[] }> { if (rows.length === 0) { @@ -101,7 +103,7 @@ export async function deleteEnvironmentVariableValueRows( const removed = await tx.environmentVariableValue.deleteMany({ where: { - OR: rows.map((row) => + OR: boundedIn(rows).map((row) => row.version === undefined ? { id: row.id } : { id: row.id, version: row.version } ), }, @@ -132,22 +134,18 @@ export async function deleteEnvironmentVariableValueRows( await tx.secretStore.deleteMany({ where: { - key: { in: boundedIn(deleted.map((row) => secretKey(projectId, environmentId, row.key))) }, + key: { + in: boundedIn(deleted.map((row) => secretKey(projectId, row.environmentId, row.key))), + }, }, }); - const emptied = await tx.environmentVariable.findMany({ + await tx.environmentVariable.deleteMany({ where: { id: { in: boundedIn(deleted.map((row) => row.variableId)) }, values: { none: {} }, }, - select: { id: true }, }); - if (emptied.length > 0) { - await tx.environmentVariable.deleteMany({ - where: { id: { in: boundedIn(emptied.map((variable) => variable.id)) } }, - }); - } return { deleted, skipped }; } @@ -1010,10 +1008,11 @@ export class EnvironmentVariablesRepository implements Repository { try { await $transaction(this.prismaClient, "delete env var value", async (tx) => { - await deleteEnvironmentVariableValueRows(tx, projectId, options.environmentId, [ + await deleteEnvironmentVariableValueRows(tx, projectId, [ { id: value.id, variableId: environmentVariable.id, + environmentId: options.environmentId, key: environmentVariable.key, secretReferenceKey: value.valueReference?.key, }, @@ -1101,17 +1100,13 @@ export class EnvironmentVariablesRepository implements Repository { id: own.id, version: own.version, variableId: variable.id, + environmentId: options.environmentId, key: variable.key, secretReferenceKey: own.valueReference?.key, }); } - const { deleted } = await deleteEnvironmentVariableValueRows( - tx, - projectId, - options.environmentId, - rows - ); + const { deleted } = await deleteEnvironmentVariableValueRows(tx, projectId, rows); return deleted.map((row) => row.key); } ); diff --git a/apps/webapp/test/environmentVariablesRepository.test.ts b/apps/webapp/test/environmentVariablesRepository.test.ts index 3b4e91edb1d..1758f5331a5 100644 --- a/apps/webapp/test/environmentVariablesRepository.test.ts +++ b/apps/webapp/test/environmentVariablesRepository.test.ts @@ -765,7 +765,7 @@ postgresTest( } ); -describe("EnvironmentVariablesRepository.deleteValues", () => { +describe("EnvironmentVariablesRepository value deletes", () => { const vercel = { type: "integration" as const, integration: "vercel" }; async function createBranchWithParent( @@ -954,12 +954,13 @@ describe("EnvironmentVariablesRepository.deleteValues", () => { id: value.id, version: value.variable.key === "STALE" ? value.version + 1 : value.version, variableId: value.variableId, + environmentId: branch.id, key: value.variable.key, secretReferenceKey: value.valueReference?.key, })); const result = await prisma.$transaction((tx) => - deleteEnvironmentVariableValueRows(tx, project.id, branch.id, rows) + deleteEnvironmentVariableValueRows(tx, project.id, rows) ); expect(result.deleted.map((r) => r.key)).toEqual(["FRESH"]); @@ -994,4 +995,87 @@ describe("EnvironmentVariablesRepository.deleteValues", () => { ).toEqual({ deleted: [], skipped: ["SHARED"] }); expect(await theirs.ownKeys(theirs.branch.id)).toEqual(["SHARED"]); }); + postgresTest("skips duplicate keys and keys whose row is already gone", async ({ prisma }) => { + const { project, branch, repository, write, ownKeys } = await createBranchWithParent(prisma); + await write(branch.id, { GONE: "g", KEPT: "k" }, vercel); + await prisma.environmentVariableValue.deleteMany({ + where: { environmentId: branch.id, variable: { key: "GONE" } }, + }); + + expect( + await repository.deleteValues(project.id, { + environmentId: branch.id, + keys: ["GONE", "GONE", "KEPT", "KEPT", "NEVER"], + }) + ).toEqual({ deleted: ["KEPT"], skipped: ["GONE", "NEVER"] }); + expect(await ownKeys(branch.id)).toEqual([]); + }); + + postgresTest( + "deleteValue removes the variable with its last value and its secret rows", + async ({ prisma }) => { + const { project, branch, repository, write, variableKeys, secretRows } = + await createBranchWithParent(prisma); + await write(branch.id, { ONLY: "o", OTHER: "x" }, vercel); + const variable = await prisma.environmentVariable.findFirstOrThrow({ + where: { projectId: project.id, key: "ONLY" }, + }); + + expect( + await repository.deleteValue(project.id, { id: variable.id, environmentId: branch.id }) + ).toEqual({ success: true }); + expect(await variableKeys()).toEqual(["OTHER"]); + expect(await secretRows(branch.id)).toEqual({ store: ["OTHER"], references: ["OTHER"] }); + expect( + await repository.deleteValue(project.id, { id: variable.id, environmentId: branch.id }) + ).toEqual({ success: false, error: "Environment variable not found" }); + } + ); + + postgresTest( + "deleteValue keeps the value the variable has in another environment", + async ({ prisma }) => { + const { project, parent, branch, repository, write, ownKeys, variableKeys, secretRows } = + await createBranchWithParent(prisma); + await write(branch.id, { BOTH: "branch" }, vercel); + await write(parent.id, { BOTH: "root" }, vercel); + const variable = await prisma.environmentVariable.findFirstOrThrow({ + where: { projectId: project.id, key: "BOTH" }, + }); + + expect( + await repository.deleteValue(project.id, { id: variable.id, environmentId: branch.id }) + ).toEqual({ success: true }); + expect(await variableKeys()).toEqual(["BOTH"]); + expect(await ownKeys(branch.id)).toEqual([]); + expect(await ownKeys(parent.id)).toEqual(["BOTH"]); + expect(await secretRows(branch.id)).toEqual({ store: [], references: [] }); + expect(await secretRows(parent.id)).toEqual({ store: ["BOTH"], references: ["BOTH"] }); + expect(await repository.getEnvironmentVariables(project.id, branch.id, parent.id)).toEqual([ + { key: "BOTH", value: "root" }, + ]); + expect( + await repository.deleteValue(project.id, { id: variable.id, environmentId: branch.id }) + ).toEqual({ success: false, error: "Environment variable value not found" }); + } + ); + + postgresTest("deleteValue handles a value without a secret reference", async ({ prisma }) => { + const { project, branch, repository, write, variableKeys, secretRows } = + await createBranchWithParent(prisma); + await write(branch.id, { UNLINKED: "u" }, vercel); + const variable = await prisma.environmentVariable.findFirstOrThrow({ + where: { projectId: project.id, key: "UNLINKED" }, + }); + await prisma.environmentVariableValue.updateMany({ + where: { variableId: variable.id, environmentId: branch.id }, + data: { valueReferenceId: null }, + }); + + expect( + await repository.deleteValue(project.id, { id: variable.id, environmentId: branch.id }) + ).toEqual({ success: true }); + expect(await variableKeys()).toEqual([]); + expect((await secretRows(branch.id)).store).toEqual([]); + }); }); From 79d2942d03fb7a1fb55c7029a8fa5d493cd6132d Mon Sep 17 00:00:00 2001 From: Matt Aitken Date: Thu, 17 Sep 2026 17:22:17 +0100 Subject: [PATCH 3/8] feat(sdk,core): add envvars.bulkDelete() and document the endpoint Adds `envvars.bulkDelete(projectRef, slug, { keys, onlyWrittenBy?, onlyShadowingParent? })`, resolving projectRef and slug from the task context like the neighbouring envvars functions, plus the matching `bulkDeleteEnvVars` API client method and param type. Documents the endpoint in the management API reference and caps each key at 256 characters in the request schema. --- .changeset/envvar-bulk-delete.md | 11 ++- docs/docs.json | 3 +- docs/management/envvars/bulk-delete.mdx | 4 + docs/v3-openapi.yaml | 118 ++++++++++++++++++++++++ packages/core/src/v3/apiClient/index.ts | 21 +++++ packages/core/src/v3/apiClient/types.ts | 10 ++ packages/core/src/v3/schemas/api.ts | 2 +- packages/trigger-sdk/src/v3/envvars.ts | 74 ++++++++++++++- packages/trigger-sdk/src/v3/index.ts | 5 +- 9 files changed, 243 insertions(+), 5 deletions(-) create mode 100644 docs/management/envvars/bulk-delete.mdx diff --git a/.changeset/envvar-bulk-delete.md b/.changeset/envvar-bulk-delete.md index 4b2837ffb2a..c48d1e37cc0 100644 --- a/.changeset/envvar-bulk-delete.md +++ b/.changeset/envvar-bulk-delete.md @@ -1,5 +1,14 @@ --- +"@trigger.dev/sdk": patch "@trigger.dev/core": patch --- -Add a bulk delete endpoint for environment variables, `POST /api/v1/projects/:projectRef/envvars/:slug/bulk-delete`, which removes up to 1000 variables from one environment in a single call and can be limited to values last written by a given source or to branch values that shadow a value on the parent environment. +Delete many environment variables in one call with `envvars.bulkDelete()`, backed by the new `POST /api/v1/projects/:projectRef/envvars/:slug/bulk-delete` endpoint. You can limit the delete to values last written by a given source, or to branch values that shadow a value on the parent environment, and the response lists the keys that were deleted and the keys that were skipped. + +```ts +import { envvars } from "@trigger.dev/sdk"; + +const result = await envvars.bulkDelete("proj_yubjwjsfkxnylobaqvqz", "dev", { + keys: ["SLACK_API_KEY", "STRIPE_SECRET_KEY"], +}); +``` diff --git a/docs/docs.json b/docs/docs.json index 13a867be930..0a1eb4b67bf 100644 --- a/docs/docs.json +++ b/docs/docs.json @@ -410,7 +410,8 @@ "management/envvars/create", "management/envvars/retrieve", "management/envvars/update", - "management/envvars/delete" + "management/envvars/delete", + "management/envvars/bulk-delete" ] }, { diff --git a/docs/management/envvars/bulk-delete.mdx b/docs/management/envvars/bulk-delete.mdx new file mode 100644 index 00000000000..df9c0ea35d5 --- /dev/null +++ b/docs/management/envvars/bulk-delete.mdx @@ -0,0 +1,4 @@ +--- +title: "Bulk delete env vars" +openapi: "v3-openapi POST /api/v1/projects/{projectRef}/envvars/{env}/bulk-delete" +--- diff --git a/docs/v3-openapi.yaml b/docs/v3-openapi.yaml index 6c182fad5bc..7f04dddc971 100644 --- a/docs/v3-openapi.yaml +++ b/docs/v3-openapi.yaml @@ -2185,6 +2185,124 @@ paths: override: false }); + "/api/v1/projects/{projectRef}/envvars/{env}/bulk-delete": + parameters: + - $ref: "#/components/parameters/projectRef" + - $ref: "#/components/parameters/env" + post: + operationId: bulk_delete_project_envvars_v1 + summary: Bulk delete environment variables + description: Delete up to 1000 environment variables from a specific project and environment in one request. Keys that have no value in the environment, or that the filters exclude, are returned as skipped. + requestBody: + required: true + content: + application/json: + schema: + type: object + properties: + keys: + type: array + description: The names of the variables to delete from this environment. + minItems: 1 + maxItems: 1000 + items: + type: string + minLength: 1 + maxLength: 256 + onlyWrittenBy: + description: Only delete values that were last written by this source. + oneOf: + - type: object + properties: + type: + type: string + enum: ["user"] + userId: + type: string + required: ["type", "userId"] + - type: object + properties: + type: + type: string + enum: ["integration"] + integration: + type: string + required: ["type", "integration"] + onlyShadowingParent: + type: boolean + description: On a preview branch, only delete values whose key also has a value on the parent environment, so the branch falls back to the shared value. + default: false + required: ["keys"] + responses: + "200": + description: The request completed. Each requested key appears in either `deleted` or `skipped`. + content: + application/json: + schema: + type: object + properties: + deleted: + type: array + description: Keys whose value was removed from the environment. + items: + type: string + skipped: + type: array + description: Keys that were left untouched, because the environment had no value for them or a filter excluded them. + items: + type: string + required: ["deleted", "skipped"] + "400": + description: Invalid request parameters or body + content: + application/json: + schema: + "$ref": "#/components/schemas/ErrorResponse" + "401": + description: Unauthorized request + content: + application/json: + schema: + "$ref": "#/components/schemas/ErrorResponse" + "404": + description: Resource not found + content: + application/json: + schema: + "$ref": "#/components/schemas/ErrorResponse" + tags: + - envvars + security: + - secretKey: [] + - personalAccessToken: [] + x-codeSamples: + - lang: typescript + label: Outside of a task + source: |- + import { envvars } from "@trigger.dev/sdk"; + + const result = await envvars.bulkDelete("proj_yubjwjsfkxnylobaqvqz", "dev", { + keys: ["SLACK_API_KEY", "STRIPE_SECRET_KEY"], + }); + + console.log(result.deleted, result.skipped); + - lang: typescript + label: Inside a task + source: |- + import { envvars, task } from "@trigger.dev/sdk"; + + export const myTask = task({ + id: "my-task", + run: async () => { + // projectRef and env are automatically inferred from the task context + const result = await envvars.bulkDelete({ + keys: ["SLACK_API_KEY", "STRIPE_SECRET_KEY"], + }); + + console.log(result.deleted, result.skipped); + } + }) + "/api/v1/projects/{projectRef}/envvars/{env}/{name}": parameters: - $ref: "#/components/parameters/projectRef" diff --git a/packages/core/src/v3/apiClient/index.ts b/packages/core/src/v3/apiClient/index.ts index 58d074e61b4..61f5fd7ced9 100644 --- a/packages/core/src/v3/apiClient/index.ts +++ b/packages/core/src/v3/apiClient/index.ts @@ -53,6 +53,7 @@ import { CreateUploadPayloadUrlResponseBody, SessionTranscriptResponseBody, CreateWaitpointTokenResponseBody, + BulkDeleteEnvironmentVariablesResponseBody, CreatedSessionResponseBody, DeletedScheduleObject, EndAndContinueSessionResponseBody, @@ -129,6 +130,7 @@ import { STREAM_START_HEADER, } from "./runStream.js"; import type { + BulkDeleteEnvironmentVariablesParams, CreateBulkActionOptions, CreateEnvironmentVariableParams, ImportEnvironmentVariablesParams, @@ -154,6 +156,7 @@ export type CreateBatchApiResponse = Prettify< >; export type { + BulkDeleteEnvironmentVariablesParams, CreateBulkActionOptions, CreateEnvironmentVariableParams, ImportEnvironmentVariablesParams, @@ -1187,6 +1190,24 @@ export class ApiClient { ); } + bulkDeleteEnvVars( + projectRef: string, + slug: string, + body: BulkDeleteEnvironmentVariablesParams, + requestOptions?: ZodFetchOptions + ) { + return zodfetch( + BulkDeleteEnvironmentVariablesResponseBody, + `${this.baseUrl}/api/v1/projects/${projectRef}/envvars/${slug}/bulk-delete`, + { + method: "POST", + headers: this.#getHeaders(false), + body: JSON.stringify(body), + }, + mergeRequestOptions(this.defaultRequestOptions, requestOptions) + ); + } + updateRunMetadata( runId: string, body: UpdateMetadataRequestBody, diff --git a/packages/core/src/v3/apiClient/types.ts b/packages/core/src/v3/apiClient/types.ts index 0684118ff28..1a9ae2e656e 100644 --- a/packages/core/src/v3/apiClient/types.ts +++ b/packages/core/src/v3/apiClient/types.ts @@ -1,4 +1,5 @@ import type { + EnvironmentVariableSource, MachinePresetName, QueueTypeName, RunStatus, @@ -29,6 +30,15 @@ export interface UpdateEnvironmentVariableParams { value: string; } +export type BulkDeleteEnvironmentVariablesParams = { + /** The variables to delete from the environment, up to 1000 per call. */ + keys: string[]; + /** Only delete values that were last written by this source. */ + onlyWrittenBy?: EnvironmentVariableSource; + /** Only delete values on a branch whose key also has a value on the parent environment. */ + onlyShadowingParent?: boolean; +}; + export interface ListRunsQueryParams extends CursorPageParams { status?: Array | RunStatus; taskIdentifier?: Array | string; diff --git a/packages/core/src/v3/schemas/api.ts b/packages/core/src/v3/schemas/api.ts index 7c3432d40fd..f29e24359db 100644 --- a/packages/core/src/v3/schemas/api.ts +++ b/packages/core/src/v3/schemas/api.ts @@ -1623,7 +1623,7 @@ export type ImportEnvironmentVariablesRequestBody = z.infer< >; export const BulkDeleteEnvironmentVariablesRequestBody = z.object({ - keys: z.array(z.string()).min(1).max(1000), + keys: z.array(z.string().min(1).max(256)).min(1).max(1000), /** Only remove values last written by this source. */ onlyWrittenBy: EnvironmentVariableSource.optional(), /** Only remove values whose key also has a value on the parent environment. */ diff --git a/packages/trigger-sdk/src/v3/envvars.ts b/packages/trigger-sdk/src/v3/envvars.ts index 8ff68ab8907..853831bc6e3 100644 --- a/packages/trigger-sdk/src/v3/envvars.ts +++ b/packages/trigger-sdk/src/v3/envvars.ts @@ -1,6 +1,8 @@ import type { ApiPromise, ApiRequestOptions, + BulkDeleteEnvironmentVariablesParams, + BulkDeleteEnvironmentVariablesResponseBody, CreateEnvironmentVariableParams, EnvironmentVariableResponseBody, EnvironmentVariableWithSecret, @@ -15,7 +17,11 @@ import { } from "@trigger.dev/core/v3"; import { tracer } from "./tracer.js"; -export type { CreateEnvironmentVariableParams, ImportEnvironmentVariablesParams }; +export type { + BulkDeleteEnvironmentVariablesParams, + CreateEnvironmentVariableParams, + ImportEnvironmentVariablesParams, +}; export function upload( projectRef: string, @@ -278,6 +284,72 @@ export function del( return apiClient.deleteEnvVar($projectRef, $slug, $name, $requestOptions); } +export function bulkDelete( + projectRef: string, + slug: string, + params: BulkDeleteEnvironmentVariablesParams, + requestOptions?: ApiRequestOptions +): ApiPromise; +export function bulkDelete( + params: BulkDeleteEnvironmentVariablesParams, + requestOptions?: ApiRequestOptions +): ApiPromise; +export function bulkDelete( + projectRefOrParams: string | BulkDeleteEnvironmentVariablesParams, + slugOrRequestOptions?: string | ApiRequestOptions, + params?: BulkDeleteEnvironmentVariablesParams, + requestOptions?: ApiRequestOptions +): ApiPromise { + let $projectRef: string; + let $params: BulkDeleteEnvironmentVariablesParams; + let $slug: string; + const $requestOptions = overloadRequestOptions( + "bulkDelete", + slugOrRequestOptions, + requestOptions + ); + + if (taskContext.ctx) { + if (typeof projectRefOrParams === "string") { + $projectRef = projectRefOrParams; + $slug = + typeof slugOrRequestOptions === "string" + ? slugOrRequestOptions + : taskContext.ctx.environment.slug; + + if (!params) { + throw new Error("params is required"); + } + + $params = params; + } else { + $params = projectRefOrParams; + $projectRef = taskContext.ctx.project.ref; + $slug = taskContext.ctx.environment.slug; + } + } else { + if (typeof projectRefOrParams !== "string") { + throw new Error("projectRef is required"); + } + + if (!slugOrRequestOptions || typeof slugOrRequestOptions !== "string") { + throw new Error("slug is required"); + } + + if (!params) { + throw new Error("params is required"); + } + + $projectRef = projectRefOrParams; + $slug = slugOrRequestOptions; + $params = params; + } + + const apiClient = apiClientManager.clientOrThrow(); + + return apiClient.bulkDeleteEnvVars($projectRef, $slug, $params, $requestOptions); +} + export function update( projectRef: string, slug: string, diff --git a/packages/trigger-sdk/src/v3/index.ts b/packages/trigger-sdk/src/v3/index.ts index 4bdb582d7ae..4ff21a2c5fc 100644 --- a/packages/trigger-sdk/src/v3/index.ts +++ b/packages/trigger-sdk/src/v3/index.ts @@ -65,7 +65,10 @@ export { } from "./deployments.js"; export * as envvars from "./envvars.js"; export * as queues from "./queues.js"; -export type { ImportEnvironmentVariablesParams } from "./envvars.js"; +export type { + BulkDeleteEnvironmentVariablesParams, + ImportEnvironmentVariablesParams, +} from "./envvars.js"; export { configure, auth } from "./auth.js"; export { TriggerClient, type TriggerClientConfig } from "./triggerClient.js"; From f4cca482d3948d1a345efcf1ce789c266f33b545 Mon Sep 17 00:00:00 2001 From: Matt Aitken Date: Thu, 17 Sep 2026 17:30:14 +0100 Subject: [PATCH 4/8] docs(core): clarify bulk env var delete skips and the branch-only filter Says in the API reference, the request schema and the client param type that a key is also skipped when its value changed while the delete ran, and that onlyShadowingParent only takes effect when the request addresses a preview branch. Adds a test for the padded row list path and matches the docs page title to its siblings. --- .../test/environmentVariablesRepository.test.ts | 15 +++++++++++++++ docs/management/envvars/bulk-delete.mdx | 2 +- docs/v3-openapi.yaml | 6 +++--- packages/core/src/v3/apiClient/types.ts | 6 +++++- packages/core/src/v3/schemas/api.ts | 11 ++++++++++- 5 files changed, 34 insertions(+), 6 deletions(-) diff --git a/apps/webapp/test/environmentVariablesRepository.test.ts b/apps/webapp/test/environmentVariablesRepository.test.ts index 1758f5331a5..8927c45e097 100644 --- a/apps/webapp/test/environmentVariablesRepository.test.ts +++ b/apps/webapp/test/environmentVariablesRepository.test.ts @@ -1078,4 +1078,19 @@ describe("EnvironmentVariablesRepository value deletes", () => { expect(await variableKeys()).toEqual([]); expect((await secretRows(branch.id)).store).toEqual([]); }); + + postgresTest("deletes three of four keys through a padded row list", async ({ prisma }) => { + const { project, branch, repository, write, ownKeys, secretRows } = + await createBranchWithParent(prisma); + await write(branch.id, { A: "a", B: "b", C: "c", D: "d" }, vercel); + + expect( + await repository.deleteValues(project.id, { environmentId: branch.id, keys: ["A", "B", "C"] }) + ).toEqual({ deleted: ["A", "B", "C"], skipped: [] }); + expect(await ownKeys(branch.id)).toEqual(["D"]); + expect(await secretRows(branch.id)).toEqual({ store: ["D"], references: ["D"] }); + expect(await repository.getEnvironmentVariables(project.id, branch.id)).toEqual([ + { key: "D", value: "d" }, + ]); + }); }); diff --git a/docs/management/envvars/bulk-delete.mdx b/docs/management/envvars/bulk-delete.mdx index df9c0ea35d5..7979aa60d52 100644 --- a/docs/management/envvars/bulk-delete.mdx +++ b/docs/management/envvars/bulk-delete.mdx @@ -1,4 +1,4 @@ --- -title: "Bulk delete env vars" +title: "Bulk Delete Env Vars" openapi: "v3-openapi POST /api/v1/projects/{projectRef}/envvars/{env}/bulk-delete" --- diff --git a/docs/v3-openapi.yaml b/docs/v3-openapi.yaml index 7f04dddc971..eb265f762cc 100644 --- a/docs/v3-openapi.yaml +++ b/docs/v3-openapi.yaml @@ -2192,7 +2192,7 @@ paths: post: operationId: bulk_delete_project_envvars_v1 summary: Bulk delete environment variables - description: Delete up to 1000 environment variables from a specific project and environment in one request. Keys that have no value in the environment, or that the filters exclude, are returned as skipped. + description: Delete up to 1000 environment variables from a specific project and environment in one request. Keys that have no value in the environment, that the filters exclude, or whose value changed while the delete ran are returned as skipped. requestBody: required: true content: @@ -2230,7 +2230,7 @@ paths: required: ["type", "integration"] onlyShadowingParent: type: boolean - description: On a preview branch, only delete values whose key also has a value on the parent environment, so the branch falls back to the shared value. + description: Only delete values whose key also has a value on the parent environment, so the branch falls back to the shared value. Takes effect only when the request addresses a preview branch, via the `x-trigger-branch` header or the SDK client's `previewBranch` option; on an environment with no parent every key is skipped. default: false required: ["keys"] responses: @@ -2248,7 +2248,7 @@ paths: type: string skipped: type: array - description: Keys that were left untouched, because the environment had no value for them or a filter excluded them. + description: Keys that were left untouched, because the environment had no value for them, a filter excluded them, or their value changed while the delete ran. A skip caused by a concurrent change is transient and a retry can delete the key. items: type: string required: ["deleted", "skipped"] diff --git a/packages/core/src/v3/apiClient/types.ts b/packages/core/src/v3/apiClient/types.ts index 1a9ae2e656e..956084d2347 100644 --- a/packages/core/src/v3/apiClient/types.ts +++ b/packages/core/src/v3/apiClient/types.ts @@ -35,7 +35,11 @@ export type BulkDeleteEnvironmentVariablesParams = { keys: string[]; /** Only delete values that were last written by this source. */ onlyWrittenBy?: EnvironmentVariableSource; - /** Only delete values on a branch whose key also has a value on the parent environment. */ + /** + * Only delete values whose key also has a value on the parent environment. Takes effect only + * when the request addresses a preview branch (`x-trigger-branch` header or the API client's + * `previewBranch` option); on an environment with no parent every key is skipped. + */ onlyShadowingParent?: boolean; }; diff --git a/packages/core/src/v3/schemas/api.ts b/packages/core/src/v3/schemas/api.ts index f29e24359db..4ada5437db7 100644 --- a/packages/core/src/v3/schemas/api.ts +++ b/packages/core/src/v3/schemas/api.ts @@ -1626,7 +1626,11 @@ export const BulkDeleteEnvironmentVariablesRequestBody = z.object({ keys: z.array(z.string().min(1).max(256)).min(1).max(1000), /** Only remove values last written by this source. */ onlyWrittenBy: EnvironmentVariableSource.optional(), - /** Only remove values whose key also has a value on the parent environment. */ + /** + * Only remove values whose key also has a value on the parent environment. Takes effect only + * when the request addresses a preview branch (`x-trigger-branch` header or the API client's + * `previewBranch` option); on an environment with no parent every key is skipped. + */ onlyShadowingParent: z.boolean().optional(), }); @@ -1635,7 +1639,12 @@ export type BulkDeleteEnvironmentVariablesRequestBody = z.infer< >; export const BulkDeleteEnvironmentVariablesResponseBody = z.object({ + /** Keys whose value was removed from the environment. */ deleted: z.array(z.string()), + /** + * Keys left untouched: the environment had no value for them, a filter excluded them, or their + * value changed while the delete ran. A skip caused by a concurrent change is transient. + */ skipped: z.array(z.string()), }); From d7af05f02e0cc2619aa4dea755d1be9299269f91 Mon Sep 17 00:00:00 2001 From: Matt Aitken Date: Thu, 17 Sep 2026 17:43:22 +0100 Subject: [PATCH 5/8] fix(webapp): lock rows the env var bulk delete depends on The shared value delete locks the affected variable rows FOR UPDATE before sweeping variables left with no values, so a value inserted at the same moment is seen rather than cascaded away. With onlyShadowingParent the bulk delete also locks the parent values its candidates rely on and skips any key whose parent value is gone by the time the lock is granted, so a branch is never left without a value. Documents that a retry of an already committed request reports the keys it deleted as skipped, since the delete is idempotent. --- .../environmentVariablesRepository.server.ts | 43 ++++++++++---- .../environmentVariablesRepository.test.ts | 56 +++++++++++++++++++ docs/v3-openapi.yaml | 2 +- packages/core/src/v3/schemas/api.ts | 4 +- 4 files changed, 93 insertions(+), 12 deletions(-) diff --git a/apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts b/apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts index edfd7455de8..9133cac8463 100644 --- a/apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts +++ b/apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts @@ -88,9 +88,11 @@ export type EnvironmentVariableValueRow = { /** * The single code path that removes value rows together with their secret store entries and * secret references, in a fixed number of statements for any number of rows. A variable left - * with no values afterwards is removed as well; under READ COMMITTED a value created for that - * variable at the same moment can still be swept away with it, a pre-existing window that this - * narrows but does not close. + * with no values afterwards is removed as well. The affected variable rows are locked FOR UPDATE + * first: inserting a value takes a FOR KEY SHARE lock on its variable row, which conflicts with + * FOR UPDATE, so an in-flight insert makes the lock wait and the emptiness check then sees the + * new value, while an insert that starts later waits for the commit and recreates the variable + * through its upsert. */ export async function deleteEnvironmentVariableValueRows( tx: PrismaClientOrTransaction, @@ -140,9 +142,13 @@ export async function deleteEnvironmentVariableValueRows( }, }); + const variableIds = boundedIn(deleted.map((row) => row.variableId)); + await tx.$queryRaw<{ id: string }[]>` + SELECT "id" FROM "EnvironmentVariable" WHERE "id" IN (${Prisma.join(variableIds)}) FOR UPDATE + `; await tx.environmentVariable.deleteMany({ where: { - id: { in: boundedIn(deleted.map((row) => row.variableId)) }, + id: { in: boundedIn(variableIds) }, values: { none: {} }, }, }); @@ -1081,7 +1087,8 @@ export class EnvironmentVariablesRepository implements Repository { }, }); - const rows: EnvironmentVariableValueRow[] = []; + let rows: EnvironmentVariableValueRow[] = []; + const parentValueIdByVariable = new Map(); for (const variable of variables) { const own = variable.values.find((v) => v.environmentId === options.environmentId); if (!own) { @@ -1090,11 +1097,14 @@ export class EnvironmentVariablesRepository implements Repository { if (options.onlyWrittenBy && !isSameUpdater(own.lastUpdatedBy, options.onlyWrittenBy)) { continue; } - if ( - parentEnvironmentId && - !variable.values.some((v) => v.environmentId === parentEnvironmentId) - ) { - continue; + if (parentEnvironmentId) { + const parentValue = variable.values.find( + (v) => v.environmentId === parentEnvironmentId + ); + if (!parentValue) { + continue; + } + parentValueIdByVariable.set(variable.id, parentValue.id); } rows.push({ id: own.id, @@ -1106,6 +1116,19 @@ export class EnvironmentVariablesRepository implements Repository { }); } + if (parentEnvironmentId && rows.length > 0) { + const lockedParentValues = await tx.$queryRaw<{ id: string }[]>` + SELECT "id" FROM "EnvironmentVariableValue" + WHERE "id" IN (${Prisma.join(boundedIn(Array.from(parentValueIdByVariable.values())))}) + FOR UPDATE + `; + const lockedIds = new Set(lockedParentValues.map((value) => value.id)); + rows = rows.filter((row) => { + const parentValueId = parentValueIdByVariable.get(row.variableId); + return parentValueId !== undefined && lockedIds.has(parentValueId); + }); + } + const { deleted } = await deleteEnvironmentVariableValueRows(tx, projectId, rows); return deleted.map((row) => row.key); } diff --git a/apps/webapp/test/environmentVariablesRepository.test.ts b/apps/webapp/test/environmentVariablesRepository.test.ts index 8927c45e097..b62f18b201e 100644 --- a/apps/webapp/test/environmentVariablesRepository.test.ts +++ b/apps/webapp/test/environmentVariablesRepository.test.ts @@ -1093,4 +1093,60 @@ describe("EnvironmentVariablesRepository value deletes", () => { { key: "D", value: "d" }, ]); }); + + postgresTest( + "onlyShadowingParent skips a key whose parent value was removed beforehand", + async ({ prisma }) => { + const { project, parent, branch, repository, write, ownKeys } = + await createBranchWithParent(prisma); + await write(branch.id, { SHARED: "branch-copy", STILL_SHARED: "branch-copy" }, vercel); + await write(parent.id, { SHARED: "root", STILL_SHARED: "root" }, vercel); + await prisma.environmentVariableValue.deleteMany({ + where: { environmentId: parent.id, variable: { key: "SHARED" } }, + }); + + expect( + await repository.deleteValues(project.id, { + environmentId: branch.id, + keys: ["SHARED", "STILL_SHARED"], + onlyShadowingParent: true, + }) + ).toEqual({ deleted: ["STILL_SHARED"], skipped: ["SHARED"] }); + expect(await ownKeys(branch.id)).toEqual(["SHARED"]); + } + ); + + postgresTest( + "onlyShadowingParent skips a key whose parent value is being removed", + async ({ prisma }) => { + const { project, parent, branch, repository, write, ownKeys } = + await createBranchWithParent(prisma); + await write(branch.id, { SHARED: "branch-copy" }, vercel); + await write(parent.id, { SHARED: "root" }, vercel); + const parentValue = await prisma.environmentVariableValue.findFirstOrThrow({ + where: { environmentId: parent.id }, + }); + + let releaseDeleter: () => void = () => {}; + const deleterHoldsRow = new Promise((resolve) => { + releaseDeleter = resolve; + }); + const deleter = prisma.$transaction(async (tx) => { + await tx.environmentVariableValue.delete({ where: { id: parentValue.id } }); + await deleterHoldsRow; + }); + const bulk = repository.deleteValues(project.id, { + environmentId: branch.id, + keys: ["SHARED"], + onlyShadowingParent: true, + }); + await new Promise((resolve) => setTimeout(resolve, 300)); + releaseDeleter(); + await deleter; + + expect(await bulk).toEqual({ deleted: [], skipped: ["SHARED"] }); + expect(await ownKeys(branch.id)).toEqual(["SHARED"]); + expect(await ownKeys(parent.id)).toEqual([]); + } + ); }); diff --git a/docs/v3-openapi.yaml b/docs/v3-openapi.yaml index eb265f762cc..7a372c714cc 100644 --- a/docs/v3-openapi.yaml +++ b/docs/v3-openapi.yaml @@ -2248,7 +2248,7 @@ paths: type: string skipped: type: array - description: Keys that were left untouched, because the environment had no value for them, a filter excluded them, or their value changed while the delete ran. A skip caused by a concurrent change is transient and a retry can delete the key. + description: Keys that were left untouched, because the environment had no value for them, a filter excluded them, or their value changed while the delete ran. A skip caused by a concurrent change is transient and a retry can delete the key. The delete is idempotent, so a retry of a request whose first attempt committed reports the keys that attempt deleted as skipped. items: type: string required: ["deleted", "skipped"] diff --git a/packages/core/src/v3/schemas/api.ts b/packages/core/src/v3/schemas/api.ts index 4ada5437db7..3f35b2e6c56 100644 --- a/packages/core/src/v3/schemas/api.ts +++ b/packages/core/src/v3/schemas/api.ts @@ -1643,7 +1643,9 @@ export const BulkDeleteEnvironmentVariablesResponseBody = z.object({ deleted: z.array(z.string()), /** * Keys left untouched: the environment had no value for them, a filter excluded them, or their - * value changed while the delete ran. A skip caused by a concurrent change is transient. + * value changed while the delete ran. A skip caused by a concurrent change is transient. The + * delete is idempotent, so a retry of a request whose first attempt committed reports the keys + * that attempt deleted as skipped. */ skipped: z.array(z.string()), }); From 71b7e15a66250586377f50ba5c312ea918e7e4b5 Mon Sep 17 00:00:00 2001 From: Matt Aitken Date: Thu, 17 Sep 2026 17:46:33 +0100 Subject: [PATCH 6/8] fix(webapp): take env var delete locks in variable-then-value order The bulk delete now locks the variable rows of every candidate before it touches any value row, and with onlyShadowingParent it does so before locking the parent values as well. That is the order a concurrent import takes through its variable upsert, so an import and a delete for the same variable can no longer wait on each other in a cycle and have one side aborted. --- .../environmentVariablesRepository.server.ts | 39 ++++++++++++++----- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts b/apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts index 9133cac8463..520634f01ff 100644 --- a/apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts +++ b/apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts @@ -85,14 +85,30 @@ export type EnvironmentVariableValueRow = { secretReferenceKey?: string; }; +/** + * Locks the given variable rows for the rest of the transaction. Every delete path takes this + * lock before touching any value row, the same variable-then-value order `create` uses through + * its variable upsert, so a concurrent import and a delete never wait on each other in a cycle. + */ +async function lockEnvironmentVariableRows(tx: PrismaClientOrTransaction, variableIds: string[]) { + if (variableIds.length === 0) { + return; + } + await tx.$queryRaw<{ id: string }[]>` + SELECT "id" FROM "EnvironmentVariable" + WHERE "id" IN (${Prisma.join(boundedIn(variableIds))}) + FOR UPDATE + `; +} + /** * The single code path that removes value rows together with their secret store entries and * secret references, in a fixed number of statements for any number of rows. A variable left * with no values afterwards is removed as well. The affected variable rows are locked FOR UPDATE - * first: inserting a value takes a FOR KEY SHARE lock on its variable row, which conflicts with - * FOR UPDATE, so an in-flight insert makes the lock wait and the emptiness check then sees the - * new value, while an insert that starts later waits for the commit and recreates the variable - * through its upsert. + * before anything else: inserting a value takes a FOR KEY SHARE lock on its variable row, which + * conflicts with FOR UPDATE, so an in-flight insert makes the lock wait and the emptiness check + * then sees the new value, while an insert that starts later waits for the commit and recreates + * the variable through its upsert. */ export async function deleteEnvironmentVariableValueRows( tx: PrismaClientOrTransaction, @@ -103,6 +119,11 @@ export async function deleteEnvironmentVariableValueRows( return { deleted: [], skipped: [] }; } + await lockEnvironmentVariableRows( + tx, + rows.map((row) => row.variableId) + ); + const removed = await tx.environmentVariableValue.deleteMany({ where: { OR: boundedIn(rows).map((row) => @@ -142,13 +163,9 @@ export async function deleteEnvironmentVariableValueRows( }, }); - const variableIds = boundedIn(deleted.map((row) => row.variableId)); - await tx.$queryRaw<{ id: string }[]>` - SELECT "id" FROM "EnvironmentVariable" WHERE "id" IN (${Prisma.join(variableIds)}) FOR UPDATE - `; await tx.environmentVariable.deleteMany({ where: { - id: { in: boundedIn(variableIds) }, + id: { in: boundedIn(deleted.map((row) => row.variableId)) }, values: { none: {} }, }, }); @@ -1117,6 +1134,10 @@ export class EnvironmentVariablesRepository implements Repository { } if (parentEnvironmentId && rows.length > 0) { + await lockEnvironmentVariableRows( + tx, + rows.map((row) => row.variableId) + ); const lockedParentValues = await tx.$queryRaw<{ id: string }[]>` SELECT "id" FROM "EnvironmentVariableValue" WHERE "id" IN (${Prisma.join(boundedIn(Array.from(parentValueIdByVariable.values())))}) From 9e943eac0ed433f1812f3b9d3e5bda139d267125 Mon Sep 17 00:00:00 2001 From: Matt Aitken Date: Thu, 17 Sep 2026 17:48:25 +0100 Subject: [PATCH 7/8] fix(webapp): lock the variable row first when editing env var values Editing a variable across several environments locked the value rows it updated before the insert of a new value took its implicit lock on the variable row, the opposite order to imports and deletes. It now locks the variable row as its first statement so every writer takes locks in the same order and none can wait on another in a cycle. --- .../environmentVariablesRepository.server.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts b/apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts index 520634f01ff..0bd87a6bf73 100644 --- a/apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts +++ b/apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts @@ -485,6 +485,8 @@ export class EnvironmentVariablesRepository implements Repository { try { await $transaction(this.prismaClient, "edit env var", async (tx) => { + await lockEnvironmentVariableRows(tx, [options.id]); + const secretStore = getSecretStore("DATABASE", { prismaClient: tx, }); From c3847e664ebf72e9c799cb874642fc4539c3b018 Mon Sep 17 00:00:00 2001 From: Matt Aitken Date: Thu, 17 Sep 2026 18:00:52 +0100 Subject: [PATCH 8/8] fix(webapp): lock the variable row in every env var writer Creating values for an existing variable and editing a single value did not lock the variable row before touching value rows, so a concurrent delete could still form a lock cycle with them. Both now lock the variable row up front, the same way edit and the delete paths do, and the lock helper's docblock is the single statement of that order. Also makes the concurrent parent-delete test deterministic by starting the bulk delete only after the competing delete has run. --- .../environmentVariablesRepository.server.ts | 15 ++++++++++----- .../test/environmentVariablesRepository.test.ts | 15 +++++++++++++-- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts b/apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts index 0bd87a6bf73..ea1a6636e1d 100644 --- a/apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts +++ b/apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts @@ -86,9 +86,9 @@ export type EnvironmentVariableValueRow = { }; /** - * Locks the given variable rows for the rest of the transaction. Every delete path takes this - * lock before touching any value row, the same variable-then-value order `create` uses through - * its variable upsert, so a concurrent import and a delete never wait on each other in a cycle. + * Locks the given variable rows for the rest of the transaction. Every writer of value rows + * calls this before touching any value row, so all of them take locks in the same + * variable-then-value order and no two can wait on each other in a cycle. */ async function lockEnvironmentVariableRows(tx: PrismaClientOrTransaction, variableIds: string[]) { if (variableIds.length === 0) { @@ -107,8 +107,9 @@ async function lockEnvironmentVariableRows(tx: PrismaClientOrTransaction, variab * with no values afterwards is removed as well. The affected variable rows are locked FOR UPDATE * before anything else: inserting a value takes a FOR KEY SHARE lock on its variable row, which * conflicts with FOR UPDATE, so an in-flight insert makes the lock wait and the emptiness check - * then sees the new value, while an insert that starts later waits for the commit and recreates - * the variable through its upsert. + * then sees the new value. An insert that starts later waits for the commit and then either + * recreates the variable or fails its foreign key check and fails the import for that key, in + * both cases without losing data. */ export async function deleteEnvironmentVariableValueRows( tx: PrismaClientOrTransaction, @@ -314,6 +315,8 @@ export class EnvironmentVariablesRepository implements Repository { update: {}, }); + await lockEnvironmentVariableRows(tx, [environmentVariable.id]); + const secretStore = getSecretStore("DATABASE", { prismaClient: tx, }); @@ -631,6 +634,8 @@ export class EnvironmentVariablesRepository implements Repository { try { await $transaction(this.prismaClient, "edit env var value", async (tx) => { + await lockEnvironmentVariableRows(tx, [options.id]); + const secretStore = getSecretStore("DATABASE", { prismaClient: tx, }); diff --git a/apps/webapp/test/environmentVariablesRepository.test.ts b/apps/webapp/test/environmentVariablesRepository.test.ts index b62f18b201e..7843a89ba9f 100644 --- a/apps/webapp/test/environmentVariablesRepository.test.ts +++ b/apps/webapp/test/environmentVariablesRepository.test.ts @@ -1131,17 +1131,28 @@ describe("EnvironmentVariablesRepository value deletes", () => { const deleterHoldsRow = new Promise((resolve) => { releaseDeleter = resolve; }); + let markRowDeleted: () => void = () => {}; + const rowDeleted = new Promise((resolve) => { + markRowDeleted = resolve; + }); const deleter = prisma.$transaction(async (tx) => { await tx.environmentVariableValue.delete({ where: { id: parentValue.id } }); + markRowDeleted(); await deleterHoldsRow; }); + await rowDeleted; + const bulk = repository.deleteValues(project.id, { environmentId: branch.id, keys: ["SHARED"], onlyShadowingParent: true, }); - await new Promise((resolve) => setTimeout(resolve, 300)); - releaseDeleter(); + bulk.catch(() => undefined); + try { + await new Promise((resolve) => setTimeout(resolve, 300)); + } finally { + releaseDeleter(); + } await deleter; expect(await bulk).toEqual({ deleted: [], skipped: ["SHARED"] });