Skip to content

Commit 24f12e7

Browse files
d-csclaude
d-cs
and
claude
committed
fix(webapp): only maintain the cohort dial map for enrolled orgs
The map-maintenance jsonb_set ran on every org flag save, so an unrelated admin save on a never-enrolled org wrote its entry as off. The resolver reads a present off as a per-org opt-out that beats the global dial, which would silently pin that org off the fleet rollout once the global dial was raised. Gate the map write (and its zero-rows guard) on the one-way enrollment latch in the blob being written, in all three write paths (v1, v2 save, v2 clear-all), so only a genuinely enrolled org is recorded. Cover the never-enrolled skip and the enrolled-maintain cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 137e6de commit 24f12e7

3 files changed

Lines changed: 85 additions & 37 deletions

File tree

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

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -149,20 +149,24 @@ export async function action({ request, params }: ActionFunctionArgs) {
149149
},
150150
});
151151

152-
// Maintain this org's entry in the global cohort dial map with a single atomic jsonb_set (no
153-
// read-modify-write of the map, so concurrent org saves can't clobber each other). Presence
154-
// is the one-way enrollment latch; "off" is a stored value, never a deletion.
155-
const orgDialParsed = FeatureFlagCatalog[FEATURE_FLAG.snapshotStoreOrgMode].safeParse(
156-
mergedFlags[FEATURE_FLAG.snapshotStoreOrgMode]
157-
);
158-
const orgDial = orgDialParsed.success ? orgDialParsed.data : "off";
159-
const affected = await tx.$executeRaw`
160-
UPDATE "FeatureFlag"
161-
SET "value" = jsonb_set(COALESCE("value", '{}'::jsonb), ARRAY[${organizationId}], to_jsonb(${orgDial}::text)),
162-
"updatedAt" = now()
163-
WHERE "key" = ${FEATURE_FLAG.snapshotStoreOrgDials}`;
164-
if (affected === 0) {
165-
throw new Error("snapshotStoreOrgDials flag row missing; run the backfill migration");
152+
// Maintain the cohort map ONLY for an enrolled org (latch set in the stamped blob). Writing a
153+
// never-enrolled org as "off" would auto-enroll it: the resolver reads a present "off" as an
154+
// opt-out that beats the global dial, silently pinning the org off the fleet rollout. Single
155+
// atomic jsonb_set; "off" is a stored value for a genuinely-enrolled org, never a deletion.
156+
const enrolled = mergedFlags[FEATURE_FLAG.snapshotStoreOrgEverEnabled] === true;
157+
if (enrolled) {
158+
const orgDialParsed = FeatureFlagCatalog[FEATURE_FLAG.snapshotStoreOrgMode].safeParse(
159+
mergedFlags[FEATURE_FLAG.snapshotStoreOrgMode]
160+
);
161+
const orgDial = orgDialParsed.success ? orgDialParsed.data : "off";
162+
const affected = await tx.$executeRaw`
163+
UPDATE "FeatureFlag"
164+
SET "value" = jsonb_set(COALESCE("value", '{}'::jsonb), ARRAY[${organizationId}], to_jsonb(${orgDial}::text)),
165+
"updatedAt" = now()
166+
WHERE "key" = ${FEATURE_FLAG.snapshotStoreOrgDials}`;
167+
if (affected === 0) {
168+
throw new Error("snapshotStoreOrgDials flag row missing; run the backfill migration");
169+
}
166170
}
167171

168172
return updated;

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

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -137,15 +137,19 @@ export async function action({ request, params }: ActionFunctionArgs) {
137137
},
138138
});
139139

140-
// A wipe clears the org's dial, so record "off" in the cohort map. Single atomic jsonb_set;
141-
// the entry is never deleted, so presence keeps recording the org's one-way enrollment.
142-
const affected = await tx.$executeRaw`
143-
UPDATE "FeatureFlag"
144-
SET "value" = jsonb_set(COALESCE("value", '{}'::jsonb), ARRAY[${organizationId}], to_jsonb('off'::text)),
145-
"updatedAt" = now()
146-
WHERE "key" = ${FEATURE_FLAG.snapshotStoreOrgDials}`;
147-
if (affected === 0) {
148-
throw new Error("snapshotStoreOrgDials flag row missing; run the backfill migration");
140+
// A wipe clears the org's dial, so record "off" in the cohort map, but ONLY for an org still
141+
// enrolled after the wipe (clearedOrgFlagsPreservingLatch keeps the latch for one that was).
142+
// A never-enrolled org must not be auto-enrolled as "off" here (see the main save path).
143+
const enrolled = preserved?.[FEATURE_FLAG.snapshotStoreOrgEverEnabled] === true;
144+
if (enrolled) {
145+
const affected = await tx.$executeRaw`
146+
UPDATE "FeatureFlag"
147+
SET "value" = jsonb_set(COALESCE("value", '{}'::jsonb), ARRAY[${organizationId}], to_jsonb('off'::text)),
148+
"updatedAt" = now()
149+
WHERE "key" = ${FEATURE_FLAG.snapshotStoreOrgDials}`;
150+
if (affected === 0) {
151+
throw new Error("snapshotStoreOrgDials flag row missing; run the backfill migration");
152+
}
149153
}
150154

151155
return true;
@@ -221,20 +225,24 @@ export async function action({ request, params }: ActionFunctionArgs) {
221225
data: { featureFlags: stamped as Prisma.InputJsonValue },
222226
});
223227

