fix(webapp): disclose cascaded stamp deletes, and write only changed flags

Two defects this branch introduced.

The confirm dialog understated a deletion. Unsetting a graced primary clears
its two stamps, and this branch moved those stamps into the locked set, so
they left the page's editable keys and the change list stopped mentioning
them. Three rows were deleted and one was shown. Before this branch the
stamps were editable, so all three appeared.

The change list moves into buildFlagChangeList, which adds the cascade. Only
an unset cascades: a change re-stamps instead. A stamp that is not stored is
not listed. The key topology moves to the shared flag module, since the page
and the server both need it and a second copy would drift.

The save also wrote every submitted flag. Stamping needs read-then-write, so
this branch replaced a batch transaction with an interactive one, where each
upsert is its own round trip against the interactive timeout. It now reads
the submitted keys once and writes only the values that differ, so a typical
save costs two round trips rather than one per flag.
This commit is contained in:
Daniel Sutton
2026-08-24 11:50:33 +01:00
parent 8c40565a6e
commit 4edaee2eec
5 changed files with 256 additions and 51 deletions
@@ -0,0 +1,53 @@
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 they never
* appear in `editableKeys`, and listing only the editable keys understated the deletion.
*/
export function buildFlagChangeList(params: {
editableKeys: readonly string[];
lockedKeys: readonly string[];
initialValues: Record<string, unknown>;
newValues: Record<string, unknown>;
}): FlagChange[] {
const { editableKeys, initialValues, 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 initialValues)
.map<FlagChange>((derived) => ({
key: derived,
type: "removed",
oldVal: String(initialValues[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);
}
+2 -29
View File
@@ -30,6 +30,7 @@ import {
DialogFooter,
} from "~/components/primitives/Dialog";
import { cn } from "~/utils/cn";
import { buildFlagChangeList } from "~/components/admin/flagChangeList";
import {
UNSET_VALUE,
BooleanControl,
@@ -471,35 +472,7 @@ 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, newValues });
return (
<Dialog open={open} onOpenChange={onOpenChange}>
+40 -22
View File
@@ -5,6 +5,7 @@ import {
type FeatureFlagCatalogSchema,
type FeatureFlagKey,
FeatureFlagCatalog,
GRACED_FLAG_GROUPS,
} from "~/v3/featureFlags";
import { stampMintKindFlip } from "~/v3/runOpsMigration/mintFlipGrace";
import { stampMintShardSetFlip } from "~/v3/runOpsMigration/mintShardGrace";
@@ -180,27 +181,12 @@ export function makeSetMultipleFlags(_prisma: PrismaClientOrTransaction = prisma
};
}
// Global flag groups whose value carries its own grace stamp. `primary` is operator-supplied;
// `derived` is computed server-side and must never be written from a request body. The pair is
// named explicitly rather than by position, so a group declared in another order stays correct.
const GRACED_GLOBAL_GROUPS = [
{
primary: FEATURE_FLAG.runOpsMintKind as FeatureFlagKey,
derived: [
FEATURE_FLAG.runOpsMintKindPrev,
FEATURE_FLAG.runOpsMintKindFlippedAt,
] as FeatureFlagKey[],
stamp: stampMintKindFlip,
},
{
primary: FEATURE_FLAG.runOpsMintShardSet as FeatureFlagKey,
derived: [
FEATURE_FLAG.runOpsMintShardSetPrev,
FEATURE_FLAG.runOpsMintShardSetFlippedAt,
] as FeatureFlagKey[],
stamp: stampMintShardSetFlip,
},
] as const;
// 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 GRACED_GLOBAL_KEYS: FeatureFlagKey[] = GRACED_GLOBAL_GROUPS.flatMap((g) => [
g.primary,
@@ -218,6 +204,21 @@ export function touchesGracedGroup(requestedFlags: Record<string, unknown>): boo
}
// 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<string, unknown>,
existing: Record<string, unknown>
): Record<string, unknown> {
const out: Record<string, unknown> = {};
for (const [key, value] of Object.entries(requested)) {
if (JSON.stringify(existing[key] ?? null) !== JSON.stringify(value ?? null)) {
out[key] = value;
}
}
return out;
}
export function withoutDerivedKeys(
requestedFlags: Partial<z.infer<typeof FeatureFlagCatalogSchema>>
): Record<string, unknown> {
@@ -339,7 +340,24 @@ export async function replaceGlobalFeatureFlags(
}
}
await makeSetMultipleFlags(tx)(toWrite as Partial<z.infer<typeof FeatureFlagCatalogSchema>>);
// 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<string, unknown> = {};
for (const row of storedRows) {
stored[row.key] = row.value;
}
await makeSetMultipleFlags(tx)(
flagsNeedingWrite(toWrite, stored) as Partial<z.infer<typeof FeatureFlagCatalogSchema>>
);
}
if (keysToDelete.length > 0) {
await tx.featureFlag.deleteMany({ where: { key: { in: boundedIn(keysToDelete) } } });
+25
View File
@@ -190,6 +190,31 @@ export const ORG_LOCKED_FLAGS: FeatureFlagKey[] = [
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
@@ -0,0 +1,136 @@
// 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" },
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.
const changes = buildFlagChangeList({
editableKeys: EDITABLE,
lockedKeys: LOCKED,
initialValues: {
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" },
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", 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: { 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(
{}
);
});
});