Skip to content

Commit ce00de1

Browse files
committed
fix(webapp): correct mint-flag grace baseline and lock down derived fields
An org's first per-org mint-flag override computed its cutover grace window against the wrong baseline, which could skip the grace window and briefly let two concurrent triggers sharing an idempotency key resolve to different databases. Seed the baseline from the effective global flag, strip the derived stamp fields from the request body, and apply the read-and-stamp atomically.
1 parent 4660a0a commit ce00de1

4 files changed

Lines changed: 232 additions & 76 deletions

File tree

apps/webapp/app/routes/admin.api.v1.orgs.$organizationId.feature-flags.ts

Lines changed: 62 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,12 @@ import { env } from "~/env.server";
66
import { prisma } from "~/db.server";
77
import { requireAdminApiRequest } from "~/services/personalAccessToken.server";
88
import { controlPlaneResolver } from "~/v3/runOpsMigration/controlPlaneResolver.server";
9-
import { stampMintKindFlip } from "~/v3/runOpsMigration/mintFlipGrace";
9+
import {
10+
selectMintBaselineSource,
11+
stampMintKindFlip,
12+
} from "~/v3/runOpsMigration/mintFlipGrace";
1013
import { validatePartialFeatureFlags } from "~/v3/featureFlags";
14+
import { flags as getGlobalFlags } from "~/v3/featureFlags.server";
1115