224-
// Maintain this org's entry in the global cohort dial map with a single atomic jsonb_set (no
225-
// read-modify-write of the map, so concurrent org saves can't clobber each other). Presence
226-
// is the one-way enrollment latch; "off" is a stored value, never a deletion.
227-
const orgDialParsed = FeatureFlagCatalog[FEATURE_FLAG.snapshotStoreOrgMode].safeParse(
228-
stamped[FEATURE_FLAG.snapshotStoreOrgMode]
229-
);
230-
const orgDial = orgDialParsed.success ? orgDialParsed.data : "off";
231-
const affected = await tx.$executeRaw`
232-
UPDATE "FeatureFlag"
233-
SET "value" = jsonb_set(COALESCE("value", '{}'::jsonb), ARRAY[${organizationId}], to_jsonb(${orgDial}::text)),
234-
"updatedAt" = now()
235-
WHERE "key" = ${FEATURE_FLAG.snapshotStoreOrgDials}`;
236-
if (affected === 0) {
237-
throw new Error("snapshotStoreOrgDials flag row missing; run the backfill migration");
228+
// Maintain the cohort map ONLY for an enrolled org (latch set in the stamped blob). Writing a
229+
// never-enrolled org as "off" would auto-enroll it: the resolver reads a present "off" as an
230+
// opt-out that beats the global dial, silently pinning the org off the fleet rollout. Single
231+
// atomic jsonb_set; "off" is a stored value for a genuinely-enrolled org, never a deletion.
232+
const enrolled = stamped[FEATURE_FLAG.snapshotStoreOrgEverEnabled] === true;
233+
if (enrolled) {
234+
const orgDialParsed = FeatureFlagCatalog[FEATURE_FLAG.snapshotStoreOrgMode].safeParse(
235+
stamped[FEATURE_FLAG.snapshotStoreOrgMode]
236+
);
237+
const orgDial = orgDialParsed.success ? orgDialParsed.data : "off";
238+
const affected = await tx.$executeRaw`
239+
UPDATE "FeatureFlag"
240+
SET "value" = jsonb_set(COALESCE("value", '{}'::jsonb), ARRAY[${organizationId}], to_jsonb(${orgDial}::text)),
241+
"updatedAt" = now()
242+
WHERE "key" = ${FEATURE_FLAG.snapshotStoreOrgDials}`;
243+
if (affected === 0) {
244+
throw new Error("snapshotStoreOrgDials flag row missing; run the backfill migration");
245+
}
238246
}
239247

240248
return true;

‎apps/webapp/test/adminOrgFeatureFlagsDials.test.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ import { action } from "~/routes/admin.api.v1.orgs.$organizationId.feature-flags
4444

4545
const MODE = FEATURE_FLAG.snapshotStoreOrgMode;
4646
const DIALS = FEATURE_FLAG.snapshotStoreOrgDials;
47+
// An org flag unrelated to the snapshot dial, used to prove an ordinary save never enrolls an org.
48+
const UNRELATED = FEATURE_FLAG.hasAiAccess;
4749

4850
let orgSeq = 0;
4951

@@ -104,6 +106,40 @@ describe("admin org feature-flags route maintains the snapshotStoreOrgDials coho
104106
expect(dials?.[id]).toBe("redis-read");
105107
});
106108

109+
postgresTest(
110+
"an unrelated save on a never-enrolled org leaves it out of the map",
111+
async ({ prisma }) => {
112+
await seedDialsRow(prisma);
113+
const id = await seedOrg(prisma);
114+
115+
// No snapshotStoreOrgMode: the one-way latch is never stamped, so the org is not enrolled and
116+
// must NOT be auto-written as "off" (which the resolver would read as an opt-out beating the
117+
// global dial, pinning the org off the fleet rollout).
118+
const response = await post(id, { [UNRELATED]: true });
119+
120+
expect(response.status).toBe(200);
121+
const dials = await readDials(prisma);
122+
expect(Object.prototype.hasOwnProperty.call(dials ?? {}, id)).toBe(false);
123+
expect(dials?.[id]).toBeUndefined();
124+
}
125+
);
126+
127+
postgresTest(
128+
"an unrelated save on an already-enrolled org maintains its entry",
129+
async ({ prisma }) => {
130+
await seedDialsRow(prisma);
131+
const id = await seedOrg(prisma);
132+
133+
// Enroll first (stamps the latch), then a later unrelated save keeps the entry at its dial.
134+
await post(id, { [MODE]: "redis-read" });
135+
const response = await post(id, { [UNRELATED]: true });
136+
137+
expect(response.status).toBe(200);
138+
const dials = await readDials(prisma);
139+
expect(dials?.[id]).toBe("redis-read");
140+
}
141+
);
142+
107143
postgresTest(
108144
"a later save to off stores off and keeps the entry present",
109145
async ({ prisma }) => {

0 commit comments

Comments
 (0)