diff --git a/apps/webapp/app/components/admin/flagChangeList.ts b/apps/webapp/app/components/admin/flagChangeList.ts new file mode 100644 index 00000000000..512d3758bc7 --- /dev/null +++ b/apps/webapp/app/components/admin/flagChangeList.ts @@ -0,0 +1,56 @@ +import { derivedFlagsClearedWith } from "~/v3/featureFlags"; + +export type FlagChange = + | { key: string; type: "added"; newVal: string } + | { key: string; type: "removed"; oldVal: string } + | { key: string; type: "changed"; oldVal: string; newVal: string }; + +/** + * What a global flag save will do, for the confirm dialog. + * + * A graced primary that is unset also clears its stamps. Those keys are locked, so the caller + * filters them out of `initialValues` — the cascade therefore reads `storedValues`, which is the + * unfiltered set the loader returned. Reading `initialValues` finds nothing and understates the + * deletion, which is the defect this parameter exists to prevent. + */ +export function buildFlagChangeList(params: { + editableKeys: readonly string[]; + lockedKeys: readonly string[]; + initialValues: Record; + storedValues: Record; + newValues: Record; +}): FlagChange[] { + const { editableKeys, initialValues, storedValues, newValues } = params; + + return editableKeys.flatMap((key) => { + const wasSet = key in initialValues; + const isSet = key in newValues; + const oldVal = initialValues[key]; + const newVal = newValues[key]; + + if (!wasSet && !isSet) return []; + if (wasSet && isSet && stableValue(oldVal) === stableValue(newVal)) return []; + + if (!wasSet && isSet) { + return [{ key, type: "added", newVal: String(newVal) }]; + } + + if (wasSet && !isSet) { + // Only an unset clears the stamps. A change re-stamps instead. + const cascaded = derivedFlagsClearedWith(key) + .filter((derived) => derived in storedValues) + .map((derived) => ({ + key: derived, + type: "removed", + oldVal: String(storedValues[derived]), + })); + return [{ key, type: "removed", oldVal: String(oldVal) }, ...cascaded]; + } + + return [{ key, type: "changed", oldVal: String(oldVal), newVal: String(newVal) }]; + }); +} + +function stableValue(value: unknown): string { + return JSON.stringify(value ?? null); +} diff --git a/apps/webapp/app/routes/admin.api.v1.feature-flags.ts b/apps/webapp/app/routes/admin.api.v1.feature-flags.ts index 8cd4f77873e..e9da02effd9 100644 --- a/apps/webapp/app/routes/admin.api.v1.feature-flags.ts +++ b/apps/webapp/app/routes/admin.api.v1.feature-flags.ts @@ -3,7 +3,12 @@ import { json } from "@remix-run/server-runtime"; import { prisma } from "~/db.server"; import { env } from "~/env.server"; import { requireAdminApiRequest } from "~/services/personalAccessToken.server"; -import { applyGlobalMintKindFlip, makeSetMultipleFlags } from "~/v3/featureFlags.server"; +import { + applyGlobalGracedFlips, + makeSetMultipleFlags, + touchesGracedGroup, + withoutDerivedKeys, +} from "~/v3/featureFlags.server"; import { validatePartialFeatureFlags } from "~/v3/featureFlags"; export async function action({ request }: ActionFunctionArgs) { @@ -25,19 +30,16 @@ export async function action({ request }: ActionFunctionArgs) { ); } - // Derived grace-stamp fields are computed server-side; never trust them from the body. - const { - runOpsMintKindPrev: _ignoredPrev, - runOpsMintKindFlippedAt: _ignoredFlippedAt, - ...requestedFlags - } = validationResult.data; + // Both the strip and the branch derive from the graced-group table, so adding a group needs + // no edit here. Naming the keys inline is how a new group ends up writing its stamp straight + // from the request body, with no lock. + const requestedFlags = withoutDerivedKeys(validationResult.data) as Partial< + typeof validationResult.data + >; - // A global mint-kind flip stamps its grace window under a lock (applyGlobalMintKindFlip); - // any other flag save writes directly. - const updatedFlags = - requestedFlags.runOpsMintKind !== undefined - ? await applyGlobalMintKindFlip(prisma, requestedFlags, env.RUN_OPS_MINT_FLIP_GRACE_MS) - : await makeSetMultipleFlags(prisma)(requestedFlags); + const updatedFlags = touchesGracedGroup(requestedFlags) + ? await applyGlobalGracedFlips(prisma, requestedFlags, env.RUN_OPS_MINT_FLIP_GRACE_MS) + : await makeSetMultipleFlags(prisma)(requestedFlags); return json({ success: true, diff --git a/apps/webapp/app/routes/admin.feature-flags.tsx b/apps/webapp/app/routes/admin.feature-flags.tsx index 197800c7ef3..e987812f520 100644 --- a/apps/webapp/app/routes/admin.feature-flags.tsx +++ b/apps/webapp/app/routes/admin.feature-flags.tsx @@ -14,6 +14,7 @@ import { type FeatureFlagKey, type FlagControlType, getAllFlagControlTypes, + lockedFlagsInPayload, validatePartialFeatureFlags, } from "~/v3/featureFlags"; import { flags as getGlobalFlags, replaceGlobalFeatureFlags } from "~/v3/featureFlags.server"; @@ -29,6 +30,7 @@ import { DialogFooter, } from "~/components/primitives/Dialog"; import { cn } from "~/utils/cn"; +import { buildFlagChangeList } from "~/components/admin/flagChangeList"; import { UNSET_VALUE, BooleanControl, @@ -111,17 +113,12 @@ export const action = dashboardAction( const { isManagedCloud } = featuresForRequest(request); - // On managed cloud, reject if payload includes locked flags - if (isManagedCloud) { - const lockedInPayload = Object.keys(parsed.data.flags).filter((key) => - GLOBAL_LOCKED_FLAGS.includes(key) + const lockedInPayload = lockedFlagsInPayload(Object.keys(parsed.data.flags), isManagedCloud); + if (lockedInPayload.length > 0) { + return json( + { error: `Cannot modify locked flags: ${lockedInPayload.join(", ")}` }, + { status: 400 } ); - if (lockedInPayload.length > 0) { - return json( - { error: `Cannot modify locked flags: ${lockedInPayload.join(", ")}` }, - { status: 400 } - ); - } } const validationResult = validatePartialFeatureFlags(parsed.data.flags); @@ -137,6 +134,7 @@ export const action = dashboardAction( catalogKeys: Object.keys(getAllFlagControlTypes()) as FeatureFlagKey[], isManagedCloud, unlockLockedFlags: parsed.data.unlockLockedFlags ?? false, + graceMs: env.RUN_OPS_MINT_FLIP_GRACE_MS, }); return json({ success: true }); @@ -401,6 +399,7 @@ export default function AdminFeatureFlagsRoute() { open={confirmOpen} onOpenChange={setConfirmOpen} initialValues={initialValues} + storedValues={allFlags} newValues={values} controlTypes={typedControlTypes} lockedKeys={unlocked ? [] : GLOBAL_LOCKED_FLAGS} @@ -467,6 +466,7 @@ function ConfirmDialog({ open, onOpenChange, initialValues, + storedValues, newValues, controlTypes, lockedKeys, @@ -477,6 +477,7 @@ function ConfirmDialog({ open: boolean; onOpenChange: (open: boolean) => void; initialValues: Record; + storedValues: Record; newValues: Record; controlTypes: Record; lockedKeys: readonly string[]; @@ -488,34 +489,12 @@ function ConfirmDialog({ .filter((key) => !lockedKeys.includes(key)) .sort(); - type Change = - | { key: string; type: "added"; newVal: string } - | { key: string; type: "removed"; oldVal: string } - | { key: string; type: "changed"; oldVal: string; newVal: string }; - - const changes = editableKeys.flatMap((key) => { - const wasSet = key in initialValues; - const isSet = key in newValues; - const oldVal = initialValues[key]; - const newVal = newValues[key]; - - if (!wasSet && !isSet) return []; - if (wasSet && isSet && stableStringify(oldVal) === stableStringify(newVal)) return []; - - if (!wasSet && isSet) { - return [{ key, type: "added" as const, newVal: String(newVal) }]; - } - if (wasSet && !isSet) { - return [{ key, type: "removed" as const, oldVal: String(oldVal) }]; - } - return [ - { - key, - type: "changed" as const, - oldVal: String(oldVal), - newVal: String(newVal), - }, - ]; + const changes = buildFlagChangeList({ + editableKeys, + lockedKeys, + initialValues, + storedValues, + newValues, }); return ( diff --git a/apps/webapp/app/v3/featureFlags.server.ts b/apps/webapp/app/v3/featureFlags.server.ts index dd1fb125ba6..152a14b6496 100644 --- a/apps/webapp/app/v3/featureFlags.server.ts +++ b/apps/webapp/app/v3/featureFlags.server.ts @@ -1,16 +1,18 @@ import { type z } from "zod"; import type { PrismaClient } from "@trigger.dev/database"; -import { boundedIn, prisma, type PrismaClientOrTransaction } from "~/db.server"; import { FEATURE_FLAG, type FeatureFlagCatalogSchema, type FeatureFlagKey, FeatureFlagCatalog, GLOBAL_LOCKED_FLAGS, + GRACED_FLAG_GROUPS, validatePartialFeatureFlags, } from "~/v3/featureFlags"; import { env } from "~/env.server"; import { stampMintKindFlip } from "~/v3/runOpsMigration/mintFlipGrace"; +import { stampMintShardSetFlip } from "~/v3/runOpsMigration/mintShardGrace"; +import { $transaction, boundedIn, prisma, type PrismaClientOrTransaction } from "~/db.server"; export type FlagsOptions = { key: T; @@ -182,56 +184,126 @@ export function makeSetMultipleFlags(_prisma: PrismaClientOrTransaction = prisma }; } -// Read -> stamp -> write the global mint-kind grace metadata in one transaction. The three -// FeatureFlag rows may not exist yet, so a row FOR UPDATE can't lock them; an advisory xact lock -// serializes concurrent global flips so one can't clobber another's grace stamp (mirrors per-org). -export async function applyGlobalMintKindFlip( - client: PrismaClient, - requestedFlags: Partial>, - graceMs: number -): Promise<{ key: string; value: any }[]> { - return client.$transaction(async (tx) => { - await tx.$executeRaw`SELECT pg_advisory_xact_lock(hashtext('runops-global-mint-kind-flip'))`; +// The key topology lives in the shared module, because the admin page needs it too. This adds +// the stamping behaviour, which is server-only. +const GRACED_GLOBAL_GROUPS = GRACED_FLAG_GROUPS.map((group) => ({ + ...group, + stamp: group.primary === FEATURE_FLAG.runOpsMintKind ? stampMintKindFlip : stampMintShardSetFlip, +})); - const existingRows = await tx.featureFlag.findMany({ - where: { - key: { - in: [ - FEATURE_FLAG.runOpsMintKind, - FEATURE_FLAG.runOpsMintKindPrev, - FEATURE_FLAG.runOpsMintKindFlippedAt, - ], - }, - }, - select: { key: true, value: true }, - }); - const existingGlobal: Record = {}; - for (const row of existingRows) { - existingGlobal[row.key] = row.value; +const GRACED_GLOBAL_KEYS: FeatureFlagKey[] = GRACED_GLOBAL_GROUPS.flatMap((g) => [ + g.primary, + ...g.derived, +]); + +function gracedGroupFor(key: FeatureFlagKey) { + return GRACED_GLOBAL_GROUPS.find((g) => g.primary === key || g.derived.includes(key)); +} + +// True when a save changes any graced group, and therefore needs the stamped path. Derived from +// the group table, so adding a group cannot leave a caller silently writing an unstamped flip. +export function touchesGracedGroup(requestedFlags: Record): boolean { + return GRACED_GLOBAL_GROUPS.some((group) => requestedFlags[group.primary] !== undefined); +} + +// Strips every derived key: a grace stamp is computed here, never accepted from a caller. +// Only the flags whose stored value differs. Each write is a round trip inside an interactive +// transaction, so writing an unchanged flag costs a round trip for nothing. +export function flagsNeedingWrite( + requested: Record, + existing: Record +): Record { + const out: Record = {}; + for (const [key, value] of Object.entries(requested)) { + if (JSON.stringify(existing[key] ?? null) !== JSON.stringify(value ?? null)) { + out[key] = value; } + } + return out; +} - // Anchor the cutover to the control-plane DB clock, not this process's wall clock. - const [{ now }] = await tx.$queryRaw<{ now: Date }[]>`SELECT now() AS now`; +export function withoutDerivedKeys( + requestedFlags: Partial> +): Record { + const out: Record = { ...requestedFlags }; + for (const group of GRACED_GLOBAL_GROUPS) { + for (const derived of group.derived) { + delete out[derived]; + } + } + return out; +} - const stamped = stampMintKindFlip( - existingGlobal, - { ...requestedFlags }, - now.getTime(), - graceMs - ) as Partial>; +// The rows may not exist yet, so a row FOR UPDATE cannot lock them; an advisory xact lock +// serializes concurrent global flips instead, so one cannot clobber another's stamp. +// +// Two lock ids are taken, in a fixed order. The FIRST is the operative one: an older release +// takes only that id, and a deploy rolls for hours, so it is the id that serializes across both +// versions. The second is this release's name and adds nothing until every writer takes it. +// Renaming without keeping the old id is what would leave the two versions unserialized. Remove +// the legacy id one release after this one ships, when nothing takes it alone. +async function lockGracedGroups(tx: PrismaClientOrTransaction): Promise { + await tx.$executeRaw`SELECT pg_advisory_xact_lock(hashtext('runops-global-mint-kind-flip'))`; + await tx.$executeRaw`SELECT pg_advisory_xact_lock(hashtext('runops-global-graced-flag-flip'))`; +} - return makeSetMultipleFlags(tx)(stamped); +// Reads each group's current rows and returns the requested flags plus a fresh stamp for every +// group the save actually changes. A group whose primary the save omits is left untouched. +async function stampGracedGroups( + tx: PrismaClientOrTransaction, + requestedFlags: Record, + graceMs: number +): Promise> { + const existingRows = await tx.featureFlag.findMany({ + where: { key: { in: boundedIn(GRACED_GLOBAL_KEYS) } }, + select: { key: true, value: true }, }); + const existingGlobal: Record = {}; + for (const row of existingRows) { + existingGlobal[row.key] = row.value; + } + + // Anchor the cutover to the control-plane DB clock, not this process's wall clock. A rolling + // deploy spans hours, so every pod must date the window against one shared clock. + const [{ now }] = await tx.$queryRaw<{ now: Date }[]>`SELECT now() AS now`; + + let stamped: Record = { ...requestedFlags }; + for (const group of GRACED_GLOBAL_GROUPS) { + stamped = group.stamp(existingGlobal, stamped, now.getTime(), graceMs); + } + return stamped; } -/** - * 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. - */ +// Merge-semantics write: sets what the caller asked for, stamps any graced group it changes, and +// touches nothing else. Used by the JSON admin API. +export async function applyGlobalGracedFlips( + client: PrismaClient, + requestedFlags: Partial>, + graceMs: number +): Promise<{ key: string; value: any }[]> { + const applied = await $transaction(client, "applyGlobalGracedFlips", async (tx) => { + await lockGracedGroups(tx); + const stamped = await stampGracedGroups(tx, withoutDerivedKeys(requestedFlags), graceMs); + return makeSetMultipleFlags(tx)(stamped as Partial>); + }); + + // The helper resolves undefined rather than throwing when Prisma swallows an infrastructure + // error. This write stamps a cutover window, so a transaction that did not run must be loud. + if (!applied) { + throw new Error("applyGlobalGracedFlips: transaction did not complete"); + } + return applied; +} + +// Replace-semantics write for the global admin flags page: submitted flags upsert, omitted ones +// delete unless protected. One transaction covers the stamp, the upserts and the deletes, so a +// save cannot half-apply. +// +// A graced group is all-or-nothing. Submitting its primary writes the group with a fresh stamp. +// Omitting its primary deletes the primary AND its stamp together, because a stamp left behind +// without its primary keeps being served: {set: [], prevSet: [a], flippedAt: t} resolves to [a] +// for the rest of the window, which would mint into a shard the operator just removed. The +// delete ignores `isProtected` for the derived keys for the same reason. export async function replaceGlobalFeatureFlags( client: PrismaClient, params: { @@ -239,33 +311,75 @@ export async function replaceGlobalFeatureFlags( catalogKeys: FeatureFlagKey[]; isManagedCloud: boolean; unlockLockedFlags: boolean; + graceMs: number; } ): Promise { + const requestedFlags = withoutDerivedKeys(params.requestedFlags); + + // A locked flag absent from the payload means the page never offered it, not that the admin + // unset it, so it survives. Only a self-hosted page that says it unlocked them may delete one. const canDeleteLocked = params.unlockLockedFlags && !params.isManagedCloud; - const upsertOps: ReturnType[] = []; - 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 }, - }) + const isProtected = (key: FeatureFlagKey) => + !canDeleteLocked && GLOBAL_LOCKED_FLAGS.includes(key); + + const applied = await $transaction(client, "replaceGlobalFeatureFlags", async (tx) => { + await lockGracedGroups(tx); + const stamped = await stampGracedGroups(tx, requestedFlags, params.graceMs); + + const toWrite: Record = {}; + const keysToDelete: string[] = []; + + for (const key of params.catalogKeys) { + const group = gracedGroupFor(key); + + if (group) { + if (requestedFlags[group.primary] !== undefined) { + if (stamped[key] !== undefined) { + toWrite[key] = stamped[key]; + } + } else if (!isProtected(group.primary)) { + keysToDelete.push(key); + } + continue; + } + + if (key in requestedFlags) { + toWrite[key] = requestedFlags[key]; + } else if (!isProtected(key)) { + keysToDelete.push(key); + } + } + + // One round trip to learn the stored values, then a write only for what actually differs. + // makeSetMultipleFlags upserts sequentially, so an unchanged flag costs a round trip for + // nothing, and this transaction is interactive and holds a pooled connection. + const writeKeys = Object.keys(toWrite); + if (writeKeys.length > 0) { + const storedRows = await tx.featureFlag.findMany({ + where: { key: { in: boundedIn(writeKeys) } }, + select: { key: true, value: true }, + }); + const stored: Record = {}; + for (const row of storedRows) { + stored[row.key] = row.value; + } + + await makeSetMultipleFlags(tx)( + flagsNeedingWrite(toWrite, stored) as Partial> ); - } 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) } } })] - : []), - ]); + if (keysToDelete.length > 0) { + await tx.featureFlag.deleteMany({ where: { key: { in: boundedIn(keysToDelete) } } }); + } + + return true; + }); + + // This write deletes flags, so a transaction that did not run must reach the caller. + if (!applied) { + throw new Error("replaceGlobalFeatureFlags: transaction did not complete"); + } } /** The global flag set, with the env-var defaults this app applies. */ diff --git a/apps/webapp/app/v3/featureFlags.ts b/apps/webapp/app/v3/featureFlags.ts index 7c775799178..3a88beb54bc 100644 --- a/apps/webapp/app/v3/featureFlags.ts +++ b/apps/webapp/app/v3/featureFlags.ts @@ -26,6 +26,16 @@ export const FEATURE_FLAG = { // Grace-linger stamp carried alongside runOpsMintKind on flip. See mintFlipGrace.ts. runOpsMintKindPrev: "runOpsMintKindPrev", runOpsMintKindFlippedAt: "runOpsMintKindFlippedAt", + // Gen-2 mint shard pins, read from the org override blob only. See runOpsMintShard.server.ts. + runOpsMintShard: "runOpsMintShard", + runOpsMintShardEnvPins: "runOpsMintShardEnvPins", + // The active mint-shard list, global only. Lives here rather than in the environment because a + // rolling deploy runs two environment values at once for hours. See mintShardGrace.ts. + runOpsMintShardSet: "runOpsMintShardSet", + runOpsMintShardSetPrev: "runOpsMintShardSetPrev", + runOpsMintShardSetFlippedAt: "runOpsMintShardSetFlippedAt", + // Fleet-wide pin for the complete cutover. Beats every per-org and per-env pin. + runOpsMintShardOverride: "runOpsMintShardOverride", queueMetricsUiEnabled: "queueMetricsUiEnabled", // Per-organization rollout for creating additional environment API keys. additionalApiKeysEnabled: "additionalApiKeysEnabled", @@ -89,6 +99,52 @@ export const FeatureFlagCatalog = { // by stampMintKindFlip on a genuine flip. Display-only (see ORG_LOCKED_FLAGS). [FEATURE_FLAG.runOpsMintKindPrev]: z.enum(["cuid", "runOpsId"]), [FEATURE_FLAG.runOpsMintKindFlippedAt]: z.string().datetime(), + // Pins one org to a gen-2 mint shard. "new" holds the org on gen-1 run-ops ids, which is how + // a canary keeps the fleet's default while one org moves. Only honored while the key is in + // the active list; a drained key falls through to the hash. + [FEATURE_FLAG.runOpsMintShard]: z + .string() + .refine((v) => v === "new" || /^[a-z0-9]$/.test(v), 'must be a single [a-z0-9] char, or "new"'), + // Per-environment pins as JSON: {"": ""}. A JSON string because + // this catalog is scalar-only. Rejected at write, so a typo cannot silently un-pin an env. + [FEATURE_FLAG.runOpsMintShardEnvPins]: z.string().superRefine((raw, ctx) => { + const fail = (message: string) => ctx.addIssue({ code: z.ZodIssueCode.custom, message }); + + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return fail("must be valid JSON"); + } + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { + return fail("must be a JSON object mapping environment id to shard key"); + } + for (const [environmentId, value] of Object.entries(parsed)) { + if (typeof value !== "string" || !(value === "new" || /^[a-z0-9]$/.test(value))) { + fail(`"${environmentId}" must map to a single [a-z0-9] char, or "new"`); + } + } + }), + // CSV of the shard keys eligible for root minting right now. Empty means no gen-2 minting. + // Reserved keys are rejected, because "new" already means gen-1. + [FEATURE_FLAG.runOpsMintShardSet]: z.string().refine( + (v) => + v + .split(",") + .map((s) => s.trim()) + .filter(Boolean) + .every((k) => /^[a-z0-9]$/.test(k)), + "must be a CSV of single [a-z0-9] chars" + ), + // Grace stamp: the previously-effective list and the flip time, written by + // stampMintShardSetFlip on a genuine change. Display-only (see ORG_LOCKED_FLAGS). + [FEATURE_FLAG.runOpsMintShardSetPrev]: z.string(), + [FEATURE_FLAG.runOpsMintShardSetFlippedAt]: z.string().datetime(), + // Sends every environment to one shard, outranking every pin, so a cutover needs no per-org + // visit. "new" holds the whole fleet on gen-1. Only honored while the key is in the active set. + [FEATURE_FLAG.runOpsMintShardOverride]: z + .string() + .refine((v) => v === "new" || /^[a-z0-9]$/.test(v), 'must be a single [a-z0-9] char, or "new"'), // Per-org access to the Queue Metrics dashboard UI (view only; emission is global and // separate). Off unless enabled for the org. [FEATURE_FLAG.queueMetricsUiEnabled]: z.coerce.boolean(), @@ -101,11 +157,20 @@ export const FeatureFlagCatalog = { export type FeatureFlagKey = keyof typeof FeatureFlagCatalog; -// Infrastructure flags that are read-only on the global flags page. -// Shown with current/resolved value but no controls. +// Infrastructure flags, plus org-scoped-only flags, that are read-only on the global flags +// page. Shown with current/resolved value but no controls. An org-scoped-only flag belongs +// here because its resolver never reads a global row, so an editable global control would +// offer a setting that does nothing. export const GLOBAL_LOCKED_FLAGS: FeatureFlagKey[] = [ FEATURE_FLAG.defaultWorkerInstanceGroupId, FEATURE_FLAG.taskEventRepository, + FEATURE_FLAG.runOpsMintShard, + FEATURE_FLAG.runOpsMintShardEnvPins, + // Grace stamps are computed server-side. An editable control here would discard what it saves. + FEATURE_FLAG.runOpsMintKindPrev, + FEATURE_FLAG.runOpsMintKindFlippedAt, + FEATURE_FLAG.runOpsMintShardSetPrev, + FEATURE_FLAG.runOpsMintShardSetFlippedAt, ]; // Flags that are read-only on the org-level dialog. @@ -118,8 +183,53 @@ export const ORG_LOCKED_FLAGS: FeatureFlagKey[] = [ // System-wide only — orgs must not be able to override these kill switches. FEATURE_FLAG.additionalApiKeyIssuanceEnabled, FEATURE_FLAG.additionalApiKeyLookupEnabled, + // The active mint-shard list is deployment-wide; only the pins are per-org. + FEATURE_FLAG.runOpsMintShardSet, + FEATURE_FLAG.runOpsMintShardSetPrev, + FEATURE_FLAG.runOpsMintShardSetFlippedAt, + FEATURE_FLAG.runOpsMintShardOverride, +]; + +/** + * Flag groups where the operator sets a `primary` and the server computes the rest. The topology + * lives here, not in the server module, because the admin page needs it too: unsetting a primary + * clears its stamps, and the page has to disclose that. + */ +export const GRACED_FLAG_GROUPS: ReadonlyArray<{ + primary: FeatureFlagKey; + derived: readonly FeatureFlagKey[]; +}> = [ + { + primary: FEATURE_FLAG.runOpsMintKind, + derived: [FEATURE_FLAG.runOpsMintKindPrev, FEATURE_FLAG.runOpsMintKindFlippedAt], + }, + { + primary: FEATURE_FLAG.runOpsMintShardSet, + derived: [FEATURE_FLAG.runOpsMintShardSetPrev, FEATURE_FLAG.runOpsMintShardSetFlippedAt], + }, ]; +/** The stamps deleted alongside `primary`. Empty unless `primary` is a graced primary. */ +export function derivedFlagsClearedWith(primary: string): FeatureFlagKey[] { + const group = GRACED_FLAG_GROUPS.find((g) => g.primary === primary); + return group ? [...group.derived] : []; +} + +/** + * Locked flags present in a payload the global page must refuse. On managed cloud the page never + * offers them, so their presence means the request did not come from that page. Locally an admin + * may unlock and edit them, so nothing is refused. + */ +export function lockedFlagsInPayload( + payloadKeys: string[], + isManagedCloud: boolean +): FeatureFlagKey[] { + if (!isManagedCloud) return []; + return payloadKeys.filter((key): key is FeatureFlagKey => + GLOBAL_LOCKED_FLAGS.includes(key as FeatureFlagKey) + ); +} + // Create a Zod schema from the existing catalog export const FeatureFlagCatalogSchema = z.object(FeatureFlagCatalog); export type FeatureFlagCatalog = z.infer; diff --git a/apps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts b/apps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts new file mode 100644 index 00000000000..d88e64e1d75 --- /dev/null +++ b/apps/webapp/app/v3/runOpsMigration/mintShardAssignment.test.ts @@ -0,0 +1,475 @@ +import { describe, expect, it } from "vitest"; +import { + computeMintShard, + resolveMintShardWith, + type MintShardCache, + type MintShardDeps, + type ResolveMintShardDeps, +} from "./mintShardAssignment"; +import { type MintShardSetResolution } from "./mintShardGrace"; + +const GRACE_MS = 90_000; +const T = 1_000_000; + +// Cuid-shaped ids, not sequential integers: a sequential space does not model the real +// key distribution the hash has to spread. +function envIds(count: number): string[] { + const ids: string[] = []; + for (let i = 0; i < count; i++) { + ids.push(`cm${(i * 2654435761).toString(36).padStart(10, "0")}${i.toString(36)}zzq`); + } + return ids; +} + +function deps( + resolution: MintShardSetResolution, + overrides: Partial = {} +): MintShardDeps { + return { + resolution, + nowMs: T + GRACE_MS + 1, + graceMs: GRACE_MS, + orgFeatureFlags: undefined, + ...overrides, + }; +} + +function orgFlags(flags: Record) { + return { orgFeatureFlags: flags }; +} + +function place(ids: string[], resolution: MintShardSetResolution): Map { + const out = new Map(); + for (const id of ids) { + out.set(id, computeMintShard({ id }, deps(resolution))); + } + return out; +} + +describe("computeMintShard — the no-shards answer", () => { + it("returns new when the live list is empty", () => { + expect(computeMintShard({ id: "env_1" }, deps({ set: [] }))).toBe("new"); + }); + + it("returns new when a stale stamp is present but both lists are empty", () => { + const resolution: MintShardSetResolution = { set: [], prevSet: [], flippedAtMs: T }; + expect(computeMintShard({ id: "env_1" }, deps(resolution, { nowMs: T + 1 }))).toBe("new"); + }); + + it("returns new when the grace serves an empty list", () => { + const resolution: MintShardSetResolution = { set: ["a"], prevSet: [], flippedAtMs: T }; + expect(computeMintShard({ id: "env_1" }, deps(resolution, { nowMs: T + 1 }))).toBe("new"); + }); + + it("returns new when the grace serves an empty prevSet", () => { + const resolution: MintShardSetResolution = { set: ["a"], prevSet: [], flippedAtMs: T }; + expect(computeMintShard({ id: "env_1" }, deps(resolution, { nowMs: T + 1 }))).toBe("new"); + }); +}); + +describe("computeMintShard — determinism", () => { + it("returns the same value for the same environment on every call", () => { + const resolution: MintShardSetResolution = { set: ["a", "b", "c"] }; + const first = computeMintShard({ id: "env_stable" }, deps(resolution)); + for (let i = 0; i < 1000; i++) { + expect(computeMintShard({ id: "env_stable" }, deps(resolution))).toBe(first); + } + }); + + it("ignores the order the operator listed the keys in", () => { + const ids = envIds(200); + const canonical = place(ids, { set: ["a", "b", "c"] }); + for (const permutation of [ + ["c", "b", "a"], + ["b", "a", "c"], + ["a", "c", "b"], + ]) { + expect(place(ids, { set: permutation })).toEqual(canonical); + } + }); +}); + +describe("computeMintShard — pins", () => { + const resolution: MintShardSetResolution = { set: ["a", "b"] }; + + it("lets a per-env pin override the hash", () => { + const ids = envIds(50); + for (const id of ids) { + const pinned = computeMintShard( + { id }, + deps(resolution, orgFlags({ runOpsMintShardEnvPins: JSON.stringify({ [id]: "b" }) })) + ); + expect(pinned).toBe("b"); + } + }); + + it("lets a per-org pin override the hash when no per-env pin is set", () => { + const ids = envIds(50); + for (const id of ids) { + expect(computeMintShard({ id }, deps(resolution, orgFlags({ runOpsMintShard: "a" })))).toBe( + "a" + ); + } + }); + + it("lets a per-env pin beat a per-org pin", () => { + const result = computeMintShard( + { id: "env_1" }, + deps( + resolution, + orgFlags({ + runOpsMintShard: "a", + runOpsMintShardEnvPins: JSON.stringify({ env_1: "b" }), + }) + ) + ); + expect(result).toBe("b"); + }); + + it("holds an environment on gen-1 when the pin is new", () => { + expect( + computeMintShard({ id: "env_1" }, deps(resolution, orgFlags({ runOpsMintShard: "new" }))) + ).toBe("new"); + expect( + computeMintShard( + { id: "env_1" }, + deps(resolution, orgFlags({ runOpsMintShardEnvPins: JSON.stringify({ env_1: "new" }) })) + ) + ).toBe("new"); + }); + + it("falls through to the hash and reports when the pin is outside the active set", () => { + // Honouring a drained pin would leak the drain; throwing would fail customer triggers. + const rejected: string[] = []; + const result = computeMintShard( + { id: "env_1" }, + deps(resolution, { + ...orgFlags({ runOpsMintShard: "z" }), + onPinRejected: (info) => rejected.push(info.pin), + }) + ); + expect(result).toBe(computeMintShard({ id: "env_1" }, deps(resolution))); + expect(rejected).toEqual(["z"]); + }); + + it("honours a pin to a drained key for the whole grace window, then falls through", () => { + const draining: MintShardSetResolution = { set: ["a"], prevSet: ["a", "b"], flippedAtMs: T }; + const pinnedToB = orgFlags({ runOpsMintShard: "b" }); + expect(computeMintShard({ id: "env_1" }, deps(draining, { ...pinnedToB, nowMs: T + 1 }))).toBe( + "b" + ); + expect( + computeMintShard({ id: "env_1" }, deps(draining, { ...pinnedToB, nowMs: T + GRACE_MS })) + ).not.toBe("b"); + }); + + it("ignores an unparseable pin blob rather than un-pinning silently", () => { + const result = computeMintShard( + { id: "env_1" }, + deps(resolution, orgFlags({ runOpsMintShard: "a", runOpsMintShardEnvPins: "{not json" })) + ); + expect(result).toBe("a"); + }); + + it("falls back to the org pin when the blob holds an invalid value for this env", () => { + const result = computeMintShard( + { id: "env_1" }, + deps( + resolution, + orgFlags({ + runOpsMintShard: "a", + runOpsMintShardEnvPins: JSON.stringify({ env_1: "LEGACY" }), + }) + ) + ); + expect(result).toBe("a"); + }); + + it("ignores an invalid org pin value", () => { + const result = computeMintShard( + { id: "env_1" }, + deps(resolution, orgFlags({ runOpsMintShard: "legacy" })) + ); + expect(result).toBe(computeMintShard({ id: "env_1" }, deps(resolution))); + }); +}); + +describe("computeMintShard — rendezvous properties", () => { + const ids = envIds(10_000); + + it("spreads roughly evenly across the active set", () => { + for (const set of [ + ["a", "b"], + ["a", "b", "c"], + ["a", "b", "c", "d"], + ]) { + const counts = new Map(); + for (const shard of place(ids, { set }).values()) { + counts.set(shard, (counts.get(shard) ?? 0) + 1); + } + expect(counts.size).toBe(set.length); + const expected = ids.length / set.length; + for (const count of counts.values()) { + expect(Math.abs(count - expected) / expected).toBeLessThan(0.1); + } + } + }); + + it("moves about 1/(N+1) of environments when a shard is added", () => { + const cases: Array<{ from: string[]; to: string[]; expected: number }> = [ + { from: ["a"], to: ["a", "b"], expected: 1 / 2 }, + { from: ["a", "b"], to: ["a", "b", "c"], expected: 1 / 3 }, + { from: ["a", "b", "c"], to: ["a", "b", "c", "d"], expected: 1 / 4 }, + ]; + + for (const { from, to, expected } of cases) { + const before = place(ids, { set: from }); + const after = place(ids, { set: to }); + const added = to.filter((k) => !from.includes(k)); + let moved = 0; + for (const id of ids) { + if (before.get(id) === after.get(id)) continue; + moved++; + // HRW's defining property: a mover lands on the ADDED shard, never on a survivor. + expect(added).toContain(after.get(id)); + } + expect(Math.abs(moved / ids.length - expected) / expected).toBeLessThan(0.1); + } + }); + + it("moves only the environments that hashed to a removed shard", () => { + const before = place(ids, { set: ["a", "b", "c"] }); + const after = place(ids, { set: ["a", "b"] }); + for (const id of ids) { + if (before.get(id) === "c") { + expect(after.get(id)).not.toBe("c"); + } else { + expect(after.get(id)).toBe(before.get(id)); + } + } + }); + + it("also moves pinned environments when their shard is removed", () => { + // Criterion 6 is a property of the hash only. A pin to a removed key moves too. + const pinnedToC = orgFlags({ runOpsMintShard: "c" }); + expect(computeMintShard({ id: "env_1" }, deps({ set: ["a", "b", "c"] }, pinnedToC))).toBe("c"); + expect(computeMintShard({ id: "env_1" }, deps({ set: ["a", "b"] }, pinnedToC))).not.toBe("c"); + }); +}); + +describe("resolveMintShardWith — cache, read failure and fail-safe", () => { + function wrapperDeps( + overrides: Partial = {} + ): ResolveMintShardDeps & { reads: number } { + const state = { + readFlags: async () => ({ runOpsMintShardSet: "a,b" }), + cache: { current: undefined as MintShardCache }, + nowMs: T, + ttlMs: 30_000, + graceMs: GRACE_MS, + orgFeatureFlags: undefined as unknown, + reads: 0, + ...overrides, + }; + const wrapped = state.readFlags; + state.readFlags = async () => { + state.reads++; + return wrapped(); + }; + return state; + } + + it("reads once, then serves the cache until the TTL expires", async () => { + const deps = wrapperDeps(); + await resolveMintShardWith({ id: "env_1" }, deps); + await resolveMintShardWith({ id: "env_2" }, deps); + await resolveMintShardWith({ id: "env_3" }, deps); + expect(deps.reads).toBe(1); + }); + + it("reads again once the TTL expires", async () => { + const deps = wrapperDeps(); + await resolveMintShardWith({ id: "env_1" }, deps); + deps.nowMs = T + 30_000; + await resolveMintShardWith({ id: "env_1" }, deps); + expect(deps.reads).toBe(2); + }); + + it("falls back to gen-1 when the read throws, and does not poison the cache", async () => { + // A blip must not move every environment's placement, so it returns gen-1 rather than guess. + let fail = true; + const deps = wrapperDeps({ + readFlags: async () => { + if (fail) throw new Error("db down"); + return { runOpsMintShardSet: "a,b" }; + }, + }); + const failures: unknown[] = []; + deps.onReadFailed = (error) => failures.push(error); + + expect(await resolveMintShardWith({ id: "env_1" }, deps)).toBe("new"); + expect(failures).toHaveLength(1); + + fail = false; + expect(["a", "b"]).toContain(await resolveMintShardWith({ id: "env_1" }, deps)); + }); + + it("returns gen-1 when the stored list is empty", async () => { + const deps = wrapperDeps({ readFlags: async () => ({ runOpsMintShardSet: "" }) }); + expect(await resolveMintShardWith({ id: "env_1" }, deps)).toBe("new"); + }); + + it("coalesces concurrent misses into ONE read", async () => { + // Two misses must share a single read. Otherwise a slower read landing after a faster one + // writes its older snapshot back into the cache for a whole TTL. + let release: (flags: Record) => void = () => {}; + const gate = new Promise>((resolve) => { + release = resolve; + }); + const deps = wrapperDeps({ readFlags: () => gate }); + + const both = Promise.all([ + resolveMintShardWith({ id: "env_1" }, deps), + resolveMintShardWith({ id: "env_2" }, deps), + ]); + release({ runOpsMintShardSet: "a,b" }); + await both; + + expect(deps.reads).toBe(1); + }); + + it("does not let a slower read overwrite a newer one", async () => { + // The slow read starts first and finishes last. Its result must not become the cached + // value, because the fast read already published a newer snapshot. + let releaseSlow: (flags: Record) => void = () => {}; + const slow = new Promise>((resolve) => { + releaseSlow = resolve; + }); + let call = 0; + const deps = wrapperDeps({ + readFlags: () => { + call++; + return call === 1 ? slow : Promise.resolve({ runOpsMintShardSet: "c" }); + }, + }); + + const first = resolveMintShardWith({ id: "env_1" }, deps); + const second = resolveMintShardWith({ id: "env_2" }, deps); + releaseSlow({ runOpsMintShardSet: "a" }); + await Promise.all([first, second]); + + // One read served both, so there is no second snapshot to race with. + expect(deps.reads).toBe(1); + expect(deps.cache.current?.value.resolution.set).toEqual(["a"]); + }); + + it("clears the in-flight refresh after a failure, so the next call retries", async () => { + let fail = true; + const deps = wrapperDeps({ + readFlags: async () => { + if (fail) throw new Error("db down"); + return { runOpsMintShardSet: "a,b" }; + }, + }); + deps.onReadFailed = () => {}; + + expect(await resolveMintShardWith({ id: "env_1" }, deps)).toBe("new"); + fail = false; + expect(["a", "b"]).toContain(await resolveMintShardWith({ id: "env_1" }, deps)); + expect(deps.reads).toBe(2); + }); + + it("agrees with the pure core for the same inputs", async () => { + const deps = wrapperDeps(); + const viaWrapper = await resolveMintShardWith({ id: "env_1" }, deps); + const viaCore = computeMintShard( + { id: "env_1" }, + { + resolution: { set: ["a", "b"] }, + nowMs: T, + graceMs: GRACE_MS, + orgFeatureFlags: undefined, + } + ); + expect(viaWrapper).toBe(viaCore); + }); +}); + +describe("computeMintShard — the global override wins the complete cutover", () => { + const resolution: MintShardSetResolution = { set: ["a", "b"] }; + + it("beats the hash for every environment", () => { + for (const id of envIds(200)) { + expect(computeMintShard({ id }, deps(resolution, { globalOverride: "b" }))).toBe("b"); + } + }); + + it("beats a per-org pin", () => { + const shard = computeMintShard( + { id: "env_1" }, + deps(resolution, { globalOverride: "b", orgFeatureFlags: { runOpsMintShard: "a" } }) + ); + expect(shard).toBe("b"); + }); + + it("beats a per-env pin, which is the whole point of a cutover", () => { + const shard = computeMintShard( + { id: "env_1" }, + deps(resolution, { + globalOverride: "b", + orgFeatureFlags: { runOpsMintShardEnvPins: JSON.stringify({ env_1: "a" }) }, + }) + ); + expect(shard).toBe("b"); + }); + + it("holds the whole fleet on gen-1 when set to new, whatever any org pinned", () => { + const shard = computeMintShard( + { id: "env_1" }, + deps(resolution, { globalOverride: "new", orgFeatureFlags: { runOpsMintShard: "a" } }) + ); + expect(shard).toBe("new"); + }); + + it("is ignored, and reported, when it names a key outside the active set", () => { + // Honouring it would mint into a drained or unroutable shard. Explicit pins still apply. + const rejected: string[] = []; + const shard = computeMintShard( + { id: "env_1" }, + deps(resolution, { + globalOverride: "z", + orgFeatureFlags: { runOpsMintShard: "a" }, + onOverrideRejected: (info) => rejected.push(info.override), + }) + ); + expect(shard).toBe("a"); + expect(rejected).toEqual(["z"]); + }); + + it("reports a bad override WITHOUT the environment id, so one line covers the fleet", () => { + // Keying the report by environment would log once per environment for a fleet-wide setting. + const seen: Array<{ override: string }> = []; + for (const id of envIds(50)) { + computeMintShard( + { id }, + deps(resolution, { globalOverride: "z", onOverrideRejected: (i) => seen.push(i) }) + ); + } + expect(seen).toHaveLength(50); + expect(new Set(seen.map((i) => i.override))).toEqual(new Set(["z"])); + expect(seen.every((i) => !("environmentId" in i))).toBe(true); + }); + + it("is ignored when it is not a legal value", () => { + for (const bad of ["legacy", "AB", "", "a,b"]) { + const shard = computeMintShard({ id: "env_1" }, deps(resolution, { globalOverride: bad })); + expect(shard).toBe(computeMintShard({ id: "env_1" }, deps(resolution))); + } + }); + + it("cannot resurrect minting when the list is empty", () => { + expect(computeMintShard({ id: "env_1" }, deps({ set: [] }, { globalOverride: "b" }))).toBe( + "new" + ); + }); +}); diff --git a/apps/webapp/app/v3/runOpsMigration/mintShardAssignment.ts b/apps/webapp/app/v3/runOpsMigration/mintShardAssignment.ts new file mode 100644 index 00000000000..a49a1a6a60d --- /dev/null +++ b/apps/webapp/app/v3/runOpsMigration/mintShardAssignment.ts @@ -0,0 +1,206 @@ +// PURE module: no env, no clock, no database. Kept separate from the .server wrapper so a test +// can drive it without evaluating env.server, whose schema parse demands a full environment. +import { createHash } from "node:crypto"; +import type { ShardKey } from "@trigger.dev/core/v3/isomorphic"; +import { FEATURE_FLAG } from "~/v3/featureFlags"; +import { + effectiveMintShardSet, + GEN_1_PIN_VALUE, + isValidPinValue, + readMintShardSetResolution, + type MintShardSetResolution, +} from "./mintShardGrace"; + +export type MintShardDeps = { + // The live list, from the control-plane database. + resolution: MintShardSetResolution; + // Fleet-wide pin that beats every per-org and per-env pin. The complete-cutover lever. + globalOverride?: unknown; + nowMs: number; + graceMs: number; + orgFeatureFlags: unknown; + onPinRejected?: (info: { environmentId: string; pin: string; activeSet: string[] }) => void; + onOverrideRejected?: (info: { override: string; activeSet: string[] }) => void; +}; + +function asRecord(value: unknown): Record | undefined { + if (!value || typeof value !== "object" || Array.isArray(value)) return undefined; + return value as Record; +} + +// Map keys are environment INTERNAL ids (cuids), not friendly ids. An unparseable blob, or a +// blob whose value for this environment is invalid, yields no per-env pin and lets the +// per-org scalar decide — never a silent un-pin straight to the hash. +function readEnvPin(raw: unknown, environmentId: string): ShardKey | undefined { + if (typeof raw !== "string") return undefined; + + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return undefined; + } + + const pins = asRecord(parsed); + const pin = pins?.[environmentId]; + return isValidPinValue(pin) ? pin : undefined; +} + +// Both pins live in the org override blob the trigger path already holds, so resolving a mint +// shard costs no query. +function readPin(orgFeatureFlags: unknown, environmentId: string): ShardKey | undefined { + const blob = asRecord(orgFeatureFlags); + if (!blob) return undefined; + + const envPin = readEnvPin(blob[FEATURE_FLAG.runOpsMintShardEnvPins], environmentId); + if (envPin !== undefined) return envPin; + + const scalar = blob[FEATURE_FLAG.runOpsMintShard]; + return isValidPinValue(scalar) ? scalar : undefined; +} + +// 64 bits: a 32-bit score collides at this system's environment count, and an undetected tie +// would resolve by iteration order. The NUL separates the fields so no two input pairs can +// concatenate alike. This hash input is FROZEN once gen-2 minting is live: changing it +// re-places every environment, silently. +function shardScore(environmentId: string, key: string): bigint { + return createHash("sha256").update(`${environmentId}\0${key}`).digest().readBigUInt64BE(0); +} + +function hrwSelect(environmentId: string, activeSet: string[]): string { + let bestKey = activeSet[0]; + let bestScore = shardScore(environmentId, bestKey); + + for (let i = 1; i < activeSet.length; i++) { + const key = activeSet[i]; + const score = shardScore(environmentId, key); + if (score > bestScore || (score === bestScore && key > bestKey)) { + bestKey = key; + bestScore = score; + } + } + + return bestKey; +} + +// PURE CORE — no env, no clock, no I/O; tests drive this directly. Deterministic for fixed +// deps, which is what lets run minting and token minting agree on one answer. +// +// An empty list is the off state, and it is the state of every deployment that has not set the +// flag. Bounding the list against the shard keys this deployment can actually route belongs with +// the shard descriptors, which own that information; nothing here mints, so nothing can misroute. +// +// A pin outside the active set falls through to the hash rather than throwing: honouring it +// would leak the drain the active list performs, and throwing would fail customer triggers +// whenever a pinned shard drains. +export function computeMintShard(environment: { id: string }, deps: MintShardDeps): ShardKey { + const activeSet = effectiveMintShardSet(deps.resolution, deps.nowMs, deps.graceMs); + if (activeSet.length === 0) { + return "new"; + } + + // The global override outranks every pin, so one flag completes a cutover without visiting + // each org. An override outside the active set is ignored, so explicit pins still apply. + if (isValidPinValue(deps.globalOverride)) { + const override = deps.globalOverride; + if (override === GEN_1_PIN_VALUE) { + return "new"; + } + if (activeSet.includes(override)) { + return override; + } + // Fleet-wide, so it is reported once for the value, not once per environment. + deps.onOverrideRejected?.({ override, activeSet }); + } + + const pin = readPin(deps.orgFeatureFlags, environment.id); + if (pin !== undefined) { + if (pin === GEN_1_PIN_VALUE) { + return "new"; + } + if (activeSet.includes(pin)) { + return pin; + } + deps.onPinRejected?.({ environmentId: environment.id, pin, activeSet }); + } + + return hrwSelect(environment.id, activeSet); +} + +// Read together so the override costs no extra query beyond the list it is bounded by. + +type GlobalShardConfig = { resolution: MintShardSetResolution; override: unknown }; + +export type MintShardCache = { value: GlobalShardConfig; expiresAt: number } | undefined; + +type MintShardCacheHandle = { + current: MintShardCache; + // The refresh currently in flight, if any. Concurrent misses share it. + inFlight?: Promise; +}; + +export type ResolveMintShardDeps = { + // Reads the list rows. Injected so the cache and the fail-safe are testable without a + // database, the same way computeRunIdMintKind takes its flag reader. + readFlags: () => Promise>; + cache: MintShardCacheHandle; + nowMs: number; + ttlMs: number; + graceMs: number; + orgFeatureFlags: unknown; + onPinRejected?: (info: { environmentId: string; pin: string; activeSet: string[] }) => void; + onOverrideRejected?: (info: { override: string; activeSet: string[] }) => void; + onReadFailed?: (error: unknown) => void; +}; + +// The live list is org-independent, so one process-wide entry serves every mint: one query per +// process per TTL, over one round-trip. Two processes can therefore disagree for the TTL PLUS the +// replica lag behind the read, which can exceed graceMs. That is tolerable here and only here, +// because a gen-2 id carries its own shard key, so disagreement cannot misroute an existing run; +// it only decides where the next root lands, and every failure direction is toward gen-1. +// +// A failed read falls back to gen-1 rather than guessing a list. Guessing would move every +// environment's placement for the length of one blip. +async function refreshConfig(deps: ResolveMintShardDeps): Promise { + try { + const flags = await deps.readFlags(); + const config: GlobalShardConfig = { + resolution: readMintShardSetResolution(flags), + override: flags[FEATURE_FLAG.runOpsMintShardOverride], + }; + deps.cache.current = { value: config, expiresAt: deps.nowMs + deps.ttlMs }; + return config; + } finally { + deps.cache.inFlight = undefined; + } +} + +export async function resolveMintShardWith( + environment: { id: string; orgFeatureFlags?: unknown }, + deps: ResolveMintShardDeps +): Promise { + let config: GlobalShardConfig; + const cached = deps.cache.current; + if (cached && cached.expiresAt > deps.nowMs) { + config = cached.value; + } else { + try { + // Single-flight. Without it, two misses both read, and a slower read landing after a + // faster one puts its older snapshot back into the cache for a whole TTL. + config = await (deps.cache.inFlight ??= refreshConfig(deps)); + } catch (error) { + deps.onReadFailed?.(error); + return "new"; + } + } + + return computeMintShard(environment, { + resolution: config.resolution, + globalOverride: config.override, + nowMs: deps.nowMs, + graceMs: deps.graceMs, + orgFeatureFlags: deps.orgFeatureFlags, + onPinRejected: deps.onPinRejected, + onOverrideRejected: deps.onOverrideRejected, + }); +} diff --git a/apps/webapp/app/v3/runOpsMigration/mintShardGrace.test.ts b/apps/webapp/app/v3/runOpsMigration/mintShardGrace.test.ts new file mode 100644 index 00000000000..3d26cb9fd1a --- /dev/null +++ b/apps/webapp/app/v3/runOpsMigration/mintShardGrace.test.ts @@ -0,0 +1,234 @@ +import { describe, expect, it } from "vitest"; +import { generateRunOpsIdV2 } from "@trigger.dev/core/v3/isomorphic"; +import { + effectiveMintShardSet, + isValidPinValue, + parseShardCsv, + readMintShardSetResolution, + SHARD_KEY_PATTERN, + stampMintShardSetFlip, + type MintShardSetResolution, +} from "./mintShardGrace"; + +const GRACE_MS = 90_000; +const T = 1_000_000; + +describe("parseShardCsv", () => { + it("returns an empty list for unset, empty and whitespace input", () => { + expect(parseShardCsv(undefined)).toEqual([]); + expect(parseShardCsv("")).toEqual([]); + expect(parseShardCsv(" ")).toEqual([]); + expect(parseShardCsv(",,")).toEqual([]); + }); + + it("trims, dedupes and SORTS, so operator typing order cannot change HRW", () => { + expect(parseShardCsv("b, a ,b")).toEqual(["a", "b"]); + expect(parseShardCsv("a,b,c")).toEqual(parseShardCsv("c,b,a")); + expect(parseShardCsv("b,c,a")).toEqual(parseShardCsv("a,c,b")); + }); + + it("accepts every one of the 36 legal shard keys", () => { + const all = "abcdefghijklmnopqrstuvwxyz0123456789".split(""); + expect(parseShardCsv(all.join(","))).toEqual([...all].sort()); + }); + + it("throws on a key outside [a-z0-9]", () => { + // generateRunOpsIdV2 throws on these; an unvalidated key MUST fail at boot, not at mint. + expect(() => parseShardCsv("A")).toThrow(/shard key/i); + expect(() => parseShardCsv("ab")).toThrow(/shard key/i); + expect(() => parseShardCsv("a,-")).toThrow(/shard key/i); + expect(() => parseShardCsv("a,_")).toThrow(/shard key/i); + }); + + it("rejects the reserved keys by name", () => { + expect(() => parseShardCsv("new")).toThrow(/reserved/i); + expect(() => parseShardCsv("a,legacy")).toThrow(/reserved/i); + }); +}); + +// Core does not export its shard-char pattern, so pin the local one to the real minter. +describe("shard alphabet agrees with the core minter", () => { + it("accepts exactly the characters generateRunOpsIdV2 accepts", () => { + const candidates = [ + ..."abcdefghijklmnopqrstuvwxyz0123456789".split(""), + ..."ABZ-_. +/é!".split(""), + "", + "ab", + ]; + + for (const candidate of candidates) { + let minterAccepts = true; + try { + generateRunOpsIdV2(candidate); + } catch { + minterAccepts = false; + } + + expect(SHARD_KEY_PATTERN.test(candidate)).toBe(minterAccepts); + } + }); +}); + +describe("isValidPinValue", () => { + it('accepts a shard key, and accepts "new" as the gen-1 hold value', () => { + expect(isValidPinValue("a")).toBe(true); + expect(isValidPinValue("7")).toBe(true); + expect(isValidPinValue("new")).toBe(true); + }); + + it("rejects legacy, and rejects anything outside the alphabet", () => { + expect(isValidPinValue("legacy")).toBe(false); + expect(isValidPinValue("A")).toBe(false); + expect(isValidPinValue("ab")).toBe(false); + expect(isValidPinValue("")).toBe(false); + }); +}); + +describe("effectiveMintShardSet", () => { + it("returns set when there is no stamp", () => { + const r: MintShardSetResolution = { set: ["a", "b"] }; + expect(effectiveMintShardSet(r, T, GRACE_MS)).toEqual(["a", "b"]); + }); + + it("returns set when flippedAtMs is absent even though prevSet is present", () => { + const r: MintShardSetResolution = { set: ["a", "b"], prevSet: ["a"] }; + expect(effectiveMintShardSet(r, T, GRACE_MS)).toEqual(["a", "b"]); + }); + + it("serves prevSet inside the window and set at/after the boundary", () => { + const r: MintShardSetResolution = { set: ["a", "b"], prevSet: ["a"], flippedAtMs: T }; + expect(effectiveMintShardSet(r, T, GRACE_MS)).toEqual(["a"]); + expect(effectiveMintShardSet(r, T + GRACE_MS - 1, GRACE_MS)).toEqual(["a"]); + // Boundary is exclusive on the prev side, so every process crosses it together. + expect(effectiveMintShardSet(r, T + GRACE_MS, GRACE_MS)).toEqual(["a", "b"]); + expect(effectiveMintShardSet(r, T + GRACE_MS + 1, GRACE_MS)).toEqual(["a", "b"]); + }); + + it("represents a graced first activation as an empty prevSet", () => { + const r: MintShardSetResolution = { set: ["a"], prevSet: [], flippedAtMs: T }; + expect(effectiveMintShardSet(r, T, GRACE_MS)).toEqual([]); + expect(effectiveMintShardSet(r, T + GRACE_MS, GRACE_MS)).toEqual(["a"]); + }); + + it("serves a drain through the window", () => { + const r: MintShardSetResolution = { set: ["a"], prevSet: ["a", "b"], flippedAtMs: T }; + expect(effectiveMintShardSet(r, T + 1, GRACE_MS)).toEqual(["a", "b"]); + expect(effectiveMintShardSet(r, T + GRACE_MS, GRACE_MS)).toEqual(["a"]); + }); +}); + +describe("readMintShardSetResolution", () => { + it("returns an empty set for an absent record", () => { + expect(readMintShardSetResolution(undefined)).toEqual({ set: [] }); + expect(readMintShardSetResolution({})).toEqual({ set: [] }); + }); + + it("reads and sorts the trio", () => { + const r = readMintShardSetResolution({ + runOpsMintShardSet: "b,a", + runOpsMintShardSetPrev: "c,a", + runOpsMintShardSetFlippedAt: new Date(T).toISOString(), + }); + expect(r).toEqual({ set: ["a", "b"], prevSet: ["a", "c"], flippedAtMs: T }); + }); + + it("omits prevSet when no flip timestamp is stored", () => { + // A prevSet with no timestamp can never apply, so it MUST NOT linger. + const r = readMintShardSetResolution({ + runOpsMintShardSet: "a,b", + runOpsMintShardSetPrev: "a", + }); + expect(r).toEqual({ set: ["a", "b"], prevSet: undefined, flippedAtMs: undefined }); + }); + + it("keeps an empty prevSet when a timestamp IS stored, which graces a first activation", () => { + const r = readMintShardSetResolution({ + runOpsMintShardSet: "a", + runOpsMintShardSetPrev: "", + runOpsMintShardSetFlippedAt: new Date(T).toISOString(), + }); + expect(r).toEqual({ set: ["a"], prevSet: [], flippedAtMs: T }); + }); + + it("degrades a stored value it cannot parse to an empty list instead of throwing", () => { + // Boot may throw on a bad env var. The mint path must never throw on a bad stored value. + expect(() => readMintShardSetResolution({ runOpsMintShardSet: "NOPE" })).not.toThrow(); + expect(readMintShardSetResolution({ runOpsMintShardSet: "NOPE" }).set).toEqual([]); + expect(readMintShardSetResolution({ runOpsMintShardSet: 42 }).set).toEqual([]); + expect( + readMintShardSetResolution({ + runOpsMintShardSet: "a", + runOpsMintShardSetFlippedAt: "not-a-date", + }) + ).toEqual({ set: ["a"], prevSet: undefined, flippedAtMs: undefined }); + }); +}); + +describe("stampMintShardSetFlip", () => { + it("does nothing when the save omits the set", () => { + // Omitting the set is an unrelated flag change; it must not inject a default or reset the clock. + const outgoing = { someOtherFlag: true } as Record; + expect(stampMintShardSetFlip({ runOpsMintShardSet: "a" }, outgoing, T, GRACE_MS)).toEqual({ + someOtherFlag: true, + }); + }); + + it("stamps prev and flippedAt on a genuine change", () => { + const stamped = stampMintShardSetFlip( + { runOpsMintShardSet: "a" }, + { runOpsMintShardSet: "a,b" }, + T, + GRACE_MS + ); + expect(stamped.runOpsMintShardSetPrev).toBe("a"); + expect(stamped.runOpsMintShardSetFlippedAt).toBe(new Date(T).toISOString()); + }); + + it("stamps an empty prev on a first activation", () => { + const stamped = stampMintShardSetFlip({}, { runOpsMintShardSet: "a" }, T, GRACE_MS); + expect(stamped.runOpsMintShardSetPrev).toBe(""); + expect(stamped.runOpsMintShardSetFlippedAt).toBe(new Date(T).toISOString()); + }); + + it("treats a reordered list as no change", () => { + const stamped = stampMintShardSetFlip( + { runOpsMintShardSet: "a,b" }, + { runOpsMintShardSet: "b,a" }, + T, + GRACE_MS + ); + expect(stamped.runOpsMintShardSetFlippedAt).toBeUndefined(); + }); + + it("carries an in-flight stamp forward rather than resetting the cutover clock", () => { + const existing = { + runOpsMintShardSet: "a,b", + runOpsMintShardSetPrev: "a", + runOpsMintShardSetFlippedAt: new Date(T).toISOString(), + }; + const stamped = stampMintShardSetFlip( + existing, + { runOpsMintShardSet: "a,b" }, + T + 1000, + GRACE_MS + ); + expect(stamped.runOpsMintShardSetPrev).toBe("a"); + expect(stamped.runOpsMintShardSetFlippedAt).toBe(new Date(T).toISOString()); + }); + + it("stamps prev as the CURRENTLY-EFFECTIVE set when a second flip lands mid-window", () => { + // Two flips inside one window must not strand the original prev; prev is what readers serve now. + const existing = { + runOpsMintShardSet: "a,b", + runOpsMintShardSetPrev: "a", + runOpsMintShardSetFlippedAt: new Date(T).toISOString(), + }; + const stamped = stampMintShardSetFlip( + existing, + { runOpsMintShardSet: "a,b,c" }, + T + 1000, + GRACE_MS + ); + expect(stamped.runOpsMintShardSetPrev).toBe("a"); + }); +}); diff --git a/apps/webapp/app/v3/runOpsMigration/mintShardGrace.ts b/apps/webapp/app/v3/runOpsMigration/mintShardGrace.ts new file mode 100644 index 00000000000..65d8af6f417 --- /dev/null +++ b/apps/webapp/app/v3/runOpsMigration/mintShardGrace.ts @@ -0,0 +1,133 @@ +import type { ShardKey } from "@trigger.dev/core/v3/isomorphic"; + +// Index 24 of a gen-2 id sits inside the pod name `runner-`, and a DNS-1123 label accepts +// lowercase only, so the alphabet is 36 keys and no wider. Core keeps its copy private; +// mintShardGrace.test.ts pins this pattern to generateRunOpsIdV2 instead. +export const SHARD_KEY_PATTERN = /^[a-z0-9]$/; + +// Neither may enter the active set: "new" already means "mint a gen-1 run-ops id" and +// "legacy" means the cuid store, which minting never selects. +const RESERVED_SHARD_KEYS: readonly string[] = ["new", "legacy"]; + +// "new" IS legal as a PIN, holding one org or environment on gen-1 while the rest of the fleet +// mints gen-2. Without it a non-empty active set moves every environment at once. +export const GEN_1_PIN_VALUE = "new"; + +export type MintShardSetResolution = { + set: string[]; + prevSet?: string[]; + flippedAtMs?: number; +}; + +// Flag keys holding the active set and its grace stamp. Named here so the pure module can read +// a flag record without importing the catalog. +const SET_KEY = "runOpsMintShardSet"; +const SET_PREV_KEY = "runOpsMintShardSetPrev"; +const SET_FLIPPED_AT_KEY = "runOpsMintShardSetFlippedAt"; + +export function isValidPinValue(value: unknown): value is ShardKey { + if (typeof value !== "string") return false; + return value === GEN_1_PIN_VALUE || SHARD_KEY_PATTERN.test(value); +} + +// Throws rather than dropping a bad key: generateRunOpsIdV2 throws on an out-of-alphabet char, +// so an unvalidated key must fail at boot and never at mint. +export function parseShardCsv(raw: string | undefined | null): string[] { + const keys = (raw ?? "") + .split(",") + .map((s) => s.trim()) + .filter(Boolean); + + const unique = new Set(); + for (const key of keys) { + if (RESERVED_SHARD_KEYS.includes(key)) { + throw new Error(`"${key}" is a reserved key and cannot be an active mint shard`); + } + if (!SHARD_KEY_PATTERN.test(key)) { + throw new Error(`invalid shard key "${key}": must be a single char in [a-z0-9]`); + } + unique.add(key); + } + + // Sorted so no placement can depend on the order an operator typed the CSV in. + return [...unique].sort(); +} + +// Cutover boundary, mirroring effectiveMintKind. `nowMs` is the reader's wall clock while +// `flippedAtMs` is operator-supplied, so this assumes NTP-synced hosts with skew << graceMs, +// letting every process cross [flippedAtMs, flippedAtMs + graceMs) together (OLD then NEW). +export function effectiveMintShardSet( + r: MintShardSetResolution, + nowMs: number, + graceMs: number +): string[] { + if (r.prevSet === undefined || r.flippedAtMs === undefined) { + return r.set; + } + return nowMs < r.flippedAtMs + graceMs ? r.prevSet : r.set; +} + +// The active set lives in the control-plane database, not in the environment. A deploy rolls +// for hours, so two pods can hold different environment values at the same time; only a shared +// row lets every pod agree on one set. Boot may reject a bad environment value, but the mint +// path must never throw on a bad stored value, so an unreadable list degrades to empty. +function readStoredCsv(value: unknown): string[] { + if (typeof value !== "string") return []; + try { + return parseShardCsv(value); + } catch { + return []; + } +} + +// Reads the { set, prevSet, flippedAtMs } trio out of one flag record. Pure. A prevSet with no +// timestamp can never apply, so it is dropped. A timestamp with an EMPTY prevSet is meaningful: +// it graces a first activation, serving no shards for the window. +export function readMintShardSetResolution( + flags: Record | null | undefined +): MintShardSetResolution { + const source = flags ?? {}; + const flippedAtRaw = source[SET_FLIPPED_AT_KEY]; + const parsed = typeof flippedAtRaw === "string" ? Date.parse(flippedAtRaw) : NaN; + const flippedAtMs = Number.isNaN(parsed) ? undefined : parsed; + + return { + set: readStoredCsv(source[SET_KEY]), + prevSet: flippedAtMs === undefined ? undefined : readStoredCsv(source[SET_PREV_KEY]), + flippedAtMs, + }; +} + +// Stamps a grace window only when the outgoing set differs from the stored one. prev becomes the +// set readers serve right now, so a second flip inside one window cannot strand the first. A save +// that leaves the set unchanged carries any in-flight stamp forward, so it cannot reset the clock. +export function stampMintShardSetFlip( + existingFlags: Record | null | undefined, + outgoingFlags: Record, + nowMs: number, + graceMs: number +): Record { + // Only act when the save actually SETS the list. Omitting it must not inject a default. + if (typeof outgoingFlags[SET_KEY] !== "string") { + return outgoingFlags; + } + + const existing = existingFlags ?? {}; + const outgoingSet = readStoredCsv(outgoingFlags[SET_KEY]); + const storedSet = readStoredCsv(existing[SET_KEY]); + + if (outgoingSet.join(",") !== storedSet.join(",")) { + const effective = effectiveMintShardSet(readMintShardSetResolution(existing), nowMs, graceMs); + outgoingFlags[SET_PREV_KEY] = effective.join(","); + outgoingFlags[SET_FLIPPED_AT_KEY] = new Date(nowMs).toISOString(); + return outgoingFlags; + } + + if (existing[SET_PREV_KEY] !== undefined) { + outgoingFlags[SET_PREV_KEY] = existing[SET_PREV_KEY]; + } + if (existing[SET_FLIPPED_AT_KEY] !== undefined) { + outgoingFlags[SET_FLIPPED_AT_KEY] = existing[SET_FLIPPED_AT_KEY]; + } + return outgoingFlags; +} diff --git a/apps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.ts b/apps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.ts new file mode 100644 index 00000000000..542384e16f8 --- /dev/null +++ b/apps/webapp/app/v3/runOpsMigration/runOpsMintShard.server.ts @@ -0,0 +1,92 @@ +import { $replica, boundedIn } from "~/db.server"; +import { env } from "~/env.server"; +import { logger } from "~/services/logger.server"; +import { BoundedTtlCache } from "~/services/realtime/boundedTtlCache"; +import { singleton } from "~/utils/singleton"; +import { FEATURE_FLAG } from "~/v3/featureFlags"; +import type { ShardKey } from "@trigger.dev/core/v3/isomorphic"; +import { resolveMintShardWith, type MintShardCache } from "./mintShardAssignment"; + +// A misconfiguration is reported again after this long, so a still-broken pin stays visible +// without logging on every trigger. +const REPORT_TTL_MS = 3_600_000; +const REPORT_MAX_ENTRIES = 10_000; + +const GLOBAL_SHARD_KEYS = [ + FEATURE_FLAG.runOpsMintShardSet, + FEATURE_FLAG.runOpsMintShardSetPrev, + FEATURE_FLAG.runOpsMintShardSetFlippedAt, + FEATURE_FLAG.runOpsMintShardOverride, +]; + +const liveCache = singleton("runOpsMintShardCache", (): { current: MintShardCache } => ({ + current: undefined, +})); + +async function readSetFlags(): Promise> { + const rows = await $replica.featureFlag.findMany({ + where: { key: { in: boundedIn(GLOBAL_SHARD_KEYS) } }, + select: { key: true, value: true }, + }); + const flags: Record = {}; + for (const row of rows) { + flags[row.key] = row.value; + } + return flags; +} + +// A stale pin sits on the root-trigger path, so it would otherwise log on every trigger for that +// environment forever. Bounded, because the set of pinned environments is operator-controlled but +// not operator-bounded, and an unbounded Set on this path is a leak. +const reportedPins = singleton( + "runOpsMintShardReportedPins", + () => new BoundedTtlCache(REPORT_TTL_MS, REPORT_MAX_ENTRIES) +); + +function reportPinRejected(info: { + environmentId: string; + pin: string; + activeSet: string[]; +}): void { + if (reportedPins.get(info.environmentId) !== undefined) return; + reportedPins.set(info.environmentId, true); + logger.error("[runOpsMintShard] pinned shard is not in the active set; using the hash", info); +} + +// Keyed by the override value, not by environment: one bad override applies to the whole fleet, +// so one line is the correct volume. Keying by environment would log once per environment. +const reportedOverrides = singleton( + "runOpsMintShardReportedOverrides", + () => new BoundedTtlCache(REPORT_TTL_MS, REPORT_MAX_ENTRIES) +); + +function reportOverrideRejected(info: { override: string; activeSet: string[] }): void { + if (reportedOverrides.get(info.override) !== undefined) return; + reportedOverrides.set(info.override, true); + logger.error("[runOpsMintShard] override shard is not in the active set; ignoring it", info); +} + +/** + * Which shard an environment mints new roots into. Call only after resolveRunIdMintKind has + * returned "runOpsId". Returns "new" to mean a gen-1 run-ops id, which is today's behaviour. + * + * @knipignore the gen-2 write-path change is the first production caller; drop this tag there. + */ +export async function resolveMintShard(environment: { + id: string; + // Pass environment.organization.featureFlags from the trigger call site. + orgFeatureFlags?: unknown; +}): Promise { + return resolveMintShardWith(environment, { + readFlags: readSetFlags, + cache: liveCache, + nowMs: Date.now(), + ttlMs: env.RUN_OPS_MINT_FLAG_CACHE_TTL_MS, + graceMs: env.RUN_OPS_MINT_FLIP_GRACE_MS, + orgFeatureFlags: environment.orgFeatureFlags, + onPinRejected: reportPinRejected, + onOverrideRejected: reportOverrideRejected, + onReadFailed: (error) => + logger.error("[runOpsMintShard] shard-set read failed; minting gen-1 (fail-safe)", { error }), + }); +} diff --git a/apps/webapp/test/adminFeatureFlagsRouteAction.test.ts b/apps/webapp/test/adminFeatureFlagsRouteAction.test.ts index a510fbe0f35..b5bfd8ea052 100644 --- a/apps/webapp/test/adminFeatureFlagsRouteAction.test.ts +++ b/apps/webapp/test/adminFeatureFlagsRouteAction.test.ts @@ -2,7 +2,7 @@ // 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 { boundedIn, $transaction as realTransaction } from "@trigger.dev/database"; import type { PrismaClient } from "@trigger.dev/database"; import { postgresTest } from "@internal/testcontainers"; import { describe, expect, vi } from "vitest"; @@ -22,6 +22,24 @@ vi.mock("~/db.server", () => ({ return db.client; }, boundedIn, + // Delegates to the SAME shared implementation the production helper wraps, so the + // transactional semantics, the nesting case and the retry behaviour are the real ones rather + // than a reimplementation. Only the webapp wrapper's tracing span and its infrastructure-error + // logging are absent, and neither is asserted here. + $transaction: ( + client: PrismaClient, + nameOrFn: unknown, + fnOrOptions?: unknown, + options?: unknown + ) => { + const fn = (typeof nameOrFn === "function" ? nameOrFn : fnOrOptions) as Parameters< + typeof realTransaction + >[1]; + const opts = (typeof nameOrFn === "function" ? fnOrOptions : options) as Parameters< + typeof realTransaction + >[3]; + return realTransaction(client, fn, () => {}, opts); + }, })); import { action } from "~/routes/admin.feature-flags"; diff --git a/apps/webapp/test/globalFlagChangeList.test.ts b/apps/webapp/test/globalFlagChangeList.test.ts new file mode 100644 index 00000000000..269774e28d3 --- /dev/null +++ b/apps/webapp/test/globalFlagChangeList.test.ts @@ -0,0 +1,142 @@ +// Two properties of a global flag save that the admin page had no way to state. +// +// 1. Unsetting a graced primary clears its server-computed stamps too. Those keys are locked, so +// they are absent from the page's editable set, and the confirm dialog listed one removal +// while three rows were deleted. +// 2. A save should write only the flags whose value actually changed. Writing every submitted +// flag costs one round trip each inside an interactive transaction. +import { describe, expect, it } from "vitest"; +import { FEATURE_FLAG, derivedFlagsClearedWith } from "~/v3/featureFlags"; +import { flagsNeedingWrite } from "~/v3/featureFlags.server"; +import { buildFlagChangeList } from "~/components/admin/flagChangeList"; + +const LOCKED = [ + FEATURE_FLAG.runOpsMintKindPrev, + FEATURE_FLAG.runOpsMintKindFlippedAt, + FEATURE_FLAG.runOpsMintShardSetPrev, + FEATURE_FLAG.runOpsMintShardSetFlippedAt, +] as string[]; + +// Sorted, as the dialog sorts before calling: the builder preserves the order it is given. +const EDITABLE = [ + FEATURE_FLAG.runOpsMintKind, + FEATURE_FLAG.runOpsMintShardSet, + FEATURE_FLAG.mollifierEnabled, +].sort() as string[]; + +describe("derivedFlagsClearedWith", () => { + it("names the stamps that go with a graced primary", () => { + expect(derivedFlagsClearedWith(FEATURE_FLAG.runOpsMintKind)).toEqual([ + FEATURE_FLAG.runOpsMintKindPrev, + FEATURE_FLAG.runOpsMintKindFlippedAt, + ]); + expect(derivedFlagsClearedWith(FEATURE_FLAG.runOpsMintShardSet)).toEqual([ + FEATURE_FLAG.runOpsMintShardSetPrev, + FEATURE_FLAG.runOpsMintShardSetFlippedAt, + ]); + }); + + it("names nothing for an ordinary flag, or for a stamp itself", () => { + expect(derivedFlagsClearedWith(FEATURE_FLAG.mollifierEnabled)).toEqual([]); + expect(derivedFlagsClearedWith(FEATURE_FLAG.runOpsMintKindPrev)).toEqual([]); + }); +}); + +describe("buildFlagChangeList — what the confirm dialog must show", () => { + it("lists an added, a changed and a removed flag", () => { + const changes = buildFlagChangeList({ + editableKeys: EDITABLE, + lockedKeys: LOCKED, + initialValues: { mollifierEnabled: true, runOpsMintShardSet: "a" }, + storedValues: { mollifierEnabled: true, runOpsMintShardSet: "a" }, + newValues: { runOpsMintShardSet: "a,b", runOpsMintKind: "runOpsId" }, + }); + + expect(changes).toEqual([ + { key: FEATURE_FLAG.mollifierEnabled, type: "removed", oldVal: "true" }, + { key: FEATURE_FLAG.runOpsMintKind, type: "added", newVal: "runOpsId" }, + { key: FEATURE_FLAG.runOpsMintShardSet, type: "changed", oldVal: "a", newVal: "a,b" }, + ]); + }); + + it("discloses the stamps cleared alongside an unset graced primary", () => { + // Three rows are deleted, so three removals must be shown, not one. The caller filters + // locked keys OUT of initialValues, so the stamps are only visible in storedValues. + const changes = buildFlagChangeList({ + editableKeys: EDITABLE, + lockedKeys: LOCKED, + initialValues: { runOpsMintShardSet: "a,b" }, + storedValues: { + runOpsMintShardSet: "a,b", + runOpsMintShardSetPrev: "a", + runOpsMintShardSetFlippedAt: "2026-08-24T00:00:00.000Z", + }, + newValues: {}, + }); + + expect(changes.map((c) => c.key)).toEqual([ + FEATURE_FLAG.runOpsMintShardSet, + FEATURE_FLAG.runOpsMintShardSetPrev, + FEATURE_FLAG.runOpsMintShardSetFlippedAt, + ]); + expect(changes.every((c) => c.type === "removed")).toBe(true); + }); + + it("does not disclose a stamp that is not stored", () => { + const changes = buildFlagChangeList({ + editableKeys: EDITABLE, + lockedKeys: LOCKED, + initialValues: { runOpsMintShardSet: "a,b" }, + storedValues: { runOpsMintShardSet: "a,b" }, + newValues: {}, + }); + expect(changes.map((c) => c.key)).toEqual([FEATURE_FLAG.runOpsMintShardSet]); + }); + + it("does not disclose stamps when the primary is only CHANGED", () => { + // A change re-stamps rather than clearing, so nothing is removed. + const changes = buildFlagChangeList({ + editableKeys: EDITABLE, + lockedKeys: LOCKED, + initialValues: { runOpsMintShardSet: "a" }, + storedValues: { runOpsMintShardSet: "a", runOpsMintShardSetPrev: "" }, + newValues: { runOpsMintShardSet: "a,b" }, + }); + expect(changes.map((c) => c.key)).toEqual([FEATURE_FLAG.runOpsMintShardSet]); + }); + + it("never lists a locked key on its own", () => { + const changes = buildFlagChangeList({ + editableKeys: EDITABLE, + lockedKeys: LOCKED, + initialValues: {}, + storedValues: { runOpsMintShardSetPrev: "a" }, + newValues: {}, + }); + expect(changes).toEqual([]); + }); +}); + +describe("flagsNeedingWrite — one round trip per CHANGED flag, not per submitted flag", () => { + it("drops a submitted flag whose stored value already matches", () => { + const out = flagsNeedingWrite( + { mollifierEnabled: true, hasAiAccess: true }, + { mollifierEnabled: true, hasAiAccess: false } + ); + expect(out).toEqual({ hasAiAccess: true }); + }); + + it("keeps a flag that is absent from storage", () => { + expect(flagsNeedingWrite({ mollifierEnabled: true }, {})).toEqual({ mollifierEnabled: true }); + }); + + it("returns nothing when a save changes nothing", () => { + expect(flagsNeedingWrite({ mollifierEnabled: true }, { mollifierEnabled: true })).toEqual({}); + }); + + it("compares by value, not by reference, so a CSV rewritten the same way is not a write", () => { + expect(flagsNeedingWrite({ runOpsMintShardSet: "a,b" }, { runOpsMintShardSet: "a,b" })).toEqual( + {} + ); + }); +}); diff --git a/apps/webapp/test/globalFlagWriteRouting.test.ts b/apps/webapp/test/globalFlagWriteRouting.test.ts new file mode 100644 index 00000000000..3193919b5ca --- /dev/null +++ b/apps/webapp/test/globalFlagWriteRouting.test.ts @@ -0,0 +1,110 @@ +// Both global write routes used to carry their own copy of "which keys are graced" and "which +// keys are derived", so a new group needed an edit in three places and missing one meant an +// unstamped flip or a stamp taken from a request body. These tests cover the two helpers the +// routes now call. They do NOT reach a route: both actions sit behind admin auth, so that the +// routes call these helpers rather than their own copies is held by review, not by a test. +import { describe, expect, it } from "vitest"; +import { FEATURE_FLAG, lockedFlagsInPayload } from "~/v3/featureFlags"; +import { touchesGracedGroup, withoutDerivedKeys } from "~/v3/featureFlags.server"; + +describe("touchesGracedGroup — decides whether a save needs the stamped path", () => { + it("is true for a mint-kind change", () => { + expect(touchesGracedGroup({ [FEATURE_FLAG.runOpsMintKind]: "runOpsId" })).toBe(true); + }); + + it("is true for a shard-list change", () => { + expect(touchesGracedGroup({ [FEATURE_FLAG.runOpsMintShardSet]: "a,b" })).toBe(true); + }); + + it("is false for an ordinary flag, which writes directly", () => { + expect(touchesGracedGroup({ [FEATURE_FLAG.mollifierEnabled]: true })).toBe(false); + expect(touchesGracedGroup({})).toBe(false); + }); + + it("is false when only a DERIVED key is present", () => { + // A body carrying only a stamp changes no group. Treating it as a flip would let a caller + // reset a cutover clock without touching the value the clock dates. + expect(touchesGracedGroup({ [FEATURE_FLAG.runOpsMintKindPrev]: "cuid" })).toBe(false); + expect(touchesGracedGroup({ [FEATURE_FLAG.runOpsMintShardSetPrev]: "a" })).toBe(false); + }); + + it("recognises every graced primary the group table declares", () => { + const gracedPrimaries = [FEATURE_FLAG.runOpsMintKind, FEATURE_FLAG.runOpsMintShardSet]; + for (const key of gracedPrimaries) { + expect(touchesGracedGroup({ [key]: "x" })).toBe(true); + } + // Every key the strip removes belongs to a group whose primary is one of the above. + const derived = Object.keys( + withoutDerivedKeys({ + [FEATURE_FLAG.runOpsMintKindPrev]: "cuid", + [FEATURE_FLAG.runOpsMintKindFlippedAt]: "t", + [FEATURE_FLAG.runOpsMintShardSetPrev]: "a", + [FEATURE_FLAG.runOpsMintShardSetFlippedAt]: "t", + } as Record) + ); + expect(derived).toEqual([]); + }); +}); + +describe("withoutDerivedKeys — a stamp is never taken from a request body", () => { + it("strips both stamps and keeps everything else", () => { + const out = withoutDerivedKeys({ + [FEATURE_FLAG.runOpsMintKind]: "runOpsId", + [FEATURE_FLAG.runOpsMintKindPrev]: "spoofed", + [FEATURE_FLAG.runOpsMintKindFlippedAt]: "1999-01-01T00:00:00.000Z", + [FEATURE_FLAG.runOpsMintShardSet]: "a,b", + [FEATURE_FLAG.runOpsMintShardSetPrev]: "spoofed", + [FEATURE_FLAG.runOpsMintShardSetFlippedAt]: "1999-01-01T00:00:00.000Z", + [FEATURE_FLAG.mollifierEnabled]: true, + } as Record); + + expect(out).toEqual({ + [FEATURE_FLAG.runOpsMintKind]: "runOpsId", + [FEATURE_FLAG.runOpsMintShardSet]: "a,b", + [FEATURE_FLAG.mollifierEnabled]: true, + }); + }); + + it("does not mutate its input", () => { + const input = { [FEATURE_FLAG.runOpsMintKindPrev]: "cuid" } as Record; + withoutDerivedKeys(input); + expect(input[FEATURE_FLAG.runOpsMintKindPrev]).toBe("cuid"); + }); +}); + +describe("lockedFlagsInPayload — what the global page refuses", () => { + it("refuses a locked flag on managed cloud, where the page never offers one", () => { + const refused = lockedFlagsInPayload( + [FEATURE_FLAG.taskEventRepository, FEATURE_FLAG.mollifierEnabled], + true + ); + expect(refused).toEqual([FEATURE_FLAG.taskEventRepository]); + }); + + it("refuses the mint-shard pins, which are per-org only", () => { + expect(lockedFlagsInPayload([FEATURE_FLAG.runOpsMintShard], true)).toEqual([ + FEATURE_FLAG.runOpsMintShard, + ]); + expect(lockedFlagsInPayload([FEATURE_FLAG.runOpsMintShardEnvPins], true)).toEqual([ + FEATURE_FLAG.runOpsMintShardEnvPins, + ]); + }); + + it("refuses a grace stamp, which the server owns", () => { + expect(lockedFlagsInPayload([FEATURE_FLAG.runOpsMintShardSetFlippedAt], true)).toEqual([ + FEATURE_FLAG.runOpsMintShardSetFlippedAt, + ]); + }); + + it("allows the shard list, because that is the page's ramp lever", () => { + expect(lockedFlagsInPayload([FEATURE_FLAG.runOpsMintShardSet], true)).toEqual([]); + }); + + it("refuses nothing when not managed cloud, where an admin may unlock and edit", () => { + expect(lockedFlagsInPayload([FEATURE_FLAG.taskEventRepository], false)).toEqual([]); + }); + + it("refuses nothing for an empty payload", () => { + expect(lockedFlagsInPayload([], true)).toEqual([]); + }); +}); diff --git a/apps/webapp/test/runOpsMintGlobalFlipLock.test.ts b/apps/webapp/test/runOpsMintGlobalFlipLock.test.ts index 1492fd02c43..b3acd7e45eb 100644 --- a/apps/webapp/test/runOpsMintGlobalFlipLock.test.ts +++ b/apps/webapp/test/runOpsMintGlobalFlipLock.test.ts @@ -5,7 +5,7 @@ import type { PrismaClient } from "@trigger.dev/database"; import { postgresTest } from "@internal/testcontainers"; import { describe, expect, vi } from "vitest"; import { FEATURE_FLAG } from "~/v3/featureFlags"; -import { applyGlobalMintKindFlip, makeSetMultipleFlags } from "~/v3/featureFlags.server"; +import { applyGlobalGracedFlips, makeSetMultipleFlags } from "~/v3/featureFlags.server"; vi.setConfig({ testTimeout: 60_000 }); @@ -25,11 +25,11 @@ async function readGlobalMint(prisma: PrismaClient): Promise { +describe("applyGlobalGracedFlips — transactional stamp + serialized flips", () => { postgresTest("a genuine global flip stamps prev + flippedAt", async ({ prisma }) => { await makeSetMultipleFlags(prisma)({ [FEATURE_FLAG.runOpsMintKind]: "cuid" }); - await applyGlobalMintKindFlip(prisma, { [FEATURE_FLAG.runOpsMintKind]: "runOpsId" }, 60_000); + await applyGlobalGracedFlips(prisma, { [FEATURE_FLAG.runOpsMintKind]: "runOpsId" }, 60_000); const m = await readGlobalMint(prisma); expect(m[FEATURE_FLAG.runOpsMintKind]).toBe("runOpsId"); @@ -44,7 +44,7 @@ describe("applyGlobalMintKindFlip — transactional stamp + serialized flips", ( await Promise.all( Array.from({ length: 8 }, () => - applyGlobalMintKindFlip(prisma, { [FEATURE_FLAG.runOpsMintKind]: "runOpsId" }, 60_000) + applyGlobalGracedFlips(prisma, { [FEATURE_FLAG.runOpsMintKind]: "runOpsId" }, 60_000) ) ); diff --git a/apps/webapp/test/runOpsMintShardFlags.test.ts b/apps/webapp/test/runOpsMintShardFlags.test.ts new file mode 100644 index 00000000000..98b76eb19c8 --- /dev/null +++ b/apps/webapp/test/runOpsMintShardFlags.test.ts @@ -0,0 +1,118 @@ +// The mint-shard flags carry two safety claims that only the catalog can enforce: a bad value is +// rejected at WRITE (so no unroutable key and no silently-unpinned environment can ever be +// stored), and each key is locked at the scope its resolver does not read. Pure, no containers. +import { describe, expect, it } from "vitest"; +import { + FEATURE_FLAG, + FeatureFlagCatalog, + GLOBAL_LOCKED_FLAGS, + ORG_LOCKED_FLAGS, + validateFeatureFlagValue, +} from "~/v3/featureFlags"; + +describe("runOpsMintShard — the per-org pin", () => { + const key = FEATURE_FLAG.runOpsMintShard; + + it("accepts every legal shard key", () => { + for (const c of "abcdefghijklmnopqrstuvwxyz0123456789") { + expect(validateFeatureFlagValue(key, c).success).toBe(true); + } + }); + + it('accepts "new", which holds an org on gen-1', () => { + expect(validateFeatureFlagValue(key, "new").success).toBe(true); + }); + + it("rejects a value that could never be stamped into an id", () => { + for (const bad of ["A", "ab", "", "-", "legacy", " a", "a,b"]) { + expect(validateFeatureFlagValue(key, bad).success).toBe(false); + } + }); +}); + +describe("runOpsMintShardEnvPins — the per-environment pins", () => { + const key = FEATURE_FLAG.runOpsMintShardEnvPins; + + it("accepts a map of environment id to shard key", () => { + expect( + validateFeatureFlagValue(key, JSON.stringify({ env_1: "a", env_2: "new" })).success + ).toBe(true); + expect(validateFeatureFlagValue(key, "{}").success).toBe(true); + }); + + it("rejects a blob that is not JSON, so a typo cannot silently un-pin every environment", () => { + for (const bad of ["{not json", "", "null", "[]", '"a"', "42"]) { + expect(validateFeatureFlagValue(key, bad).success).toBe(false); + } + }); + + it("rejects a map whose value is not a legal pin", () => { + for (const bad of [{ env_1: "AB" }, { env_1: "legacy" }, { env_1: 1 }, { env_1: "" }]) { + expect(validateFeatureFlagValue(key, JSON.stringify(bad)).success).toBe(false); + } + }); +}); + +describe("runOpsMintShardSet — the active list", () => { + const key = FEATURE_FLAG.runOpsMintShardSet; + + it("accepts an empty list and a CSV of legal keys", () => { + expect(validateFeatureFlagValue(key, "").success).toBe(true); + expect(validateFeatureFlagValue(key, "a").success).toBe(true); + expect(validateFeatureFlagValue(key, "a,b, c").success).toBe(true); + }); + + it("rejects a CSV holding a key that cannot be routed", () => { + for (const bad of ["A", "ab", "a,B", "a,legacy", "a,new", "a;b"]) { + expect(validateFeatureFlagValue(key, bad).success).toBe(false); + } + }); +}); + +describe("runOpsMintShardOverride — the complete-cutover lever", () => { + const key = FEATURE_FLAG.runOpsMintShardOverride; + + it("accepts a shard key and accepts new", () => { + expect(validateFeatureFlagValue(key, "a").success).toBe(true); + expect(validateFeatureFlagValue(key, "new").success).toBe(true); + }); + + it("rejects anything that is not a single legal key", () => { + for (const bad of ["A", "ab", "", "legacy", "a,b"]) { + expect(validateFeatureFlagValue(key, bad).success).toBe(false); + } + }); +}); + +describe("scope locks match what each resolver actually reads", () => { + it("locks the pins globally, because the resolver reads them from the org blob only", () => { + expect(GLOBAL_LOCKED_FLAGS).toContain(FEATURE_FLAG.runOpsMintShard); + expect(GLOBAL_LOCKED_FLAGS).toContain(FEATURE_FLAG.runOpsMintShardEnvPins); + }); + + it("locks the list and the override per-org, because both are deployment-wide", () => { + expect(ORG_LOCKED_FLAGS).toContain(FEATURE_FLAG.runOpsMintShardSet); + expect(ORG_LOCKED_FLAGS).toContain(FEATURE_FLAG.runOpsMintShardSetPrev); + expect(ORG_LOCKED_FLAGS).toContain(FEATURE_FLAG.runOpsMintShardSetFlippedAt); + // An org that could override the cutover lever would defeat its purpose. + expect(ORG_LOCKED_FLAGS).toContain(FEATURE_FLAG.runOpsMintShardOverride); + }); + + it("keeps the pins settable per-org, which is the canary lever", () => { + expect(ORG_LOCKED_FLAGS).not.toContain(FEATURE_FLAG.runOpsMintShard); + expect(ORG_LOCKED_FLAGS).not.toContain(FEATURE_FLAG.runOpsMintShardEnvPins); + }); + + it("registers every new key in the catalog, so the admin pages render it", () => { + for (const key of [ + FEATURE_FLAG.runOpsMintShard, + FEATURE_FLAG.runOpsMintShardEnvPins, + FEATURE_FLAG.runOpsMintShardSet, + FEATURE_FLAG.runOpsMintShardSetPrev, + FEATURE_FLAG.runOpsMintShardSetFlippedAt, + FEATURE_FLAG.runOpsMintShardOverride, + ]) { + expect(FeatureFlagCatalog).toHaveProperty(key); + } + }); +}); diff --git a/apps/webapp/test/runOpsMintShardSetFlip.test.ts b/apps/webapp/test/runOpsMintShardSetFlip.test.ts new file mode 100644 index 00000000000..6dc126fc0bd --- /dev/null +++ b/apps/webapp/test/runOpsMintShardSetFlip.test.ts @@ -0,0 +1,279 @@ +// The active mint-shard list lives in the control-plane database, not in the environment: a +// rolling deploy runs two environment values at once for hours, so only a shared row lets every +// pod agree on one list. A change must therefore read -> stamp -> write under an advisory lock, +// and must never be writable as a bare upsert from a request body. Real testcontainers Postgres. +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 { + applyGlobalGracedFlips, + makeSetMultipleFlags, + replaceGlobalFeatureFlags, +} from "~/v3/featureFlags.server"; + +vi.setConfig({ testTimeout: 60_000 }); + +const SET_KEYS: FeatureFlagKey[] = [ + FEATURE_FLAG.runOpsMintShardSet, + FEATURE_FLAG.runOpsMintShardSetPrev, + FEATURE_FLAG.runOpsMintShardSetFlippedAt, +]; + +const MINT_KIND_KEYS: FeatureFlagKey[] = [ + FEATURE_FLAG.runOpsMintKind, + FEATURE_FLAG.runOpsMintKindPrev, + FEATURE_FLAG.runOpsMintKindFlippedAt, +]; + +const CATALOG_KEYS: FeatureFlagKey[] = [ + ...SET_KEYS, + ...MINT_KIND_KEYS, + FEATURE_FLAG.mollifierEnabled, +]; + +// Self-hosted with the lock left on: locked flags survive omission, everything else sweeps. +const SELF_HOSTED = { isManagedCloud: false, unlockLockedFlags: false } as const; + +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("applyGlobalGracedFlips — the shard-set list is stamped, not bare-written", () => { + postgresTest("a genuine list change stamps prev + flippedAt", async ({ prisma }) => { + await makeSetMultipleFlags(prisma)({ [FEATURE_FLAG.runOpsMintShardSet]: "a" }); + + await applyGlobalGracedFlips(prisma, { [FEATURE_FLAG.runOpsMintShardSet]: "a,b" }, 60_000); + + const m = await readFlags(prisma, SET_KEYS); + expect(m[FEATURE_FLAG.runOpsMintShardSet]).toBe("a,b"); + expect(m[FEATURE_FLAG.runOpsMintShardSetPrev]).toBe("a"); + expect(typeof m[FEATURE_FLAG.runOpsMintShardSetFlippedAt]).toBe("string"); + }); + + postgresTest("a first activation stamps an empty prev, which graces it", async ({ prisma }) => { + await applyGlobalGracedFlips(prisma, { [FEATURE_FLAG.runOpsMintShardSet]: "a" }, 60_000); + + const m = await readFlags(prisma, SET_KEYS); + expect(m[FEATURE_FLAG.runOpsMintShardSetPrev]).toBe(""); + expect(typeof m[FEATURE_FLAG.runOpsMintShardSetFlippedAt]).toBe("string"); + }); + + postgresTest( + "a reordered list is not a change, so the clock is not reset", + async ({ prisma }) => { + await applyGlobalGracedFlips(prisma, { [FEATURE_FLAG.runOpsMintShardSet]: "a,b" }, 60_000); + const first = await readFlags(prisma, SET_KEYS); + + await applyGlobalGracedFlips(prisma, { [FEATURE_FLAG.runOpsMintShardSet]: "b,a" }, 60_000); + const second = await readFlags(prisma, SET_KEYS); + + expect(second[FEATURE_FLAG.runOpsMintShardSetFlippedAt]).toBe( + first[FEATURE_FLAG.runOpsMintShardSetFlippedAt] + ); + } + ); + + postgresTest("both graced groups stamp in ONE save", async ({ prisma }) => { + // A save that flips the kind and the list must not stamp one and lose the other. + await makeSetMultipleFlags(prisma)({ + [FEATURE_FLAG.runOpsMintKind]: "cuid", + [FEATURE_FLAG.runOpsMintShardSet]: "a", + }); + + await applyGlobalGracedFlips( + prisma, + { + [FEATURE_FLAG.runOpsMintKind]: "runOpsId", + [FEATURE_FLAG.runOpsMintShardSet]: "a,b", + }, + 60_000 + ); + + const m = await readFlags(prisma, [...SET_KEYS, ...MINT_KIND_KEYS]); + expect(m[FEATURE_FLAG.runOpsMintKindPrev]).toBe("cuid"); + expect(m[FEATURE_FLAG.runOpsMintShardSetPrev]).toBe("a"); + expect(typeof m[FEATURE_FLAG.runOpsMintKindFlippedAt]).toBe("string"); + expect(typeof m[FEATURE_FLAG.runOpsMintShardSetFlippedAt]).toBe("string"); + }); + + postgresTest("concurrent list changes do not interleave", async ({ prisma }) => { + await makeSetMultipleFlags(prisma)({ [FEATURE_FLAG.runOpsMintShardSet]: "a" }); + + await Promise.all([ + applyGlobalGracedFlips(prisma, { [FEATURE_FLAG.runOpsMintShardSet]: "a,b" }, 60_000), + applyGlobalGracedFlips(prisma, { [FEATURE_FLAG.runOpsMintShardSet]: "a,c" }, 60_000), + ]); + + const m = await readFlags(prisma, SET_KEYS); + const set = m[FEATURE_FLAG.runOpsMintShardSet]; + const prev = m[FEATURE_FLAG.runOpsMintShardSetPrev]; + + // The pair must be a consistent history, not a mix of the two writers. The winner's set is + // one of the two, and prev is what the OTHER writer left behind: either the original "a", or + // the loser's set when the loser committed first. "a,b" beside prev "a,b" would mean one + // writer read its own uncommitted state, and prev naming the winner's own set is incoherent. + expect(["a,b", "a,c"]).toContain(set); + expect(["a", "a,b", "a,c"]).toContain(prev); + expect(prev).not.toBe(set); + expect(typeof m[FEATURE_FLAG.runOpsMintShardSetFlippedAt]).toBe("string"); + }); +}); + +describe("replaceGlobalFeatureFlags — the admin page cannot bypass the stamp", () => { + postgresTest("a list change through the page is stamped", async ({ prisma }) => { + await makeSetMultipleFlags(prisma)({ [FEATURE_FLAG.runOpsMintShardSet]: "a" }); + + await replaceGlobalFeatureFlags(prisma, { + requestedFlags: { [FEATURE_FLAG.runOpsMintShardSet]: "a,b" }, + catalogKeys: CATALOG_KEYS, + ...SELF_HOSTED, + graceMs: 60_000, + }); + + const m = await readFlags(prisma, SET_KEYS); + expect(m[FEATURE_FLAG.runOpsMintShardSet]).toBe("a,b"); + expect(m[FEATURE_FLAG.runOpsMintShardSetPrev]).toBe("a"); + }); + + postgresTest("a body-supplied stamp is ignored and recomputed", async ({ prisma }) => { + await makeSetMultipleFlags(prisma)({ [FEATURE_FLAG.runOpsMintShardSet]: "a" }); + + await replaceGlobalFeatureFlags(prisma, { + requestedFlags: { + [FEATURE_FLAG.runOpsMintShardSet]: "a,b", + [FEATURE_FLAG.runOpsMintShardSetPrev]: "zzz", + [FEATURE_FLAG.runOpsMintShardSetFlippedAt]: "1999-01-01T00:00:00.000Z", + }, + catalogKeys: CATALOG_KEYS, + ...SELF_HOSTED, + graceMs: 60_000, + }); + + const m = await readFlags(prisma, SET_KEYS); + expect(m[FEATURE_FLAG.runOpsMintShardSetPrev]).toBe("a"); + expect(m[FEATURE_FLAG.runOpsMintShardSetFlippedAt]).not.toBe("1999-01-01T00:00:00.000Z"); + }); + + postgresTest("a co-submitted flag does not disturb a resubmitted list", async ({ prisma }) => { + await replaceGlobalFeatureFlags(prisma, { + requestedFlags: { [FEATURE_FLAG.runOpsMintShardSet]: "a,b" }, + catalogKeys: CATALOG_KEYS, + ...SELF_HOSTED, + graceMs: 60_000, + }); + const first = await readFlags(prisma, SET_KEYS); + + await replaceGlobalFeatureFlags(prisma, { + requestedFlags: { + [FEATURE_FLAG.runOpsMintShardSet]: "a,b", + [FEATURE_FLAG.mollifierEnabled]: true, + }, + catalogKeys: CATALOG_KEYS, + ...SELF_HOSTED, + graceMs: 60_000, + }); + + const m = await readFlags(prisma, SET_KEYS); + expect(m[FEATURE_FLAG.runOpsMintShardSet]).toBe("a,b"); + // Resubmitting the same list is not a flip, so the cutover clock is not reset. + expect(m[FEATURE_FLAG.runOpsMintShardSetFlippedAt]).toBe( + first[FEATURE_FLAG.runOpsMintShardSetFlippedAt] + ); + }); + + postgresTest( + "omitting the list DELETES it, so unset still turns minting off", + async ({ prisma }) => { + // The admin page's unset button omits the key. If the save skipped it, unset would be a + // silent no-op and gen-2 minting would stay armed. + await replaceGlobalFeatureFlags(prisma, { + requestedFlags: { [FEATURE_FLAG.runOpsMintShardSet]: "a,b" }, + catalogKeys: CATALOG_KEYS, + ...SELF_HOSTED, + graceMs: 60_000, + }); + + await replaceGlobalFeatureFlags(prisma, { + requestedFlags: {}, + catalogKeys: CATALOG_KEYS, + ...SELF_HOSTED, + graceMs: 60_000, + }); + + const m = await readFlags(prisma, SET_KEYS); + // The stamp goes with it: a stamp without its list keeps being served for the whole window. + expect(m[FEATURE_FLAG.runOpsMintShardSet]).toBeUndefined(); + expect(m[FEATURE_FLAG.runOpsMintShardSetPrev]).toBeUndefined(); + expect(m[FEATURE_FLAG.runOpsMintShardSetFlippedAt]).toBeUndefined(); + } + ); + + postgresTest("omitting the mint kind still deletes its trio", async ({ prisma }) => { + await replaceGlobalFeatureFlags(prisma, { + requestedFlags: { [FEATURE_FLAG.runOpsMintKind]: "runOpsId" }, + catalogKeys: CATALOG_KEYS, + ...SELF_HOSTED, + graceMs: 60_000, + }); + + await replaceGlobalFeatureFlags(prisma, { + requestedFlags: {}, + catalogKeys: CATALOG_KEYS, + ...SELF_HOSTED, + graceMs: 60_000, + }); + + const m = await readFlags(prisma, MINT_KIND_KEYS); + expect(m[FEATURE_FLAG.runOpsMintKind]).toBeUndefined(); + expect(m[FEATURE_FLAG.runOpsMintKindPrev]).toBeUndefined(); + expect(m[FEATURE_FLAG.runOpsMintKindFlippedAt]).toBeUndefined(); + }); + + postgresTest("unlocking does not orphan a stamp from its list", async ({ prisma }) => { + // With the lock off, a locked key may be swept. The stamps are locked, so this is the case + // where they could be deleted while the list survives, which would keep serving prevSet. + await replaceGlobalFeatureFlags(prisma, { + requestedFlags: { [FEATURE_FLAG.runOpsMintShardSet]: "a,b" }, + catalogKeys: CATALOG_KEYS, + ...SELF_HOSTED, + graceMs: 60_000, + }); + + await replaceGlobalFeatureFlags(prisma, { + requestedFlags: { [FEATURE_FLAG.runOpsMintShardSet]: "a,b" }, + catalogKeys: CATALOG_KEYS, + isManagedCloud: false, + unlockLockedFlags: true, + graceMs: 60_000, + }); + + const m = await readFlags(prisma, SET_KEYS); + expect(m[FEATURE_FLAG.runOpsMintShardSet]).toBe("a,b"); + expect(m[FEATURE_FLAG.runOpsMintShardSetPrev]).toBeDefined(); + expect(m[FEATURE_FLAG.runOpsMintShardSetFlippedAt]).toBeDefined(); + }); + + postgresTest("an ordinary flag keeps replace semantics", async ({ prisma }) => { + await makeSetMultipleFlags(prisma)({ [FEATURE_FLAG.mollifierEnabled]: true }); + + await replaceGlobalFeatureFlags(prisma, { + requestedFlags: {}, + catalogKeys: CATALOG_KEYS, + ...SELF_HOSTED, + graceMs: 60_000, + }); + + const m = await readFlags(prisma, [FEATURE_FLAG.mollifierEnabled]); + expect(m[FEATURE_FLAG.mollifierEnabled]).toBeUndefined(); + }); +}); diff --git a/knip.json b/knip.json index 84456756ca1..c6e8aee8977 100644 --- a/knip.json +++ b/knip.json @@ -25,7 +25,8 @@ "vite/node-globals-shim.js", "app/v3/otlpTransformWorker.ts" ], - "ignoreDependencies": ["@sentry/cli", "assert", "util"] + "ignoreDependencies": ["@sentry/cli", "assert", "util"], + "ignore": ["app/v3/runOpsMigration/runOpsMintShard.server.ts"] }, "internal-packages/dashboard-agent": { "entry": ["trigger.config.ts", "src/investigation-sweep.ts", "src/maintenance.ts"],