fix(webapp): remove stale branch env var copies on parent-level import

When an environment variable import to a preview branch also writes
parentVariables, the branch's own value rows for those keys are now
deleted if they were last written by the same source and the parent
now holds a value for the key. Rows written by anyone else, keys the
parent still has no value for, and keys the same request also sets on
the branch itself are left alone.

Prune failures are logged and never fail the import: the imported
values have already been written by then.

Author: matt-aitken

.server-changes/envvar-import-prune-branch-copies.md

@@ -0,0 +1,6 @@
+---
+area: webapp
+type: fix
+---
+
+Importing environment variables to a preview branch's parent now removes that same source's earlier copies of those variables on the branch, so the branch picks up the shared value instead of a stale override. Variables set on the branch by anyone else are left untouched.

apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.import.ts

@@ -11,6 +11,7 @@ import {
   authenticateEnvVarApiRequest,
   authorizeEnvVarApiRequest,
 } from "~/services/environmentVariableApiAccess.server";
+import { logger } from "~/services/logger.server";
 import { EnvironmentVariablesRepository } from "~/v3/environmentVariables/environmentVariablesRepository.server";
 
 const ParamsSchema = z.object({
@@ -82,6 +83,17 @@ export async function action({ params, request }: ActionFunctionArgs) {
       lastUpdatedBy: body.source,
     });
 
+    if (parentResult.success && body.source) {
+      const branchKeys = new Set(Object.keys(body.variables));
+      await pruneBranchCopies(repository, {
+        projectId: environment.project.id,
+        environmentId: environment.id,
+        parentEnvironmentId: environment.parentEnvironmentId,
+        keys: Object.keys(body.parentVariables).filter((key) => !branchKeys.has(key)),
+        source: body.source,
+      });
+    }
+
     let childFailure = !result.success ? result : undefined;
     let parentFailure = !parentResult.success ? parentResult : undefined;
 
@@ -105,6 +117,37 @@ export async function action({ params, request }: ActionFunctionArgs) {
   }
 }
 
+/**
+ * Keys the source now writes at the parent level lose the source's own earlier copy on the
+ * branch. A failure here never fails the import: the values are already written.
+ */
+async function pruneBranchCopies(
+  repository: EnvironmentVariablesRepository,
+  options: {
+    projectId: string;
+    environmentId: string;
+    parentEnvironmentId: string;
+    keys: string[];
+    source: NonNullable<ImportEnvironmentVariablesRequestBody["source"]>;
+  }
+) {
+  const { projectId, ...pruneOptions } = options;
+  try {
+    const pruned = await repository.pruneBranchValuesShadowingParent(projectId, pruneOptions);
+    if (pruned.failures.length > 0) {
+      logger.warn("Failed to prune some branch copies of parent environment variables", {
+        ...options,
+        failures: pruned.failures,
+      });
+    }
+  } catch (error) {
+    logger.warn("Failed to prune branch copies of parent environment variables", {
+      ...options,
+      error: error instanceof Error ? error.message : String(error),
+    });
+  }
+}
+
 async function parseImportBody(request: Request): Promise<ImportEnvironmentVariablesRequestBody> {
   const contentType = request.headers.get("content-type") ?? "application/json";
 

apps/webapp/app/v3/environmentVariables/environmentVariablesRepository.server.ts

@@ -27,8 +27,12 @@ import {
   type EditEnvironmentVariable,
   type EditEnvironmentVariableValue,
   type EnvironmentVariable,
+  type EnvironmentVariableUpdater,
+  EnvironmentVariableUpdaterSchema,
   type EnvironmentVariableWithSecret,
   type ProjectEnvironmentVariable,
+  type PruneBranchValuesResult,
+  type PruneBranchValuesShadowingParent,
   type Repository,
   type Result,
 } from "./repository";
@@ -56,6 +60,20 @@ 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 class EnvironmentVariablesRepository implements Repository {
   constructor(
     private prismaClient: PrismaClient = prisma,
@@ -952,6 +970,58 @@ export class EnvironmentVariablesRepository implements Repository {
       };
     }
   }
+
+  async pruneBranchValuesShadowingParent(
+    projectId: string,
+    options: PruneBranchValuesShadowingParent
+  ): Promise<PruneBranchValuesResult> {
+    const result: PruneBranchValuesResult = { prunedKeys: [], failures: [] };
+    if (options.keys.length === 0) {
+      return result;
+    }
+
+    const variables = await this.prismaClient.environmentVariable.findMany({
+      where: {
+        projectId,
+        key: { in: boundedIn(options.keys) },
+        project: { deletedAt: null },
+      },
+      select: {
+        id: true,
+        key: true,
+        values: {
+          where: { environmentId: { in: [options.environmentId, options.parentEnvironmentId] } },
+          select: { environmentId: true, lastUpdatedBy: true },
+        },
+      },
+    });
+
+    for (const variable of variables) {
+      const branchValue = variable.values.find((v) => v.environmentId === options.environmentId);
+      const parentValue = variable.values.find(
+        (v) => v.environmentId === options.parentEnvironmentId
+      );
+      if (
+        !branchValue ||
+        !parentValue ||
+        !isSameUpdater(branchValue.lastUpdatedBy, options.source)
+      ) {
+        continue;
+      }
+
+      const deleted = await this.deleteValue(projectId, {
+        id: variable.id,
+        environmentId: options.environmentId,
+      });
+      if (deleted.success) {
+        result.prunedKeys.push(variable.key);
+      } else {
+        result.failures.push({ key: variable.key, error: deleted.error });
+      }
+    }
+
+    return result;
+  }
 }
 
 // Derived from the slim AuthenticatedEnvironment so a full AE satisfies

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<typeof DeleteEnvironmentVariableValue>;
 
+export type PruneBranchValuesShadowingParent = {
+  environmentId: string;
+  parentEnvironmentId: string;
+  keys: string[];
+  source: EnvironmentVariableUpdater;
+};
+
+export type PruneBranchValuesResult = {
+  prunedKeys: string[];
+  failures: { key: string; error: string }[];
+};
+
 // Forms preserve explicit empty strings through their custom coercion.
 // A missing field is still invalid.
 export const EnvironmentVariableValue = z.string();
@@ -140,4 +152,14 @@ export interface Repository {
   ): Promise<EnvironmentVariable[]>;
   delete(projectId: string, options: DeleteEnvironmentVariable): Promise<Result>;
   deleteValue(projectId: string, options: DeleteEnvironmentVariableValue): Promise<Result>;
+  /**
+   * A source that writes a key at the parent level retracts its own earlier copy of that key on
+   * the branch. Deletes the branch's own value rows for `keys` that were last written by `source`
+   * and that now shadow a value the parent holds. Rows written by anyone else, and rows whose key
+   * the parent has no value for, are untouched.
+   */
+  pruneBranchValuesShadowingParent(
+    projectId: string,
+    options: PruneBranchValuesShadowingParent
+  ): Promise<PruneBranchValuesResult>;
 }