From 21524fb6b8e233d695ba4fe4cb6608875a54ba08 Mon Sep 17 00:00:00 2001 From: Blake Date: Tue, 7 Jul 2026 16:10:32 -0400 Subject: [PATCH 01/13] feat(flags): FlagsClient.setConfiguration with context matching (FFL-2688) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add offline configuration loading to the core FlagsClient: - setConfiguration(config): decodes a precomputed configuration into the flag cache. When no context is set yet, it adopts the configuration's embedded context (implicit set) so offline evaluation works with no native fetch. When a context is already set, the configuration's context must match it. - Context matching: new configuration/context.ts normalizes the flat wire context through the same processEvaluationContext as the active context, then deep-compares — a config with no embedded context is context-agnostic. - Mismatch -> serve no values and report INVALID_CONTEXT (added to FlagErrorCode); empty/unsupported/invalid config -> PROVIDER_NOT_READY (not FLAG_NOT_FOUND). - A subsequent native setEvaluationContext fetch supersedes the offline config. Tests cover context normalization/matching and the setConfiguration flows (implicit-set no-fetch, match/mismatch against an explicit context, invalid config, and fetch supersession). fetchPolicy (PR3) and the OpenFeature provider lifecycle (PR4) are unchanged here. The parse-failure vs valid-but-empty distinction (L2) remains for a follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/core/src/flags/FlagsClient.ts | 143 ++++++++++++++++++ .../src/flags/__tests__/FlagsClient.test.ts | 129 ++++++++++++++++ .../configuration/__tests__/context.test.ts | 105 +++++++++++++ .../core/src/flags/configuration/context.ts | 75 +++++++++ .../core/src/flags/configuration/index.ts | 1 + packages/core/src/flags/types.ts | 3 +- 6 files changed, 455 insertions(+), 1 deletion(-) create mode 100644 packages/core/src/flags/configuration/__tests__/context.test.ts create mode 100644 packages/core/src/flags/configuration/context.ts diff --git a/packages/core/src/flags/FlagsClient.ts b/packages/core/src/flags/FlagsClient.ts index 62284cc40..3d69d950a 100644 --- a/packages/core/src/flags/FlagsClient.ts +++ b/packages/core/src/flags/FlagsClient.ts @@ -8,10 +8,23 @@ import { InternalLog } from '../InternalLog'; import { SdkVerbosity } from '../config/types/SdkVerbosity'; import type { DdNativeFlagsType } from '../nativeModulesTypes'; +import { + contextMatchesConfiguration, + decodePrecomputedFlags, + normalizeWireContext +} from './configuration'; +import type { ParsedFlagsConfiguration } from './configuration'; import { processEvaluationContext } from './internal'; import type { FlagCacheEntry } from './internal'; import type { JsonValue, EvaluationContext, FlagDetails } from './types'; +/** + * Tracks how a configuration supplied via {@link FlagsClient.setConfiguration} relates + * to the active evaluation context. `'none'` means no offline configuration is engaged + * (the online/fetch path is in effect). + */ +type ConfigurationStatus = 'none' | 'ready' | 'mismatch' | 'invalid'; + export class FlagsClient { // eslint-disable-next-line global-require, @typescript-eslint/no-var-requires private nativeFlags: DdNativeFlagsType = require('../specs/NativeDdFlags') @@ -23,6 +36,12 @@ export class FlagsClient { private flagsCache: Record = {}; + private loadedConfiguration: + | ParsedFlagsConfiguration + | undefined = undefined; + + private configurationStatus: ConfigurationStatus = 'none'; + constructor(clientName: string = 'default') { this.clientName = clientName; } @@ -65,6 +84,11 @@ export class FlagsClient { this.evaluationContext = processedContext; this.flagsCache = result; + + // An explicit online fetch supersedes any previously loaded offline + // configuration, so drop the offline overlay to keep state coherent. + this.loadedConfiguration = undefined; + this.configurationStatus = 'none'; } catch (error) { if (error instanceof Error) { InternalLog.log( @@ -77,6 +101,105 @@ export class FlagsClient { } }; + /** + * Load a configuration (parsed from a `ConfigurationWire` string via + * `configurationFromString`) into the client for offline evaluation. + * + * For a precomputed configuration this populates the flag cache and, when no context + * has been set yet, adopts the configuration's embedded evaluation context — **no + * network request is made**. If a context has already been set, the configuration's + * context must match it; otherwise the client serves no values and reports + * `INVALID_CONTEXT`. + * + * @param configuration The configuration to load. + * + * @example + * ```ts + * const configuration = configurationFromString(wire); + * flagsClient.setConfiguration(configuration); + * + * const value = flagsClient.getBooleanValue('new-feature', false); + * ``` + */ + setConfiguration = (configuration: ParsedFlagsConfiguration): void => { + this.loadedConfiguration = configuration; + this.applyConfiguration(); + }; + + /** + * Reconcile the loaded configuration against the active evaluation context and + * (re)compute the servable flag cache and configuration status. + */ + private applyConfiguration = (): void => { + const precomputed = this.loadedConfiguration?.precomputed; + + // Only precomputed configurations are supported for now. An empty configuration + // (an invalid/failed wire parse, or a server-only wire) is not usable. + if (!precomputed) { + this.flagsCache = {}; + this.configurationStatus = 'invalid'; + InternalLog.log( + `No usable precomputed configuration was provided for '${this.clientName}'.`, + SdkVerbosity.WARN + ); + return; + } + + let decoded: Record; + try { + decoded = decodePrecomputedFlags(precomputed.response); + } catch (error) { + this.flagsCache = {}; + this.configurationStatus = 'invalid'; + if (error instanceof Error) { + InternalLog.log( + `Unsupported flags configuration for '${this.clientName}': ${error.message}`, + SdkVerbosity.WARN + ); + } + return; + } + + // If no context has been set yet, adopt the configuration's embedded context + // (implicit set — no native fetch). A context-agnostic configuration falls back + // to an empty context so evaluation can proceed. + if (!this.evaluationContext) { + if (precomputed.context) { + this.evaluationContext = normalizeWireContext( + precomputed.context + ); + } else { + InternalLog.log( + `The provided configuration for '${this.clientName}' has no embedded context; treating it as context-agnostic.`, + SdkVerbosity.WARN + ); + this.evaluationContext = { targetingKey: '', attributes: {} }; + } + + this.flagsCache = decoded; + this.configurationStatus = 'ready'; + return; + } + + // A context is already set — the configuration must match it. + if ( + contextMatchesConfiguration( + precomputed.context, + this.evaluationContext + ) + ) { + this.flagsCache = decoded; + this.configurationStatus = 'ready'; + } else { + this.flagsCache = {}; + this.configurationStatus = 'mismatch'; + InternalLog.log( + `The provided configuration for '${this.clientName}' does not match the active evaluation context.`, + SdkVerbosity.WARN + ); + } + }; + private track = (flag: FlagCacheEntry, context: EvaluationContext) => { // A non-blocking call; don't await this. this.nativeFlags @@ -102,6 +225,26 @@ export class FlagsClient { defaultValue: T, type: 'boolean' | 'string' | 'number' | 'object' ): FlagDetails => { + if (this.configurationStatus === 'mismatch') { + return { + key, + value: defaultValue, + reason: 'ERROR', + errorCode: 'INVALID_CONTEXT', + errorMessage: `The loaded configuration for '${this.clientName}' does not match the active evaluation context.` + }; + } + + if (this.configurationStatus === 'invalid') { + return { + key, + value: defaultValue, + reason: 'ERROR', + errorCode: 'PROVIDER_NOT_READY', + errorMessage: `The loaded configuration for '${this.clientName}' is not usable. Provide a valid precomputed configuration.` + }; + } + if (!this.evaluationContext) { return { key, diff --git a/packages/core/src/flags/__tests__/FlagsClient.test.ts b/packages/core/src/flags/__tests__/FlagsClient.test.ts index 06903099c..fb9b082ba 100644 --- a/packages/core/src/flags/__tests__/FlagsClient.test.ts +++ b/packages/core/src/flags/__tests__/FlagsClient.test.ts @@ -9,6 +9,7 @@ import { NativeModules } from 'react-native'; import { InternalLog } from '../../InternalLog'; import { SdkVerbosity } from '../../config/types/SdkVerbosity'; import { DdFlags } from '../DdFlags'; +import { configurationFromString } from '../configuration'; jest.spyOn(NativeModules.DdFlags, 'setEvaluationContext').mockResolvedValue({ 'test-boolean-flag': { @@ -317,4 +318,132 @@ describe('FlagsClient', () => { expect(numberFlagAsString).toBe('default'); }); }); + + describe('setConfiguration', () => { + const offlineFlags = { + 'offline-bool': { + variationType: 'boolean', + variationValue: true, + variationKey: 'true', + allocationKey: 'alloc-1', + reason: 'STATIC', + doLog: false, + extraLogging: {} + } + }; + + const buildConfig = ( + flags: Record, + context?: Record + ) => + configurationFromString( + JSON.stringify({ + version: 1, + precomputed: { + response: JSON.stringify({ + data: { attributes: { obfuscated: false, flags } } + }), + context + } + }) + ); + + it('serves flags from the configuration without a native fetch', () => { + const flagsClient = DdFlags.getClient(); + + flagsClient.setConfiguration( + buildConfig(offlineFlags, { + targetingKey: 'user-1', + country: 'US' + }) + ); + + expect(flagsClient.getBooleanValue('offline-bool', false)).toBe( + true + ); + expect( + NativeModules.DdFlags.setEvaluationContext + ).not.toHaveBeenCalled(); + }); + + it('serves flags when an explicit matching context was set first', async () => { + const flagsClient = DdFlags.getClient(); + await flagsClient.setEvaluationContext({ + targetingKey: 'user-1', + attributes: { country: 'US' } + }); + + flagsClient.setConfiguration( + buildConfig(offlineFlags, { + targetingKey: 'user-1', + country: 'US' + }) + ); + + expect(flagsClient.getBooleanValue('offline-bool', false)).toBe( + true + ); + }); + + it('returns INVALID_CONTEXT when the config does not match an explicit context', async () => { + const flagsClient = DdFlags.getClient(); + await flagsClient.setEvaluationContext({ + targetingKey: 'user-1', + attributes: { country: 'US' } + }); + + flagsClient.setConfiguration( + buildConfig(offlineFlags, { + targetingKey: 'user-2', + country: 'US' + }) + ); + + expect( + flagsClient.getBooleanDetails('offline-bool', false) + ).toMatchObject({ + value: false, + reason: 'ERROR', + errorCode: 'INVALID_CONTEXT' + }); + }); + + it('returns PROVIDER_NOT_READY for an empty/invalid configuration', () => { + const flagsClient = DdFlags.getClient(); + + flagsClient.setConfiguration(configurationFromString('garbage')); + + expect( + flagsClient.getBooleanDetails('offline-bool', false) + ).toMatchObject({ + value: false, + reason: 'ERROR', + errorCode: 'PROVIDER_NOT_READY' + }); + }); + + it('is superseded by a later native fetch', async () => { + const flagsClient = DdFlags.getClient(); + flagsClient.setConfiguration( + buildConfig(offlineFlags, { + targetingKey: 'user-1', + country: 'US' + }) + ); + + // A subsequent explicit context fetch replaces the offline configuration + // with the native snapshot (mocked in __mocks__/react-native.ts + above). + await flagsClient.setEvaluationContext({ + targetingKey: 'user-1', + attributes: { country: 'US' } + }); + + expect( + flagsClient.getBooleanDetails('offline-bool', false) + ).toMatchObject({ errorCode: 'FLAG_NOT_FOUND' }); + expect( + flagsClient.getBooleanValue('test-boolean-flag', false) + ).toBe(true); + }); + }); }); diff --git a/packages/core/src/flags/configuration/__tests__/context.test.ts b/packages/core/src/flags/configuration/__tests__/context.test.ts new file mode 100644 index 000000000..de03670f3 --- /dev/null +++ b/packages/core/src/flags/configuration/__tests__/context.test.ts @@ -0,0 +1,105 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. + * This product includes software developed at Datadog (https://www.datadoghq.com/). + * Copyright 2016-Present Datadog, Inc. + */ + +import type { EvaluationContext } from '../../types'; +import { contextMatchesConfiguration, normalizeWireContext } from '../context'; + +jest.mock('../../../InternalLog', () => { + return { + InternalLog: { log: jest.fn() }, + DATADOG_MESSAGE_PREFIX: 'DATADOG:' + }; +}); + +describe('normalizeWireContext', () => { + it('maps a flat wire context to { targetingKey, attributes }', () => { + expect( + normalizeWireContext({ + targetingKey: 'user-1', + country: 'US', + age: 25 + }) + ).toEqual({ + targetingKey: 'user-1', + attributes: { country: 'US', age: 25 } + }); + }); + + it('defaults a missing targeting key to an empty string', () => { + expect(normalizeWireContext({ country: 'US' })).toEqual({ + targetingKey: '', + attributes: { country: 'US' } + }); + }); + + it('drops non-primitive attributes', () => { + expect( + normalizeWireContext({ + targetingKey: 'user-1', + country: 'US', + nested: { a: 1 } + }) + ).toEqual({ targetingKey: 'user-1', attributes: { country: 'US' } }); + }); +}); + +describe('contextMatchesConfiguration', () => { + const active: EvaluationContext = { + targetingKey: 'user-1', + attributes: { country: 'US' } + }; + + it('matches any context when the config has no embedded context', () => { + expect(contextMatchesConfiguration(undefined, active)).toBe(true); + }); + + it('matches an equal context', () => { + expect( + contextMatchesConfiguration( + { targetingKey: 'user-1', country: 'US' }, + active + ) + ).toBe(true); + }); + + it('does not match a different targeting key', () => { + expect( + contextMatchesConfiguration( + { targetingKey: 'user-2', country: 'US' }, + active + ) + ).toBe(false); + }); + + it('does not match a different attribute value', () => { + expect( + contextMatchesConfiguration( + { targetingKey: 'user-1', country: 'CA' }, + active + ) + ).toBe(false); + }); + + it('does not match when the wire has an extra primitive attribute', () => { + expect( + contextMatchesConfiguration( + { targetingKey: 'user-1', country: 'US', plan: 'pro' }, + active + ) + ).toBe(false); + }); + + it('ignores dropped non-primitive attributes on both sides', () => { + // The nested attribute is dropped by the same normalization applied to the + // active context, so a wire context that only differs by it still matches. + expect( + contextMatchesConfiguration( + { targetingKey: 'user-1', country: 'US', nested: { a: 1 } }, + active + ) + ).toBe(true); + }); +}); diff --git a/packages/core/src/flags/configuration/context.ts b/packages/core/src/flags/configuration/context.ts new file mode 100644 index 000000000..06044db02 --- /dev/null +++ b/packages/core/src/flags/configuration/context.ts @@ -0,0 +1,75 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. + * This product includes software developed at Datadog (https://www.datadoghq.com/). + * Copyright 2016-Present Datadog, Inc. + */ + +import { processEvaluationContext } from '../internal'; +import type { EvaluationContext, PrimitiveValue } from '../types'; + +import type { WireEvaluationContext } from './types'; + +/** + * Normalize a wire (OpenFeature-flat) evaluation context into the SDK's internal + * `{ targetingKey, attributes }` shape, applying the **same** processing the active + * context went through (`processEvaluationContext`) so the two are comparable — the + * flat wire shape and the internal shape are otherwise never equal. + */ +export const normalizeWireContext = ( + wireContext: WireEvaluationContext +): EvaluationContext => { + const { targetingKey, ...attributes } = wireContext; + + return processEvaluationContext({ + targetingKey: targetingKey ?? '', + // `processEvaluationContext` drops non-primitive attributes; casting here mirrors + // how the active context's attributes are typed before that same processing. + attributes: attributes as Record + }); +}; + +/** + * Whether a precomputed configuration's embedded context matches the active evaluation + * context. + * + * - A configuration with **no** embedded context is context-agnostic and matches any + * active context. + * - Otherwise the embedded context must match the active context exactly (after + * normalizing both through the same processing). + */ +export const contextMatchesConfiguration = ( + wireContext: WireEvaluationContext | undefined, + activeContext: EvaluationContext +): boolean => { + if (!wireContext) { + return true; + } + + return contextsEqual(normalizeWireContext(wireContext), activeContext); +}; + +const contextsEqual = (a: EvaluationContext, b: EvaluationContext): boolean => { + if (a.targetingKey !== b.targetingKey) { + return false; + } + + return attributesEqual(a.attributes ?? {}, b.attributes ?? {}); +}; + +/** + * Compare two attribute maps. After `processEvaluationContext`, attribute values are + * primitives, so a key-set + strict-value comparison is sufficient. + */ +const attributesEqual = ( + a: Record, + b: Record +): boolean => { + const aKeys = Object.keys(a); + const bKeys = Object.keys(b); + + if (aKeys.length !== bKeys.length) { + return false; + } + + return aKeys.every(key => a[key] === b[key]); +}; diff --git a/packages/core/src/flags/configuration/index.ts b/packages/core/src/flags/configuration/index.ts index 3890eefc3..0ce6673c6 100644 --- a/packages/core/src/flags/configuration/index.ts +++ b/packages/core/src/flags/configuration/index.ts @@ -14,6 +14,7 @@ export { decodePrecomputedFlags, UnsupportedConfigurationError } from './precomputed'; +export { contextMatchesConfiguration, normalizeWireContext } from './context'; export type { ParsedFlagsConfiguration, ParsedPrecomputedConfiguration, diff --git a/packages/core/src/flags/types.ts b/packages/core/src/flags/types.ts index 12bfb18f6..e451f4e72 100644 --- a/packages/core/src/flags/types.ts +++ b/packages/core/src/flags/types.ts @@ -169,7 +169,8 @@ type FlagErrorCode = | 'PROVIDER_NOT_READY' | 'FLAG_NOT_FOUND' | 'PARSE_ERROR' - | 'TYPE_MISMATCH'; + | 'TYPE_MISMATCH' + | 'INVALID_CONTEXT'; /** * Detailed information about a feature flag evaluation. From b2cfdc2beef9bfbcb3881cfca6e7cee15c23c5a4 Mon Sep 17 00:00:00 2001 From: Blake Date: Tue, 7 Jul 2026 17:41:20 -0400 Subject: [PATCH 02/13] refactor(flags): address PR2 review (rules seam, proto-safe context, tests) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on PR2: - #7: build the processed evaluation context with a Map + Object.fromEntries so reserved keys like "__proto__" (notably a "__proto__": null attribute, which a plain assignment would have used to null the object's prototype) are handled as data without prototype manipulation. - #1: mark the forward-compat seam in applyConfiguration so a future server/rules configuration is not rejected as "invalid" (rules configs are context-agnostic). - #3/#4/#5: document the deferred seams — fetch-failure/staleness fallback and the NEVER re-match belong to the fetch-policy step; the PROVIDER_ERROR provider event belongs to the OpenFeature provider step. - #6: add tests for a context-agnostic configuration, configuration replacement, and the __proto__ context-attribute / proto-null safety cases. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/core/src/flags/FlagsClient.ts | 19 ++++++++ .../src/flags/__tests__/FlagsClient.test.ts | 37 +++++++++++++++ .../core/src/flags/__tests__/internal.test.ts | 45 +++++++++++++++++++ .../configuration/__tests__/context.test.ts | 13 ++++++ packages/core/src/flags/internal.ts | 9 ++-- 5 files changed, 120 insertions(+), 3 deletions(-) create mode 100644 packages/core/src/flags/__tests__/internal.test.ts diff --git a/packages/core/src/flags/FlagsClient.ts b/packages/core/src/flags/FlagsClient.ts index 3d69d950a..c0d1c317e 100644 --- a/packages/core/src/flags/FlagsClient.ts +++ b/packages/core/src/flags/FlagsClient.ts @@ -87,9 +87,18 @@ export class FlagsClient { // An explicit online fetch supersedes any previously loaded offline // configuration, so drop the offline overlay to keep state coherent. + // + // PROVISIONAL: this reflects the current fetch-always default. When the + // `NEVER` fetch policy lands, that path must skip the fetch and re-run + // `applyConfiguration()` against the new context instead of dropping the + // loaded configuration. this.loadedConfiguration = undefined; this.configurationStatus = 'none'; } catch (error) { + // NOTE: a failed fetch leaves any previously loaded offline configuration in + // place, so the client may keep serving it (and attribute exposures to its + // context). Fetch-failure/staleness fallback is deferred to the fetch-policy + // step. if (error instanceof Error) { InternalLog.log( `Error setting flag evaluation context: ${error.message}`, @@ -135,6 +144,10 @@ export class FlagsClient { // Only precomputed configurations are supported for now. An empty configuration // (an invalid/failed wire parse, or a server-only wire) is not usable. + // + // FORWARD-COMPAT SEAM: when the `server`/rules branch is parsed (see + // `ParsedFlagsConfiguration`), it must be handled BEFORE this guard — a rules + // configuration is context-agnostic and must NOT be rejected here as `invalid`. if (!precomputed) { this.flagsCache = {}; this.configurationStatus = 'invalid'; @@ -191,6 +204,9 @@ export class FlagsClient { this.flagsCache = decoded; this.configurationStatus = 'ready'; } else { + // Per spec, a context mismatch must not serve values. This also blocks a + // previously-fetched online cache until a matching config or a new fetch; + // the fetch policy (a later step) formalizes online/offline precedence. this.flagsCache = {}; this.configurationStatus = 'mismatch'; InternalLog.log( @@ -235,6 +251,9 @@ export class FlagsClient { }; } + // A loaded-but-unusable configuration surfaces as PROVIDER_NOT_READY at the + // evaluation layer (distinct from FLAG_NOT_FOUND). The dedicated PROVIDER_ERROR + // provider event is wired by the OpenFeature provider in a later step. if (this.configurationStatus === 'invalid') { return { key, diff --git a/packages/core/src/flags/__tests__/FlagsClient.test.ts b/packages/core/src/flags/__tests__/FlagsClient.test.ts index fb9b082ba..efcca17ed 100644 --- a/packages/core/src/flags/__tests__/FlagsClient.test.ts +++ b/packages/core/src/flags/__tests__/FlagsClient.test.ts @@ -422,6 +422,43 @@ describe('FlagsClient', () => { }); }); + it('serves a context-agnostic configuration (no embedded context)', () => { + const flagsClient = DdFlags.getClient(); + + flagsClient.setConfiguration(buildConfig(offlineFlags)); + + expect(flagsClient.getBooleanValue('offline-bool', false)).toBe( + true + ); + expect( + NativeModules.DdFlags.setEvaluationContext + ).not.toHaveBeenCalled(); + }); + + it('replaces a previously loaded configuration', () => { + const flagsClient = DdFlags.getClient(); + + flagsClient.setConfiguration( + buildConfig( + { 'flag-a': offlineFlags['offline-bool'] }, + { targetingKey: 'user-1' } + ) + ); + expect(flagsClient.getBooleanValue('flag-a', false)).toBe(true); + + flagsClient.setConfiguration( + buildConfig( + { 'flag-b': offlineFlags['offline-bool'] }, + { targetingKey: 'user-1' } + ) + ); + + expect( + flagsClient.getBooleanDetails('flag-a', false) + ).toMatchObject({ errorCode: 'FLAG_NOT_FOUND' }); + expect(flagsClient.getBooleanValue('flag-b', false)).toBe(true); + }); + it('is superseded by a later native fetch', async () => { const flagsClient = DdFlags.getClient(); flagsClient.setConfiguration( diff --git a/packages/core/src/flags/__tests__/internal.test.ts b/packages/core/src/flags/__tests__/internal.test.ts new file mode 100644 index 000000000..eedccacbf --- /dev/null +++ b/packages/core/src/flags/__tests__/internal.test.ts @@ -0,0 +1,45 @@ +/* + * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. + * This product includes software developed at Datadog (https://www.datadoghq.com/). + * Copyright 2016-Present Datadog, Inc. + */ + +import { processEvaluationContext } from '../internal'; + +jest.mock('../../InternalLog', () => { + return { + InternalLog: { log: jest.fn() }, + DATADOG_MESSAGE_PREFIX: 'DATADOG:' + }; +}); + +describe('processEvaluationContext', () => { + it('keeps primitive attributes and drops non-primitive ones', () => { + expect( + processEvaluationContext({ + targetingKey: 'user-1', + attributes: { + country: 'US', + age: 25, + beta: true, + // Dropped: non-primitive. + nested: { a: 1 } as never + } + }) + ).toEqual({ + targetingKey: 'user-1', + attributes: { country: 'US', age: 25, beta: true } + }); + }); + + it('does not null the prototype for a "__proto__": null attribute', () => { + const result = processEvaluationContext({ + targetingKey: 'user-1', + attributes: { ['__proto__']: null } + }); + + // A plain `attributes[key] = value` would have set the object's prototype to + // null here; the Map + Object.fromEntries build keeps it a normal object. + expect(Object.getPrototypeOf(result.attributes)).toBe(Object.prototype); + }); +}); diff --git a/packages/core/src/flags/configuration/__tests__/context.test.ts b/packages/core/src/flags/configuration/__tests__/context.test.ts index de03670f3..f75891993 100644 --- a/packages/core/src/flags/configuration/__tests__/context.test.ts +++ b/packages/core/src/flags/configuration/__tests__/context.test.ts @@ -44,6 +44,19 @@ describe('normalizeWireContext', () => { }) ).toEqual({ targetingKey: 'user-1', attributes: { country: 'US' } }); }); + + it('handles a "__proto__" attribute without polluting the prototype', () => { + const normalized = normalizeWireContext({ + targetingKey: 'user-1', + ['__proto__']: 'x' + }); + + // The reserved key is safely discarded; the prototype is untouched. + expect(Object.getPrototypeOf(normalized.attributes)).toBe( + Object.prototype + ); + expect(({} as Record).x).toBeUndefined(); + }); }); describe('contextMatchesConfiguration', () => { diff --git a/packages/core/src/flags/internal.ts b/packages/core/src/flags/internal.ts index 6d504d8cc..fc47b2109 100644 --- a/packages/core/src/flags/internal.ts +++ b/packages/core/src/flags/internal.ts @@ -30,7 +30,10 @@ export const processEvaluationContext = ( const providedAttributes: Record = context.attributes ?? {}; - const attributes: Record = {}; + // Accumulate in a Map so reserved keys such as "__proto__" are handled as data + // instead of hitting the Object.prototype setter (which would silently drop them or + // pollute the prototype). Object.fromEntries then materializes own properties safely. + const attributes = new Map(); for (const [key, value] of Object.entries(providedAttributes)) { const isPrimitiveValue = @@ -53,11 +56,11 @@ export const processEvaluationContext = ( continue; } - attributes[key] = value; + attributes.set(key, value as PrimitiveValue); } return { targetingKey, - attributes + attributes: Object.fromEntries(attributes) }; }; From db42f71683027ec6b3f583f5b156928d1d842185 Mon Sep 17 00:00:00 2001 From: Blake Date: Tue, 7 Jul 2026 18:05:25 -0400 Subject: [PATCH 03/13] feat(flags): add FlagsClient.setEvaluationContextWithoutFetching MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The offline counterpart to setEvaluationContext: records the active context and reconciles a configuration loaded via setConfiguration against it (context matching for a precomputed config) with no native request. This is the core seam an offline OpenFeature provider uses to update context on initialize/onContextChange without ever fetching from the CDN — replacing the previously-planned fetchPolicy flag with a provider-level choice. Tests cover the no-fetch reconcile (match serves, context-change mismatch -> INVALID_CONTEXT) and assert the native fetch is never called. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/core/src/flags/FlagsClient.ts | 24 ++++++ .../src/flags/__tests__/FlagsClient.test.ts | 74 +++++++++++++++++++ 2 files changed, 98 insertions(+) diff --git a/packages/core/src/flags/FlagsClient.ts b/packages/core/src/flags/FlagsClient.ts index c0d1c317e..392c87c6e 100644 --- a/packages/core/src/flags/FlagsClient.ts +++ b/packages/core/src/flags/FlagsClient.ts @@ -110,6 +110,30 @@ export class FlagsClient { } }; + /** + * Set the evaluation context **without** fetching a configuration from the network, + * then reconcile any configuration loaded via {@link setConfiguration} against it. + * + * This is the offline counterpart to {@link setEvaluationContext}: it records the + * active context and re-evaluates the loaded configuration (context matching, for a + * precomputed configuration) with no native request. It is intended for offline + * providers that own their configuration via `setConfiguration` and must not fetch on + * a context change. With no configuration loaded yet, the context is simply recorded. + * + * @param context The evaluation context to associate with the current client. + */ + setEvaluationContextWithoutFetching = ( + context: EvaluationContext + ): void => { + this.evaluationContext = processEvaluationContext(context); + + // Re-evaluate a loaded offline configuration against the new context. Readiness + // when no configuration is loaded yet is the provider's concern. + if (this.loadedConfiguration) { + this.applyConfiguration(); + } + }; + /** * Load a configuration (parsed from a `ConfigurationWire` string via * `configurationFromString`) into the client for offline evaluation. diff --git a/packages/core/src/flags/__tests__/FlagsClient.test.ts b/packages/core/src/flags/__tests__/FlagsClient.test.ts index efcca17ed..f49fea1d9 100644 --- a/packages/core/src/flags/__tests__/FlagsClient.test.ts +++ b/packages/core/src/flags/__tests__/FlagsClient.test.ts @@ -483,4 +483,78 @@ describe('FlagsClient', () => { ).toBe(true); }); }); + + describe('setEvaluationContextWithoutFetching', () => { + const offlineFlags = { + 'offline-bool': { + variationType: 'boolean', + variationValue: true, + variationKey: 'true', + allocationKey: 'alloc-1', + reason: 'STATIC', + doLog: false, + extraLogging: {} + } + }; + + const buildConfig = (context: Record) => + configurationFromString( + JSON.stringify({ + version: 1, + precomputed: { + response: JSON.stringify({ + data: { + attributes: { + obfuscated: false, + flags: offlineFlags + } + } + }), + context + } + }) + ); + + it('reconciles a loaded config against the context without fetching', () => { + const flagsClient = DdFlags.getClient(); + flagsClient.setConfiguration( + buildConfig({ targetingKey: 'user-1', country: 'US' }) + ); + + flagsClient.setEvaluationContextWithoutFetching({ + targetingKey: 'user-1', + attributes: { country: 'US' } + }); + + expect(flagsClient.getBooleanValue('offline-bool', false)).toBe( + true + ); + expect( + NativeModules.DdFlags.setEvaluationContext + ).not.toHaveBeenCalled(); + }); + + it('re-validates on a context change (mismatch -> INVALID_CONTEXT), still no fetch', () => { + const flagsClient = DdFlags.getClient(); + flagsClient.setConfiguration( + buildConfig({ targetingKey: 'user-1', country: 'US' }) + ); + + flagsClient.setEvaluationContextWithoutFetching({ + targetingKey: 'user-2', + attributes: { country: 'US' } + }); + + expect( + flagsClient.getBooleanDetails('offline-bool', false) + ).toMatchObject({ + value: false, + reason: 'ERROR', + errorCode: 'INVALID_CONTEXT' + }); + expect( + NativeModules.DdFlags.setEvaluationContext + ).not.toHaveBeenCalled(); + }); + }); }); From f746fc96abff31fe8cd8e46d1efdf6cf16202752 Mon Sep 17 00:00:00 2001 From: Blake Date: Thu, 16 Jul 2026 21:45:20 -0400 Subject: [PATCH 04/13] refactor(flags): ignore a mismatched runtime context for offline precomputed (PR2 review) An offline precomputed configuration is a single-subject snapshot and offline never fetches, so a runtime evaluation context that differs from the one the snapshot was computed for can no longer be honored. Instead of the previous mismatch -> PROVIDER_ERROR/INVALID_CONTEXT behavior, we now ignore the differing context (keep serving the snapshot against its embedded context) and log a warning once, on the context change. Per-context evaluation is the job of the future rules-based configuration. Drops the 'mismatch' ConfigurationStatus and the getDetails INVALID_CONTEXT branch. Fixes the stale-cache case (Copilot review): setEvaluationContextWithoutFetching now clears the cache when no configuration is loaded. Un-exports contextMatchesConfiguration from the configuration module index (now an internal helper imported directly by FlagsClient). Corrects the __proto__ context test comment to only assert what it verifies (no prototype pollution). Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/core/src/flags/FlagsClient.ts | 65 +++++++++---------- .../src/flags/__tests__/FlagsClient.test.ts | 36 +++++----- .../configuration/__tests__/context.test.ts | 4 +- .../core/src/flags/configuration/index.ts | 4 +- 4 files changed, 57 insertions(+), 52 deletions(-) diff --git a/packages/core/src/flags/FlagsClient.ts b/packages/core/src/flags/FlagsClient.ts index 392c87c6e..5ea813c10 100644 --- a/packages/core/src/flags/FlagsClient.ts +++ b/packages/core/src/flags/FlagsClient.ts @@ -8,11 +8,11 @@ import { InternalLog } from '../InternalLog'; import { SdkVerbosity } from '../config/types/SdkVerbosity'; import type { DdNativeFlagsType } from '../nativeModulesTypes'; -import { - contextMatchesConfiguration, - decodePrecomputedFlags, - normalizeWireContext -} from './configuration'; +// Imported directly (not via the module index): context matching is an internal helper, +// used here only to detect and warn about a runtime context that an offline precomputed +// configuration cannot honor. +import { contextMatchesConfiguration } from './configuration/context'; +import { decodePrecomputedFlags, normalizeWireContext } from './configuration'; import type { ParsedFlagsConfiguration } from './configuration'; import { processEvaluationContext } from './internal'; import type { FlagCacheEntry } from './internal'; @@ -23,7 +23,7 @@ import type { JsonValue, EvaluationContext, FlagDetails } from './types'; * to the active evaluation context. `'none'` means no offline configuration is engaged * (the online/fetch path is in effect). */ -type ConfigurationStatus = 'none' | 'ready' | 'mismatch' | 'invalid'; +type ConfigurationStatus = 'none' | 'ready' | 'invalid'; export class FlagsClient { // eslint-disable-next-line global-require, @typescript-eslint/no-var-requires @@ -127,10 +127,14 @@ export class FlagsClient { ): void => { this.evaluationContext = processEvaluationContext(context); - // Re-evaluate a loaded offline configuration against the new context. Readiness - // when no configuration is loaded yet is the provider's concern. + // Re-evaluate a loaded offline configuration against the new context. if (this.loadedConfiguration) { this.applyConfiguration(); + } else { + // No offline configuration is engaged: do not keep serving any previously + // cached flags (e.g. from a prior online fetch) against the new context. + this.flagsCache = {}; + this.configurationStatus = 'none'; } }; @@ -138,11 +142,11 @@ export class FlagsClient { * Load a configuration (parsed from a `ConfigurationWire` string via * `configurationFromString`) into the client for offline evaluation. * - * For a precomputed configuration this populates the flag cache and, when no context - * has been set yet, adopts the configuration's embedded evaluation context — **no - * network request is made**. If a context has already been set, the configuration's - * context must match it; otherwise the client serves no values and reports - * `INVALID_CONTEXT`. + * For a precomputed configuration this populates the flag cache and adopts the + * configuration's embedded evaluation context — **no network request is made**. A + * precomputed snapshot is single-subject: if a different runtime context has been set, + * it is ignored (the snapshot is served for its embedded context) and a warning is + * logged. Use a rules-based configuration for per-context evaluation. * * @param configuration The configuration to load. * @@ -218,26 +222,29 @@ export class FlagsClient { return; } - // A context is already set — the configuration must match it. + // A context is already set. An offline precomputed configuration is a snapshot + // bound to the context it was computed for, and offline never fetches, so it cannot + // be recomputed for a different subject. A runtime context that does not match is + // therefore IGNORED: revert to the embedded context, keep serving the snapshot, and + // warn once. (Precomputed is single-subject by design — a rules-based configuration + // is the path for per-context evaluation.) if ( - contextMatchesConfiguration( + !contextMatchesConfiguration( precomputed.context, this.evaluationContext ) ) { - this.flagsCache = decoded; - this.configurationStatus = 'ready'; - } else { - // Per spec, a context mismatch must not serve values. This also blocks a - // previously-fetched online cache until a matching config or a new fetch; - // the fetch policy (a later step) formalizes online/offline precedence. - this.flagsCache = {}; - this.configurationStatus = 'mismatch'; InternalLog.log( - `The provided configuration for '${this.clientName}' does not match the active evaluation context.`, + `Ignoring the evaluation context set for '${this.clientName}': an offline precomputed configuration is served against the context it was computed for. Set a matching context, or use a rules-based configuration for per-context evaluation.`, SdkVerbosity.WARN ); + this.evaluationContext = precomputed.context + ? normalizeWireContext(precomputed.context) + : { targetingKey: '', attributes: {} }; } + + this.flagsCache = decoded; + this.configurationStatus = 'ready'; }; private track = (flag: FlagCacheEntry, context: EvaluationContext) => { @@ -265,16 +272,6 @@ export class FlagsClient { defaultValue: T, type: 'boolean' | 'string' | 'number' | 'object' ): FlagDetails => { - if (this.configurationStatus === 'mismatch') { - return { - key, - value: defaultValue, - reason: 'ERROR', - errorCode: 'INVALID_CONTEXT', - errorMessage: `The loaded configuration for '${this.clientName}' does not match the active evaluation context.` - }; - } - // A loaded-but-unusable configuration surfaces as PROVIDER_NOT_READY at the // evaluation layer (distinct from FLAG_NOT_FOUND). The dedicated PROVIDER_ERROR // provider event is wired by the OpenFeature provider in a later step. diff --git a/packages/core/src/flags/__tests__/FlagsClient.test.ts b/packages/core/src/flags/__tests__/FlagsClient.test.ts index f49fea1d9..af1245bbe 100644 --- a/packages/core/src/flags/__tests__/FlagsClient.test.ts +++ b/packages/core/src/flags/__tests__/FlagsClient.test.ts @@ -385,7 +385,7 @@ describe('FlagsClient', () => { ); }); - it('returns INVALID_CONTEXT when the config does not match an explicit context', async () => { + it('ignores a differing explicit context, serving the snapshot and warning', async () => { const flagsClient = DdFlags.getClient(); await flagsClient.setEvaluationContext({ targetingKey: 'user-1', @@ -399,13 +399,15 @@ describe('FlagsClient', () => { }) ); - expect( - flagsClient.getBooleanDetails('offline-bool', false) - ).toMatchObject({ - value: false, - reason: 'ERROR', - errorCode: 'INVALID_CONTEXT' - }); + // Offline precomputed is single-subject: the differing context is ignored and + // the snapshot is served for its embedded context, with a warning. + expect(flagsClient.getBooleanValue('offline-bool', false)).toBe( + true + ); + expect(InternalLog.log).toHaveBeenCalledWith( + expect.stringContaining('Ignoring the evaluation context'), + expect.anything() + ); }); it('returns PROVIDER_NOT_READY for an empty/invalid configuration', () => { @@ -534,7 +536,7 @@ describe('FlagsClient', () => { ).not.toHaveBeenCalled(); }); - it('re-validates on a context change (mismatch -> INVALID_CONTEXT), still no fetch', () => { + it('ignores a differing context change, still serving without fetching', () => { const flagsClient = DdFlags.getClient(); flagsClient.setConfiguration( buildConfig({ targetingKey: 'user-1', country: 'US' }) @@ -545,13 +547,15 @@ describe('FlagsClient', () => { attributes: { country: 'US' } }); - expect( - flagsClient.getBooleanDetails('offline-bool', false) - ).toMatchObject({ - value: false, - reason: 'ERROR', - errorCode: 'INVALID_CONTEXT' - }); + // The differing context is ignored (offline never fetches) and the snapshot is + // still served for its embedded context, with a warning. + expect(flagsClient.getBooleanValue('offline-bool', false)).toBe( + true + ); + expect(InternalLog.log).toHaveBeenCalledWith( + expect.stringContaining('Ignoring the evaluation context'), + expect.anything() + ); expect( NativeModules.DdFlags.setEvaluationContext ).not.toHaveBeenCalled(); diff --git a/packages/core/src/flags/configuration/__tests__/context.test.ts b/packages/core/src/flags/configuration/__tests__/context.test.ts index f75891993..ec0155bad 100644 --- a/packages/core/src/flags/configuration/__tests__/context.test.ts +++ b/packages/core/src/flags/configuration/__tests__/context.test.ts @@ -51,7 +51,9 @@ describe('normalizeWireContext', () => { ['__proto__']: 'x' }); - // The reserved key is safely discarded; the prototype is untouched. + // A reserved "__proto__" attribute does not pollute the prototype: the normalized + // attributes keep `Object.prototype` and nothing leaks onto the global prototype. + // (Whether the key is retained or dropped is an implementation detail we don't assert.) expect(Object.getPrototypeOf(normalized.attributes)).toBe( Object.prototype ); diff --git a/packages/core/src/flags/configuration/index.ts b/packages/core/src/flags/configuration/index.ts index 0ce6673c6..6a943c9b2 100644 --- a/packages/core/src/flags/configuration/index.ts +++ b/packages/core/src/flags/configuration/index.ts @@ -14,7 +14,9 @@ export { decodePrecomputedFlags, UnsupportedConfigurationError } from './precomputed'; -export { contextMatchesConfiguration, normalizeWireContext } from './context'; +// `contextMatchesConfiguration` is intentionally NOT re-exported — it is an internal +// helper consumed directly by `FlagsClient` (see its import from `./configuration/context`). +export { normalizeWireContext } from './context'; export type { ParsedFlagsConfiguration, ParsedPrecomputedConfiguration, From 8a5d4c49f32b8b3d8bfd372aa551cc90e32cf49e Mon Sep 17 00:00:00 2001 From: Blake Date: Fri, 17 Jul 2026 08:58:07 -0400 Subject: [PATCH 05/13] docs(flags): drop stale context/server comments in FlagsClient (PR2) Follow-up to the ignore-a-mismatched-context change: the setEvaluationContextWithoutFetching doc no longer describes 'context matching' (it re-applies and ignores a differing precomputed context), and the applyConfiguration forward-compat note drops the reference to the removed 'server' wire branch / ParsedFlagsConfiguration while keeping the (still-valid) rules seam. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/core/src/flags/FlagsClient.ts | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/packages/core/src/flags/FlagsClient.ts b/packages/core/src/flags/FlagsClient.ts index 5ea813c10..e23965ade 100644 --- a/packages/core/src/flags/FlagsClient.ts +++ b/packages/core/src/flags/FlagsClient.ts @@ -112,13 +112,15 @@ export class FlagsClient { /** * Set the evaluation context **without** fetching a configuration from the network, - * then reconcile any configuration loaded via {@link setConfiguration} against it. + * then re-apply any configuration loaded via {@link setConfiguration} against it. * * This is the offline counterpart to {@link setEvaluationContext}: it records the - * active context and re-evaluates the loaded configuration (context matching, for a - * precomputed configuration) with no native request. It is intended for offline - * providers that own their configuration via `setConfiguration` and must not fetch on - * a context change. With no configuration loaded yet, the context is simply recorded. + * active context and re-applies the loaded configuration with no native request. A + * precomputed configuration is single-subject, so a context that differs from the one + * it was computed for is ignored (the snapshot keeps serving) with a warning. Intended + * for offline providers that own their configuration via `setConfiguration` and must + * not fetch on a context change. With no configuration loaded, previously served flags + * are cleared. * * @param context The evaluation context to associate with the current client. */ @@ -171,11 +173,11 @@ export class FlagsClient { const precomputed = this.loadedConfiguration?.precomputed; // Only precomputed configurations are supported for now. An empty configuration - // (an invalid/failed wire parse, or a server-only wire) is not usable. + // (an invalid/failed wire parse, or a wire with no precomputed branch) is not usable. // - // FORWARD-COMPAT SEAM: when the `server`/rules branch is parsed (see - // `ParsedFlagsConfiguration`), it must be handled BEFORE this guard — a rules - // configuration is context-agnostic and must NOT be rejected here as `invalid`. + // FORWARD-COMPAT SEAM: when a rules-based configuration is supported, it must be + // handled BEFORE this guard — rules are context-agnostic and must NOT be rejected + // here as `invalid`. if (!precomputed) { this.flagsCache = {}; this.configurationStatus = 'invalid'; From 190a040231c15f7a630e5c06554dd9f8942426b8 Mon Sep 17 00:00:00 2001 From: Blake Date: Fri, 17 Jul 2026 10:15:58 -0400 Subject: [PATCH 06/13] refactor(flags): address PR2 review findings (dead code, wire safety, tests) - Remove the unused 'INVALID_CONTEXT' error code from FlagErrorCode: it was added for the old context-mismatch path, which the ignore-and-warn change removed, so the SDK never emits it. - Guard a non-string wire targetingKey in normalizeWireContext (the wire is untrusted): a non-string value is treated as absent instead of landing in a string-typed field. - Correct the 'warn once' comment (it warns on each differing context change, which is the intended behavior). - Add a test asserting exposures are attributed to the snapshot's embedded context, not an ignored runtime context. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/core/src/flags/FlagsClient.ts | 4 ++-- .../src/flags/__tests__/FlagsClient.test.ts | 24 +++++++++++++++++++ .../core/src/flags/configuration/context.ts | 3 ++- packages/core/src/flags/types.ts | 3 +-- 4 files changed, 29 insertions(+), 5 deletions(-) diff --git a/packages/core/src/flags/FlagsClient.ts b/packages/core/src/flags/FlagsClient.ts index e23965ade..974ba881a 100644 --- a/packages/core/src/flags/FlagsClient.ts +++ b/packages/core/src/flags/FlagsClient.ts @@ -228,8 +228,8 @@ export class FlagsClient { // bound to the context it was computed for, and offline never fetches, so it cannot // be recomputed for a different subject. A runtime context that does not match is // therefore IGNORED: revert to the embedded context, keep serving the snapshot, and - // warn once. (Precomputed is single-subject by design — a rules-based configuration - // is the path for per-context evaluation.) + // warn on the change. (Precomputed is single-subject by design — a rules-based + // configuration is the path for per-context evaluation.) if ( !contextMatchesConfiguration( precomputed.context, diff --git a/packages/core/src/flags/__tests__/FlagsClient.test.ts b/packages/core/src/flags/__tests__/FlagsClient.test.ts index af1245bbe..bbe3f19e1 100644 --- a/packages/core/src/flags/__tests__/FlagsClient.test.ts +++ b/packages/core/src/flags/__tests__/FlagsClient.test.ts @@ -560,5 +560,29 @@ describe('FlagsClient', () => { NativeModules.DdFlags.setEvaluationContext ).not.toHaveBeenCalled(); }); + + it('attributes exposures to the embedded context when a differing context is ignored', () => { + const flagsClient = DdFlags.getClient(); + flagsClient.setConfiguration( + buildConfig({ targetingKey: 'user-1', country: 'US' }) + ); + + flagsClient.setEvaluationContextWithoutFetching({ + targetingKey: 'user-2', + attributes: { country: 'US' } + }); + + flagsClient.getBooleanValue('offline-bool', false); + + // The exposure is attributed to the snapshot's embedded context (user-1), not + // the ignored runtime context (user-2). + expect(NativeModules.DdFlags.trackEvaluation).toHaveBeenCalledWith( + expect.any(String), + 'offline-bool', + expect.any(Object), + 'user-1', + expect.objectContaining({ country: 'US' }) + ); + }); }); }); diff --git a/packages/core/src/flags/configuration/context.ts b/packages/core/src/flags/configuration/context.ts index 06044db02..436ad96b5 100644 --- a/packages/core/src/flags/configuration/context.ts +++ b/packages/core/src/flags/configuration/context.ts @@ -21,7 +21,8 @@ export const normalizeWireContext = ( const { targetingKey, ...attributes } = wireContext; return processEvaluationContext({ - targetingKey: targetingKey ?? '', + // The wire is untrusted, so a non-string targetingKey is treated as absent. + targetingKey: typeof targetingKey === 'string' ? targetingKey : '', // `processEvaluationContext` drops non-primitive attributes; casting here mirrors // how the active context's attributes are typed before that same processing. attributes: attributes as Record diff --git a/packages/core/src/flags/types.ts b/packages/core/src/flags/types.ts index e451f4e72..12bfb18f6 100644 --- a/packages/core/src/flags/types.ts +++ b/packages/core/src/flags/types.ts @@ -169,8 +169,7 @@ type FlagErrorCode = | 'PROVIDER_NOT_READY' | 'FLAG_NOT_FOUND' | 'PARSE_ERROR' - | 'TYPE_MISMATCH' - | 'INVALID_CONTEXT'; + | 'TYPE_MISMATCH'; /** * Detailed information about a feature flag evaluation. From 084a09a40b4a4960664987b9c00359f80daeade3 Mon Sep 17 00:00:00 2001 From: Blake Date: Fri, 17 Jul 2026 10:40:02 -0400 Subject: [PATCH 07/13] refactor(flags): store flagsCache as a Map to avoid prototype-chain lookups (PR2 review) flagsCache was a plain object indexed by flag key, so a key equal to an inherited property name ('toString', 'constructor', '__proto__') resolved an Object.prototype member instead of undefined, bypassing the not-found guard and returning TYPE_MISMATCH rather than FLAG_NOT_FOUND. Use a Map with .get(): the online native result is wrapped via new Map(Object.entries(result)), the offline path consumes the Map from decodePrecomputedFlags directly, and lookups never touch the prototype chain. Fixes it for both online and offline paths. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/core/src/flags/FlagsClient.ts | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/core/src/flags/FlagsClient.ts b/packages/core/src/flags/FlagsClient.ts index 974ba881a..4881cc552 100644 --- a/packages/core/src/flags/FlagsClient.ts +++ b/packages/core/src/flags/FlagsClient.ts @@ -34,7 +34,11 @@ export class FlagsClient { private evaluationContext: EvaluationContext | undefined = undefined; - private flagsCache: Record = {}; + // A Map (not a plain object) so a flag keyed like an inherited property + // ("toString", "constructor", "__proto__") is looked up as data via `.get()` and never + // resolves an `Object.prototype` member. This matters now that the cache can be fed + // from untrusted wire data via `setConfiguration`. + private flagsCache: Map = new Map(); private loadedConfiguration: | ParsedFlagsConfiguration @@ -83,7 +87,7 @@ export class FlagsClient { ); this.evaluationContext = processedContext; - this.flagsCache = result; + this.flagsCache = new Map(Object.entries(result)); // An explicit online fetch supersedes any previously loaded offline // configuration, so drop the offline overlay to keep state coherent. @@ -135,7 +139,7 @@ export class FlagsClient { } else { // No offline configuration is engaged: do not keep serving any previously // cached flags (e.g. from a prior online fetch) against the new context. - this.flagsCache = {}; + this.flagsCache = new Map(); this.configurationStatus = 'none'; } }; @@ -179,7 +183,7 @@ export class FlagsClient { // handled BEFORE this guard — rules are context-agnostic and must NOT be rejected // here as `invalid`. if (!precomputed) { - this.flagsCache = {}; + this.flagsCache = new Map(); this.configurationStatus = 'invalid'; InternalLog.log( `No usable precomputed configuration was provided for '${this.clientName}'.`, @@ -188,11 +192,11 @@ export class FlagsClient { return; } - let decoded: Record; + let decoded: Map; try { decoded = decodePrecomputedFlags(precomputed.response); } catch (error) { - this.flagsCache = {}; + this.flagsCache = new Map(); this.configurationStatus = 'invalid'; if (error instanceof Error) { InternalLog.log( @@ -298,7 +302,7 @@ export class FlagsClient { } // Retrieve the flag from the cache. - const flag = this.flagsCache[key]; + const flag = this.flagsCache.get(key); if (!flag) { return { From 41ab2ac5dd4d39a4f33f43894c72340fbeed9a46 Mon Sep 17 00:00:00 2001 From: Blake Date: Mon, 20 Jul 2026 17:28:07 -0400 Subject: [PATCH 08/13] refactor(flags): model ConfigurationStatus as an as-const object (PR2 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the bare string union with an `as const` object + derived union so assignment/comparison sites read ConfigurationStatus.None/Ready/Invalid — self-documenting and typo-safe — while the type stays an erasable string union (no TS enum runtime code, friendlier to Babel and tree-shaking). Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/core/src/flags/FlagsClient.ts | 33 +++++++++++++++++++------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/packages/core/src/flags/FlagsClient.ts b/packages/core/src/flags/FlagsClient.ts index 4881cc552..f691e409a 100644 --- a/packages/core/src/flags/FlagsClient.ts +++ b/packages/core/src/flags/FlagsClient.ts @@ -22,8 +22,23 @@ import type { JsonValue, EvaluationContext, FlagDetails } from './types'; * Tracks how a configuration supplied via {@link FlagsClient.setConfiguration} relates * to the active evaluation context. `'none'` means no offline configuration is engaged * (the online/fetch path is in effect). + * + * Modelled as an `as const` object rather than a TS `enum` so call sites read + * `ConfigurationStatus.Ready` — self-documenting and typo-safe — while the type stays a + * plain string union that erases at build time (no runtime enum code, friendlier to Babel + * and tree-shaking). */ -type ConfigurationStatus = 'none' | 'ready' | 'invalid'; +const ConfigurationStatus = { + None: 'none', + Ready: 'ready', + Invalid: 'invalid' +} as const; + +// Intentional value + type merging: the const above provides the `.Ready`/`.None`/`.Invalid` +// accessors, this alias provides the string-union type. Same name is the idiomatic pattern; +// no-redeclare doesn't model TS's separate value/type namespaces. +// eslint-disable-next-line no-redeclare, @typescript-eslint/no-redeclare +type ConfigurationStatus = typeof ConfigurationStatus[keyof typeof ConfigurationStatus]; export class FlagsClient { // eslint-disable-next-line global-require, @typescript-eslint/no-var-requires @@ -44,7 +59,7 @@ export class FlagsClient { | ParsedFlagsConfiguration | undefined = undefined; - private configurationStatus: ConfigurationStatus = 'none'; + private configurationStatus: ConfigurationStatus = ConfigurationStatus.None; constructor(clientName: string = 'default') { this.clientName = clientName; @@ -97,7 +112,7 @@ export class FlagsClient { // `applyConfiguration()` against the new context instead of dropping the // loaded configuration. this.loadedConfiguration = undefined; - this.configurationStatus = 'none'; + this.configurationStatus = ConfigurationStatus.None; } catch (error) { // NOTE: a failed fetch leaves any previously loaded offline configuration in // place, so the client may keep serving it (and attribute exposures to its @@ -140,7 +155,7 @@ export class FlagsClient { // No offline configuration is engaged: do not keep serving any previously // cached flags (e.g. from a prior online fetch) against the new context. this.flagsCache = new Map(); - this.configurationStatus = 'none'; + this.configurationStatus = ConfigurationStatus.None; } }; @@ -184,7 +199,7 @@ export class FlagsClient { // here as `invalid`. if (!precomputed) { this.flagsCache = new Map(); - this.configurationStatus = 'invalid'; + this.configurationStatus = ConfigurationStatus.Invalid; InternalLog.log( `No usable precomputed configuration was provided for '${this.clientName}'.`, SdkVerbosity.WARN @@ -197,7 +212,7 @@ export class FlagsClient { decoded = decodePrecomputedFlags(precomputed.response); } catch (error) { this.flagsCache = new Map(); - this.configurationStatus = 'invalid'; + this.configurationStatus = ConfigurationStatus.Invalid; if (error instanceof Error) { InternalLog.log( `Unsupported flags configuration for '${this.clientName}': ${error.message}`, @@ -224,7 +239,7 @@ export class FlagsClient { } this.flagsCache = decoded; - this.configurationStatus = 'ready'; + this.configurationStatus = ConfigurationStatus.Ready; return; } @@ -250,7 +265,7 @@ export class FlagsClient { } this.flagsCache = decoded; - this.configurationStatus = 'ready'; + this.configurationStatus = ConfigurationStatus.Ready; }; private track = (flag: FlagCacheEntry, context: EvaluationContext) => { @@ -281,7 +296,7 @@ export class FlagsClient { // A loaded-but-unusable configuration surfaces as PROVIDER_NOT_READY at the // evaluation layer (distinct from FLAG_NOT_FOUND). The dedicated PROVIDER_ERROR // provider event is wired by the OpenFeature provider in a later step. - if (this.configurationStatus === 'invalid') { + if (this.configurationStatus === ConfigurationStatus.Invalid) { return { key, value: defaultValue, From af0cb9d20285dcd36c13b4081146bc53f1cea61b Mon Sep 17 00:00:00 2001 From: Blake Date: Tue, 21 Jul 2026 15:52:15 -0400 Subject: [PATCH 09/13] refactor(flags): error on context mismatch for offline precomputed (PR2) Switch the offline precomputed flow from "ignore a mismatched runtime context and keep serving the snapshot" to treating it as an error that serves coded defaults, so the OpenFeature provider can surface the correct error state. - Widen the offline APIs to return a discriminated ConfigurationResult (ready, or error with a precise errorCode) so the reason propagates: INVALID_CONTEXT (mismatch), PROVIDER_NOT_READY (no config loaded), GENERAL (unusable/undecodable/unsupported config). - Store load validity structurally (LoadedConfigurationState) and decode once at load, so a later context change can never promote an invalid load to ready and never re-decodes. - Add resetEvaluationContextWithoutFetching (clear the external override and reconcile) so clearing/omitting context re-adopts the embedded context. - getDetails serves coded defaults with the precise errorCode on error; no exposure tracking while serving defaults. - Add INVALID_CONTEXT and GENERAL to FlagErrorCode; drop the stale fetch-policy comments in setEvaluationContext. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/core/src/flags/FlagsClient.ts | 308 ++++++++++------ .../src/flags/__tests__/FlagsClient.test.ts | 337 +++++++++++++----- packages/core/src/flags/types.ts | 4 +- 3 files changed, 431 insertions(+), 218 deletions(-) diff --git a/packages/core/src/flags/FlagsClient.ts b/packages/core/src/flags/FlagsClient.ts index f691e409a..0f1a0725e 100644 --- a/packages/core/src/flags/FlagsClient.ts +++ b/packages/core/src/flags/FlagsClient.ts @@ -9,36 +9,59 @@ import { SdkVerbosity } from '../config/types/SdkVerbosity'; import type { DdNativeFlagsType } from '../nativeModulesTypes'; // Imported directly (not via the module index): context matching is an internal helper, -// used here only to detect and warn about a runtime context that an offline precomputed -// configuration cannot honor. +// used here only to detect a runtime context an offline precomputed configuration cannot honor. import { contextMatchesConfiguration } from './configuration/context'; import { decodePrecomputedFlags, normalizeWireContext } from './configuration'; -import type { ParsedFlagsConfiguration } from './configuration'; +import type { + ParsedFlagsConfiguration, + ParsedPrecomputedConfiguration +} from './configuration'; import { processEvaluationContext } from './internal'; import type { FlagCacheEntry } from './internal'; import type { JsonValue, EvaluationContext, FlagDetails } from './types'; /** - * Tracks how a configuration supplied via {@link FlagsClient.setConfiguration} relates - * to the active evaluation context. `'none'` means no offline configuration is engaged - * (the online/fetch path is in effect). + * Error codes an offline configuration result can carry: + * - `INVALID_CONTEXT`: the active context does not match the precomputed snapshot. + * - `PROVIDER_NOT_READY`: an offline operation ran with no configuration loaded. + * - `GENERAL`: the loaded configuration is unusable (malformed/unsupported/undecodable). + */ +export type ConfigurationErrorCode = + | 'INVALID_CONTEXT' + | 'PROVIDER_NOT_READY' + | 'GENERAL'; + +/** + * Outcome of loading a configuration or reconciling it against the active evaluation context, + * returned by the offline APIs ({@link FlagsClient.setConfiguration}, + * {@link FlagsClient.setEvaluationContextWithoutFetching}, + * {@link FlagsClient.resetEvaluationContextWithoutFetching}). + * + * `'ready'` means the configuration can be served against the active context; `'error'` carries + * the precise reason so the OpenFeature provider surfaces the correct code and evaluation returns + * that code with the coded default. + * + * @internal Consumers observe OpenFeature provider events/evaluation details, not this result. + */ +export type ConfigurationResult = + | { status: 'ready' } + | { status: 'error'; errorCode: ConfigurationErrorCode }; + +/** + * The decoded outcome of the last loaded offline configuration. Load validity (`kind`) is stored + * separately from context compatibility so that later context changes reconcile against a decoded + * snapshot **without re-decoding** and can never promote an invalid load to a servable state. * - * Modelled as an `as const` object rather than a TS `enum` so call sites read - * `ConfigurationStatus.Ready` — self-documenting and typo-safe — while the type stays a - * plain string union that erases at build time (no runtime enum code, friendlier to Babel - * and tree-shaking). + * `'none'` = no offline configuration engaged (the online/fetch path, or nothing loaded yet). */ -const ConfigurationStatus = { - None: 'none', - Ready: 'ready', - Invalid: 'invalid' -} as const; - -// Intentional value + type merging: the const above provides the `.Ready`/`.None`/`.Invalid` -// accessors, this alias provides the string-union type. Same name is the idiomatic pattern; -// no-redeclare doesn't model TS's separate value/type namespaces. -// eslint-disable-next-line no-redeclare, @typescript-eslint/no-redeclare -type ConfigurationStatus = typeof ConfigurationStatus[keyof typeof ConfigurationStatus]; +type LoadedConfigurationState = + | { kind: 'none' } + | { kind: 'invalid'; errorCode: ConfigurationErrorCode } + | { + kind: 'precomputed'; + configuration: ParsedPrecomputedConfiguration; + flags: Map; + }; export class FlagsClient { // eslint-disable-next-line global-require, @typescript-eslint/no-var-requires @@ -49,17 +72,23 @@ export class FlagsClient { private evaluationContext: EvaluationContext | undefined = undefined; - // A Map (not a plain object) so a flag keyed like an inherited property - // ("toString", "constructor", "__proto__") is looked up as data via `.get()` and never - // resolves an `Object.prototype` member. This matters now that the cache can be fed - // from untrusted wire data via `setConfiguration`. + // The servable flag cache. On the online path it holds the native-fetched flags; on the + // offline path it mirrors the decoded snapshot when the configuration is servable. A Map + // (not a plain object) so a flag keyed like an inherited property ("toString", + // "__proto__") is looked up as data via `.get()` and never resolves an `Object.prototype` + // member — important now the cache can be fed from untrusted wire data. private flagsCache: Map = new Map(); - private loadedConfiguration: - | ParsedFlagsConfiguration - | undefined = undefined; + // The decoded outcome of the last loaded offline configuration (see the type). + private loadedConfiguration: LoadedConfigurationState = { kind: 'none' }; + + // Internal serving status. `'none'` = online path (serve `flagsCache`); `'ready'` = serve + // the offline snapshot; `'error'` = serve coded defaults with `configurationError`. + private configurationStatus: 'none' | 'ready' | 'error' = 'none'; - private configurationStatus: ConfigurationStatus = ConfigurationStatus.None; + private configurationError: + | { errorCode: ConfigurationErrorCode; errorMessage: string } + | undefined = undefined; constructor(clientName: string = 'default') { this.clientName = clientName; @@ -104,20 +133,16 @@ export class FlagsClient { this.evaluationContext = processedContext; this.flagsCache = new Map(Object.entries(result)); - // An explicit online fetch supersedes any previously loaded offline - // configuration, so drop the offline overlay to keep state coherent. - // - // PROVISIONAL: this reflects the current fetch-always default. When the - // `NEVER` fetch policy lands, that path must skip the fetch and re-run - // `applyConfiguration()` against the new context instead of dropping the - // loaded configuration. - this.loadedConfiguration = undefined; - this.configurationStatus = ConfigurationStatus.None; + // An explicit online fetch supersedes any previously loaded offline configuration. + // Online vs. offline is a property of the provider — the offline provider adopts + // context via `setEvaluationContextWithoutFetching` and never calls this method — so + // drop the offline overlay here to keep state coherent. + this.loadedConfiguration = { kind: 'none' }; + this.configurationStatus = 'none'; + this.configurationError = undefined; } catch (error) { // NOTE: a failed fetch leaves any previously loaded offline configuration in - // place, so the client may keep serving it (and attribute exposures to its - // context). Fetch-failure/staleness fallback is deferred to the fetch-policy - // step. + // place, so the client may keep serving it (and attribute exposures to its context). if (error instanceof Error) { InternalLog.log( `Error setting flag evaluation context: ${error.message}`, @@ -130,44 +155,49 @@ export class FlagsClient { }; /** - * Set the evaluation context **without** fetching a configuration from the network, - * then re-apply any configuration loaded via {@link setConfiguration} against it. + * Set the evaluation context **without** fetching a configuration from the network, then + * reconcile the loaded configuration against it. * - * This is the offline counterpart to {@link setEvaluationContext}: it records the - * active context and re-applies the loaded configuration with no native request. A - * precomputed configuration is single-subject, so a context that differs from the one - * it was computed for is ignored (the snapshot keeps serving) with a warning. Intended - * for offline providers that own their configuration via `setConfiguration` and must - * not fetch on a context change. With no configuration loaded, previously served flags - * are cleared. + * The offline counterpart to {@link setEvaluationContext}: records the active context and + * reconciles with no native request. A precomputed configuration is single-subject, so a + * context that does not match the one it was computed for reconciles to an error + * (`INVALID_CONTEXT`) and evaluation serves defaults. With no configuration loaded the + * result is `PROVIDER_NOT_READY`. Intended for offline providers that own their + * configuration via `setConfiguration` and must not fetch on a context change. * * @param context The evaluation context to associate with the current client. */ setEvaluationContextWithoutFetching = ( context: EvaluationContext - ): void => { + ): ConfigurationResult => { this.evaluationContext = processEvaluationContext(context); - // Re-evaluate a loaded offline configuration against the new context. - if (this.loadedConfiguration) { - this.applyConfiguration(); - } else { - // No offline configuration is engaged: do not keep serving any previously - // cached flags (e.g. from a prior online fetch) against the new context. - this.flagsCache = new Map(); - this.configurationStatus = ConfigurationStatus.None; - } + return this.reconcile(); + }; + + /** + * Clear any externally-set evaluation context and reconcile. + * + * This is the offline counterpart to clearing/omitting an OpenFeature context: it drops the + * external override so a loaded precomputed configuration is served against **its embedded + * context** again. Clearing the override (rather than skipping) matters so that a + * configuration loaded *after* a clear is not judged against a stale override. With no + * configuration loaded the result is `PROVIDER_NOT_READY`. + */ + resetEvaluationContextWithoutFetching = (): ConfigurationResult => { + this.evaluationContext = undefined; + + return this.reconcile(); }; /** * Load a configuration (parsed from a `ConfigurationWire` string via - * `configurationFromString`) into the client for offline evaluation. + * `configurationFromString`) into the client for offline evaluation, then reconcile it + * against the active context. * - * For a precomputed configuration this populates the flag cache and adopts the - * configuration's embedded evaluation context — **no network request is made**. A - * precomputed snapshot is single-subject: if a different runtime context has been set, - * it is ignored (the snapshot is served for its embedded context) and a warning is - * logged. Use a rules-based configuration for per-context evaluation. + * For a precomputed configuration this decodes the snapshot once and adopts its embedded + * evaluation context when none is set — **no network request is made**. An unusable + * configuration reconciles to an error (`GENERAL`); a context mismatch to `INVALID_CONTEXT`. * * @param configuration The configuration to load. * @@ -179,56 +209,90 @@ export class FlagsClient { * const value = flagsClient.getBooleanValue('new-feature', false); * ``` */ - setConfiguration = (configuration: ParsedFlagsConfiguration): void => { - this.loadedConfiguration = configuration; - this.applyConfiguration(); + setConfiguration = ( + configuration: ParsedFlagsConfiguration + ): ConfigurationResult => { + this.loadedConfiguration = this.loadConfiguration(configuration); + + return this.reconcile(); }; /** - * Reconcile the loaded configuration against the active evaluation context and - * (re)compute the servable flag cache and configuration status. + * Decode a supplied configuration into a {@link LoadedConfigurationState} **once**, capturing + * whether the load itself is valid (independent of the active context). Later context changes + * reconcile against this stored state without re-decoding, and can never turn an invalid load + * into a servable one. + * + * FORWARD-COMPAT SEAM: when a rules-based configuration is supported, it must be handled here + * BEFORE the precomputed guard — rules are context-agnostic and must NOT be classified invalid. */ - private applyConfiguration = (): void => { - const precomputed = this.loadedConfiguration?.precomputed; - - // Only precomputed configurations are supported for now. An empty configuration - // (an invalid/failed wire parse, or a wire with no precomputed branch) is not usable. - // - // FORWARD-COMPAT SEAM: when a rules-based configuration is supported, it must be - // handled BEFORE this guard — rules are context-agnostic and must NOT be rejected - // here as `invalid`. + private loadConfiguration = ( + configuration: ParsedFlagsConfiguration + ): LoadedConfigurationState => { + const precomputed = configuration?.precomputed; + + // An empty configuration (a failed/lenient wire parse, or a wire with no precomputed + // branch) is unusable. `configurationFromString` collapses malformed input and + // unsupported versions to the same empty shape, so this is classified as `GENERAL`. if (!precomputed) { - this.flagsCache = new Map(); - this.configurationStatus = ConfigurationStatus.Invalid; InternalLog.log( `No usable precomputed configuration was provided for '${this.clientName}'.`, SdkVerbosity.WARN ); - return; + return { kind: 'invalid', errorCode: 'GENERAL' }; } - let decoded: Map; try { - decoded = decodePrecomputedFlags(precomputed.response); + const flags = decodePrecomputedFlags(precomputed.response); + + return { kind: 'precomputed', configuration: precomputed, flags }; } catch (error) { - this.flagsCache = new Map(); - this.configurationStatus = ConfigurationStatus.Invalid; + // Decoding rejects unsupported payloads (e.g. obfuscated) — an unsupported kind, + // classified as `GENERAL`. if (error instanceof Error) { InternalLog.log( `Unsupported flags configuration for '${this.clientName}': ${error.message}`, SdkVerbosity.WARN ); } - return; + return { kind: 'invalid', errorCode: 'GENERAL' }; + } + }; + + /** + * Reconcile the stored {@link LoadedConfigurationState} against the active evaluation context + * and (re)compute the servable flag cache and configuration status/result. + * + * The stored load validity is consulted **before** context compatibility, so a context change + * can never promote an invalid (or absent) load to `ready`. + */ + private reconcile = (): ConfigurationResult => { + const loaded = this.loadedConfiguration; + + // No offline configuration engaged: an offline operation with nothing loaded is not ready. + if (loaded.kind === 'none') { + return this.enterError( + 'PROVIDER_NOT_READY', + `The evaluation context is not usable for '${this.clientName}': no configuration is loaded. Provide a configuration via setConfiguration.` + ); } - // If no context has been set yet, adopt the configuration's embedded context - // (implicit set — no native fetch). A context-agnostic configuration falls back - // to an empty context so evaluation can proceed. + // The load itself failed (malformed/unsupported/undecodable) — independent of context. + if (loaded.kind === 'invalid') { + return this.enterError( + loaded.errorCode, + `The loaded configuration for '${this.clientName}' is not usable. Provide a valid precomputed configuration.` + ); + } + + const { configuration, flags } = loaded; + + // Adopt the configuration's embedded context when no external context is set (implicit + // set — no native fetch). A context-agnostic configuration falls back to an empty context. if (!this.evaluationContext) { - if (precomputed.context) { + if (configuration.context) { this.evaluationContext = normalizeWireContext( - precomputed.context + configuration.context ); } else { InternalLog.log( @@ -237,35 +301,43 @@ export class FlagsClient { ); this.evaluationContext = { targetingKey: '', attributes: {} }; } - - this.flagsCache = decoded; - this.configurationStatus = ConfigurationStatus.Ready; - return; } - // A context is already set. An offline precomputed configuration is a snapshot - // bound to the context it was computed for, and offline never fetches, so it cannot - // be recomputed for a different subject. A runtime context that does not match is - // therefore IGNORED: revert to the embedded context, keep serving the snapshot, and - // warn on the change. (Precomputed is single-subject by design — a rules-based - // configuration is the path for per-context evaluation.) + // A precomputed snapshot is bound to the subject it was computed for. A non-matching + // context cannot be served (offline never fetches), so it is an error and evaluation + // serves coded defaults. The decoded snapshot is retained for a later matching context. if ( !contextMatchesConfiguration( - precomputed.context, + configuration.context, this.evaluationContext ) ) { - InternalLog.log( - `Ignoring the evaluation context set for '${this.clientName}': an offline precomputed configuration is served against the context it was computed for. Set a matching context, or use a rules-based configuration for per-context evaluation.`, - SdkVerbosity.WARN + // The decoded snapshot stays in `loadedConfiguration.flags`; a later matching context + // reconciles back to `ready` and repopulates `flagsCache` — no re-decode needed. + return this.enterError( + 'INVALID_CONTEXT', + `The evaluation context does not match the precomputed configuration for '${this.clientName}'. Serving default values. Set a matching context, or use a rules-based configuration for per-context evaluation.` ); - this.evaluationContext = precomputed.context - ? normalizeWireContext(precomputed.context) - : { targetingKey: '', attributes: {} }; } - this.flagsCache = decoded; - this.configurationStatus = ConfigurationStatus.Ready; + this.flagsCache = flags; + this.configurationStatus = 'ready'; + this.configurationError = undefined; + + return { status: 'ready' }; + }; + + /** Record an error status + message, clear the servable cache, and return the result. */ + private enterError = ( + errorCode: ConfigurationErrorCode, + errorMessage: string + ): ConfigurationResult => { + this.flagsCache = new Map(); + this.configurationStatus = 'error'; + this.configurationError = { errorCode, errorMessage }; + InternalLog.log(errorMessage, SdkVerbosity.WARN); + + return { status: 'error', errorCode }; }; private track = (flag: FlagCacheEntry, context: EvaluationContext) => { @@ -293,16 +365,16 @@ export class FlagsClient { defaultValue: T, type: 'boolean' | 'string' | 'number' | 'object' ): FlagDetails => { - // A loaded-but-unusable configuration surfaces as PROVIDER_NOT_READY at the - // evaluation layer (distinct from FLAG_NOT_FOUND). The dedicated PROVIDER_ERROR - // provider event is wired by the OpenFeature provider in a later step. - if (this.configurationStatus === ConfigurationStatus.Invalid) { + // An offline configuration that cannot be served against the active context surfaces the + // precise error code (INVALID_CONTEXT / GENERAL / PROVIDER_NOT_READY) with the coded + // default. The OpenFeature provider maps this to a PROVIDER_ERROR / ERROR state. + if (this.configurationStatus === 'error' && this.configurationError) { return { key, value: defaultValue, reason: 'ERROR', - errorCode: 'PROVIDER_NOT_READY', - errorMessage: `The loaded configuration for '${this.clientName}' is not usable. Provide a valid precomputed configuration.` + errorCode: this.configurationError.errorCode, + errorMessage: this.configurationError.errorMessage }; } diff --git a/packages/core/src/flags/__tests__/FlagsClient.test.ts b/packages/core/src/flags/__tests__/FlagsClient.test.ts index bbe3f19e1..7668610d6 100644 --- a/packages/core/src/flags/__tests__/FlagsClient.test.ts +++ b/packages/core/src/flags/__tests__/FlagsClient.test.ts @@ -319,45 +319,47 @@ describe('FlagsClient', () => { }); }); - describe('setConfiguration', () => { - const offlineFlags = { - 'offline-bool': { - variationType: 'boolean', - variationValue: true, - variationKey: 'true', - allocationKey: 'alloc-1', - reason: 'STATIC', - doLog: false, - extraLogging: {} - } - }; - - const buildConfig = ( - flags: Record, - context?: Record - ) => - configurationFromString( - JSON.stringify({ - version: 1, - precomputed: { - response: JSON.stringify({ - data: { attributes: { obfuscated: false, flags } } - }), - context - } - }) - ); + const offlineFlags = { + 'offline-bool': { + variationType: 'boolean', + variationValue: true, + variationKey: 'true', + allocationKey: 'alloc-1', + reason: 'STATIC', + doLog: false, + extraLogging: {} + } + }; + + const buildConfig = ( + flags: Record, + context?: Record, + obfuscated = false + ) => + configurationFromString( + JSON.stringify({ + version: 1, + precomputed: { + response: JSON.stringify({ + data: { attributes: { obfuscated, flags } } + }), + context + } + }) + ); + describe('setConfiguration', () => { it('serves flags from the configuration without a native fetch', () => { const flagsClient = DdFlags.getClient(); - flagsClient.setConfiguration( + const result = flagsClient.setConfiguration( buildConfig(offlineFlags, { targetingKey: 'user-1', country: 'US' }) ); + expect(result).toEqual({ status: 'ready' }); expect(flagsClient.getBooleanValue('offline-bool', false)).toBe( true ); @@ -385,45 +387,69 @@ describe('FlagsClient', () => { ); }); - it('ignores a differing explicit context, serving the snapshot and warning', async () => { + it('errors with INVALID_CONTEXT and serves defaults when an explicit context differs', async () => { const flagsClient = DdFlags.getClient(); await flagsClient.setEvaluationContext({ targetingKey: 'user-1', attributes: { country: 'US' } }); - flagsClient.setConfiguration( + const result = flagsClient.setConfiguration( buildConfig(offlineFlags, { targetingKey: 'user-2', country: 'US' }) ); - // Offline precomputed is single-subject: the differing context is ignored and - // the snapshot is served for its embedded context, with a warning. - expect(flagsClient.getBooleanValue('offline-bool', false)).toBe( - true - ); - expect(InternalLog.log).toHaveBeenCalledWith( - expect.stringContaining('Ignoring the evaluation context'), - expect.anything() - ); + // Offline precomputed is single-subject: a differing context cannot be served, so + // it is an error and evaluation returns the coded default with INVALID_CONTEXT. + expect(result).toEqual({ + status: 'error', + errorCode: 'INVALID_CONTEXT' + }); + expect( + flagsClient.getBooleanDetails('offline-bool', false) + ).toMatchObject({ + value: false, + reason: 'ERROR', + errorCode: 'INVALID_CONTEXT' + }); + // No exposure is tracked while serving defaults. + expect( + NativeModules.DdFlags.trackEvaluation + ).not.toHaveBeenCalled(); }); - it('returns PROVIDER_NOT_READY for an empty/invalid configuration', () => { + it('errors with GENERAL for an empty/unparseable configuration', () => { const flagsClient = DdFlags.getClient(); - flagsClient.setConfiguration(configurationFromString('garbage')); + const result = flagsClient.setConfiguration( + configurationFromString('garbage') + ); + expect(result).toEqual({ status: 'error', errorCode: 'GENERAL' }); expect( flagsClient.getBooleanDetails('offline-bool', false) ).toMatchObject({ value: false, reason: 'ERROR', - errorCode: 'PROVIDER_NOT_READY' + errorCode: 'GENERAL' }); }); + it('errors with GENERAL for an unsupported (obfuscated) configuration', () => { + const flagsClient = DdFlags.getClient(); + + const result = flagsClient.setConfiguration( + buildConfig(offlineFlags, { targetingKey: 'user-1' }, true) + ); + + expect(result).toEqual({ status: 'error', errorCode: 'GENERAL' }); + expect( + flagsClient.getBooleanDetails('offline-bool', false) + ).toMatchObject({ errorCode: 'GENERAL' }); + }); + it('serves a context-agnostic configuration (no embedded context)', () => { const flagsClient = DdFlags.getClient(); @@ -461,6 +487,45 @@ describe('FlagsClient', () => { expect(flagsClient.getBooleanValue('flag-b', false)).toBe(true); }); + it('does not resurrect a prior valid snapshot after an invalid replacement', () => { + const flagsClient = DdFlags.getClient(); + + // Valid A. + flagsClient.setConfiguration( + buildConfig(offlineFlags, { targetingKey: 'user-1' }) + ); + expect(flagsClient.getBooleanValue('offline-bool', false)).toBe( + true + ); + + // Invalid replacement. + expect( + flagsClient.setConfiguration(configurationFromString('garbage')) + ).toEqual({ status: 'error', errorCode: 'GENERAL' }); + + // A later context change must NOT promote the invalid load back to ready. + const afterContextChange = flagsClient.setEvaluationContextWithoutFetching( + { targetingKey: 'user-1', attributes: {} } + ); + expect(afterContextChange).toEqual({ + status: 'error', + errorCode: 'GENERAL' + }); + expect( + flagsClient.getBooleanDetails('offline-bool', false) + ).toMatchObject({ value: false, errorCode: 'GENERAL' }); + + // A valid replacement recovers. + expect( + flagsClient.setConfiguration( + buildConfig(offlineFlags, { targetingKey: 'user-1' }) + ) + ).toEqual({ status: 'ready' }); + expect(flagsClient.getBooleanValue('offline-bool', false)).toBe( + true + ); + }); + it('is superseded by a later native fetch', async () => { const flagsClient = DdFlags.getClient(); flagsClient.setConfiguration( @@ -487,47 +552,21 @@ describe('FlagsClient', () => { }); describe('setEvaluationContextWithoutFetching', () => { - const offlineFlags = { - 'offline-bool': { - variationType: 'boolean', - variationValue: true, - variationKey: 'true', - allocationKey: 'alloc-1', - reason: 'STATIC', - doLog: false, - extraLogging: {} - } - }; - - const buildConfig = (context: Record) => - configurationFromString( - JSON.stringify({ - version: 1, - precomputed: { - response: JSON.stringify({ - data: { - attributes: { - obfuscated: false, - flags: offlineFlags - } - } - }), - context - } - }) - ); - - it('reconciles a loaded config against the context without fetching', () => { + it('reconciles a loaded config against a matching context without fetching', () => { const flagsClient = DdFlags.getClient(); flagsClient.setConfiguration( - buildConfig({ targetingKey: 'user-1', country: 'US' }) + buildConfig(offlineFlags, { + targetingKey: 'user-1', + country: 'US' + }) ); - flagsClient.setEvaluationContextWithoutFetching({ + const result = flagsClient.setEvaluationContextWithoutFetching({ targetingKey: 'user-1', attributes: { country: 'US' } }); + expect(result).toEqual({ status: 'ready' }); expect(flagsClient.getBooleanValue('offline-bool', false)).toBe( true ); @@ -536,53 +575,153 @@ describe('FlagsClient', () => { ).not.toHaveBeenCalled(); }); - it('ignores a differing context change, still serving without fetching', () => { + it('errors with INVALID_CONTEXT on a differing context change, serving defaults', () => { const flagsClient = DdFlags.getClient(); flagsClient.setConfiguration( - buildConfig({ targetingKey: 'user-1', country: 'US' }) + buildConfig(offlineFlags, { + targetingKey: 'user-1', + country: 'US' + }) ); - flagsClient.setEvaluationContextWithoutFetching({ + const result = flagsClient.setEvaluationContextWithoutFetching({ targetingKey: 'user-2', attributes: { country: 'US' } }); - // The differing context is ignored (offline never fetches) and the snapshot is - // still served for its embedded context, with a warning. + expect(result).toEqual({ + status: 'error', + errorCode: 'INVALID_CONTEXT' + }); + expect( + flagsClient.getBooleanDetails('offline-bool', false) + ).toMatchObject({ value: false, errorCode: 'INVALID_CONTEXT' }); + expect( + NativeModules.DdFlags.setEvaluationContext + ).not.toHaveBeenCalled(); + // Defaults are served, so no exposure is tracked. + expect( + NativeModules.DdFlags.trackEvaluation + ).not.toHaveBeenCalled(); + }); + + it('recovers to ready when a matching context is set after a mismatch', () => { + const flagsClient = DdFlags.getClient(); + flagsClient.setConfiguration( + buildConfig(offlineFlags, { targetingKey: 'user-1' }) + ); + + flagsClient.setEvaluationContextWithoutFetching({ + targetingKey: 'user-2', + attributes: {} + }); + expect( + flagsClient.getBooleanDetails('offline-bool', false) + ).toMatchObject({ errorCode: 'INVALID_CONTEXT' }); + + const recovered = flagsClient.setEvaluationContextWithoutFetching({ + targetingKey: 'user-1', + attributes: {} + }); + + expect(recovered).toEqual({ status: 'ready' }); expect(flagsClient.getBooleanValue('offline-bool', false)).toBe( true ); - expect(InternalLog.log).toHaveBeenCalledWith( - expect.stringContaining('Ignoring the evaluation context'), - expect.anything() - ); + }); + + it('returns PROVIDER_NOT_READY (not FLAG_NOT_FOUND) with no configuration loaded', () => { + const flagsClient = DdFlags.getClient(); + + const result = flagsClient.setEvaluationContextWithoutFetching({ + targetingKey: 'user-1', + attributes: {} + }); + + expect(result).toEqual({ + status: 'error', + errorCode: 'PROVIDER_NOT_READY' + }); expect( - NativeModules.DdFlags.setEvaluationContext - ).not.toHaveBeenCalled(); + flagsClient.getBooleanDetails('offline-bool', false) + ).toMatchObject({ + value: false, + reason: 'ERROR', + errorCode: 'PROVIDER_NOT_READY' + }); }); + }); - it('attributes exposures to the embedded context when a differing context is ignored', () => { + describe('resetEvaluationContextWithoutFetching', () => { + it('re-adopts the embedded context, recovering from a mismatch', () => { const flagsClient = DdFlags.getClient(); flagsClient.setConfiguration( - buildConfig({ targetingKey: 'user-1', country: 'US' }) + buildConfig(offlineFlags, { targetingKey: 'user-1' }) ); flagsClient.setEvaluationContextWithoutFetching({ targetingKey: 'user-2', - attributes: { country: 'US' } + attributes: {} }); + expect( + flagsClient.getBooleanDetails('offline-bool', false) + ).toMatchObject({ errorCode: 'INVALID_CONTEXT' }); - flagsClient.getBooleanValue('offline-bool', false); + const result = flagsClient.resetEvaluationContextWithoutFetching(); - // The exposure is attributed to the snapshot's embedded context (user-1), not - // the ignored runtime context (user-2). - expect(NativeModules.DdFlags.trackEvaluation).toHaveBeenCalledWith( - expect.any(String), - 'offline-bool', - expect.any(Object), - 'user-1', - expect.objectContaining({ country: 'US' }) + expect(result).toEqual({ status: 'ready' }); + expect(flagsClient.getBooleanValue('offline-bool', false)).toBe( + true ); }); + + it('clears the override so a config loaded after reset adopts its embedded context', () => { + const flagsClient = DdFlags.getClient(); + + // No config yet, an external context is set, then cleared. + flagsClient.setEvaluationContextWithoutFetching({ + targetingKey: 'user-2', + attributes: {} + }); + flagsClient.resetEvaluationContextWithoutFetching(); + + // Loading A now adopts A's embedded context (not the stale user-2 override). + const result = flagsClient.setConfiguration( + buildConfig(offlineFlags, { targetingKey: 'user-1' }) + ); + + expect(result).toEqual({ status: 'ready' }); + expect(flagsClient.getBooleanValue('offline-bool', false)).toBe( + true + ); + }); + + it('clears the override after an invalid load so a later valid load is ready', () => { + const flagsClient = DdFlags.getClient(); + + flagsClient.setConfiguration(configurationFromString('garbage')); + flagsClient.setEvaluationContextWithoutFetching({ + targetingKey: 'user-2', + attributes: {} + }); + flagsClient.resetEvaluationContextWithoutFetching(); + + const result = flagsClient.setConfiguration( + buildConfig(offlineFlags, { targetingKey: 'user-1' }) + ); + + expect(result).toEqual({ status: 'ready' }); + expect(flagsClient.getBooleanValue('offline-bool', false)).toBe( + true + ); + }); + + it('returns PROVIDER_NOT_READY with no configuration loaded', () => { + const flagsClient = DdFlags.getClient(); + + expect( + flagsClient.resetEvaluationContextWithoutFetching() + ).toEqual({ status: 'error', errorCode: 'PROVIDER_NOT_READY' }); + }); }); }); diff --git a/packages/core/src/flags/types.ts b/packages/core/src/flags/types.ts index 12bfb18f6..ccd0fe04b 100644 --- a/packages/core/src/flags/types.ts +++ b/packages/core/src/flags/types.ts @@ -169,7 +169,9 @@ type FlagErrorCode = | 'PROVIDER_NOT_READY' | 'FLAG_NOT_FOUND' | 'PARSE_ERROR' - | 'TYPE_MISMATCH'; + | 'TYPE_MISMATCH' + | 'INVALID_CONTEXT' + | 'GENERAL'; /** * Detailed information about a feature flag evaluation. From 0c77f19a64442ec02975f398dae35c45b3c12680 Mon Sep 17 00:00:00 2001 From: Blake Date: Tue, 21 Jul 2026 17:30:05 -0400 Subject: [PATCH 10/13] fix(flags): don't error when replacing an offline snapshot for a new subject MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Track the app-set external context separately from the effective evaluation context. Previously, adopting a configuration's embedded context stored it in `evaluationContext`, so loading a replacement snapshot for a different subject was mistaken for a mismatch against an "already set" context and returned INVALID_CONTEXT — even though the app never set a context. reconcile() now checks only an explicit `externalContext` for a mismatch; the effective context is `externalContext ?? embedded`. Replacing snapshot A with snapshot B (or loading a subject-bound config after a context-agnostic one) with no external override stays `ready`. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/core/src/flags/FlagsClient.ts | 57 +++++++++++-------- .../src/flags/__tests__/FlagsClient.test.ts | 49 ++++++++++++++++ 2 files changed, 82 insertions(+), 24 deletions(-) diff --git a/packages/core/src/flags/FlagsClient.ts b/packages/core/src/flags/FlagsClient.ts index 0f1a0725e..e9f83fcc5 100644 --- a/packages/core/src/flags/FlagsClient.ts +++ b/packages/core/src/flags/FlagsClient.ts @@ -70,6 +70,15 @@ export class FlagsClient { private clientName: string; + // The context the app explicitly set — the online fetch context, or the offline override via + // {@link setEvaluationContextWithoutFetching}. `undefined` means "no external override": an + // offline precomputed configuration is then served against its own embedded context. Tracked + // separately from {@link evaluationContext} so that adopting a configuration's embedded context + // is never mistaken for an app-set override (which would spuriously fail a later replacement). + private externalContext: EvaluationContext | undefined = undefined; + + // The effective context evaluation and exposure tracking run against: the external override + // when set, otherwise the loaded configuration's embedded context. private evaluationContext: EvaluationContext | undefined = undefined; // The servable flag cache. On the online path it holds the native-fetched flags; on the @@ -130,6 +139,7 @@ export class FlagsClient { processedContext.attributes ?? {} ); + this.externalContext = processedContext; this.evaluationContext = processedContext; this.flagsCache = new Map(Object.entries(result)); @@ -170,7 +180,7 @@ export class FlagsClient { setEvaluationContextWithoutFetching = ( context: EvaluationContext ): ConfigurationResult => { - this.evaluationContext = processEvaluationContext(context); + this.externalContext = processEvaluationContext(context); return this.reconcile(); }; @@ -185,7 +195,7 @@ export class FlagsClient { * configuration loaded the result is `PROVIDER_NOT_READY`. */ resetEvaluationContextWithoutFetching = (): ConfigurationResult => { - this.evaluationContext = undefined; + this.externalContext = undefined; return this.reconcile(); }; @@ -287,39 +297,29 @@ export class FlagsClient { const { configuration, flags } = loaded; - // Adopt the configuration's embedded context when no external context is set (implicit - // set — no native fetch). A context-agnostic configuration falls back to an empty context. - if (!this.evaluationContext) { - if (configuration.context) { - this.evaluationContext = normalizeWireContext( - configuration.context - ); - } else { - InternalLog.log( - `The provided configuration for '${this.clientName}' has no embedded context; treating it as context-agnostic.`, - SdkVerbosity.WARN - ); - this.evaluationContext = { targetingKey: '', attributes: {} }; - } - } - - // A precomputed snapshot is bound to the subject it was computed for. A non-matching - // context cannot be served (offline never fetches), so it is an error and evaluation - // serves coded defaults. The decoded snapshot is retained for a later matching context. + // A precomputed snapshot is bound to the subject it was computed for. If the app set an + // *external* context that does not match, it cannot be served (offline never fetches), so + // it is an error and evaluation serves coded defaults. Only an external override is checked + // here — the configuration's own embedded context (adopted below when no override is set) + // matches by construction, so replacing one snapshot with another for a different subject + // stays `ready`. The decoded snapshot is retained for a later matching context. if ( + this.externalContext && !contextMatchesConfiguration( configuration.context, - this.evaluationContext + this.externalContext ) ) { - // The decoded snapshot stays in `loadedConfiguration.flags`; a later matching context - // reconciles back to `ready` and repopulates `flagsCache` — no re-decode needed. return this.enterError( 'INVALID_CONTEXT', `The evaluation context does not match the precomputed configuration for '${this.clientName}'. Serving default values. Set a matching context, or use a rules-based configuration for per-context evaluation.` ); } + // Serve against the external override when set, otherwise the configuration's embedded + // context (a context-agnostic configuration falls back to an empty context). + this.evaluationContext = + this.externalContext ?? this.embeddedContext(configuration); this.flagsCache = flags; this.configurationStatus = 'ready'; this.configurationError = undefined; @@ -327,6 +327,15 @@ export class FlagsClient { return { status: 'ready' }; }; + /** The evaluation context a precomputed configuration was computed for (empty if agnostic). */ + private embeddedContext = ( + configuration: ParsedPrecomputedConfiguration + ): EvaluationContext => { + return configuration.context + ? normalizeWireContext(configuration.context) + : { targetingKey: '', attributes: {} }; + }; + /** Record an error status + message, clear the servable cache, and return the result. */ private enterError = ( errorCode: ConfigurationErrorCode, diff --git a/packages/core/src/flags/__tests__/FlagsClient.test.ts b/packages/core/src/flags/__tests__/FlagsClient.test.ts index 7668610d6..be236ed69 100644 --- a/packages/core/src/flags/__tests__/FlagsClient.test.ts +++ b/packages/core/src/flags/__tests__/FlagsClient.test.ts @@ -487,6 +487,55 @@ describe('FlagsClient', () => { expect(flagsClient.getBooleanValue('flag-b', false)).toBe(true); }); + it('replaces a snapshot for one subject with a snapshot for another (no external context)', () => { + const flagsClient = DdFlags.getClient(); + + // Load A for user-1 — no external context is set, so A's embedded context is adopted. + expect( + flagsClient.setConfiguration( + buildConfig( + { 'flag-a': offlineFlags['offline-bool'] }, + { targetingKey: 'user-1' } + ) + ) + ).toEqual({ status: 'ready' }); + + // Replace with B for a DIFFERENT subject. Because the app never set an external + // context, this must adopt B's embedded context and stay ready — not error as a + // mismatch against A's adopted context. + expect( + flagsClient.setConfiguration( + buildConfig( + { 'flag-b': offlineFlags['offline-bool'] }, + { targetingKey: 'user-2' } + ) + ) + ).toEqual({ status: 'ready' }); + expect(flagsClient.getBooleanValue('flag-b', false)).toBe(true); + expect( + flagsClient.getBooleanDetails('flag-a', false) + ).toMatchObject({ errorCode: 'FLAG_NOT_FOUND' }); + }); + + it('adopts a subject-bound snapshot loaded after a context-agnostic one (no external context)', () => { + const flagsClient = DdFlags.getClient(); + + flagsClient.setConfiguration(buildConfig(offlineFlags)); + expect(flagsClient.getBooleanValue('offline-bool', false)).toBe( + true + ); + + expect( + flagsClient.setConfiguration( + buildConfig( + { 'flag-b': offlineFlags['offline-bool'] }, + { targetingKey: 'user-2' } + ) + ) + ).toEqual({ status: 'ready' }); + expect(flagsClient.getBooleanValue('flag-b', false)).toBe(true); + }); + it('does not resurrect a prior valid snapshot after an invalid replacement', () => { const flagsClient = DdFlags.getClient(); From 68a929b0e96b41b621539aca4f1a2ef861606214 Mon Sep 17 00:00:00 2001 From: Blake Date: Tue, 21 Jul 2026 17:32:32 -0400 Subject: [PATCH 11/13] fix(flags): drop the offline overlay when entering online mode Using one FlagsClient for both the offline and online providers is unsupported, but it should degrade safely rather than serve stale data. Previously the offline overlay was cleared only after a *successful* native fetch, so a failed online fetch left the offline snapshot in place and kept serving (and tracking) it. setEvaluationContext now discards any offline overlay up front (with a warning) before fetching, so a failed fetch falls back to coded defaults instead of the stale offline snapshot. Online-only clients have no overlay, so their keep-last-known-flags-on-failure behavior is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/core/src/flags/FlagsClient.ts | 30 ++++++++++++------ .../src/flags/__tests__/FlagsClient.test.ts | 31 +++++++++++++++++++ 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/packages/core/src/flags/FlagsClient.ts b/packages/core/src/flags/FlagsClient.ts index e9f83fcc5..fdaa1391c 100644 --- a/packages/core/src/flags/FlagsClient.ts +++ b/packages/core/src/flags/FlagsClient.ts @@ -132,6 +132,24 @@ export class FlagsClient { // Make sure to process the incoming context because we don't support nested object values in context. const processedContext = processEvaluationContext(context); + // Entering online mode drops any offline overlay *before* fetching, so a failed fetch falls + // back to coded defaults rather than continuing to serve a stale offline snapshot. Using one + // client for both online and offline is unsupported (give the offline provider its own + // `clientName`), so warn when an offline configuration is discarded here. Online-only clients + // have no overlay, so this does not touch their keep-last-known-flags-on-failure behavior. + if (this.loadedConfiguration.kind !== 'none') { + InternalLog.log( + `An offline configuration was loaded for '${this.clientName}' but an online fetch was requested; discarding it and serving default values on failure. Use a separate client for the offline provider.`, + SdkVerbosity.WARN + ); + this.loadedConfiguration = { kind: 'none' }; + this.configurationStatus = 'none'; + this.configurationError = undefined; + this.flagsCache = new Map(); + this.evaluationContext = undefined; + this.externalContext = undefined; + } + try { const result = await this.nativeFlags.setEvaluationContext( this.clientName, @@ -142,17 +160,9 @@ export class FlagsClient { this.externalContext = processedContext; this.evaluationContext = processedContext; this.flagsCache = new Map(Object.entries(result)); - - // An explicit online fetch supersedes any previously loaded offline configuration. - // Online vs. offline is a property of the provider — the offline provider adopts - // context via `setEvaluationContextWithoutFetching` and never calls this method — so - // drop the offline overlay here to keep state coherent. - this.loadedConfiguration = { kind: 'none' }; - this.configurationStatus = 'none'; - this.configurationError = undefined; } catch (error) { - // NOTE: a failed fetch leaves any previously loaded offline configuration in - // place, so the client may keep serving it (and attribute exposures to its context). + // A failed fetch leaves the previous online cache in place (keep-last-known); any offline + // overlay was already dropped above, so no stale offline snapshot is served. if (error instanceof Error) { InternalLog.log( `Error setting flag evaluation context: ${error.message}`, diff --git a/packages/core/src/flags/__tests__/FlagsClient.test.ts b/packages/core/src/flags/__tests__/FlagsClient.test.ts index be236ed69..8816d57ab 100644 --- a/packages/core/src/flags/__tests__/FlagsClient.test.ts +++ b/packages/core/src/flags/__tests__/FlagsClient.test.ts @@ -598,6 +598,37 @@ describe('FlagsClient', () => { flagsClient.getBooleanValue('test-boolean-flag', false) ).toBe(true); }); + + it('drops the offline overlay when entering online mode, serving defaults if the fetch fails', async () => { + const flagsClient = DdFlags.getClient(); + flagsClient.setConfiguration( + buildConfig(offlineFlags, { targetingKey: 'user-1' }) + ); + expect(flagsClient.getBooleanValue('offline-bool', false)).toBe( + true + ); + + NativeModules.DdFlags.setEvaluationContext.mockRejectedValueOnce( + new Error('network down') + ); + await expect( + flagsClient.setEvaluationContext({ + targetingKey: 'user-1', + attributes: {} + }) + ).rejects.toThrow('network down'); + + // Using one client for both modes is unsupported: the online fetch discards the offline + // overlay up front and warns, so a failed fetch serves coded defaults rather than the + // stale offline snapshot. + expect(flagsClient.getBooleanValue('offline-bool', false)).toBe( + false + ); + expect(InternalLog.log).toHaveBeenCalledWith( + expect.stringContaining('online fetch was requested'), + expect.anything() + ); + }); }); describe('setEvaluationContextWithoutFetching', () => { From 67aa667a2992411f8d66cb7e52f406b14a49be68 Mon Sep 17 00:00:00 2001 From: Blake Date: Tue, 21 Jul 2026 17:35:17 -0400 Subject: [PATCH 12/13] fix(flags): reject structurally malformed precomputed responses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit decodePrecomputedFlags treated a null/non-object envelope or a missing `data.attributes.flags` as an empty flag map, so a JSON-valid but structurally malformed configuration decoded to an empty — but "ready" — snapshot and evaluations returned FLAG_NOT_FOUND instead of an error. Validate that `flags` is a plain object (rejecting null, arrays, and missing envelopes) and throw UnsupportedConfigurationError otherwise, so setConfiguration classifies it as GENERAL. A genuinely empty `flags: {}` is still accepted. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../__tests__/precomputed.test.ts | 20 ++++++++++++++----- .../src/flags/configuration/precomputed.ts | 20 +++++++++++++++++-- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/packages/core/src/flags/configuration/__tests__/precomputed.test.ts b/packages/core/src/flags/configuration/__tests__/precomputed.test.ts index 77a3ce93b..db8497286 100644 --- a/packages/core/src/flags/configuration/__tests__/precomputed.test.ts +++ b/packages/core/src/flags/configuration/__tests__/precomputed.test.ts @@ -225,12 +225,22 @@ describe('decodePrecomputedFlags', () => { expect(cache.get('inf')).toBeUndefined(); }); - it('returns an empty map for a structurally broken response', () => { - expect( + it.each([ + ['a null response', null], + ['a non-object response', 'nonsense'], + ['a missing data envelope', {}], + ['a missing attributes envelope', { data: {} }], + ['a missing flags map', { data: { attributes: {} } }], + ['a null flags map', { data: { attributes: { flags: null } } }], + ['an array flags map', { data: { attributes: { flags: [] } } }] + ])('throws for a structurally malformed response (%s)', (_label, input) => { + expect(() => decodePrecomputedFlags( - ({} as unknown) as Parameters[0] - ).size - ).toBe(0); + (input as unknown) as Parameters< + typeof decodePrecomputedFlags + >[0] + ) + ).toThrow(UnsupportedConfigurationError); }); it('accepts any JSON value for an object flag (array, null, primitive)', () => { diff --git a/packages/core/src/flags/configuration/precomputed.ts b/packages/core/src/flags/configuration/precomputed.ts index 49bba1d58..e9a453006 100644 --- a/packages/core/src/flags/configuration/precomputed.ts +++ b/packages/core/src/flags/configuration/precomputed.ts @@ -36,7 +36,8 @@ export class UnsupportedConfigurationError extends Error { * booleans, `String(...)` otherwise) because native Android exposure tracking * rebuilds the flag from the string form. * - * @throws {UnsupportedConfigurationError} if the response is obfuscated. + * @throws {UnsupportedConfigurationError} if the response is obfuscated, or its envelope is + * structurally malformed (missing/non-object `data.attributes.flags`). */ export const decodePrecomputedFlags = ( response: PrecomputedConfigurationResponse @@ -53,7 +54,17 @@ export const decodePrecomputedFlags = ( ); } - const flags = attributes?.flags ?? {}; + // Validate the response envelope before trusting it. An untrusted wire can be JSON-valid yet + // structurally malformed (a null/non-object envelope, or a `flags` that is null or an array). + // Fail predictably so the caller classifies it as an error instead of silently decoding it to + // an empty — but "ready" — configuration. A genuinely empty `flags: {}` is still accepted. + const flags = attributes?.flags; + if (!isFlagsMap(flags)) { + throw new UnsupportedConfigurationError( + "Malformed precomputed configuration: 'data.attributes.flags' must be an object." + ); + } + // A Map (returned as-is) so a pathological flag keyed "__proto__" is stored as data // and later looked up via `.get()` — never hitting the `Object.prototype` "__proto__" // setter (on write) or the prototype chain (on read) the way a plain object would. @@ -69,6 +80,11 @@ export const decodePrecomputedFlags = ( return cache; }; +// A well-formed flags container is a plain object (a possibly-empty map of flag key -> flag). +// `null`, arrays, and primitives are malformed envelopes. +const isFlagsMap = (value: unknown): value is Record => + typeof value === 'object' && value !== null && !Array.isArray(value); + const toFlagCacheEntry = ( key: string, flag: unknown From f7e019ff9008468b8cdc12820f91a5aa6f0c58a7 Mon Sep 17 00:00:00 2001 From: Blake Date: Wed, 22 Jul 2026 10:14:36 -0400 Subject: [PATCH 13/13] test(flags): assert exposure attribution on replacement + malformed envelope - After replacing snapshot A with B (different subject, no external context), assert trackEvaluation is called with B's embedded targetingKey, so a regression that updated the flags but kept A's context would be caught. - Add a public-boundary test: a wire whose precomputed response omits `data.attributes.flags` reconciles to GENERAL and serves coded defaults. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/flags/__tests__/FlagsClient.test.ts | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/packages/core/src/flags/__tests__/FlagsClient.test.ts b/packages/core/src/flags/__tests__/FlagsClient.test.ts index 8816d57ab..87e2bb575 100644 --- a/packages/core/src/flags/__tests__/FlagsClient.test.ts +++ b/packages/core/src/flags/__tests__/FlagsClient.test.ts @@ -450,6 +450,28 @@ describe('FlagsClient', () => { ).toMatchObject({ errorCode: 'GENERAL' }); }); + it('errors with GENERAL for a structurally malformed response envelope', () => { + const flagsClient = DdFlags.getClient(); + + // A wire whose precomputed response omits `data.attributes.flags` entirely. + const wire = JSON.stringify({ + version: 1, + precomputed: { + response: JSON.stringify({ data: { attributes: {} } }), + context: { targetingKey: 'user-1' } + } + }); + + const result = flagsClient.setConfiguration( + configurationFromString(wire) + ); + + expect(result).toEqual({ status: 'error', errorCode: 'GENERAL' }); + expect( + flagsClient.getBooleanDetails('offline-bool', false) + ).toMatchObject({ value: false, errorCode: 'GENERAL' }); + }); + it('serves a context-agnostic configuration (no embedded context)', () => { const flagsClient = DdFlags.getClient(); @@ -515,6 +537,16 @@ describe('FlagsClient', () => { expect( flagsClient.getBooleanDetails('flag-a', false) ).toMatchObject({ errorCode: 'FLAG_NOT_FOUND' }); + + // The exposure must be attributed to B's embedded context (user-2), not A's — a + // regression that updated the flags but kept A's context would misattribute here. + expect(NativeModules.DdFlags.trackEvaluation).toHaveBeenCalledWith( + 'default', + 'flag-b', + expect.any(Object), + 'user-2', + {} + ); }); it('adopts a subject-bound snapshot loaded after a context-agnostic one (no external context)', () => {