feat(remote-feature-flag-controller): return threshold values directly and track threshold groups

packages/remote-feature-flag-controller/CHANGELOG.md

@@ -7,8 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Add optional `featureFlagThresholdGroups` field to `RemoteFeatureFlagControllerState` to map feature flag names to their selected threshold group names ([#9289](https://github.com/MetaMask/core/pull/9289))
+
 ### Changed
 
+- **BREAKING:** Threshold feature flags now return the selected `value` directly instead of a `{ name, value }` wrapper. The selected threshold group name is stored separately in `featureFlagThresholdGroups` on controller state when the selected threshold entry includes `name` ([#9289](https://github.com/MetaMask/core/pull/9289))
 - Merge `localOverrides` into `remoteFeatureFlags` at the controller level so consumers receive effective flag values directly ([#9259](https://github.com/MetaMask/core/pull/9259))
 - Bump `@metamask/utils` from `^11.9.0` to `^11.11.0` ([#9074](https://github.com/MetaMask/core/pull/9074))
 - Bump `@metamask/controller-utils` from `^12.1.0` to `^12.3.0` ([#9058](https://github.com/MetaMask/core/pull/9058), [#9083](https://github.com/MetaMask/core/pull/9083), [#9218](https://github.com/MetaMask/core/pull/9218))

packages/remote-feature-flag-controller/src/remote-feature-flag-controller.test.ts

@@ -478,83 +478,15 @@ describe('RemoteFeatureFlagController', () => {
 
       // With MOCK_METRICS_ID + 'testFlagForThreshold' hashed:
       // Threshold = 0.380673, which falls in groupB range (0.3 < t <= 0.5)
-      expect(
-        controller.state.remoteFeatureFlags.testFlagForThreshold,
-      ).toStrictEqual({
-        name: 'groupB',
-        value: 'valueB',
-      });
-    });
-
-    it('preserves selected legacy threshold object value wrappers', async () => {
-      const thresholdFlagValue = {
-        enabled: true,
-        minimumVersion: '13.10.0',
-        attemptsMax: 5,
-      };
-      const mockFlags = {
-        thresholdObjectFlag: [
-          {
-            name: 'enabled',
-            scope: { type: 'threshold', value: 1.0 },
-            value: thresholdFlagValue,
-          },
-        ],
-      };
-      const clientConfigApiService = buildClientConfigApiService({
-        remoteFeatureFlags: mockFlags,
-      });
-      const { controller, messenger } = createController({
-        clientConfigApiService,
-        getMetaMetricsId: () => MOCK_METRICS_ID,
-      });
-
-      await messenger.call(
-        'RemoteFeatureFlagController:updateRemoteFeatureFlags',
+      expect(controller.state.remoteFeatureFlags.testFlagForThreshold).toBe(
+        'valueB',
       );
-
-      expect(
-        controller.state.remoteFeatureFlags.thresholdObjectFlag,
-      ).toStrictEqual({
-        name: 'enabled',
-        value: thresholdFlagValue,
+      expect(controller.state.featureFlagThresholdGroups).toStrictEqual({
+        testFlagForThreshold: 'groupB',
       });
     });
 
-    it('returns selected threshold version 2 values without wrapper metadata', async () => {
-      const thresholdFlagValue = {
-        enabled: true,
-        minimumVersion: '13.10.0',
-        attemptsMax: 5,
-      };
-      const mockFlags = {
-        thresholdObjectFlag: [
-          {
-            thresholdName: 'enabled',
-            thresholdVersion: ThresholdVersion.DirectValue,
-            scope: { type: 'threshold', value: 1.0 },
-            value: thresholdFlagValue,
-          },
-        ],
-      };
-      const clientConfigApiService = buildClientConfigApiService({
-        remoteFeatureFlags: mockFlags,
-      });
-      const { controller, messenger } = createController({
-        clientConfigApiService,
-        getMetaMetricsId: () => MOCK_METRICS_ID,
-      });
-
-      await messenger.call(
-        'RemoteFeatureFlagController:updateRemoteFeatureFlags',
-      );
-
-      expect(
-        controller.state.remoteFeatureFlags.thresholdObjectFlag,
-      ).toStrictEqual(thresholdFlagValue);
-    });
-
-    it('falls back to legacy threshold wrappers for unrecognized threshold versions', async () => {
+    it('ignores threshold version field, processing threshold values normally', async () => {
       const thresholdFlagValue = {
         enabled: true,
       };
@@ -582,9 +514,9 @@ describe('RemoteFeatureFlagController', () => {
 
       expect(
         controller.state.remoteFeatureFlags.thresholdObjectFlag,
-      ).toStrictEqual({
-        name: 'enabled',
-        value: thresholdFlagValue,
+      ).toStrictEqual(thresholdFlagValue);
+      expect(controller.state.featureFlagThresholdGroups).toStrictEqual({
+        thresholdObjectFlag: 'enabled',
       });
     });
 
@@ -649,9 +581,13 @@ describe('RemoteFeatureFlagController', () => {
       // Assert - User gets different groups because each flag uses unique seed
       const { featureA, featureB } = controller.state.remoteFeatureFlags;
       // featureA: hash(MOCK_METRICS_ID + 'featureA') → threshold 0.966682 → groupA2
-      expect(featureA).toStrictEqual({ name: 'groupA2', value: 'A2' });
+      expect(featureA).toBe('A2');
       // featureB: hash(MOCK_METRICS_ID + 'featureB') → threshold 0.398654 → groupB1
-      expect(featureB).toStrictEqual({ name: 'groupB1', value: 'B1' });
+      expect(featureB).toBe('B1');
+      expect(controller.state.featureFlagThresholdGroups).toStrictEqual({
+        featureA: 'groupA2',
+        featureB: 'groupB1',
+      });
       // Different groups proves independence!
     });
 
@@ -711,9 +647,11 @@ describe('RemoteFeatureFlagController', () => {
       );
 
       // Assert - Invalid item skipped, valid item selected
-      expect(controller.state.remoteFeatureFlags.mixedArray).toStrictEqual({
-        name: 'validGroup',
-        value: 'selectedValue',
+      expect(controller.state.remoteFeatureFlags.mixedArray).toBe(
+        'selectedValue',
+      );
+      expect(controller.state.featureFlagThresholdGroups).toStrictEqual({
+        mixedArray: 'validGroup',
       });
     });
 
@@ -761,7 +699,10 @@ describe('RemoteFeatureFlagController', () => {
       // Assert - Same user always gets same group (deterministic)
       // testFlag: hash(MOCK_METRICS_ID + 'testFlag') → threshold 0.496587 → control
       expect(firstResult).toStrictEqual(secondResult);
-      expect(firstResult).toStrictEqual({ name: 'control', value: false });
+      expect(firstResult).toBe(false);
+      expect(controller1.state.featureFlagThresholdGroups).toStrictEqual({
+        testFlag: 'control',
+      });
     });
   });
 
@@ -1073,8 +1014,11 @@ describe('RemoteFeatureFlagController', () => {
       // With MOCK_METRICS_ID + 'multiVersionABFlag' hashed:
       // Threshold = 0.094878, which falls in groupA range (t <= 0.3)
       expect(multiVersionABFlag).toStrictEqual({
-        name: 'groupA',
-        value: { feature: 'A', enabled: true },
+        feature: 'A',
+        enabled: true,
+      });
+      expect(controller.state.featureFlagThresholdGroups).toStrictEqual({
+        multiVersionABFlag: 'groupA',
       });
       expect(regularFlag).toBe(true);
     });
@@ -1406,6 +1350,75 @@ describe('RemoteFeatureFlagController', () => {
       jest.useRealTimers();
     });
 
+    it('removes stale featureFlagThresholdGroups entries when flags are removed from server', async () => {
+      jest.useRealTimers();
+      const clientConfigApiService = buildClientConfigApiService({
+        remoteFeatureFlags: {
+          flagA: [
+            {
+              name: 'groupA',
+              thresholdVersion: ThresholdVersion.DirectValue,
+              scope: { type: 'threshold', value: 1.0 },
+              value: true,
+            },
+          ],
+          flagB: [
+            {
+              name: 'groupB',
+              thresholdVersion: ThresholdVersion.DirectValue,
+              scope: { type: 'threshold', value: 1.0 },
+              value: false,
+            },
+          ],
+        },
+      });
+      const { controller, messenger } = createController({
+        clientConfigApiService,
+        getMetaMetricsId: () => MOCK_METRICS_ID,
+      });
+
+      await messenger.call(
+        'RemoteFeatureFlagController:updateRemoteFeatureFlags',
+      );
+      expect(controller.state.featureFlagThresholdGroups).toStrictEqual({
+        flagA: 'groupA',
+        flagB: 'groupB',
+      });
+
+      jest
+        .spyOn(clientConfigApiService, 'fetchRemoteFeatureFlags')
+        .mockResolvedValue({
+          remoteFeatureFlags: {
+            flagB: [
+              {
+                name: 'groupB',
+                thresholdVersion: ThresholdVersion.DirectValue,
+                scope: { type: 'threshold', value: 1.0 },
+                value: false,
+              },
+            ],
+          },
+          cacheTimestamp: Date.now(),
+        });
+
+      jest.useFakeTimers();
+
+      try {
+        jest.advanceTimersByTime(2 * DEFAULT_CACHE_DURATION);
+
+        await messenger.call(
+          'RemoteFeatureFlagController:updateRemoteFeatureFlags',
+        );
+        await flushPromises();
+
+        expect(controller.state.featureFlagThresholdGroups).toStrictEqual({
+          flagB: 'groupB',
+        });
+      } finally {
+        jest.useRealTimers();
+      }
+    });
+
     it('preserves threshold cache entries for flags still in server response', async () => {
       // Arrange
       const mockFlags = {

packages/remote-feature-flag-controller/src/remote-feature-flag-controller.ts

@@ -9,7 +9,6 @@ import type { Json, SemVerVersion } from '@metamask/utils';
 
 import type { AbstractClientConfigApiService } from './client-config-api-service/abstract-client-config-api-service';
 import type { RemoteFeatureFlagControllerMethodActions } from './remote-feature-flag-controller-method-action-types';
-import { ThresholdVersion } from './remote-feature-flag-controller-types';
 import type {
   FeatureFlags,
   ServiceResponse,
@@ -34,6 +33,7 @@ export type RemoteFeatureFlagControllerState = {
   rawRemoteFeatureFlags?: FeatureFlags;
   cacheTimestamp: number;
   thresholdCache?: Record<string, number>;
+  featureFlagThresholdGroups?: Record<string, string>;
 };
 
 const remoteFeatureFlagControllerMetadata = {
@@ -67,6 +67,12 @@ const remoteFeatureFlagControllerMetadata = {
     includeInDebugSnapshot: false,
     usedInUi: false,
   },
+  featureFlagThresholdGroups: {
+    includeInStateLogs: true,
+    persist: true,
+    includeInDebugSnapshot: true,
+    usedInUi: false,
+  },
 };
 
 // === MESSENGER ===
@@ -119,19 +125,6 @@ export function getDefaultRemoteFeatureFlagControllerState(): RemoteFeatureFlagC
   };
 }
 
-function normalizeThresholdValue(featureFlag: FeatureFlagScopeValue): Json {
-  if (featureFlag.thresholdVersion === ThresholdVersion.DirectValue) {
-    return featureFlag.value;
-  }
-
-  // Unknown threshold versions fall back to the legacy wrapper shape for
-  // backwards compatibility with existing threshold feature flag configs.
-  return {
-    name: featureFlag.name,
-    value: featureFlag.value,
-  };
-}
-
 /**
  * The RemoteFeatureFlagController manages the retrieval and caching of remote feature flags.
  * It fetches feature flags from a remote API, caches them, and provides methods to access
@@ -288,8 +281,11 @@ export class RemoteFeatureFlagController extends BaseController<
    * @param remoteFeatureFlags - The new feature flags to cache.
    */
   async #updateCache(remoteFeatureFlags: FeatureFlags): Promise<void> {
-    const { processedFlags, thresholdCacheUpdates } =
-      await this.#processRemoteFeatureFlags(remoteFeatureFlags);
+    const {
+      processedFlags,
+      thresholdCacheUpdates,
+      featureFlagThresholdGroupUpdates,
+    } = await this.#processRemoteFeatureFlags(remoteFeatureFlags);
 
     const metaMetricsId = this.#getMetaMetricsId();
     const currentFlagNames = Object.keys(remoteFeatureFlags);
@@ -327,6 +323,7 @@ export class RemoteFeatureFlagController extends BaseController<
         rawRemoteFeatureFlags: remoteFeatureFlags,
         cacheTimestamp: Date.now(),
         thresholdCache: updatedThresholdCache,
+        featureFlagThresholdGroups: featureFlagThresholdGroupUpdates,
       };
     });
   }
@@ -348,10 +345,12 @@ export class RemoteFeatureFlagController extends BaseController<
   async #processRemoteFeatureFlags(remoteFeatureFlags: FeatureFlags): Promise<{
     processedFlags: FeatureFlags;
     thresholdCacheUpdates: Record<string, number>;
+    featureFlagThresholdGroupUpdates: Record<string, string>;
   }> {
     const processedFlags: FeatureFlags = {};
     const metaMetricsId = this.#getMetaMetricsId();
     const thresholdCacheUpdates: Record<string, number> = {};
+    const featureFlagThresholdGroupUpdates: Record<string, string> = {};
 
     for (const [
       remoteFeatureFlagName,
@@ -409,14 +408,22 @@ export class RemoteFeatureFlagController extends BaseController<
         );
 
         if (selectedGroup) {
-          processedValue = normalizeThresholdValue(selectedGroup);
+          processedValue = selectedGroup.value;
+          if (selectedGroup.name) {
+            featureFlagThresholdGroupUpdates[remoteFeatureFlagName] =
+              selectedGroup.name;
+          }
         }
       }
 
       processedFlags[remoteFeatureFlagName] = processedValue;
     }
 
-    return { processedFlags, thresholdCacheUpdates };
+    return {
+      processedFlags,
+      thresholdCacheUpdates,
+      featureFlagThresholdGroupUpdates,
+    };
   }
 
   /**