Merge remote-tracking branch 'origin/main' into feat/waitpoint-store-coordinator-tri-13440

Author: d-cs

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<string, unknown>;
+  storedValues: Record<string, unknown>;
+  newValues: Record<string, unknown>;
+}): FlagChange[] {
+  const { editableKeys, initialValues, storedValues, newValues } = params;
+
+  return editableKeys.flatMap<FlagChange>((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<FlagChange>((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);
+}

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,

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<string, unknown>;
+  storedValues: Record<string, unknown>;
   newValues: Record<string, unknown>;
   controlTypes: Record<string, FlagControlType>;
   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<Change>((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 (