fix(webapp): stop saving global flags from unsetting the locked ones (#4751)

## Summary

On a self-hosted instance, saving anything on the global admin feature
flags page also deleted the two read-only flags,
`defaultWorkerInstanceGroupId` and `taskEventRepository`. Losing the
first one leaves deployed runs with no default worker group. Neither
deletion showed up in the confirm dialog, so the flags disappeared
silently.

## Root cause

The page submits only the flags its UI is managing, and strips the
read-only ones from the payload unless "Unlock read-only flags" is
ticked. The action treated every catalog key absent from that payload as
"the admin unset this", and protected the locked keys only when the
instance was managed cloud. Anywhere else, both locked rows fell
straight into the delete sweep.

The protection now keys off what the client says it was editing rather
than off the deployment:

```ts
const canDeleteLocked = params.unlockLockedFlags && !params.isManagedCloud;
...
} else if (canDeleteLocked || !GLOBAL_LOCKED_FLAGS.includes(key)) {
  keysToDelete.push(key);
}
```

Exactly one case changes: a locked flag, on a non managed-cloud
instance, with the flags not unlocked, is now kept instead of deleted.
Managed cloud behaviour is bit for bit identical, and ticking the unlock
box still gives a self-hosted instance full control. The write moves
into `replaceGlobalFeatureFlags` so it can be driven directly in tests
against a real Postgres.

apps/webapp/app/routes/admin.feature-flags.tsx

@@ -5,17 +5,18 @@ import { json } from "@remix-run/server-runtime";
 import { typedjson, useTypedLoaderData } from "remix-typedjson";
 import { z } from "zod";
 import { LockClosedIcon } from "@heroicons/react/20/solid";
-import { boundedIn, prisma } from "~/db.server";
+import { prisma } from "~/db.server";
 import { env } from "~/env.server";
 import { dashboardAction, dashboardLoader } from "~/services/routeBuilders/dashboardBuilder";
 import {
   FEATURE_FLAG,
   GLOBAL_LOCKED_FLAGS,
+  type FeatureFlagKey,
   type FlagControlType,
   getAllFlagControlTypes,
   validatePartialFeatureFlags,
 } from "~/v3/featureFlags";
-import { flags as getGlobalFlags } from "~/v3/featureFlags.server";
+import { flags as getGlobalFlags, replaceGlobalFeatureFlags } from "~/v3/featureFlags.server";
 import { featuresForRequest } from "~/features.server";
 import { Button } from "~/components/primitives/Buttons";
 import { Callout } from "~/components/primitives/Callout";
@@ -38,6 +39,12 @@ import {
   type WorkerGroup,
 } from "~/components/admin/FlagControls";
 
+/** What the page posts to the action. See the note on payloadSchema. */
+type SaveFlagsBody = {
+  flags: Record<string, unknown>;
+  unlockLockedFlags: boolean;
+};
+
 export const loader = dashboardLoader(
   { authorization: { requireSuper: true } },
   async ({ request }) => {
@@ -87,7 +94,16 @@ export const action = dashboardAction(
       return json({ error: "Invalid JSON body" }, { status: 400 });
     }
 
-    const payloadSchema = z.object({ flags: z.record(z.unknown()) });
+    // The zod schema leaves unlockLockedFlags optional so a tab opened before this shipped still
+    // saves, defaulting to the safe answer. SaveFlagsBody keeps it required for our own client, so
+    // dropping it from the page is a compile error rather than a silently disabled unlock.
+    const payloadSchema = z.object({
+      flags: z.record(z.unknown()),
+      // The page only submits the flags it is managing, so an omitted key is ambiguous for the
+      // locked flags: this says whether the admin unlocked them and is therefore authoritative
+      // over them too.
+      unlockLockedFlags: z.boolean().optional(),
+    });
     const parsed = payloadSchema.safeParse(body);
     if (!parsed.success) {
       return json({ error: "Invalid payload" }, { status: 400 });
@@ -116,39 +132,12 @@ export const action = dashboardAction(
       );
     }
 
-    const validatedFlags = validationResult.data as Record<string, unknown>;
-    const controlTypes = getAllFlagControlTypes();
-    const catalogKeys = Object.keys(controlTypes);
-
-    const keysToDelete: string[] = [];
-    const upsertOps: ReturnType<typeof prisma.featureFlag.upsert>[] = [];
-
-    for (const key of catalogKeys) {
-      if (key in validatedFlags) {
-        upsertOps.push(
-          prisma.featureFlag.upsert({
-            where: { key },
-            create: { key, value: validatedFlags[key] as any },
-            update: { value: validatedFlags[key] as any },
-          })
-        );
-      } else {
-        // On cloud, never delete locked flags (they're not in the payload
-        // because the UI doesn't include them). Locally, delete everything
-        // the user didn't include - full control.
-        const isProtected = isManagedCloud && GLOBAL_LOCKED_FLAGS.includes(key);
-        if (!isProtected) {
-          keysToDelete.push(key);
-        }
-      }
-    }
-
-    await prisma.$transaction([
-      ...upsertOps,
-      ...(keysToDelete.length > 0
-        ? [prisma.featureFlag.deleteMany({ where: { key: { in: boundedIn(keysToDelete) } } })]
-        : []),
-    ]);
+    await replaceGlobalFeatureFlags(prisma, {
+      requestedFlags: validationResult.data as Record<string, unknown>,
+      catalogKeys: Object.keys(getAllFlagControlTypes()) as FeatureFlagKey[],
+      isManagedCloud,
+      unlockLockedFlags: parsed.data.unlockLockedFlags ?? false,
+    });
 
     return json({ success: true });
   }
@@ -213,7 +202,8 @@ export default function AdminFeatureFlagsRoute() {
   };
 
   const handleSave = () => {
-    saveFetcher.submit(JSON.stringify({ flags: values }), {
+    const body: SaveFlagsBody = { flags: values, unlockLockedFlags: unlocked };
+    saveFetcher.submit(JSON.stringify(body), {
       method: "POST",
       encType: "application/json",
     });

apps/webapp/app/v3/featureFlags.server.ts

@@ -1,11 +1,12 @@
 import { type z } from "zod";
 import type { PrismaClient } from "@trigger.dev/database";
-import { prisma, type PrismaClientOrTransaction } from "~/db.server";
+import { boundedIn, prisma, type PrismaClientOrTransaction } from "~/db.server";
 import {
   FEATURE_FLAG,
   type FeatureFlagCatalogSchema,
   type FeatureFlagKey,
   FeatureFlagCatalog,
+  GLOBAL_LOCKED_FLAGS,
   validatePartialFeatureFlags,
 } from "~/v3/featureFlags";
 import { env } from "~/env.server";
@@ -223,6 +224,50 @@ export async function applyGlobalMintKindFlip(
   });
 }
 
+/**
+ * Replace-semantics write for the global admin flags page: catalog keys present in
+ * `requestedFlags` are upserted, catalog keys absent from it are deleted.
+ *
+ * A locked flag absent from the payload means the page never offered it for editing, not that
+ * the admin unset it, so it survives the sweep. Only a self-hosted page that says it unlocked
+ * them can delete one.
+ */
+export async function replaceGlobalFeatureFlags(
+  client: PrismaClient,
+  params: {
+    requestedFlags: Record<string, unknown>;
+    catalogKeys: FeatureFlagKey[];
+    isManagedCloud: boolean;
+    unlockLockedFlags: boolean;
+  }
+): Promise<void> {
+  const canDeleteLocked = params.unlockLockedFlags && !params.isManagedCloud;
+  const upsertOps: ReturnType<typeof client.featureFlag.upsert>[] = [];
+  const keysToDelete: string[] = [];
+
+  for (const key of params.catalogKeys) {
+    if (key in params.requestedFlags) {
+      const value = params.requestedFlags[key];
+      upsertOps.push(
+        client.featureFlag.upsert({
+          where: { key },
+          create: { key, value: value as any },
+          update: { value: value as any },
+        })
+      );
+    } else if (canDeleteLocked || !GLOBAL_LOCKED_FLAGS.includes(key)) {
+      keysToDelete.push(key);
+    }
+  }
+
+  await client.$transaction([
+    ...upsertOps,
+    ...(keysToDelete.length > 0
+      ? [client.featureFlag.deleteMany({ where: { key: { in: boundedIn(keysToDelete) } } })]
+      : []),
+  ]);
+}
+
 /** The global flag set, with the env-var defaults this app applies. */
 export async function globalFeatureFlags() {
   return flags({

apps/webapp/test/adminFeatureFlagsRouteAction.test.ts

@@ -0,0 +1,137 @@
+// The page posts only the flags its UI manages, so how the action reads an absent key is the whole
+// bug surface. These drive the real exported action against a real Postgres and assert on the rows
+// it leaves behind. The only module substituted is the auth wrapper, so the handler can be called
+// without a super-admin session; the database is the genuine article, injected into db.server.
+import { boundedIn } from "@trigger.dev/database";
+import type { PrismaClient } from "@trigger.dev/database";
+import { postgresTest } from "@internal/testcontainers";
+import { describe, expect, vi } from "vitest";
+import { FEATURE_FLAG } from "~/v3/featureFlags";
+
+vi.setConfig({ testTimeout: 60_000 });
+
+const db = vi.hoisted(() => ({ client: null as unknown as PrismaClient }));
+
+vi.mock("~/services/routeBuilders/dashboardBuilder", () => ({
+  dashboardAction: (_options: unknown, handler: unknown) => handler,
+  dashboardLoader: (_options: unknown, handler: unknown) => handler,
+}));
+
+vi.mock("~/db.server", () => ({
+  get prisma() {
+    return db.client;
+  },
+  boundedIn,
+}));
+
+import { action } from "~/routes/admin.feature-flags";
+
+const WORKER_GROUP_ID = "clwg000000000000000000000";
+
+async function post(host: string, body: unknown) {
+  const request = new Request(`https://${host}/admin/feature-flags`, {
+    method: "POST",
+    body: JSON.stringify(body),
+    headers: { "content-type": "application/json" },
+  });
+  return (await (action as any)({ request, params: {}, context: {} })) as Response;
+}
+
+async function readFlag(prisma: PrismaClient, key: string) {
+  const row = await prisma.featureFlag.findFirst({ where: { key }, select: { value: true } });
+  return row?.value;
+}
+
+async function seed(prisma: PrismaClient) {
+  db.client = prisma;
+  await prisma.featureFlag.createMany({
+    data: [
+      { id: "ff_locked", key: FEATURE_FLAG.defaultWorkerInstanceGroupId, value: WORKER_GROUP_ID },
+      { id: "ff_plain", key: FEATURE_FLAG.mollifierEnabled, value: true },
+    ],
+  });
+}
+
+describe("admin feature flags action", () => {
+  postgresTest("keeps the locked flag when the page did not unlock it", async ({ prisma }) => {
+    await seed(prisma);
+
+    const response = await post("localhost:3030", { flags: {} });
+
+    expect(response.status).toBe(200);
+    expect(await readFlag(prisma, FEATURE_FLAG.defaultWorkerInstanceGroupId)).toBe(WORKER_GROUP_ID);
+    expect(await readFlag(prisma, FEATURE_FLAG.mollifierEnabled)).toBeUndefined();
+  });
+
+  postgresTest("keeps the locked flag when the body omits the unlock field", async ({ prisma }) => {
+    await seed(prisma);
+
+    // A tab opened before the field existed posts the old shape.
+    const response = await post("localhost:3030", { flags: {}, unlockLockedFlags: undefined });
+
+    expect(response.status).toBe(200);
+    expect(await readFlag(prisma, FEATURE_FLAG.defaultWorkerInstanceGroupId)).toBe(WORKER_GROUP_ID);
+  });
+
+  postgresTest("deletes the locked flag when the page unlocked it", async ({ prisma }) => {
+    await seed(prisma);
+
+    await post("localhost:3030", { flags: {}, unlockLockedFlags: true });
+
+    expect(await readFlag(prisma, FEATURE_FLAG.defaultWorkerInstanceGroupId)).toBeUndefined();
+  });
+
+  postgresTest(
+    "keeps the locked flag on managed cloud despite the unlock claim",
+    async ({ prisma }) => {
+      await seed(prisma);
+
+      await post("cloud.trigger.dev", { flags: {}, unlockLockedFlags: true });
+
+      expect(await readFlag(prisma, FEATURE_FLAG.defaultWorkerInstanceGroupId)).toBe(
+        WORKER_GROUP_ID
+      );
+    }
+  );
+
+  postgresTest(
+    "rejects a locked flag submitted to managed cloud, writing nothing",
+    async ({ prisma }) => {
+      await seed(prisma);
+
+      const response = await post("cloud.trigger.dev", {
+        flags: { [FEATURE_FLAG.defaultWorkerInstanceGroupId]: "clwg999" },
+      });
+
+      expect(response.status).toBe(400);
+      expect(await readFlag(prisma, FEATURE_FLAG.defaultWorkerInstanceGroupId)).toBe(
+        WORKER_GROUP_ID
+      );
+      expect(await readFlag(prisma, FEATURE_FLAG.mollifierEnabled)).toBe(true);
+    }
+  );
+
+  postgresTest("rejects a value the catalog refuses, writing nothing", async ({ prisma }) => {
+    await seed(prisma);
+
+    const response = await post("localhost:3030", {
+      flags: { [FEATURE_FLAG.realtimeBackend]: "not-a-backend" },
+    });
+
+    expect(response.status).toBe(400);
+    expect(await readFlag(prisma, FEATURE_FLAG.mollifierEnabled)).toBe(true);
+  });
+
+  postgresTest("upserts what was submitted and sweeps what was not", async ({ prisma }) => {
+    await seed(prisma);
+
+    await post("localhost:3030", {
+      flags: { [FEATURE_FLAG.hasAiAccess]: true },
+      unlockLockedFlags: false,
+    });
+
+    expect(await readFlag(prisma, FEATURE_FLAG.hasAiAccess)).toBe(true);
+    expect(await readFlag(prisma, FEATURE_FLAG.mollifierEnabled)).toBeUndefined();
+    expect(await readFlag(prisma, FEATURE_FLAG.defaultWorkerInstanceGroupId)).toBe(WORKER_GROUP_ID);
+  });
+});

apps/webapp/test/globalFeatureFlagsLockedFlags.test.ts

@@ -0,0 +1,150 @@
+// With "Unlock read-only flags" off, the page strips GLOBAL_LOCKED_FLAGS from its payload, so an
+// omitted locked key means "the UI never offered it", not "the admin unset it".
+import type { PrismaClient } from "@trigger.dev/database";
+import { postgresTest } from "@internal/testcontainers";
+import { describe, expect, vi } from "vitest";
+import { FEATURE_FLAG, FeatureFlagCatalog, type FeatureFlagKey } from "~/v3/featureFlags";
+import { makeSetMultipleFlags, replaceGlobalFeatureFlags } from "~/v3/featureFlags.server";
+
+vi.setConfig({ testTimeout: 60_000 });
+
+const CATALOG_KEYS = Object.keys(FeatureFlagCatalog) as FeatureFlagKey[];
+const WORKER_GROUP_ID = "clwg000000000000000000000";
+
+async function readFlag(prisma: PrismaClient, key: FeatureFlagKey): Promise<unknown> {
+  const row = await prisma.featureFlag.findFirst({ where: { key }, select: { value: true } });
+  return row?.value;
+}
+
+describe("replaceGlobalFeatureFlags — locked flags the UI never submitted", () => {
+  postgresTest(
+    "keeps defaultWorkerInstanceGroupId when a locked flag is absent from the payload",
+    async ({ prisma }) => {
+      await makeSetMultipleFlags(prisma)({
+        [FEATURE_FLAG.defaultWorkerInstanceGroupId]: WORKER_GROUP_ID,
+        [FEATURE_FLAG.mollifierEnabled]: true,
+      });
+
+      // What the page posts when an admin unsets mollifierEnabled on a self-hosted instance.
+      await replaceGlobalFeatureFlags(prisma, {
+        requestedFlags: {},
+        catalogKeys: CATALOG_KEYS,
+        isManagedCloud: false,
+        unlockLockedFlags: false,
+      });
+
+      expect(await readFlag(prisma, FEATURE_FLAG.defaultWorkerInstanceGroupId)).toBe(
+        WORKER_GROUP_ID
+      );
+      expect(await readFlag(prisma, FEATURE_FLAG.mollifierEnabled)).toBeUndefined();
+    }
+  );
+
+  postgresTest("an unlocked self-hosted page can still unset a locked flag", async ({ prisma }) => {
+    await makeSetMultipleFlags(prisma)({
+      [FEATURE_FLAG.defaultWorkerInstanceGroupId]: WORKER_GROUP_ID,
+    });
+
+    await replaceGlobalFeatureFlags(prisma, {
+      requestedFlags: {},
+      catalogKeys: CATALOG_KEYS,
+      isManagedCloud: false,
+      unlockLockedFlags: true,
+    });
+
+    expect(await readFlag(prisma, FEATURE_FLAG.defaultWorkerInstanceGroupId)).toBeUndefined();
+  });
+
+  postgresTest(
+    "managed cloud keeps locked flags even when unlocking is claimed",
+    async ({ prisma }) => {
+      await makeSetMultipleFlags(prisma)({
+        [FEATURE_FLAG.defaultWorkerInstanceGroupId]: WORKER_GROUP_ID,
+      });
+
+      await replaceGlobalFeatureFlags(prisma, {
+        requestedFlags: {},
+        catalogKeys: CATALOG_KEYS,
+        isManagedCloud: true,
+        unlockLockedFlags: true,
+      });
+
+      expect(await readFlag(prisma, FEATURE_FLAG.defaultWorkerInstanceGroupId)).toBe(
+        WORKER_GROUP_ID
+      );
+    }
+  );
+
+  postgresTest("managed cloud still sweeps ordinary flags it was not sent", async ({ prisma }) => {
+    await makeSetMultipleFlags(prisma)({
+      [FEATURE_FLAG.defaultWorkerInstanceGroupId]: WORKER_GROUP_ID,
+      [FEATURE_FLAG.hasAiAccess]: true,
+    });
+
+    await replaceGlobalFeatureFlags(prisma, {
+      requestedFlags: { [FEATURE_FLAG.mollifierEnabled]: true },
+      catalogKeys: CATALOG_KEYS,
+      isManagedCloud: true,
+      unlockLockedFlags: false,
+    });
+
+    expect(await readFlag(prisma, FEATURE_FLAG.defaultWorkerInstanceGroupId)).toBe(WORKER_GROUP_ID);
+    expect(await readFlag(prisma, FEATURE_FLAG.hasAiAccess)).toBeUndefined();
+    expect(await readFlag(prisma, FEATURE_FLAG.mollifierEnabled)).toBe(true);
+  });
+
+  // The upsert and the sweep share one statement, which is only safe while no key is in both.
+  postgresTest("a submitted key is never also swept", async ({ prisma }) => {
+    await makeSetMultipleFlags(prisma)({
+      [FEATURE_FLAG.mollifierEnabled]: true,
+      [FEATURE_FLAG.hasAiAccess]: true,
+      [FEATURE_FLAG.defaultWorkerInstanceGroupId]: WORKER_GROUP_ID,
+    });
+
+    await replaceGlobalFeatureFlags(prisma, {
+      requestedFlags: {
+        [FEATURE_FLAG.mollifierEnabled]: false,
+        [FEATURE_FLAG.hasAiAccess]: true,
+      },
+      catalogKeys: CATALOG_KEYS,
+      isManagedCloud: false,
+      unlockLockedFlags: true,
+    });
+
+    // Both submitted keys survive with their new values rather than being swept by the same
+    // statement that wrote them.
+    expect(await readFlag(prisma, FEATURE_FLAG.mollifierEnabled)).toBe(false);
+    expect(await readFlag(prisma, FEATURE_FLAG.hasAiAccess)).toBe(true);
+    expect(await readFlag(prisma, FEATURE_FLAG.defaultWorkerInstanceGroupId)).toBeUndefined();
+  });
+
+  postgresTest("writes nothing when there is nothing to write", async ({ prisma }) => {
+    await makeSetMultipleFlags(prisma)({ [FEATURE_FLAG.mollifierEnabled]: true });
+
+    await replaceGlobalFeatureFlags(prisma, {
+      requestedFlags: {},
+      catalogKeys: [],
+      isManagedCloud: false,
+      unlockLockedFlags: false,
+    });
+
+    expect(await readFlag(prisma, FEATURE_FLAG.mollifierEnabled)).toBe(true);
+  });
+
+  postgresTest("submitted flags are upserted and omitted ones swept", async ({ prisma }) => {
+    await makeSetMultipleFlags(prisma)({
+      [FEATURE_FLAG.mollifierEnabled]: true,
+      [FEATURE_FLAG.hasAiAccess]: true,
+    });
+
+    await replaceGlobalFeatureFlags(prisma, {
+      requestedFlags: { [FEATURE_FLAG.mollifierEnabled]: false },
+      catalogKeys: CATALOG_KEYS,
+      isManagedCloud: false,
+      unlockLockedFlags: false,
+    });
+
+    expect(await readFlag(prisma, FEATURE_FLAG.mollifierEnabled)).toBe(false);
+    expect(await readFlag(prisma, FEATURE_FLAG.hasAiAccess)).toBeUndefined();
+  });
+});