1216
const ParamsSchema = z.object({
1317
organizationId: z.string(),
@@ -51,20 +55,6 @@ export async function action({ request, params }: ActionFunctionArgs) {
5155

5256
const { organizationId } = ParamsSchema.parse(params);
5357

54-
const organization = await prisma.organization.findFirst({
55-
where: {
56-
id: organizationId,
57-
},
58-
select: {
59-
id: true,
60-
featureFlags: true,
61-
},
62-
});
63-
64-
if (!organization) {
65-
return json({ error: "Organization not found" }, { status: 404 });
66-
}
67-
6858
try {
6959
const body = await request.json();
7060

@@ -80,40 +70,66 @@ export async function action({ request, params }: ActionFunctionArgs) {
8070
);
8171
}
8272

83-
// Merge new flags with existing flags
84-
const existingFlags = organization.featureFlags
85-
? validatePartialFeatureFlags(organization.featureFlags as Record<string, unknown>)
86-
: { success: false as const };
87-
88-
// Stamp the flip from the control-plane DB clock so the grace-window cutover is anchored to one
89-
// authoritative time source, not whichever webapp process happened to handle this request.
90-
const [{ now: controlPlaneNow }] = await prisma.$queryRaw<{ now: Date }[]>`SELECT now() AS now`;
91-
92-
const mergedFlags = stampMintKindFlip(
93-
existingFlags.success ? existingFlags.data : {},
94-
{
95-
...(existingFlags.success ? existingFlags.data : {}),
96-
...validationResult.data,
97-
},
98-
controlPlaneNow.getTime(),
99-
env.RUN_OPS_MINT_FLIP_GRACE_MS
100-
);
73+
// Derived grace-stamp fields are computed server-side; never trust them from the body.
74+
const {
75+
runOpsMintKindPrev: _ignoredPrev,
76+
runOpsMintKindFlippedAt: _ignoredFlippedAt,
77+
...requestedFlags
78+
} = validationResult.data;
79+
80+
// Seed the flip baseline from the current GLOBAL mint flags so an org's FIRST per-org override
81+
// is graced from the currently-effective global kind, not the hardcoded default "cuid".
82+
const globalFlags = (await getGlobalFlags()) as Record<string, unknown>;
83+
84+
// Lock the org row for the whole read -> merge -> stamp -> write so a concurrent flag save
85+
// can't clobber the grace metadata (read-then-write race). PK lookup, one row, held to commit.
86+
const updatedOrganization = await prisma.$transaction(async (tx) => {
87+
const rows = await tx.$queryRaw<{ featureFlags: unknown }[]>`
88+
SELECT "featureFlags" FROM "Organization" WHERE "id" = ${organizationId} FOR UPDATE`;
89+
90+
if (rows.length === 0) {
91+
return null;
92+
}
93+
94+
const existingRaw = rows[0].featureFlags as Record<string, unknown> | null;
95+
const existingResult = existingRaw
96+
? validatePartialFeatureFlags(existingRaw)
97+
: ({ success: false } as const);
98+
const existingData = existingResult.success ? existingResult.data : {};
99+
100+
// Stamp the flip from the control-plane DB clock so the grace-window cutover is anchored to
101+
// one authoritative time source, not whichever webapp process handled this request.
102+
const [{ now: controlPlaneNow }] = await tx.$queryRaw<{ now: Date }[]>`SELECT now() AS now`;
103+
104+
const mergedFlags = stampMintKindFlip(
105+
selectMintBaselineSource(existingRaw, globalFlags),
106+
{
107+
...existingData,
108+
...requestedFlags,
109+
},
110+
controlPlaneNow.getTime(),
111+
env.RUN_OPS_MINT_FLIP_GRACE_MS
112+
);
101113

102-
// Update the organization's feature flags
103-
const updatedOrganization = await prisma.organization.update({
104-
where: {
105-
id: organizationId,
106-
},
107-
data: {
108-
featureFlags: mergedFlags as Prisma.InputJsonValue,
109-
},
110-
select: {
111-
id: true,
112-
slug: true,
113-
featureFlags: true,
114-
},
114+
return tx.organization.update({
115+
where: {
116+
id: organizationId,
117+
},
118+
data: {
119+
featureFlags: mergedFlags as Prisma.InputJsonValue,
120+
},
121+
select: {
122+
id: true,
123+
slug: true,
124+
featureFlags: true,
125+
},
126+
});
115127
});
116128

129+
if (!updatedOrganization) {
130+
return json({ error: "Organization not found" }, { status: 404 });
131+
}
132+
117133
// Org feature flags are embedded in every env of the org; drop all its cached env rows.
118134
controlPlaneResolver.invalidateOrganization(organizationId);
119135

apps/webapp/app/routes/admin.api.v2.orgs.$organizationId.feature-flags.ts

Lines changed: 64 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@ import { env } from "~/env.server";
66
import { prisma } from "~/db.server";
77
import { requireUser } from "~/services/session.server";
88
import { controlPlaneResolver } from "~/v3/runOpsMigration/controlPlaneResolver.server";
9-
import { stampMintKindFlip } from "~/v3/runOpsMigration/mintFlipGrace";
9+
import {
10+
selectMintBaselineSource,
11+
stampMintKindFlip,
12+
} from "~/v3/runOpsMigration/mintFlipGrace";
1013
import { flags as getGlobalFlags } from "~/v3/featureFlags.server";
1114
import {
1215
FEATURE_FLAG,
@@ -105,47 +108,78 @@ export async function action({ request, params }: ActionFunctionArgs) {
105108
return json({ error: "Invalid JSON body" }, { status: 400 });
106109
}
107110

108-
let featureFlags: typeof Prisma.JsonNull | Record<string, unknown>;
109-
110111
if (
111112
body === null ||
112113
(typeof body === "object" && !Array.isArray(body) && Object.keys(body).length === 0)
113114
) {
114-
featureFlags = Prisma.JsonNull;
115-
} else {
116-
const validationResult = validatePartialFeatureFlags(body as Record<string, unknown>);
117-
if (!validationResult.success) {
118-
return json(
119-
{ error: "Invalid feature flags", details: validationResult.error.issues },
120-
{ status: 400 }
121-
);
115+
// Clear all flags. No grace stamp (nothing to flip) and no read-then-write race.
116+
try {
117+
await prisma.organization.update({
118+
where: { id: organizationId },
119+
data: { featureFlags: Prisma.JsonNull },
120+
});
121+
} catch (e) {
122+
if (e instanceof Prisma.PrismaClientKnownRequestError && e.code === "P2025") {
123+
throw new Response("Organization not found", { status: 404 });
124+
}
125+
throw e;
122126
}
123-
featureFlags = validationResult.data;
124127

125-
const existingOrg = await prisma.organization.findFirst({
126-
where: { id: organizationId },
127-
select: { featureFlags: true },
128-
});
129-
// Anchor the flip stamp to the control-plane DB clock (see the v1 route) rather than this process's.
130-
const [{ now: controlPlaneNow }] = await prisma.$queryRaw<{ now: Date }[]>`SELECT now() AS now`;
131-
featureFlags = stampMintKindFlip(
132-
existingOrg?.featureFlags as Record<string, unknown> | null | undefined,
133-
featureFlags,
128+
controlPlaneResolver.invalidateOrganization(organizationId);
129+
return json({ success: true });
130+
}
131+
132+
const validationResult = validatePartialFeatureFlags(body as Record<string, unknown>);
133+
if (!validationResult.success) {
134+
return json(
135+
{ error: "Invalid feature flags", details: validationResult.error.issues },
136+
{ status: 400 }
137+
);
138+
}
139+
140+
// Derived grace-stamp fields are computed server-side; never trust them from the body.
141+
const {
142+
runOpsMintKindPrev: _ignoredPrev,
143+
runOpsMintKindFlippedAt: _ignoredFlippedAt,
144+
...requestedFlags
145+
} = validationResult.data;
146+
147+
// Seed the flip baseline from the current GLOBAL mint flags so an org's FIRST per-org override
148+
// is graced from the currently-effective global kind, not the hardcoded default "cuid".
149+
const globalFlags = (await getGlobalFlags()) as Record<string, unknown>;
150+
151+
// Lock the org row for the whole read -> stamp -> write so a concurrent flag save can't clobber
152+
// the grace metadata (read-then-write race). PK lookup, one row, held to commit.
153+
const updated = await prisma.$transaction(async (tx) => {
154+
const rows = await tx.$queryRaw<{ featureFlags: unknown }[]>`
155+
SELECT "featureFlags" FROM "Organization" WHERE "id" = ${organizationId} FOR UPDATE`;
156+
157+
if (rows.length === 0) {
158+
return false;
159+
}
160+
161+
const existingRaw = rows[0].featureFlags as Record<string, unknown> | null;
162+
163+
// Anchor the flip stamp to the control-plane DB clock (see the v1 route), not this process's.
164+
const [{ now: controlPlaneNow }] = await tx.$queryRaw<{ now: Date }[]>`SELECT now() AS now`;
165+
166+
const stamped = stampMintKindFlip(
167+
selectMintBaselineSource(existingRaw, globalFlags),
168+
requestedFlags,
134169
controlPlaneNow.getTime(),
135170
env.RUN_OPS_MINT_FLIP_GRACE_MS
136171
);
137-
}
138172

139-
try {
140-
await prisma.organization.update({
173+
await tx.organization.update({
141174
where: { id: organizationId },
142-
data: { featureFlags: featureFlags as Prisma.InputJsonValue },
175+
data: { featureFlags: stamped as Prisma.InputJsonValue },
143176
});
144-
} catch (e) {
145-
if (e instanceof Prisma.PrismaClientKnownRequestError && e.code === "P2025") {
146-
throw new Response("Organization not found", { status: 404 });
147-
}
148-
throw e;
177+
178+
return true;
179+
});
180+
181+
if (!updated) {
182+
throw new Response("Organization not found", { status: 404 });
149183
}
150184

151185
// Org feature flags are embedded in every env of the org; drop all its cached env rows.

apps/webapp/app/v3/runOpsMigration/mintFlipGrace.test.ts

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
22
import {
33
effectiveMintKind,
4+
readMintResolution,
45
resolveMintFlag,
6+
selectMintBaselineSource,
57
stampMintKindFlip,
68
type MintFlagResolution,
79
} from "./mintFlipGrace";
@@ -229,3 +231,92 @@ describe("resolveMintFlag", () => {
229231
});
230232
});
231233
});
234+
235+
// #3b: an org's FIRST per-org runOpsMintKind override must be stamped against the currently
236+
// EFFECTIVE kind — the global FeatureFlag resolution when the org has no override yet — not the
237+
// hardcoded default "cuid". selectMintBaselineSource picks that same source (per-org blob if it
238+
// sets runOpsMintKind, else the global rows) so stampMintKindFlip's baseline (storedKind +
239+
// prev + carry-forward) is correct.
240+
describe("selectMintBaselineSource", () => {
241+
it("returns the per-org blob when it sets runOpsMintKind (override owns the baseline)", () => {
242+
const perOrg = { runOpsMintKind: "cuid" };
243+
const global = { runOpsMintKind: "runOpsId" };
244+
expect(selectMintBaselineSource(perOrg, global)).toBe(perOrg);
245+
});
246+
247+
it("falls back to the global rows when the org has no runOpsMintKind override", () => {
248+
const perOrg = { someOtherFlag: true };
249+
const global = { runOpsMintKind: "runOpsId" };
250+
expect(selectMintBaselineSource(perOrg, global)).toBe(global);
251+
});
252+
253+
it("returns an empty record when neither source sets a kind", () => {
254+
expect(selectMintBaselineSource(null, null)).toEqual({});
255+
expect(selectMintBaselineSource({ someOtherFlag: true }, null)).toEqual({});
256+
});
257+
});
258+
259+
describe("first per-org override stamps prev against the effective GLOBAL kind (#3b)", () => {
260+
beforeEach(() => vi.useFakeTimers());
261+
afterEach(() => vi.useRealTimers());
262+
263+
it("global=runOpsId, org's FIRST override -> cuid: genuine flip, prev=runOpsId, graced", () => {
264+
const globalFlags = { runOpsMintKind: "runOpsId" };
265+
const orgExisting = {}; // no per-org override yet
266+
const outgoing = { runOpsMintKind: "cuid" };
267+
const result = stampMintKindFlip(
268+
selectMintBaselineSource(orgExisting, globalFlags),
269+
outgoing,
270+
T,
271+
GRACE_MS
272+
);
273+
274+
expect(result.runOpsMintKind).toBe("cuid");
275+
// Previously stamped prev="cuid" (the hardcoded default) OR skipped the flip entirely; must
276+
// now be "runOpsId" (the effective global kind) so the org serves it through the grace window.
277+
expect(result.runOpsMintKindPrev).toBe("runOpsId");
278+
expect(result.runOpsMintKindFlippedAt).toBe(new Date(T).toISOString());
279+
280+
const resolution = readMintResolution(result);
281+
expect(effectiveMintKind(resolution, T + GRACE_MS - 1, GRACE_MS)).toBe("runOpsId");
282+
expect(effectiveMintKind(resolution, T + GRACE_MS, GRACE_MS)).toBe("cuid");
283+
});
284+
285+
it("global=runOpsId, org's FIRST override -> runOpsId (redundant): NOT a spurious flip, no phantom regression to cuid", () => {
286+
const globalFlags = { runOpsMintKind: "runOpsId" };
287+
const orgExisting = {};
288+
const outgoing = { runOpsMintKind: "runOpsId" };
289+
const result = stampMintKindFlip(
290+
selectMintBaselineSource(orgExisting, globalFlags),
291+
outgoing,
292+
T,
293+
GRACE_MS
294+
);
295+
296+
expect(result.runOpsMintKind).toBe("runOpsId");
297+
// Previously: storedKind defaulted to "cuid", so this looked like a genuine flip and stamped
298+
// prev="cuid" — making the org serve cuid during a phantom grace window (a regression).
299+
expect(result.runOpsMintKindPrev).toBeUndefined();
300+
expect(result.runOpsMintKindFlippedAt).toBeUndefined();
301+
});
302+
303+
it("mid-grace global flip carried into a redundant org override keeps the global stamp", () => {
304+
const globalFlags = {
305+
runOpsMintKind: "runOpsId",
306+
runOpsMintKindPrev: "cuid",
307+
runOpsMintKindFlippedAt: new Date(T).toISOString(),
308+
};
309+
const orgExisting = {};
310+
const outgoing = { runOpsMintKind: "runOpsId" };
311+
const result = stampMintKindFlip(
312+
selectMintBaselineSource(orgExisting, globalFlags),
313+
outgoing,
314+
T + 10_000,
315+
GRACE_MS
316+
);
317+
318+
expect(result.runOpsMintKind).toBe("runOpsId");
319+
expect(result.runOpsMintKindPrev).toBe("cuid");
320+
expect(result.runOpsMintKindFlippedAt).toBe(new Date(T).toISOString());
321+
});
322+
});

apps/webapp/app/v3/runOpsMigration/mintFlipGrace.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,21 @@ export function resolveMintFlag(
5858
return readMintResolution(globalFlags);
5959
}
6060

61+
// Picks the flag record that currently determines an org's effective mint kind: the per-org
62+
// override blob when it sets runOpsMintKind, otherwise the global FeatureFlag rows. Same source
63+
// resolveMintFlag() reads, but returned as a record so it can seed stampMintKindFlip's baseline
64+
// (storedKind for flip-detection, prev on a genuine flip, and stamp carry-forward). This makes an
65+
// org's first per-org override stamp against the effective GLOBAL kind, not the default "cuid".
66+
export function selectMintBaselineSource(
67+
perOrgOverrides: Record<string, unknown> | null | undefined,
68+
globalFlags: Record<string, unknown> | null | undefined
69+
): Record<string, unknown> {
70+
if (readMintKind(perOrgOverrides ?? {}, "runOpsMintKind") !== undefined) {
71+
return perOrgOverrides ?? {};
72+
}
73+
return globalFlags ?? {};
74+
}
75+
6176
function resolveEffectiveFromFlags(
6277
flags: Record<string, unknown> | null | undefined,
6378
nowMs: number,

0 commit comments

Comments
 (0)