diff --git a/apps/webapp/app/routes/admin.feature-flags.tsx b/apps/webapp/app/routes/admin.feature-flags.tsx index 76ba62ff8e..652a6e2e2d 100644 --- a/apps/webapp/app/routes/admin.feature-flags.tsx +++ b/apps/webapp/app/routes/admin.feature-flags.tsx @@ -11,11 +11,12 @@ import { dashboardAction, dashboardLoader } from "~/services/routeBuilders/dashb 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"; @@ -116,39 +117,15 @@ export const action = dashboardAction( ); } - const validatedFlags = validationResult.data as Record; - const controlTypes = getAllFlagControlTypes(); - const catalogKeys = Object.keys(controlTypes); - - const keysToDelete: string[] = []; - const upsertOps: ReturnType[] = []; - - 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); - } - } - } + const catalogKeys = Object.keys(getAllFlagControlTypes()) as FeatureFlagKey[]; - await prisma.$transaction([ - ...upsertOps, - ...(keysToDelete.length > 0 - ? [prisma.featureFlag.deleteMany({ where: { key: { in: keysToDelete } } })] - : []), - ]); + await replaceGlobalFeatureFlags(prisma, { + requestedFlags: validationResult.data, + catalogKeys, + // On cloud, never delete locked flags (the UI omits them). Locally, full control. + isProtected: (key) => isManagedCloud && GLOBAL_LOCKED_FLAGS.includes(key), + graceMs: env.RUN_OPS_MINT_FLIP_GRACE_MS, + }); return json({ success: true }); } diff --git a/apps/webapp/app/v3/featureFlags.server.ts b/apps/webapp/app/v3/featureFlags.server.ts index a813bc13b5..548b7167a2 100644 --- a/apps/webapp/app/v3/featureFlags.server.ts +++ b/apps/webapp/app/v3/featureFlags.server.ts @@ -9,6 +9,14 @@ import { } from "~/v3/featureFlags"; import { stampMintKindFlip } from "~/v3/runOpsMigration/mintFlipGrace"; +// The runOpsMintKind trio is managed only through applyGlobalMintKindFlip (grace-stamped under an +// advisory lock), never a bare upsert or the replace sweep, so it stays out of both. +const GLOBAL_MINT_KEYS: FeatureFlagKey[] = [ + FEATURE_FLAG.runOpsMintKind, + FEATURE_FLAG.runOpsMintKindPrev, + FEATURE_FLAG.runOpsMintKindFlippedAt, +]; + export type FlagsOptions = { key: T; defaultValue?: z.infer<(typeof FeatureFlagCatalog)[T]>; @@ -191,3 +199,58 @@ export async function applyGlobalMintKindFlip( return makeSetMultipleFlags(tx)(stamped); }); } + +// Replace-semantics write for the global admin flags page: upsert submitted catalog flags, delete +// omitted ones (unless protected), and route any runOpsMintKind change through the graced flip path. +export async function replaceGlobalFeatureFlags( + client: PrismaClient, + params: { + requestedFlags: Partial>; + catalogKeys: FeatureFlagKey[]; + isProtected: (key: FeatureFlagKey) => boolean; + graceMs: number; + } +): Promise { + // Derived grace-stamp fields are computed server-side; never trust them from the body. + const { + runOpsMintKindPrev: _ignoredPrev, + runOpsMintKindFlippedAt: _ignoredFlippedAt, + ...requestedFlags + } = params.requestedFlags; + + if (requestedFlags.runOpsMintKind !== undefined) { + await applyGlobalMintKindFlip( + client, + { [FEATURE_FLAG.runOpsMintKind]: requestedFlags.runOpsMintKind }, + params.graceMs + ); + } + + const upsertOps: ReturnType[] = []; + const keysToDelete: string[] = []; + + for (const key of params.catalogKeys) { + if (GLOBAL_MINT_KEYS.includes(key)) { + continue; + } + if (key in requestedFlags) { + const value = (requestedFlags as Record)[key]; + upsertOps.push( + client.featureFlag.upsert({ + where: { key }, + create: { key, value: value as any }, + update: { value: value as any }, + }) + ); + } else if (!params.isProtected(key)) { + keysToDelete.push(key); + } + } + + await client.$transaction([ + ...upsertOps, + ...(keysToDelete.length > 0 + ? [client.featureFlag.deleteMany({ where: { key: { in: keysToDelete } } })] + : []), + ]); +} diff --git a/apps/webapp/test/globalFeatureFlagsReplace.test.ts b/apps/webapp/test/globalFeatureFlagsReplace.test.ts new file mode 100644 index 0000000000..b5dccbdeee --- /dev/null +++ b/apps/webapp/test/globalFeatureFlagsReplace.test.ts @@ -0,0 +1,183 @@ +// A runOpsMintKind change on the global admin flags page must go through the graced flip path +// (prev + flippedAt stamped), not a bare upsert. Real testcontainers Postgres, never mocked. +import type { PrismaClient } from "@trigger.dev/database"; +import { postgresTest } from "@internal/testcontainers"; +import { describe, expect, vi } from "vitest"; +import { FEATURE_FLAG, type FeatureFlagKey } from "~/v3/featureFlags"; +import { makeSetMultipleFlags, replaceGlobalFeatureFlags } from "~/v3/featureFlags.server"; + +vi.setConfig({ testTimeout: 60_000 }); + +const MINT_KEYS: FeatureFlagKey[] = [ + FEATURE_FLAG.runOpsMintKind, + FEATURE_FLAG.runOpsMintKindPrev, + FEATURE_FLAG.runOpsMintKindFlippedAt, +]; + +// Mint trio + two ordinary boolean flags, to exercise the sweep alongside the carved-out mint keys. +const CATALOG_KEYS: FeatureFlagKey[] = [ + ...MINT_KEYS, + FEATURE_FLAG.mollifierEnabled, + FEATURE_FLAG.hasAiAccess, +]; + +const NEVER_PROTECTED = () => false; + +async function readFlags( + prisma: PrismaClient, + keys: FeatureFlagKey[] +): Promise> { + const rows = await prisma.featureFlag.findMany({ + where: { key: { in: keys } }, + select: { key: true, value: true }, + }); + const m: Record = {}; + for (const row of rows) m[row.key] = row.value; + return m; +} + +describe("replaceGlobalFeatureFlags — graces runOpsMintKind, preserves replace semantics", () => { + postgresTest( + "a runOpsMintKind flip stamps prev + flippedAt (graced, not a bare upsert)", + async ({ prisma }) => { + await makeSetMultipleFlags(prisma)({ [FEATURE_FLAG.runOpsMintKind]: "cuid" }); + + await replaceGlobalFeatureFlags(prisma, { + requestedFlags: { [FEATURE_FLAG.runOpsMintKind]: "runOpsId" }, + catalogKeys: CATALOG_KEYS, + isProtected: NEVER_PROTECTED, + graceMs: 60_000, + }); + + const m = await readFlags(prisma, MINT_KEYS); + expect(m[FEATURE_FLAG.runOpsMintKind]).toBe("runOpsId"); + expect(m[FEATURE_FLAG.runOpsMintKindPrev]).toBe("cuid"); + expect(typeof m[FEATURE_FLAG.runOpsMintKindFlippedAt]).toBe("string"); + } + ); + + postgresTest( + "derived stamp fields supplied in the body are ignored (computed server-side)", + async ({ prisma }) => { + await makeSetMultipleFlags(prisma)({ [FEATURE_FLAG.runOpsMintKind]: "cuid" }); + + await replaceGlobalFeatureFlags(prisma, { + // Caller forges the stamp: prev="runOpsId", flippedAt=epoch. Both must be ignored. + requestedFlags: { + [FEATURE_FLAG.runOpsMintKind]: "runOpsId", + [FEATURE_FLAG.runOpsMintKindPrev]: "runOpsId", + [FEATURE_FLAG.runOpsMintKindFlippedAt]: "1970-01-01T00:00:00.000Z", + }, + catalogKeys: CATALOG_KEYS, + isProtected: NEVER_PROTECTED, + graceMs: 60_000, + }); + + const m = await readFlags(prisma, MINT_KEYS); + expect(m[FEATURE_FLAG.runOpsMintKindPrev]).toBe("cuid"); + expect(m[FEATURE_FLAG.runOpsMintKindFlippedAt]).not.toBe("1970-01-01T00:00:00.000Z"); + } + ); + + postgresTest( + "re-applying the same runOpsMintKind does not reset the grace clock", + async ({ prisma }) => { + await makeSetMultipleFlags(prisma)({ [FEATURE_FLAG.runOpsMintKind]: "cuid" }); + + await replaceGlobalFeatureFlags(prisma, { + requestedFlags: { [FEATURE_FLAG.runOpsMintKind]: "runOpsId" }, + catalogKeys: CATALOG_KEYS, + isProtected: NEVER_PROTECTED, + graceMs: 60_000, + }); + const first = await readFlags(prisma, MINT_KEYS); + + await replaceGlobalFeatureFlags(prisma, { + requestedFlags: { [FEATURE_FLAG.runOpsMintKind]: "runOpsId" }, + catalogKeys: CATALOG_KEYS, + isProtected: NEVER_PROTECTED, + graceMs: 60_000, + }); + const second = await readFlags(prisma, MINT_KEYS); + + expect(second[FEATURE_FLAG.runOpsMintKind]).toBe("runOpsId"); + expect(second[FEATURE_FLAG.runOpsMintKindPrev]).toBe(first[FEATURE_FLAG.runOpsMintKindPrev]); + expect(second[FEATURE_FLAG.runOpsMintKindFlippedAt]).toBe( + first[FEATURE_FLAG.runOpsMintKindFlippedAt] + ); + } + ); + + postgresTest( + "the mint trio survives a replace that omits runOpsMintKind (not swept)", + async ({ prisma }) => { + await replaceGlobalFeatureFlags(prisma, { + requestedFlags: { [FEATURE_FLAG.runOpsMintKind]: "runOpsId" }, + catalogKeys: CATALOG_KEYS, + isProtected: NEVER_PROTECTED, + graceMs: 60_000, + }); + const before = await readFlags(prisma, MINT_KEYS); + + // A later save that edits only an unrelated flag and omits the mint keys entirely. + await replaceGlobalFeatureFlags(prisma, { + requestedFlags: { [FEATURE_FLAG.mollifierEnabled]: true }, + catalogKeys: CATALOG_KEYS, + isProtected: NEVER_PROTECTED, + graceMs: 60_000, + }); + + const after = await readFlags(prisma, MINT_KEYS); + expect(after[FEATURE_FLAG.runOpsMintKind]).toBe("runOpsId"); + expect(after[FEATURE_FLAG.runOpsMintKindPrev]).toBe(before[FEATURE_FLAG.runOpsMintKindPrev]); + expect(after[FEATURE_FLAG.runOpsMintKindFlippedAt]).toBe( + before[FEATURE_FLAG.runOpsMintKindFlippedAt] + ); + const mollifier = await readFlags(prisma, [FEATURE_FLAG.mollifierEnabled]); + expect(mollifier[FEATURE_FLAG.mollifierEnabled]).toBe(true); + } + ); + + postgresTest( + "non-mint flags keep replace semantics: submitted upserts, omitted deletes", + async ({ prisma }) => { + await replaceGlobalFeatureFlags(prisma, { + requestedFlags: { [FEATURE_FLAG.mollifierEnabled]: true }, + catalogKeys: CATALOG_KEYS, + isProtected: NEVER_PROTECTED, + graceMs: 60_000, + }); + + await replaceGlobalFeatureFlags(prisma, { + requestedFlags: { [FEATURE_FLAG.hasAiAccess]: true }, + catalogKeys: CATALOG_KEYS, + isProtected: NEVER_PROTECTED, + graceMs: 60_000, + }); + + const m = await readFlags(prisma, [FEATURE_FLAG.mollifierEnabled, FEATURE_FLAG.hasAiAccess]); + expect(m[FEATURE_FLAG.hasAiAccess]).toBe(true); + expect(m[FEATURE_FLAG.mollifierEnabled]).toBeUndefined(); + } + ); + + postgresTest("protected omitted flags are not deleted", async ({ prisma }) => { + await replaceGlobalFeatureFlags(prisma, { + requestedFlags: { [FEATURE_FLAG.mollifierEnabled]: true }, + catalogKeys: CATALOG_KEYS, + isProtected: NEVER_PROTECTED, + graceMs: 60_000, + }); + + // A replace that omits mollifierEnabled but marks it protected must keep it. + await replaceGlobalFeatureFlags(prisma, { + requestedFlags: { [FEATURE_FLAG.hasAiAccess]: true }, + catalogKeys: CATALOG_KEYS, + isProtected: (key) => key === FEATURE_FLAG.mollifierEnabled, + graceMs: 60_000, + }); + + const m = await readFlags(prisma, [FEATURE_FLAG.mollifierEnabled]); + expect(m[FEATURE_FLAG.mollifierEnabled]).toBe(true); + }); +});