From e74cc3a9b3069cf80d07fbe87cee4cbd6d992f72 Mon Sep 17 00:00:00 2001 From: Milind Chatrabhuji Date: Mon, 26 Oct 2026 13:59:59 +0530 Subject: [PATCH] fix(feature-flags): preserve valid org flags when stored flags contain invalid values (#5058) --- .../components/admin/FeatureFlagsDialog.tsx | 8 +++++++- apps/webapp/app/v3/featureFlags.server.ts | 7 +++---- apps/webapp/test/storedOrgFlags.test.ts | 18 ++++++++++++++++++ 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/apps/webapp/app/components/admin/FeatureFlagsDialog.tsx b/apps/webapp/app/components/admin/FeatureFlagsDialog.tsx index 7e2a145f6df..b7c25cd2092 100644 --- a/apps/webapp/app/components/admin/FeatureFlagsDialog.tsx +++ b/apps/webapp/app/components/admin/FeatureFlagsDialog.tsx @@ -15,6 +15,7 @@ import { CheckboxWithLabel } from "~/components/primitives/Checkbox"; import { cn } from "~/utils/cn"; import { FEATURE_FLAG, + derivedFlagsClearedWith, ORG_LOCKED_FLAG_EFFECT, ORG_LOCKED_FLAGS, isUnchangedInvalidFlag, @@ -96,7 +97,9 @@ export function FeatureFlagsDialog({ const [saveError, setSaveError] = useState(null); const [unlocked, setUnlocked] = useState(false); - const isLocked = (key: string) => !unlocked && ORG_LOCKED_FLAGS.includes(key); + const isLocked = (key: string) => + (!unlocked || ORG_LOCKED_FLAG_EFFECT[key as FeatureFlagKey] === "derived") && + ORG_LOCKED_FLAGS.includes(key); useEffect(() => { if (open && orgId) { @@ -148,6 +151,9 @@ export function FeatureFlagsDialog({ setOverrides((prev) => { const next = { ...prev }; delete next[key]; + for (const derivedKey of derivedFlagsClearedWith(key)) { + delete next[derivedKey]; + } return next; }); }; diff --git a/apps/webapp/app/v3/featureFlags.server.ts b/apps/webapp/app/v3/featureFlags.server.ts index a519a5ed356..9216527ebad 100644 --- a/apps/webapp/app/v3/featureFlags.server.ts +++ b/apps/webapp/app/v3/featureFlags.server.ts @@ -7,6 +7,7 @@ import { FeatureFlagCatalog, GLOBAL_LOCKED_FLAGS, GRACED_FLAG_GROUPS, + parseStoredOrgFlags, validatePartialFeatureFlags, } from "~/v3/featureFlags"; import { env } from "~/env.server"; @@ -400,10 +401,8 @@ export function mergeOrgFeatureFlags( globalFlags: Partial, orgFeatureFlags: unknown ) { - const parsed = orgFeatureFlags - ? validatePartialFeatureFlags(orgFeatureFlags as Record) - : ({ success: false } as const); - return { ...globalFlags, ...(parsed.success ? parsed.data : {}) }; + const { valid } = parseStoredOrgFlags(orgFeatureFlags); + return { ...globalFlags, ...valid }; } /** diff --git a/apps/webapp/test/storedOrgFlags.test.ts b/apps/webapp/test/storedOrgFlags.test.ts index a3483e38b02..fcd51bb1c96 100644 --- a/apps/webapp/test/storedOrgFlags.test.ts +++ b/apps/webapp/test/storedOrgFlags.test.ts @@ -1,3 +1,4 @@ +import { mergeOrgFeatureFlags } from "~/v3/featureFlags.server"; import { describe, expect, it } from "vitest"; import { FEATURE_FLAG, @@ -94,3 +95,20 @@ describe("isUnchangedInvalidFlag", () => { expect(isUnchangedInvalidFlag(KEY, { [KEY]: { x: null } }, {})).toBe(false); }); }); + +describe("mergeOrgFeatureFlags", () => { + it("keeps valid org overrides when one stored flag value is invalid", () => { + const globalFlags = { + [FEATURE_FLAG.hasLogsPageAccess]: false, + [FEATURE_FLAG.realtimeBackend]: "redis" as const, + }; + const orgFeatureFlags = { + [FEATURE_FLAG.hasLogsPageAccess]: true, + [FEATURE_FLAG.realtimeBackend]: "not-a-backend", + }; + + const merged = mergeOrgFeatureFlags(globalFlags, orgFeatureFlags); + expect(merged[FEATURE_FLAG.hasLogsPageAccess]).toBe(true); + expect(merged[FEATURE_FLAG.realtimeBackend]).toBe("redis"); + }); +});