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

[asalsys]

Summary

• BREAKING: Threshold feature flags now return the selected value directly from remoteFeatureFlags instead of a { name, value } wrapper object.
• Add optional featureFlagThresholdGroups state field (Record<string, string>) that maps feature flag names to their selected threshold group name when the selected threshold entry includes name.
• Remove normalizeThresholdValue and the legacy { name, value } wrapper fallback.

References

• Fixes MCWP-655

Motivation

Separating the processed flag value from threshold group metadata makes it easier for consumers to use threshold flags as plain values (booleans, objects, etc.) without unwrapping. The new featureFlagThresholdGroups field provides explicit access to A/B group assignment when the selected threshold entry includes name.

Entries that only define thresholdName (for example in thresholdVersion: 2 configs) still resolve to the selected value, but do not populate featureFlagThresholdGroups because that field is driven by name.

Migration

Consumers that previously read threshold group names from remoteFeatureFlags[flagName].name should instead:

1. Read the flag value directly from remoteFeatureFlags[flagName].
2. Read the threshold group name from featureFlagThresholdGroups[flagName] when available (populated when the selected threshold entry includes name).

Before:

const flag = remoteFeatureFlags.myThresholdFlag;
// { name: 'groupB', value: { enabled: true } }
const groupName = flag.name;
const value = flag.value;

After:

const value = remoteFeatureFlags.myThresholdFlag;
// { enabled: true }

const groupName = featureFlagThresholdGroups.myThresholdFlag;
// 'groupB'

Related

• Documentation: MetaMask/contributor-docs#176

Test plan

• yarn workspace @metamask/remote-feature-flag-controller run test — all tests pass with 100% coverage
• Threshold flags return the selected value directly from remoteFeatureFlags
• Threshold flags populate featureFlagThresholdGroups when the selected entry includes name
• Threshold flags with only thresholdName return the value directly and leave featureFlagThresholdGroups empty
• Stale featureFlagThresholdGroups entries are cleaned up when flags are removed from the server response

---

Note

Medium Risk
Breaking change to the public shape of threshold flag values affects all consumers of remoteFeatureFlags; persisted state gains a new field but assignment logic is otherwise unchanged.

Overview
BREAKING: Threshold/A/B flags in remoteFeatureFlags now expose the selected entry’s value only (string, boolean, object, etc.) instead of a { name, value } wrapper. Group assignment moves to a new optional persisted state field featureFlagThresholdGroups (flagName → group name) when the winning threshold item has a name.

Processing drops normalizeThresholdValue and ThresholdVersion branching—legacy wrappers and “direct value vs wrapper” version handling are gone; threshold arrays are resolved the same way regardless of thresholdVersion. On each fetch, featureFlagThresholdGroups is rebuilt from the current server payload so entries for removed flags are cleared (covered by tests).

Migration: read values from remoteFeatureFlags[flag]; read group names from featureFlagThresholdGroups[flag] when present.

^{Reviewed by Cursor Bugbot for commit 98b74a2. Bugbot is set up for automated code reviews on this repo. Configure here.}

[Gudahtt]

Where is thresholdName coming from? Currently our flags are using name, not thresholdName

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 9d628ce. Configure here.}

[DDDDDanica]

@asalsys Thanks for the change, could you also update the doc to reflect new API？
https://github.com/MetaMask/contributor-docs/blob/main/docs/remote-feature-flags.md?plain=1#L97

[asalsys]

@DDDDDanica I updated the docs in this pr: MetaMask/contributor-docs#177

[Gudahtt]

I noticed that the way we're updating state here is suboptimal. We're replacing the entire state each time, rather than updating just the properties that have changed.

But since it's a pre-existing problem, better not to solve it in this PR. Maybe we can address it in a later PR.

[Gudahtt]

Nit: This test case seems redundant now that we've removed the "threshold v2" feature

[Gudahtt]

Nit: This test case seems redundant now that we've removed the "threshold v2" feature

[Gudahtt]

Actually nevermind, maybe we keep this one so that we ensure the thresholdVersion flag is ignored and does not interfere with feature flag processing? We'd want to re-word the decsription though.

Really all thresholdVersion values are "unrecognized" now. So we could say, for example:

Suggested change

	it('returns selected threshold values for unrecognized threshold versions', async () => {
	it('ignores threshold version field, processing threshold values normally', async () => {

[Gudahtt]

It would be safer to put this in an afterEach hook or in a try.. finally block, so that it still runs even if there is a crash.

if this test crashes after file timers are enabled but before restoring real timers, the fake timers could remain in-place for later tests as well, causing confusing cascading test failures.

[Gudahtt]

Generally looks good! At least one change needed for the test suite, but the rest of my comments are nits

[Gudahtt]

LGTM!